All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"michal.simek@amd.com" <michal.simek@amd.com>,
	"p.zabel@pengutronix.de" <p.zabel@pengutronix.de>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH v2 3/3] usb: dwc3: xilinx: unwind ZynqMP platform init on probe failure and remove
Date: Fri, 2 Oct 2026 02:15:00 +0000	[thread overview]
Message-ID: <ar8Te9BLqIxOC2S9@vbox> (raw)
In-Reply-To: <20260922182125.11067-4-radhey.shyam.pandey@amd.com>

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

      reply	other threads:[~2026-10-02  2:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ar8Te9BLqIxOC2S9@vbox \
    --to=thinh.nguyen@synopsys.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=michal.simek@amd.com \
    --cc=p.zabel@pengutronix.de \
    --cc=radhey.shyam.pandey@amd.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.