[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-iio
Subject:    Re: [PATCH v1] iio: pressure: ms5611: ms5611_prom_is_valid false negative bug
From:       Jonathan Cameron <jic23 () kernel ! org>
Date:       2023-09-30 17:37:48
Message-ID: 20230930183748.2f20a00a () jic23-huawei
[Download RAW message or body]

On Wed, 20 Sep 2023 10:01:10 +1000
Alexander Zangerl <az@breathe-safe.com> wrote:

> the ms5611 driver falsely rejects lots of MS5607-02BA03-50 chips
> with "PROM integrity check failed" because it doesn't accept a prom crc
> value of zero as legitimate.
> 
> according to the datasheet for this chip (and the manufacturer's
> application note about the prom crc), none of the possible
> values for the crc are excluded - but the current code
> in ms5611_prom_is_valid() ends with
> 
> return crc_orig != 0x0000 && crc == crc_orig
> 
> i've discussed this with the original author of the driver (tomasz
> duszynski) and he indicated that at that time (2015) he was dealing with
> some faulty chip samples which returned blank data under some
> circumstances and/or followed example code which indicated crc zero
> being bad (i can't find any traces of any such online, however).
> 
> as far as i can tell this exception should not be applied anymore; we've
> got a few hundred custom boards here with this chip where large numbers
> of the prom have a legitimate crc value 0, and do work fine, but which the
> current driver code wrongly rejects.
> 
> Signed-off-by: Alexander Zangerl <az@breathe-safe.com>
> Fixes: c0644160a8b5 ("iio: pressure: add support for MS5611 pressure and temperature sensor")

Applied with some tweaks to the patch description to shorten it
a little whilst keeping all the information.  Also marked for stable inclusion.

Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan

> ---
>  drivers/iio/pressure/ms5611_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
> index 5c7a734ede54..9980c6f3335e 100644
> --- a/drivers/iio/pressure/ms5611_core.c
> +++ b/drivers/iio/pressure/ms5611_core.c
> @@ -79,7 +79,7 @@ static bool ms5611_prom_is_valid(u16 *prom, size_t len)
>  
>  	crc = (crc >> 12) & 0x000F;
>  
> -	return crc_orig != 0x0000 && crc == crc_orig;
> +	return crc == crc_orig;
>  }
>  
>  static int ms5611_read_prom(struct iio_dev *indio_dev)

[prev in list] [next in list] [prev in thread] [next in thread]