[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<MN0PR12MB59538C2F8613A9C0641BEAFBB782A@MN0PR12MB5953.namprd12.prod.outlook.com>
Date: Fri, 9 Jan 2026 06:01:43 +0000
From: "Pandey, Radhey Shyam" <radhey.shyam.pandey@....com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>, Sean Anderson
<sean.anderson@...ux.dev>
CC: "open list:DESIGNWARE USB3 DRD IP DRIVER" <linux-usb@...r.kernel.org>,
"Frager, Neal" <neal.frager@....com>, "Simek, Michal" <michal.simek@....com>,
open list <linux-kernel@...r.kernel.org>, "moderated list:ARM/ZYNQ
ARCHITECTURE" <linux-arm-kernel@...ts.infradead.org>, Philipp Zabel
<p.zabel@...gutronix.de>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: RE: [PATCH] usb: dwc3: Always deassert xilinx resets
[AMD Official Use Only - AMD Internal Distribution Only]
> -----Original Message-----
> From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
> Sent: Friday, January 9, 2026 6:19 AM
> To: Sean Anderson <sean.anderson@...ux.dev>
> Cc: Thinh Nguyen <Thinh.Nguyen@...opsys.com>; open list:DESIGNWARE
> USB3 DRD IP DRIVER <linux-usb@...r.kernel.org>; Frager, Neal
> <neal.frager@....com>; Simek, Michal <michal.simek@....com>; open list
> <linux-kernel@...r.kernel.org>; moderated list:ARM/ZYNQ ARCHITECTURE
> <linux-arm-kernel@...ts.infradead.org>; Philipp Zabel <p.zabel@...gutronix.de>;
> Pandey, Radhey Shyam <radhey.shyam.pandey@....com>; Greg Kroah-Hartman
> <gregkh@...uxfoundation.org>
> Subject: Re: [PATCH] usb: dwc3: Always deassert xilinx resets
>
> 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.
Is it a customized bootloader ? i.e it assert reset but don't deassert.
I think ideally core /APB reset should be done independent on
MAC 2.0/3.0 configuration. Not sure where the recommendation is
coming from to only do it for 3.0 MAC. Let me check the IP spec.
Thinh: Please chime in with your thoughts.
> >
>
> > Signed-off-by: Sean Anderson <sean.anderson@...ux.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@...opsys.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
> >
Powered by blists - more mailing lists