* [PATCH v4 0/1] usb: dwc3: Support for USB3340x ULPI PHY
@ 2026-03-03 13:37 Ingo Rohloff
2026-03-03 13:37 ` [PATCH v4 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-03-03 13:37 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
Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com>
---
Changes in v4:
- Rename dwc3_ulpi_detect_quirks() -> dwc3_ulpi_detect_config()
- Rename dwc3_hs_apply_ulpi_quirks() -> dwc3_ulpi_apply_config()
- Call dwc3_ulpi_apply_config() each time dwc3_core_init() is called.
Makes sure config is applied when resuming.
- Link to v3: https://lore.kernel.org/linux-usb/20260227133038.45150-1-ingo.rohloff@lauterbach.com/
---
Changes in v3:
- Do not mention 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 | 44 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 64 insertions(+)
--
2.52.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation.
2026-03-03 13:37 [PATCH v4 0/1] usb: dwc3: Support for USB3340x ULPI PHY Ingo Rohloff
@ 2026-03-03 13:37 ` Ingo Rohloff
2026-03-03 23:49 ` Thinh Nguyen
0 siblings, 1 reply; 5+ messages in thread
From: Ingo Rohloff @ 2026-03-03 13:37 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 | 44 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 64 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index cacc4ec9f7ce..da1572d268d0 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_ulpi_apply_config(struct dwc3 *dwc)
+{
+ int index;
+ u32 reg;
+
+ if (dwc->enable_usb2_transceiver_delay) {
+ 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
@@ -1363,6 +1377,8 @@ int dwc3_core_init(struct dwc3 *dwc)
dwc->ulpi_ready = true;
}
+ dwc3_ulpi_apply_config(dwc);
+
if (!dwc->phys_ready) {
ret = dwc3_core_get_phy(dwc);
if (ret)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 67bcc8dccc89..7d0845184223 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..7197bddae88a 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,46 @@ static const struct ulpi_ops dwc3_ulpi_ops = {
.write = dwc3_ulpi_write,
};
+static void dwc3_ulpi_detect_config(struct dwc3 *dwc)
+{
+ u16 product_id;
+ u16 vendor_id;
+ int ret;
+
+ /* 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;
+
+ dev_dbg(
+ dwc->dev, "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 +134,8 @@ int dwc3_ulpi_init(struct dwc3 *dwc)
return PTR_ERR(dwc->ulpi);
}
+ dwc3_ulpi_detect_config(dwc);
+
return 0;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation.
2026-03-03 13:37 ` [PATCH v4 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation Ingo Rohloff
@ 2026-03-03 23:49 ` Thinh Nguyen
2026-03-04 15:25 ` Ingo Rohloff
0 siblings, 1 reply; 5+ messages in thread
From: Thinh Nguyen @ 2026-03-03 23:49 UTC (permalink / raw)
To: Ingo Rohloff
Cc: Thinh Nguyen, gregkh@linuxfoundation.org,
linux-usb@vger.kernel.org
On Tue, Mar 03, 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!dH4jByMJyCF1lSbySxDHGOa6PY_7bhIKFpS-3mCJvU2A455tQc1lHjiYNl_FX0x3vZWNBXoF_xKbuQqbaab8dA5yV-dtEvM$
> 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 | 44 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 64 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index cacc4ec9f7ce..da1572d268d0 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_ulpi_apply_config(struct dwc3 *dwc)
> +{
> + int index;
> + u32 reg;
> +
> + if (dwc->enable_usb2_transceiver_delay) {
> + 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 done in dwc3_hs_phy_setup(). See more comments about this
below.
> + }
> + }
> +}
> +
> /**
> * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
> * @dwc: Pointer to our controller context structure
> @@ -1363,6 +1377,8 @@ int dwc3_core_init(struct dwc3 *dwc)
> dwc->ulpi_ready = true;
> }
>
> + dwc3_ulpi_apply_config(dwc);
> +
> if (!dwc->phys_ready) {
> ret = dwc3_core_get_phy(dwc);
> if (ret)
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 67bcc8dccc89..7d0845184223 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..7197bddae88a 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,46 @@ static const struct ulpi_ops dwc3_ulpi_ops = {
> .write = dwc3_ulpi_write,
> };
>
> +static void dwc3_ulpi_detect_config(struct dwc3 *dwc)
We can rename this to dwc3_ulpi_get_properties() and declare it in
core.h. Then we can have the dwc3_get_properties() call this function.
This will ensure that the order of the setting of the GUSB2PHYCFG is
done in dwc3_hs_phy_setup(), which is prior to registering the ulpi phy.
> +{
> + u16 product_id;
> + u16 vendor_id;
> + int ret;
> +
> + /* 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;
Do we need to check for data integrity here? That check will be done
during ulpi init, where it has proper checks and can fail properly
there.
BR,
Thinh
> +
> + 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;
> +
> + dev_dbg(
> + dwc->dev, "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 +134,8 @@ int dwc3_ulpi_init(struct dwc3 *dwc)
> return PTR_ERR(dwc->ulpi);
> }
>
> + dwc3_ulpi_detect_config(dwc);
> +
> return 0;
> }
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation.
2026-03-03 23:49 ` Thinh Nguyen
@ 2026-03-04 15:25 ` Ingo Rohloff
2026-03-05 1:32 ` Thinh Nguyen
0 siblings, 1 reply; 5+ messages in thread
From: Ingo Rohloff @ 2026-03-04 15:25 UTC (permalink / raw)
To: Thinh Nguyen; +Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org
Hello Thinh,
> > +static void dwc3_ulpi_detect_config(struct dwc3 *dwc)
>
> We can rename this to dwc3_ulpi_get_properties() and declare it in
> core.h. Then we can have the dwc3_get_properties() call this function.
I tested this and this doesn't work:
At the time dwc3_get_properties() runs, the DWC3 controller hasn't been
initialized at all yet.
Any access to a DWC3 hardware register fails (I think the clocks to the
DWC3 IP are not even enabled at that point of time) and the kernel crashes.
To read out the ULPI PHY Vendor/Product ID at least the ULPI
interface of the DWC3 controller must be up and running.
> This will ensure that the order of the setting of the GUSB2PHYCFG is
> done in dwc3_hs_phy_setup(), which is prior to registering the ulpi phy.
I originally had the same idea: Make sure GUSB2PHYCFG settings are
correct before I do any access to the ULPI PHY; that's why I originally
used a device tree property.
But because I now use the Vendor/Product ID, this doesn't work;
instead what I implemented right now does this:
- First setup the ULPI interface to be able to access the ULPI PHY
- Check what ULPI PHY model is connected (via Vendor/Product ID)
- Apply further workarounds depending on the detected ULPI PHY model.
The setting of the XCVRDLY in GUSB2PHYCFG only has an effect once you
enable the USB connection.
> > +static void dwc3_ulpi_apply_config(struct dwc3 *dwc)
> > +{
> > ...
>
> This should be done in dwc3_hs_phy_setup(). See more comments about this
> below.
The problem is: As far as I can tell, dwc3_hs_phy_setup() has to run first,
to setup the DWC3 controller to access the ULPI PHY at all.
Right at the beginning the code in dwc3_hs_phy_setup() reads:
/* Select the HS PHY interface */
switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3)) {
case DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI:
I think that's also the reason why dwc3_core_init() uses this order
ret = dwc3_phy_setup(dwc);
...
if (!dwc->ulpi_ready) {
ret = dwc3_core_ulpi_init(dwc);
You first have to make sure that dwc3_phy_setup() has setup the interfaces
to the PHYs (both High-Speed and Super-Speed) before trying to
access the HS PHY.
> > +static void dwc3_ulpi_detect_config(struct dwc3 *dwc)
> > +{
> > + u16 product_id;
> > + u16 vendor_id;
> > + int ret;
> > +
> > + /* Test the interface */
> > ...
>
> Do we need to check for data integrity here? That check will be done
> during ulpi init, where it has proper checks and can fail properly
> there.
Thanks!
OMG I am stupid :-):
I misunderstood the code of ulpi_register_interface():
I thought ulpi_register_interface() would only do this check and read out
the Vendor/Product ID, if there is a matching device tree node, but it
turns out it will always do the check and read out the
Vendor/Product ID, even if there is no matching device tree node.
This in turn means: I don't even have to replicate any code,
because ulpi_register_interface() will already read out the
ULPI PHYs Vendor/Product ID.
If drivers/usb/dwc3/core.c includes "linux/ulpi/driver.h" the code in
core.c already has access to the ULPI PHY Vendor/Product ID via
dwc->ulpi->id.vendor
dwc->ulpi->id.product
Which means: I directly can use this to enable the XCVRDLY in GUSB2PHYCFG
if the specific Vendor/Product ID is found.
What do you think?
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 v4 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation.
2026-03-04 15:25 ` Ingo Rohloff
@ 2026-03-05 1:32 ` Thinh Nguyen
0 siblings, 0 replies; 5+ messages in thread
From: Thinh Nguyen @ 2026-03-05 1:32 UTC (permalink / raw)
To: Ingo Rohloff
Cc: Thinh Nguyen, gregkh@linuxfoundation.org,
linux-usb@vger.kernel.org
On Wed, Mar 04, 2026, Ingo Rohloff wrote:
> Hello Thinh,
>
> > > +static void dwc3_ulpi_detect_config(struct dwc3 *dwc)
> >
> > We can rename this to dwc3_ulpi_get_properties() and declare it in
> > core.h. Then we can have the dwc3_get_properties() call this function.
>
> I tested this and this doesn't work:
> At the time dwc3_get_properties() runs, the DWC3 controller hasn't been
> initialized at all yet.
>
> Any access to a DWC3 hardware register fails (I think the clocks to the
> DWC3 IP are not even enabled at that point of time) and the kernel crashes.
Ah, that's right.
>
> To read out the ULPI PHY Vendor/Product ID at least the ULPI
> interface of the DWC3 controller must be up and running.
>
> > This will ensure that the order of the setting of the GUSB2PHYCFG is
> > done in dwc3_hs_phy_setup(), which is prior to registering the ulpi phy.
>
> I originally had the same idea: Make sure GUSB2PHYCFG settings are
> correct before I do any access to the ULPI PHY; that's why I originally
> used a device tree property.
>
> But because I now use the Vendor/Product ID, this doesn't work;
> instead what I implemented right now does this:
>
> - First setup the ULPI interface to be able to access the ULPI PHY
> - Check what ULPI PHY model is connected (via Vendor/Product ID)
> - Apply further workarounds depending on the detected ULPI PHY model.
>
> The setting of the XCVRDLY in GUSB2PHYCFG only has an effect once you
> enable the USB connection.
>
Right. I guess we can't avoid separating this step.
>
> > > +static void dwc3_ulpi_apply_config(struct dwc3 *dwc)
> > > +{
> > > ...
> >
> > This should be done in dwc3_hs_phy_setup(). See more comments about this
> > below.
>
> The problem is: As far as I can tell, dwc3_hs_phy_setup() has to run first,
> to setup the DWC3 controller to access the ULPI PHY at all.
>
> Right at the beginning the code in dwc3_hs_phy_setup() reads:
>
> /* Select the HS PHY interface */
> switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3)) {
> case DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI:
>
> I think that's also the reason why dwc3_core_init() uses this order
>
> ret = dwc3_phy_setup(dwc);
> ...
> if (!dwc->ulpi_ready) {
> ret = dwc3_core_ulpi_init(dwc);
>
> You first have to make sure that dwc3_phy_setup() has setup the interfaces
> to the PHYs (both High-Speed and Super-Speed) before trying to
> access the HS PHY.
>
>
> > > +static void dwc3_ulpi_detect_config(struct dwc3 *dwc)
> > > +{
> > > + u16 product_id;
> > > + u16 vendor_id;
> > > + int ret;
> > > +
> > > + /* Test the interface */
> > > ...
> >
> > Do we need to check for data integrity here? That check will be done
> > during ulpi init, where it has proper checks and can fail properly
> > there.
>
> Thanks!
> OMG I am stupid :-):
> I misunderstood the code of ulpi_register_interface():
> I thought ulpi_register_interface() would only do this check and read out
> the Vendor/Product ID, if there is a matching device tree node, but it
> turns out it will always do the check and read out the
> Vendor/Product ID, even if there is no matching device tree node.
>
> This in turn means: I don't even have to replicate any code,
> because ulpi_register_interface() will already read out the
> ULPI PHYs Vendor/Product ID.
>
> If drivers/usb/dwc3/core.c includes "linux/ulpi/driver.h" the code in
> core.c already has access to the ULPI PHY Vendor/Product ID via
> dwc->ulpi->id.vendor
> dwc->ulpi->id.product
>
> Which means: I directly can use this to enable the XCVRDLY in GUSB2PHYCFG
> if the specific Vendor/Product ID is found.
>
> What do you think?
>
Sounds good! But can we have the checking for Vendor/Product ID in
ulpi.c instead of core.c? I want to keep the core.c generic.
Thanks,
Thinh
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-03-05 1:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-03 13:37 [PATCH v4 0/1] usb: dwc3: Support for USB3340x ULPI PHY Ingo Rohloff
2026-03-03 13:37 ` [PATCH v4 1/1] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation Ingo Rohloff
2026-03-03 23:49 ` Thinh Nguyen
2026-03-04 15:25 ` Ingo Rohloff
2026-03-05 1:32 ` Thinh Nguyen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.