Re: iio: pressure: ms5611: ms5611_prom_is_valid false negative bug [PATCH]

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Mon, 18 Sep 2023 09:52:20 +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
> 
> datasheet: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=srchrtrv&DocNm=MS5607-02BA03&DocType=Data+Sheet&DocLang=English
> application note: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=srchrtrv&DocNm=AN520_C-code_example_for_MS56xx&DocType=SS&DocLang=EN
> 
> i've discussed this whith 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.
> 
> (i can provide some example prom dumps if required.)
> 
> the attached tiny patch is against 4.19. but that
> section of the code is unchanged up to and including 6.6. 
> 
> 

> From 653b5cf063e07d126e67386b152e4e76d4f8c1dc Mon Sep 17 00:00:00 2001
> From: Alexander Zangerl <az@breathe-safe.com>
> Date: Mon, 18 Sep 2023 09:44:00 +1000
> Subject: [PATCH] ms5611: crc zero is valid and should not be rejected

Please resend with this formatting as a patch, so the above should be in a cover
letter with this sent in reply to that.  git format-patch --cover-letter 
will set it it up right.

Needs a
Signed-off-by and Fixes tag as well before I can take this.

Otherwise, description all makes sense so good to get this fix in place.

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)
> -- 
> 2.30.2
> 
> 
> 
> Best Regards,
> Alexander Zangerl
> IT Engineer
> 
> -- 
> P +61 7 3276 7833 | M +61 415 482 341
> E az@breathe-safe.com | W breathe-safe.com.au
> A 62 Mica Street, Carole Park, 4300, QLD