usb: dwc3: Add a quirk to set GUCTL.SPRSCTRLTRANSEN bit

PATCH In review Yaqin Pan 2 participants 9 messages Dec 30, 2021 – Jan 4, 2022

Subsystems (3)

Device tree bindings USB DesignWare USB3 DRD IP driver

lore

Share & tools

lore

Yaqin Pan

author

2021-12-30 14:04 UTC #0 lore raw

v3 cover usb: dwc3: Add a quirk to set GUCTL.SPRSCTRLTRANSEN bit

Hi,

Sorry for the late reply.

Changes in v3:
modify Documentation/devicetree/bindings/usb/snps,dwc3.yaml because the errors in dt-binding check.

Changes in v2:
modify Documentation/devicetree/bindings/usb/snps,dwc3.yaml

Yaqin Pan (2):
usb: dwc3: Add a quirk to set GUCTL.SPRSCTRLTRANSEN bit.
dt-bindings: usb: document snps,sprs-ctrl-trans-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(+)

Signature · 1 line

2.17.1

Yaqin Pan

author

2021-12-30 14:04 UTC #1 lore raw

PATCH v3 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(+)

drivers/usb/dwc3/core.c +4 -0

index ba74ad7f6995..93ac2c79a2c0 100644
@@ -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;

drivers/usb/dwc3/core.h +3 -0

index 5991766239ba..6048087df1d1 100644
@@ -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;

Signature · 1 line

2.17.1

Greg Kroah-Hartman

maintainer

2021-12-30 14:12 UTC #2 lore raw

Re: [PATCH v3 1/2] usb: dwc3: Add a quirk to set GUCTL.SPRSCTRLTRANSEN bit.

Quoted 17 lines · Yaqin Pan

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
Quoted 1 line
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

Quoted 5 lines

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.

If this is needed for all devices (i.e. you do not know what device is going to be plugged in), why not just enable it for all controllers? Why whould you NOT want this enabled?

Or is this a broken hardware device and only specific host controllers need this? If so, how do we know which ones need this set and which do not?

thanks,

greg k-h

Yaqin Pan

author

2021-12-30 15:36 UTC #3 lore raw

Re: [PATCH v3 1/2] usb: dwc3: Add a quirk to set GUCTL.SPRSCTRLTRANSEN bit.

Quoted 23 lines · On Thu, 30 Dec 2021 15:12:27 +0100 Greg Kroah-Hartman wrote:

Quoted 17 lines
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
Quoted 1 line
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
Quoted 5 lines
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.

Quoted 7 lines

If this is needed for all devices (i.e. you do not know what device is going to be plugged in), why not just enable it for all controllers? Why whould you NOT want this enabled?
Or is this a broken hardware device and only specific host controllers need this? If so, how do we know which ones need this set and which do not?

I think not all dwc3 controllers need this. For cell phone,customers may use various usb devices, we can enable this quirk to fix some compatibility issues. For some chip platform of qcom, i encounter this issue, not every platform i encounter this problem.

If enabled for all controllers, it will reduce the speed of Control transfers. So i think it would be better for user to enable it by their own purposes.

Greg KH

maintainer

2021-12-30 15:48 UTC #4 lore raw

Re: [PATCH v3 1/2] usb: dwc3: Add a quirk to set GUCTL.SPRSCTRLTRANSEN bit.

Quoted 32 lines · Yaqin Pan

Quoted 23 lines · On Thu, 30 Dec 2021 15:12:27 +0100 Greg Kroah-Hartman wrote:
Quoted 17 lines
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
Quoted 1 line
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
Quoted 5 lines
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.
Quoted 7 lines
If this is needed for all devices (i.e. you do not know what device is going to be plugged in), why not just enable it for all controllers? Why whould you NOT want this enabled?
Or is this a broken hardware device and only specific host controllers need this? If so, how do we know which ones need this set and which do not?

Quoted 7 lines

I think not all dwc3 controllers need this. For cell phone,customers may use various usb devices, we can enable this quirk to fix some compatibility issues. For some chip platform of qcom, i encounter this issue, not every platform i encounter this problem.
If enabled for all controllers, it will reduce the speed of Control transfers. So i think it would be better for user to enable it by their own purposes.

But how do hardware vendors know to enable this? Can we trigger off of PCI ids? Do we need a list of quirks to show which host controllers are broken this way?

Burying something as basic as "reliable device connection" in a DT quirk seems very sloppy to me. We want reliable systems, right?

thanks,

greg k-h

Yaqin Pan

author

2021-12-31 11:59 UTC #5 lore raw

Re: [PATCH v3 1/2] usb: dwc3: Add a quirk to set GUCTL.SPRSCTRLTRANSEN bit.

Quoted 41 lines · On Thu, 30 Dec 2021 16:48:09 +0100 Greg Kroah-Hartman wrote:

Quoted 32 lines · Yaqin Pan
Quoted 23 lines · On Thu, 30 Dec 2021 15:12:27 +0100 Greg Kroah-Hartman wrote:
Quoted 17 lines
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
Quoted 1 line
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
Quoted 5 lines
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.
Quoted 7 lines
If this is needed for all devices (i.e. you do not know what device is going to be plugged in), why not just enable it for all controllers? Why whould you NOT want this enabled?
Or is this a broken hardware device and only specific host controllers need this? If so, how do we know which ones need this set and which do not?
Quoted 7 lines
I think not all dwc3 controllers need this. For cell phone,customers may use various usb devices, we can enable this quirk to fix some compatibility issues. For some chip platform of qcom, i encounter this issue, not every platform i encounter this problem.
If enabled for all controllers, it will reduce the speed of Control transfers. So i think it would be better for user to enable it by their own purposes.

Quoted 6 lines

But how do hardware vendors know to enable this? Can we trigger off of PCI ids? Do we need a list of quirks to show which host controllers are broken this way?
Burying something as basic as "reliable device connection" in a DT quirk seems very sloppy to me. We want reliable systems, right?

Yes, we want reliable systems. But i don't have a good ideal about this issue. when we meet this problem, and from the dwc-usb3 controller datasheet,we know enable one bit in dwc-usb3 controller's register can fixed this issue.

Of course, i can list the host controllers that i used broken this way if needed.

thanks,

Yaqin pan

Greg KH

maintainer

2022-01-03 13:35 UTC #6 lore raw

Re: [PATCH v3 1/2] usb: dwc3: Add a quirk to set GUCTL.SPRSCTRLTRANSEN bit.

Quoted 46 lines · Yaqin Pan

Quoted 45 lines · On Thu, 30 Dec 2021 16:48:09 +0100 Greg Kroah-Hartman wrote:
Quoted 32 lines · Yaqin Pan
Quoted 23 lines · On Thu, 30 Dec 2021 15:12:27 +0100 Greg Kroah-Hartman wrote:
Quoted 17 lines
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
Quoted 1 line
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
Quoted 5 lines
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.
Quoted 7 lines
If this is needed for all devices (i.e. you do not know what device is going to be plugged in), why not just enable it for all controllers? Why whould you NOT want this enabled?
Or is this a broken hardware device and only specific host controllers need this? If so, how do we know which ones need this set and which do not?
Quoted 7 lines
I think not all dwc3 controllers need this. For cell phone,customers may use various usb devices, we can enable this quirk to fix some compatibility issues. For some chip platform of qcom, i encounter this issue, not every platform i encounter this problem.
If enabled for all controllers, it will reduce the speed of Control transfers. So i think it would be better for user to enable it by their own purposes.
But how do hardware vendors know to enable this? Can we trigger off of PCI ids? Do we need a list of quirks to show which host controllers are broken this way?

Quoted 8 lines

Quoted 2 lines
Burying something as basic as "reliable device connection" in a DT quirk seems very sloppy to me. We want reliable systems, right?
Yes, we want reliable systems. But i don't have a good ideal about this issue. when we meet this problem, and from the dwc-usb3 controller datasheet,we know enable one bit in dwc-usb3 controller's register can fixed this issue.
Of course, i can list the host controllers that i used broken this way if needed.

Please have a list of controller that this is needed for, and add the quirk for them only. Don't require this to be in a DT file as that will never be noticed.

thanks,

greg k-h

Yaqin Pan

author

2022-01-04 14:57 UTC #7 lore raw

Re: [PATCH v3 1/2] usb: dwc3: Add a quirk to set GUCTL.SPRSCTRLTRANSEN bit.

Quoted 54 lines · On Thu, 2022-01-03 13:35 UTC Greg Kroah-Hartman wrote:

Quoted 53 lines · Yaqin Pan
Quoted 41 lines · On Thu, 30 Dec 2021 16:48:09 +0100 Greg Kroah-Hartman wrote:
Quoted 32 lines · Yaqin Pan
Quoted 23 lines · On Thu, 30 Dec 2021 15:12:27 +0100 Greg Kroah-Hartman wrote:
Quoted 17 lines
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
Quoted 1 line
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
Quoted 5 lines
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.
Quoted 7 lines
If this is needed for all devices (i.e. you do not know what device is going to be plugged in), why not just enable it for all controllers? Why whould you NOT want this enabled?
Or is this a broken hardware device and only specific host controllers need this? If so, how do we know which ones need this set and which do not?
Quoted 7 lines
I think not all dwc3 controllers need this. For cell phone,customers may use various usb devices, we can enable this quirk to fix some compatibility issues. For some chip platform of qcom, i encounter this issue, not every platform i encounter this problem.
If enabled for all controllers, it will reduce the speed of Control transfers. So i think it would be better for user to enable it by their own purposes.
Quoted 6 lines
But how do hardware vendors know to enable this? Can we trigger off of PCI ids? Do we need a list of quirks to show which host controllers are broken this way?
Burying something as basic as "reliable device connection" in a DT quirk seems very sloppy to me. We want reliable systems, right?
Yes, we want reliable systems. But i don't have a good ideal about this issue. when we meet this problem, and from the dwc-usb3 controller datasheet,we know enable one bit in dwc-usb3 controller's register can fixed this issue.

Quoted 5 lines

Quoted 1 line
Of course, i can list the host controllers that i used broken this way if needed.
Please have a list of controller that this is needed for, and add the quirk for them only. Don't require this to be in a DT file as that will never be noticed.

The dwc3-core i list below:
qcom,sm8350-dwc3;
qcom,sm7325-dwc3;
qcom,sm6225-dwc3;
....
And i will try to contact with qcom for further help.

thanks,

Yaqin pan

Yaqin Pan

author

2021-12-30 14:04 UTC #8 lore raw

PATCH v3 2/2 dt-bindings: usb: document snps,sprs-ctrl-trans-quirk property in dwc3

In reply to Yaqin Pan

Add snps,sprs-ctrl-trans-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(+)

Documentation/devicetree/bindings/usb/snps,dwc3.yaml +6 -0

index 41416fbd92aa..7a127f0cb530 100644
@@ -226,6 +226,12 @@ properties:
       avoid -EPROTO errors with usbhid on some devices (Hikey 970).
     type: boolean
 
+  snps,sprs-ctrl-trans-quirk:
+    description:
+      When set, change the way host controller schedules transations for a Control transfer.
+      Avoid failing to enumerate some devices due to usb compatibility issues.
+    type: boolean
+
   snps,is-utmi-l1-suspend:
     description:
       True when DWC3 asserts output signal utmi_l1_suspend_n, false when

Signature · 1 line

2.17.1