Re: [PATCH v2 3/3] usb: dwc3: xilinx: unwind ZynqMP platform init on probe failure and remove
From: Thinh Nguyen
Date: Thu Oct 01 2026 - 22:15:33 EST
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@xxxxxxxxxxxxxxx
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xxxxxxx>
> ---
> 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@xxxxxxxxxxxx>
Thanks,
Thinh