* [PATCH 1/1] phy: tegra: xusb: Fix unbalanced regulator disable in UTMI PHY mode
@ 2025-05-02 9:26 Wayne Chang
2025-05-15 11:03 ` Jon Hunter
2025-06-15 16:52 ` Vinod Koul
0 siblings, 2 replies; 4+ messages in thread
From: Wayne Chang @ 2025-05-02 9:26 UTC (permalink / raw)
To: waynec, jckuo, vkoul, kishon, thierry.reding, jonathanh
Cc: linux-phy, linux-tegra, linux-kernel, stable
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;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] phy: tegra: xusb: Fix unbalanced regulator disable in UTMI PHY mode
2025-05-02 9:26 [PATCH 1/1] phy: tegra: xusb: Fix unbalanced regulator disable in UTMI PHY mode Wayne Chang
@ 2025-05-15 11:03 ` Jon Hunter
2025-06-10 13:45 ` Jon Hunter
2025-06-15 16:52 ` Vinod Koul
1 sibling, 1 reply; 4+ messages in thread
From: Jon Hunter @ 2025-05-15 11:03 UTC (permalink / raw)
To: Wayne Chang, jckuo, vkoul, kishon, thierry.reding
Cc: linux-phy, linux-tegra, linux-kernel, stable
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!
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] phy: tegra: xusb: Fix unbalanced regulator disable in UTMI PHY mode
2025-05-15 11:03 ` Jon Hunter
@ 2025-06-10 13:45 ` Jon Hunter
0 siblings, 0 replies; 4+ messages in thread
From: Jon Hunter @ 2025-06-10 13:45 UTC (permalink / raw)
To: Wayne Chang, jckuo, vkoul, kishon, thierry.reding
Cc: linux-phy, linux-tegra, linux-kernel, stable
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
--
nvpublic
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] phy: tegra: xusb: Fix unbalanced regulator disable in UTMI PHY mode
2025-05-02 9:26 [PATCH 1/1] phy: tegra: xusb: Fix unbalanced regulator disable in UTMI PHY mode Wayne Chang
2025-05-15 11:03 ` Jon Hunter
@ 2025-06-15 16:52 ` Vinod Koul
1 sibling, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2025-06-15 16:52 UTC (permalink / raw)
To: jckuo, kishon, thierry.reding, jonathanh, Wayne Chang
Cc: linux-phy, linux-tegra, linux-kernel, stable
On Fri, 02 May 2025 17:26:06 +0800, 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:
>
> [...]
Applied, thanks!
[1/1] phy: tegra: xusb: Fix unbalanced regulator disable in UTMI PHY mode
commit: cefc1caee9dd06c69e2d807edc5949b329f52b22
Best regards,
--
~Vinod
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-06-15 16:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-02 9:26 [PATCH 1/1] phy: tegra: xusb: Fix unbalanced regulator disable in UTMI PHY mode Wayne Chang
2025-05-15 11:03 ` Jon Hunter
2025-06-10 13:45 ` Jon Hunter
2025-06-15 16:52 ` Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).