Hi Jonathan, Jacopo,
On Mon, Aug 5, 2019 at 7:15 PM Jonathan Cameron jic23@kernel.org wrote:
On Mon, 5 Aug 2019 17:55:15 +0200 Jacopo Mondi jacopo+renesas@jmondi.org wrote:
The max9611 driver reads the die temperature at probe time to validate the communication channel. Use the actual read value to perform the test instead of the read function return value, which was mistakenly used so far.
The temperature reading test was only successful because the 0 return value is in the range of supported temperatures.
Fixes: 69780a3bbc0b ("iio: adc: Add Maxim max9611 ADC driver") Signed-off-by: Jacopo Mondi jacopo+renesas@jmondi.org
Applied to the fixes-togreg branch of iio.git and marked for stable. That'll be a bit fiddly given other changes around this so we may need to do backports.
This is now commit b9ddd5091160793e ("iio: adc: max9611: Fix temperature reading in probe") in v5.3-rc5, and has been backported to 4.14, 4.19, and 5.2.
--- a/drivers/iio/adc/max9611.c +++ b/drivers/iio/adc/max9611.c @@ -480,7 +480,7 @@ static int max9611_init(struct max9611_dev *max9611) if (ret) return ret;
regval = ret & MAX9611_TEMP_MASK;
regval &= MAX9611_TEMP_MASK; if ((regval > MAX9611_TEMP_MAX_POS && regval < MAX9611_TEMP_MIN_NEG) ||
While this did fix a bug, it also introduced a regression: on Salvator-XS, which has two max9611 instances, I now see intermittent failures
max9611 4-007c: Invalid value received from ADC 0x8000: aborting max9611: probe of 4-007c failed with error -5
and/or
max9611 4-007f: Invalid value received from ADC 0x8000: aborting max9611: probe of 4-007f failed with error -5
during boot.
Retrying on failure fixes the issue, e.g.:
max9611_init:483: regval = 0x8000 max9611 4-007f: Invalid value received from ADC 0x8000: aborting max9611_init:483: regval = 0x2780
According to the datasheet, 0x8000 is the Power-On Reset value. Looks like it should be ignored, and retried?
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert
On Wed, Aug 21, 2019 at 01:28:16PM +0200, Geert Uytterhoeven wrote:
Hi Jonathan, Jacopo,
On Mon, Aug 5, 2019 at 7:15 PM Jonathan Cameron jic23@kernel.org wrote:
On Mon, 5 Aug 2019 17:55:15 +0200 Jacopo Mondi jacopo+renesas@jmondi.org wrote:
The max9611 driver reads the die temperature at probe time to validate the communication channel. Use the actual read value to perform the test instead of the read function return value, which was mistakenly used so far.
The temperature reading test was only successful because the 0 return value is in the range of supported temperatures.
Fixes: 69780a3bbc0b ("iio: adc: Add Maxim max9611 ADC driver") Signed-off-by: Jacopo Mondi jacopo+renesas@jmondi.org
Applied to the fixes-togreg branch of iio.git and marked for stable. That'll be a bit fiddly given other changes around this so we may need to do backports.
This is now commit b9ddd5091160793e ("iio: adc: max9611: Fix temperature reading in probe") in v5.3-rc5, and has been backported to 4.14, 4.19, and 5.2.
--- a/drivers/iio/adc/max9611.c +++ b/drivers/iio/adc/max9611.c @@ -480,7 +480,7 @@ static int max9611_init(struct max9611_dev *max9611) if (ret) return ret;
regval = ret & MAX9611_TEMP_MASK;
regval &= MAX9611_TEMP_MASK; if ((regval > MAX9611_TEMP_MAX_POS && regval < MAX9611_TEMP_MIN_NEG) ||While this did fix a bug, it also introduced a regression: on Salvator-XS, which has two max9611 instances, I now see intermittent failures
max9611 4-007c: Invalid value received from ADC 0x8000: aborting max9611: probe of 4-007c failed with error -5and/or
max9611 4-007f: Invalid value received from ADC 0x8000: aborting max9611: probe of 4-007f failed with error -5during boot.
AH! I didn't notice! I booted the board a few times only, maybe it didn't trigger (it was a Salvator-X H3, not an XS, but it shouldn't make any difference).
Retrying on failure fixes the issue, e.g.:
max9611_init:483: regval = 0x8000 max9611 4-007f: Invalid value received from ADC 0x8000: aborting max9611_init:483: regval = 0x2780According to the datasheet, 0x8000 is the Power-On Reset value. Looks like it should be ignored, and retried?
Indeed... I haven't found a characterization of the delay required to release registers from their POR values after power up, so I guess we could read the register value again with a little timeout between reads (whose value would be arbitrary, anyway..)
I'm a bit suprised though.. The max9611 chips are powered from the +3.3V rail, and should have exited POR long before the driver gets to probe, isn't it?
Thanks for reporting and sorry for having missed it in first place
Gr{oetje,eeting}s,
Geert-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
linux-stable-mirror@lists.linaro.org
-
Geert Uytterhoeven -
Jacopo Mondi