[U-Boot] [PATCH v2 5/9] arm: exynos: Wait till ADC stabilizes before checking Odroid HC1 revision

Lukasz Majewski
2019-02-17T22:31:22Z

Hi Krzysztof,

> On Fri, Feb 15, 2019 at 08:15:45AM +0100, Lukasz Majewski wrote:
> > On Wed, 13 Feb 2019 17:46:44 +0100
> > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > > Fix detection of Odroid HC1 (Exynos5422) after reboot if kernel
> > > disabled the LDO4/VDD_ADC regulator.
> > >
> > > The LDO4 supplies both ADC block and the ADC input AIN9. Voltage
> > > on AIN9 will rise slowly, so be patient and wait for it to
> > > stabilize.
> > >
> > > First reads on Odroid HC1 return 305, 1207, 1297 and finally 1308
> > > (reference value is 1309).
> > >
> > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > > ---
> > > board/samsung/common/exynos5-dt-types.c | 38
> > > ++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1
> > > deletion(-)
> > >
> > > diff --git a/board/samsung/common/exynos5-dt-types.c
> > > b/board/samsung/common/exynos5-dt-types.c index
> > > af711e727a78..8aed64183837 100644 ---
> > > a/board/samsung/common/exynos5-dt-types.c +++
> > > b/board/samsung/common/exynos5-dt-types.c @@ -57,12 +57,48 @@
> > > static unsigned int odroid_get_rev(void) return 0;
> > > }
> > >
> > > +/*
> > > + * Read ADC at least twice and check the resuls. If regulator
> > > providing voltage
> > > + * on to measured point was just turned on, first reads might
> > > require time
> > > + * to stabilize.
> > > + */
> > > +static int odroid_get_adc_val(unsigned int *adcval)
> > > +{
> > > + unsigned int adcval_prev = 0;
> > > + int ret, retries = 20;
> > > +
> > > + ret = adc_channel_single_shot("adc",
> > > CONFIG_ODROID_REV_AIN,
> > > + &adcval_prev);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + while (retries--) {
> > > + mdelay(5);
> > > +
> > > + ret = adc_channel_single_shot("adc",
> > > CONFIG_ODROID_REV_AIN,
> > > + adcval);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + /*
> > > + * If difference between ADC reads is less than
> > > 3%,
> > > + * accept the result
> > > + */
> > > + if ((100 * abs(*adcval - adcval_prev) /
> > > adcval_prev) < 3)
> > > + return ret;
> > > +
> > > + adcval_prev = *adcval;
> > > + }
> >
> > Is there in the documentation any required time to wait before
> > reading the ADC value?
> >
> > If yes then maybe get_timer() based approach shall be used (if
> > get_timer() is available in this context)?
> >
> > Please see for example drivers/net/fec_mxc.c for how timeouts are
> > handled there.
>
> I must admit that I do not see benefit of timers...
> 1. Make code slightly more complicated (instead of simple retries and
> mdelay()).
> 2. Introduce no delay by itself so the ADC reads happen one after
> another. Probing ADC value fast does not work with my approach
> of waiting till the values get closer to each other...
>
> With timer-based approach, without delay I got:
> ADC = 660
> ADC = 887
> ADC = 1031
> ADC = 1125
> ADC = 1186
> ADC = 1226
> ADC = 1253
> return - because the difference is too small (<3 %).
>
> I could narrow my threshold to 1%... but then:
> ADC = 651
> ADC = 881
> ADC = 1027
> ADC = 1122
> ADC = 1184
> ADC = 1225
> ADC = 1253
> ADC = 1271
> ADC = 1284
> ADC = 1292
>
> I prefer simpler method with delay.

Ok. I see.

Just please add proper comment /or patch description.

Thanks for your effort :-)

>
> Best regards,
> Krzysztof
>

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190217/75ba1805/attachment.sig>