[PATCH] usb: dwc3: Always deassert xilinx resets

Sean Anderson posted 1 patch 1 month ago
There is a newer version of this series
drivers/usb/dwc3/dwc3-xilinx.c | 67 ++++++++++++++++------------------
1 file changed, 32 insertions(+), 35 deletions(-)
[PATCH] usb: dwc3: Always deassert xilinx resets
Posted by Sean Anderson 1 month ago
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
Posted by Thinh Nguyen 4 weeks, 1 day ago
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
Posted by Sean Anderson 4 weeks ago
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
>>