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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 7 Jan 2020 23:02:05 +0100
From:   Pavel Machek <pavel@...x.de>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Jacopo Mondi <jacopo+renesas@...ndi.org>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 4.19 006/115] iio: adc: max9611: Fix too short conversion
 time delay

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

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)