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

[Krzysztof Kozlowski]

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 at 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 at 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.

Best regards,
Krzysztof