[U-Boot] [PATCH 2/6] dm: adc: uclass: get reference regulator once

[Fabrice Gasnier]

On 07/24/2018 01:48 AM, Simon Glass wrote:
> Hi Fabrice,
> 
> On 23 July 2018 at 06:35, Fabrice Gasnier <fabrice.gasnier at st.com> wrote:
>> device_get_supply_regulator() only needs to be called once.
>> But each time there's call to adc_vxx_value() for instance, it calls
>> adc_vxx_platdata_update() -> device_get_supply_regulator().
>>
>> This also allows vdd_supply/vss_supply to be provided directly from
>> uc_pdata, e.g dt-binding variant like stm32-adc provide its own
>> 'vref-supply'.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier at st.com>
>> ---
>>
>>  drivers/adc/adc-uclass.c | 21 +++++++++++++--------
>>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> The original code doesn't look right to me.
> 
> Reading from the DT should happen in the ofdata_to_platdata() method,
> except (as here) where we need to probe another device, iwc we can use
> the probe() method.
> 
> So can you move this code into a new probe() method, so it just
> happens once, when the device is probed?

Hi Simon,

I just sent an updated version of this patchset. I agree with you, so I
moved device_get_supply_regulator() call to pre_probe().

But I get confused about "a new probe()" as you mention above.
So, please take a look at v2, and tell me if this change is in line with
your view.

Many thanks for reviewing,
Best Regards,
Fabrice

> 
>>
>> diff --git a/drivers/adc/adc-uclass.c b/drivers/adc/adc-uclass.c
>> index 17c1a4e..70f4cde 100644
>> --- a/drivers/adc/adc-uclass.c
>> +++ b/drivers/adc/adc-uclass.c
>> @@ -264,10 +264,13 @@ static int adc_vdd_platdata_update(struct udevice *dev)
>>          * will bind before its supply regulator device, then the below 'get'
>>          * will return an error.
>>          */
>> -       ret = device_get_supply_regulator(dev, "vdd-supply",
>> -                                         &uc_pdata->vdd_supply);
>> -       if (ret)
>> -               return ret;
>> +       if (!uc_pdata->vdd_supply) {
>> +               /* Only get vdd_supply once */
>> +               ret = device_get_supply_regulator(dev, "vdd-supply",
>> +                                                 &uc_pdata->vdd_supply);
>> +               if (ret)
>> +                       return ret;
>> +       }
>>
>>         ret = regulator_get_value(uc_pdata->vdd_supply);
>>         if (ret < 0)
>> @@ -283,10 +286,12 @@ static int adc_vss_platdata_update(struct udevice *dev)
>>         struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
>>         int ret;
>>
>> -       ret = device_get_supply_regulator(dev, "vss-supply",
>> -                                         &uc_pdata->vss_supply);
>> -       if (ret)
>> -               return ret;
>> +       if (!uc_pdata->vss_supply) {
>> +               ret = device_get_supply_regulator(dev, "vss-supply",
>> +                                                 &uc_pdata->vss_supply);
>> +               if (ret)
>> +                       return ret;
>> +       }
>>
>>         ret = regulator_get_value(uc_pdata->vss_supply);
>>         if (ret < 0)
>> --
>> 1.9.1
>>
> 
> Regards,
> Simon
>