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

["Jaehoon Chung" <jh80.chung@samsung.com>]

Hi,

> -----Original Message-----
> From: Jonas Karlman <jonas@kwiboo.se>
> Sent: Wednesday, November 1, 2023 6:33 PM
> To: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: sjg@chromium.org; patrice.chotard@foss.st.com; eugen.hristev@collabora.com; trini@konsulko.com; u-
> boot@lists.denx.de
> Subject: Re: [PATCH] power: regulator: Fix an handling error about EALREADY
> 
> On 2023-11-01 09:19, Jaehoon Chung wrote:
> >
> >
> >> -----Original Message-----
> >> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Jaehoon Chung
> >> Sent: Wednesday, November 1, 2023 5:11 PM
> >> To: 'Jonas Karlman' <jonas@kwiboo.se>
> >> Cc: sjg@chromium.org; patrice.chotard@foss.st.com; eugen.hristev@collabora.com; trini@konsulko.com;
> u-
> >> boot@lists.denx.de
> >> Subject: RE: [PATCH] power: regulator: Fix an handling error about EALREADY
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Jaehoon Chung
> >>> Sent: Wednesday, November 1, 2023 5:07 PM
> >>> To: 'Jonas Karlman' <jonas@kwiboo.se>
> >>> Cc: sjg@chromium.org; patrice.chotard@foss.st.com; eugen.hristev@collabora.com; trini@konsulko.com;
> >> u-
> >>> boot@lists.denx.de
> >>> Subject: RE: [PATCH] power: regulator: Fix an handling error about EALREADY
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Jonas Karlman <jonas@kwiboo.se>
> >>>> Sent: Wednesday, November 1, 2023 4:47 PM
> >>>> To: Jaehoon Chung <jh80.chung@samsung.com>
> >>>> Cc: sjg@chromium.org; patrice.chotard@foss.st.com; eugen.hristev@collabora.com;
> trini@konsulko.com;
> >>> u-
> >>>> boot@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@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.

Ok. I understood.

> 
> 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://protect2.fireeye.com/v1/url?k=d568c889-b4136201-d56943c6-74fe4860018a-
> 0f3e262ba2f98d29&q=1&e=fab7227e-c990-4824-b601-
> b8ba154d2255&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fcover%2F1810049%2F

Thanks for informing this.
Then discard this patch. It seems that make sense to fix meson usb. How about?
I will send the patch about meson-usb.


Best Regards,
Jaehoon Chung

> 
> 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;
> >>>>>  }
> >>>>>
> >>>
> >>
> >
> >