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

[Pavel Machek]

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: signature.asc
Description: Digital signature