Re: [PATCH 4.19 006/115] iio: adc: max9611: Fix too short conversion time delay

Pavel Machek
2020-01-07T22:02:08Z

[-- Attachment #1: Type: text/plain, Size: 1701 bytes --]

Hi!

>
> Fix this by increasing the delay from 1000-2000 µs to 3000-3300 µs.
>
> Note that this issue has always been present, but it was exposed by the
> aformentioned commit.

> +++ b/drivers/iio/adc/max9611.c
> @@ -92,6 +92,12 @@
> #define MAX9611_TEMP_SCALE_NUM 1000000
> #define MAX9611_TEMP_SCALE_DIV 2083
>
> +/*
> + * Conversion time is 2 ms (typically) at Ta=25 degreeC
> + * No maximum value is known, so play it safe.
> + */
> +#define MAX9611_CONV_TIME_US_RANGE 3000, 3300
> +
> struct max9611_dev {
> struct device *dev;
> struct i2c_client *i2c_client;

This is evil. It looks like a constant, but it is two
constants. Just... don't do this.

What about

static inline usleep_conversion(void) { usleep_range(3000, 3300); }

? (Plus, normally we use bigger ranges to make the job of highres
infrastructure easier. 3 to 6ms would be typical.)..

Best regards,
Pavel

> - /*
> - * need a delay here to make register configuration
> - * stabilize. 1 msec at least, from empirical testing.
> - */
> - usleep_range(1000, 2000);
> + /* need a delay here to make register configuration stabilize. */
> +
> + usleep_range(MAX9611_CONV_TIME_US_RANGE);
>
> ret = i2c_smbus_read_word_swapped(max9611->i2c_client, reg_addr);
> if (ret < 0) {
> @@ -510,7 +514,7 @@ static int max9611_init(struct max9611_dev *max9611)
> MAX9611_REG_CTRL2, 0);
> return ret;
> }
> - usleep_range(1000, 2000);
> + usleep_range(MAX9611_CONV_TIME_US_RANGE);
>
> return 0;
> }

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]