Re: [PATCH] usb: xhci-mvebu: use regulator_set_enable_if_allowed()

On Fri, Aug 21, 2026 at 4:25 AM Marek Vasut <[email protected]> wrote:
>
> On 8/20/26 5:10 PM, Bruno Banelli wrote:
> > Since commit 4fcba5d556b4 ("regulator: implement basic reference
> > counter") regulator_set_enable() returns -EALREADY when a fixed or
> > GPIO regulator has already been enabled. On boards whose VBUS
> > regulator carries regulator-always-on or regulator-boot-on -
> > armada-8040-mcbin.dts is one - regulator_autoset() enables it during
> > the regulator's own probe, so the subsequent enable in xhci_usb_probe()
> > fails and the controller is never registered:
> >
> >    starting USB...
> >    Failed to turn ON the VBUS regulator
> >    Bus usb3@500000: probe failed, error -114
> >    No USB controllers found
> >
> > Use regulator_set_enable_if_allowed(), which tolerates -EALREADY, in
> > the same way as commit 0830333c4743 ("usb: ehci-generic: Use
> > regulator_set_enable_if_allowed") and the other drivers converted
> > after the reference counter was introduced.
> >
> > Tested on a SolidRun MACCHIATObin (Armada 8040), where USB mass
> > storage now enumerates.
> >
> > Fixes: 4fcba5d556b4 ("regulator: implement basic reference counter")
> > Signed-off-by: Bruno Banelli <[email protected]>
> > ---
> >   drivers/usb/host/xhci-mvebu.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
> > index c294a56b3c..53b3e82b66 100644
> > --- a/drivers/usb/host/xhci-mvebu.c
> > +++ b/drivers/usb/host/xhci-mvebu.c
> > @@ -51,7 +51,7 @@ static int xhci_usb_probe(struct udevice *dev)
> >
> >       ret = device_get_supply_regulator(dev, "vbus-supply", &regulator);
> >       if (!ret) {
> > -             ret = regulator_set_enable(regulator, true);
> > +             ret = regulator_set_enable_if_allowed(regulator, true);
> >               if (ret) {
> >                       printf("Failed to turn ON the VBUS regulator\n");
> Do you need to handle -ENOSYS return value here ?

No - regulator_set_enable_if_allowed() already absorbs it, which is the
reason for using it rather than open-coding the check in the driver:

  int regulator_set_enable_if_allowed(struct udevice *dev, bool enable)
  {
          int ret;

          ret = regulator_set_enable(dev, enable);
          if (ret == -ENOSYS || ret == -EACCES)
                  return 0;
          /* if we want to disable but it's in use by someone else */
          if (!enable && ret == -EBUSY)
                  return 0;
          /* if it's already enabled/disabled */
          if (ret == -EALREADY)
                  return 0;

          return ret;
  }

  (drivers/power/regulator/regulator-uclass.c)

For what it is worth, -ENOSYS is not the code this patch is actually
fixing. On the boards that fail, the vbus supply is a regulator-fixed
with a valid GPIO and both regulator-always-on and regulator-boot-on -
on MACCHIATObin (arch/arm/dts/armada-8040-mcbin.dts):

          reg_usb3h0_vbus: usb3-vbus0 {
                  compatible = "regulator-fixed";
                  regulator-name = "reg-usb3h0-vbus";
                  startup-delay-us = <500000>;
                  enable-active-high;
                  regulator-always-on;
                  regulator-boot-on;
                  gpio = <&cp0_gpio1 15 GPIO_ACTIVE_HIGH>;
          };

regulator_post_probe() -> regulator_autoset() has therefore already
enabled it and bumped enable_count, since for REGULATOR_TYPE_FIXED
autoset calls regulator_set_enable(dev, true) directly. By the time
xhci_usb_probe() makes its own call we land in:

          if (enable && plat->enable_count > 0) {
                  plat->enable_count++;
                  return -EALREADY;
          }

  (drivers/power/regulator/regulator_common.c)

and the current driver treats that as fatal, so xhci_register() is never
reached and USB is dead on the board.

So the codes that matter here are -EALREADY (always-on/boot-on supplies,
the case above) and -ENOSYS/-EACCES (supplies whose driver has no
set_enable op, or a regulator-fixed with no GPIO). Rather than the driver
enumerating those itself, regulator_set_enable_if_allowed() is exactly
the helper for it, and this makes xhci-mvebu consistent with the other
xhci glue drivers that already use it.

Happy to respin if you would prefer the driver to be explicit about which
codes it tolerates.

With my best,

B