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

Tim Harvey tharvey at gateworks.com
Tue May 2 18:13:18 CEST 2023


On Mon, May 1, 2023 at 11:51 PM Eugen Hristev
<eugen.hristev at 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 at 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 at 32e40000: regulator_set_enable regulator-usb-otg1 enable
^^^ from ehci_usb_probe
Bus usb at 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


More information about the U-Boot mailing list