[PATCH] usb: dwc3: Always deassert xilinx resets

[PATCH] usb: dwc3: Always deassert xilinx resets

[Sean Anderson]

If we don't have a usb3 phy we don't need to assert the core resets.
Deassert them even if we didn't assert them to support booting when the
bootloader never released the core from reset.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 drivers/usb/dwc3/dwc3-xilinx.c | 67 ++++++++++++++++------------------
 1 file changed, 32 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
index 0a8c47876ff9..f41b0da5e89d 100644
--- a/drivers/usb/dwc3/dwc3-xilinx.c
+++ b/drivers/usb/dwc3/dwc3-xilinx.c
@@ -132,21 +132,6 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
 		goto err;
 	}
 
-	/*
-	 * The following core resets are not required unless a USB3 PHY
-	 * is used, and the subsequent register settings are not required
-	 * unless a core reset is performed (they should be set properly
-	 * by the first-stage boot loader, but may be reverted by a core
-	 * reset). They may also break the configuration if USB3 is actually
-	 * in use but the usb3-phy entry is missing from the device tree.
-	 * Therefore, skip these operations in this case.
-	 */
-	if (!priv_data->usb3_phy) {
-		/* Deselect the PIPE Clock Select bit in FPD PIPE Clock register */
-		writel(PIPE_CLK_DESELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
-		goto skip_usb3_phy;
-	}
-
 	crst = devm_reset_control_get_exclusive(dev, "usb_crst");
 	if (IS_ERR(crst)) {
 		ret = PTR_ERR(crst);
@@ -171,22 +156,31 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
 		goto err;
 	}
 
-	ret = reset_control_assert(crst);
-	if (ret < 0) {
-		dev_err(dev, "Failed to assert core reset\n");
-		goto err;
-	}
+	/*
+	 * Asserting the core resets is not required unless a USB3 PHY is used.
+	 * They may also break the configuration if USB3 is actually in use but
+	 * the usb3-phy entry is missing from the device tree. Therefore, skip
+	 * a full reset cycle and just deassert the resets if the phy is
+	 * absent.
+	 */
+	if (priv_data->usb3_phy) {
+		ret = reset_control_assert(crst);
+		if (ret < 0) {
+			dev_err(dev, "Failed to assert core reset\n");
+			goto err;
+		}
 
-	ret = reset_control_assert(hibrst);
-	if (ret < 0) {
-		dev_err(dev, "Failed to assert hibernation reset\n");
-		goto err;
-	}
+		ret = reset_control_assert(hibrst);
+		if (ret < 0) {
+			dev_err(dev, "Failed to assert hibernation reset\n");
+			goto err;
+		}
 
-	ret = reset_control_assert(apbrst);
-	if (ret < 0) {
-		dev_err(dev, "Failed to assert APB reset\n");
-		goto err;
+		ret = reset_control_assert(apbrst);
+		if (ret < 0) {
+			dev_err(dev, "Failed to assert APB reset\n");
+			goto err;
+		}
 	}
 
 	ret = phy_init(priv_data->usb3_phy);
@@ -201,11 +195,15 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
 		goto err;
 	}
 
-	/* Set PIPE Power Present signal in FPD Power Present Register*/
-	writel(FPD_POWER_PRSNT_OPTION, priv_data->regs + XLNX_USB_FPD_POWER_PRSNT);
-
-	/* Set the PIPE Clock Select bit in FPD PIPE Clock register */
-	writel(PIPE_CLK_SELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
+	if (priv_data->usb3_phy) {
+		/* Set PIPE Power Present signal in FPD Power Present Register*/
+		writel(FPD_POWER_PRSNT_OPTION, priv_data->regs + XLNX_USB_FPD_POWER_PRSNT);
+		/* Set the PIPE Clock Select bit in FPD PIPE Clock register */
+		writel(PIPE_CLK_SELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
+	} else {
+		/* Deselect the PIPE Clock Select bit in FPD PIPE Clock register */
+		writel(PIPE_CLK_DESELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
+	}
 
 	ret = reset_control_deassert(crst);
 	if (ret < 0) {
@@ -225,7 +223,6 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
 		goto err;
 	}
 
-skip_usb3_phy:
 	/* ulpi reset via gpio-modepin or gpio-framework driver */
 	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(reset_gpio)) {
-- 
2.35.1.1320.gc452695387.dirty

Re: [PATCH] usb: dwc3: Always deassert xilinx resets

[Thinh Nguyen]

On Tue, Jan 06, 2026, Sean Anderson wrote:
> If we don't have a usb3 phy we don't need to assert the core resets.
> Deassert them even if we didn't assert them to support booting when the
> bootloader never released the core from reset.
> 

> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> 
>  drivers/usb/dwc3/dwc3-xilinx.c | 67 ++++++++++++++++------------------
>  1 file changed, 32 insertions(+), 35 deletions(-)
> 

This sounds like a fix. Does it need to be Cc Stable and backported?

Regardless,

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

> diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
> index 0a8c47876ff9..f41b0da5e89d 100644
> --- a/drivers/usb/dwc3/dwc3-xilinx.c
> +++ b/drivers/usb/dwc3/dwc3-xilinx.c
> @@ -132,21 +132,6 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>  		goto err;
>  	}
>  
> -	/*
> -	 * The following core resets are not required unless a USB3 PHY
> -	 * is used, and the subsequent register settings are not required
> -	 * unless a core reset is performed (they should be set properly
> -	 * by the first-stage boot loader, but may be reverted by a core
> -	 * reset). They may also break the configuration if USB3 is actually
> -	 * in use but the usb3-phy entry is missing from the device tree.
> -	 * Therefore, skip these operations in this case.
> -	 */
> -	if (!priv_data->usb3_phy) {
> -		/* Deselect the PIPE Clock Select bit in FPD PIPE Clock register */
> -		writel(PIPE_CLK_DESELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
> -		goto skip_usb3_phy;
> -	}
> -
>  	crst = devm_reset_control_get_exclusive(dev, "usb_crst");
>  	if (IS_ERR(crst)) {
>  		ret = PTR_ERR(crst);
> @@ -171,22 +156,31 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>  		goto err;
>  	}
>  
> -	ret = reset_control_assert(crst);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to assert core reset\n");
> -		goto err;
> -	}
> +	/*
> +	 * Asserting the core resets is not required unless a USB3 PHY is used.
> +	 * They may also break the configuration if USB3 is actually in use but
> +	 * the usb3-phy entry is missing from the device tree. Therefore, skip
> +	 * a full reset cycle and just deassert the resets if the phy is
> +	 * absent.
> +	 */
> +	if (priv_data->usb3_phy) {
> +		ret = reset_control_assert(crst);
> +		if (ret < 0) {
> +			dev_err(dev, "Failed to assert core reset\n");
> +			goto err;
> +		}
>  
> -	ret = reset_control_assert(hibrst);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to assert hibernation reset\n");
> -		goto err;
> -	}
> +		ret = reset_control_assert(hibrst);
> +		if (ret < 0) {
> +			dev_err(dev, "Failed to assert hibernation reset\n");
> +			goto err;
> +		}
>  
> -	ret = reset_control_assert(apbrst);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to assert APB reset\n");
> -		goto err;
> +		ret = reset_control_assert(apbrst);
> +		if (ret < 0) {
> +			dev_err(dev, "Failed to assert APB reset\n");
> +			goto err;
> +		}
>  	}
>  
>  	ret = phy_init(priv_data->usb3_phy);
> @@ -201,11 +195,15 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>  		goto err;
>  	}
>  
> -	/* Set PIPE Power Present signal in FPD Power Present Register*/
> -	writel(FPD_POWER_PRSNT_OPTION, priv_data->regs + XLNX_USB_FPD_POWER_PRSNT);
> -
> -	/* Set the PIPE Clock Select bit in FPD PIPE Clock register */
> -	writel(PIPE_CLK_SELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
> +	if (priv_data->usb3_phy) {
> +		/* Set PIPE Power Present signal in FPD Power Present Register*/
> +		writel(FPD_POWER_PRSNT_OPTION, priv_data->regs + XLNX_USB_FPD_POWER_PRSNT);
> +		/* Set the PIPE Clock Select bit in FPD PIPE Clock register */
> +		writel(PIPE_CLK_SELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
> +	} else {
> +		/* Deselect the PIPE Clock Select bit in FPD PIPE Clock register */
> +		writel(PIPE_CLK_DESELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
> +	}
>  
>  	ret = reset_control_deassert(crst);
>  	if (ret < 0) {
> @@ -225,7 +223,6 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>  		goto err;
>  	}
>  
> -skip_usb3_phy:
>  	/* ulpi reset via gpio-modepin or gpio-framework driver */
>  	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>  	if (IS_ERR(reset_gpio)) {
> -- 
> 2.35.1.1320.gc452695387.dirty
> 

RE: [PATCH] usb: dwc3: Always deassert xilinx resets

[Pandey, Radhey Shyam]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> Sent: Friday, January 9, 2026 6:19 AM
> To: Sean Anderson <sean.anderson@linux.dev>
> Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>; open list:DESIGNWARE
> USB3 DRD IP DRIVER <linux-usb@vger.kernel.org>; Frager, Neal
> <neal.frager@amd.com>; Simek, Michal <michal.simek@amd.com>; open list
> <linux-kernel@vger.kernel.org>; moderated list:ARM/ZYNQ ARCHITECTURE
> <linux-arm-kernel@lists.infradead.org>; Philipp Zabel <p.zabel@pengutronix.de>;
> Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>
> Subject: Re: [PATCH] usb: dwc3: Always deassert xilinx resets
>
> On Tue, Jan 06, 2026, Sean Anderson wrote:
> > If we don't have a usb3 phy we don't need to assert the core resets.
> > Deassert them even if we didn't assert them to support booting when
> > the bootloader never released the core from reset.
Is it a customized bootloader ? i.e it assert reset but don't deassert.

I think ideally core /APB reset should be done independent on
MAC 2.0/3.0 configuration. Not sure where the recommendation is
coming from to only do it for 3.0 MAC. Let me check the IP spec.
Thinh: Please chime in with your thoughts.


> >
>
> > Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> > ---
> >
> >  drivers/usb/dwc3/dwc3-xilinx.c | 67
> > ++++++++++++++++------------------
> >  1 file changed, 32 insertions(+), 35 deletions(-)
> >
>
> This sounds like a fix. Does it need to be Cc Stable and backported?
>
> Regardless,
>
> Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>
> Thanks,
> Thinh
>
> > diff --git a/drivers/usb/dwc3/dwc3-xilinx.c
> > b/drivers/usb/dwc3/dwc3-xilinx.c index 0a8c47876ff9..f41b0da5e89d
> > 100644
> > --- a/drivers/usb/dwc3/dwc3-xilinx.c
> > +++ b/drivers/usb/dwc3/dwc3-xilinx.c
> > @@ -132,21 +132,6 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx
> *priv_data)
> >             goto err;
> >     }
> >
> > -   /*
> > -    * The following core resets are not required unless a USB3 PHY
> > -    * is used, and the subsequent register settings are not required
> > -    * unless a core reset is performed (they should be set properly
> > -    * by the first-stage boot loader, but may be reverted by a core
> > -    * reset). They may also break the configuration if USB3 is actually
> > -    * in use but the usb3-phy entry is missing from the device tree.
> > -    * Therefore, skip these operations in this case.
> > -    */
> > -   if (!priv_data->usb3_phy) {
> > -           /* Deselect the PIPE Clock Select bit in FPD PIPE Clock register */
> > -           writel(PIPE_CLK_DESELECT, priv_data->regs +
> XLNX_USB_FPD_PIPE_CLK);
> > -           goto skip_usb3_phy;
> > -   }
> > -
> >     crst = devm_reset_control_get_exclusive(dev, "usb_crst");
> >     if (IS_ERR(crst)) {
> >             ret = PTR_ERR(crst);
> > @@ -171,22 +156,31 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx
> *priv_data)
> >             goto err;
> >     }
> >
> > -   ret = reset_control_assert(crst);
> > -   if (ret < 0) {
> > -           dev_err(dev, "Failed to assert core reset\n");
> > -           goto err;
> > -   }
> > +   /*
> > +    * Asserting the core resets is not required unless a USB3 PHY is used.
> > +    * They may also break the configuration if USB3 is actually in use but
> > +    * the usb3-phy entry is missing from the device tree. Therefore, skip
> > +    * a full reset cycle and just deassert the resets if the phy is
> > +    * absent.
> > +    */
> > +   if (priv_data->usb3_phy) {
> > +           ret = reset_control_assert(crst);
> > +           if (ret < 0) {
> > +                   dev_err(dev, "Failed to assert core reset\n");
> > +                   goto err;
> > +           }
> >
> > -   ret = reset_control_assert(hibrst);
> > -   if (ret < 0) {
> > -           dev_err(dev, "Failed to assert hibernation reset\n");
> > -           goto err;
> > -   }
> > +           ret = reset_control_assert(hibrst);
> > +           if (ret < 0) {
> > +                   dev_err(dev, "Failed to assert hibernation reset\n");
> > +                   goto err;
> > +           }
> >
> > -   ret = reset_control_assert(apbrst);
> > -   if (ret < 0) {
> > -           dev_err(dev, "Failed to assert APB reset\n");
> > -           goto err;
> > +           ret = reset_control_assert(apbrst);
> > +           if (ret < 0) {
> > +                   dev_err(dev, "Failed to assert APB reset\n");
> > +                   goto err;
> > +           }
> >     }
> >
> >     ret = phy_init(priv_data->usb3_phy); @@ -201,11 +195,15 @@ static
> > int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
> >             goto err;
> >     }
> >
> > -   /* Set PIPE Power Present signal in FPD Power Present Register*/
> > -   writel(FPD_POWER_PRSNT_OPTION, priv_data->regs +
> XLNX_USB_FPD_POWER_PRSNT);
> > -
> > -   /* Set the PIPE Clock Select bit in FPD PIPE Clock register */
> > -   writel(PIPE_CLK_SELECT, priv_data->regs +
> XLNX_USB_FPD_PIPE_CLK);
> > +   if (priv_data->usb3_phy) {
> > +           /* Set PIPE Power Present signal in FPD Power Present Register*/
> > +           writel(FPD_POWER_PRSNT_OPTION, priv_data->regs +
> XLNX_USB_FPD_POWER_PRSNT);
> > +           /* Set the PIPE Clock Select bit in FPD PIPE Clock register */
> > +           writel(PIPE_CLK_SELECT, priv_data->regs +
> XLNX_USB_FPD_PIPE_CLK);
> > +   } else {
> > +           /* Deselect the PIPE Clock Select bit in FPD PIPE Clock register */
> > +           writel(PIPE_CLK_DESELECT, priv_data->regs +
> XLNX_USB_FPD_PIPE_CLK);
> > +   }
> >
> >     ret = reset_control_deassert(crst);
> >     if (ret < 0) {
> > @@ -225,7 +223,6 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx
> *priv_data)
> >             goto err;
> >     }
> >
> > -skip_usb3_phy:
> >     /* ulpi reset via gpio-modepin or gpio-framework driver */
> >     reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> >     if (IS_ERR(reset_gpio)) {
> > --
> > 2.35.1.1320.gc452695387.dirty
> >

Re: [PATCH] usb: dwc3: Always deassert xilinx resets

[Sean Anderson]

On 1/9/26 01:01, Pandey, Radhey Shyam wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
>> -----Original Message-----
>> From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>> Sent: Friday, January 9, 2026 6:19 AM
>> To: Sean Anderson <sean.anderson@linux.dev>
>> Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>; open list:DESIGNWARE
>> USB3 DRD IP DRIVER <linux-usb@vger.kernel.org>; Frager, Neal
>> <neal.frager@amd.com>; Simek, Michal <michal.simek@amd.com>; open list
>> <linux-kernel@vger.kernel.org>; moderated list:ARM/ZYNQ ARCHITECTURE
>> <linux-arm-kernel@lists.infradead.org>; Philipp Zabel <p.zabel@pengutronix.de>;
>> Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org>
>> Subject: Re: [PATCH] usb: dwc3: Always deassert xilinx resets
>>
>> On Tue, Jan 06, 2026, Sean Anderson wrote:
>> > If we don't have a usb3 phy we don't need to assert the core resets.
>> > Deassert them even if we didn't assert them to support booting when
>> > the bootloader never released the core from reset.
> Is it a customized bootloader ? i.e it assert reset but don't deassert.

No. Most peripheral resets are asserted on PoR. So if the bootloader
doesn't deassert them then Linux has to.

My goal is to make init_serdes() in psu_init_gpl.c optional and do all
serdes initialization in the phy driver (and in the consumer drivers). I
have this working for DP/PCIe. I'm working on SATA, and I don't think
USB/SGMII need much special. This gives the following advantages:

- On some boards (mine) the reference clocks may not be configured in
  SPL/FSBL. So ILL calibration will fail (and take a long time to do so)
  unless we defer initialization to U-Boot/Linux where the phy driver
  can request the clocks.
- If PCIe/SATA are not used in U-Boot, ILL calibration can be deferred
  until Linux when it can be done it parallel with other initialization.
- We will have flexibility to switch between different serdes
  configurations at runtime. For example, this could allow the
  bootloader to fixup the devicetree to support PCIe and SATA M.2
  drives, depending on what the user has plugged in.

> I think ideally core /APB reset should be done independent on
> MAC 2.0/3.0 configuration.

I agree, but I think the existing code does this optimization to reduce
boot time when the bootloader has already initialized USB. I have
preserved that in this patch.

--Sean

> Not sure where the recommendation is
> coming from to only do it for 3.0 MAC. Let me check the IP spec.
> Thinh: Please chime in with your thoughts.
> 
>> >
>>
>> > Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> > ---
>> >
>> >  drivers/usb/dwc3/dwc3-xilinx.c | 67
>> > ++++++++++++++++------------------
>> >  1 file changed, 32 insertions(+), 35 deletions(-)
>> >
>>
>> This sounds like a fix. Does it need to be Cc Stable and backported?
>>
>> Regardless,
>>
>> Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>>
>> Thanks,
>> Thinh
>>
>> > diff --git a/drivers/usb/dwc3/dwc3-xilinx.c
>> > b/drivers/usb/dwc3/dwc3-xilinx.c index 0a8c47876ff9..f41b0da5e89d
>> > 100644
>> > --- a/drivers/usb/dwc3/dwc3-xilinx.c
>> > +++ b/drivers/usb/dwc3/dwc3-xilinx.c
>> > @@ -132,21 +132,6 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx
>> *priv_data)
>> >             goto err;
>> >     }
>> >
>> > -   /*
>> > -    * The following core resets are not required unless a USB3 PHY
>> > -    * is used, and the subsequent register settings are not required
>> > -    * unless a core reset is performed (they should be set properly
>> > -    * by the first-stage boot loader, but may be reverted by a core
>> > -    * reset). They may also break the configuration if USB3 is actually
>> > -    * in use but the usb3-phy entry is missing from the device tree.
>> > -    * Therefore, skip these operations in this case.
>> > -    */
>> > -   if (!priv_data->usb3_phy) {
>> > -           /* Deselect the PIPE Clock Select bit in FPD PIPE Clock register */
>> > -           writel(PIPE_CLK_DESELECT, priv_data->regs +
>> XLNX_USB_FPD_PIPE_CLK);
>> > -           goto skip_usb3_phy;
>> > -   }
>> > -
>> >     crst = devm_reset_control_get_exclusive(dev, "usb_crst");
>> >     if (IS_ERR(crst)) {
>> >             ret = PTR_ERR(crst);
>> > @@ -171,22 +156,31 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx
>> *priv_data)
>> >             goto err;
>> >     }
>> >
>> > -   ret = reset_control_assert(crst);
>> > -   if (ret < 0) {
>> > -           dev_err(dev, "Failed to assert core reset\n");
>> > -           goto err;
>> > -   }
>> > +   /*
>> > +    * Asserting the core resets is not required unless a USB3 PHY is used.
>> > +    * They may also break the configuration if USB3 is actually in use but
>> > +    * the usb3-phy entry is missing from the device tree. Therefore, skip
>> > +    * a full reset cycle and just deassert the resets if the phy is
>> > +    * absent.
>> > +    */
>> > +   if (priv_data->usb3_phy) {
>> > +           ret = reset_control_assert(crst);
>> > +           if (ret < 0) {
>> > +                   dev_err(dev, "Failed to assert core reset\n");
>> > +                   goto err;
>> > +           }
>> >
>> > -   ret = reset_control_assert(hibrst);
>> > -   if (ret < 0) {
>> > -           dev_err(dev, "Failed to assert hibernation reset\n");
>> > -           goto err;
>> > -   }
>> > +           ret = reset_control_assert(hibrst);
>> > +           if (ret < 0) {
>> > +                   dev_err(dev, "Failed to assert hibernation reset\n");
>> > +                   goto err;
>> > +           }
>> >
>> > -   ret = reset_control_assert(apbrst);
>> > -   if (ret < 0) {
>> > -           dev_err(dev, "Failed to assert APB reset\n");
>> > -           goto err;
>> > +           ret = reset_control_assert(apbrst);
>> > +           if (ret < 0) {
>> > +                   dev_err(dev, "Failed to assert APB reset\n");
>> > +                   goto err;
>> > +           }
>> >     }
>> >
>> >     ret = phy_init(priv_data->usb3_phy); @@ -201,11 +195,15 @@ static
>> > int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>> >             goto err;
>> >     }
>> >
>> > -   /* Set PIPE Power Present signal in FPD Power Present Register*/
>> > -   writel(FPD_POWER_PRSNT_OPTION, priv_data->regs +
>> XLNX_USB_FPD_POWER_PRSNT);
>> > -
>> > -   /* Set the PIPE Clock Select bit in FPD PIPE Clock register */
>> > -   writel(PIPE_CLK_SELECT, priv_data->regs +
>> XLNX_USB_FPD_PIPE_CLK);
>> > +   if (priv_data->usb3_phy) {
>> > +           /* Set PIPE Power Present signal in FPD Power Present Register*/
>> > +           writel(FPD_POWER_PRSNT_OPTION, priv_data->regs +
>> XLNX_USB_FPD_POWER_PRSNT);
>> > +           /* Set the PIPE Clock Select bit in FPD PIPE Clock register */
>> > +           writel(PIPE_CLK_SELECT, priv_data->regs +
>> XLNX_USB_FPD_PIPE_CLK);
>> > +   } else {
>> > +           /* Deselect the PIPE Clock Select bit in FPD PIPE Clock register */
>> > +           writel(PIPE_CLK_DESELECT, priv_data->regs +
>> XLNX_USB_FPD_PIPE_CLK);
>> > +   }
>> >
>> >     ret = reset_control_deassert(crst);
>> >     if (ret < 0) {
>> > @@ -225,7 +223,6 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx
>> *priv_data)
>> >             goto err;
>> >     }
>> >
>> > -skip_usb3_phy:
>> >     /* ulpi reset via gpio-modepin or gpio-framework driver */
>> >     reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>> >     if (IS_ERR(reset_gpio)) {
>> > --
>> > 2.35.1.1320.gc452695387.dirty
>> >

Re: [PATCH] usb: dwc3: Always deassert xilinx resets

[Thinh Nguyen]

On Fri, Jan 09, 2026, Sean Anderson wrote:
> On 1/9/26 01:01, Pandey, Radhey Shyam wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> > 
> >> -----Original Message-----
> >> From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> >> Sent: Friday, January 9, 2026 6:19 AM
> >> To: Sean Anderson <sean.anderson@linux.dev>
> >> Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>; open list:DESIGNWARE
> >> USB3 DRD IP DRIVER <linux-usb@vger.kernel.org>; Frager, Neal
> >> <neal.frager@amd.com>; Simek, Michal <michal.simek@amd.com>; open list
> >> <linux-kernel@vger.kernel.org>; moderated list:ARM/ZYNQ ARCHITECTURE
> >> <linux-arm-kernel@lists.infradead.org>; Philipp Zabel <p.zabel@pengutronix.de>;
> >> Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Greg Kroah-Hartman
> >> <gregkh@linuxfoundation.org>
> >> Subject: Re: [PATCH] usb: dwc3: Always deassert xilinx resets
> >>
> >> On Tue, Jan 06, 2026, Sean Anderson wrote:
> >> > If we don't have a usb3 phy we don't need to assert the core resets.
> >> > Deassert them even if we didn't assert them to support booting when
> >> > the bootloader never released the core from reset.
> > Is it a customized bootloader ? i.e it assert reset but don't deassert.
> 
> No. Most peripheral resets are asserted on PoR. So if the bootloader
> doesn't deassert them then Linux has to.
> 
> My goal is to make init_serdes() in psu_init_gpl.c optional and do all
> serdes initialization in the phy driver (and in the consumer drivers). I
> have this working for DP/PCIe. I'm working on SATA, and I don't think
> USB/SGMII need much special. This gives the following advantages:
> 
> - On some boards (mine) the reference clocks may not be configured in
>   SPL/FSBL. So ILL calibration will fail (and take a long time to do so)
>   unless we defer initialization to U-Boot/Linux where the phy driver
>   can request the clocks.
> - If PCIe/SATA are not used in U-Boot, ILL calibration can be deferred
>   until Linux when it can be done it parallel with other initialization.
> - We will have flexibility to switch between different serdes
>   configurations at runtime. For example, this could allow the
>   bootloader to fixup the devicetree to support PCIe and SATA M.2
>   drives, depending on what the user has plugged in.
> 
> > I think ideally core /APB reset should be done independent on
> > MAC 2.0/3.0 configuration.
> 
> I agree, but I think the existing code does this optimization to reduce
> boot time when the bootloader has already initialized USB. I have
> preserved that in this patch.
> 

I think all this info is useful. Can we include it in the change log?

Thanks,
Thinh

Re: [PATCH] usb: dwc3: Always deassert xilinx resets

[Sean Anderson]

On 1/12/26 19:49, Thinh Nguyen wrote:
> On Fri, Jan 09, 2026, Sean Anderson wrote:
>> On 1/9/26 01:01, Pandey, Radhey Shyam wrote:
>> > [AMD Official Use Only - AMD Internal Distribution Only]
>> > 
>> >> -----Original Message-----
>> >> From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>> >> Sent: Friday, January 9, 2026 6:19 AM
>> >> To: Sean Anderson <sean.anderson@linux.dev>
>> >> Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>; open list:DESIGNWARE
>> >> USB3 DRD IP DRIVER <linux-usb@vger.kernel.org>; Frager, Neal
>> >> <neal.frager@amd.com>; Simek, Michal <michal.simek@amd.com>; open list
>> >> <linux-kernel@vger.kernel.org>; moderated list:ARM/ZYNQ ARCHITECTURE
>> >> <linux-arm-kernel@lists.infradead.org>; Philipp Zabel <p.zabel@pengutronix.de>;
>> >> Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Greg Kroah-Hartman
>> >> <gregkh@linuxfoundation.org>
>> >> Subject: Re: [PATCH] usb: dwc3: Always deassert xilinx resets
>> >>
>> >> On Tue, Jan 06, 2026, Sean Anderson wrote:
>> >> > If we don't have a usb3 phy we don't need to assert the core resets.
>> >> > Deassert them even if we didn't assert them to support booting when
>> >> > the bootloader never released the core from reset.
>> > Is it a customized bootloader ? i.e it assert reset but don't deassert.
>> 
>> No. Most peripheral resets are asserted on PoR. So if the bootloader
>> doesn't deassert them then Linux has to.
>> 
>> My goal is to make init_serdes() in psu_init_gpl.c optional and do all
>> serdes initialization in the phy driver (and in the consumer drivers). I
>> have this working for DP/PCIe. I'm working on SATA, and I don't think
>> USB/SGMII need much special. This gives the following advantages:
>> 
>> - On some boards (mine) the reference clocks may not be configured in
>>   SPL/FSBL. So ILL calibration will fail (and take a long time to do so)
>>   unless we defer initialization to U-Boot/Linux where the phy driver
>>   can request the clocks.
>> - If PCIe/SATA are not used in U-Boot, ILL calibration can be deferred
>>   until Linux when it can be done it parallel with other initialization.
>> - We will have flexibility to switch between different serdes
>>   configurations at runtime. For example, this could allow the
>>   bootloader to fixup the devicetree to support PCIe and SATA M.2
>>   drives, depending on what the user has plugged in.
>> 
>> > I think ideally core /APB reset should be done independent on
>> > MAC 2.0/3.0 configuration.
>> 
>> I agree, but I think the existing code does this optimization to reduce
>> boot time when the bootloader has already initialized USB. I have
>> preserved that in this patch.
>> 
> 
> I think all this info is useful. Can we include it in the change log?

OK, how about:

I am working on moving serdes initialization to the phy (and consumer)
drivers to improve flexibility and boot times (depending on
configuration). Currently, core resets are released in the bootloader by
init_serdes() in psu_init_gpl.c. In order to remove init_serdes, we
need to handle the case where the bootloader never released the core
resets. If we don't have a usb3 phy we don't need to assert the core
resets, but deassert them anyway to handle this case.

We could assert all resets every boot, but I believe the existing
procedure is an optimization to reduce boot time when the bootloader has
already initialized USB. So this patch preserves the separate code
paths.

Re: [PATCH] usb: dwc3: Always deassert xilinx resets

[Thinh Nguyen]

On Tue, Jan 13, 2026, Sean Anderson wrote:
> On 1/12/26 19:49, Thinh Nguyen wrote:
> > On Fri, Jan 09, 2026, Sean Anderson wrote:
> >> On 1/9/26 01:01, Pandey, Radhey Shyam wrote:
> >> > [AMD Official Use Only - AMD Internal Distribution Only]
> >> > 
> >> >> -----Original Message-----
> >> >> From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> >> >> Sent: Friday, January 9, 2026 6:19 AM
> >> >> To: Sean Anderson <sean.anderson@linux.dev>
> >> >> Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>; open list:DESIGNWARE
> >> >> USB3 DRD IP DRIVER <linux-usb@vger.kernel.org>; Frager, Neal
> >> >> <neal.frager@amd.com>; Simek, Michal <michal.simek@amd.com>; open list
> >> >> <linux-kernel@vger.kernel.org>; moderated list:ARM/ZYNQ ARCHITECTURE
> >> >> <linux-arm-kernel@lists.infradead.org>; Philipp Zabel <p.zabel@pengutronix.de>;
> >> >> Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Greg Kroah-Hartman
> >> >> <gregkh@linuxfoundation.org>
> >> >> Subject: Re: [PATCH] usb: dwc3: Always deassert xilinx resets
> >> >>
> >> >> On Tue, Jan 06, 2026, Sean Anderson wrote:
> >> >> > If we don't have a usb3 phy we don't need to assert the core resets.
> >> >> > Deassert them even if we didn't assert them to support booting when
> >> >> > the bootloader never released the core from reset.
> >> > Is it a customized bootloader ? i.e it assert reset but don't deassert.
> >> 
> >> No. Most peripheral resets are asserted on PoR. So if the bootloader
> >> doesn't deassert them then Linux has to.
> >> 
> >> My goal is to make init_serdes() in psu_init_gpl.c optional and do all
> >> serdes initialization in the phy driver (and in the consumer drivers). I
> >> have this working for DP/PCIe. I'm working on SATA, and I don't think
> >> USB/SGMII need much special. This gives the following advantages:
> >> 
> >> - On some boards (mine) the reference clocks may not be configured in
> >>   SPL/FSBL. So ILL calibration will fail (and take a long time to do so)
> >>   unless we defer initialization to U-Boot/Linux where the phy driver
> >>   can request the clocks.
> >> - If PCIe/SATA are not used in U-Boot, ILL calibration can be deferred
> >>   until Linux when it can be done it parallel with other initialization.
> >> - We will have flexibility to switch between different serdes
> >>   configurations at runtime. For example, this could allow the
> >>   bootloader to fixup the devicetree to support PCIe and SATA M.2
> >>   drives, depending on what the user has plugged in.
> >> 
> >> > I think ideally core /APB reset should be done independent on
> >> > MAC 2.0/3.0 configuration.
> >> 
> >> I agree, but I think the existing code does this optimization to reduce
> >> boot time when the bootloader has already initialized USB. I have
> >> preserved that in this patch.
> >> 
> > 
> > I think all this info is useful. Can we include it in the change log?
> 
> OK, how about:
> 
> I am working on moving serdes initialization to the phy (and consumer)
> drivers to improve flexibility and boot times (depending on
> configuration). Currently, core resets are released in the bootloader by
> init_serdes() in psu_init_gpl.c. In order to remove init_serdes, we
> need to handle the case where the bootloader never released the core
> resets. If we don't have a usb3 phy we don't need to assert the core
> resets, but deassert them anyway to handle this case.
> 
> We could assert all resets every boot, but I believe the existing
> procedure is an optimization to reduce boot time when the bootloader has
> already initialized USB. So this patch preserves the separate code
> paths.

Sounds good to me! Typically we write it in imperative mood, but it's
just minor nit. I care more about the motivation behind these changes.

Radhey can comment if there's doubt.

Otherwise, you can add this in v2:

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

Re: [PATCH] usb: dwc3: Always deassert xilinx resets

[Thinh Nguyen]

On Fri, Jan 09, 2026, Pandey, Radhey Shyam wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> > -----Original Message-----
> > From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> > Sent: Friday, January 9, 2026 6:19 AM
> > To: Sean Anderson <sean.anderson@linux.dev>
> > Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>; open list:DESIGNWARE
> > USB3 DRD IP DRIVER <linux-usb@vger.kernel.org>; Frager, Neal
> > <neal.frager@amd.com>; Simek, Michal <michal.simek@amd.com>; open list
> > <linux-kernel@vger.kernel.org>; moderated list:ARM/ZYNQ ARCHITECTURE
> > <linux-arm-kernel@lists.infradead.org>; Philipp Zabel <p.zabel@pengutronix.de>;
> > Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org>
> > Subject: Re: [PATCH] usb: dwc3: Always deassert xilinx resets
> >
> > On Tue, Jan 06, 2026, Sean Anderson wrote:
> > > If we don't have a usb3 phy we don't need to assert the core resets.
> > > Deassert them even if we didn't assert them to support booting when
> > > the bootloader never released the core from reset.
> Is it a customized bootloader ? i.e it assert reset but don't deassert.
> 
> I think ideally core /APB reset should be done independent on
> MAC 2.0/3.0 configuration. Not sure where the recommendation is
> coming from to only do it for 3.0 MAC. Let me check the IP spec.
> Thinh: Please chime in with your thoughts.
> 

The dwc3 programming flow initiating the soft reset has not changed. How
each platform handles SoC specific resets at initialization varies
between implementation. I reviewed the change base on what's available
in the current code and the commit messages, and it sounded reasonable
to me. You may know more about this platform to provide better feedback
to Sean.

BR,
Thinh

Re: [PATCH] usb: dwc3: Always deassert xilinx resets

[Sean Anderson]

On 1/8/26 19:49, Thinh Nguyen wrote:
> On Tue, Jan 06, 2026, Sean Anderson wrote:
>> If we don't have a usb3 phy we don't need to assert the core resets.
>> Deassert them even if we didn't assert them to support booting when the
>> bootloader never released the core from reset.
>> 
> 
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> ---
>> 
>>  drivers/usb/dwc3/dwc3-xilinx.c | 67 ++++++++++++++++------------------
>>  1 file changed, 32 insertions(+), 35 deletions(-)
>> 
> 
> This sounds like a fix. Does it need to be Cc Stable and backported?

No, it's not a fix since the bootloaders currently in use on this
platform always initialize the serdes. But I want to add support for
skipping this initialization and this patch is necessary for that.

--Sean

> Regardless,
> 
> Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> 
> Thanks,
> Thinh
> 
>> diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
>> index 0a8c47876ff9..f41b0da5e89d 100644
>> --- a/drivers/usb/dwc3/dwc3-xilinx.c
>> +++ b/drivers/usb/dwc3/dwc3-xilinx.c
>> @@ -132,21 +132,6 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>>  		goto err;
>>  	}
>>  
>> -	/*
>> -	 * The following core resets are not required unless a USB3 PHY
>> -	 * is used, and the subsequent register settings are not required
>> -	 * unless a core reset is performed (they should be set properly
>> -	 * by the first-stage boot loader, but may be reverted by a core
>> -	 * reset). They may also break the configuration if USB3 is actually
>> -	 * in use but the usb3-phy entry is missing from the device tree.
>> -	 * Therefore, skip these operations in this case.
>> -	 */
>> -	if (!priv_data->usb3_phy) {
>> -		/* Deselect the PIPE Clock Select bit in FPD PIPE Clock register */
>> -		writel(PIPE_CLK_DESELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
>> -		goto skip_usb3_phy;
>> -	}
>> -
>>  	crst = devm_reset_control_get_exclusive(dev, "usb_crst");
>>  	if (IS_ERR(crst)) {
>>  		ret = PTR_ERR(crst);
>> @@ -171,22 +156,31 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>>  		goto err;
>>  	}
>>  
>> -	ret = reset_control_assert(crst);
>> -	if (ret < 0) {
>> -		dev_err(dev, "Failed to assert core reset\n");
>> -		goto err;
>> -	}
>> +	/*
>> +	 * Asserting the core resets is not required unless a USB3 PHY is used.
>> +	 * They may also break the configuration if USB3 is actually in use but
>> +	 * the usb3-phy entry is missing from the device tree. Therefore, skip
>> +	 * a full reset cycle and just deassert the resets if the phy is
>> +	 * absent.
>> +	 */
>> +	if (priv_data->usb3_phy) {
>> +		ret = reset_control_assert(crst);
>> +		if (ret < 0) {
>> +			dev_err(dev, "Failed to assert core reset\n");
>> +			goto err;
>> +		}
>>  
>> -	ret = reset_control_assert(hibrst);
>> -	if (ret < 0) {
>> -		dev_err(dev, "Failed to assert hibernation reset\n");
>> -		goto err;
>> -	}
>> +		ret = reset_control_assert(hibrst);
>> +		if (ret < 0) {
>> +			dev_err(dev, "Failed to assert hibernation reset\n");
>> +			goto err;
>> +		}
>>  
>> -	ret = reset_control_assert(apbrst);
>> -	if (ret < 0) {
>> -		dev_err(dev, "Failed to assert APB reset\n");
>> -		goto err;
>> +		ret = reset_control_assert(apbrst);
>> +		if (ret < 0) {
>> +			dev_err(dev, "Failed to assert APB reset\n");
>> +			goto err;
>> +		}
>>  	}
>>  
>>  	ret = phy_init(priv_data->usb3_phy);
>> @@ -201,11 +195,15 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>>  		goto err;
>>  	}
>>  
>> -	/* Set PIPE Power Present signal in FPD Power Present Register*/
>> -	writel(FPD_POWER_PRSNT_OPTION, priv_data->regs + XLNX_USB_FPD_POWER_PRSNT);
>> -
>> -	/* Set the PIPE Clock Select bit in FPD PIPE Clock register */
>> -	writel(PIPE_CLK_SELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
>> +	if (priv_data->usb3_phy) {
>> +		/* Set PIPE Power Present signal in FPD Power Present Register*/
>> +		writel(FPD_POWER_PRSNT_OPTION, priv_data->regs + XLNX_USB_FPD_POWER_PRSNT);
>> +		/* Set the PIPE Clock Select bit in FPD PIPE Clock register */
>> +		writel(PIPE_CLK_SELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
>> +	} else {
>> +		/* Deselect the PIPE Clock Select bit in FPD PIPE Clock register */
>> +		writel(PIPE_CLK_DESELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
>> +	}
>>  
>>  	ret = reset_control_deassert(crst);
>>  	if (ret < 0) {
>> @@ -225,7 +223,6 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>>  		goto err;
>>  	}
>>  
>> -skip_usb3_phy:
>>  	/* ulpi reset via gpio-modepin or gpio-framework driver */
>>  	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>>  	if (IS_ERR(reset_gpio)) {
>> -- 
>> 2.35.1.1320.gc452695387.dirty
>>