On Sun, 9 Jul 2017 20:24:49 +0200
Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> Hi,
>
> On 09-07-17 20:22, Jonathan Cameron wrote:
> > On Sat, 8 Jul 2017 15:11:57 +0200
> > Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> >
> >> I noticed in its DSDT that one of my tablets actually is using the GPADC
> >> pin for temperature monitoring.
> >>
> >> The whole axp288_adc_set_ts() function is a bit weird, in the past it was
> >> removed because it seems to make no sense, then this was reverted because
> >> of regressions.
> >>
> >> So I decided to test the special GPADC pin handling on this tablet.
> >> Conclusion: not only is axp288_adc_set_ts() necessary, we need to sleep a
> >> bit after making the AXP288_ADC_TS_PIN_CTRL changes before sampling the
> >> GPADC, otherwise it will often (about 80% of the time) read 0 instead of
> >> its actual value.
> >>
> >> It seems that there is only 1 bias current source and to be able to use it
> >> for the GPIO0 pin in GPADC mode it must be temporarily turned off for the
> >> TS pin, but the datasheet does not mention this.
> >>
> >> This commit adds a sleep after disabling the TS pin bias current,
> >> fixing the GPADC more often then not wrongly returning 0.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> > Hans,
> >
> > This is sufficiently weird that I want your opinion on what
> > to do with this. Are we looking at a necessary fix for a hardware
> > platform that we should push out asap and mark for stable?
> >
> > Or is this a bit of an odd corner case where a slower path makes
> > more sense?
>
> Since we have had this bug for ages and no-one has noticed I think
> taking this a bit slower is fine. Would be nice to get it into 4.13,
> but no need for a Cc stable IMHO.
>
> Thanks & Regards,
>
> Hans
Thanks,
Applied to the fixes-togreg branch of iio.git.
It may still get picked up for stable, but I'm not going to explicitly
push it for such.
Jonathan
>
>
> >
> > I'm happy either way.
> >
> > Jonathan
> >> ---
> >> drivers/iio/adc/axp288_adc.c | 12 +++++++++++-
> >> 1 file changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/iio/adc/axp288_adc.c b/drivers/iio/adc/axp288_adc.c
> >> index 7fd24949c0c1..462a99c13e7a 100644
> >> --- a/drivers/iio/adc/axp288_adc.c
> >> +++ b/drivers/iio/adc/axp288_adc.c
> >> @@ -126,11 +126,21 @@ static int axp288_adc_read_channel(int *val, unsigned long address,
> >> static int axp288_adc_set_ts(struct regmap *regmap, unsigned int mode,
> >> unsigned long address)
> >> {
> >> + int ret;
> >> +
> >> /* channels other than GPADC do not need to switch TS pin */
> >> if (address != AXP288_GP_ADC_H)
> >> return 0;
> >>
> >> - return regmap_write(regmap, AXP288_ADC_TS_PIN_CTRL, mode);
> >> + ret = regmap_write(regmap, AXP288_ADC_TS_PIN_CTRL, mode);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + /* When switching to the GPADC pin give things some time to settle */
> >> + if (mode == AXP288_ADC_TS_PIN_GPADC)
> >> + usleep_range(6000, 10000);
> >> +
> >> + return 0;
> >> }
> >>
> >> static int axp288_adc_read_raw(struct iio_dev *indio_dev,
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
