[PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[Eugen Hristev]

regulator_set_enable_if_allowed already handles cases when the
regulator is already enabled, or already disabled, and does not
treat these as errors.
With this change, the driver can work correctly even if the regulator
is already taken or already disabled by another consumer.

Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
---
Hi Tim,

I have not tested this as I do not have a mx6 board. can you please
try it ?

Thanks,
Eugen

 drivers/usb/host/ehci-mx6.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 91633f013a55..7806a4de71e0 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -480,9 +480,9 @@ static int mx6_init_after_reset(struct ehci_ctrl *dev)
 #if CONFIG_IS_ENABLED(DM_REGULATOR)
 	if (priv->vbus_supply) {
 		int ret;
-		ret = regulator_set_enable(priv->vbus_supply,
-					   (type == USB_INIT_DEVICE) ?
-					   false : true);
+		ret = regulator_set_enable_if_allowed(priv->vbus_supply,
+						      (type == USB_INIT_DEVICE) ?
+						      false : true);
 		if (ret && ret != -ENOSYS) {
 			printf("Error enabling VBUS supply (ret=%i)\n", ret);
 			return ret;
@@ -707,9 +707,9 @@ static int ehci_usb_probe(struct udevice *dev)
 
 #if CONFIG_IS_ENABLED(DM_REGULATOR)
 	if (priv->vbus_supply) {
-		ret = regulator_set_enable(priv->vbus_supply,
-					   (type == USB_INIT_DEVICE) ?
-					   false : true);
+		ret = regulator_set_enable_if_allowed(priv->vbus_supply,
+						      (type == USB_INIT_DEVICE) ?
+						      false : true);
 		if (ret && ret != -ENOSYS) {
 			printf("Error enabling VBUS supply (ret=%i)\n", ret);
 			goto err_clk;
@@ -748,7 +748,7 @@ err_regulator:
 #endif
 #if CONFIG_IS_ENABLED(DM_REGULATOR)
 	if (priv->vbus_supply)
-		regulator_set_enable(priv->vbus_supply, false);
+		regulator_set_enable_if_allowed(priv->vbus_supply, false);
 #endif
 err_clk:
 #if CONFIG_IS_ENABLED(CLK)
@@ -772,7 +772,7 @@ int ehci_usb_remove(struct udevice *dev)
 
 #if CONFIG_IS_ENABLED(DM_REGULATOR)
 	if (priv->vbus_supply)
-		regulator_set_enable(priv->vbus_supply, false);
+		regulator_set_enable_if_allowed(priv->vbus_supply, false);
 #endif
 
 #if CONFIG_IS_ENABLED(CLK)
-- 
2.34.1

Re: [PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[Marek Vasut]

On 5/2/23 08:51, Eugen Hristev wrote:
> regulator_set_enable_if_allowed already handles cases when the
> regulator is already enabled, or already disabled, and does not
> treat these as errors.
> With this change, the driver can work correctly even if the regulator
> is already taken or already disabled by another consumer.

Can that ever happen for Vbus supply (the 5V coming out of USB port) ?
Can you elaborate how ?

Re: [PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[Eugen Hristev]

On 5/2/23 12:18, Marek Vasut wrote:
> On 5/2/23 08:51, Eugen Hristev wrote:
>> regulator_set_enable_if_allowed already handles cases when the
>> regulator is already enabled, or already disabled, and does not
>> treat these as errors.
>> With this change, the driver can work correctly even if the regulator
>> is already taken or already disabled by another consumer.
> 
> Can that ever happen for Vbus supply (the 5V coming out of USB port) ?
> Can you elaborate how ?


Hi Marek,

Recently I developed a series of patches to add a reference counter for 
regulators :

https://marc.info/?l=u-boot&m=168191189309879&w=2

But with this series, having a regulator already enabled or already 
disabled results in an error returned by regulator_set_enable

Hence, one option is to replace calls with regulator_set_enable_if_allowed

There is a discussion ongoing here:

https://marc.info/?l=u-boot&m=168295920316621&w=2


Eugen

Re: [PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[Marek Vasut]

On 5/2/23 12:41, Eugen Hristev wrote:
> On 5/2/23 12:18, Marek Vasut wrote:
>> On 5/2/23 08:51, Eugen Hristev wrote:
>>> regulator_set_enable_if_allowed already handles cases when the
>>> regulator is already enabled, or already disabled, and does not
>>> treat these as errors.
>>> With this change, the driver can work correctly even if the regulator
>>> is already taken or already disabled by another consumer.
>>
>> Can that ever happen for Vbus supply (the 5V coming out of USB port) ?
>> Can you elaborate how ?
> 
> 
> Hi Marek,

Hi,

> Recently I developed a series of patches to add a reference counter for 
> regulators :
> 
> https://marc.info/?l=u-boot&m=168191189309879&w=2

Ah yes, this is super cool stuff, thanks !

> But with this series, having a regulator already enabled or already 
> disabled results in an error returned by regulator_set_enable
> 
> Hence, one option is to replace calls with regulator_set_enable_if_allowed
> 
> There is a discussion ongoing here:
> 
> https://marc.info/?l=u-boot&m=168295920316621&w=2

Pardon my ignorance, but uh ... how does Linux deal with regulators 
which are enabled by prior stage, like U-Boot ? Does it set their enable 
refcount to =1 or =0 ?

Re: [PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[Eugen Hristev]

On 5/2/23 13:53, Marek Vasut wrote:
> On 5/2/23 12:41, Eugen Hristev wrote:
>> On 5/2/23 12:18, Marek Vasut wrote:
>>> On 5/2/23 08:51, Eugen Hristev wrote:
>>>> regulator_set_enable_if_allowed already handles cases when the
>>>> regulator is already enabled, or already disabled, and does not
>>>> treat these as errors.
>>>> With this change, the driver can work correctly even if the regulator
>>>> is already taken or already disabled by another consumer.
>>>
>>> Can that ever happen for Vbus supply (the 5V coming out of USB port) ?
>>> Can you elaborate how ?
>>
>>
>> Hi Marek,
> 
> Hi,
> 
>> Recently I developed a series of patches to add a reference counter 
>> for regulators :
>>
>> https://marc.info/?l=u-boot&m=168191189309879&w=2
> 
> Ah yes, this is super cool stuff, thanks !
> 
>> But with this series, having a regulator already enabled or already 
>> disabled results in an error returned by regulator_set_enable
>>
>> Hence, one option is to replace calls with 
>> regulator_set_enable_if_allowed
>>
>> There is a discussion ongoing here:
>>
>> https://marc.info/?l=u-boot&m=168295920316621&w=2
> 
> Pardon my ignorance, but uh ... how does Linux deal with regulators 
> which are enabled by prior stage, like U-Boot ? Does it set their enable 
> refcount to =1 or =0 ?

Please correct me if I am wrong, I did not dig too much into regulators, 
but , from my understanding :

Normally software(in this case Linux) reads the state of the regulator 
when it's probed, and sets the refcount accordingly. If the state cannot 
be read then regulator-boot-on tells Linux that it should have been left 
enabled by prior stage
In Uboot I believe that regulators that have 'regulator-boot-on' are 
added to a list and set to enable during probe, and the probe is forced 
even if Uboot normally uses lazy probing (should have the same behavior 
for regulator-always-on property)

So if a regulator is enabled by an initial stage but it's not described 
with 'regulator-boot-on' , appears to be a bad hardware description in DT.

Re: [PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[Tim Harvey]

On Mon, May 1, 2023 at 11:51 PM Eugen Hristev
<eugen.hristev@collabora.com> wrote:
>
> regulator_set_enable_if_allowed already handles cases when the
> regulator is already enabled, or already disabled, and does not
> treat these as errors.
> With this change, the driver can work correctly even if the regulator
> is already taken or already disabled by another consumer.
>
> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
> ---
> Hi Tim,
>
> I have not tested this as I do not have a mx6 board. can you please
> try it ?
>
> Thanks,
> Eugen

Eugen,

This does resolve the issue that occurs after your refcount series [1].

However after thinking about this more and seeing Marek's responses
this wasn't an issue of multiple consumers sharing the same regulator.
Instead this particular issue was that the vbus regulator is getting
enabled twice without being disabled. Adding a print in
regulator_set_enable shows me:
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -165,6 +165,7 @@ int regulator_set_enable(struct udevice *dev, bool enable)
        struct dm_regulator_uclass_plat *uc_pdata;
        int ret, old_enable = 0;

+printf("%s %s %s\n", __func__, dev->name, enable ? "enable" : "disable");
        if (!ops || !ops->set_enable)
                return -ENOSYS;

u-boot=> usb start
starting USB...
Bus usb@32e40000: regulator_set_enable regulator-usb-otg1 enable
^^^ from ehci_usb_probe
Bus usb@32e50000: regulator_set_enable regulator-usb-otg2 enable
^^^ from ehci_usb_probe
regulator_set_enable regulator-usb-otg2 enable
^^^ from mx6_init_after_reset - 2nd enable without a disable

So while I think this patch does make sense to cover the case where a
regulator could be shared there probably is a follow-on patch needed
to balance the regulator calls (unrelated to your series).

Marek,

Should vbus regulator enable really be in init_after_reset call?

Best Regards,

Tim

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=351536

Re: [PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[Marek Vasut]

On 5/2/23 18:13, Tim Harvey wrote:
> On Mon, May 1, 2023 at 11:51 PM Eugen Hristev
> <eugen.hristev@collabora.com> wrote:
>>
>> regulator_set_enable_if_allowed already handles cases when the
>> regulator is already enabled, or already disabled, and does not
>> treat these as errors.
>> With this change, the driver can work correctly even if the regulator
>> is already taken or already disabled by another consumer.
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
>> ---
>> Hi Tim,
>>
>> I have not tested this as I do not have a mx6 board. can you please
>> try it ?
>>
>> Thanks,
>> Eugen
> 
> Eugen,
> 
> This does resolve the issue that occurs after your refcount series [1].
> 
> However after thinking about this more and seeing Marek's responses
> this wasn't an issue of multiple consumers sharing the same regulator.
> Instead this particular issue was that the vbus regulator is getting
> enabled twice without being disabled. Adding a print in
> regulator_set_enable shows me:
> --- a/drivers/power/regulator/regulator-uclass.c
> +++ b/drivers/power/regulator/regulator-uclass.c
> @@ -165,6 +165,7 @@ int regulator_set_enable(struct udevice *dev, bool enable)
>          struct dm_regulator_uclass_plat *uc_pdata;
>          int ret, old_enable = 0;
> 
> +printf("%s %s %s\n", __func__, dev->name, enable ? "enable" : "disable");
>          if (!ops || !ops->set_enable)
>                  return -ENOSYS;
> 
> u-boot=> usb start
> starting USB...
> Bus usb@32e40000: regulator_set_enable regulator-usb-otg1 enable
> ^^^ from ehci_usb_probe
> Bus usb@32e50000: regulator_set_enable regulator-usb-otg2 enable
> ^^^ from ehci_usb_probe
> regulator_set_enable regulator-usb-otg2 enable
> ^^^ from mx6_init_after_reset - 2nd enable without a disable
> 
> So while I think this patch does make sense to cover the case where a
> regulator could be shared

Does such a case really still exist after the discovery you made above ?

> there probably is a follow-on patch needed
> to balance the regulator calls (unrelated to your series).
> 
> Marek,
> 
> Should vbus regulator enable really be in init_after_reset call?

Yes, but I am not entirely convinced the VBUS should be enabled in 
ehci_usb_probe(), because in ehci_usb_probe() resp. in ehci_register() 
which is called at the end, we might detect that the port is configured 
as PERIPHERAL and we don't want to enable VBUS in that case .

So I suspect regulator_set_enable() should rather be dropped from 
ehci_usb_probe() . What do you think ?

Re: [PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[Marek Vasut]

On 5/2/23 13:18, Eugen Hristev wrote:
> On 5/2/23 13:53, Marek Vasut wrote:
>> On 5/2/23 12:41, Eugen Hristev wrote:
>>> On 5/2/23 12:18, Marek Vasut wrote:
>>>> On 5/2/23 08:51, Eugen Hristev wrote:
>>>>> regulator_set_enable_if_allowed already handles cases when the
>>>>> regulator is already enabled, or already disabled, and does not
>>>>> treat these as errors.
>>>>> With this change, the driver can work correctly even if the regulator
>>>>> is already taken or already disabled by another consumer.
>>>>
>>>> Can that ever happen for Vbus supply (the 5V coming out of USB port) ?
>>>> Can you elaborate how ?
>>>
>>>
>>> Hi Marek,
>>
>> Hi,
>>
>>> Recently I developed a series of patches to add a reference counter 
>>> for regulators :
>>>
>>> https://marc.info/?l=u-boot&m=168191189309879&w=2
>>
>> Ah yes, this is super cool stuff, thanks !
>>
>>> But with this series, having a regulator already enabled or already 
>>> disabled results in an error returned by regulator_set_enable
>>>
>>> Hence, one option is to replace calls with 
>>> regulator_set_enable_if_allowed
>>>
>>> There is a discussion ongoing here:
>>>
>>> https://marc.info/?l=u-boot&m=168295920316621&w=2
>>
>> Pardon my ignorance, but uh ... how does Linux deal with regulators 
>> which are enabled by prior stage, like U-Boot ? Does it set their 
>> enable refcount to =1 or =0 ?
> 
> Please correct me if I am wrong, I did not dig too much into regulators, 
> but , from my understanding :
> 
> Normally software(in this case Linux) reads the state of the regulator 
> when it's probed, and sets the refcount accordingly. If the state cannot 
> be read then regulator-boot-on tells Linux that it should have been left 
> enabled by prior stage
> In Uboot I believe that regulators that have 'regulator-boot-on' are 
> added to a list and set to enable during probe, and the probe is forced 
> even if Uboot normally uses lazy probing (should have the same behavior 
> for regulator-always-on property)
> 
> So if a regulator is enabled by an initial stage but it's not described 
> with 'regulator-boot-on' , appears to be a bad hardware description in DT.

Thanks for checking this. It seems like Tim found the root cause of the 
problem in the meantime, so let's see where that discussion moves on to.

Re: [PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[Eugen Hristev]

On 5/4/23 00:52, Marek Vasut wrote:
> On 5/2/23 18:13, Tim Harvey wrote:
>> On Mon, May 1, 2023 at 11:51 PM Eugen Hristev
>> <eugen.hristev@collabora.com> wrote:
>>>
>>> regulator_set_enable_if_allowed already handles cases when the
>>> regulator is already enabled, or already disabled, and does not
>>> treat these as errors.
>>> With this change, the driver can work correctly even if the regulator
>>> is already taken or already disabled by another consumer.
>>>
>>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
>>> ---
>>> Hi Tim,
>>>
>>> I have not tested this as I do not have a mx6 board. can you please
>>> try it ?
>>>
>>> Thanks,
>>> Eugen
>>
>> Eugen,
>>
>> This does resolve the issue that occurs after your refcount series [1].
>>
>> However after thinking about this more and seeing Marek's responses
>> this wasn't an issue of multiple consumers sharing the same regulator.
>> Instead this particular issue was that the vbus regulator is getting
>> enabled twice without being disabled.

This kind of issues can be uncovered when we add the refcounter :)
On one hand I am happy to uncover such bugs.
However, this leads me to wonder, even without a refcount, even without 
my series, enabling a regulator that is already enabled should return 
-EALREADY

On the other hand, it appears it will take longer to get the refcount 
applied though.

  Adding a print in
>> regulator_set_enable shows me:
>> --- a/drivers/power/regulator/regulator-uclass.c
>> +++ b/drivers/power/regulator/regulator-uclass.c
>> @@ -165,6 +165,7 @@ int regulator_set_enable(struct udevice *dev, bool 
>> enable)
>>          struct dm_regulator_uclass_plat *uc_pdata;
>>          int ret, old_enable = 0;
>>
>> +printf("%s %s %s\n", __func__, dev->name, enable ? "enable" : 
>> "disable");
>>          if (!ops || !ops->set_enable)
>>                  return -ENOSYS;
>>
>> u-boot=> usb start
>> starting USB...
>> Bus usb@32e40000: regulator_set_enable regulator-usb-otg1 enable
>> ^^^ from ehci_usb_probe
>> Bus usb@32e50000: regulator_set_enable regulator-usb-otg2 enable
>> ^^^ from ehci_usb_probe
>> regulator_set_enable regulator-usb-otg2 enable
>> ^^^ from mx6_init_after_reset - 2nd enable without a disable
>>
>> So while I think this patch does make sense to cover the case where a
>> regulator could be shared
> 
> Does such a case really still exist after the discovery you made above ?
> 
>> there probably is a follow-on patch needed
>> to balance the regulator calls (unrelated to your series).
>>
>> Marek,
>>
>> Should vbus regulator enable really be in init_after_reset call?
> 
> Yes, but I am not entirely convinced the VBUS should be enabled in 
> ehci_usb_probe(), because in ehci_usb_probe() resp. in ehci_register() 
> which is called at the end, we might detect that the port is configured 
> as PERIPHERAL and we don't want to enable VBUS in that case .
> 
> So I suspect regulator_set_enable() should rather be dropped from 
> ehci_usb_probe() . What do you think ?

Re: [PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[Marek Vasut]

On 5/4/23 10:12, Eugen Hristev wrote:
> On 5/4/23 00:52, Marek Vasut wrote:
>> On 5/2/23 18:13, Tim Harvey wrote:
>>> On Mon, May 1, 2023 at 11:51 PM Eugen Hristev
>>> <eugen.hristev@collabora.com> wrote:
>>>>
>>>> regulator_set_enable_if_allowed already handles cases when the
>>>> regulator is already enabled, or already disabled, and does not
>>>> treat these as errors.
>>>> With this change, the driver can work correctly even if the regulator
>>>> is already taken or already disabled by another consumer.
>>>>
>>>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
>>>> ---
>>>> Hi Tim,
>>>>
>>>> I have not tested this as I do not have a mx6 board. can you please
>>>> try it ?
>>>>
>>>> Thanks,
>>>> Eugen
>>>
>>> Eugen,
>>>
>>> This does resolve the issue that occurs after your refcount series [1].
>>>
>>> However after thinking about this more and seeing Marek's responses
>>> this wasn't an issue of multiple consumers sharing the same regulator.
>>> Instead this particular issue was that the vbus regulator is getting
>>> enabled twice without being disabled.
> 
> This kind of issues can be uncovered when we add the refcounter :)

Yeah, and that's a good thing. The refcounter is much appreciated (am I 
repeating myself?)

> On one hand I am happy to uncover such bugs.
> However, this leads me to wonder, even without a refcount, even without 
> my series, enabling a regulator that is already enabled should return 
> -EALREADY

I don't that so, you can have regulators which supply multiple things 
and which are claimed by multiple driver instances, at which point they 
will each enable the regulator (refcount++) and that shouldn't end up 
returning -EALREADY . Right ?

One such example is buck regulator supplying e.g. SD and eMMC Vcc .

> On the other hand, it appears it will take longer to get the refcount 
> applied though.

I think we need to sort out the fallout, but that's probably still fine.

Re: [PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[Eugen Hristev]

On 5/4/23 14:41, Marek Vasut wrote:
> On 5/4/23 10:12, Eugen Hristev wrote:
>> On 5/4/23 00:52, Marek Vasut wrote:
>>> On 5/2/23 18:13, Tim Harvey wrote:
>>>> On Mon, May 1, 2023 at 11:51 PM Eugen Hristev
>>>> <eugen.hristev@collabora.com> wrote:
>>>>>
>>>>> regulator_set_enable_if_allowed already handles cases when the
>>>>> regulator is already enabled, or already disabled, and does not
>>>>> treat these as errors.
>>>>> With this change, the driver can work correctly even if the regulator
>>>>> is already taken or already disabled by another consumer.
>>>>>
>>>>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
>>>>> ---
>>>>> Hi Tim,
>>>>>
>>>>> I have not tested this as I do not have a mx6 board. can you please
>>>>> try it ?
>>>>>
>>>>> Thanks,
>>>>> Eugen
>>>>
>>>> Eugen,
>>>>
>>>> This does resolve the issue that occurs after your refcount series [1].
>>>>
>>>> However after thinking about this more and seeing Marek's responses
>>>> this wasn't an issue of multiple consumers sharing the same regulator.
>>>> Instead this particular issue was that the vbus regulator is getting
>>>> enabled twice without being disabled.
>>
>> This kind of issues can be uncovered when we add the refcounter :)
> 
> Yeah, and that's a good thing. The refcounter is much appreciated (am I 
> repeating myself?)
> 
>> On one hand I am happy to uncover such bugs.
>> However, this leads me to wonder, even without a refcount, even 
>> without my series, enabling a regulator that is already enabled should 
>> return -EALREADY
> 
> I don't that so, you can have regulators which supply multiple things 
> and which are claimed by multiple driver instances, at which point they 
> will each enable the regulator (refcount++) and that shouldn't end up 
> returning -EALREADY . Right ?

That was my thought. And I was returning success if regulator was 
already enabled.
Then Simon asked me to return error codes on each path (review on the v2 
of the patches):

https://marc.info/?l=u-boot&m=168012024923488&w=2

> 
> One such example is buck regulator supplying e.g. SD and eMMC Vcc .
> 
>> On the other hand, it appears it will take longer to get the refcount 
>> applied though.
> 
> I think we need to sort out the fallout, but that's probably still fine.

Re: [PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

[Marek Vasut]

On 5/4/23 13:51, Eugen Hristev wrote:
> On 5/4/23 14:41, Marek Vasut wrote:
>> On 5/4/23 10:12, Eugen Hristev wrote:
>>> On 5/4/23 00:52, Marek Vasut wrote:
>>>> On 5/2/23 18:13, Tim Harvey wrote:
>>>>> On Mon, May 1, 2023 at 11:51 PM Eugen Hristev
>>>>> <eugen.hristev@collabora.com> wrote:
>>>>>>
>>>>>> regulator_set_enable_if_allowed already handles cases when the
>>>>>> regulator is already enabled, or already disabled, and does not
>>>>>> treat these as errors.
>>>>>> With this change, the driver can work correctly even if the regulator
>>>>>> is already taken or already disabled by another consumer.
>>>>>>
>>>>>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
>>>>>> ---
>>>>>> Hi Tim,
>>>>>>
>>>>>> I have not tested this as I do not have a mx6 board. can you please
>>>>>> try it ?
>>>>>>
>>>>>> Thanks,
>>>>>> Eugen
>>>>>
>>>>> Eugen,
>>>>>
>>>>> This does resolve the issue that occurs after your refcount series 
>>>>> [1].
>>>>>
>>>>> However after thinking about this more and seeing Marek's responses
>>>>> this wasn't an issue of multiple consumers sharing the same regulator.
>>>>> Instead this particular issue was that the vbus regulator is getting
>>>>> enabled twice without being disabled.
>>>
>>> This kind of issues can be uncovered when we add the refcounter :)
>>
>> Yeah, and that's a good thing. The refcounter is much appreciated (am 
>> I repeating myself?)
>>
>>> On one hand I am happy to uncover such bugs.
>>> However, this leads me to wonder, even without a refcount, even 
>>> without my series, enabling a regulator that is already enabled 
>>> should return -EALREADY
>>
>> I don't that so, you can have regulators which supply multiple things 
>> and which are claimed by multiple driver instances, at which point 
>> they will each enable the regulator (refcount++) and that shouldn't 
>> end up returning -EALREADY . Right ?
> 
> That was my thought. And I was returning success if regulator was 
> already enabled.
> Then Simon asked me to return error codes on each path (review on the v2 
> of the patches):
> 
> https://marc.info/?l=u-boot&m=168012024923488&w=2

I would argue the regulator API should behave the same way as Linux one, 
to avoid confusing people developing on both. Simon ?