Re: [PATCH v2 2/3] usb: dwc3: xilinx: re-assert resets on ZynqMP init error paths
From: Pandey, Radhey Shyam
Date: Sat Sep 26 2026 - 08:34:24 EST
On 9/23/2026 2:43 PM, Philipp Zabel wrote:
On Di, 2026-09-22 at 23:51 +0530, Radhey Shyam Pandey wrote:
If reset deassert or PHY setup fails partway through
dwc3_xlnx_init_zynqmp(), re-assert any resets that were already
released before unwinding the PHY. Use fall-through error labels so
unwind matches how far init progressed, for both USB2 and USB3 paths.
Save the ZynqMP reset handles in driver private data so later probe
teardown can re-assert released resets.
Fixes: 84770f028fab ("usb: dwc3: Add driver for Xilinx platforms")
Cc: stable@xxxxxxxxxxxxxxx
Assisted-by: Claude:claude-opus-5
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xxxxxxx>
---
Changes in v2:
- Split out of the combined five patch series; see patch 1.
- Reordered ahead of the platform-data cleanups.
- Added Cc: stable.
- No functional change to the patch itself.
drivers/usb/dwc3/dwc3-xilinx.c | 49 +++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
index 8c63e02575f1..d18d3e364381 100644
--- a/drivers/usb/dwc3/dwc3-xilinx.c
+++ b/drivers/usb/dwc3/dwc3-xilinx.c
@@ -48,6 +48,10 @@ struct dwc3_xlnx {
void __iomem *regs;
int (*pltfm_init)(struct dwc3_xlnx *data);
struct phy *usb3_phy;
+ struct reset_control *usb_crst;
+ struct reset_control *usb_hibrst;
+ struct reset_control *usb_apbrst;
+ bool usb_resets_released;
};
static void dwc3_xlnx_mask_phy_rst(struct dwc3_xlnx *priv_data, bool mask)
@@ -112,7 +116,6 @@ static int dwc3_xlnx_init_versal(struct dwc3_xlnx *priv_data)
static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
{
struct device *dev = priv_data->dev;
- struct reset_control *crst, *hibrst, *apbrst;
struct gpio_desc *reset_gpio;
int ret = 0;
@@ -124,25 +127,25 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
goto err;
}
- crst = devm_reset_control_get_exclusive(dev, "usb_crst");
- if (IS_ERR(crst)) {
- ret = PTR_ERR(crst);
+ priv_data->usb_crst = devm_reset_control_get_exclusive(dev, "usb_crst");
+ if (IS_ERR(priv_data->usb_crst)) {
+ ret = PTR_ERR(priv_data->usb_crst);
dev_err_probe(dev, ret,
"failed to get core reset signal\n");
goto err;
}
- hibrst = devm_reset_control_get_exclusive(dev, "usb_hibrst");
- if (IS_ERR(hibrst)) {
- ret = PTR_ERR(hibrst);
+ priv_data->usb_hibrst = devm_reset_control_get_exclusive(dev, "usb_hibrst");
+ if (IS_ERR(priv_data->usb_hibrst)) {
+ ret = PTR_ERR(priv_data->usb_hibrst);
dev_err_probe(dev, ret,
"failed to get hibernation reset signal\n");
goto err;
}
- apbrst = devm_reset_control_get_exclusive(dev, "usb_apbrst");
- if (IS_ERR(apbrst)) {
- ret = PTR_ERR(apbrst);
+ priv_data->usb_apbrst = devm_reset_control_get_exclusive(dev, "usb_apbrst");
+ if (IS_ERR(priv_data->usb_apbrst)) {
+ ret = PTR_ERR(priv_data->usb_apbrst);
dev_err_probe(dev, ret,
"failed to get APB reset signal\n");
goto err;
@@ -156,19 +159,19 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
* absent.
*/
if (priv_data->usb3_phy) {
- ret = reset_control_assert(crst);
+ ret = reset_control_assert(priv_data->usb_crst);
if (ret < 0) {
dev_err(dev, "Failed to assert core reset\n");
goto err;
}
- ret = reset_control_assert(hibrst);
+ ret = reset_control_assert(priv_data->usb_hibrst);
if (ret < 0) {
dev_err(dev, "Failed to assert hibernation reset\n");
goto err;
}
- ret = reset_control_assert(apbrst);
+ ret = reset_control_assert(priv_data->usb_apbrst);
if (ret < 0) {
dev_err(dev, "Failed to assert APB reset\n");
goto err;
@@ -179,7 +182,7 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
if (ret < 0)
goto err;
- ret = reset_control_deassert(apbrst);
+ ret = reset_control_deassert(priv_data->usb_apbrst);
if (ret < 0) {
dev_err(dev, "Failed to release APB reset\n");
goto err_phy_exit;
@@ -195,21 +198,21 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
writel(PIPE_CLK_DESELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
}
- ret = reset_control_deassert(crst);
+ ret = reset_control_deassert(priv_data->usb_crst);
if (ret < 0) {
dev_err(dev, "Failed to release core reset\n");
- goto err_phy_exit;
+ goto err_apbrst_assert;
}
- ret = reset_control_deassert(hibrst);
+ ret = reset_control_deassert(priv_data->usb_hibrst);
if (ret < 0) {
dev_err(dev, "Failed to release hibernation reset\n");
- goto err_phy_exit;
+ goto err_crst_assert;
}
ret = phy_power_on(priv_data->usb3_phy);
if (ret < 0)
- goto err_phy_exit;
+ goto err_hibrst_assert;
/* ulpi reset via gpio-modepin or gpio-framework driver */
reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
Are the resets asserted if this fails?
YEs, if reset_gpio fails resets are asserted.
Valid point. In this patch, only the init_zynqmp() error handling is fixed; probe failure handling is addressed in the subsequent patch.
Same question about if probe fails after pltfm_init() succeeded.
I agree on it, it simplifies the error handling and will spin v3.
@@ -226,10 +229,18 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
dwc3_xlnx_set_coherency(priv_data, XLNX_USB_TRAFFIC_ROUTE_CONFIG);
+ priv_data->usb_resets_released = true;
+
return 0;
err_phy_power_off:
phy_power_off(priv_data->usb3_phy);
+err_hibrst_assert:
+ reset_control_assert(priv_data->usb_hibrst);
+err_crst_assert:
+ reset_control_assert(priv_data->usb_crst);
+err_apbrst_assert:
+ reset_control_assert(priv_data->usb_apbrst);
err_phy_exit:
phy_exit(priv_data->usb3_phy);
err:
Rather than storing usb_resets_released state, and then having
conditional teardown in the next patch, this could be handled via
devm_add_action_or_reset.
Thanks,
Radhey