diff mbox series

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

Message ID 20220221192020.346622-1-andrew.smirnov@gmail.com
State Superseded
Headers show
Series [v2] usb: dwc3: Don't switch OTG -> peripheral if extcon is present | expand

Commit Message

Andrey Smirnov Feb. 21, 2022, 7:20 p.m. UTC
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>
---

v1 of the patch:

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 | 56 ++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/dwc3/drd.c  | 50 ------------------------------------
 2 files changed, 55 insertions(+), 51 deletions(-)

--
2.25.1

Comments

Thinh Nguyen Feb. 23, 2022, 12:58 a.m. UTC | #1
Hi,

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.
> 
> 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>
> ---
> 
> v1 of the patch:
> 
> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20220206014532.372109-1-andrew.smirnov@gmail.com/T/*u__;Iw!!A4F2R9G_pg!MkC0ZJrDw6TYoTkd6CcnR0EBo8D-uvDlqJDRFdaR82BX-16j0ydzGz0FJR4z4VQtdKhZ$ 
> 
> previons discussion:
> 
> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20220131192102.4115473-1-andrew.smirnov@gmail.com/__;!!A4F2R9G_pg!MkC0ZJrDw6TYoTkd6CcnR0EBo8D-uvDlqJDRFdaR82BX-16j0ydzGz0FJR4z4Syxv17D$ 
> 
> 
>  drivers/usb/dwc3/core.c | 56 ++++++++++++++++++++++++++++++++++++++++-
>  drivers/usb/dwc3/drd.c  | 50 ------------------------------------
>  2 files changed, 55 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index f2448d0a9d39..09f283288ce7 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>
> @@ -84,7 +85,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))
> @@ -1462,6 +1463,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;
> @@ -1561,6 +1607,14 @@ 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");

My comment earlier was meant to add "\n" to the error message above, not
adding a new line to the code.

Thanks,
Thinh

> +
> +		goto err3;
> +	}
> +
>  	ret = dwc3_get_dr_mode(dwc);
>  	if (ret)
>  		goto err3;
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index f2448d0a9d39..09f283288ce7 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>
@@ -84,7 +85,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))
@@ -1462,6 +1463,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;
@@ -1561,6 +1607,14 @@  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");
+
+		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 e2b68bb770d1..9a414edc439a 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/platform_device.h>
 #include <linux/property.h>

@@ -438,51 +437,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,
@@ -575,10 +529,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);