[PATCH v2 0/6] Fixes to Tegra USB role switching and phy handling

[PATCH v2 0/6] Fixes to Tegra USB role switching and phy handling

[Diogo Ivo]

Hello,

This patch series contains fixes/improvements for USB role switching on the
Tegra210 and Tegra186 SoCs.

The first patch addresses a wrong check on the logic that disables the
VBUS regulator.

The second patch removes a redundant mutex lock when setting the PHY
mode.

The third patch guarantees proper ordering of events when switching PHY
roles.

The remaining patches are included to standardize the PHY .set_mode()
callback between Tegra186 and Tegra210.

With this patch series this feature can only be controlled from userspace,
by writing the desired role to sysfs as

echo "role" > /sys/class/usb_role/usb2-0-role-switch/role

with role being one of {device, host, none}.

Further patches will enable automatic role switching via the 'cros_ec_typec'
driver which is currently broken on Smaug.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
Changes in v2:
- Remove DT patches already taken to be upstreamed
- Add standardization between Tegra210 and Tegra186
- Address review comments from v1, detailed descriptions in each patch
- Link to v1: https://lore.kernel.org/r/20251204-diogo-tegra_phy-v1-0-51a2016d0be8@tecnico.ulisboa.pt

---
Diogo Ivo (6):
      phy: tegra: xusb: Fix USB2 port regulator disable logic
      usb: xhci: tegra: Remove redundant mutex when setting phy mode
      phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports
      phy: tegra: xusb: Add ID override support to padctl
      phy: tegra: xusb: Move .set_mode() to a shared location
      phy: tegra: xusb: Move T186 .set_mode() to common implementation

 drivers/phy/tegra/xusb-tegra186.c   | 73 +++++----------------------------
 drivers/phy/tegra/xusb-tegra210.c   | 42 +------------------
 drivers/phy/tegra/xusb.c            | 80 +++++++++++++++++++++++++++++++++++++
 drivers/phy/tegra/xusb.h            |  4 ++
 drivers/usb/gadget/udc/tegra-xudc.c |  4 ++
 drivers/usb/host/xhci-tegra.c       | 14 ++++---
 include/linux/phy/tegra/xusb.h      |  3 ++
 7 files changed, 111 insertions(+), 109 deletions(-)
---
base-commit: b02a5530af8abe0d3cd4852ba48990716e962934
change-id: 20251201-diogo-tegra_phy-86c89cab7377

Best regards,
-- 
Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>

[PATCH v2 1/6] phy: tegra: xusb: Fix USB2 port regulator disable logic

[Diogo Ivo]

The USB2 PHY mode handling on Tegra210 incorrectly relied on
regulator_is_enabled() when determining whether the VBUS supply should
be disabled during role changes. This is because regulator_is_enabled()
reports exactly what is states and not if there is an unbalanced number
of calls between regulator_enable() and regulator_disable(). For
example, regulator_is_enabled() always reports true on a fixed-regulator
with no enable gpio, which is the case on the Pixel C.

This then leads to the PHY driver wrongfully calling regulator_disable()
when transitioning from USB_ROLE_DEVICE to USB_ROLE_NONE since the driver
did not previously call the corresponding regulator_enable().

Fix this by keeping track of the current role and updating the logic to
disable the regulator only when the previous role was USB_ROLE_HOST.

Cc: stable@vger.kernel.org
Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
v1->v2:
- Do not fix typo in comment
---
 drivers/phy/tegra/xusb-tegra210.c | 4 +++-
 drivers/phy/tegra/xusb.h          | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
index 3409924498e9..3abdf0182d67 100644
--- a/drivers/phy/tegra/xusb-tegra210.c
+++ b/drivers/phy/tegra/xusb-tegra210.c
@@ -1936,12 +1936,14 @@ static int tegra210_usb2_phy_set_mode(struct phy *phy, enum phy_mode mode,
 			 * USB_ROLE_NONE from USB_ROLE_DEVICE, regulator is not
 			 * be enabled.
 			 */
-			if (regulator_is_enabled(port->supply))
+			if (port->role == USB_ROLE_HOST)
 				regulator_disable(port->supply);
 
 			tegra210_xusb_padctl_id_override(padctl, false);
 			tegra210_xusb_padctl_vbus_override(padctl, false);
 		}
+
+		port->role = submode;
 	}
 
 	mutex_unlock(&padctl->lock);
diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
index d2b5f9565132..273af147dfd3 100644
--- a/drivers/phy/tegra/xusb.h
+++ b/drivers/phy/tegra/xusb.h
@@ -317,6 +317,7 @@ struct tegra_xusb_usb2_port {
 	enum usb_dr_mode mode;
 	bool internal;
 	int usb3_port_fake;
+	enum usb_role role;
 };
 
 static inline struct tegra_xusb_usb2_port *

-- 
2.52.0

[PATCH v2 2/6] usb: xhci: tegra: Remove redundant mutex when setting phy mode

[Diogo Ivo]

As the PHY subsystem already synchronizes concurrent accesses to a PHY
instance with a core-internal mutex remove the driver specific mutex
synchronization.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
v1->v2:
- New patch
---
 drivers/usb/host/xhci-tegra.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 8b492871d21d..927861ca14f2 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -1357,15 +1357,11 @@ static void tegra_xhci_id_work(struct work_struct *work)
 
 	dev_dbg(tegra->dev, "host mode %s\n", str_on_off(tegra->host_mode));
 
-	mutex_lock(&tegra->lock);
-
 	if (tegra->host_mode)
 		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
 	else
 		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
 
-	mutex_unlock(&tegra->lock);
-
 	tegra->otg_usb3_port = tegra_xusb_padctl_get_usb3_companion(tegra->padctl,
 								    tegra->otg_usb2_port);
 

-- 
2.52.0

[PATCH v2 3/6] phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports

[Diogo Ivo]

The current implementation of USB2 role switching on Tegra relies on
whichever the previous USB controller driver was using the PHY to first
"yield" it back to USB_ROLE_NONE before the next controller configures
it for the new role. However, no mechanism to guarantee this ordering
was implemented, and currently, in the general case, the configuration
functions tegra_xhci_id_work() and tegra_xudc_usb_role_sw_work() end up
running in the same order regardless of the transition being HOST->DEVICE
or DEVICE->HOST, leading to one of these transitions ending up in a
non-working state due to the new configuration being clobbered by the
previous controller driver setting USB_ROLE_NONE after the fact.

Fix this by introducing a helper that waits for the USB2 port’s current
role to become USB_ROLE_NONE and add it in the configuration functions
above before setting the role to either USB_ROLE_HOST or
USB_ROLE_DEVICE.

The specific parameters of the helper function were determined based on
my testing on a Tegra210 platform, Smaug, with some extra slack added in.
With these parameters I never observed any timeout in role switching.

As mentioned, this was tested on a Tegra210 platform. However, due to the
similar approach in Tegra186 it is likely that not only this problem exists
there but that this patch also fixes it.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
v1->v2:
- Edit commit message and add code comment explaining origin of wait
  parameters in tegra_xusb_usb2_port_wait_role_none()
- Export tegra_xusb_usb2_port_wait_role_none()
- Fix NULL pointer dereference in error message
- Remove extra blank line
---
 drivers/phy/tegra/xusb.c            | 30 ++++++++++++++++++++++++++++++
 drivers/usb/gadget/udc/tegra-xudc.c |  4 ++++
 drivers/usb/host/xhci-tegra.c       | 14 ++++++++++----
 include/linux/phy/tegra/xusb.h      |  1 +
 4 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index c89df95aa6ca..03fd6269fdbe 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -740,6 +740,36 @@ static void tegra_xusb_parse_usb_role_default_mode(struct tegra_xusb_port *port)
 	}
 }
 
+/*
+ * Helper function that waits for the port's role to become USB_ROLE_NONE. As the
+ * TRMs do not specify how long the transition is expected to take the sleep duration
+ * and number of retries were determined empirically on a Tegra210 platform, with some
+ * extra slack added in.
+ */
+bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl *padctl, int index)
+{
+	struct tegra_xusb_usb2_port *usb2 = tegra_xusb_find_usb2_port(padctl,
+								      index);
+	int retries = 5;
+
+	if (!usb2) {
+		dev_err(padctl->dev, "no port found for USB2 lane %u\n", index);
+		return false;
+	}
+
+	do {
+		if (usb2->role == USB_ROLE_NONE)
+			return true;
+
+		usleep_range(50, 60);
+	} while (retries--);
+
+	dev_err(&usb2->base.dev, "timed out waiting for USB_ROLE_NONE");
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(tegra_xusb_usb2_port_wait_role_none);
+
 static int tegra_xusb_usb2_port_parse_dt(struct tegra_xusb_usb2_port *usb2)
 {
 	struct tegra_xusb_port *port = &usb2->base;
diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
index 9d2007f448c0..24b0a9ce75d9 100644
--- a/drivers/usb/gadget/udc/tegra-xudc.c
+++ b/drivers/usb/gadget/udc/tegra-xudc.c
@@ -698,8 +698,12 @@ static void tegra_xudc_restore_port_speed(struct tegra_xudc *xudc)
 
 static void tegra_xudc_device_mode_on(struct tegra_xudc *xudc)
 {
+	int port = tegra_xusb_padctl_get_port_number(xudc->curr_utmi_phy);
 	int err;
 
+	if (!tegra_xusb_usb2_port_wait_role_none(xudc->padctl, port))
+		return;
+
 	pm_runtime_get_sync(xudc->dev);
 
 	tegra_phy_xusb_utmi_pad_power_on(xudc->curr_utmi_phy);
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 927861ca14f2..b51afb4036b5 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -1352,15 +1352,21 @@ static void tegra_xhci_id_work(struct work_struct *work)
 	struct tegra_xusb_mbox_msg msg;
 	struct phy *phy = tegra_xusb_get_phy(tegra, "usb2",
 						    tegra->otg_usb2_port);
+	enum usb_role role = USB_ROLE_NONE;
 	u32 status;
 	int ret;
 
 	dev_dbg(tegra->dev, "host mode %s\n", str_on_off(tegra->host_mode));
 
-	if (tegra->host_mode)
-		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
-	else
-		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
+	if (tegra->host_mode) {
+		if (!tegra_xusb_usb2_port_wait_role_none(tegra->padctl,
+							 tegra->otg_usb2_port))
+			return;
+
+		role = USB_ROLE_HOST;
+	}
+
+	phy_set_mode_ext(phy, PHY_MODE_USB_OTG, role);
 
 	tegra->otg_usb3_port = tegra_xusb_padctl_get_usb3_companion(tegra->padctl,
 								    tegra->otg_usb2_port);
diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h
index 6ca51e0080ec..a0d3d5b7cf33 100644
--- a/include/linux/phy/tegra/xusb.h
+++ b/include/linux/phy/tegra/xusb.h
@@ -33,5 +33,6 @@ int tegra_xusb_padctl_disable_phy_sleepwalk(struct tegra_xusb_padctl *padctl, st
 int tegra_xusb_padctl_enable_phy_wake(struct tegra_xusb_padctl *padctl, struct phy *phy);
 int tegra_xusb_padctl_disable_phy_wake(struct tegra_xusb_padctl *padctl, struct phy *phy);
 bool tegra_xusb_padctl_remote_wake_detected(struct tegra_xusb_padctl *padctl, struct phy *phy);
+bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl *padctl, int index);
 
 #endif /* PHY_TEGRA_XUSB_H */

-- 
2.52.0

[PATCH v2 4/6] phy: tegra: xusb: Add ID override support to padctl

[Diogo Ivo]

In preparation to move the XUSB PHY .set_mode() callback into a common
implementation introduce a generic id_override callback in
tegra_xusb_padctl_ops that delegates to a SoC-specific implementation
and register Tegra210 with the new callback.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
v1->v2:
- New patch
---
 drivers/phy/tegra/xusb-tegra210.c |  1 +
 drivers/phy/tegra/xusb.c          | 10 ++++++++++
 drivers/phy/tegra/xusb.h          |  1 +
 include/linux/phy/tegra/xusb.h    |  2 ++
 4 files changed, 14 insertions(+)

diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
index 3abdf0182d67..be03a17afd7e 100644
--- a/drivers/phy/tegra/xusb-tegra210.c
+++ b/drivers/phy/tegra/xusb-tegra210.c
@@ -3262,6 +3262,7 @@ static const struct tegra_xusb_padctl_ops tegra210_xusb_padctl_ops = {
 	.usb3_set_lfps_detect = tegra210_usb3_set_lfps_detect,
 	.hsic_set_idle = tegra210_hsic_set_idle,
 	.vbus_override = tegra210_xusb_padctl_vbus_override,
+	.id_override = tegra210_xusb_padctl_id_override,
 	.utmi_port_reset = tegra210_utmi_port_reset,
 };
 
diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index 03fd6269fdbe..0443465bcf50 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -1498,6 +1498,16 @@ int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl,
 }
 EXPORT_SYMBOL_GPL(tegra_xusb_padctl_set_vbus_override);
 
+int tegra_xusb_padctl_set_id_override(struct tegra_xusb_padctl *padctl,
+							bool val)
+{
+	if (padctl->soc->ops->id_override)
+		return padctl->soc->ops->id_override(padctl, val);
+
+	return -ENOTSUPP;
+}
+EXPORT_SYMBOL_GPL(tegra_xusb_padctl_set_id_override);
+
 int tegra_phy_xusb_utmi_port_reset(struct phy *phy)
 {
 	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
index 273af147dfd3..08053a730d54 100644
--- a/drivers/phy/tegra/xusb.h
+++ b/drivers/phy/tegra/xusb.h
@@ -411,6 +411,7 @@ struct tegra_xusb_padctl_ops {
 	int (*usb3_set_lfps_detect)(struct tegra_xusb_padctl *padctl,
 				    unsigned int index, bool enable);
 	int (*vbus_override)(struct tegra_xusb_padctl *padctl, bool set);
+	int (*id_override)(struct tegra_xusb_padctl *padctl, bool set);
 	int (*utmi_port_reset)(struct phy *phy);
 	void (*utmi_pad_power_on)(struct phy *phy);
 	void (*utmi_pad_power_down)(struct phy *phy);
diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h
index a0d3d5b7cf33..116d0c6609cb 100644
--- a/include/linux/phy/tegra/xusb.h
+++ b/include/linux/phy/tegra/xusb.h
@@ -21,6 +21,8 @@ int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl *padctl,
 					   unsigned int port, bool enable);
 int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl,
 					bool val);
+int tegra_xusb_padctl_set_id_override(struct tegra_xusb_padctl *padctl,
+				      bool val);
 void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy);
 void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy);
 int tegra_phy_xusb_utmi_port_reset(struct phy *phy);

-- 
2.52.0

[PATCH v2 5/6] phy: tegra: xusb: Move .set_mode() to a shared location

[Diogo Ivo]

As both Tegra210 and Tegra186 can have a common XUSB .set_mode()
implementation move it to a location where it can be used by both
platforms. Move Tegra210 to this common implementation.

While at it fix a typo in a comment.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
v1->v2:
- New patch
---
 drivers/phy/tegra/xusb-tegra210.c | 43 +--------------------------------------
 drivers/phy/tegra/xusb.c          | 40 ++++++++++++++++++++++++++++++++++++
 drivers/phy/tegra/xusb.h          |  2 ++
 3 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
index be03a17afd7e..14e24296641b 100644
--- a/drivers/phy/tegra/xusb-tegra210.c
+++ b/drivers/phy/tegra/xusb-tegra210.c
@@ -1910,47 +1910,6 @@ static int tegra210_xusb_padctl_id_override(struct tegra_xusb_padctl *padctl,
 	return 0;
 }
 
-static int tegra210_usb2_phy_set_mode(struct phy *phy, enum phy_mode mode,
-				      int submode)
-{
-	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
-	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
-	struct tegra_xusb_usb2_port *port = tegra_xusb_find_usb2_port(padctl,
-								lane->index);
-	int err = 0;
-
-	mutex_lock(&padctl->lock);
-
-	dev_dbg(&port->base.dev, "%s: mode %d", __func__, mode);
-
-	if (mode == PHY_MODE_USB_OTG) {
-		if (submode == USB_ROLE_HOST) {
-			tegra210_xusb_padctl_id_override(padctl, true);
-
-			err = regulator_enable(port->supply);
-		} else if (submode == USB_ROLE_DEVICE) {
-			tegra210_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
-			 * be enabled.
-			 */
-			if (port->role == USB_ROLE_HOST)
-				regulator_disable(port->supply);
-
-			tegra210_xusb_padctl_id_override(padctl, false);
-			tegra210_xusb_padctl_vbus_override(padctl, false);
-		}
-
-		port->role = submode;
-	}
-
-	mutex_unlock(&padctl->lock);
-
-	return err;
-}
-
 static int tegra210_usb2_phy_power_on(struct phy *phy)
 {
 	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
@@ -2174,7 +2133,7 @@ static const struct phy_ops tegra210_usb2_phy_ops = {
 	.exit = tegra210_usb2_phy_exit,
 	.power_on = tegra210_usb2_phy_power_on,
 	.power_off = tegra210_usb2_phy_power_off,
-	.set_mode = tegra210_usb2_phy_set_mode,
+	.set_mode = tegra_xusb_usb2_phy_set_mode,
 	.owner = THIS_MODULE,
 };
 
diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index 0443465bcf50..2327275740f8 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -770,6 +770,46 @@ bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl *padctl, int i
 }
 EXPORT_SYMBOL_GPL(tegra_xusb_usb2_port_wait_role_none);
 
+int tegra_xusb_usb2_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
+{
+	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
+	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
+	struct tegra_xusb_usb2_port *port = tegra_xusb_find_usb2_port(padctl,
+								lane->index);
+	int err = 0;
+
+	mutex_lock(&padctl->lock);
+
+	dev_dbg(&port->base.dev, "%s: mode %d", __func__, mode);
+
+	if (mode == PHY_MODE_USB_OTG) {
+		if (submode == USB_ROLE_HOST) {
+			tegra_xusb_padctl_set_id_override(padctl, true);
+
+			err = regulator_enable(port->supply);
+		} else if (submode == USB_ROLE_DEVICE) {
+			tegra_xusb_padctl_set_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 (port->role == USB_ROLE_HOST)
+				regulator_disable(port->supply);
+
+			tegra_xusb_padctl_set_id_override(padctl, false);
+			tegra_xusb_padctl_set_vbus_override(padctl, false);
+		}
+
+		port->role = submode;
+	}
+
+	mutex_unlock(&padctl->lock);
+
+	return err;
+}
+
 static int tegra_xusb_usb2_port_parse_dt(struct tegra_xusb_usb2_port *usb2)
 {
 	struct tegra_xusb_port *port = &usb2->base;
diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
index 08053a730d54..36cc87ae757e 100644
--- a/drivers/phy/tegra/xusb.h
+++ b/drivers/phy/tegra/xusb.h
@@ -501,6 +501,8 @@ struct tegra_xusb_lane *tegra_xusb_find_lane(struct tegra_xusb_padctl *padctl,
 					     const char *name,
 					     unsigned int index);
 
+int tegra_xusb_usb2_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode);
+
 #if defined(CONFIG_ARCH_TEGRA_124_SOC) || defined(CONFIG_ARCH_TEGRA_132_SOC)
 extern const struct tegra_xusb_padctl_soc tegra124_xusb_padctl_soc;
 #endif

-- 
2.52.0

[PATCH v2 6/6] phy: tegra: xusb: Move T186 .set_mode() to common implementation

[Diogo Ivo]

Move the Tegra186 PHY .set_mode() callback to a common implementation.
In order to do this first revert cefc1caee9dd.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
v1->v2:
- New patch
---
 drivers/phy/tegra/xusb-tegra186.c | 73 ++++++---------------------------------
 1 file changed, 10 insertions(+), 63 deletions(-)

diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
index bec9616c4a2e..bf678829245d 100644
--- a/drivers/phy/tegra/xusb-tegra186.c
+++ b/drivers/phy/tegra/xusb-tegra186.c
@@ -786,15 +786,13 @@ static int tegra186_xusb_padctl_vbus_override(struct tegra_xusb_padctl *padctl,
 }
 
 static int tegra186_xusb_padctl_id_override(struct tegra_xusb_padctl *padctl,
-					    struct tegra_xusb_usb2_port *port, bool status)
+					    bool status)
 {
-	u32 value, id_override;
-	int err = 0;
+	u32 value;
 
 	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) {
@@ -805,68 +803,16 @@ static int tegra186_xusb_padctl_id_override(struct tegra_xusb_padctl *padctl,
 			value = padctl_readl(padctl, USB2_VBUS_ID);
 		}
 
-		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;
-			}
-		}
+		value &= ~ID_OVERRIDE(~0);
+		value |= ID_OVERRIDE_GROUNDED;
 	} else {
-		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);
-		}
+		value &= ~ID_OVERRIDE(~0);
+		value |= ID_OVERRIDE_FLOATING;
 	}
 
-	return 0;
-}
-
-static int tegra186_utmi_phy_set_mode(struct phy *phy, enum phy_mode mode,
-				      int submode)
-{
-	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
-	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
-	struct tegra_xusb_usb2_port *port = tegra_xusb_find_usb2_port(padctl,
-								lane->index);
-	int err = 0;
-
-	mutex_lock(&padctl->lock);
+	padctl_writel(padctl, value, USB2_VBUS_ID);
 
-	dev_dbg(&port->base.dev, "%s: mode %d", __func__, mode);
-
-	if (mode == PHY_MODE_USB_OTG) {
-		if (submode == USB_ROLE_HOST) {
-			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) {
-			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;
+	return 0;
 }
 
 static int tegra186_utmi_phy_power_on(struct phy *phy)
@@ -1017,7 +963,7 @@ static const struct phy_ops utmi_phy_ops = {
 	.exit = tegra186_utmi_phy_exit,
 	.power_on = tegra186_utmi_phy_power_on,
 	.power_off = tegra186_utmi_phy_power_off,
-	.set_mode = tegra186_utmi_phy_set_mode,
+	.set_mode = tegra_xusb_usb2_phy_set_mode,
 	.owner = THIS_MODULE,
 };
 
@@ -1578,6 +1524,7 @@ static const struct tegra_xusb_padctl_ops tegra186_xusb_padctl_ops = {
 	.suspend_noirq = tegra186_xusb_padctl_suspend_noirq,
 	.resume_noirq = tegra186_xusb_padctl_resume_noirq,
 	.vbus_override = tegra186_xusb_padctl_vbus_override,
+	.id_override = tegra186_xusb_padctl_id_override,
 	.utmi_pad_power_on = tegra186_utmi_pad_power_on,
 	.utmi_pad_power_down = tegra186_utmi_pad_power_down,
 };

-- 
2.52.0

Re: [PATCH v2 0/6] Fixes to Tegra USB role switching and phy handling

[Diogo Ivo]

Hello,

Gentle ping on this series.

Best regards,
Diogo

On 1/27/26 15:11, Diogo Ivo wrote:
> Hello,
> 
> This patch series contains fixes/improvements for USB role switching on the
> Tegra210 and Tegra186 SoCs.
> 
> The first patch addresses a wrong check on the logic that disables the
> VBUS regulator.
> 
> The second patch removes a redundant mutex lock when setting the PHY
> mode.
> 
> The third patch guarantees proper ordering of events when switching PHY
> roles.
> 
> The remaining patches are included to standardize the PHY .set_mode()
> callback between Tegra186 and Tegra210.
> 
> With this patch series this feature can only be controlled from userspace,
> by writing the desired role to sysfs as
> 
> echo "role" > /sys/class/usb_role/usb2-0-role-switch/role
> 
> with role being one of {device, host, none}.
> 
> Further patches will enable automatic role switching via the 'cros_ec_typec'
> driver which is currently broken on Smaug.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
> Changes in v2:
> - Remove DT patches already taken to be upstreamed
> - Add standardization between Tegra210 and Tegra186
> - Address review comments from v1, detailed descriptions in each patch
> - Link to v1: https://lore.kernel.org/r/20251204-diogo-tegra_phy-v1-0-51a2016d0be8@tecnico.ulisboa.pt
> 
> ---
> Diogo Ivo (6):
>        phy: tegra: xusb: Fix USB2 port regulator disable logic
>        usb: xhci: tegra: Remove redundant mutex when setting phy mode
>        phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports
>        phy: tegra: xusb: Add ID override support to padctl
>        phy: tegra: xusb: Move .set_mode() to a shared location
>        phy: tegra: xusb: Move T186 .set_mode() to common implementation
> 
>   drivers/phy/tegra/xusb-tegra186.c   | 73 +++++----------------------------
>   drivers/phy/tegra/xusb-tegra210.c   | 42 +------------------
>   drivers/phy/tegra/xusb.c            | 80 +++++++++++++++++++++++++++++++++++++
>   drivers/phy/tegra/xusb.h            |  4 ++
>   drivers/usb/gadget/udc/tegra-xudc.c |  4 ++
>   drivers/usb/host/xhci-tegra.c       | 14 ++++---
>   include/linux/phy/tegra/xusb.h      |  3 ++
>   7 files changed, 111 insertions(+), 109 deletions(-)
> ---
> base-commit: b02a5530af8abe0d3cd4852ba48990716e962934
> change-id: 20251201-diogo-tegra_phy-86c89cab7377
> 
> Best regards,

Re: [PATCH v2 0/6] Fixes to Tegra USB role switching and phy handling

[Vinod Koul]

On 27-01-26, 15:11, Diogo Ivo wrote:
> Hello,
> 
> This patch series contains fixes/improvements for USB role switching on the
> Tegra210 and Tegra186 SoCs.

Thierry, Jonathan

can you folks check this and r-b/t-b please

-- 
~Vinod

Re: [PATCH v2 0/6] Fixes to Tegra USB role switching and phy handling

[Diogo Ivo]

Hello,

Ping on this series as it has been quite a while since this was sent out
originally.

Best regards,

Diogo

On 1/27/26 15:11, Diogo Ivo wrote:
> Hello,
> 
> This patch series contains fixes/improvements for USB role switching on the
> Tegra210 and Tegra186 SoCs.
> 
> The first patch addresses a wrong check on the logic that disables the
> VBUS regulator.
> 
> The second patch removes a redundant mutex lock when setting the PHY
> mode.
> 
> The third patch guarantees proper ordering of events when switching PHY
> roles.
> 
> The remaining patches are included to standardize the PHY .set_mode()
> callback between Tegra186 and Tegra210.
> 
> With this patch series this feature can only be controlled from userspace,
> by writing the desired role to sysfs as
> 
> echo "role" > /sys/class/usb_role/usb2-0-role-switch/role
> 
> with role being one of {device, host, none}.
> 
> Further patches will enable automatic role switching via the 'cros_ec_typec'
> driver which is currently broken on Smaug.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
> Changes in v2:
> - Remove DT patches already taken to be upstreamed
> - Add standardization between Tegra210 and Tegra186
> - Address review comments from v1, detailed descriptions in each patch
> - Link to v1: https://lore.kernel.org/r/20251204-diogo-tegra_phy-v1-0-51a2016d0be8@tecnico.ulisboa.pt
> 
> ---
> Diogo Ivo (6):
>        phy: tegra: xusb: Fix USB2 port regulator disable logic
>        usb: xhci: tegra: Remove redundant mutex when setting phy mode
>        phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports
>        phy: tegra: xusb: Add ID override support to padctl
>        phy: tegra: xusb: Move .set_mode() to a shared location
>        phy: tegra: xusb: Move T186 .set_mode() to common implementation
> 
>   drivers/phy/tegra/xusb-tegra186.c   | 73 +++++----------------------------
>   drivers/phy/tegra/xusb-tegra210.c   | 42 +------------------
>   drivers/phy/tegra/xusb.c            | 80 +++++++++++++++++++++++++++++++++++++
>   drivers/phy/tegra/xusb.h            |  4 ++
>   drivers/usb/gadget/udc/tegra-xudc.c |  4 ++
>   drivers/usb/host/xhci-tegra.c       | 14 ++++---
>   include/linux/phy/tegra/xusb.h      |  3 ++
>   7 files changed, 111 insertions(+), 109 deletions(-)
> ---
> base-commit: b02a5530af8abe0d3cd4852ba48990716e962934
> change-id: 20251201-diogo-tegra_phy-86c89cab7377
> 
> Best regards,

Re: [PATCH v2 6/6] phy: tegra: xusb: Move T186 .set_mode() to common implementation

[Jon Hunter]

On 27/01/2026 15:11, Diogo Ivo wrote:
> Move the Tegra186 PHY .set_mode() callback to a common implementation.
> In order to do this first revert cefc1caee9dd.

This commit message does not seem complete.

Furthermore, I am not sure why we want to revert cefc1caee9dd. We 
purposely moved the regulator_enable/disable into 
tegra186_xusb_padctl_id_override() because it is tied to setting the 
USB2_VBUS_ID. So I would prefer to keep it this way and move the 
Tegra210 implementation in the same direction (if possible).

Jon

-- 
nvpublic

Re: [PATCH v2 6/6] phy: tegra: xusb: Move T186 .set_mode() to common implementation

[Diogo Ivo]

On 3/24/26 10:16, Jon Hunter wrote:
> 
> On 27/01/2026 15:11, Diogo Ivo wrote:
>> Move the Tegra186 PHY .set_mode() callback to a common implementation.
>> In order to do this first revert cefc1caee9dd.
> 
> This commit message does not seem complete.

How so? It is succint but it states exactly what the commit does. It
reverts cefc1caee9dd and changes T186 to the common implementation
prepared in the previous patch.

> Furthermore, I am not sure why we want to revert cefc1caee9dd. We 
> purposely moved the regulator_enable/disable into 
> tegra186_xusb_padctl_id_override() because it is tied to setting the 
> USB2_VBUS_ID. So I would prefer to keep it this way and move the 
> Tegra210 implementation in the same direction (if possible).

I don't agree that this is the best solution.

We really benefit from a common implementation for the two platforms, not
only because of duplicate code but more importantly because without it
whenever a bug is found and fixed on either platform it most likely will
not be fixed on the other one. Case in point, cefc1caee9dd fixed a bug
on T186 but not the same bug on T210 (which then led to this series) since
the implementation was not shared among them. Were it the case that they
shared the implementation the fix would have come "free" for T210.

This will keep happening for as long as we have duplicate implementations,
which becomes more relevant since there is a severe lack of testing in
older Tegra platforms. I also thought about making the id_override()
implementation shared between T186 and T210 but that would be take more
changes since register definitions would need to be moved somewhere
else too.

Diogo

> Jon
> 

Re: [PATCH v2 2/6] usb: xhci: tegra: Remove redundant mutex when setting phy mode

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]

On Tue, Jan 27, 2026 at 03:11:48PM +0000, Diogo Ivo wrote:
> As the PHY subsystem already synchronizes concurrent accesses to a PHY
> instance with a core-internal mutex remove the driver specific mutex
> synchronization.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
> v1->v2:
> - New patch
> ---
>  drivers/usb/host/xhci-tegra.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 8b492871d21d..927861ca14f2 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -1357,15 +1357,11 @@ static void tegra_xhci_id_work(struct work_struct *work)
>  
>  	dev_dbg(tegra->dev, "host mode %s\n", str_on_off(tegra->host_mode));
>  
> -	mutex_lock(&tegra->lock);
> -
>  	if (tegra->host_mode)
>  		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
>  	else
>  		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
>  
> -	mutex_unlock(&tegra->lock);
> -

It looks to me like the mutex here is trying to protect against
tegra->host_mode changing while we're setting a different mode. That
doesn't seem to be taken care of by the PHY internal mutex.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 2/6] usb: xhci: tegra: Remove redundant mutex when setting phy mode

[Diogo Ivo]

On 3/24/26 11:48, Thierry Reding wrote:
> On Tue, Jan 27, 2026 at 03:11:48PM +0000, Diogo Ivo wrote:
>> As the PHY subsystem already synchronizes concurrent accesses to a PHY
>> instance with a core-internal mutex remove the driver specific mutex
>> synchronization.
>>
>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>> ---
>> v1->v2:
>> - New patch
>> ---
>>   drivers/usb/host/xhci-tegra.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
>> index 8b492871d21d..927861ca14f2 100644
>> --- a/drivers/usb/host/xhci-tegra.c
>> +++ b/drivers/usb/host/xhci-tegra.c
>> @@ -1357,15 +1357,11 @@ static void tegra_xhci_id_work(struct work_struct *work)
>>   
>>   	dev_dbg(tegra->dev, "host mode %s\n", str_on_off(tegra->host_mode));
>>   
>> -	mutex_lock(&tegra->lock);
>> -
>>   	if (tegra->host_mode)
>>   		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
>>   	else
>>   		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
>>   
>> -	mutex_unlock(&tegra->lock);
>> -
> 
> It looks to me like the mutex here is trying to protect against
> tegra->host_mode changing while we're setting a different mode. That
> doesn't seem to be taken care of by the PHY internal mutex.

I'm not sure I understand your point. Do you mean guarding against
tegra->host_mode changing in tegra_xhci_id_notify()? If so I don't see
how it would guard against that.

Diogo

> Thierry

Re: [PATCH v2 6/6] phy: tegra: xusb: Move T186 .set_mode() to common implementation

[Jon Hunter]

On 24/03/2026 11:31, Diogo Ivo wrote:
> 
> 
> On 3/24/26 10:16, Jon Hunter wrote:
>>
>> On 27/01/2026 15:11, Diogo Ivo wrote:
>>> Move the Tegra186 PHY .set_mode() callback to a common implementation.
>>> In order to do this first revert cefc1caee9dd.
>>
>> This commit message does not seem complete.
> 
> How so? It is succint but it states exactly what the commit does. It
> reverts cefc1caee9dd and changes T186 to the common implementation
> prepared in the previous patch.

It does not read clearly to me. The 2nd sentence sounds like that's all 
this is doing but we are not, we are reverting and doing the move.
>> Furthermore, I am not sure why we want to revert cefc1caee9dd. We 
>> purposely moved the regulator_enable/disable into 
>> tegra186_xusb_padctl_id_override() because it is tied to setting the 
>> USB2_VBUS_ID. So I would prefer to keep it this way and move the 
>> Tegra210 implementation in the same direction (if possible).
> 
> I don't agree that this is the best solution.
> 
> We really benefit from a common implementation for the two platforms, not
> only because of duplicate code but more importantly because without it
> whenever a bug is found and fixed on either platform it most likely will
> not be fixed on the other one. Case in point, cefc1caee9dd fixed a bug
> on T186 but not the same bug on T210 (which then led to this series) since
> the implementation was not shared among them. Were it the case that they
> shared the implementation the fix would have come "free" for T210.
> 
> This will keep happening for as long as we have duplicate implementations,
> which becomes more relevant since there is a severe lack of testing in
> older Tegra platforms. I also thought about making the id_override()
> implementation shared between T186 and T210 but that would be take more
> changes since register definitions would need to be moved somewhere
> else too.

I am all for a common implementation. I believe that in the 
tegra186_xusb_padctl_id_override() function the only thing that is 
different is the offset for the USB2_VBUS_ID register, which should be 
easy to handle.

Jon

-- 
nvpublic

Re: [PATCH v2 6/6] phy: tegra: xusb: Move T186 .set_mode() to common implementation

[Diogo Ivo]

On 3/24/26 13:33, Jon Hunter wrote:
> 
> 
> On 24/03/2026 11:31, Diogo Ivo wrote:
>>
>>
>> On 3/24/26 10:16, Jon Hunter wrote:
>>>
>>> On 27/01/2026 15:11, Diogo Ivo wrote:
>>>> Move the Tegra186 PHY .set_mode() callback to a common implementation.
>>>> In order to do this first revert cefc1caee9dd.
>>>
>>> This commit message does not seem complete.
>>
>> How so? It is succint but it states exactly what the commit does. It
>> reverts cefc1caee9dd and changes T186 to the common implementation
>> prepared in the previous patch.
> 
> It does not read clearly to me. The 2nd sentence sounds like that's all 
> this is doing but we are not, we are reverting and doing the move.
>>> Furthermore, I am not sure why we want to revert cefc1caee9dd. We 
>>> purposely moved the regulator_enable/disable into 
>>> tegra186_xusb_padctl_id_override() because it is tied to setting the 
>>> USB2_VBUS_ID. So I would prefer to keep it this way and move the 
>>> Tegra210 implementation in the same direction (if possible).
>>
>> I don't agree that this is the best solution.
>>
>> We really benefit from a common implementation for the two platforms, not
>> only because of duplicate code but more importantly because without it
>> whenever a bug is found and fixed on either platform it most likely will
>> not be fixed on the other one. Case in point, cefc1caee9dd fixed a bug
>> on T186 but not the same bug on T210 (which then led to this series) 
>> since
>> the implementation was not shared among them. Were it the case that they
>> shared the implementation the fix would have come "free" for T210.
>>
>> This will keep happening for as long as we have duplicate 
>> implementations,
>> which becomes more relevant since there is a severe lack of testing in
>> older Tegra platforms. I also thought about making the id_override()
>> implementation shared between T186 and T210 but that would be take more
>> changes since register definitions would need to be moved somewhere
>> else too.
> 
> I am all for a common implementation. I believe that in the 
> tegra186_xusb_padctl_id_override() function the only thing that is 
> different is the offset for the USB2_VBUS_ID register, which should be 
> easy to handle.

Ok, I can make it common there as well. However I still feel like
reverting cefc1caee9dd leads to cleaner code since vbus_override() and
id_override() will look similar and only do exactly what they state in
their names and the overall logic looks cleaner.

Diogo

> Jon
> 

Re: [PATCH v2 2/6] usb: xhci: tegra: Remove redundant mutex when setting phy mode

[Diogo Ivo]

Hello,

On 3/24/26 11:48, Thierry Reding wrote:
> On Tue, Jan 27, 2026 at 03:11:48PM +0000, Diogo Ivo wrote:
>> As the PHY subsystem already synchronizes concurrent accesses to a PHY
>> instance with a core-internal mutex remove the driver specific mutex
>> synchronization.
>>
>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>> ---
>> v1->v2:
>> - New patch
>> ---
>>   drivers/usb/host/xhci-tegra.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
>> index 8b492871d21d..927861ca14f2 100644
>> --- a/drivers/usb/host/xhci-tegra.c
>> +++ b/drivers/usb/host/xhci-tegra.c
>> @@ -1357,15 +1357,11 @@ static void tegra_xhci_id_work(struct work_struct *work)
>>   
>>   	dev_dbg(tegra->dev, "host mode %s\n", str_on_off(tegra->host_mode));
>>   
>> -	mutex_lock(&tegra->lock);
>> -
>>   	if (tegra->host_mode)
>>   		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
>>   	else
>>   		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
>>   
>> -	mutex_unlock(&tegra->lock);
>> -
> 
> It looks to me like the mutex here is trying to protect against
> tegra->host_mode changing while we're setting a different mode. That
> doesn't seem to be taken care of by the PHY internal mutex.

After taking another look at it I think I understand your point for the
mutex, but in that case wouldn't it also need to be held in the writer
of host_mode, tegra_xhci_id_notify()?

This patch has been picked up as-is into usb-next so it would be nice to
figure this out before it gets merged in the next merge window.

Diogo

> Thierry

Re: [PATCH v2 2/6] usb: xhci: tegra: Remove redundant mutex when setting phy mode

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2325 bytes --]

On Thu, Mar 26, 2026 at 02:17:33PM +0000, Diogo Ivo wrote:
> Hello,
> 
> On 3/24/26 11:48, Thierry Reding wrote:
> > On Tue, Jan 27, 2026 at 03:11:48PM +0000, Diogo Ivo wrote:
> > > As the PHY subsystem already synchronizes concurrent accesses to a PHY
> > > instance with a core-internal mutex remove the driver specific mutex
> > > synchronization.
> > > 
> > > Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> > > ---
> > > v1->v2:
> > > - New patch
> > > ---
> > >   drivers/usb/host/xhci-tegra.c | 4 ----
> > >   1 file changed, 4 deletions(-)
> > > 
> > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> > > index 8b492871d21d..927861ca14f2 100644
> > > --- a/drivers/usb/host/xhci-tegra.c
> > > +++ b/drivers/usb/host/xhci-tegra.c
> > > @@ -1357,15 +1357,11 @@ static void tegra_xhci_id_work(struct work_struct *work)
> > >   	dev_dbg(tegra->dev, "host mode %s\n", str_on_off(tegra->host_mode));
> > > -	mutex_lock(&tegra->lock);
> > > -
> > >   	if (tegra->host_mode)
> > >   		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
> > >   	else
> > >   		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
> > > -	mutex_unlock(&tegra->lock);
> > > -
> > 
> > It looks to me like the mutex here is trying to protect against
> > tegra->host_mode changing while we're setting a different mode. That
> > doesn't seem to be taken care of by the PHY internal mutex.
> 
> After taking another look at it I think I understand your point for the
> mutex, but in that case wouldn't it also need to be held in the writer
> of host_mode, tegra_xhci_id_notify()?

Yes, I think it probably would need to. I don't know how likely it is,
but I think the purpose of this is to protect against the ID notifier
firing quickly in succession. Although, given that this runs on a work
queue and work queue instances are non-reentrant to my knowledge, I
don't think we need the mutex here after all.

> This patch has been picked up as-is into usb-next so it would be nice to
> figure this out before it gets merged in the next merge window.

Given the above, I think it's fine. Maybe the commit message doesn't
give a correct reason for why we don't need the mutex, but the resulting
code looks like it should be fine regardless.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 6/6] phy: tegra: xusb: Move T186 .set_mode() to common implementation

[Jon Hunter]

On 24/03/2026 14:36, Diogo Ivo wrote:

...

> Ok, I can make it common there as well. However I still feel like
> reverting cefc1caee9dd leads to cleaner code since vbus_override() and
> id_override() will look similar and only do exactly what they state in
> their names and the overall logic looks cleaner.

Just so you know that while commit cefc1caee9dd was being prepared for
upstream submission, the following had been proposed for this ...

@@ -825,11 +826,11 @@ static int tegra186_utmi_phy_set_mode(struct phy *phy, enum phy_mode mode,
  			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.
+			 * The regulator is disabled only when the role transitions
+			 * from USB_ROLE_HOST to USB_ROLE_NONE.
  			 */
-			if (regulator_is_enabled(port->supply))
+			value = padctl_readl(padctl, USB2_VBUS_ID);
+			if (!(value & ID_OVERRIDE_FLOATING))
  				regulator_disable(port->supply);

This shows the relationship between ID override and the regulator and
hence it was moved into id_override(). This is different to your fix
in patch 5/6. So given that we have been using cefc1caee9dd now for
sometime, I don't wish to change the implementation unless there is a
valid reason.

Jon

-- 
nvpublic