Hi Vinod,

On 15/05/2025 12:03, Jon Hunter wrote:

On 02/05/2025 10:26, Wayne Chang wrote:

...

When transitioning from USB_ROLE_DEVICE to USB_ROLE_NONE, the code assumed that the regulator should be disabled. However, if the regulator is marked as always-on, regulator_is_enabled() continues to return true, leading to an incorrect attempt to disable a regulator which is not enabled.

This can result in warnings such as:

[  250.155624] WARNING: CPU: 1 PID: 7326 at drivers/regulator/core.c:3004 _regulator_disable+0xe4/0x1a0 [  250.155652] unbalanced disables for VIN_SYS_5V0

To fix this, we move the regulator control logic into tegra186_xusb_padctl_id_override() function since it's directly related to the ID override state. The regulator is now only disabled when the role transitions from USB_ROLE_HOST to USB_ROLE_NONE, by checking the VBUS_ID register. This ensures that regulator enable/disable operations are properly balanced and only occur when actually transitioning to/from host mode.

Fixes: 49d46e3c7e59 ("phy: tegra: xusb: Add set_mode support for UTMI phy on Tegra186") Cc: stable@vger.kernel.org Signed-off-by: Wayne Chang waynec@nvidia.com

---

drivers/phy/tegra/xusb-tegra186.c | 59 +++++++++++++++++++------------   1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/ xusb-tegra186.c index fae6242aa730..1b35d50821f7 100644 --- a/drivers/phy/tegra/xusb-tegra186.c +++ b/drivers/phy/tegra/xusb-tegra186.c @@ -774,13 +774,15 @@ static int tegra186_xusb_padctl_vbus_override(struct tegra_xusb_padctl *padctl,   }   static int tegra186_xusb_padctl_id_override(struct tegra_xusb_padctl *padctl, -                        bool status) +                        struct tegra_xusb_usb2_port *port, bool status)   { -    u32 value; +    u32 value, id_override; +    int err = 0;       dev_dbg(padctl->dev, "%s id override\n", status ? "set" : "clear");       value = padctl_readl(padctl, USB2_VBUS_ID); +    id_override = value & ID_OVERRIDE(~0);       if (status) {           if (value & VBUS_OVERRIDE) { @@ -791,15 +793,35 @@ static int tegra186_xusb_padctl_id_override(struct tegra_xusb_padctl *padctl,               value = padctl_readl(padctl, USB2_VBUS_ID);           } -        value &= ~ID_OVERRIDE(~0); -        value |= ID_OVERRIDE_GROUNDED; +        if (id_override != ID_OVERRIDE_GROUNDED) { +            value &= ~ID_OVERRIDE(~0); +            value |= ID_OVERRIDE_GROUNDED; +            padctl_writel(padctl, value, USB2_VBUS_ID);

+            err = regulator_enable(port->supply); +            if (err) { +                dev_err(padctl->dev, "Failed to enable regulator: %d\n", err); +                return err; +            } +        }       } else { -        value &= ~ID_OVERRIDE(~0); -        value |= ID_OVERRIDE_FLOATING; +        if (id_override == ID_OVERRIDE_GROUNDED) { +            /* +             * The regulator is disabled only when the role transitions +             * from USB_ROLE_HOST to USB_ROLE_NONE. +             */ +            err = regulator_disable(port->supply); +            if (err) { +                dev_err(padctl->dev, "Failed to disable regulator: %d\n", err); +                return err; +            }

+            value &= ~ID_OVERRIDE(~0); +            value |= ID_OVERRIDE_FLOATING; +            padctl_writel(padctl, value, USB2_VBUS_ID); +        }       } -    padctl_writel(padctl, value, USB2_VBUS_ID);

return 0;   } @@ -818,27 +840,20 @@ static int tegra186_utmi_phy_set_mode(struct phy *phy, enum phy_mode mode,       if (mode == PHY_MODE_USB_OTG) {           if (submode == USB_ROLE_HOST) { -            tegra186_xusb_padctl_id_override(padctl, true);

-            err = regulator_enable(port->supply); +            err = tegra186_xusb_padctl_id_override(padctl, port, true); +            if (err) +                goto out;           } else if (submode == USB_ROLE_DEVICE) {               tegra186_xusb_padctl_vbus_override(padctl, true);           } else if (submode == USB_ROLE_NONE) { -            /* -             * When port is peripheral only or role transitions to -             * USB_ROLE_NONE from USB_ROLE_DEVICE, regulator is not -             * enabled. -             */ -            if (regulator_is_enabled(port->supply)) -                regulator_disable(port->supply);

-            tegra186_xusb_padctl_id_override(padctl, false); +            err = tegra186_xusb_padctl_id_override(padctl, port, false); +            if (err) +                goto out;               tegra186_xusb_padctl_vbus_override(padctl, false);           }       }

+out:       mutex_unlock(&padctl->lock);

return err;   }

Reviewed-by: Jon Hunter jonathanh@nvidia.com Tested-by: Jon Hunter jonathanh@nvidia.com

Thanks for fixing this!

Can we get this into -next now?

Thanks!

Jon