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

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

[Jaehoon Chung]

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;
+
 	return ret;
 }
 
-- 
2.25.1

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

[Jonas Karlman]

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.

Regards,
Jonas

>  	return ret;
>  }
>  

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

[Jaehoon Chung]

> -----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.

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

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

[Jaehoon Chung]

> -----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.

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

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

[Jaehoon Chung]

> -----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?

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

Re: [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@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.

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

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

[Jaehoon Chung]

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