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

[Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>]

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