| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8f0e04a5-49df-4fe3-9c76-9a1c0bd822a0@linux.dev>
Date: Fri, 9 Jan 2026 10:51:55 -0500
From: Sean Anderson <sean.anderson@...ux.dev>
To: "Pandey, Radhey Shyam" <radhey.shyam.pandey@....com>,
Thinh Nguyen <Thinh.Nguyen@...opsys.com>
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
On 1/9/26 01:01, Pandey, Radhey Shyam wrote:
> [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.
No. Most peripheral resets are asserted on PoR. So if the bootloader
doesn't deassert them then Linux has to.
My goal is to make init_serdes() in psu_init_gpl.c optional and do all
serdes initialization in the phy driver (and in the consumer drivers). I
have this working for DP/PCIe. I'm working on SATA, and I don't think
USB/SGMII need much special. This gives the following advantages:
- On some boards (mine) the reference clocks may not be configured in
SPL/FSBL. So ILL calibration will fail (and take a long time to do so)
unless we defer initialization to U-Boot/Linux where the phy driver
can request the clocks.
- If PCIe/SATA are not used in U-Boot, ILL calibration can be deferred
until Linux when it can be done it parallel with other initialization.
- We will have flexibility to switch between different serdes
configurations at runtime. For example, this could allow the
bootloader to fixup the devicetree to support PCIe and SATA M.2
drives, depending on what the user has plugged in.
> I think ideally core /APB reset should be done independent on
> MAC 2.0/3.0 configuration.
I agree, but I think the existing code does this optimization to reduce
boot time when the bootloader has already initialized USB. I have
preserved that in this patch.
--Sean
> 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