2021-12-15 13:03:43

by Yaqin Pan

[permalink] [raw]
Subject: [PATCH 0/2] usb: dwc3: Add a quirk to set GUCTL.SPRSCTRLTRANSEN bit

Add a quirk to set dwc3 controller GUCTL.SPRSCTRLTRANSEN bit.

Yaqin Pan (2):
usb: dwc3: Add a quirk to set GUCTL.SPRSCTRLTRANSEN bit.
dt-bindings: usb: document snps,dis_split_quirk property in dwc3

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

--
2.17.1



2021-12-15 13:03:48

by Yaqin Pan

[permalink] [raw]
Subject: [PATCH 1/2] usb: dwc3: Add a quirk to set GUCTL.SPRSCTRLTRANSEN bit.

This quirk is only for dwc3 host mode.
the dwc3 controller can't emurate some devices successfully.
For example, TF card reader (aaaa:8816):
failed log
usb 1-1: new high-speed USB device number 2 using xhci-hcd
usb 1-1: device descriptor read/all, error -110
From the usb analyzer, always return NAK in the data phase.
if enable the GUCTL.SPRSCTRLTRANSEN bit. then the log is:
usb 2-1: new high-speed USB device number 3 using xhci-hcd
usb 2-1: New USB device found, idVendor=aaaa,
idProduct=8816, bcdDevice=13.08
usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 2-1: Product: MXT USB Device
usb 2-1: Manufacturer: MXTronics
usb 2-1: SerialNumber: 150101v01
usb 2-1: New USB device found, VID=aaaa, PID=8816

Some devices are slow in responding to Control transfers.
Scheduling mulitiple transactions in one microframe/frame
can cause the devices to misbehave. if this qurik is enabled,
the host controller schedules transations for a Control transfer
in defferent microframes/frame.

Signed-off-by: Yaqin Pan <[email protected]>
---
drivers/usb/dwc3/core.c | 4 ++++
drivers/usb/dwc3/core.h | 3 +++
2 files changed, 7 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index ba74ad7f6995..93ac2c79a2c0 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1071,6 +1071,8 @@ static int dwc3_core_init(struct dwc3 *dwc)
* packet with Retry=1 & Nump != 0)
*/
reg |= DWC3_GUCTL_HSTINAUTORETRY;
+ if (dwc->sprs_ctrl_trans_quirk)
+ reg |= DWC3_GUCTL_SPRSCTRLTRANSEN;

dwc3_writel(dwc->regs, DWC3_GUCTL, reg);
}
@@ -1377,6 +1379,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)

dwc->dis_split_quirk = device_property_read_bool(dev,
"snps,dis-split-quirk");
+ dwc->sprs_ctrl_trans_quirk = device_property_read_bool(dev,
+ "snps,sprs-ctrl-trans-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 5991766239ba..6048087df1d1 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -254,6 +254,7 @@

/* Global User Control Register */
#define DWC3_GUCTL_HSTINAUTORETRY BIT(14)
+#define DWC3_GUCTL_SPRSCTRLTRANSEN BIT(17)

/* Global User Control 1 Register */
#define DWC3_GUCTL1_PARKMODE_DISABLE_SS BIT(17)
@@ -1077,6 +1078,7 @@ struct dwc3_scratchpad_array {
* 3 - Reserved
* @dis_metastability_quirk: set to disable metastability quirk.
* @dis_split_quirk: set to disable split boundary.
+ * @sprs_ctrl_trans_quirk: set to enable sparse control transaction quirk.
* @imod_interval: set the interrupt moderation interval in 250ns
* increments or 0 to disable.
*/
@@ -1279,6 +1281,7 @@ struct dwc3 {
unsigned dis_metastability_quirk:1;

unsigned dis_split_quirk:1;
+ unsigned sprs_ctrl_trans_quirk:1;
unsigned async_callbacks:1;

u16 imod_interval;
--
2.17.1


2021-12-15 13:03:53

by Yaqin Pan

[permalink] [raw]
Subject: [PATCH 2/2] dt-bindings: usb: document snps,dis_split_quirk property in dwc3

Add snps,dis_split_quirk property for dwc3 controller

Signed-off-by: Yaqin Pan <[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 41416fbd92aa..e9615ca8f447 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -226,6 +226,12 @@ properties:
avoid -EPROTO errors with usbhid on some devices (Hikey 970).
type: boolean

+ snps,dis_split_quirk:
+ description:
+ When set, change the way host controller schedules transations for a Control transfer.
+ Needed to avoid emurate some devices fail.
+ type: boolean
+
snps,is-utmi-l1-suspend:
description:
True when DWC3 asserts output signal utmi_l1_suspend_n, false when
--
2.17.1


2021-12-16 08:26:50

by Sergey Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: usb: document snps,dis_split_quirk property in dwc3

On 15.12.2021 16:03, Yaqin Pan wrote:

> Add snps,dis_split_quirk property for dwc3 controller
>
> Signed-off-by: Yaqin Pan <[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 41416fbd92aa..e9615ca8f447 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -226,6 +226,12 @@ properties:
> avoid -EPROTO errors with usbhid on some devices (Hikey 970).
> type: boolean
>
> + snps,dis_split_quirk:
> + description:
> + When set, change the way host controller schedules transations for a Control transfer.

Transactions.

> + Needed to avoid emurate some devices fail.

Avoid failing to enumerating some devices?

[...]

MBR, Sergey

2021-12-16 15:59:40

by Sergey Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: usb: document snps,dis_split_quirk property in dwc3

On 12/16/21 11:26 AM, Sergey Shtylyov wrote:

>> Add snps,dis_split_quirk property for dwc3 controller
>>
>> Signed-off-by: Yaqin Pan <[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 41416fbd92aa..e9615ca8f447 100644
>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> @@ -226,6 +226,12 @@ properties:
>>         avoid -EPROTO errors with usbhid on some devices (Hikey 970).
>>       type: boolean
>>   +  snps,dis_split_quirk:
>> +    description:
>> +      When set, change the way host controller schedules transations for a Control transfer.
>
>    Transactions.
>
>> +      Needed to avoid emurate some devices fail.
>
>    Avoid failing to enumerating some devices?

Sorry. enumarate. :-)

[...]

MBR, Sergey

2021-12-16 20:19:06

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: usb: document snps,dis_split_quirk property in dwc3

On Thu, Dec 16, 2021 at 06:59:27PM +0300, Sergey Shtylyov wrote:
> On 12/16/21 11:26 AM, Sergey Shtylyov wrote:
>
> >> Add snps,dis_split_quirk property for dwc3 controller
> >>
> >> Signed-off-by: Yaqin Pan <[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 41416fbd92aa..e9615ca8f447 100644
> >> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >> @@ -226,6 +226,12 @@ properties:
> >> ??????? avoid -EPROTO errors with usbhid on some devices (Hikey 970).
> >> ????? type: boolean
> >> ? +? snps,dis_split_quirk:

I'm tired of the never ending DWC3 quirks. Imply this from the
compatible unless it varies by board.

Also, don't use '_' in DT names.

> >> +??? description:
> >> +????? When set, change the way host controller schedules transations for a Control transfer.
> >
> > ?? Transactions.
> >
> >> +????? Needed to avoid emurate some devices fail.
> >
> > ?? Avoid failing to enumerating some devices?
>
> Sorry. enumarate. :-)

Or enumerate


2021-12-17 14:46:03

by Yaqin Pan

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: usb: document snps,dis_split_quirk property in dwc3

>> >> Add snps,dis_split_quirk property for dwc3 controller
>> >>
>> >> Signed-off-by: Yaqin Pan <[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 41416fbd92aa..e9615ca8f447 100644
>> >> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> >> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> >> @@ -226,6 +226,12 @@ properties:
>> >> avoid -EPROTO errors with usbhid on some devices (Hikey 970).
>> >> type: boolean
>> >> + snps,dis_split_quirk:
>
>I'm tired of the never ending DWC3 quirks. Imply this from the
>compatible unless it varies by board.

Yes, this is a DWC3 controller compatible issue.

>
>Also, don't use '_' in DT names.

Sure, i will change "snps,dis_split_quirk" to "snps,dis-split-quirk".

>
>> >> + description:
>> >> + When set, change the way host controller schedules transations for a Control transfer.
>> >
>> > Transactions.
>> >
>> >> + Needed to avoid emurate some devices fail.
>> >
>> > Avoid failing to enumerating some devices?

Yes. Avoid failing to enumerate some devices.

>>
>> Sorry. enumarate. :-)

> Or enumerate

2021-12-17 14:47:27

by Yaqin Pan

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: usb: document snps,dis_split_quirk property in dwc3

>> >> Add snps,dis_split_quirk property for dwc3 controller
>> >>
>> >> Signed-off-by: Yaqin Pan <[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 41416fbd92aa..e9615ca8f447 100644
>> >> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> >> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> >> @@ -226,6 +226,12 @@ properties:
>> >> avoid -EPROTO errors with usbhid on some devices (Hikey 970).
>> >> type: boolean
>> >> + snps,dis_split_quirk:
>
>I'm tired of the never ending DWC3 quirks. Imply this from the
>compatible unless it varies by board.

Yes, this is a DWC3 controller compatible issue.

>
>Also, don't use '_' in DT names.

Sure, i will change "snps,dis_split_quirk" to "snps,dis-split-quirk".

>
>> >> + description:
>> >> + When set, change the way host controller schedules transations for a Control transfer.
>> >
>> > Transactions.
>> >
>> >> + Needed to avoid emurate some devices fail.
>> >
>> > Avoid failing to enumerating some devices?

Yes. Avoid failing to enumerate some devices.

>>
>> Sorry. enumarate. :-)

> Or enumerate

2022-01-05 16:48:02

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: usb: document snps,dis_split_quirk property in dwc3

On Fri, Dec 17, 2021 at 8:46 AM Yaqin Pan <[email protected]> wrote:
>
> >> >> Add snps,dis_split_quirk property for dwc3 controller
> >> >>
> >> >> Signed-off-by: Yaqin Pan <[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 41416fbd92aa..e9615ca8f447 100644
> >> >> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >> >> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >> >> @@ -226,6 +226,12 @@ properties:
> >> >> avoid -EPROTO errors with usbhid on some devices (Hikey 970).
> >> >> type: boolean
> >> >> + snps,dis_split_quirk:
> >
> >I'm tired of the never ending DWC3 quirks. Imply this from the
> >compatible unless it varies by board.
>
> Yes, this is a DWC3 controller compatible issue.

Again, imply this from the compatible STRING (or the compatible string
of the parent node). I don't really want to see more quirk properties
unless they are board or instance specific (meaning for a given SoC,
the setting varies).

What is the downside to always setting this bit in the controller even
if not needed? Answer that question in your commit message.

Rob