* [PATCH v3 0/1] usb: dwc3: Support for USB3340x ULPI PHY
@ 2026-02-27 13:30 Ingo Rohloff
2026-02-27 13:30 ` [PATCH v3 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation Ingo Rohloff
0 siblings, 1 reply; 5+ messages in thread
From: Ingo Rohloff @ 2026-02-27 13:30 UTC (permalink / raw)
To: Thinh.Nguyen; +Cc: gregkh, linux-usb, Ingo Rohloff
The problem only pops up if you combine a DWC3 controller with a
Microchip USB3340 ULPI PHY.
This patch uses the USB Vendor/Product ID of the ULPI PHY
to detect the USB3340 model and then applies the necessary fix,
if this model is found.
Benefits of this approach
- Does not require any modification to existing device trees.
- Should work for all DWC3 IP / USB3340 combinations, not only
for the Ultrascale+ ZyngMP DWC3 implementation, where this
problem was originally found.
- Easy to extend in the future if a similar situation arises again.
Caveats:
- Replicates code from drivers/usb/common/ulpi.c
---
Changes in v3:
- Do not meantion what the DWC3_GUSB2PHYCFG_XCVRDLY does.
- Do not use device tree property to set enable_usb2_transceiver_delay.
Instead autodetect if it's necessary to set enable_usb2_transceiver_delay.
- Link to v2: https://lore.kernel.org/linux-usb/20260225130323.24606-1-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 (1):
usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation.
drivers/usb/dwc3/core.c | 16 +++++++++++++++
drivers/usb/dwc3/core.h | 4 ++++
drivers/usb/dwc3/ulpi.c | 43 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 63 insertions(+)
--
2.52.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation.
2026-02-27 13:30 [PATCH v3 0/1] usb: dwc3: Support for USB3340x ULPI PHY Ingo Rohloff
@ 2026-02-27 13:30 ` Ingo Rohloff
2026-02-28 0:59 ` Thinh Nguyen
0 siblings, 1 reply; 5+ messages in thread
From: Ingo Rohloff @ 2026-02-27 13:30 UTC (permalink / raw)
To: Thinh.Nguyen; +Cc: gregkh, 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."
This patch uses the USB PHY Vendor-ID and Product-ID to detect the
USB3340 PHY and then applies the necessary fix if this PHY is found.
Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com>
---
drivers/usb/dwc3/core.c | 16 +++++++++++++++
drivers/usb/dwc3/core.h | 4 ++++
drivers/usb/dwc3/ulpi.c | 43 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 63 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index cacc4ec9f7ce..cc1818635eb4 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -782,6 +782,20 @@ static int dwc3_hs_phy_setup(struct dwc3 *dwc, int index)
return 0;
}
+static void dwc3_hs_apply_ulpi_quirks(struct dwc3 *dwc)
+{
+ if (dwc->enable_usb2_transceiver_delay) {
+ int index;
+ u32 reg;
+
+ for (index = 0; index < dwc->num_usb2_ports; index++) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(index));
+ reg |= DWC3_GUSB2PHYCFG_XCVRDLY;
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
+ }
+ }
+}
+
/**
* dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
* @dwc: Pointer to our controller context structure
@@ -1361,6 +1375,8 @@ int dwc3_core_init(struct dwc3 *dwc)
return ret;
}
dwc->ulpi_ready = true;
+
+ dwc3_hs_apply_ulpi_quirks(dwc);
}
if (!dwc->phys_ready) {
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 67bcc8dccc89..94a2a55518c9 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)
@@ -1163,6 +1164,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.
@@ -1406,6 +1409,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;
diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
index 57daad15f502..e1a1a58f862c 100644
--- a/drivers/usb/dwc3/ulpi.c
+++ b/drivers/usb/dwc3/ulpi.c
@@ -14,6 +14,8 @@
#include "core.h"
#include "io.h"
+#define USB_VENDOR_MICROCHIP 0x0424
+
#define DWC3_ULPI_ADDR(a) \
((a >= ULPI_EXT_VENDOR_SPECIFIC) ? \
DWC3_GUSB2PHYACC_ADDR(ULPI_ACCESS_EXTENDED) | \
@@ -83,6 +85,45 @@ static const struct ulpi_ops dwc3_ulpi_ops = {
.write = dwc3_ulpi_write,
};
+static void dwc3_ulpi_detect_quirks(struct dwc3 *dwc)
+{
+ int ret;
+ u16 vendor_id, product_id;
+
+ /* Test the interface */
+ ret = dwc3_ulpi_write(dwc->dev, ULPI_SCRATCH, 0xaa);
+ if (ret < 0)
+ return;
+
+ ret = dwc3_ulpi_read(dwc->dev, ULPI_SCRATCH);
+ if (ret < 0)
+ return;
+
+ if (ret != 0xaa)
+ return;
+
+ vendor_id = dwc3_ulpi_read(dwc->dev, ULPI_VENDOR_ID_LOW);
+ vendor_id |= dwc3_ulpi_read(dwc->dev, ULPI_VENDOR_ID_HIGH) << 8;
+
+ product_id = dwc3_ulpi_read(dwc->dev, ULPI_PRODUCT_ID_LOW);
+ product_id |= dwc3_ulpi_read(dwc->dev, ULPI_PRODUCT_ID_HIGH) << 8;
+
+ pr_info(
+ "dwc3_ulpi: VendorID 0x%04x, ProductID 0x%04x\n",
+ vendor_id, product_id
+ );
+ switch (vendor_id) {
+ case USB_VENDOR_MICROCHIP:
+ switch (product_id) {
+ case 0x0009:
+ /* Microchip USB3340 ULPI PHY */
+ dwc->enable_usb2_transceiver_delay = true;
+ break;
+ }
+ break;
+ }
+}
+
int dwc3_ulpi_init(struct dwc3 *dwc)
{
/* Register the interface */
@@ -92,6 +133,8 @@ int dwc3_ulpi_init(struct dwc3 *dwc)
return PTR_ERR(dwc->ulpi);
}
+ dwc3_ulpi_detect_quirks(dwc);
+
return 0;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation.
2026-02-27 13:30 ` [PATCH v3 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation Ingo Rohloff
@ 2026-02-28 0:59 ` Thinh Nguyen
2026-03-02 11:53 ` Ingo Rohloff
0 siblings, 1 reply; 5+ messages in thread
From: Thinh Nguyen @ 2026-02-28 0:59 UTC (permalink / raw)
To: Ingo Rohloff
Cc: Thinh Nguyen, gregkh@linuxfoundation.org,
linux-usb@vger.kernel.org
On Fri, Feb 27, 2026, 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!cuAZjiUZyG51WLifZERoNCQsqZwqS5JqH_qsck0l8jAxBUuaarEH5CovWVC6WK7ltamHxavcQzHaeUxX2KS4IPnb0KcLs0U$
> 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."
>
> This patch uses the USB PHY Vendor-ID and Product-ID to detect the
> USB3340 PHY and then applies the necessary fix if this PHY is found.
>
> Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com>
> ---
> drivers/usb/dwc3/core.c | 16 +++++++++++++++
> drivers/usb/dwc3/core.h | 4 ++++
> drivers/usb/dwc3/ulpi.c | 43 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 63 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index cacc4ec9f7ce..cc1818635eb4 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -782,6 +782,20 @@ static int dwc3_hs_phy_setup(struct dwc3 *dwc, int index)
> return 0;
> }
>
> +static void dwc3_hs_apply_ulpi_quirks(struct dwc3 *dwc)
> +{
> + if (dwc->enable_usb2_transceiver_delay) {
> + int index;
> + u32 reg;
> +
> + for (index = 0; index < dwc->num_usb2_ports; index++) {
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(index));
> + reg |= DWC3_GUSB2PHYCFG_XCVRDLY;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
> + }
> + }
This should be placed in dwc3_ulpi_init() or dwc3_ulpi_detect_quirks().
> +}
> +
> /**
> * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
> * @dwc: Pointer to our controller context structure
> @@ -1361,6 +1375,8 @@ int dwc3_core_init(struct dwc3 *dwc)
> return ret;
> }
> dwc->ulpi_ready = true;
> +
> + dwc3_hs_apply_ulpi_quirks(dwc);
> }
>
> if (!dwc->phys_ready) {
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 67bcc8dccc89..94a2a55518c9 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)
Use tab for spacing.
> #define DWC3_GUSB2PHYCFG_PHYIF(n) (n << 3)
> #define DWC3_GUSB2PHYCFG_PHYIF_MASK DWC3_GUSB2PHYCFG_PHYIF(1)
> #define DWC3_GUSB2PHYCFG_USBTRDTIM(n) (n << 10)
> @@ -1163,6 +1164,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.
> @@ -1406,6 +1409,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;
> diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
> index 57daad15f502..e1a1a58f862c 100644
> --- a/drivers/usb/dwc3/ulpi.c
> +++ b/drivers/usb/dwc3/ulpi.c
> @@ -14,6 +14,8 @@
> #include "core.h"
> #include "io.h"
>
> +#define USB_VENDOR_MICROCHIP 0x0424
> +
> #define DWC3_ULPI_ADDR(a) \
> ((a >= ULPI_EXT_VENDOR_SPECIFIC) ? \
> DWC3_GUSB2PHYACC_ADDR(ULPI_ACCESS_EXTENDED) | \
> @@ -83,6 +85,45 @@ static const struct ulpi_ops dwc3_ulpi_ops = {
> .write = dwc3_ulpi_write,
> };
>
> +static void dwc3_ulpi_detect_quirks(struct dwc3 *dwc)
Can we rename this to dwc3_ulpi_setup() or dwc3_ulpi_config()?
I would avoid to use the term "quirk", it's to indicate something's
broken. In your case, I think it's more correct to note that it just
a config specific to your chip.
> +{
> + int ret;
> + u16 vendor_id, product_id;
Just minor formatting style request: Can we declare variables in
separate lines and keep them in reversed christmas tree style?
> +
> + /* Test the interface */
> + ret = dwc3_ulpi_write(dwc->dev, ULPI_SCRATCH, 0xaa);
> + if (ret < 0)
> + return;
> +
> + ret = dwc3_ulpi_read(dwc->dev, ULPI_SCRATCH);
> + if (ret < 0)
> + return;
> +
> + if (ret != 0xaa)
> + return;
> +
> + vendor_id = dwc3_ulpi_read(dwc->dev, ULPI_VENDOR_ID_LOW);
> + vendor_id |= dwc3_ulpi_read(dwc->dev, ULPI_VENDOR_ID_HIGH) << 8;
> +
> + product_id = dwc3_ulpi_read(dwc->dev, ULPI_PRODUCT_ID_LOW);
> + product_id |= dwc3_ulpi_read(dwc->dev, ULPI_PRODUCT_ID_HIGH) << 8;
> +
> + pr_info(
Use dev_debug(). Normal users probably don't need to know this info.
> + "dwc3_ulpi: VendorID 0x%04x, ProductID 0x%04x\n",
> + vendor_id, product_id
> + );
> + switch (vendor_id) {
> + case USB_VENDOR_MICROCHIP:
> + switch (product_id) {
> + case 0x0009:
> + /* Microchip USB3340 ULPI PHY */
> + dwc->enable_usb2_transceiver_delay = true;
Do we really need this field enable_usb2_transceiver_delay anymore now
that we have this check base on the ProductID?
> + break;
> + }
> + break;
> + }
> +}
> +
> int dwc3_ulpi_init(struct dwc3 *dwc)
> {
> /* Register the interface */
> @@ -92,6 +133,8 @@ int dwc3_ulpi_init(struct dwc3 *dwc)
> return PTR_ERR(dwc->ulpi);
> }
>
> + dwc3_ulpi_detect_quirks(dwc);
> +
> return 0;
> }
>
> --
> 2.52.0
>
This is a much better solution than before!
Thanks,
Thinh
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation.
2026-02-28 0:59 ` Thinh Nguyen
@ 2026-03-02 11:53 ` Ingo Rohloff
2026-03-03 0:22 ` Thinh Nguyen
0 siblings, 1 reply; 5+ messages in thread
From: Ingo Rohloff @ 2026-03-02 11:53 UTC (permalink / raw)
To: Thinh Nguyen; +Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org
Hello Thinh,
I hope I didn't cut out too much of the discussion:
> > --- a/drivers/usb/dwc3/ulpi.c
> > +++ b/drivers/usb/dwc3/ulpi.c
> >
> > +static void dwc3_ulpi_detect_quirks(struct dwc3 *dwc)
> > +...
> > + switch (vendor_id) {
> > + case USB_VENDOR_MICROCHIP:
> > + switch (product_id) {
> > + case 0x0009:
> > + /* Microchip USB3340 ULPI PHY */
> > + dwc->enable_usb2_transceiver_delay = true;
> > ...
>
>
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> >
> > +static void dwc3_hs_apply_ulpi_quirks(struct dwc3 *dwc)
> > +{
> > + if (dwc->enable_usb2_transceiver_delay) {
> > + int index;
> > + u32 reg;
> > +
> > + for (index = 0; index < dwc->num_usb2_ports; index++) {
> > + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(index));
> > + reg |= DWC3_GUSB2PHYCFG_XCVRDLY;
> > + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
> > + }
> > + }
> > ...
>
> This should be placed in dwc3_ulpi_init() or dwc3_ulpi_detect_quirks().
>
> ...
>
> Do we really need this field enable_usb2_transceiver_delay anymore now
> that we have this check base on the ProductID?
I have thought about this, but I think I have to do it like above.
I think modifying the GUSB2PHYCFG in drivers/usb/dwc3/ulpi.c is the wrong
place.
"ulpi.c" currently doesn't touch the DWC3 controller itself at all.
It seems all of the config for GUSB2PHYCFG is done via `dwc3_core_init()`.
This function is also called for resuming operation after a suspend:
dwc3_plat_runtime_resume
dwc3_runtime_resume
dwc3_resume_common
dwc3_core_init_for_resume
dwc3_core_init
So I think the right place to modify GUSB2PHYCFG according to the detected
UPLI PHY is in dwc3_core_init().
Note: dwc3_core_init() detects the ULPI PHY only once:
if (!dwc->ulpi_ready) {
...
dwc->ulpi_ready = true;
}
My previous v3 patch has the bug to call `dwc3_hs_apply_ulpi_quirks`
(to be renamed in v4) ONLY ONCE. I think that's wrong:
The strategy is: When the ULPI PHY is detected, set
dwc->enable_usb2_transceiver_delay
if necessary.
Then use this bit to modify GUSB2PHYCFG if necessary:
* Once after the ULPI PHY is detected initially,
via dwc3_core_probe().
* Each time operation is resumed after a suspend.
That's why `dwc3_hs_apply_ulpi_quirks` has to be called
each time dwc3_core_init() is called.
**ALTERNATIVE**
Store the found ULPI Vendor/Product ID in `struct dwc3` instead of the
`enable_usb2_transceiver_delay` boolean and then directly use the
ULPI Vendor/Product ID to apply the necessary config of GUSB2PHYCFG in
`dwc3_hs_apply_ulpi_quirks`.
What do you think?
I will try to incorporate all other feedback you gave me.
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] 5+ messages in thread
* Re: [PATCH v3 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation.
2026-03-02 11:53 ` Ingo Rohloff
@ 2026-03-03 0:22 ` Thinh Nguyen
0 siblings, 0 replies; 5+ messages in thread
From: Thinh Nguyen @ 2026-03-03 0:22 UTC (permalink / raw)
To: Ingo Rohloff
Cc: Thinh Nguyen, gregkh@linuxfoundation.org,
linux-usb@vger.kernel.org
On Mon, Mar 02, 2026, Ingo Rohloff wrote:
> Hello Thinh,
>
> I hope I didn't cut out too much of the discussion:
>
> > > --- a/drivers/usb/dwc3/ulpi.c
> > > +++ b/drivers/usb/dwc3/ulpi.c
> > >
> > > +static void dwc3_ulpi_detect_quirks(struct dwc3 *dwc)
> > > +...
> > > + switch (vendor_id) {
> > > + case USB_VENDOR_MICROCHIP:
> > > + switch (product_id) {
> > > + case 0x0009:
> > > + /* Microchip USB3340 ULPI PHY */
> > > + dwc->enable_usb2_transceiver_delay = true;
> > > ...
> >
> >
> > > --- a/drivers/usb/dwc3/core.c
> > > +++ b/drivers/usb/dwc3/core.c
> > >
> > > +static void dwc3_hs_apply_ulpi_quirks(struct dwc3 *dwc)
> > > +{
> > > + if (dwc->enable_usb2_transceiver_delay) {
> > > + int index;
> > > + u32 reg;
> > > +
> > > + for (index = 0; index < dwc->num_usb2_ports; index++) {
> > > + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(index));
> > > + reg |= DWC3_GUSB2PHYCFG_XCVRDLY;
> > > + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
> > > + }
> > > + }
> > > ...
> >
> > This should be placed in dwc3_ulpi_init() or dwc3_ulpi_detect_quirks().
> >
> > ...
> >
> > Do we really need this field enable_usb2_transceiver_delay anymore now
> > that we have this check base on the ProductID?
>
> I have thought about this, but I think I have to do it like above.
>
> I think modifying the GUSB2PHYCFG in drivers/usb/dwc3/ulpi.c is the wrong
> place.
> "ulpi.c" currently doesn't touch the DWC3 controller itself at all.
>
> It seems all of the config for GUSB2PHYCFG is done via `dwc3_core_init()`.
> This function is also called for resuming operation after a suspend:
>
> dwc3_plat_runtime_resume
> dwc3_runtime_resume
> dwc3_resume_common
> dwc3_core_init_for_resume
> dwc3_core_init
>
> So I think the right place to modify GUSB2PHYCFG according to the detected
> UPLI PHY is in dwc3_core_init().
>
> Note: dwc3_core_init() detects the ULPI PHY only once:
>
> if (!dwc->ulpi_ready) {
> ...
> dwc->ulpi_ready = true;
> }
>
Ah! I forgot that the ULPI init function is only executed once.
> My previous v3 patch has the bug to call `dwc3_hs_apply_ulpi_quirks`
> (to be renamed in v4) ONLY ONCE. I think that's wrong:
>
> The strategy is: When the ULPI PHY is detected, set
> dwc->enable_usb2_transceiver_delay
> if necessary.
> Then use this bit to modify GUSB2PHYCFG if necessary:
> * Once after the ULPI PHY is detected initially,
> via dwc3_core_probe().
> * Each time operation is resumed after a suspend.
> That's why `dwc3_hs_apply_ulpi_quirks` has to be called
> each time dwc3_core_init() is called.
>
Yes. Do this. If there's a poweroff and vcc reset is asserted, then the
GUSB2PHYCFG will need to be re-initialized.
>
> **ALTERNATIVE**
> Store the found ULPI Vendor/Product ID in `struct dwc3` instead of the
> `enable_usb2_transceiver_delay` boolean and then directly use the
> ULPI Vendor/Product ID to apply the necessary config of GUSB2PHYCFG in
> `dwc3_hs_apply_ulpi_quirks`.
>
Not this.
> What do you think?
>
Sounds good. Thanks for double checking.
>
>
> I will try to incorporate all other feedback you gave me.
>
Thanks!
BR,
Thinh
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-03-03 0:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-27 13:30 [PATCH v3 0/1] usb: dwc3: Support for USB3340x ULPI PHY Ingo Rohloff
2026-02-27 13:30 ` [PATCH v3 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation Ingo Rohloff
2026-02-28 0:59 ` Thinh Nguyen
2026-03-02 11:53 ` Ingo Rohloff
2026-03-03 0:22 ` Thinh Nguyen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox