[PATCH 0/4] usb: dwc2: Add USB DRD support on CV1800 Series SoC

[PATCH 0/4] usb: dwc2: Add USB DRD support on CV1800 Series SoC

[Inochi Amaoto]

The current DRD implementation of DRD will cause a interrupt flow on
CV1800 Series SoC. It has two reasons:
1. The DRD does not sync the phy mode when change the controller mode,
   it is a violation of the DRD as it does not rely on a OTG phy.
2. The CV1800 is entered with a Session End Detected interrupt, but the
   DRD code has no change on it, so the controller will enter the
   interrupt storm.

This patch add fixes for these two problem, and I have tested this on
a Huashan PI board (it has a CV1812H SoC).

Note: This patches set does not resolve all the problems, as
      usb_add_hcd() will override the role set by dwc2, but I am not
      sure whether it should change skip_phy_initialization of hcd.
      So I kept it as it is and users still needs to set the role
      in the userspace as "role-switch-default-mode" in devicetree
      takes no effect.

Inochi Amaoto (4):
  usb: dwc2: gadget: honor pullup requests in core_connect
  usb: dwc2: drd: Sync the generic PHY mode with role requests
  usb: dwc2: Add USB DRD support on CV1800 series SoC
  riscv: dts: sophgo: Enable USB role switch on Huashan PI

 .../boot/dts/sophgo/cv1812h-huashan-pi.dts    |  4 +-
 drivers/usb/dwc2/core.h                       |  5 +-
 drivers/usb/dwc2/drd.c                        | 77 +++++++++++++++----
 drivers/usb/dwc2/gadget.c                     |  3 +-
 drivers/usb/dwc2/params.c                     |  1 +
 drivers/usb/dwc2/platform.c                   |  4 +-
 6 files changed, 73 insertions(+), 21 deletions(-)

--
2.56.0

[PATCH 1/4] usb: dwc2: gadget: honor pullup requests in core_connect

[Inochi Amaoto]

The connector ID work initializes the peripheral controller and calls
dwc2_hsotg_core_connect() even when no gadget driver has requested a
connection. After a host-to-device role change, this clears SFTDISCON
and lets the host enumerate an unconfigured gadget.

Check enabled in the connection helper. So the device will keep
disconnect until the controller is ready

Fixes: 510ffaa48eac ("usb: dwc2: Initialize the USB core for peripheral mode")
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/usb/dwc2/gadget.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 6c8a9f05fff0..5c8e85493289 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3664,7 +3664,8 @@ void dwc2_hsotg_core_disconnect(struct dwc2_hsotg *hsotg)
 void dwc2_hsotg_core_connect(struct dwc2_hsotg *hsotg)
 {
 	/* remove the soft-disconnect and let's go */
-	if (!hsotg->role_sw || (dwc2_readl(hsotg, GOTGCTL) & GOTGCTL_BSESVLD))
+	if (hsotg->enabled &&
+	    (!hsotg->role_sw || (dwc2_readl(hsotg, GOTGCTL) & GOTGCTL_BSESVLD)))
 		dwc2_clear_bit(hsotg, DCTL, DCTL_SFTDISCON);
 }
 
-- 
2.56.0

[PATCH 2/4] usb: dwc2: drd: Sync the generic PHY mode with role requests

[Inochi Amaoto]

The USB DRD does not reply on a OTG phy, which means the controller
should not reply on the switch behavior from PHY. Instead, it should
sync the phy state to avoid a brolen role.

Set the generic PHY mode after resolving the default role and before
updating the controller. And disable the controller clock if there
is a failure and it is disabled.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/usb/dwc2/core.h     |  2 +-
 drivers/usb/dwc2/drd.c      | 48 ++++++++++++++++++++++++++++++-------
 drivers/usb/dwc2/platform.c |  4 +++-
 3 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 015a5fb7842d..298b418e6859 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1400,7 +1400,7 @@ static inline int dwc2_is_device_mode(struct dwc2_hsotg *hsotg)
 
 int dwc2_drd_init(struct dwc2_hsotg *hsotg);
 void dwc2_drd_suspend(struct dwc2_hsotg *hsotg);
-void dwc2_drd_resume(struct dwc2_hsotg *hsotg);
+int dwc2_drd_resume(struct dwc2_hsotg *hsotg);
 void dwc2_drd_exit(struct dwc2_hsotg *hsotg);
 
 /*
diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
index 1ad8fa3f862a..1ad3804cbc80 100644
--- a/drivers/usb/dwc2/drd.c
+++ b/drivers/usb/dwc2/drd.c
@@ -9,6 +9,7 @@
 
 #include <linux/clk.h>
 #include <linux/iopoll.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/usb/role.h>
 #include "core.h"
@@ -88,6 +89,7 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
 	struct dwc2_hsotg *hsotg = usb_role_switch_get_drvdata(sw);
 	unsigned long flags;
 	int already = 0;
+	int ret = 0;
 
 	/* Skip session not in line with dr_mode */
 	if ((role == USB_ROLE_DEVICE && hsotg->dr_mode == USB_DR_MODE_HOST) ||
@@ -111,14 +113,11 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
 	 * mode and sessions. It is the case if cable is plugged at boot.
 	 */
 	if (!hsotg->ll_hw_enabled && hsotg->clk) {
-		int ret = clk_prepare_enable(hsotg->clk);
-
+		ret = clk_prepare_enable(hsotg->clk);
 		if (ret)
 			return ret;
 	}
 
-	spin_lock_irqsave(&hsotg->lock, flags);
-
 	if (role == USB_ROLE_NONE) {
 		/* default operation mode when usb role is USB_ROLE_NONE */
 		if (hsotg->role_sw_default_mode == USB_DR_MODE_HOST)
@@ -127,6 +126,15 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
 			role = USB_ROLE_DEVICE;
 	}
 
+	if (role == USB_ROLE_HOST || role == USB_ROLE_DEVICE) {
+		ret = phy_set_mode(hsotg->phy, role == USB_ROLE_HOST ?
+				   PHY_MODE_USB_HOST : PHY_MODE_USB_DEVICE);
+		if (ret)
+			goto out_clk;
+	}
+
+	spin_lock_irqsave(&hsotg->lock, flags);
+
 	if ((IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) ||
 	     IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)) &&
 	     dwc2_is_device_mode(hsotg) &&
@@ -160,26 +168,40 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
 		/* This will raise a Connector ID Status Change Interrupt */
 		dwc2_force_mode(hsotg, role == USB_ROLE_HOST);
 
-	if (!hsotg->ll_hw_enabled && hsotg->clk)
-		clk_disable_unprepare(hsotg->clk);
-
 	dev_dbg(hsotg->dev, "%s-session valid\n",
 		role == USB_ROLE_NONE ? "No" :
 		role == USB_ROLE_HOST ? "A" : "B");
 
-	return 0;
+out_clk:
+	if (!hsotg->ll_hw_enabled && hsotg->clk)
+		clk_disable_unprepare(hsotg->clk);
+
+	return ret;
 }
 
 int dwc2_drd_init(struct dwc2_hsotg *hsotg)
 {
 	struct usb_role_switch_desc role_sw_desc = {0};
 	struct usb_role_switch *role_sw;
+	bool is_host;
 	int ret;
 
 	if (!device_property_read_bool(hsotg->dev, "usb-role-switch"))
 		return 0;
 
 	hsotg->role_sw_default_mode = usb_get_role_switch_default_mode(hsotg->dev);
+
+	if (hsotg->role_sw_default_mode != USB_DR_MODE_UNKNOWN)
+		is_host = hsotg->dr_mode == USB_DR_MODE_HOST;
+	else
+		is_host = hsotg->role_sw_default_mode == USB_DR_MODE_HOST;
+
+	ret = phy_set_mode(hsotg->phy,
+			   is_host ? PHY_MODE_USB_HOST : PHY_MODE_USB_DEVICE);
+	if (ret)
+		return dev_err_probe(hsotg->dev, ret,
+				     "failed to set default PHY mode\n");
+
 	role_sw_desc.driver_data = hsotg;
 	role_sw_desc.fwnode = dev_fwnode(hsotg->dev);
 	role_sw_desc.set = dwc2_drd_role_sw_set;
@@ -214,10 +236,11 @@ void dwc2_drd_suspend(struct dwc2_hsotg *hsotg)
 	}
 }
 
-void dwc2_drd_resume(struct dwc2_hsotg *hsotg)
+int dwc2_drd_resume(struct dwc2_hsotg *hsotg)
 {
 	u32 gintsts, gintmsk;
 	enum usb_role role;
+	int ret;
 
 	if (hsotg->role_sw) {
 		/* get last known role (as the get ops isn't implemented by this driver) */
@@ -230,6 +253,11 @@ void dwc2_drd_resume(struct dwc2_hsotg *hsotg)
 				role = USB_ROLE_DEVICE;
 		}
 
+		ret = phy_set_mode(hsotg->phy, role == USB_ROLE_HOST ?
+				   PHY_MODE_USB_HOST : PHY_MODE_USB_DEVICE);
+		if (ret)
+			return ret;
+
 		/* restore last role that may have been lost */
 		if (role == USB_ROLE_HOST)
 			dwc2_ovr_avalid(hsotg, true);
@@ -250,6 +278,8 @@ void dwc2_drd_resume(struct dwc2_hsotg *hsotg)
 		gintmsk |= GINTSTS_CONIDSTSCHNG;
 		dwc2_writel(hsotg, gintmsk, GINTMSK);
 	}
+
+	return 0;
 }
 
 void dwc2_drd_exit(struct dwc2_hsotg *hsotg)
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index ef0d73077034..ab39d6b36b19 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -783,7 +783,9 @@ static int __maybe_unused dwc2_resume(struct device *dev)
 		/* Need to restore FORCEDEVMODE/FORCEHOSTMODE */
 		dwc2_force_dr_mode(dwc2);
 	} else {
-		dwc2_drd_resume(dwc2);
+		ret = dwc2_drd_resume(dwc2);
+		if (ret)
+			return ret;
 	}
 
 	if (dwc2_is_device_mode(dwc2))
-- 
2.56.0

[PATCH 3/4] usb: dwc2: Add USB DRD support on CV1800 series SoC

[Inochi Amaoto]

On CV1800 series SoC, it has a problem that it enters the role switch
with a persistent Session End Detected interrupt. As a result, GOTGINT
bit 2 remains set after write-one-to-clear accesses and prevents a
normal boot.

Keeping the BVALID override asserted in host mode avoids the observed
interrupt storm while allowing host enumeration and device operation.
As the BVALOVAL field is device only in the datasheet, this change is
added as a quirk.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/usb/dwc2/core.h   |  3 +++
 drivers/usb/dwc2/drd.c    | 29 +++++++++++++++++++++--------
 drivers/usb/dwc2/params.c |  1 +
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 298b418e6859..52ac64f67f41 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -357,6 +357,8 @@ enum dwc2_ep0_state {
  *                      case.
  *                      0 - No (default)
  *                      1 - Yes
+ * @force_bvalid_in_host: Set the BVALID override value to 1 in host mode to
+ *                       avoid persistent Session End Detected interrupts.
  * @power_down:         Specifies whether the controller support power_down.
  *			If power_down is enabled, the controller will enter
  *			power_down in both peripheral and host mode when
@@ -460,6 +462,7 @@ struct dwc2_core_params {
 	bool reload_ctl;
 	bool uframe_sched;
 	bool external_id_pin_ctl;
+	bool force_bvalid_in_host;
 
 	int power_down;
 #define DWC2_POWER_DOWN_PARAM_NONE		0
diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
index 1ad3804cbc80..4f7441a87c85 100644
--- a/drivers/usb/dwc2/drd.c
+++ b/drivers/usb/dwc2/drd.c
@@ -28,10 +28,13 @@ static void dwc2_ovr_init(struct dwc2_hsotg *hsotg)
 	gotgctl = dwc2_readl(hsotg, GOTGCTL);
 	dwc2_ovr_gotgctl(gotgctl);
 	gotgctl &= ~(GOTGCTL_BVALOVAL | GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL);
-	if (hsotg->role_sw_default_mode == USB_DR_MODE_HOST)
+	if (hsotg->role_sw_default_mode == USB_DR_MODE_HOST) {
 		gotgctl |= GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL;
-	else if (hsotg->role_sw_default_mode == USB_DR_MODE_PERIPHERAL)
+		if (hsotg->params.force_bvalid_in_host)
+			gotgctl |= GOTGCTL_BVALOVAL;
+	} else if (hsotg->role_sw_default_mode == USB_DR_MODE_PERIPHERAL) {
 		gotgctl |= GOTGCTL_BVALOVAL | GOTGCTL_VBVALOVAL;
+	}
 	dwc2_writel(hsotg, gotgctl, GOTGCTL);
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
@@ -45,18 +48,26 @@ static int dwc2_ovr_avalid(struct dwc2_hsotg *hsotg, bool valid)
 	u32 gotgctl = dwc2_readl(hsotg, GOTGCTL);
 
 	/* Check if A-Session is already in the right state */
-	if ((valid && (gotgctl & GOTGCTL_ASESVLD)) ||
+	if ((valid && (gotgctl & GOTGCTL_ASESVLD) &&
+	     (!hsotg->params.force_bvalid_in_host ||
+	      (dwc2_is_host_mode(hsotg) && (gotgctl & GOTGCTL_BVALOVAL) &&
+	       (gotgctl & GOTGCTL_AVALOVAL)))) ||
 	    (!valid && !(gotgctl & GOTGCTL_ASESVLD)))
 		return -EALREADY;
 
 	/* Always enable overrides to handle the resume case */
 	dwc2_ovr_gotgctl(gotgctl);
 
-	gotgctl &= ~GOTGCTL_BVALOVAL;
-	if (valid)
+	if (valid) {
 		gotgctl |= GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL;
-	else
-		gotgctl &= ~(GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL);
+		if (hsotg->params.force_bvalid_in_host)
+			gotgctl |= GOTGCTL_BVALOVAL;
+		else
+			gotgctl &= ~GOTGCTL_BVALOVAL;
+	} else {
+		gotgctl &= ~(GOTGCTL_BVALOVAL | GOTGCTL_AVALOVAL |
+			     GOTGCTL_VBVALOVAL);
+	}
 	dwc2_writel(hsotg, gotgctl, GOTGCTL);
 
 	return 0;
@@ -67,7 +78,9 @@ static int dwc2_ovr_bvalid(struct dwc2_hsotg *hsotg, bool valid)
 	u32 gotgctl = dwc2_readl(hsotg, GOTGCTL);
 
 	/* Check if B-Session is already in the right state */
-	if ((valid && (gotgctl & GOTGCTL_BSESVLD)) ||
+	if ((valid && (gotgctl & GOTGCTL_BSESVLD) &&
+	     (!hsotg->params.force_bvalid_in_host ||
+	      (dwc2_is_device_mode(hsotg) && !(gotgctl & GOTGCTL_AVALOVAL)))) ||
 	    (!valid && !(gotgctl & GOTGCTL_BSESVLD)))
 		return -EALREADY;
 
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 091bfcfef753..16db9c3c1991 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -222,6 +222,7 @@ static void dwc2_set_cv1800_params(struct dwc2_hsotg *hsotg)
 {
 	struct dwc2_core_params *p = &hsotg->params;
 
+	p->force_bvalid_in_host = true;
 	p->otg_caps.hnp_support = false;
 	p->otg_caps.srp_support = false;
 	p->host_dma = false;
-- 
2.56.0

[PATCH 4/4] riscv: dts: sophgo: Enable USB role switch on Huashan PI

[Inochi Amaoto]

As current the DWC2 support role switch on CV18XX SoCs. Enable it
on Huashan PI.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 arch/riscv/boot/dts/sophgo/cv1812h-huashan-pi.dts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/boot/dts/sophgo/cv1812h-huashan-pi.dts b/arch/riscv/boot/dts/sophgo/cv1812h-huashan-pi.dts
index aedf79f47407..bf3d58c355c3 100644
--- a/arch/riscv/boot/dts/sophgo/cv1812h-huashan-pi.dts
+++ b/arch/riscv/boot/dts/sophgo/cv1812h-huashan-pi.dts
@@ -88,6 +88,8 @@ &uart0 {
 };
 
 &usb {
-	dr_mode = "host";
+	dr_mode = "otg";
+	usb-role-switch;
+	role-switch-default-mode = "host";
 	status = "okay";
 };
-- 
2.56.0

Re: [PATCH 4/4] riscv: dts: sophgo: Enable USB role switch on Huashan PI

[sashiko-bot]

> As current the DWC2 support role switch on CV18XX SoCs. Enable it
> on Huashan PI.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110070627.217946-1-inochiama@gmail.com?part=4

Re: [PATCH 2/4] usb: dwc2: drd: Sync the generic PHY mode with role requests

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] usb: dwc2: drd: inverted default mode check in dwc2_drd_init()
- [Medium] usb: dwc2: drd: unupdated PHY mode on USB_ROLE_NONE in
  dwc2_drd_role_sw_set()
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [High] usb: dwc2: early role switch registration race in
  dwc2_driver_probe() [1]
--

commit 5fa24c9321d10a0419f734968d84804314b53dd6
Author: Inochi Amaoto <inochiama@gmail.com>

usb: dwc2: drd: Sync the generic PHY mode with role requests

This commit updates the generic PHY mode via phy_set_mode() during DRD
initialization, role switch updates, and system resume.

> diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
> index 1ad8fa3f862a1..1ad3804cbc805 100644
> --- a/drivers/usb/dwc2/drd.c
> +++ b/drivers/usb/dwc2/drd.c

[ ... ]

> @@ -111,14 +113,11 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
>  	 * mode and sessions. It is the case if cable is plugged at boot.
>  	 */
>  	if (!hsotg->ll_hw_enabled && hsotg->clk) {
> -		int ret = clk_prepare_enable(hsotg->clk);
> -
> +		ret = clk_prepare_enable(hsotg->clk);
>  		if (ret)
>  			return ret;
>  	}
>  
> -	spin_lock_irqsave(&hsotg->lock, flags);
> -
>  	if (role == USB_ROLE_NONE) {
>  		/* default operation mode when usb role is USB_ROLE_NONE */
>  		if (hsotg->role_sw_default_mode == USB_DR_MODE_HOST)
> @@ -127,6 +126,15 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
>  			role = USB_ROLE_DEVICE;
>  	}
>  
> +	if (role == USB_ROLE_HOST || role == USB_ROLE_DEVICE) {
> +		ret = phy_set_mode(hsotg->phy, role == USB_ROLE_HOST ?
> +				   PHY_MODE_USB_HOST : PHY_MODE_USB_DEVICE);
> +		if (ret)
> +			goto out_clk;
> +	}

[Severity: Medium]
Does guarding phy_set_mode() on USB_ROLE_HOST || USB_ROLE_DEVICE leave
hsotg->phy stuck in PHY_MODE_USB_HOST when switching from USB_ROLE_HOST to
USB_ROLE_NONE?

When hsotg->role_sw_default_mode is USB_DR_MODE_UNKNOWN and hsotg->dr_mode
is USB_DR_MODE_OTG, role remains USB_ROLE_NONE.  This skips phy_set_mode(),
whereas dwc2_force_mode(hsotg, role == USB_ROLE_HOST) below forces the
controller into device mode (matching dwc2_drd_init() and
dwc2_drd_resume(), which both set PHY_MODE_USB_DEVICE for USB_ROLE_NONE).

[ ... ]

> @@ -160,26 +168,40 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
>  		/* This will raise a Connector ID Status Change Interrupt */
>  		dwc2_force_mode(hsotg, role == USB_ROLE_HOST);

[Severity: High]
This problem wasn't introduced by this patch, but registering the role
switch in dwc2_drd_init() before dwc2_driver_probe() finishes initializing
the core and workqueue allows concurrent role switches to race with probe
[1]:

CPU0 (probe)                      CPU1 (role switch / IRQ)
dwc2_driver_probe()
  dwc2_drd_init()
    usb_role_switch_register()
                                  dwc2_drd_role_sw_set()
                                    dwc2_force_mode()
                                      /* unlocked RMW on GUSBCFG */
  dwc2_core_init()
    /* RMW on GUSBCFG */
                                  dwc2_handle_conn_id_status_change_intr()
                                    if (hsotg->wq_otg) /* NULL */
                                      queue_work(...)  <-- event dropped

dwc2_force_mode() modifies GUSBCFG without holding hsotg->lock, racing with
dwc2_core_init().  In addition, because hsotg->wq_otg is not allocated yet,
dwc2_handle_conn_id_status_change_intr() drops the connector ID status
change interrupt, leaving the controller in a mismatched state.

>  
> -	if (!hsotg->ll_hw_enabled && hsotg->clk)
> -		clk_disable_unprepare(hsotg->clk);
> -
>  	dev_dbg(hsotg->dev, "%s-session valid\n",
>  		role == USB_ROLE_NONE ? "No" :
>  		role == USB_ROLE_HOST ? "A" : "B");
>  
> -	return 0;
> +out_clk:
> +	if (!hsotg->ll_hw_enabled && hsotg->clk)
> +		clk_disable_unprepare(hsotg->clk);
> +
> +	return ret;
>  }
>  
>  int dwc2_drd_init(struct dwc2_hsotg *hsotg)
>  {
>  	struct usb_role_switch_desc role_sw_desc = {0};
>  	struct usb_role_switch *role_sw;
> +	bool is_host;
>  	int ret;
>  
>  	if (!device_property_read_bool(hsotg->dev, "usb-role-switch"))
>  		return 0;
>  
>  	hsotg->role_sw_default_mode = usb_get_role_switch_default_mode(hsotg->dev);
> +
> +	if (hsotg->role_sw_default_mode != USB_DR_MODE_UNKNOWN)
> +		is_host = hsotg->dr_mode == USB_DR_MODE_HOST;
> +	else
> +		is_host = hsotg->role_sw_default_mode == USB_DR_MODE_HOST;
> +
> +	ret = phy_set_mode(hsotg->phy,
> +			   is_host ? PHY_MODE_USB_HOST : PHY_MODE_USB_DEVICE);

[Severity: High]
Are the if and else branches inverted here?

When role_sw_default_mode is configured (for example USB_DR_MODE_HOST on an
OTG controller where dr_mode is USB_DR_MODE_OTG), the if branch ignores
role_sw_default_mode and checks dr_mode instead.  When role_sw_default_mode
is USB_DR_MODE_UNKNOWN, the else branch evaluates USB_DR_MODE_UNKNOWN ==
USB_DR_MODE_HOST, which is always false.  This sets hsotg->phy to
PHY_MODE_USB_DEVICE while dwc2_ovr_init() at the end of dwc2_drd_init()
forces the controller into host mode:

drivers/usb/dwc2/drd.c:dwc2_ovr_init() {
    ...
	dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST) ||
				(hsotg->role_sw_default_mode == USB_DR_MODE_HOST));
}

[1] https://sashiko.dev/#/bug/linux-a825f666-6a3b-40af-8377-023f8afa7caf

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110070627.217946-1-inochiama@gmail.com?part=2

Re: [PATCH 1/4] usb: dwc2: gadget: honor pullup requests in core_connect

[sashiko-bot]

> The connector ID work initializes the peripheral controller and calls
> dwc2_hsotg_core_connect() even when no gadget driver has requested a
> connection. After a host-to-device role change, this clears SFTDISCON
> and lets the host enumerate an unconfigured gadget.
> 
> Check enabled in the connection helper. So the device will keep
> disconnect until the controller is ready
> 
> Fixes: 510ffaa48eac ("usb: dwc2: Initialize the USB core for peripheral mode")
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110070627.217946-1-inochiama@gmail.com?part=1

Re: [PATCH 2/4] usb: dwc2: drd: Sync the generic PHY mode with role requests

[Inochi Amaoto]

On Tue, Nov 10, 2026 at 07:18:48AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issues to consider:
> - [High] usb: dwc2: drd: inverted default mode check in dwc2_drd_init()
> - [Medium] usb: dwc2: drd: unupdated PHY mode on USB_ROLE_NONE in
>   dwc2_drd_role_sw_set()
> It also noticed 1 related pre-existing issue this patch does not
> introduce:
> - [High] usb: dwc2: early role switch registration race in
>   dwc2_driver_probe() [1]
> --
> 
> commit 5fa24c9321d10a0419f734968d84804314b53dd6
> Author: Inochi Amaoto <inochiama@gmail.com>
> 
> usb: dwc2: drd: Sync the generic PHY mode with role requests
> 
> This commit updates the generic PHY mode via phy_set_mode() during DRD
> initialization, role switch updates, and system resume.
> 
> > diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
> > index 1ad8fa3f862a1..1ad3804cbc805 100644
> > --- a/drivers/usb/dwc2/drd.c
> > +++ b/drivers/usb/dwc2/drd.c
> 
> [ ... ]
> 
> > @@ -111,14 +113,11 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
> >  	 * mode and sessions. It is the case if cable is plugged at boot.
> >  	 */
> >  	if (!hsotg->ll_hw_enabled && hsotg->clk) {
> > -		int ret = clk_prepare_enable(hsotg->clk);
> > -
> > +		ret = clk_prepare_enable(hsotg->clk);
> >  		if (ret)
> >  			return ret;
> >  	}
> >  
> > -	spin_lock_irqsave(&hsotg->lock, flags);
> > -
> >  	if (role == USB_ROLE_NONE) {
> >  		/* default operation mode when usb role is USB_ROLE_NONE */
> >  		if (hsotg->role_sw_default_mode == USB_DR_MODE_HOST)
> > @@ -127,6 +126,15 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
> >  			role = USB_ROLE_DEVICE;
> >  	}
> >  
> > +	if (role == USB_ROLE_HOST || role == USB_ROLE_DEVICE) {
> > +		ret = phy_set_mode(hsotg->phy, role == USB_ROLE_HOST ?
> > +				   PHY_MODE_USB_HOST : PHY_MODE_USB_DEVICE);
> > +		if (ret)
> > +			goto out_clk;
> > +	}
> 
> [Severity: Medium]
> Does guarding phy_set_mode() on USB_ROLE_HOST || USB_ROLE_DEVICE leave
> hsotg->phy stuck in PHY_MODE_USB_HOST when switching from USB_ROLE_HOST to
> USB_ROLE_NONE?
> 
> When hsotg->role_sw_default_mode is USB_DR_MODE_UNKNOWN and hsotg->dr_mode
> is USB_DR_MODE_OTG, role remains USB_ROLE_NONE.  This skips phy_set_mode(),
> whereas dwc2_force_mode(hsotg, role == USB_ROLE_HOST) below forces the
> controller into device mode (matching dwc2_drd_init() and
> dwc2_drd_resume(), which both set PHY_MODE_USB_DEVICE for USB_ROLE_NONE).
> 

Yes, the check can be removed

> [ ... ]
> 
> > @@ -160,26 +168,40 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
> >  		/* This will raise a Connector ID Status Change Interrupt */
> >  		dwc2_force_mode(hsotg, role == USB_ROLE_HOST);
> 
> [Severity: High]
> This problem wasn't introduced by this patch, but registering the role
> switch in dwc2_drd_init() before dwc2_driver_probe() finishes initializing
> the core and workqueue allows concurrent role switches to race with probe
> [1]:
> 
> CPU0 (probe)                      CPU1 (role switch / IRQ)
> dwc2_driver_probe()
>   dwc2_drd_init()
>     usb_role_switch_register()
>                                   dwc2_drd_role_sw_set()
>                                     dwc2_force_mode()
>                                       /* unlocked RMW on GUSBCFG */
>   dwc2_core_init()
>     /* RMW on GUSBCFG */
>                                   dwc2_handle_conn_id_status_change_intr()
>                                     if (hsotg->wq_otg) /* NULL */
>                                       queue_work(...)  <-- event dropped
> 
> dwc2_force_mode() modifies GUSBCFG without holding hsotg->lock, racing with
> dwc2_core_init().  In addition, because hsotg->wq_otg is not allocated yet,
> dwc2_handle_conn_id_status_change_intr() drops the connector ID status
> change interrupt, leaving the controller in a mismatched state.
> 
> >  
> > -	if (!hsotg->ll_hw_enabled && hsotg->clk)
> > -		clk_disable_unprepare(hsotg->clk);
> > -
> >  	dev_dbg(hsotg->dev, "%s-session valid\n",
> >  		role == USB_ROLE_NONE ? "No" :
> >  		role == USB_ROLE_HOST ? "A" : "B");
> >  
> > -	return 0;
> > +out_clk:
> > +	if (!hsotg->ll_hw_enabled && hsotg->clk)
> > +		clk_disable_unprepare(hsotg->clk);
> > +
> > +	return ret;
> >  }
> >  
> >  int dwc2_drd_init(struct dwc2_hsotg *hsotg)
> >  {
> >  	struct usb_role_switch_desc role_sw_desc = {0};
> >  	struct usb_role_switch *role_sw;
> > +	bool is_host;
> >  	int ret;
> >  
> >  	if (!device_property_read_bool(hsotg->dev, "usb-role-switch"))
> >  		return 0;
> >  
> >  	hsotg->role_sw_default_mode = usb_get_role_switch_default_mode(hsotg->dev);
> > +
> > +	if (hsotg->role_sw_default_mode != USB_DR_MODE_UNKNOWN)
> > +		is_host = hsotg->dr_mode == USB_DR_MODE_HOST;
> > +	else
> > +		is_host = hsotg->role_sw_default_mode == USB_DR_MODE_HOST;
> > +
> > +	ret = phy_set_mode(hsotg->phy,
> > +			   is_host ? PHY_MODE_USB_HOST : PHY_MODE_USB_DEVICE);
> 
> [Severity: High]
> Are the if and else branches inverted here?
> 
> When role_sw_default_mode is configured (for example USB_DR_MODE_HOST on an
> OTG controller where dr_mode is USB_DR_MODE_OTG), the if branch ignores
> role_sw_default_mode and checks dr_mode instead.  When role_sw_default_mode
> is USB_DR_MODE_UNKNOWN, the else branch evaluates USB_DR_MODE_UNKNOWN ==
> USB_DR_MODE_HOST, which is always false.  This sets hsotg->phy to
> PHY_MODE_USB_DEVICE while dwc2_ovr_init() at the end of dwc2_drd_init()
> forces the controller into host mode:
> 
> drivers/usb/dwc2/drd.c:dwc2_ovr_init() {
>     ...
> 	dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST) ||
> 				(hsotg->role_sw_default_mode == USB_DR_MODE_HOST));
> }
> 

Yes, it is inverted.

I will fix them in the next version.

Regards,
Inochi

> [1] https://sashiko.dev/#/bug/linux-a825f666-6a3b-40af-8377-023f8afa7caf
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261110070627.217946-1-inochiama@gmail.com?part=2

Re: [PATCH 3/4] usb: dwc2: Add USB DRD support on CV1800 series SoC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] usb: dwc2: BVALOVAL cleared in host mode in
  dwc2_ovr_init()/dwc2_ovr_avalid()
- [High] usb: dwc2: inverted role_sw_default_mode check in
  dwc2_drd_init()
--

commit 46e81140c7728597dbfee9abd2cadd6fb5aee6a2
Author: Inochi Amaoto <inochiama@gmail.com>

usb: dwc2: Add USB DRD support on CV1800 series SoC

This commit adds a force_bvalid_in_host quirk parameter to keep the BVALID
override asserted in host mode and enables it on Sophgo CV1800 series SoCs
to avoid a persistent Session End Detected interrupt.

> diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
> index 1ad3804cbc805..4f7441a87c857 100644
> --- a/drivers/usb/dwc2/drd.c
> +++ b/drivers/usb/dwc2/drd.c
> @@ -28,10 +28,13 @@ static void dwc2_ovr_init(struct dwc2_hsotg *hsotg)
>  	gotgctl = dwc2_readl(hsotg, GOTGCTL);
>  	dwc2_ovr_gotgctl(gotgctl);
>  	gotgctl &= ~(GOTGCTL_BVALOVAL | GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL);
> -	if (hsotg->role_sw_default_mode == USB_DR_MODE_HOST)
> +	if (hsotg->role_sw_default_mode == USB_DR_MODE_HOST) {
>  		gotgctl |= GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL;
> -	else if (hsotg->role_sw_default_mode == USB_DR_MODE_PERIPHERAL)
> +		if (hsotg->params.force_bvalid_in_host)
> +			gotgctl |= GOTGCTL_BVALOVAL;
> +	} else if (hsotg->role_sw_default_mode == USB_DR_MODE_PERIPHERAL) {
>  		gotgctl |= GOTGCTL_BVALOVAL | GOTGCTL_VBVALOVAL;
> +	}
>  	dwc2_writel(hsotg, gotgctl, GOTGCTL);
>  
>  	spin_unlock_irqrestore(&hsotg->lock, flags);

[Severity: High]
Are the if/else branches inverted for the role_sw_default_mode check in
dwc2_drd_init() introduced by commit "usb: dwc2: drd: Sync the generic PHY
mode with role requests"?

drivers/usb/dwc2/drd.c:dwc2_drd_init() {
    ...
	if (hsotg->role_sw_default_mode != USB_DR_MODE_UNKNOWN)
		is_host = hsotg->dr_mode == USB_DR_MODE_HOST;
	else
		is_host = hsotg->role_sw_default_mode == USB_DR_MODE_HOST;

	ret = phy_set_mode(hsotg->phy,
			   is_host ? PHY_MODE_USB_HOST : PHY_MODE_USB_DEVICE);
    ...
}

When dr_mode is USB_DR_MODE_OTG and role_sw_default_mode is
USB_DR_MODE_HOST, is_host evaluates to false and phy_set_mode() puts the
PHY into PHY_MODE_USB_DEVICE, while dwc2_ovr_init() then configures
GOTGCTL and forces the controller into host mode.  Likewise, when
role_sw_default_mode is USB_DR_MODE_UNKNOWN, the else branch compares
USB_DR_MODE_UNKNOWN against USB_DR_MODE_HOST, so is_host is false even
when dr_mode is USB_DR_MODE_HOST.

> @@ -45,18 +48,26 @@ static int dwc2_ovr_avalid(struct dwc2_hsotg *hsotg, bool valid)
>  	u32 gotgctl = dwc2_readl(hsotg, GOTGCTL);
>  
>  	/* Check if A-Session is already in the right state */
> -	if ((valid && (gotgctl & GOTGCTL_ASESVLD)) ||
> +	if ((valid && (gotgctl & GOTGCTL_ASESVLD) &&
> +	     (!hsotg->params.force_bvalid_in_host ||
> +	      (dwc2_is_host_mode(hsotg) && (gotgctl & GOTGCTL_BVALOVAL) &&
> +	       (gotgctl & GOTGCTL_AVALOVAL)))) ||
>  	    (!valid && !(gotgctl & GOTGCTL_ASESVLD)))
>  		return -EALREADY;
>  
>  	/* Always enable overrides to handle the resume case */
>  	dwc2_ovr_gotgctl(gotgctl);
>  
> -	gotgctl &= ~GOTGCTL_BVALOVAL;
> -	if (valid)
> +	if (valid) {
>  		gotgctl |= GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL;
> -	else
> -		gotgctl &= ~(GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL);
> +		if (hsotg->params.force_bvalid_in_host)
> +			gotgctl |= GOTGCTL_BVALOVAL;
> +		else
> +			gotgctl &= ~GOTGCTL_BVALOVAL;
> +	} else {
> +		gotgctl &= ~(GOTGCTL_BVALOVAL | GOTGCTL_AVALOVAL |
> +			     GOTGCTL_VBVALOVAL);
> +	}
>  	dwc2_writel(hsotg, gotgctl, GOTGCTL);
>  
>  	return 0;

[Severity: High]
Can dwc2_ovr_avalid(hsotg, false) and dwc2_ovr_init() still leave
GOTGCTL_BVALOVAL cleared while the controller is in host mode when
force_bvalid_in_host is set?

In dwc2_drd_role_sw_set(), switching from USB_ROLE_HOST to USB_ROLE_NONE
calls dwc2_ovr_avalid(hsotg, false) and clears GOTGCTL_BVALOVAL while the
controller is still in host mode, then drops hsotg->lock and re-enables
interrupts before dwc2_force_mode() switches the controller to device mode:

drivers/usb/dwc2/drd.c:dwc2_drd_role_sw_set() {
    ...
		} else {
			dwc2_ovr_avalid(hsotg, false);
		}
	}

	spin_unlock_irqrestore(&hsotg->lock, flags);

	if (!already && hsotg->dr_mode == USB_DR_MODE_OTG)
		/* This will raise a Connector ID Status Change Interrupt */
		dwc2_force_mode(hsotg, role == USB_ROLE_HOST);
    ...
}

On CV1800, having GOTGCTL_BVALOVAL cleared with GOTGCTL_BVALOEN enabled
while in host mode triggers the un-clearable GOTGINT_SES_END_DET interrupt
storm before dwc2_force_mode() completes (or permanently if dr_mode is
USB_DR_MODE_HOST).

Similarly, dwc2_ovr_init() forces host mode when either dr_mode or
role_sw_default_mode is USB_DR_MODE_HOST, but only sets GOTGCTL_BVALOVAL
when role_sw_default_mode is USB_DR_MODE_HOST, leaving GOTGCTL_BVALOVAL
cleared in host mode if dr_mode is USB_DR_MODE_HOST and
role_sw_default_mode is not.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110070627.217946-1-inochiama@gmail.com?part=3

Re: [PATCH 4/4] riscv: dts: sophgo: Enable USB role switch on Huashan PI

[Krzysztof Kozlowski]

On 10/11/2026 08:06, Inochi Amaoto wrote:
> As current the DWC2 support role switch on CV18XX SoCs. Enable it
> on Huashan PI.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  arch/riscv/boot/dts/sophgo/cv1812h-huashan-pi.dts | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

DTS cannot be combined in this patchset and must be split. Don't mix
patches from independent subsystems when you send something explicitly
targeting only one subsystem.
See also: Documentation/devicetree/bindings/submitting-patches.rst


Best regards,
Krzysztof

Re: [PATCH 4/4] riscv: dts: sophgo: Enable USB role switch on Huashan PI

[Inochi Amaoto]

On Tue, Nov 10, 2026 at 11:41:37AM +0100, Krzysztof Kozlowski wrote:
> On 10/11/2026 08:06, Inochi Amaoto wrote:
> > As current the DWC2 support role switch on CV18XX SoCs. Enable it
> > on Huashan PI.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> >  arch/riscv/boot/dts/sophgo/cv1812h-huashan-pi.dts | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> DTS cannot be combined in this patchset and must be split. Don't mix
> patches from independent subsystems when you send something explicitly
> targeting only one subsystem.
> See also: Documentation/devicetree/bindings/submitting-patches.rst
> 
> 
> Best regards,
> Krzysztof


Sorry for this mistake, I will spilt them in the next patch. Thanks.

Regards,
Inochi