[PATCH 0/2] USB Fixes for Gadget Mode on DWC3

[PATCH 0/2] USB Fixes for Gadget Mode on DWC3

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

In order to get gadget mode functional (for fastboot and ums) I need to
pull two patches in from mainline Linux. After applying these two
patches to U-Boot I am able to use ums and fastboot on my Anbernic
RG353P device for testing purposes.

Chris Morgan (2):
  usb: dwc3: gadget: Don't send unintended link state change
  usb: dwc3: core: improve reset sequence

 drivers/usb/dwc3/core.c   | 31 +++++++++++++++++++++----------
 drivers/usb/dwc3/gadget.c | 20 +++++++++-----------
 drivers/usb/dwc3/gadget.h | 14 ++++++++++++++
 3 files changed, 44 insertions(+), 21 deletions(-)

-- 
2.43.0

[PATCH 1/2] usb: dwc3: gadget: Don't send unintended link state change

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

DCTL.ULSTCHNGREQ is a write-only field. When doing a read-modify-write
to DCTL, the driver must make sure that there's no unintended link state
change request from whatever is read from DCTL.ULSTCHNGREQ. Set link
state change to no-action when the driver writes to DCTL.

Note that this patch was submitted upstream in Linux in 2020 [1],
and I've confirmed I need it in U-Boot to enable gadget mode.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch/drivers/usb/dwc3?id=5b738211fb59e114727381d07c647a77c0010996

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/usb/dwc3/gadget.c | 20 +++++++++-----------
 drivers/usb/dwc3/gadget.h | 14 ++++++++++++++
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 2b01113d54c..24ae0c232f6 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -62,7 +62,7 @@ int dwc3_gadget_set_test_mode(struct dwc3 *dwc, int mode)
 		return -EINVAL;
 	}
 
-	dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+	dwc3_gadget_dctl_write_safe(dwc, reg);
 
 	return 0;
 }
@@ -1396,7 +1396,7 @@ static int dwc3_gadget_set_selfpowered(struct usb_gadget *g,
 static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
 {
 	u32			reg;
-	u32			timeout = 500;
+	u32			timeout = 2000;
 
 	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
 	if (is_on) {
@@ -1422,9 +1422,10 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
 		dwc->pullups_connected = false;
 	}
 
-	dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+	dwc3_gadget_dctl_write_safe(dwc, reg);
 
 	do {
+		udelay(2000);
 		reg = dwc3_readl(dwc->regs, DWC3_DSTS);
 		if (is_on) {
 			if (!(reg & DWC3_DSTS_DEVCTRLHLT))
@@ -1436,7 +1437,6 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
 		timeout--;
 		if (!timeout)
 			return -ETIMEDOUT;
-		udelay(1);
 	} while (1);
 
 	dev_vdbg(dwc->dev, "gadget %s data soft-%s\n",
@@ -2137,10 +2137,8 @@ static void dwc3_gadget_disconnect_interrupt(struct dwc3 *dwc)
 
 	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
 	reg &= ~DWC3_DCTL_INITU1ENA;
-	dwc3_writel(dwc->regs, DWC3_DCTL, reg);
-
 	reg &= ~DWC3_DCTL_INITU2ENA;
-	dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+	dwc3_gadget_dctl_write_safe(dwc, reg);
 
 	dwc3_disconnect_gadget(dwc);
 	dwc->start_config_issued = false;
@@ -2189,7 +2187,7 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc)
 
 	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
 	reg &= ~DWC3_DCTL_TSTCTRL_MASK;
-	dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+	dwc3_gadget_dctl_write_safe(dwc, reg);
 	dwc->test_mode = false;
 
 	dwc3_stop_active_transfers(dwc);
@@ -2305,11 +2303,11 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 		if (dwc->has_lpm_erratum && dwc->revision >= DWC3_REVISION_240A)
 			reg |= DWC3_DCTL_LPM_ERRATA(dwc->lpm_nyet_threshold);
 
-		dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+		dwc3_gadget_dctl_write_safe(dwc, reg);
 	} else {
 		reg = dwc3_readl(dwc->regs, DWC3_DCTL);
 		reg &= ~DWC3_DCTL_HIRD_THRES_MASK;
-		dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+		dwc3_gadget_dctl_write_safe(dwc, reg);
 	}
 
 	dep = dwc->eps[0];
@@ -2417,7 +2415,7 @@ static void dwc3_gadget_linksts_change_interrupt(struct dwc3 *dwc,
 
 				reg &= ~u1u2;
 
-				dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+				dwc3_gadget_dctl_write_safe(dwc, reg);
 				break;
 			default:
 				/* do nothing */
diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
index f28a9755dcb..e4f5a096956 100644
--- a/drivers/usb/dwc3/gadget.h
+++ b/drivers/usb/dwc3/gadget.h
@@ -104,4 +104,18 @@ static inline u32 dwc3_gadget_ep_get_transfer_index(struct dwc3 *dwc, u8 number)
 	return DWC3_DEPCMD_GET_RSC_IDX(res_id);
 }
 
+/**
+ * dwc3_gadget_dctl_write_safe - write to DCTL safe from link state change
+ * @dwc: pointer to our context structure
+ * @value: value to write to DCTL
+ *
+ * Use this function when doing read-modify-write to DCTL. It will not
+ * send link state change request.
+ */
+static inline void dwc3_gadget_dctl_write_safe(struct dwc3 *dwc, u32 value)
+{
+	value &= ~DWC3_DCTL_ULSTCHNGREQ_MASK;
+	dwc3_writel(dwc->regs, DWC3_DCTL, value);
+}
+
 #endif /* __DRIVERS_USB_DWC3_GADGET_H */
-- 
2.43.0

Re: [PATCH 1/2] usb: dwc3: gadget: Don't send unintended link state change

[Mattijs Korpershoek]

Hi Chris,

Thank you for the patch and sorry for the review delay. I was on
vacation and a bit swamped when coming back.

On Tue, Dec 23, 2025 at 17:13, Chris Morgan <macroalpha82@gmail.com> wrote:

> From: Chris Morgan <macromorgan@hotmail.com>
>
> DCTL.ULSTCHNGREQ is a write-only field. When doing a read-modify-write
> to DCTL, the driver must make sure that there's no unintended link state
> change request from whatever is read from DCTL.ULSTCHNGREQ. Set link
> state change to no-action when the driver writes to DCTL.
>
> Note that this patch was submitted upstream in Linux in 2020 [1],
> and I've confirmed I need it in U-Boot to enable gadget mode.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch/drivers/usb/dwc3?id=5b738211fb59e114727381d07c647a77c0010996
>
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  drivers/usb/dwc3/gadget.c | 20 +++++++++-----------
>  drivers/usb/dwc3/gadget.h | 14 ++++++++++++++
>  2 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 2b01113d54c..24ae0c232f6 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -62,7 +62,7 @@ int dwc3_gadget_set_test_mode(struct dwc3 *dwc, int mode)
>  		return -EINVAL;
>  	}
>  
> -	dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> +	dwc3_gadget_dctl_write_safe(dwc, reg);
>  
>  	return 0;
>  }
> @@ -1396,7 +1396,7 @@ static int dwc3_gadget_set_selfpowered(struct usb_gadget *g,
>  static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
>  {
>  	u32			reg;
> -	u32			timeout = 500;
> +	u32			timeout = 2000;

This change (timeout increase) is not part of patch you linked in [1]

Can you either drop this or move this to a separate patch, explaining
why it's needed?

>  

[PATCH 2/2] usb: dwc3: core: improve reset sequence

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

According to Synopsys Databook, we shouldn't be
relying on GCTL.CORESOFTRESET bit as that's only for
debugging purposes. Instead, let's use DCTL.CSFTRST
if we're OTG or PERIPHERAL mode.

Host side block will be reset by XHCI driver if
necessary. Note that this reduces amount of time
spent on dwc3_probe() by a long margin.

We're still gonna wait for reset to finish for a
long time (default to 1ms max), but tests show that
the reset polling loop executed at most 19 times
(modprobe dwc3 && modprobe -r dwc3 executed 1000
times in a row).

Note that this patch was submitted to Linux in 2016 [1], however I can
confirm it is needed to support gadget mode in U-Boot on my device.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch/drivers/usb/dwc3?id=f59dcab176293b646e1358144c93c58c3cda2813

Suggested-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/usb/dwc3/core.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 847fa1f82c3..b4de2e95a0d 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -59,11 +59,20 @@ static void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
 static int dwc3_core_soft_reset(struct dwc3 *dwc)
 {
 	u32		reg;
+	int		retries = 1000;
 
-	/* Before Resetting PHY, put Core in Reset */
-	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
-	reg |= DWC3_GCTL_CORESOFTRESET;
-	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+	/*
+	 * We're resetting only the device side because, if we're in host mode,
+	 * XHCI driver will reset the host block. If dwc3 was configured for
+	 * host-only mode, then we can return early.
+	 */
+	if (dwc->dr_mode == USB_DR_MODE_HOST)
+		return 0;
+
+	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
+	reg |= DWC3_DCTL_CSFTRST;
+	reg &= ~DWC3_DCTL_RUN_STOP;
+	dwc3_gadget_dctl_write_safe(dwc, reg);
 
 	/* Assert USB3 PHY reset */
 	reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
@@ -87,12 +96,14 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
 	reg &= ~DWC3_GUSB2PHYCFG_PHYSOFTRST;
 	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
 
-	mdelay(100);
+	do {
+		reg = dwc3_readl(dwc->regs, DWC3_DCTL);
+		if (!(reg & DWC3_DCTL_CSFTRST))
+			return 0;
+		udelay(1);
+	} while (--retries);
 
-	/* After PHYs are stable we can take Core out of reset state */
-	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
-	reg &= ~DWC3_GCTL_CORESOFTRESET;
-	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+	return -ETIMEDOUT;
 
 	return 0;
 }
@@ -137,7 +148,7 @@ static void dwc3_ref_clk_period(struct dwc3 *dwc)
 
 	if (dwc->ref_clk) {
 		rate = clk_get_rate(dwc->ref_clk);
-		if (!rate)
+		if (!rate || (long)rate < 0)
 			return;
 		period = NSEC_PER_SEC / rate;
 	} else {
-- 
2.43.0

Re: [PATCH 2/2] usb: dwc3: core: improve reset sequence

[Mattijs Korpershoek]

Hi Chris,

Thank you for the patch and sorry for the review delay.

On Tue, Dec 23, 2025 at 17:13, Chris Morgan <macroalpha82@gmail.com> wrote:

> From: Chris Morgan <macromorgan@hotmail.com>
>
> According to Synopsys Databook, we shouldn't be
> relying on GCTL.CORESOFTRESET bit as that's only for
> debugging purposes. Instead, let's use DCTL.CSFTRST
> if we're OTG or PERIPHERAL mode.
>
> Host side block will be reset by XHCI driver if
> necessary. Note that this reduces amount of time
> spent on dwc3_probe() by a long margin.
>
> We're still gonna wait for reset to finish for a
> long time (default to 1ms max), but tests show that
> the reset polling loop executed at most 19 times
> (modprobe dwc3 && modprobe -r dwc3 executed 1000
> times in a row).
>
> Note that this patch was submitted to Linux in 2016 [1], however I can
> confirm it is needed to support gadget mode in U-Boot on my device.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch/drivers/usb/dwc3?id=f59dcab176293b646e1358144c93c58c3cda2813
>
> Suggested-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  drivers/usb/dwc3/core.c | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 847fa1f82c3..b4de2e95a0d 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -59,11 +59,20 @@ static void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
>  static int dwc3_core_soft_reset(struct dwc3 *dwc)
>  {
>  	u32		reg;
> +	int		retries = 1000;
>  
> -	/* Before Resetting PHY, put Core in Reset */
> -	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> -	reg |= DWC3_GCTL_CORESOFTRESET;
> -	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> +	/*
> +	 * We're resetting only the device side because, if we're in host mode,
> +	 * XHCI driver will reset the host block. If dwc3 was configured for
> +	 * host-only mode, then we can return early.
> +	 */
> +	if (dwc->dr_mode == USB_DR_MODE_HOST)
> +		return 0;
> +
> +	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> +	reg |= DWC3_DCTL_CSFTRST;
> +	reg &= ~DWC3_DCTL_RUN_STOP;
> +	dwc3_gadget_dctl_write_safe(dwc, reg);
>  
>  	/* Assert USB3 PHY reset */
>  	reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));

Looking more closely at [1], I can see that /* Assert USB3 PHY reset */
and  /* Assert USB2 PHY reset */  have also been removed in that patch
(in linux)

Can you please explain why it has not been removed in the U-Boot
version?
Please add details of that in the commit message.

> @@ -87,12 +96,14 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
>  	reg &= ~DWC3_GUSB2PHYCFG_PHYSOFTRST;
>  	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>  
> -	mdelay(100);
> +	do {
> +		reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> +		if (!(reg & DWC3_DCTL_CSFTRST))
> +			return 0;
> +		udelay(1);
> +	} while (--retries);
>  
> -	/* After PHYs are stable we can take Core out of reset state */
> -	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> -	reg &= ~DWC3_GCTL_CORESOFTRESET;
> -	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> +	return -ETIMEDOUT;
>  
>  	return 0;
>  }
> @@ -137,7 +148,7 @@ static void dwc3_ref_clk_period(struct dwc3 *dwc)
>  
>  	if (dwc->ref_clk) {
>  		rate = clk_get_rate(dwc->ref_clk);
> -		if (!rate)
> +		if (!rate || (long)rate < 0)

Is this a spurious change? it does not appear in the linux version of
the patch and also seems unrelated to soft reset.

>  			return;
>  		period = NSEC_PER_SEC / rate;
>  	} else {
> -- 
> 2.43.0

Re: [PATCH 2/2] usb: dwc3: core: improve reset sequence

[Chris Morgan]

On Wed, Jan 14, 2026 at 12:15:56PM +0100, Mattijs Korpershoek wrote:
> Hi Chris,
> 
> Thank you for the patch and sorry for the review delay.
> 
> On Tue, Dec 23, 2025 at 17:13, Chris Morgan <macroalpha82@gmail.com> wrote:
> 
> > From: Chris Morgan <macromorgan@hotmail.com>
> >
> > According to Synopsys Databook, we shouldn't be
> > relying on GCTL.CORESOFTRESET bit as that's only for
> > debugging purposes. Instead, let's use DCTL.CSFTRST
> > if we're OTG or PERIPHERAL mode.
> >
> > Host side block will be reset by XHCI driver if
> > necessary. Note that this reduces amount of time
> > spent on dwc3_probe() by a long margin.
> >
> > We're still gonna wait for reset to finish for a
> > long time (default to 1ms max), but tests show that
> > the reset polling loop executed at most 19 times
> > (modprobe dwc3 && modprobe -r dwc3 executed 1000
> > times in a row).
> >
> > Note that this patch was submitted to Linux in 2016 [1], however I can
> > confirm it is needed to support gadget mode in U-Boot on my device.
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch/drivers/usb/dwc3?id=f59dcab176293b646e1358144c93c58c3cda2813
> >
> > Suggested-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
> > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  drivers/usb/dwc3/core.c | 31 +++++++++++++++++++++----------
> >  1 file changed, 21 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 847fa1f82c3..b4de2e95a0d 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -59,11 +59,20 @@ static void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
> >  static int dwc3_core_soft_reset(struct dwc3 *dwc)
> >  {
> >  	u32		reg;
> > +	int		retries = 1000;
> >  
> > -	/* Before Resetting PHY, put Core in Reset */
> > -	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> > -	reg |= DWC3_GCTL_CORESOFTRESET;
> > -	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> > +	/*
> > +	 * We're resetting only the device side because, if we're in host mode,
> > +	 * XHCI driver will reset the host block. If dwc3 was configured for
> > +	 * host-only mode, then we can return early.
> > +	 */
> > +	if (dwc->dr_mode == USB_DR_MODE_HOST)
> > +		return 0;
> > +
> > +	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > +	reg |= DWC3_DCTL_CSFTRST;
> > +	reg &= ~DWC3_DCTL_RUN_STOP;
> > +	dwc3_gadget_dctl_write_safe(dwc, reg);
> >  
> >  	/* Assert USB3 PHY reset */
> >  	reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
> 
> Looking more closely at [1], I can see that /* Assert USB3 PHY reset */
> and  /* Assert USB2 PHY reset */  have also been removed in that patch
> (in linux)
> 
> Can you please explain why it has not been removed in the U-Boot
> version?
> Please add details of that in the commit message.

I don't remember honestly, I think I kept that there because I wasn't
entirely sure if it was still necessary by other boards or not. For v2
what I'm going to do instead is basically port the
dwc3_core_soft_reset() function as it exists today in the mainline
v6.19-rc5 branch and implement that here instead. So at least this
function will be identical to Linux. I can confirm doing so does work
on my board and continues to resolve the problems I had previously.

> 
> > @@ -87,12 +96,14 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> >  	reg &= ~DWC3_GUSB2PHYCFG_PHYSOFTRST;
> >  	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> >  
> > -	mdelay(100);
> > +	do {
> > +		reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > +		if (!(reg & DWC3_DCTL_CSFTRST))
> > +			return 0;
> > +		udelay(1);
> > +	} while (--retries);
> >  
> > -	/* After PHYs are stable we can take Core out of reset state */
> > -	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> > -	reg &= ~DWC3_GCTL_CORESOFTRESET;
> > -	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> > +	return -ETIMEDOUT;
> >  
> >  	return 0;
> >  }
> > @@ -137,7 +148,7 @@ static void dwc3_ref_clk_period(struct dwc3 *dwc)
> >  
> >  	if (dwc->ref_clk) {
> >  		rate = clk_get_rate(dwc->ref_clk);
> > -		if (!rate)
> > +		if (!rate || (long)rate < 0)
> 
> Is this a spurious change? it does not appear in the linux version of
> the patch and also seems unrelated to soft reset.
> 

Spurious change, the device works fine without it as far as I can tell.

> >  			return;
> >  		period = NSEC_PER_SEC / rate;
> >  	} else {
> > -- 
> > 2.43.0

Thank you,
Chris

Re: [PATCH 2/2] usb: dwc3: core: improve reset sequence

[Mattijs Korpershoek]

On Thu, Jan 15, 2026 at 16:05, Chris Morgan <macromorgan@hotmail.com> wrote:

> On Wed, Jan 14, 2026 at 12:15:56PM +0100, Mattijs Korpershoek wrote:
>> Hi Chris,
>> 
>> Thank you for the patch and sorry for the review delay.
>> 
>> On Tue, Dec 23, 2025 at 17:13, Chris Morgan <macroalpha82@gmail.com> wrote:
>> 
>> > From: Chris Morgan <macromorgan@hotmail.com>
>> >
>> > According to Synopsys Databook, we shouldn't be
>> > relying on GCTL.CORESOFTRESET bit as that's only for
>> > debugging purposes. Instead, let's use DCTL.CSFTRST
>> > if we're OTG or PERIPHERAL mode.
>> >
>> > Host side block will be reset by XHCI driver if
>> > necessary. Note that this reduces amount of time
>> > spent on dwc3_probe() by a long margin.
>> >
>> > We're still gonna wait for reset to finish for a
>> > long time (default to 1ms max), but tests show that
>> > the reset polling loop executed at most 19 times
>> > (modprobe dwc3 && modprobe -r dwc3 executed 1000
>> > times in a row).
>> >
>> > Note that this patch was submitted to Linux in 2016 [1], however I can
>> > confirm it is needed to support gadget mode in U-Boot on my device.
>> >
>> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch/drivers/usb/dwc3?id=f59dcab176293b646e1358144c93c58c3cda2813
>> >
>> > Suggested-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
>> > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
>> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>> > ---
>> >  drivers/usb/dwc3/core.c | 31 +++++++++++++++++++++----------
>> >  1 file changed, 21 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> > index 847fa1f82c3..b4de2e95a0d 100644
>> > --- a/drivers/usb/dwc3/core.c
>> > +++ b/drivers/usb/dwc3/core.c
>> > @@ -59,11 +59,20 @@ static void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
>> >  static int dwc3_core_soft_reset(struct dwc3 *dwc)
>> >  {
>> >  	u32		reg;
>> > +	int		retries = 1000;
>> >  
>> > -	/* Before Resetting PHY, put Core in Reset */
>> > -	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
>> > -	reg |= DWC3_GCTL_CORESOFTRESET;
>> > -	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
>> > +	/*
>> > +	 * We're resetting only the device side because, if we're in host mode,
>> > +	 * XHCI driver will reset the host block. If dwc3 was configured for
>> > +	 * host-only mode, then we can return early.
>> > +	 */
>> > +	if (dwc->dr_mode == USB_DR_MODE_HOST)
>> > +		return 0;
>> > +
>> > +	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>> > +	reg |= DWC3_DCTL_CSFTRST;
>> > +	reg &= ~DWC3_DCTL_RUN_STOP;
>> > +	dwc3_gadget_dctl_write_safe(dwc, reg);
>> >  
>> >  	/* Assert USB3 PHY reset */
>> >  	reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
>> 
>> Looking more closely at [1], I can see that /* Assert USB3 PHY reset */
>> and  /* Assert USB2 PHY reset */  have also been removed in that patch
>> (in linux)
>> 
>> Can you please explain why it has not been removed in the U-Boot
>> version?
>> Please add details of that in the commit message.
>
> I don't remember honestly, I think I kept that there because I wasn't
> entirely sure if it was still necessary by other boards or not. For v2
> what I'm going to do instead is basically port the
> dwc3_core_soft_reset() function as it exists today in the mainline
> v6.19-rc5 branch and implement that here instead. So at least this
> function will be identical to Linux. I can confirm doing so does work
> on my board and continues to resolve the problems I had previously.

Thanks, looking forward to v2!

>
>> 
>> > @@ -87,12 +96,14 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
>> >  	reg &= ~DWC3_GUSB2PHYCFG_PHYSOFTRST;
>> >  	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> >  
>> > -	mdelay(100);
>> > +	do {
>> > +		reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>> > +		if (!(reg & DWC3_DCTL_CSFTRST))
>> > +			return 0;
>> > +		udelay(1);
>> > +	} while (--retries);
>> >  
>> > -	/* After PHYs are stable we can take Core out of reset state */
>> > -	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
>> > -	reg &= ~DWC3_GCTL_CORESOFTRESET;
>> > -	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
>> > +	return -ETIMEDOUT;
>> >  
>> >  	return 0;
>> >  }
>> > @@ -137,7 +148,7 @@ static void dwc3_ref_clk_period(struct dwc3 *dwc)
>> >  
>> >  	if (dwc->ref_clk) {
>> >  		rate = clk_get_rate(dwc->ref_clk);
>> > -		if (!rate)
>> > +		if (!rate || (long)rate < 0)
>> 
>> Is this a spurious change? it does not appear in the linux version of
>> the patch and also seems unrelated to soft reset.
>> 
>
> Spurious change, the device works fine without it as far as I can tell.
>
>> >  			return;
>> >  		period = NSEC_PER_SEC / rate;
>> >  	} else {
>> > -- 
>> > 2.43.0
>
> Thank you,
> Chris