On 2023-08-12 11:20, Svyatoslav Ryhel wrote:
> пт, 11 серп. 2023 р. о 22:55 Jonas Karlman <jo...@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 <jo...@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
