| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <31989da4-0242-470c-b77f-2b0506cdb8cd@nvidia.com>
Date: Tue, 10 Jun 2025 14:45:56 +0100
From: Jon Hunter <jonathanh@...dia.com>
To: Wayne Chang <waynec@...dia.com>, jckuo@...dia.com, vkoul@...nel.org,
kishon@...nel.org, thierry.reding@...il.com
Cc: linux-phy@...ts.infradead.org, linux-tegra@...r.kernel.org,
linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 1/1] phy: tegra: xusb: Fix unbalanced regulator disable in
UTMI PHY mode
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@...r.kernel.org
>> Signed-off-by: Wayne Chang <waynec@...dia.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@...dia.com>
> Tested-by: Jon Hunter <jonathanh@...dia.com>
>
> Thanks for fixing this!
Can we get this into -next now?
Thanks!
Jon
--
nvpublic
Powered by blists - more mailing lists