2022-06-13 16:40:19

by Piyush Mehta

[permalink] [raw]
Subject: [PATCH 0/2] usb: dwc3: core: Enable GUCTL1 bit 10 for fixing crc error after resume

This patch of the series does the following:
- Add a new DT "snps,enable_guctl1_resume_quirk" quirk
- Enable GUCTL1 bit 10 for fixing crc error after resume bug
When this bit is set to '1', the ULPI opmode will be changed
to 'normal' along with HS terminations after EOR.
This option is to support certain legacy ULPI PHYs.

Piyush Mehta (2):
dt-bindings: usb: snps,dwc3: Add 'snps,enable_guctl1_resume_quirk'
quirk
usb: dwc3: core: Enable GUCTL1 bit 10 for fixing crc error after
resume bug

.../devicetree/bindings/usb/snps,dwc3.yaml | 6 ++++++
drivers/usb/dwc3/core.c | 16 ++++++++++++++++
drivers/usb/dwc3/core.h | 6 ++++++
3 files changed, 28 insertions(+)

--
2.17.1


2022-06-13 17:07:51

by Piyush Mehta

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: usb: snps,dwc3: Add 'snps,enable_guctl1_resume_quirk' quirk

Add a new DT quirk to dwc3 core to resolved issue of CRC failed error.
On the resume path, U3/U2 exit controller fails to send proper CRC
checksum in CRC5 field. As result Transaction Error is generated.
Enabling bit 10 of GUCTL1 will correct this problem.

When this bit is set to '1', the UTMI/ULPI opmode will be changed to
"normal" along with HS terminations after EOR.
This option is to support certain legacy UTMI/ULPI PHYs.

Signed-off-by: Piyush Mehta <[email protected]>
---
Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index d41265ba8ce2..36fa87df57a9 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -234,6 +234,12 @@ properties:
avoid -EPROTO errors with usbhid on some devices (Hikey 970).
type: boolean

+ snps,enable_guctl1_resume_quirk:
+ description:
+ Set if we enable quirk for fixing improper crc generation after resume
+ from suspend.
+ type: boolean
+
snps,is-utmi-l1-suspend:
description:
True when DWC3 asserts output signal utmi_l1_suspend_n, false when
--
2.17.1

2022-06-16 22:59:05

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: usb: snps,dwc3: Add 'snps,enable_guctl1_resume_quirk' quirk

On 13/06/2022 05:47, Piyush Mehta wrote:
> Add a new DT quirk to dwc3 core to resolved issue of CRC failed error.
> On the resume path, U3/U2 exit controller fails to send proper CRC
> checksum in CRC5 field. As result Transaction Error is generated.
> Enabling bit 10 of GUCTL1 will correct this problem.
>
> When this bit is set to '1', the UTMI/ULPI opmode will be changed to
> "normal" along with HS terminations after EOR.
> This option is to support certain legacy UTMI/ULPI PHYs.
>
> Signed-off-by: Piyush Mehta <[email protected]>
> ---
> Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index d41265ba8ce2..36fa87df57a9 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -234,6 +234,12 @@ properties:
> avoid -EPROTO errors with usbhid on some devices (Hikey 970).
> type: boolean
>
> + snps,enable_guctl1_resume_quirk:

No underscores in properties, use hyphens.

> + description:
> + Set if we enable quirk for fixing improper crc generation after resume
> + from suspend.

Please describe actual issue, hardware property, not driver behavior. In
the description and property name. This could be something like
"snps,missing-src-after-resume" (or anything better).

Best regards,
Krzysztof

2022-07-19 22:22:11

by Michael Grzeschik

[permalink] [raw]
Subject: Re: [PATCH 0/2] usb: dwc3: core: Enable GUCTL1 bit 10 for fixing crc error after resume

Hi Piyush!

On Mon, Jun 13, 2022 at 06:17:01PM +0530, Piyush Mehta wrote:
>This patch of the series does the following:
>- Add a new DT "snps,enable_guctl1_resume_quirk" quirk
>- Enable GUCTL1 bit 10 for fixing crc error after resume bug
> When this bit is set to '1', the ULPI opmode will be changed
> to 'normal' along with HS terminations after EOR.
> This option is to support certain legacy ULPI PHYs.
>
>Piyush Mehta (2):
> dt-bindings: usb: snps,dwc3: Add 'snps,enable_guctl1_resume_quirk'
> quirk
> usb: dwc3: core: Enable GUCTL1 bit 10 for fixing crc error after
> resume bug
>
> .../devicetree/bindings/usb/snps,dwc3.yaml | 6 ++++++
> drivers/usb/dwc3/core.c | 16 ++++++++++++++++
> drivers/usb/dwc3/core.h | 6 ++++++
> 3 files changed, 28 insertions(+)

I found your series and am wondering if you are planning to send a v2 of
it? It would really help to see this mainline.

The Xilinx Register Reference states BIT 10 as

RESUME_TERMSEL_XCVRSEL_UNIFY

which seems to be more meaningful than GUCTL1_RESUME_QUIRK. It would
probably make sense to work this in for v2.

The Documentation is also refering more than just opmode to be 0
during EOR. (termsel, xcvrsel, opmode).

https://www.xilinx.com/htmldocs/registers/ug1087/ug1087-zynq-ultrascale-registers.html#usb3_xhci___guctl1.html

Regards,
Michael

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |


Attachments:
(No filename) (1.69 kB)
signature.asc (849.00 B)
Download all attachments