[PATCH 3/5] usb: ehci-generic: Use regulator_set_enable_if_allowed

[Jonas Karlman]

On 2023-08-12 11:20, Svyatoslav Ryhel wrote:
> пт, 11 серп. 2023 р. о 22:55 Jonas Karlman <jonas at kwiboo.se> пише:
>>
>> On 2023-08-11 21:00, Marek Vasut wrote:
>>> On 7/19/23 23:20, Jonas Karlman wrote:
>>>> With the commit 4fcba5d556b4 ("regulator: implement basic reference
>>>> counter") the return value of regulator_set_enable may be EALREADY or
>>>> EBUSY for fixed/gpio regulators.
>>>>
>>>> Change to use the more relaxed regulator_set_enable_if_allowed to
>>>> continue if regulator already was enabled or disabled.
>>>>
>>>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
>>>> ---
>>>>   drivers/usb/host/ehci-generic.c | 23 +++++++++++------------
>>>>   1 file changed, 11 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-generic.c
>>>> index a765a307a323..936e30438d9f 100644
>>>> --- a/drivers/usb/host/ehci-generic.c
>>>> +++ b/drivers/usb/host/ehci-generic.c
>>>> @@ -39,14 +39,10 @@ static int ehci_enable_vbus_supply(struct udevice *dev)
>>>>      if (ret && ret != -ENOENT)
>>>>              return ret;
>>>>
>>>> -    if (priv->vbus_supply) {
>>>> -            ret = regulator_set_enable(priv->vbus_supply, true);
>>>> -            if (ret) {
>>>> -                    dev_err(dev, "Error enabling VBUS supply (ret=%d)\n", ret);
>>>> -                    return ret;
>>>> -            }
>>>> -    } else {
>>>> -            dev_dbg(dev, "No vbus supply\n");
>>>> +    ret = regulator_set_enable_if_allowed(priv->vbus_supply, true);
>>>> +    if (ret && ret != -ENOSYS) {
>>>
>>> Same comment as 2/5 -- the priv->vbus_supply check is likely mandatory.
>>
>> It is not, I made a note of it in the cover letter.
>>
>> """
>> The regulator_set_enable_if_allowed function is more relaxed and will
>> return ENOSYS if the provided regulator is NULL or when DM_REGULATOR
>> was disabled. Using the following call convention should be safe:
>>
>>   ret = regulator_set_enable_if_allowed(<supply>, <true|false>);
>>   if (ret && ret != -ENOSYS)
> 
> Is ENOSYS check mandatory, regulator_set_enable_if_allowed should never
> return ENOSYS ever.

Yes, this is mandatory unless you have a wrapping/prior check for
CONFIG_IS_ENABLED(DM_REGULATOR).

So 'ret && ret != -ENOSYS' is safest and also works with DM_REGULATOR
disabled.

https://source.denx.de/u-boot/u-boot/-/blob/master/include/power/regulator.h#L606

Regards,
Jonas

> 
> https://github.com/u-boot/u-boot/blob/master/drivers/power/regulator/regulator-uclass.c#L198
> 
>>           return ret;
>> """
>>
>> Regards,
>> Jonas