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

[Lukasz Majewski <lukma@denx.de>]

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>