public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
* [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

* [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] 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 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 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

* 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 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] 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

* 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

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