1. Patchew
  2. linux
  3. View series

[PATCH v4] usb: dwc3: Don't switch OTG -> peripheral if extcon is present

Andrey Smirnov posted 1 patch 4 years, 2 months ago 
drivers/usb/dwc3/core.c | 55 ++++++++++++++++++++++++++++++++++++++++-
drivers/usb/dwc3/drd.c  | 50 -------------------------------------
2 files changed, 54 insertions(+), 51 deletions(-)
[PATCH v4] usb: dwc3: Don't switch OTG -> peripheral if extcon is present
Posted by Andrey Smirnov 4 years, 2 months ago 
If the extcon device exists, get the mode from the extcon device. If
the controller is DRD and the driver is unable to determine the mode,
only then default the dr_mode to USB_DR_MODE_PERIPHERAL.

Cc: Felipe Balbi <balbi@kernel.org>
Cc: Thinh Nguyen <thinhn@synopsys.com>
Cc: linux-usb@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Changes since [v3] of the patch:

  - Rebased to apply on "main" of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git

Changes since [v2] of the patch:

   - Fixed "Minor formatting change" to reflect what was meant by
     review comment

Changes since [v1] of the patch:

   - Reworded commit message
   - Minor formatting change


[v3] https://lore.kernel.org/linux-usb/20220228171252.1827788-1-andrew.smirnov@gmail.com/
[v2] https://lore.kernel.org/linux-usb/20220221192020.346622-1-andrew.smirnov@gmail.com/
[v1] https://lore.kernel.org/linux-usb/20220206014532.372109-1-andrew.smirnov@gmail.com/T/#u

previons discussion:

https://lore.kernel.org/linux-usb/20220131192102.4115473-1-andrew.smirnov@gmail.com/


 drivers/usb/dwc3/core.c | 55 ++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/dwc3/drd.c  | 50 -------------------------------------
 2 files changed, 54 insertions(+), 51 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 1170b800acdc..39956846b16d 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -23,6 +23,7 @@
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include <linux/acpi.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/reset.h>
@@ -85,7 +86,7 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc)
 		 * mode. If the controller supports DRD but the dr_mode is not
 		 * specified or set to OTG, then set the mode to peripheral.
 		 */
-		if (mode == USB_DR_MODE_OTG &&
+		if (mode == USB_DR_MODE_OTG && !dwc->edev &&
 		    (!IS_ENABLED(CONFIG_USB_ROLE_SWITCH) ||
 		     !device_property_read_bool(dwc->dev, "usb-role-switch")) &&
 		    !DWC3_VER_IS_PRIOR(DWC3, 330A))
@@ -1632,6 +1633,51 @@ static void dwc3_check_params(struct dwc3 *dwc)
 	}
 }

+static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
+{
+	struct device *dev = dwc->dev;
+	struct device_node *np_phy;
+	struct extcon_dev *edev = NULL;
+	const char *name;
+
+	if (device_property_read_bool(dev, "extcon"))
+		return extcon_get_edev_by_phandle(dev, 0);
+
+	/*
+	 * Device tree platforms should get extcon via phandle.
+	 * On ACPI platforms, we get the name from a device property.
+	 * This device property is for kernel internal use only and
+	 * is expected to be set by the glue code.
+	 */
+	if (device_property_read_string(dev, "linux,extcon-name", &name) == 0) {
+		edev = extcon_get_extcon_dev(name);
+		if (!edev)
+			return ERR_PTR(-EPROBE_DEFER);
+
+		return edev;
+	}
+
+	/*
+	 * Try to get an extcon device from the USB PHY controller's "port"
+	 * node. Check if it has the "port" node first, to avoid printing the
+	 * error message from underlying code, as it's a valid case: extcon
+	 * device (and "port" node) may be missing in case of "usb-role-switch"
+	 * or OTG mode.
+	 */
+	np_phy = of_parse_phandle(dev->of_node, "phys", 0);
+	if (of_graph_is_present(np_phy)) {
+		struct device_node *np_conn;
+
+		np_conn = of_graph_get_remote_node(np_phy, -1, -1);
+		if (np_conn)
+			edev = extcon_find_edev_by_node(np_conn);
+		of_node_put(np_conn);
+	}
+	of_node_put(np_phy);
+
+	return edev;
+}
+
 static int dwc3_probe(struct platform_device *pdev)
 {
 	struct device		*dev = &pdev->dev;
@@ -1744,6 +1790,13 @@ static int dwc3_probe(struct platform_device *pdev)
 		goto err2;
 	}

+	dwc->edev = dwc3_get_extcon(dwc);
+	if (IS_ERR(dwc->edev)) {
+		ret = PTR_ERR(dwc->edev);
+		dev_err_probe(dwc->dev, ret, "failed to get extcon\n");
+		goto err3;
+	}
+
 	ret = dwc3_get_dr_mode(dwc);
 	if (ret)
 		goto err3;
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index b60b5f7b6dff..f277bebdaa09 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -8,7 +8,6 @@
  */

 #include <linux/extcon.h>
-#include <linux/of_graph.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
@@ -439,51 +438,6 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }

-static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
-{
-	struct device *dev = dwc->dev;
-	struct device_node *np_phy;
-	struct extcon_dev *edev = NULL;
-	const char *name;
-
-	if (device_property_read_bool(dev, "extcon"))
-		return extcon_get_edev_by_phandle(dev, 0);
-
-	/*
-	 * Device tree platforms should get extcon via phandle.
-	 * On ACPI platforms, we get the name from a device property.
-	 * This device property is for kernel internal use only and
-	 * is expected to be set by the glue code.
-	 */
-	if (device_property_read_string(dev, "linux,extcon-name", &name) == 0) {
-		edev = extcon_get_extcon_dev(name);
-		if (!edev)
-			return ERR_PTR(-EPROBE_DEFER);
-
-		return edev;
-	}
-
-	/*
-	 * Try to get an extcon device from the USB PHY controller's "port"
-	 * node. Check if it has the "port" node first, to avoid printing the
-	 * error message from underlying code, as it's a valid case: extcon
-	 * device (and "port" node) may be missing in case of "usb-role-switch"
-	 * or OTG mode.
-	 */
-	np_phy = of_parse_phandle(dev->of_node, "phys", 0);
-	if (of_graph_is_present(np_phy)) {
-		struct device_node *np_conn;
-
-		np_conn = of_graph_get_remote_node(np_phy, -1, -1);
-		if (np_conn)
-			edev = extcon_find_edev_by_node(np_conn);
-		of_node_put(np_conn);
-	}
-	of_node_put(np_phy);
-
-	return edev;
-}
-
 #if IS_ENABLED(CONFIG_USB_ROLE_SWITCH)
 #define ROLE_SWITCH 1
 static int dwc3_usb_role_switch_set(struct usb_role_switch *sw,
@@ -584,10 +538,6 @@ int dwc3_drd_init(struct dwc3 *dwc)
 {
 	int ret, irq;

-	dwc->edev = dwc3_get_extcon(dwc);
-	if (IS_ERR(dwc->edev))
-		return PTR_ERR(dwc->edev);
-
 	if (ROLE_SWITCH &&
 	    device_property_read_bool(dwc->dev, "usb-role-switch")) {
 		ret = dwc3_setup_role_switch(dwc);
--
2.25.1
Re: [PATCH v4] usb: dwc3: Don't switch OTG -> peripheral if extcon is present
Posted by Andy Shevchenko 3 years, 9 months ago 
On Sun, Apr 03, 2022 at 09:49:07AM -0700, Andrey Smirnov wrote:
> If the extcon device exists, get the mode from the extcon device. If
> the controller is DRD and the driver is unable to determine the mode,
> only then default the dr_mode to USB_DR_MODE_PERIPHERAL.

According to Ferry (Cc'ed) this broke Intel Merrifield platform. Ferry, can you
share bisect log?

Taking into account the late cycle, I would like to revert the change. And
Ferry and I would help to test any other (non-regressive) approach).

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v4] usb: dwc3: Don't switch OTG -> peripheral if extcon is present
Posted by Ferry Toth 3 years, 9 months ago 
Hi,

Resending this due to html reject, sorry.

On 22-09-2022 12:08, Andy Shevchenko wrote:
> On Sun, Apr 03, 2022 at 09:49:07AM -0700, Andrey Smirnov wrote:
>> If the extcon device exists, get the mode from the extcon device. If
>> the controller is DRD and the driver is unable to determine the mode,
>> only then default the dr_mode to USB_DR_MODE_PERIPHERAL.
> According to Ferry (Cc'ed) this broke Intel Merrifield platform. Ferry, can you
> share bisect log?

I can but not right now. But what I did was bisect between 5.18.0 (good) 
and 5.19.0 (bad) then when I got near the culprit (~20 remaining) based 
on the commit message I tried 0f01017191384e3962fa31520a9fd9846c3d352f 
"usb: dwc3: Don't switch OTG -> peripheral if extcon is present" (bad) 
and commit before that (good).

The effect of the patch is that on Merrifield (I tested with Intel 
Edison Arduino board which has a HW switch to select between host and 
device mode) device mode works but in host mode USB is completely not 
working.

Currently on host mode - when working - superfluous error messages from 
tusb1210 appear. When host mode is not working there are no tusb1210 
messages in the logs / on the console at all. Seemingly tusb1210 is not 
probed, which points in the direction of a relation to extcon.

> Taking into account the late cycle, I would like to revert the change. And
> Ferry and I would help to test any other (non-regressive) approach).
>
I have not yet tested if a simple revert fixes the problem but will tonight.


I would be happy to test other approaches too.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.