[PATCH] power: regulator: Fix an handling error about EALREADY

[Jonas Karlman]

On 2023-11-01 09:19, Jaehoon Chung wrote:
> 
> 
>> -----Original Message-----
>> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Jaehoon Chung
>> Sent: Wednesday, November 1, 2023 5:11 PM
>> To: 'Jonas Karlman' <jonas at kwiboo.se>
>> Cc: sjg at chromium.org; patrice.chotard at foss.st.com; eugen.hristev at collabora.com; trini at konsulko.com; u-
>> boot at lists.denx.de
>> Subject: RE: [PATCH] power: regulator: Fix an handling error about EALREADY
>>
>>
>>
>>> -----Original Message-----
>>> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Jaehoon Chung
>>> Sent: Wednesday, November 1, 2023 5:07 PM
>>> To: 'Jonas Karlman' <jonas at kwiboo.se>
>>> Cc: sjg at chromium.org; patrice.chotard at foss.st.com; eugen.hristev at collabora.com; trini at konsulko.com;
>> u-
>>> boot at lists.denx.de
>>> Subject: RE: [PATCH] power: regulator: Fix an handling error about EALREADY
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Jonas Karlman <jonas at kwiboo.se>
>>>> Sent: Wednesday, November 1, 2023 4:47 PM
>>>> To: Jaehoon Chung <jh80.chung at samsung.com>
>>>> Cc: sjg at chromium.org; patrice.chotard at foss.st.com; eugen.hristev at collabora.com; trini at konsulko.com;
>>> u-
>>>> boot at lists.denx.de
>>>> Subject: Re: [PATCH] power: regulator: Fix an handling error about EALREADY
>>>>
>>>> On 2023-11-01 08:23, Jaehoon Chung wrote:
>>>>> If reegulator is already enabled, it will be return to EALREADY.
>>>>> But driver that its function is called can notice as error, even though
>>>>> it's working fine.
>>>>>
>>>>> Fixes: 4fcba5d556b ("regulator: implement basic reference counter")
>>>>>
>>>>> Signed-off-by: Jaehoon Chung <jh80.chung at samsung.com>
>>>>> ---
>>>>>  drivers/power/regulator/regulator-uclass.c | 4 ++++
>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-
>>> uclass.c
>>>>> index 3a6ba69f6d5f..fc1c3eb93c9d 100644
>>>>> --- a/drivers/power/regulator/regulator-uclass.c
>>>>> +++ b/drivers/power/regulator/regulator-uclass.c
>>>>> @@ -187,6 +187,10 @@ int regulator_set_enable(struct udevice *dev, bool enable)
>>>>>  		}
>>>>>  	}
>>>>>
>>>>> +	/* Regulator is already enabled */
>>>>> +	if (ret == -EALREADY)
>>>>> +		return 0;
>>>>> +
>>>>
>>>> Use of regulator_set_enable_if_allowed() will cover this error,
>>>> and regulator_set_enable() should continue to return this error.
> 
> regulator_set_enable_if_allowed() can be covered. But regulator_set_enable is called in some drivers.
> You means that it needs to replace to regulator_set_enable_if_allowed() from regulator_set_enable() about all driver?

I think that was the consensus when basic reference counter was
implemented for gpio/fixed regulators. It should return -EALREADY when
a regulator is already enabled, and if a caller need a more relaxed
enable/disable regulator_set_enable_if_allowed() could be used.

Tried to fix a few commons callers in following series, but did not
cover hw I could not test. Can see that I missed the meson usb driver
and was something I could have tested.

Keep fixed/gpio regulator enable count in balance
https://patchwork.ozlabs.org/cover/1810049/

Regards,
Jonas

> 
> Best Regards,
> Jaehoon Chung
> 
>>
>> Well.. I will recheck about your comment.
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> When I have checked on my target, It seems that it can't cover all cases.
>>>
>>> On odroid-c4, USB doesn't work, even though its regulator is enabled.
>>>
>>> => ums 0 mmc 0
>>> UMS: LUN 0, dev mmc 0, hwpart 0, sector 0x0, count 0x1dacc00
>>> No USB device found
>>> Couldn't init USB controller.
>>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>>
>>>> Regards,
>>>> Jonas
>>>>
>>>>>  	return ret;
>>>>>  }
>>>>>
>>>
>>
> 
>