* [PATCH] usb: dwc3: Support for USB3340x ULPI PHY, via "snps,enable_xcvrdly_quirk" @ 2026-02-24 14:14 Ingo Rohloff 2026-02-25 0:05 ` Thinh Nguyen 0 siblings, 1 reply; 19+ messages in thread From: Ingo Rohloff @ 2026-02-24 14:14 UTC (permalink / raw) To: Thinh.Nguyen, gregkh; +Cc: linux-usb, Ingo Rohloff The Microchip USB3340x ULPI PHY requires a delay when switching to the high-speed transmitter. Original description is/was in Xilinx Wiki under "USB Debug Guide for Zynq UltraScale+ and Versal Devices" Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com> --- drivers/usb/dwc3/core.c | 20 ++++++++++++++++++++ drivers/usb/dwc3/core.h | 2 ++ 2 files changed, 22 insertions(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 161a4d58b2ce..92ad701a9340 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -777,6 +777,23 @@ static int dwc3_hs_phy_setup(struct dwc3 *dwc, int index) if (dwc->ulpi_ext_vbus_drv) reg |= DWC3_GUSB2PHYCFG_ULPIEXTVBUSDRV; + /* + * Fixes High-speed negotiation issue with USB3340, see: + * http://ww1.microchip.com/downloads/en/DeviceDoc/80000645A.pdf + * "Device Enumeration Failure with Link IP Systems" + * According to documentation on the Internet, + * DWC3_GUSB2PHYCFG_XCVRDLY: + * Adds a delay between the assertion of the + * ULPI Transceiver Select signal (for HS) and + * the assertion of the TxValid signal during a HS Chirp. + * + * This bit also needs to be set again when the device comes out + * of hibernation, this is currently not an issue since hibernation + * is not enabled. + */ + if (dwc->enable_xcvrdly_quirk) + reg |= DWC3_GUSB2PHYCFG_XCVRDLY; + dwc3_writel(dwc, DWC3_GUSB2PHYCFG(index), reg); return 0; @@ -1855,6 +1872,9 @@ static void dwc3_get_properties(struct dwc3 *dwc) dwc->dis_split_quirk = device_property_read_bool(dev, "snps,dis-split-quirk"); + dwc->enable_xcvrdly_quirk = device_property_read_bool(dev, + "snps,enable_xcvrdly_quirk"); + dwc->lpm_nyet_threshold = lpm_nyet_threshold; dwc->tx_de_emphasis = tx_de_emphasis; diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index a35b3db1f9f3..41418f3f85e1 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -302,6 +302,7 @@ #define DWC3_GUSB2PHYCFG_SUSPHY BIT(6) #define DWC3_GUSB2PHYCFG_ULPI_UTMI BIT(4) #define DWC3_GUSB2PHYCFG_ENBLSLPM BIT(8) +#define DWC3_GUSB2PHYCFG_XCVRDLY BIT(9) #define DWC3_GUSB2PHYCFG_PHYIF(n) (n << 3) #define DWC3_GUSB2PHYCFG_PHYIF_MASK DWC3_GUSB2PHYCFG_PHYIF(1) #define DWC3_GUSB2PHYCFG_USBTRDTIM(n) (n << 10) @@ -1403,6 +1404,7 @@ struct dwc3 { unsigned dis_metastability_quirk:1; unsigned dis_split_quirk:1; + unsigned enable_xcvrdly_quirk:1; unsigned async_callbacks:1; unsigned sys_wakeup:1; unsigned wakeup_configured:1; -- 2.52.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY, via "snps,enable_xcvrdly_quirk" 2026-02-24 14:14 [PATCH] usb: dwc3: Support for USB3340x ULPI PHY, via "snps,enable_xcvrdly_quirk" Ingo Rohloff @ 2026-02-25 0:05 ` Thinh Nguyen 2026-02-25 12:49 ` Ingo Rohloff 2026-02-25 13:03 ` [PATCH v2 0/2] Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY Ingo Rohloff 0 siblings, 2 replies; 19+ messages in thread From: Thinh Nguyen @ 2026-02-25 0:05 UTC (permalink / raw) To: Ingo Rohloff Cc: Thinh Nguyen, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org On Tue, Feb 24, 2026, Ingo Rohloff wrote: > The Microchip USB3340x ULPI PHY requires a delay when switching to the > high-speed transmitter. > > Original description is/was in Xilinx Wiki under > "USB Debug Guide for Zynq UltraScale+ and Versal Devices" > > Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com> > --- > drivers/usb/dwc3/core.c | 20 ++++++++++++++++++++ > drivers/usb/dwc3/core.h | 2 ++ > 2 files changed, 22 insertions(+) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 161a4d58b2ce..92ad701a9340 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -777,6 +777,23 @@ static int dwc3_hs_phy_setup(struct dwc3 *dwc, int index) > if (dwc->ulpi_ext_vbus_drv) > reg |= DWC3_GUSB2PHYCFG_ULPIEXTVBUSDRV; > > + /* > + * Fixes High-speed negotiation issue with USB3340, see: > + * https://urldefense.com/v3/__http://ww1.microchip.com/downloads/en/DeviceDoc/80000645A.pdf__;!!A4F2R9G_pg!dXav8YHma6OhZWR1jhFvBxM4Wgv7OAUPjMgBS8CRz-P0x2OTr8PGbErUg7RtcbLfsNihsPay4lHdvoLJnN9nJoxGJqwb2qg$ > + * "Device Enumeration Failure with Link IP Systems" This platform specific info and how it was found should go into the commit message and not here. > + * According to documentation on the Internet, We should not just reference the "Internet". If you want to reference some documentation, please provide the DWC_usb3x document section and version. > + * DWC3_GUSB2PHYCFG_XCVRDLY: > + * Adds a delay between the assertion of the > + * ULPI Transceiver Select signal (for HS) and It can be for both ULPI and UTMI. > + * the assertion of the TxValid signal during a HS Chirp. > + * > + * This bit also needs to be set again when the device comes out > + * of hibernation, this is currently not an issue since hibernation > + * is not enabled. > + */ IMO, this doc should go in the commit message. > + if (dwc->enable_xcvrdly_quirk) > + reg |= DWC3_GUSB2PHYCFG_XCVRDLY; > + > dwc3_writel(dwc, DWC3_GUSB2PHYCFG(index), reg); > > return 0; > @@ -1855,6 +1872,9 @@ static void dwc3_get_properties(struct dwc3 *dwc) > dwc->dis_split_quirk = device_property_read_bool(dev, > "snps,dis-split-quirk"); > > + dwc->enable_xcvrdly_quirk = device_property_read_bool(dev, > + "snps,enable_xcvrdly_quirk"); Use "-" instead of "_" for property name, and change it to "snps,enable-usb2-transceiver-delay" Also, how are you passing this quirk? Through devicetree or software node? I don't see the user of this property. > + > dwc->lpm_nyet_threshold = lpm_nyet_threshold; > dwc->tx_de_emphasis = tx_de_emphasis; > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index a35b3db1f9f3..41418f3f85e1 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -302,6 +302,7 @@ > #define DWC3_GUSB2PHYCFG_SUSPHY BIT(6) > #define DWC3_GUSB2PHYCFG_ULPI_UTMI BIT(4) > #define DWC3_GUSB2PHYCFG_ENBLSLPM BIT(8) > +#define DWC3_GUSB2PHYCFG_XCVRDLY BIT(9) > #define DWC3_GUSB2PHYCFG_PHYIF(n) (n << 3) > #define DWC3_GUSB2PHYCFG_PHYIF_MASK DWC3_GUSB2PHYCFG_PHYIF(1) > #define DWC3_GUSB2PHYCFG_USBTRDTIM(n) (n << 10) > @@ -1403,6 +1404,7 @@ struct dwc3 { > unsigned dis_metastability_quirk:1; > > unsigned dis_split_quirk:1; > + unsigned enable_xcvrdly_quirk:1; Document the new field. Rename to enable_usb2_transceiver_delay. > unsigned async_callbacks:1; > unsigned sys_wakeup:1; > unsigned wakeup_configured:1; > -- > 2.52.0 > BR, Thinh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY, via "snps,enable_xcvrdly_quirk" 2026-02-25 0:05 ` Thinh Nguyen @ 2026-02-25 12:49 ` Ingo Rohloff 2026-02-26 2:39 ` Thinh Nguyen 2026-02-25 13:03 ` [PATCH v2 0/2] Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY Ingo Rohloff 1 sibling, 1 reply; 19+ messages in thread From: Ingo Rohloff @ 2026-02-25 12:49 UTC (permalink / raw) To: Thinh Nguyen; +Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org On Wed, 25 Feb 2026 00:05:17 +0000 Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > > + /* > > + * Fixes High-speed negotiation issue with USB3340, see: > > + * https://.../80000645A.pdf > > + * "Device Enumeration Failure with Link IP Systems" > > This platform specific info and how it was found should go into the > commit message and not here. > Done. > > + * According to documentation on the Internet, > > We should not just reference the "Internet". If you want to reference > some documentation, please provide the DWC_usb3x document section and > version. > I would love to, but Synopsys was completely unwilling to provide a programming manual to me. Synopsys told me I should contact Xilinx (now AMD) for access to the programming manual; and of course Xilinx refused, because Xilinx has signed an NDA. The "Internet" was the only source, where I found this information. The description I provide is from the forum post I now mention in the commit message. I put the description in the code, because I am afraid, that the forum post vanishes and then this information is lost to the public. I guess, if I were to gain access to the official programming manual, citing it might be a violation of the NDA I likely would have signed, but I am not a lawyer. > > > + * DWC3_GUSB2PHYCFG_XCVRDLY: > > + * Adds a delay between the assertion of the > > + * ULPI Transceiver Select signal (for HS) and > > It can be for both ULPI and UTMI. > Ah of course :-) I now mention both: ULPI/UTMI. > > + dwc->enable_xcvrdly_quirk = device_property_read_bool(dev, > > + "snps,enable_xcvrdly_quirk"); > > Use "-" instead of "_" for property name, and change it to > "snps,enable-usb2-transceiver-delay" > > ... > > + unsigned enable_xcvrdly_quirk:1; > > Document the new field. Rename to enable_usb2_transceiver_delay. Done. Note: I used "-" before to avoid having different strings for the member variable and the property name to make it easier to grep for both at the same time. > Also, how are you passing this quirk? Through devicetree or software > node? I don't see the user of this property. > On the hardware I have got (Xilinx Ultrascale+ ZynqMP) I pass it via devicetree. I now added a 2nd commit, which adds documentation for the devicetree bindings. with best regards Ingo Rohloff ------------------------------------------------------------------------- Dipl.-Inform. Ingo ROHLOFF Senior Staff Embedded Systems Engineering phone +49 8102 9876-142 - ingo.rohloff@lauterbach.com Lauterbach Engineering GmbH & Co. KG Altlaufstrasse 40, 85635 Hoehenkirchen-Siegertsbrunn, GERMANY www.lauterbach.com Registered Office: Hoehenkirchen-Siegertsbrunn, Germany, Local Court: Munich, HRA 87406, VAT ID: DE246770537, Managing Directors: Lothar Lauterbach, Stephan Lauterbach, Dr. Thomas Ullmann ------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY, via "snps,enable_xcvrdly_quirk" 2026-02-25 12:49 ` Ingo Rohloff @ 2026-02-26 2:39 ` Thinh Nguyen 2026-02-26 11:32 ` Ingo Rohloff 0 siblings, 1 reply; 19+ messages in thread From: Thinh Nguyen @ 2026-02-26 2:39 UTC (permalink / raw) To: Ingo Rohloff Cc: Thinh Nguyen, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org On Wed, Feb 25, 2026, Ingo Rohloff wrote: > On Wed, 25 Feb 2026 00:05:17 +0000 > Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > > > > + /* > > > + * Fixes High-speed negotiation issue with USB3340, see: > > > + * https://urldefense.com/v3/__https://.../80000645A.pdf__;!!A4F2R9G_pg!ZpksoYNTCoUlwA2oJmYi9dbFwlQ35mMKbZiEbcANJreOtoPEk83GTd8XwwKGwHQLMmJVNctONi-nBDgVTTPYltUA66t4AhY$ > > > + * "Device Enumeration Failure with Link IP Systems" > > > > This platform specific info and how it was found should go into the > > commit message and not here. > > > > Done. > > > > + * According to documentation on the Internet, > > > > We should not just reference the "Internet". If you want to reference > > some documentation, please provide the DWC_usb3x document section and > > version. > > > > I would love to, but Synopsys was completely unwilling to provide > a programming manual to me. > > Synopsys told me I should contact Xilinx (now AMD) for access to the > programming manual; and of course Xilinx refused, because Xilinx > has signed an NDA. > > The "Internet" was the only source, where I found this information. > The description I provide is from the forum post I now mention in the > commit message. > I put the description in the code, because I am afraid, that the forum > post vanishes and then this information is lost to the public. > > I guess, if I were to gain access to the official programming manual, > citing it might be a violation of the NDA I likely would have signed, > but I am not a lawyer. You can simply note in your commit log that your platform requires the enabling of this specific configuration, and that this solution was provided by your vendor Xilinx (if applicable). > > > > > > + * DWC3_GUSB2PHYCFG_XCVRDLY: > > > + * Adds a delay between the assertion of the > > > + * ULPI Transceiver Select signal (for HS) and This note is more relevant to the hardware designer. It doesn't explain why it is needed for your platform or any other one. I prefer we don't cite from the forum/Internet. Please remove it. > > > > It can be for both ULPI and UTMI. > > > > Ah of course :-) > I now mention both: ULPI/UTMI. > > > > > + dwc->enable_xcvrdly_quirk = device_property_read_bool(dev, > > > + "snps,enable_xcvrdly_quirk"); > > > > Use "-" instead of "_" for property name, and change it to > > "snps,enable-usb2-transceiver-delay" > > > > ... > > > + unsigned enable_xcvrdly_quirk:1; > > > > Document the new field. Rename to enable_usb2_transceiver_delay. > > Done. > Note: I used "-" before to avoid having different strings for the > member variable and the property name to make it easier to grep > for both at the same time. > > > Also, how are you passing this quirk? Through devicetree or software > > node? I don't see the user of this property. > > > > On the hardware I have got (Xilinx Ultrascale+ ZynqMP) I pass it via > devicetree. I now added a 2nd commit, which adds documentation for the > devicetree bindings. > You should pass this via software node through the glue driver. I don't think there's enough justification to create a new devicetree property just for this. BR, Thinh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY, via "snps,enable_xcvrdly_quirk" 2026-02-26 2:39 ` Thinh Nguyen @ 2026-02-26 11:32 ` Ingo Rohloff 2026-02-26 19:26 ` Thinh Nguyen 0 siblings, 1 reply; 19+ messages in thread From: Ingo Rohloff @ 2026-02-26 11:32 UTC (permalink / raw) To: Thinh Nguyen; +Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org Hello Thinh, On Thu, 26 Feb 2026 02:39:46 +0000 Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > > > Also, how are you passing this quirk? Through devicetree or software > > > node? I don't see the user of this property. > > > > > > > On the hardware I have got (Xilinx Ultrascale+ ZynqMP) I pass it via > > devicetree. I now added a 2nd commit, which adds documentation for the > > devicetree bindings. > > > > You should pass this via software node through the glue driver. I don't > think there's enough justification to create a new devicetree property > just for this. Please help me to understand: I think the glue driver you are referring to is the Xilinix specific one, so the one in `drivers/usb/dwc3/dwc3-xilinx.c`, which declares `dwc3_xlnx_driver`. Is this what you mean? I think I understand how to create a software node there and how to pass it to the main dwc3 driver. Do you mean to read in a dwc3_xlnx_driver specific device tree property and then pass this via software node to the main DWC3 driver? I am wondering if the dwc3_xlnx_driver is the right place for this. Please let me explain: The origin of the failing high-speed negotiation is not because of the ZynqMP specific hardware (so this particular specific DWC3 implementation), but because of the externally connected Microchip USB3340 ULPI PHY. Meaning: Any DWC3 implementation which is connected to this particular external ULPI PHY will have this high-speed negotiation problem. On the other hand: If you connect a different ULPI PHY to the ZynqMP DWC3 implementation, then this will work without any extra measures. The Xilinx evaluation boards use other ULPI PHYs (USB3320), which is why this problem didn't pop up with the Xilinx evaluation boards. It's luck, that the dwc3 controller has a bit which allows it to work together with this particular USB3340 ULPI PHY. The property in the device tree describes the circuit implemented on your hardware board: If you have an externally USB3340 ULPI PHY connected to a DWC3 controller, then you have to set this new property. If you have a different High-Speed USB PHY then you might not need to set this property. Of course this property will also help if you have a different High-Speed PHY which has the same behavior as the USB3340 ULPI PHY and so also requires setting the XCVRDLY bit. I think any DWC3 implementation which allows to connect an external ULPI PHY is affected by this problem. So should I really move this property to the Xilinx specific glue driver? with best regards Ingo ------------------------------------------------------------------------- Dipl.-Inform. Ingo ROHLOFF Senior Staff Embedded Systems Engineering phone +49 8102 9876-142 - ingo.rohloff@lauterbach.com Lauterbach Engineering GmbH & Co. KG Altlaufstrasse 40, 85635 Hoehenkirchen-Siegertsbrunn, GERMANY www.lauterbach.com Registered Office: Hoehenkirchen-Siegertsbrunn, Germany, Local Court: Munich, HRA 87406, VAT ID: DE246770537, Managing Directors: Lothar Lauterbach, Stephan Lauterbach, Dr. Thomas Ullmann ------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY, via "snps,enable_xcvrdly_quirk" 2026-02-26 11:32 ` Ingo Rohloff @ 2026-02-26 19:26 ` Thinh Nguyen 2026-02-26 20:27 ` Ingo Rohloff 0 siblings, 1 reply; 19+ messages in thread From: Thinh Nguyen @ 2026-02-26 19:26 UTC (permalink / raw) To: Ingo Rohloff Cc: Thinh Nguyen, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org On Thu, Feb 26, 2026, Ingo Rohloff wrote: > Hello Thinh, > > On Thu, 26 Feb 2026 02:39:46 +0000 > Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > > > > > Also, how are you passing this quirk? Through devicetree or software > > > > node? I don't see the user of this property. > > > > > > > > > > On the hardware I have got (Xilinx Ultrascale+ ZynqMP) I pass it via > > > devicetree. I now added a 2nd commit, which adds documentation for the > > > devicetree bindings. > > > > > > > You should pass this via software node through the glue driver. I don't > > think there's enough justification to create a new devicetree property > > just for this. > > Please help me to understand: > I think the glue driver you are referring to is the Xilinix specific one, > so the one in `drivers/usb/dwc3/dwc3-xilinx.c`, which declares > `dwc3_xlnx_driver`. > > Is this what you mean? > > I think I understand how to create a software node there and how to pass > it to the main dwc3 driver. > > Do you mean to read in a dwc3_xlnx_driver specific device tree property > and then pass this via software node to the main DWC3 driver? Not some device tree property, but the compatible string for your specific platform. > > I am wondering if the dwc3_xlnx_driver is the right place for this. > > Please let me explain: > > The origin of the failing high-speed negotiation is not because > of the ZynqMP specific hardware (so this particular specific DWC3 > implementation), but because of the externally connected > Microchip USB3340 ULPI PHY. > > Meaning: Any DWC3 implementation which is connected to this particular > external ULPI PHY will have this high-speed negotiation problem. > > On the other hand: If you connect a different ULPI PHY to the ZynqMP > DWC3 implementation, then this will work without any extra measures. > The Xilinx evaluation boards use other ULPI PHYs (USB3320), which is > why this problem didn't pop up with the Xilinx evaluation boards. > > It's luck, that the dwc3 controller has a bit which allows it > to work together with this particular USB3340 ULPI PHY. > > The property in the device tree describes the circuit implemented > on your hardware board: > If you have an externally USB3340 ULPI PHY connected to a > DWC3 controller, then you have to set this new property. > If you have a different High-Speed USB PHY then you might not need > to set this property. > Of course this property will also help if you have a different > High-Speed PHY which has the same behavior as the USB3340 ULPI PHY > and so also requires setting the XCVRDLY bit. > > I think any DWC3 implementation which allows to connect an external > ULPI PHY is affected by this problem. > > So should I really move this property to the Xilinx specific glue driver? > Let's keep the discussion in the same thread. Currently the dwc3-xilinx glue applies some common software node to all Xilinx platforms. You need to create a logic specific to your platform for this new dwc3 property, and the logic can be passed through of_device_id->data. This is what I noted in the dt-binding thread, and the comment[*] is the same here: It will be specific to the SoC. ie. whether the software node for this will be created will be dependent on the platform compatible string. Ideally, if someone introduces a new dwc3 property, there should be a user (with a specific compatible string or ID) for that property. BR, Thinh [*] https://lore.kernel.org/linux-usb/20260226190432.jq6c3gxwy6dydwpc@synopsys.com/T/#u ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY, via "snps,enable_xcvrdly_quirk" 2026-02-26 19:26 ` Thinh Nguyen @ 2026-02-26 20:27 ` Ingo Rohloff 2026-02-26 23:51 ` Thinh Nguyen 0 siblings, 1 reply; 19+ messages in thread From: Ingo Rohloff @ 2026-02-26 20:27 UTC (permalink / raw) To: Thinh Nguyen; +Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org Hello Thinh, Thank you for being patient! I still don't really understand, or maybe there is a general misunderstanding. > On Thu, Feb 26, 2026, Ingo Rohloff wrote: > > Do you mean to read in a dwc3_xlnx_driver specific device tree property > > and then pass this via software node to the main DWC3 driver? > > Not some device tree property, but the compatible string for your > specific platform. I am not sure what "platform" means here: My platform is a custom made PCB my company manufactures, which is part of a product we sell. On this PCB is a Xilinx Ultrascale+ ZynqMP chip connected to a Microchip USB3340 chip (and of course more non USB related chips). The dwc3_xlnx_of_match[] in drivers/usb/dwc3/dwc3-xilinx.c lists only these two devices, which are FPGAs from Xilinx. { .compatible = "xlnx,zynqmp-dwc3", .data = &dwc3_xlnx_init_zynqmp, }, { .compatible = "xlnx,versal-dwc3", .data = &dwc3_xlnx_init_versal, } We use the first entry "xlnx,zynqmp-dwc3". Do you mean these entries, when you say "platform" ? I could add a { .compatible = "lauterbach,powerdebug-v1-zynqmp-dwc3", .data = &dwc3_xlnx_init_zynqmp, }, and then use this "compatible" match to pass a property to the DWC3 driver to set the XCVRDLY bit. But this particular entry then refers to some completely specific PCB from my company, which is not publicly available (except as part of the product we sell). I don't think that you mean I should add another entry to dwc3_xlnx_of_match[] ? So I still don't understand to which "compatible" string you refer to. with best regards Ingo -- ------------------------------------------------------------------------- Dipl.-Inform. Ingo ROHLOFF Senior Staff Embedded Systems Engineering phone +49 8102 9876-142 - ingo.rohloff@lauterbach.com Lauterbach Engineering GmbH & Co. KG Altlaufstrasse 40, 85635 Hoehenkirchen-Siegertsbrunn, GERMANY www.lauterbach.com Registered Office: Hoehenkirchen-Siegertsbrunn, Germany, Local Court: Munich, HRA 87406, VAT ID: DE246770537, Managing Directors: Lothar Lauterbach, Stephan Lauterbach, Dr. Thomas Ullmann ------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY, via "snps,enable_xcvrdly_quirk" 2026-02-26 20:27 ` Ingo Rohloff @ 2026-02-26 23:51 ` Thinh Nguyen 2026-02-27 0:02 ` Thinh Nguyen 0 siblings, 1 reply; 19+ messages in thread From: Thinh Nguyen @ 2026-02-26 23:51 UTC (permalink / raw) To: Ingo Rohloff Cc: Thinh Nguyen, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org On Thu, Feb 26, 2026, Ingo Rohloff wrote: > Hello Thinh, > > Thank you for being patient! > > I still don't really understand, or maybe there is a general > misunderstanding. > > > On Thu, Feb 26, 2026, Ingo Rohloff wrote: > > > Do you mean to read in a dwc3_xlnx_driver specific device tree property > > > and then pass this via software node to the main DWC3 driver? > > > > Not some device tree property, but the compatible string for your > > specific platform. > > I am not sure what "platform" means here: SoC peripherals are exposed to Linux as platform devices such as dwc3-xilinx. > > My platform is a custom made PCB my company manufactures, > which is part of a product we sell. > > On this PCB is a Xilinx Ultrascale+ ZynqMP chip connected to > a Microchip USB3340 chip (and of course more non USB related chips). > > The dwc3_xlnx_of_match[] in drivers/usb/dwc3/dwc3-xilinx.c > lists only these two devices, which are FPGAs from Xilinx. > > > { > .compatible = "xlnx,zynqmp-dwc3", > .data = &dwc3_xlnx_init_zynqmp, > }, > { > .compatible = "xlnx,versal-dwc3", > .data = &dwc3_xlnx_init_versal, > } > > We use the first entry "xlnx,zynqmp-dwc3". > > Do you mean these entries, when you say "platform" ? > Yes, I was referring to those above such as "xlnx,zynqmp-dwc3". > > I could add a > { > .compatible = "lauterbach,powerdebug-v1-zynqmp-dwc3", > .data = &dwc3_xlnx_init_zynqmp, > }, > > and then use this "compatible" match to pass a property > to the DWC3 driver to set the XCVRDLY bit. No, don't do that. > > But this particular entry then refers to some > completely specific PCB from my company, which is not publicly > available (except as part of the product we sell). > > I don't think that you mean I should add another entry to > dwc3_xlnx_of_match[] ? > > So I still don't understand to which "compatible" string you refer to. > I see. Thanks for the explanation. I think I understand your setup better now. Perhaps what you can do is to check the phy compatible to see if it matches your USB3340 chip. Will something like this work? priv_data->usb3_phy = devm_phy_optional_get(dev, "usb3-phy"); if (!IS_ERR(priv_data->usb3_phy)) { phy_node = phy->dev.of_node; if (phy_node) { if (of_device_is_compatible(phy_node, "microchip,usb3340")) <create software node to pass property> } } else { ret = PTR_ERR(priv_data->usb3_phy); dev_err_probe(dev, ret, "failed to get USB3 PHY\n"); goto err; } BR, Thinh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY, via "snps,enable_xcvrdly_quirk" 2026-02-26 23:51 ` Thinh Nguyen @ 2026-02-27 0:02 ` Thinh Nguyen 0 siblings, 0 replies; 19+ messages in thread From: Thinh Nguyen @ 2026-02-27 0:02 UTC (permalink / raw) To: Ingo Rohloff; +Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org On Thu, Feb 26, 2026, Thinh Nguyen wrote: > On Thu, Feb 26, 2026, Ingo Rohloff wrote: > > Hello Thinh, > > > > Thank you for being patient! > > > > I still don't really understand, or maybe there is a general > > misunderstanding. > > > > > On Thu, Feb 26, 2026, Ingo Rohloff wrote: > > > > Do you mean to read in a dwc3_xlnx_driver specific device tree property > > > > and then pass this via software node to the main DWC3 driver? > > > > > > Not some device tree property, but the compatible string for your > > > specific platform. > > > > I am not sure what "platform" means here: > > SoC peripherals are exposed to Linux as platform devices such as > dwc3-xilinx. > > > > > My platform is a custom made PCB my company manufactures, > > which is part of a product we sell. > > > > On this PCB is a Xilinx Ultrascale+ ZynqMP chip connected to > > a Microchip USB3340 chip (and of course more non USB related chips). > > > > The dwc3_xlnx_of_match[] in drivers/usb/dwc3/dwc3-xilinx.c > > lists only these two devices, which are FPGAs from Xilinx. > > > > > > { > > .compatible = "xlnx,zynqmp-dwc3", > > .data = &dwc3_xlnx_init_zynqmp, > > }, > > { > > .compatible = "xlnx,versal-dwc3", > > .data = &dwc3_xlnx_init_versal, > > } > > > > We use the first entry "xlnx,zynqmp-dwc3". > > > > Do you mean these entries, when you say "platform" ? > > > > Yes, I was referring to those above such as "xlnx,zynqmp-dwc3". > > > > > I could add a > > { > > .compatible = "lauterbach,powerdebug-v1-zynqmp-dwc3", > > .data = &dwc3_xlnx_init_zynqmp, > > }, > > > > and then use this "compatible" match to pass a property > > to the DWC3 driver to set the XCVRDLY bit. > > No, don't do that. > > > > > But this particular entry then refers to some > > completely specific PCB from my company, which is not publicly > > available (except as part of the product we sell). > > > > I don't think that you mean I should add another entry to > > dwc3_xlnx_of_match[] ? > > > > So I still don't understand to which "compatible" string you refer to. > > > > I see. Thanks for the explanation. I think I understand your setup > better now. > > Perhaps what you can do is to check the phy compatible to see if it > matches your USB3340 chip. Will something like this work? > > priv_data->usb3_phy = devm_phy_optional_get(dev, "usb3-phy"); > if (!IS_ERR(priv_data->usb3_phy)) { > phy_node = phy->dev.of_node; > if (phy_node) { > if (of_device_is_compatible(phy_node, "microchip,usb3340")) > <create software node to pass property> > } > } else { > ret = PTR_ERR(priv_data->usb3_phy); > dev_err_probe(dev, ret, > "failed to get USB3 PHY\n"); > goto err; > } > Actually, Ignore the above. Since the compatible string for xlnx,zynqmp-dwc3 is not sufficient to describe different setups. This is enough justification to create a new devicetree property. BR, Thinh ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 0/2] Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY 2026-02-25 0:05 ` Thinh Nguyen 2026-02-25 12:49 ` Ingo Rohloff @ 2026-02-25 13:03 ` Ingo Rohloff 2026-02-25 13:03 ` [PATCH v2 1/2] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation Ingo Rohloff ` (2 more replies) 1 sibling, 3 replies; 19+ messages in thread From: Ingo Rohloff @ 2026-02-25 13:03 UTC (permalink / raw) To: Thinh.Nguyen; +Cc: linux-usb, Ingo Rohloff Support for USB3340 ULPI PHY high-speed negotiation, by adding a device tree property, which tells the controller to insert a delay before the assertion of the TxValid signal during a HS Chirp. Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com> --- Changes in v2: - Mention sources of information in commit message instead of code. - Renamed property to "snps,enable-usb2-transceiver-delay". - Renamed struct member to "enable_usb2_transceiver_delay". - Describe dt-bindings in a second commit. - Link to v1: https://lore.kernel.org/linux-usb/20260224141438.39524-1-ingo.rohloff@lauterbach.com/ --- Ingo Rohloff (2): usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation. dt-bindings: usb: dwc3: Add property to insert delay before TxValid. .../devicetree/bindings/usb/snps,dwc3-common.yaml | 7 +++++++ drivers/usb/dwc3/core.c | 12 ++++++++++++ drivers/usb/dwc3/core.h | 4 ++++ 3 files changed, 23 insertions(+) -- 2.52.0 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/2] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation. 2026-02-25 13:03 ` [PATCH v2 0/2] Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY Ingo Rohloff @ 2026-02-25 13:03 ` Ingo Rohloff 2026-02-25 13:03 ` [PATCH v2 2/2] dt-bindings: usb: dwc3: Add property to insert delay before TxValid Ingo Rohloff 2026-02-26 10:51 ` [PATCH v2 0/2] Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY Krzysztof Kozlowski 2 siblings, 0 replies; 19+ messages in thread From: Ingo Rohloff @ 2026-02-25 13:03 UTC (permalink / raw) To: Thinh.Nguyen; +Cc: linux-usb, Ingo Rohloff The Microchip USB3340x ULPI PHY requires a delay when switching to the high-speed transmitter. See: http://ww1.microchip.com/downloads/en/DeviceDoc/80000645A.pdf Module 2 "Device Enumeration Failure with Link IP Systems" For details on the behavior and fix, refer to the AMD (formerly Xilinx) forum post: "USB stuck in full speed mode with USB3340 ULPI PHY, ZynqMP." Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com> --- drivers/usb/dwc3/core.c | 12 ++++++++++++ drivers/usb/dwc3/core.h | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 161a4d58b2ce..903ee0cc2787 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -777,6 +777,15 @@ static int dwc3_hs_phy_setup(struct dwc3 *dwc, int index) if (dwc->ulpi_ext_vbus_drv) reg |= DWC3_GUSB2PHYCFG_ULPIEXTVBUSDRV; + /* + * DWC3_GUSB2PHYCFG_XCVRDLY: + * Adds a delay between the assertion of the + * ULPI/UTMI Transceiver Select signal (for HS) and + * the assertion of the TxValid signal during a HS Chirp. + */ + if (dwc->enable_usb2_transceiver_delay) + reg |= DWC3_GUSB2PHYCFG_XCVRDLY; + dwc3_writel(dwc, DWC3_GUSB2PHYCFG(index), reg); return 0; @@ -1855,6 +1864,9 @@ static void dwc3_get_properties(struct dwc3 *dwc) dwc->dis_split_quirk = device_property_read_bool(dev, "snps,dis-split-quirk"); + dwc->enable_usb2_transceiver_delay = device_property_read_bool(dev, + "snps,enable-usb2-transceiver-delay"); + dwc->lpm_nyet_threshold = lpm_nyet_threshold; dwc->tx_de_emphasis = tx_de_emphasis; diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index a35b3db1f9f3..ba58a14095f4 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -302,6 +302,7 @@ #define DWC3_GUSB2PHYCFG_SUSPHY BIT(6) #define DWC3_GUSB2PHYCFG_ULPI_UTMI BIT(4) #define DWC3_GUSB2PHYCFG_ENBLSLPM BIT(8) +#define DWC3_GUSB2PHYCFG_XCVRDLY BIT(9) #define DWC3_GUSB2PHYCFG_PHYIF(n) (n << 3) #define DWC3_GUSB2PHYCFG_PHYIF_MASK DWC3_GUSB2PHYCFG_PHYIF(1) #define DWC3_GUSB2PHYCFG_USBTRDTIM(n) (n << 10) @@ -1161,6 +1162,8 @@ struct dwc3_glue_ops { * 3 - Reserved * @dis_metastability_quirk: set to disable metastability quirk. * @dis_split_quirk: set to disable split boundary. + * @enable_usb2_transceiver_delay: Set to insert a delay before the + * assertion of the TxValid signal during a HS Chirp. * @sys_wakeup: set if the device may do system wakeup. * @wakeup_configured: set if the device is configured for remote wakeup. * @suspended: set to track suspend event due to U3/L2. @@ -1403,6 +1406,7 @@ struct dwc3 { unsigned dis_metastability_quirk:1; unsigned dis_split_quirk:1; + unsigned enable_usb2_transceiver_delay:1; unsigned async_callbacks:1; unsigned sys_wakeup:1; unsigned wakeup_configured:1; -- 2.52.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 2/2] dt-bindings: usb: dwc3: Add property to insert delay before TxValid. 2026-02-25 13:03 ` [PATCH v2 0/2] Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY Ingo Rohloff 2026-02-25 13:03 ` [PATCH v2 1/2] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation Ingo Rohloff @ 2026-02-25 13:03 ` Ingo Rohloff 2026-02-26 10:51 ` Krzysztof Kozlowski 2026-02-26 10:51 ` [PATCH v2 0/2] Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY Krzysztof Kozlowski 2 siblings, 1 reply; 19+ messages in thread From: Ingo Rohloff @ 2026-02-25 13:03 UTC (permalink / raw) To: Thinh.Nguyen; +Cc: linux-usb, Ingo Rohloff The Microchip USB3340x ULPI PHY requires a delay when switching to the high-speed transmitter. See: http://ww1.microchip.com/downloads/en/DeviceDoc/80000645A.pdf Module 2 "Device Enumeration Failure with Link IP Systems" The new property "snps,enable-usb2-transceiver-delay" tells the controller to insert this required delay. Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com> --- .../devicetree/bindings/usb/snps,dwc3-common.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml index 6c0b8b653824..6a90c08713cc 100644 --- a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml +++ b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml @@ -207,6 +207,13 @@ properties: avoid -EPROTO errors with usbhid on some devices (Hikey 970). type: boolean + snps,enable-usb2-transceiver-delay: + description: + When set, inserts a delay before the assertion of the TxValid signal + during a HS Chirp. This fixes USB high-speed negotiation when using + a USB 3440x ULPI PHY. + type: boolean + snps,gfladj-refclk-lpm-sel-quirk: description: When set, run the SOF/ITP counter based on ref_clk. -- 2.52.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: usb: dwc3: Add property to insert delay before TxValid. 2026-02-25 13:03 ` [PATCH v2 2/2] dt-bindings: usb: dwc3: Add property to insert delay before TxValid Ingo Rohloff @ 2026-02-26 10:51 ` Krzysztof Kozlowski 2026-02-26 16:12 ` Ingo Rohloff 0 siblings, 1 reply; 19+ messages in thread From: Krzysztof Kozlowski @ 2026-02-26 10:51 UTC (permalink / raw) To: Ingo Rohloff, Thinh.Nguyen; +Cc: linux-usb On 25/02/2026 14:03, Ingo Rohloff wrote: > The Microchip USB3340x ULPI PHY requires a delay when switching to the > high-speed transmitter. See: > http://ww1.microchip.com/downloads/en/DeviceDoc/80000645A.pdf > Module 2 "Device Enumeration Failure with Link IP Systems" So that's deducible from the compatible and you do not need this property at all? > > The new property "snps,enable-usb2-transceiver-delay" tells the controller > to insert this required delay. Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, instead use mainline) or work on fork of kernel (don't, instead use mainline). Just use b4 and everything should be fine, although remember about `b4 prep --auto-to-cc` if you added new patches to the patchset. You missed at least devicetree list (maybe more), so this won't be tested by automated tooling. Performing review on untested code might be a waste of time. Please kindly resend and include all necessary To/Cc entries. Please organize the patch documenting the compatible (DT bindings) before the patch using that compatible. See also: https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/devicetree/bindings/submitting-patches.rst#L46 > > Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com> > --- > .../devicetree/bindings/usb/snps,dwc3-common.yaml | 7 +++++++ > 1 file changed, 7 insertions(+) > Best regards, Krzysztof ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: usb: dwc3: Add property to insert delay before TxValid. 2026-02-26 10:51 ` Krzysztof Kozlowski @ 2026-02-26 16:12 ` Ingo Rohloff 2026-02-26 19:04 ` Thinh Nguyen 0 siblings, 1 reply; 19+ messages in thread From: Ingo Rohloff @ 2026-02-26 16:12 UTC (permalink / raw) To: Krzysztof Kozlowski; +Cc: Thinh.Nguyen, linux-usb Hello Krzysztof, On Thu, 26 Feb 2026 11:51:27 +0100 Krzysztof Kozlowski <krzk@kernel.org> wrote: > On 25/02/2026 14:03, Ingo Rohloff wrote: > > The Microchip USB3340x ULPI PHY requires a delay when switching to the > > high-speed transmitter. See: > > http://ww1.microchip.com/downloads/en/DeviceDoc/80000645A.pdf > > Module 2 "Device Enumeration Failure with Link IP Systems" > > So that's deducible from the compatible and you do not need this > property at all? > Thanks for giving me a new idea :-) The problem is, that the original device trees provided by Xilinx do not mention the external ULPI PHY at all, which means I do not have a "compatible" property to match to. Additionally: The patch only works for the DWC3 controller, because this particular USB controller has the necessary hardware support to insert the needed delay. Of course other USB controllers might not even need this fix at all, because other controllers might always have the required delay. Right now the patch by me uses a device tree property to tell the DWC3 controller to set the XCVRDLY bit to insert a delay (completely autonomously done in hardware). Which means: The system designer needs to know if this is necessary or not, depending on the used ULPI PHY. If the delay is needed, the system designer then sets the according device tree property. I searched other device trees, but almost none of them have a device tree "ulpi" node. The DWC3 code does call ulpi_register_interface() and this function does look for an "ulpi" device tree node (which currently doesn't exist in my device tree). By inserting such a device tree node, I can at least make "drivers/usb/common/ulpi.c" read out the vendor and product ID of the ULPI PHY via ulpi_read_id(). Using this product/vendor ID the DWC3 controller driver could automatically set the XCVRDLY bit to support this particular PHY. The condition is: * If you have a DWC3 controller, which uses an external ULPI PHY * and the external ULPI PHY is a Microchip USB3340 chip (detected via Vendor/Product ID) then set the XCVRDLY bit in the DWC3 controller. I am not sure what the right way is to implement this kind of automagic and if this kind of automagic is wanted at all. @Thinh: I need some advice: Should the DWC3 controller really match to specific ULPI Vendor/Product IDs to decide if to set the XCVRDLY bit or not? How does the DWC3 controller get access to the ULPI Vendor/Product ID or to the information that the XCVRDLY should be set? Reading the code I think an alternative way might be: Implement a "struct ulpi_driver" for this specific USB3340 ULPI PHY. Make this ulpi_driver create a software node to tell the DWC3 controller to set the XCVRDLY bit. Does this sound sensible at all? This sounds like a lot of work to only fix this particular combination of hardware. > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC.... I am sorry: I did not re-run get_maintainers.pl on v2 patch version. I will do better for the next version. with best regards Ingo -- ------------------------------------------------------------------------- Dipl.-Inform. Ingo ROHLOFF Senior Staff Embedded Systems Engineering phone +49 8102 9876-142 - ingo.rohloff@lauterbach.com Lauterbach Engineering GmbH & Co. KG Altlaufstrasse 40, 85635 Hoehenkirchen-Siegertsbrunn, GERMANY www.lauterbach.com Registered Office: Hoehenkirchen-Siegertsbrunn, Germany, Local Court: Munich, HRA 87406, VAT ID: DE246770537, Managing Directors: Lothar Lauterbach, Stephan Lauterbach, Dr. Thomas Ullmann ------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: usb: dwc3: Add property to insert delay before TxValid. 2026-02-26 16:12 ` Ingo Rohloff @ 2026-02-26 19:04 ` Thinh Nguyen 2026-02-27 0:20 ` Thinh Nguyen 0 siblings, 1 reply; 19+ messages in thread From: Thinh Nguyen @ 2026-02-26 19:04 UTC (permalink / raw) To: Ingo Rohloff; +Cc: Krzysztof Kozlowski, Thinh Nguyen, linux-usb@vger.kernel.org Hi Ingo, On Thu, Feb 26, 2026, Ingo Rohloff wrote: > Hello Krzysztof, > > On Thu, 26 Feb 2026 11:51:27 +0100 > Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > On 25/02/2026 14:03, Ingo Rohloff wrote: > > > The Microchip USB3340x ULPI PHY requires a delay when switching to the > > > high-speed transmitter. See: > > > https://urldefense.com/v3/__http://ww1.microchip.com/downloads/en/DeviceDoc/80000645A.pdf__;!!A4F2R9G_pg!YRzZkLTZr5Bhh57FiXmyq7e4wVPHUbXZdgH3a627O2kbzpOTPiw_1MKGc_zD5cr8_Y-r1h5XvyFL9c7YRNObAl-lmUNgSuU$ > > > Module 2 "Device Enumeration Failure with Link IP Systems" > > > > So that's deducible from the compatible and you do not need this > > property at all? > > > > Thanks for giving me a new idea :-) > > The problem is, that the original device trees provided by Xilinx do not > mention the external ULPI PHY at all, which means I do not have a > "compatible" property to match to. > > Additionally: The patch only works for the DWC3 controller, because this > particular USB controller has the necessary hardware support to insert the > needed delay. > Of course other USB controllers might not even need this fix at all, > because other controllers might always have the required delay. > > Right now the patch by me uses a device tree property to tell the DWC3 > controller to set the XCVRDLY bit to insert a delay (completely > autonomously done in hardware). > > Which means: The system designer needs to know if this is necessary or > not, depending on the used ULPI PHY. If the delay is needed, the system > designer then sets the according device tree property. > > I searched other device trees, but almost none of them have a device tree > "ulpi" node. > > The DWC3 code does call ulpi_register_interface() and this function does > look for an "ulpi" device tree node (which currently doesn't exist in my > device tree). > > By inserting such a device tree node, I can at least make > "drivers/usb/common/ulpi.c" read out the vendor and product ID of the ULPI > PHY via ulpi_read_id(). > > Using this product/vendor ID the DWC3 controller driver could > automatically set the XCVRDLY bit to support this particular PHY. > > The condition is: > * If you have a DWC3 controller, which uses an external ULPI PHY > * and the external ULPI PHY is a Microchip USB3340 chip > (detected via Vendor/Product ID) > then set the XCVRDLY bit in the DWC3 controller. > > I am not sure what the right way is to implement this kind of automagic > and if this kind of automagic is wanted at all. > > @Thinh: I need some advice: > > Should the DWC3 controller really match to specific > ULPI Vendor/Product IDs to decide if to set the XCVRDLY bit or not? > > How does the DWC3 controller get access to the > ULPI Vendor/Product ID or to the information that the XCVRDLY should be > set? > > Reading the code I think an alternative way might be: > Implement a "struct ulpi_driver" for this specific USB3340 ULPI PHY. > Make this ulpi_driver create a software node to tell the DWC3 controller > to set the XCVRDLY bit. > > Does this sound sensible at all? > This sounds like a lot of work to only fix this particular combination > of hardware. > As noted in my previous comment in the separate thread, I suggested to use software node instead of introducing a new devicetree property. It will be specific to the SoC. ie. whether the software node for this will be created will be dependent on the platform compatible string. Ideally, if someone introduces a new dwc3 property, there should be a user (with a specific compatible string or ID) for that property. BR, Thinh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: usb: dwc3: Add property to insert delay before TxValid. 2026-02-26 19:04 ` Thinh Nguyen @ 2026-02-27 0:20 ` Thinh Nguyen 2026-02-27 11:04 ` Ingo Rohloff 0 siblings, 1 reply; 19+ messages in thread From: Thinh Nguyen @ 2026-02-27 0:20 UTC (permalink / raw) To: Krzysztof Kozlowski; +Cc: Ingo Rohloff, linux-usb@vger.kernel.org Hi Krzysztof, On Thu, Feb 26, 2026, Thinh Nguyen wrote: > Hi Ingo, > > On Thu, Feb 26, 2026, Ingo Rohloff wrote: > > Hello Krzysztof, > > > > On Thu, 26 Feb 2026 11:51:27 +0100 > > Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > On 25/02/2026 14:03, Ingo Rohloff wrote: > > > > The Microchip USB3340x ULPI PHY requires a delay when switching to the > > > > high-speed transmitter. See: > > > > https://urldefense.com/v3/__http://ww1.microchip.com/downloads/en/DeviceDoc/80000645A.pdf__;!!A4F2R9G_pg!YRzZkLTZr5Bhh57FiXmyq7e4wVPHUbXZdgH3a627O2kbzpOTPiw_1MKGc_zD5cr8_Y-r1h5XvyFL9c7YRNObAl-lmUNgSuU$ > > > > Module 2 "Device Enumeration Failure with Link IP Systems" > > > > > > So that's deducible from the compatible and you do not need this > > > property at all? > > > > > After reading Ingo's comment again, I don't think we can simply deduct whether this property is needed from just the Xilinx dwc3 compatible. I think this one may need its own devicetree property. BR, Thinh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: usb: dwc3: Add property to insert delay before TxValid. 2026-02-27 0:20 ` Thinh Nguyen @ 2026-02-27 11:04 ` Ingo Rohloff 2026-02-28 1:05 ` Thinh Nguyen 0 siblings, 1 reply; 19+ messages in thread From: Ingo Rohloff @ 2026-02-27 11:04 UTC (permalink / raw) To: Thinh Nguyen; +Cc: Krzysztof Kozlowski, linux-usb@vger.kernel.org On Fri, 27 Feb 2026 00:20:10 +0000 Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > On Thu, Feb 26, 2026, Thinh Nguyen wrote: > > On Thu, Feb 26, 2026, Ingo Rohloff wrote: > > > On Thu, 26 Feb 2026 11:51:27 +0100 > > > Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > > On 25/02/2026 14:03, Ingo Rohloff wrote: > > > > > The Microchip USB3340x ULPI PHY requires a delay when switching > > > > > to the high-speed transmitter. See: > > > > > https://urldefense.com/v3/__http://ww1.microchip.com/downloads/en/DeviceDoc/80000645A.pdf__;!!A4F2R9G_pg!YRzZkLTZr5Bhh57FiXmyq7e4wVPHUbXZdgH3a627O2kbzpOTPiw_1MKGc_zD5cr8_Y-r1h5XvyFL9c7YRNObAl-lmUNgSuU$ > > > > > Module 2 "Device Enumeration Failure with Link IP Systems" > > > > > > > > > > > > > So that's deducible from the compatible and you do not need this > > > > property at all? > > > > > > > > > After reading Ingo's comment again, I don't think we can simply deduct > whether this property is needed from just the Xilinx dwc3 compatible. > I think this one may need its own devicetree property. Krzystof gave me another idea though: In drivers/usb/dwc3/ulpi.c the dwc3 code will call dwc3_ulpi_init(), if the DWC3 controller is connected via ULPI to an USB ULPI PHY. dwc3_ulpi_init() will call ulpi_register_interface(), which (under the right circumstances) will call ulpi_read_id(). At this point in time it would be easy to use dwc3_ulpi_read()/dwc3_ulpi_write() to read out the USB ULPI PHY VendorID/ProductID by mimicking the behavior of ulpi_read_id() from drivers/usb/common/ulpi.c Based on the VendorID/ProductID we then could set dwc3->enable_usb2_transceiver_delay if necessary. This approach means: * No new device tree property necessary. * Code autodetects the problematic combo of DWC3 + USB3340 ULPI PHY and applies the necessary settings. * Will work for any combination of DWC3 IP + USB3340 ULPI PHY, and not only for the Xilinx specific DWC3 implementation. * Future proof: If there ever is another ULPI PHY, which needs special settings in the DWC3 controller, you could then just extend the code to detect this new ULPI PHY and apply the necessary settings. Caveat: You have the code to read out the ULPI PHY VendorID/ProductID twice: * Once in ulpi_read_id() from drivers/usb/common/ulpi.c * Once in drivers/usb/dwc3/ulpi.c I do not see any easy way to get access to the VendorID/ProductID from ulpi_read_id(); in particular because right now this function is only called if the ULPI PHY has a corresponding node in the device tree, which currently is not the case. Meaning right now ulpi_read_id() is NOT called. with best regards Ingo -- ------------------------------------------------------------------------- Dipl.-Inform. Ingo ROHLOFF Senior Staff Embedded Systems Engineering phone +49 8102 9876-142 - ingo.rohloff@lauterbach.com Lauterbach Engineering GmbH & Co. KG Altlaufstrasse 40, 85635 Hoehenkirchen-Siegertsbrunn, GERMANY www.lauterbach.com Registered Office: Hoehenkirchen-Siegertsbrunn, Germany, Local Court: Munich, HRA 87406, VAT ID: DE246770537, Managing Directors: Lothar Lauterbach, Stephan Lauterbach, Dr. Thomas Ullmann ------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: usb: dwc3: Add property to insert delay before TxValid. 2026-02-27 11:04 ` Ingo Rohloff @ 2026-02-28 1:05 ` Thinh Nguyen 0 siblings, 0 replies; 19+ messages in thread From: Thinh Nguyen @ 2026-02-28 1:05 UTC (permalink / raw) To: Ingo Rohloff; +Cc: Thinh Nguyen, Krzysztof Kozlowski, linux-usb@vger.kernel.org On Fri, Feb 27, 2026, Ingo Rohloff wrote: > On Fri, 27 Feb 2026 00:20:10 +0000 > Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > > On Thu, Feb 26, 2026, Thinh Nguyen wrote: > > > On Thu, Feb 26, 2026, Ingo Rohloff wrote: > > > > On Thu, 26 Feb 2026 11:51:27 +0100 > > > > Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > > > > On 25/02/2026 14:03, Ingo Rohloff wrote: > > > > > > The Microchip USB3340x ULPI PHY requires a delay when switching > > > > > > to the high-speed transmitter. See: > > > > > > https://urldefense.com/v3/__http://ww1.microchip.com/downloads/en/DeviceDoc/80000645A.pdf__;!!A4F2R9G_pg!YRzZkLTZr5Bhh57FiXmyq7e4wVPHUbXZdgH3a627O2kbzpOTPiw_1MKGc_zD5cr8_Y-r1h5XvyFL9c7YRNObAl-lmUNgSuU$ > > > > > > Module 2 "Device Enumeration Failure with Link IP Systems" > > > > > > > > > > > > > > > > So that's deducible from the compatible and you do not need this > > > > > property at all? > > > > > > > > > > > > > After reading Ingo's comment again, I don't think we can simply deduct > > whether this property is needed from just the Xilinx dwc3 compatible. > > I think this one may need its own devicetree property. > > Krzystof gave me another idea though: > In drivers/usb/dwc3/ulpi.c the dwc3 code will call dwc3_ulpi_init(), > if the DWC3 controller is connected via ULPI to an USB ULPI PHY. That's a much better option. > > dwc3_ulpi_init() will call ulpi_register_interface(), which > (under the right circumstances) will call ulpi_read_id(). > > At this point in time it would be easy to use > dwc3_ulpi_read()/dwc3_ulpi_write() to read out the > USB ULPI PHY VendorID/ProductID > by mimicking the behavior of ulpi_read_id() from drivers/usb/common/ulpi.c > > Based on the VendorID/ProductID we then could set > dwc3->enable_usb2_transceiver_delay > if necessary. I don't think it's necessary anymore, see my feedback in your patch. (Note that even in hibernation case, we save and restore the GUSB2PHYCFG register on resume. So there's no need for the core to know about this state.) > > This approach means: > * No new device tree property necessary. > * Code autodetects the problematic combo of DWC3 + USB3340 ULPI PHY > and applies the necessary settings. > * Will work for any combination of DWC3 IP + USB3340 ULPI PHY, > and not only for the Xilinx specific DWC3 implementation. > * Future proof: If there ever is another ULPI PHY, which needs special > settings in the DWC3 controller, you could then just extend the > code to detect this new ULPI PHY and apply the necessary settings. > > Caveat: > You have the code to read out the ULPI PHY VendorID/ProductID twice: > * Once in ulpi_read_id() from drivers/usb/common/ulpi.c > * Once in drivers/usb/dwc3/ulpi.c I don't think this is a big issue. > > I do not see any easy way to get access to the VendorID/ProductID > from ulpi_read_id(); in particular because right now this function > is only called if the ULPI PHY has a corresponding node in the > device tree, which currently is not the case. > Meaning right now ulpi_read_id() is NOT called. > BR, Thinh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 0/2] Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY 2026-02-25 13:03 ` [PATCH v2 0/2] Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY Ingo Rohloff 2026-02-25 13:03 ` [PATCH v2 1/2] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation Ingo Rohloff 2026-02-25 13:03 ` [PATCH v2 2/2] dt-bindings: usb: dwc3: Add property to insert delay before TxValid Ingo Rohloff @ 2026-02-26 10:51 ` Krzysztof Kozlowski 2 siblings, 0 replies; 19+ messages in thread From: Krzysztof Kozlowski @ 2026-02-26 10:51 UTC (permalink / raw) To: Ingo Rohloff, Thinh.Nguyen; +Cc: linux-usb On 25/02/2026 14:03, Ingo Rohloff wrote: > Support for USB3340 ULPI PHY high-speed negotiation, by adding a > device tree property, which tells the controller to insert a delay > before the assertion of the TxValid signal during a HS Chirp. > > Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com> > > --- > Changes in v2: > - Mention sources of information in commit message instead of code. > - Renamed property to "snps,enable-usb2-transceiver-delay". > - Renamed struct member to "enable_usb2_transceiver_delay". > - Describe dt-bindings in a second commit. > - Link to v1: https://lore.kernel.org/linux-usb/20260224141438.39524-1-ingo.rohloff@lauterbach.com/ > > --- Do not attach (thread) your patchsets to some other threads (unrelated or older versions). This buries them deep in the mailbox and might interfere with applying entire sets. See also: https://elixir.bootlin.com/linux/v6.16-rc2/source/Documentation/process/submitting-patches.rst#L830 Best regards, Krzysztof ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2026-02-28 1:05 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-24 14:14 [PATCH] usb: dwc3: Support for USB3340x ULPI PHY, via "snps,enable_xcvrdly_quirk" Ingo Rohloff 2026-02-25 0:05 ` Thinh Nguyen 2026-02-25 12:49 ` Ingo Rohloff 2026-02-26 2:39 ` Thinh Nguyen 2026-02-26 11:32 ` Ingo Rohloff 2026-02-26 19:26 ` Thinh Nguyen 2026-02-26 20:27 ` Ingo Rohloff 2026-02-26 23:51 ` Thinh Nguyen 2026-02-27 0:02 ` Thinh Nguyen 2026-02-25 13:03 ` [PATCH v2 0/2] Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY Ingo Rohloff 2026-02-25 13:03 ` [PATCH v2 1/2] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation Ingo Rohloff 2026-02-25 13:03 ` [PATCH v2 2/2] dt-bindings: usb: dwc3: Add property to insert delay before TxValid Ingo Rohloff 2026-02-26 10:51 ` Krzysztof Kozlowski 2026-02-26 16:12 ` Ingo Rohloff 2026-02-26 19:04 ` Thinh Nguyen 2026-02-27 0:20 ` Thinh Nguyen 2026-02-27 11:04 ` Ingo Rohloff 2026-02-28 1:05 ` Thinh Nguyen 2026-02-26 10:51 ` [PATCH v2 0/2] Re: [PATCH] usb: dwc3: Support for USB3340x ULPI PHY Krzysztof Kozlowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox