2024-02-15 19:02:18

by Pandey, Radhey Shyam

[permalink] [raw]
Subject: [PATCH] dt-bindings: usb: dwc3: Add snps,enable_guctl1_ipd_quirk

From: Piyush Mehta <[email protected]>

SNPS controller when configured in HOST mode maintains Inter Packet
Delay (IPD) of ~380ns which works with most of the super-speed hubs
except VIA-LAB hubs. When IPD is ~380ns HOST controller fails to
enumerate FS/LS devices when connected behind VIA-LAB hubs.

To address the above issue, add 'snps,enable_guctl1_ipd_quirk' quirk,
This quirk set the bit 9 of GUCTL1 that enables the workaround in HW to
reduce the ULPI clock latency by 1 cycle, thus reducing the IPD (~360ns).

Signed-off-by: Piyush Mehta <[email protected]>
Signed-off-by: Radhey Shyam Pandey <[email protected]>
---
In the zynqmp public database GUCTL1 bit 9 is reserved but it is used to
enable a fix related to Inter Packet Delay in HW. The documentation team
is working to update GUCTL1 bit 9 description.
---
Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 8f5d250070c7..b226457a6e50 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -280,6 +280,13 @@ properties:
xhci reset. And the vbus will back to 5V automatically when reset done.
type: boolean

+ snps,enable_guctl1_ipd_quirk:
+ description:
+ When set, HW reduce the ULPI clock latency by 1 cycle, thus reducing
+ the IPD (~360ns) and making controller enumerate FS/LS devices
+ connected behind via-hubs.
+ type: boolean
+
snps,is-utmi-l1-suspend:
description:
True when DWC3 asserts output signal utmi_l1_suspend_n, false when
--
2.34.1



2024-02-15 19:40:18

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: usb: dwc3: Add snps,enable_guctl1_ipd_quirk

On Fri, Feb 16, 2024 at 12:31:05AM +0530, Radhey Shyam Pandey wrote:
> From: Piyush Mehta <[email protected]>
>
> SNPS controller when configured in HOST mode maintains Inter Packet
> Delay (IPD) of ~380ns which works with most of the super-speed hubs
> except VIA-LAB hubs. When IPD is ~380ns HOST controller fails to
> enumerate FS/LS devices when connected behind VIA-LAB hubs.
>
> To address the above issue, add 'snps,enable_guctl1_ipd_quirk' quirk,
> This quirk set the bit 9 of GUCTL1 that enables the workaround in HW to
> reduce the ULPI clock latency by 1 cycle, thus reducing the IPD (~360ns).
>
> Signed-off-by: Piyush Mehta <[email protected]>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>
> ---
> In the zynqmp public database GUCTL1 bit 9 is reserved but it is used to
> enable a fix related to Inter Packet Delay in HW. The documentation team
> is working to update GUCTL1 bit 9 description.

Does this just affect the zynqmp?
If it does, then you don't need a property - do this based on
compatible.
If it does affect other devices, what prevents the workaround being
performed for all dwc3 controllers?

Cheers,
Conor.

> ---
> Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index 8f5d250070c7..b226457a6e50 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -280,6 +280,13 @@ properties:
> xhci reset. And the vbus will back to 5V automatically when reset done.
> type: boolean
>
> + snps,enable_guctl1_ipd_quirk:

No underscores in properties please.

> + description:
> + When set, HW reduce the ULPI clock latency by 1 cycle, thus reducing
> + the IPD (~360ns) and making controller enumerate FS/LS devices
> + connected behind via-hubs.
> + type: boolean
> +
> snps,is-utmi-l1-suspend:
> description:
> True when DWC3 asserts output signal utmi_l1_suspend_n, false when
> --
> 2.34.1
>


Attachments:
(No filename) (2.16 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-26 17:32:04

by Pandey, Radhey Shyam

[permalink] [raw]
Subject: RE: [PATCH] dt-bindings: usb: dwc3: Add snps,enable_guctl1_ipd_quirk

> -----Original Message-----
> From: Conor Dooley <[email protected]>
> Sent: Friday, February 16, 2024 1:09 AM
> To: Pandey, Radhey Shyam <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; git (AMD-Xilinx)
> <[email protected]>; Piyush Mehta <[email protected]>
> Subject: Re: [PATCH] dt-bindings: usb: dwc3: Add
> snps,enable_guctl1_ipd_quirk
>
> On Fri, Feb 16, 2024 at 12:31:05AM +0530, Radhey Shyam Pandey wrote:
> > From: Piyush Mehta <[email protected]>
> >
> > SNPS controller when configured in HOST mode maintains Inter Packet
> > Delay (IPD) of ~380ns which works with most of the super-speed hubs
> > except VIA-LAB hubs. When IPD is ~380ns HOST controller fails to
> > enumerate FS/LS devices when connected behind VIA-LAB hubs.
> >
> > To address the above issue, add 'snps,enable_guctl1_ipd_quirk' quirk,
> > This quirk set the bit 9 of GUCTL1 that enables the workaround in HW
> > to reduce the ULPI clock latency by 1 cycle, thus reducing the IPD (~360ns).
> >
> > Signed-off-by: Piyush Mehta <[email protected]>
> > Signed-off-by: Radhey Shyam Pandey <[email protected]>
> > ---
> > In the zynqmp public database GUCTL1 bit 9 is reserved but it is used
> > to enable a fix related to Inter Packet Delay in HW. The documentation
> > team is working to update GUCTL1 bit 9 description.
>
> Does this just affect the zynqmp?
> If it does, then you don't need a property - do this based on compatible.

Yes this only affect zynqmp. I will apply IPD quirk based on zynqmp compatible.
There is related discussion ongoing[1] on how to do it ? i.e in core or platform
glue driver. Thinh suggested have it in platform code and I am working on it.
So will drop this binding patch.
[1]: https://lore.kernel.org/all/[email protected]/
Thanks,
Radhey

> If it does affect other devices, what prevents the workaround being
> performed for all dwc3 controllers?
>
> Cheers,
> Conor.
>
> > ---
> > Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > index 8f5d250070c7..b226457a6e50 100644
> > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > @@ -280,6 +280,13 @@ properties:
> > xhci reset. And the vbus will back to 5V automatically when reset done.
> > type: boolean
> >
> > + snps,enable_guctl1_ipd_quirk:
>
> No underscores in properties please.
>
> > + description:
> > + When set, HW reduce the ULPI clock latency by 1 cycle, thus reducing
> > + the IPD (~360ns) and making controller enumerate FS/LS devices
> > + connected behind via-hubs.
> > + type: boolean
> > +
> > snps,is-utmi-l1-suspend:
> > description:
> > True when DWC3 asserts output signal utmi_l1_suspend_n, false
> > when
> > --
> > 2.34.1
> >