mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/3] usb: dwc3: xilinx: error path and teardown fixes
@ 2026-09-22 18:21 Radhey Shyam Pandey
  2026-09-22 18:21 ` [PATCH v2 1/3] usb: dwc3: xilinx: fix system suspend and resume PHY handling Radhey Shyam Pandey
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Radhey Shyam Pandey @ 2026-09-22 18:21 UTC (permalink / raw)
  To: Thinh.Nguyen, gregkh, michal.simek, p.zabel
  Cc: linux-usb, linux-arm-kernel, linux-kernel, Radhey Shyam Pandey

The ZynqMP path in dwc3-xilinx deasserts three resets and initialises the
USB3 PHY, but nothing unwinds that state when a later probe step fails,
and nothing unwinds it on remove either.  System suspend and resume have
the same problem: suspend calls phy_exit() without powering the PHY off
and ignores the result, and resume can leave clocks enabled if PHY
reinitialisation fails.

These three patches make each of those paths release what it acquired.

v1 [1] sent them together with two platform-data cleanups.  Per review
feedback they are now split out so the fixes come first and can be
backported on their own; the cleanups follow in a separate series against
usb-next.

Patch 3 was reworked for that split.  In v1 it registered the teardown as
plat->exit in struct dwc3_xlnx_platdata, which one of the cleanup patches
introduces, so it could not have been backported.  It now adds a
pltfm_exit pointer alongside the existing pltfm_init in struct dwc3_xlnx
and is self-contained.  The follow-up series folds both pointers into the
platform data struct.

All three are tagged for stable.

Link to v1:
https://lore.kernel.org/all/20260810174713.2325292-1-radhey.shyam.pandey@amd.com

Radhey Shyam Pandey (3):
  usb: dwc3: xilinx: fix system suspend and resume PHY handling
  usb: dwc3: xilinx: re-assert resets on ZynqMP init error paths
  usb: dwc3: xilinx: unwind ZynqMP platform init on probe failure and
    remove

 drivers/usb/dwc3/dwc3-xilinx.c | 101 ++++++++++++++++++++++++---------
 1 file changed, 77 insertions(+), 24 deletions(-)


base-commit: abc36cbda29d8f19cf3a580cd86ca9e865186a41
-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/3] usb: dwc3: xilinx: fix system suspend and resume PHY handling
  2026-09-22 18:21 [PATCH v2 0/3] usb: dwc3: xilinx: error path and teardown fixes Radhey Shyam Pandey
@ 2026-09-22 18:21 ` Radhey Shyam Pandey
  2026-10-02  2:06   ` Thinh Nguyen
  2026-09-22 18:21 ` [PATCH v2 2/3] usb: dwc3: xilinx: re-assert resets on ZynqMP init error paths Radhey Shyam Pandey
  2026-09-22 18:21 ` [PATCH v2 3/3] usb: dwc3: xilinx: unwind ZynqMP platform init on probe failure and remove Radhey Shyam Pandey
  2 siblings, 1 reply; 9+ messages in thread
From: Radhey Shyam Pandey @ 2026-09-22 18:21 UTC (permalink / raw)
  To: Thinh.Nguyen, gregkh, michal.simek, p.zabel
  Cc: linux-usb, linux-arm-kernel, linux-kernel, Radhey Shyam Pandey, stable

System suspend and resume error paths do not handle PHY and clock
resources correctly. Suspend calls phy_exit() without first powering
off the PHY and ignores failures, while resume can leave clocks
enabled if PHY reinitialization fails.

Propagate errors to the PM core and unwind resources to ensure a
consistent state on suspend and resume failures.

Fixes: d6edcdc1ef06 ("usb: dwc3: xilinx: fix usb3 non-wakeup source resume failure")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
Changes in v2:
- Split out of the combined five patch series so the fixes can be sent
  and backported on their own, per review feedback.
- Reordered ahead of the platform-data cleanups.
- Log a failure of the phy_power_on() rollback.  phy_exit() leaves
  init_count untouched when it fails, so if the rollback also fails the
  PHY is left with power_count and init_count out of step; that is now
  at least visible in the log.
- Added Cc: stable.

 drivers/usb/dwc3/dwc3-xilinx.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
index b832505e1b04..8c63e02575f1 100644
--- a/drivers/usb/dwc3/dwc3-xilinx.c
+++ b/drivers/usb/dwc3/dwc3-xilinx.c
@@ -383,13 +383,26 @@ static int __maybe_unused dwc3_xlnx_runtime_idle(struct device *dev)
 static int __maybe_unused dwc3_xlnx_suspend(struct device *dev)
 {
 	struct dwc3_xlnx *priv_data = dev_get_drvdata(dev);
+	int ret;
 
-	phy_exit(priv_data->usb3_phy);
+	ret = phy_power_off(priv_data->usb3_phy);
+	if (ret < 0)
+		return ret;
+
+	ret = phy_exit(priv_data->usb3_phy);
+	if (ret < 0)
+		goto err_phy_power_on;
 
 	/* Disable the clocks */
 	clk_bulk_disable(priv_data->num_clocks, priv_data->clks);
 
 	return 0;
+
+err_phy_power_on:
+	if (phy_power_on(priv_data->usb3_phy))
+		dev_err(dev, "failed to restore PHY power after suspend error\n");
+
+	return ret;
 }
 
 static int __maybe_unused dwc3_xlnx_resume(struct device *dev)
@@ -403,15 +416,20 @@ static int __maybe_unused dwc3_xlnx_resume(struct device *dev)
 
 	ret = phy_init(priv_data->usb3_phy);
 	if (ret < 0)
-		return ret;
+		goto err_clk_disable;
 
 	ret = phy_power_on(priv_data->usb3_phy);
 	if (ret < 0) {
 		phy_exit(priv_data->usb3_phy);
-		return ret;
+		goto err_clk_disable;
 	}
 
 	return 0;
+
+err_clk_disable:
+	clk_bulk_disable(priv_data->num_clocks, priv_data->clks);
+
+	return ret;
 }
 
 static const struct dev_pm_ops dwc3_xlnx_dev_pm_ops = {
-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 2/3] usb: dwc3: xilinx: re-assert resets on ZynqMP init error paths
  2026-09-22 18:21 [PATCH v2 0/3] usb: dwc3: xilinx: error path and teardown fixes Radhey Shyam Pandey
  2026-09-22 18:21 ` [PATCH v2 1/3] usb: dwc3: xilinx: fix system suspend and resume PHY handling Radhey Shyam Pandey
@ 2026-09-22 18:21 ` Radhey Shyam Pandey
  2026-09-23  9:13   ` Philipp Zabel
  2026-10-02  2:14   ` Thinh Nguyen
  2026-09-22 18:21 ` [PATCH v2 3/3] usb: dwc3: xilinx: unwind ZynqMP platform init on probe failure and remove Radhey Shyam Pandey
  2 siblings, 2 replies; 9+ messages in thread
From: Radhey Shyam Pandey @ 2026-09-22 18:21 UTC (permalink / raw)
  To: Thinh.Nguyen, gregkh, michal.simek, p.zabel
  Cc: linux-usb, linux-arm-kernel, linux-kernel, Radhey Shyam Pandey, stable

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@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
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);
@@ -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:
-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 3/3] usb: dwc3: xilinx: unwind ZynqMP platform init on probe failure and remove
  2026-09-22 18:21 [PATCH v2 0/3] usb: dwc3: xilinx: error path and teardown fixes Radhey Shyam Pandey
  2026-09-22 18:21 ` [PATCH v2 1/3] usb: dwc3: xilinx: fix system suspend and resume PHY handling Radhey Shyam Pandey
  2026-09-22 18:21 ` [PATCH v2 2/3] usb: dwc3: xilinx: re-assert resets on ZynqMP init error paths Radhey Shyam Pandey
@ 2026-09-22 18:21 ` Radhey Shyam Pandey
  2026-10-02  2:15   ` Thinh Nguyen
  2 siblings, 1 reply; 9+ messages in thread
From: Radhey Shyam Pandey @ 2026-09-22 18:21 UTC (permalink / raw)
  To: Thinh.Nguyen, gregkh, michal.simek, p.zabel
  Cc: linux-usb, linux-arm-kernel, linux-kernel, Radhey Shyam Pandey, stable

dwc3_xlnx_init_zynqmp() deasserts resets and initialises the USB3 PHY,
but nothing undoes that if a later probe step fails, and nothing undoes
it on remove either. The resets stay deasserted and the PHY stays
initialised while the clocks are disabled underneath them.

Add dwc3_xlnx_exit_zynqmp() and register it as the platform exit handler
once ZynqMP init has completed. Call it from the probe error path for
failures after init succeeded, and from remove(), which also serves as
the shutdown callback.

Fixes: 84770f028fab ("usb: dwc3: Add driver for Xilinx platforms")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
Changes in v2:
- Reworked so the fix no longer depends on the platform-data cleanup.
  v1 registered the teardown as plat->exit in struct dwc3_xlnx_platdata,
  which is introduced by one of the cleanup patches; that made the fix
  unbackportable.  It now uses a pltfm_exit pointer alongside the
  existing pltfm_init in struct dwc3_xlnx, assigned by
  dwc3_xlnx_init_zynqmp() once init has succeeded.  The follow-up
  cleanup series folds both pointers into the platform data struct.
- Made dwc3_xlnx_exit_zynqmp() idempotent by returning early when
  usb_resets_released is clear, rather than guarding only the reset
  assertions.  phy_power_off() and phy_exit() decrement their counts
  unconditionally, so an unbalanced second call would underflow them.
- Rewrote the commit message to describe the bug rather than the
  implementation, since the callback it referred to no longer exists at
  this point in the series.
- Added Cc: stable.

 drivers/usb/dwc3/dwc3-xilinx.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
index d18d3e364381..31ac75b79709 100644
--- a/drivers/usb/dwc3/dwc3-xilinx.c
+++ b/drivers/usb/dwc3/dwc3-xilinx.c
@@ -47,6 +47,7 @@ struct dwc3_xlnx {
 	struct device			*dev;
 	void __iomem			*regs;
 	int				(*pltfm_init)(struct dwc3_xlnx *data);
+	void				(*pltfm_exit)(struct dwc3_xlnx *data);
 	struct phy			*usb3_phy;
 	struct reset_control		*usb_crst;
 	struct reset_control		*usb_hibrst;
@@ -113,6 +114,21 @@ static int dwc3_xlnx_init_versal(struct dwc3_xlnx *priv_data)
 	return 0;
 }
 
+static void dwc3_xlnx_exit_zynqmp(struct dwc3_xlnx *priv_data)
+{
+	if (!priv_data->usb_resets_released)
+		return;
+
+	phy_power_off(priv_data->usb3_phy);
+
+	reset_control_assert(priv_data->usb_hibrst);
+	reset_control_assert(priv_data->usb_crst);
+	reset_control_assert(priv_data->usb_apbrst);
+	priv_data->usb_resets_released = false;
+
+	phy_exit(priv_data->usb3_phy);
+}
+
 static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
 {
 	struct device		*dev = priv_data->dev;
@@ -230,6 +246,7 @@ 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;
+	priv_data->pltfm_exit = dwc3_xlnx_exit_zynqmp;
 
 	return 0;
 
@@ -326,11 +343,11 @@ static int dwc3_xlnx_probe(struct platform_device *pdev)
 
 	ret = dwc3_set_swnode(dev);
 	if (ret)
-		goto err_clk_put;
+		goto err_pltfm_exit;
 
 	ret = of_platform_populate(np, NULL, NULL, dev);
 	if (ret)
-		goto err_clk_put;
+		goto err_pltfm_exit;
 
 	pm_runtime_set_active(dev);
 	ret = devm_pm_runtime_enable(dev);
@@ -348,6 +365,10 @@ static int dwc3_xlnx_probe(struct platform_device *pdev)
 	of_platform_depopulate(dev);
 	pm_runtime_set_suspended(dev);
 
+err_pltfm_exit:
+	if (priv_data->pltfm_exit)
+		priv_data->pltfm_exit(priv_data);
+
 err_clk_put:
 	clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks);
 
@@ -361,6 +382,9 @@ static void dwc3_xlnx_remove(struct platform_device *pdev)
 
 	of_platform_depopulate(dev);
 
+	if (priv_data->pltfm_exit)
+		priv_data->pltfm_exit(priv_data);
+
 	clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks);
 	priv_data->num_clocks = 0;
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/3] usb: dwc3: xilinx: re-assert resets on ZynqMP init error paths
  2026-09-22 18:21 ` [PATCH v2 2/3] usb: dwc3: xilinx: re-assert resets on ZynqMP init error paths Radhey Shyam Pandey
@ 2026-09-23  9:13   ` Philipp Zabel
  2026-09-26 12:32     ` Pandey, Radhey Shyam
  2026-10-02  2:14   ` Thinh Nguyen
  1 sibling, 1 reply; 9+ messages in thread
From: Philipp Zabel @ 2026-09-23  9:13 UTC (permalink / raw)
  To: Radhey Shyam Pandey, Thinh.Nguyen, gregkh, michal.simek
  Cc: linux-usb, linux-arm-kernel, linux-kernel, stable

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@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> 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?

Same question about if probe fails after pltfm_init() succeeded.

> @@ -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.

regards
Philipp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/3] usb: dwc3: xilinx: re-assert resets on ZynqMP init error paths
  2026-09-23  9:13   ` Philipp Zabel
@ 2026-09-26 12:32     ` Pandey, Radhey Shyam
  0 siblings, 0 replies; 9+ messages in thread
From: Pandey, Radhey Shyam @ 2026-09-26 12:32 UTC (permalink / raw)
  To: Philipp Zabel, Radhey Shyam Pandey, Thinh.Nguyen, gregkh, michal.simek
  Cc: linux-usb, linux-arm-kernel, linux-kernel, stable

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@vger.kernel.org
>> Assisted-by: Claude:claude-opus-5
>> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
>> ---
>> 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.

> 
> Same question about if probe fails after pltfm_init() succeeded.
Valid point. In this patch, only the init_zynqmp() error handling is 
fixed; probe failure handling is addressed in the subsequent patch.

> 
>> @@ -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.
I agree on it, it simplifies the error handling and will spin v3.

Thanks,
Radhey

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/3] usb: dwc3: xilinx: fix system suspend and resume PHY handling
  2026-09-22 18:21 ` [PATCH v2 1/3] usb: dwc3: xilinx: fix system suspend and resume PHY handling Radhey Shyam Pandey
@ 2026-10-02  2:06   ` Thinh Nguyen
  0 siblings, 0 replies; 9+ messages in thread
From: Thinh Nguyen @ 2026-10-02  2:06 UTC (permalink / raw)
  To: Radhey Shyam Pandey
  Cc: Thinh Nguyen, gregkh, michal.simek, p.zabel, linux-usb,
	linux-arm-kernel, linux-kernel, stable

On Tue, Sep 22, 2026, Radhey Shyam Pandey wrote:
> System suspend and resume error paths do not handle PHY and clock
> resources correctly. Suspend calls phy_exit() without first powering
> off the PHY and ignores failures, while resume can leave clocks
> enabled if PHY reinitialization fails.
> 
> Propagate errors to the PM core and unwind resources to ensure a
> consistent state on suspend and resume failures.
> 
> Fixes: d6edcdc1ef06 ("usb: dwc3: xilinx: fix usb3 non-wakeup source resume failure")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> Changes in v2:
> - Split out of the combined five patch series so the fixes can be sent
>   and backported on their own, per review feedback.
> - Reordered ahead of the platform-data cleanups.
> - Log a failure of the phy_power_on() rollback.  phy_exit() leaves
>   init_count untouched when it fails, so if the rollback also fails the
>   PHY is left with power_count and init_count out of step; that is now
>   at least visible in the log.
> - Added Cc: stable.
> 
>  drivers/usb/dwc3/dwc3-xilinx.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
> index b832505e1b04..8c63e02575f1 100644
> --- a/drivers/usb/dwc3/dwc3-xilinx.c
> +++ b/drivers/usb/dwc3/dwc3-xilinx.c
> @@ -383,13 +383,26 @@ static int __maybe_unused dwc3_xlnx_runtime_idle(struct device *dev)
>  static int __maybe_unused dwc3_xlnx_suspend(struct device *dev)
>  {
>  	struct dwc3_xlnx *priv_data = dev_get_drvdata(dev);
> +	int ret;
>  
> -	phy_exit(priv_data->usb3_phy);
> +	ret = phy_power_off(priv_data->usb3_phy);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = phy_exit(priv_data->usb3_phy);
> +	if (ret < 0)
> +		goto err_phy_power_on;
>  
>  	/* Disable the clocks */
>  	clk_bulk_disable(priv_data->num_clocks, priv_data->clks);
>  
>  	return 0;
> +
> +err_phy_power_on:
> +	if (phy_power_on(priv_data->usb3_phy))
> +		dev_err(dev, "failed to restore PHY power after suspend error\n");
> +
> +	return ret;
>  }
>  
>  static int __maybe_unused dwc3_xlnx_resume(struct device *dev)
> @@ -403,15 +416,20 @@ static int __maybe_unused dwc3_xlnx_resume(struct device *dev)
>  
>  	ret = phy_init(priv_data->usb3_phy);
>  	if (ret < 0)
> -		return ret;
> +		goto err_clk_disable;
>  
>  	ret = phy_power_on(priv_data->usb3_phy);
>  	if (ret < 0) {
>  		phy_exit(priv_data->usb3_phy);
> -		return ret;
> +		goto err_clk_disable;
>  	}
>  
>  	return 0;
> +
> +err_clk_disable:
> +	clk_bulk_disable(priv_data->num_clocks, priv_data->clks);
> +
> +	return ret;
>  }
>  
>  static const struct dev_pm_ops dwc3_xlnx_dev_pm_ops = {
> -- 
> 2.43.0
> 

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

Thanks,
Thinh

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/3] usb: dwc3: xilinx: re-assert resets on ZynqMP init error paths
  2026-09-22 18:21 ` [PATCH v2 2/3] usb: dwc3: xilinx: re-assert resets on ZynqMP init error paths Radhey Shyam Pandey
  2026-09-23  9:13   ` Philipp Zabel
@ 2026-10-02  2:14   ` Thinh Nguyen
  1 sibling, 0 replies; 9+ messages in thread
From: Thinh Nguyen @ 2026-10-02  2:14 UTC (permalink / raw)
  To: Radhey Shyam Pandey
  Cc: Thinh Nguyen, gregkh, michal.simek, p.zabel, linux-usb,
	linux-arm-kernel, linux-kernel, stable

On Tue, Sep 22, 2026, 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@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> 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;

Minor nit:

The usb_resets_released seems to represent more than just the reset
state. It's only set after the entire initialization sequence succeeds,
use it to indicate that initialization completed successfully.

Perhaps rename to "initialized" or "init_done"?

This is only a naming suggestion and not something I'd block the patch
on:

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

Thanks,
Thinh

>  };
>  
>  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);
> @@ -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:
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/3] usb: dwc3: xilinx: unwind ZynqMP platform init on probe failure and remove
  2026-09-22 18:21 ` [PATCH v2 3/3] usb: dwc3: xilinx: unwind ZynqMP platform init on probe failure and remove Radhey Shyam Pandey
@ 2026-10-02  2:15   ` Thinh Nguyen
  0 siblings, 0 replies; 9+ messages in thread
From: Thinh Nguyen @ 2026-10-02  2:15 UTC (permalink / raw)
  To: Radhey Shyam Pandey
  Cc: Thinh Nguyen, gregkh, michal.simek, p.zabel, linux-usb,
	linux-arm-kernel, linux-kernel, stable

On Tue, Sep 22, 2026, Radhey Shyam Pandey wrote:
> dwc3_xlnx_init_zynqmp() deasserts resets and initialises the USB3 PHY,
> but nothing undoes that if a later probe step fails, and nothing undoes
> it on remove either. The resets stay deasserted and the PHY stays
> initialised while the clocks are disabled underneath them.
> 
> Add dwc3_xlnx_exit_zynqmp() and register it as the platform exit handler
> once ZynqMP init has completed. Call it from the probe error path for
> failures after init succeeded, and from remove(), which also serves as
> the shutdown callback.
> 
> Fixes: 84770f028fab ("usb: dwc3: Add driver for Xilinx platforms")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> Changes in v2:
> - Reworked so the fix no longer depends on the platform-data cleanup.
>   v1 registered the teardown as plat->exit in struct dwc3_xlnx_platdata,
>   which is introduced by one of the cleanup patches; that made the fix
>   unbackportable.  It now uses a pltfm_exit pointer alongside the
>   existing pltfm_init in struct dwc3_xlnx, assigned by
>   dwc3_xlnx_init_zynqmp() once init has succeeded.  The follow-up
>   cleanup series folds both pointers into the platform data struct.
> - Made dwc3_xlnx_exit_zynqmp() idempotent by returning early when
>   usb_resets_released is clear, rather than guarding only the reset
>   assertions.  phy_power_off() and phy_exit() decrement their counts
>   unconditionally, so an unbalanced second call would underflow them.
> - Rewrote the commit message to describe the bug rather than the
>   implementation, since the callback it referred to no longer exists at
>   this point in the series.
> - Added Cc: stable.
> 
>  drivers/usb/dwc3/dwc3-xilinx.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
> index d18d3e364381..31ac75b79709 100644
> --- a/drivers/usb/dwc3/dwc3-xilinx.c
> +++ b/drivers/usb/dwc3/dwc3-xilinx.c
> @@ -47,6 +47,7 @@ struct dwc3_xlnx {
>  	struct device			*dev;
>  	void __iomem			*regs;
>  	int				(*pltfm_init)(struct dwc3_xlnx *data);
> +	void				(*pltfm_exit)(struct dwc3_xlnx *data);
>  	struct phy			*usb3_phy;
>  	struct reset_control		*usb_crst;
>  	struct reset_control		*usb_hibrst;
> @@ -113,6 +114,21 @@ static int dwc3_xlnx_init_versal(struct dwc3_xlnx *priv_data)
>  	return 0;
>  }
>  
> +static void dwc3_xlnx_exit_zynqmp(struct dwc3_xlnx *priv_data)
> +{
> +	if (!priv_data->usb_resets_released)
> +		return;
> +
> +	phy_power_off(priv_data->usb3_phy);
> +
> +	reset_control_assert(priv_data->usb_hibrst);
> +	reset_control_assert(priv_data->usb_crst);
> +	reset_control_assert(priv_data->usb_apbrst);
> +	priv_data->usb_resets_released = false;
> +
> +	phy_exit(priv_data->usb3_phy);
> +}
> +
>  static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>  {
>  	struct device		*dev = priv_data->dev;
> @@ -230,6 +246,7 @@ 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;
> +	priv_data->pltfm_exit = dwc3_xlnx_exit_zynqmp;
>  
>  	return 0;
>  
> @@ -326,11 +343,11 @@ static int dwc3_xlnx_probe(struct platform_device *pdev)
>  
>  	ret = dwc3_set_swnode(dev);
>  	if (ret)
> -		goto err_clk_put;
> +		goto err_pltfm_exit;
>  
>  	ret = of_platform_populate(np, NULL, NULL, dev);
>  	if (ret)
> -		goto err_clk_put;
> +		goto err_pltfm_exit;
>  
>  	pm_runtime_set_active(dev);
>  	ret = devm_pm_runtime_enable(dev);
> @@ -348,6 +365,10 @@ static int dwc3_xlnx_probe(struct platform_device *pdev)
>  	of_platform_depopulate(dev);
>  	pm_runtime_set_suspended(dev);
>  
> +err_pltfm_exit:
> +	if (priv_data->pltfm_exit)
> +		priv_data->pltfm_exit(priv_data);
> +
>  err_clk_put:
>  	clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks);
>  
> @@ -361,6 +382,9 @@ static void dwc3_xlnx_remove(struct platform_device *pdev)
>  
>  	of_platform_depopulate(dev);
>  
> +	if (priv_data->pltfm_exit)
> +		priv_data->pltfm_exit(priv_data);
> +
>  	clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks);
>  	priv_data->num_clocks = 0;
>  
> -- 
> 2.43.0
> 

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

Thanks,
Thinh

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-10-02  2:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-22 18:21 [PATCH v2 0/3] usb: dwc3: xilinx: error path and teardown fixes Radhey Shyam Pandey
2026-09-22 18:21 ` [PATCH v2 1/3] usb: dwc3: xilinx: fix system suspend and resume PHY handling Radhey Shyam Pandey
2026-10-02  2:06   ` Thinh Nguyen
2026-09-22 18:21 ` [PATCH v2 2/3] usb: dwc3: xilinx: re-assert resets on ZynqMP init error paths Radhey Shyam Pandey
2026-09-23  9:13   ` Philipp Zabel
2026-09-26 12:32     ` Pandey, Radhey Shyam
2026-10-02  2:14   ` Thinh Nguyen
2026-09-22 18:21 ` [PATCH v2 3/3] usb: dwc3: xilinx: unwind ZynqMP platform init on probe failure and remove Radhey Shyam Pandey
2026-10-02  2:15   ` Thinh Nguyen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®