usb: Support USB 3.2 multi-lanes

PATCH Superseded Thinh Nguyen 4 participants 28 messages Jan 14, 2021 – Jan 20, 2021

Subsystems (3)

Device tree bindings USB DesignWare USB3 DRD IP driver

lore

Share & tools

lore

Thinh Nguyen

author maintainer

2021-01-14 02:53 UTC #0 lore raw

v6 cover usb: Support USB 3.2 multi-lanes

A USB SuperSpeed Plus device may operate at different speed and lane count (i.e. gen2x2, gen1x2, or gen2x1). The DWC_usb32 IP supports SuperSpeed Plus gen2x2. To support this, this series update a few things to the USB gadget stack and dwc3 driver:

* Accept and parse new maximum_speed devicetree property strings
* Introduce enum usb_ssp_rate to describe the speed in SuperSpeed Plus genXxY
* Capture the connected and max supported usb_ssp_rate
* Report the device sublink speeds base on the usb_ssp_rate in the BOS descriptor
* Introduce gadget ops to select SuperSpeed Plus various transfer rate and lane count
* Update dwc3 driver to support the above changes

Changes in v6:
- Rebase on Greg's usb-testing branch
- Update the cover letter and title
* Previous version 5: https://lore.kernel.org/linux-usb/[email protected]/
- To simplify things, use usb_ssp_rate enum to specify the signaling rate
generation and lane count instead of separately tracking them.
- Convert the sublink speed attributes to macros and move it to uapi
- Remove usb_sublink_speed struct
- Remove "usb: dwc3: gadget: Report sublink speed capability"
- Update dwc3 to support the new changes

Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header
- Add Rob's Reviewed-by

Changes in v4:
- Instead of using a single function to parse "maximum-speed" property for speed, gen X, and number of lanes, split those tasks to separate common functions
- Revise DWC3 driver to use those new common functions
- Fix checkpatch warnings for using "unsigned" rather than "unsigned int" and missing identifier name in udc_set_num_lanes_and_speed gadget ops

Changes in v3:
- Remove "num-lanes" and "lane-speed-mantissa-gbps" common properties
- Remove "num-lanes" and "lane-speed-mantissa-gbps" properties validation in dwc3
- Update "maximum-speed" to support variations of SSP Gen X x Y
- Update common function to parse new strings for "maximum-speed"
- Update commit messages for the new changes

Changes in v2:
- Move usb_sublink_speed attribute struct and enum to include/linux/usb/ch9.h
- Use "num-lanes" and "lane-speed-mantissa-gbps" as common properties instead
- Add common functions to get num-lanes and lsm properties
- Fix missing gen1x2 sublink speed attribute check report in dwc3

Thinh Nguyen (11):
usb: ch9: Add USB 3.2 SSP attributes
usb: gadget: composite: Use SSP sublink speed macros
usb: gadget: Introduce SSP rates and lanes
usb: gadget: Introduce udc_set_ssp_rate() for SSP
usb: gadget: composite: Report various SSP sublink speeds
dt-binding: usb: Include USB SSP rates in GenXxY
usb: common: Parse for USB SSP genXxY
usb: dwc3: core: Check maximum_speed SSP genXxY
usb: dwc3: gadget: Implement setting of SSP rate
usb: dwc3: gadget: Track connected SSP rate and lane count
usb: dwc3: gadget: Set speed only up to the max supported

 .../devicetree/bindings/usb/usb.yaml          |  3 +
 drivers/usb/common/common.c                   | 26 +++++-
 drivers/usb/dwc3/core.c                       | 37 +++++++++
 drivers/usb/dwc3/core.h                       |  9 ++
 drivers/usb/dwc3/gadget.c                     | 80 +++++++++++++++++-
 drivers/usb/gadget/composite.c                | 83 +++++++++++++------
 drivers/usb/gadget/udc/core.c                 | 16 ++--
 include/linux/usb/ch9.h                       | 20 +++++
 include/linux/usb/gadget.h                    | 11 +++
 include/uapi/linux/usb/ch9.h                  | 13 +++
 10 files changed, 263 insertions(+), 35 deletions(-)

base-commit: 67004e130aafad4c9e0ad3fff9cf67227b6347be

Signature · 1 line

2.28.0

Thinh Nguyen

author maintainer

2021-01-14 02:53 UTC #1 lore raw

PATCH v6 1/11 usb: ch9: Add USB 3.2 SSP attributes

In preparation for USB 3.2 dual-lane support, add sublink speed attribute macros and enum usb_ssp_rate. A USB device that operates in SuperSpeed Plus may operate at different speed and lane count. These additional macros and enum values help specifying that.

Signed-off-by Thinh Nguyen <[email protected]>

---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Convert the sublink speed attribute enum to macros and move it to uapi
- Remove usb_sublink_speed struct
- To simplify things, use usb_ssp_rate enum to specify the signaling rate generation and lane count
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- None
Changes in v3:
- None
Changes in v2:
- Move to include/linux/usb/ch9.h instead of under uapi

 include/linux/usb/ch9.h      |  9 +++++++++
 include/uapi/linux/usb/ch9.h | 13 +++++++++++++
 2 files changed, 22 insertions(+)

include/linux/usb/ch9.h +9 -0

index 604c6c514a50..86c50907634e 100644
@@ -36,6 +36,15 @@
 #include <linux/device.h>
 #include <uapi/linux/usb/ch9.h>
 
+/* USB 3.2 SuperSpeed Plus phy signaling rate generation and lane count */
+
+enum usb_ssp_rate {
+	USB_SSP_GEN_UNKNOWN = 0,
+	USB_SSP_GEN_2x1,
+	USB_SSP_GEN_1x2,
+	USB_SSP_GEN_2x2,
+};
+
 /**
  * usb_ep_type_string() - Returns human readable-name of the endpoint type.
  * @ep_type: The endpoint type to return human-readable name for.  If it's not

include/uapi/linux/usb/ch9.h +13 -0

index 0f865ae4ba89..17ce56198c9a 100644
@@ -968,9 +968,22 @@ struct usb_ssp_cap_descriptor {
 	__le32 bmSublinkSpeedAttr[1]; /* list of sublink speed attrib entries */
 #define USB_SSP_SUBLINK_SPEED_SSID	(0xf)		/* sublink speed ID */
 #define USB_SSP_SUBLINK_SPEED_LSE	(0x3 << 4)	/* Lanespeed exponent */
+#define USB_SSP_SUBLINK_SPEED_LSE_BPS		0
+#define USB_SSP_SUBLINK_SPEED_LSE_KBPS		1
+#define USB_SSP_SUBLINK_SPEED_LSE_MBPS		2
+#define USB_SSP_SUBLINK_SPEED_LSE_GBPS		3
+
 #define USB_SSP_SUBLINK_SPEED_ST	(0x3 << 6)	/* Sublink type */
+#define USB_SSP_SUBLINK_SPEED_ST_SYM_RX		0
+#define USB_SSP_SUBLINK_SPEED_ST_ASYM_RX	1
+#define USB_SSP_SUBLINK_SPEED_ST_SYM_TX		2
+#define USB_SSP_SUBLINK_SPEED_ST_ASYM_TX	3
+
 #define USB_SSP_SUBLINK_SPEED_RSVD	(0x3f << 8)	/* Reserved */
 #define USB_SSP_SUBLINK_SPEED_LP	(0x3 << 14)	/* Link protocol */
+#define USB_SSP_SUBLINK_SPEED_LP_SS		0
+#define USB_SSP_SUBLINK_SPEED_LP_SSP		1
+
 #define USB_SSP_SUBLINK_SPEED_LSM	(0xff << 16)	/* Lanespeed mantissa */
 } __attribute__((packed));

Signature · 1 line

2.28.0

Felipe Balbi

2021-01-14 10:00 UTC #2 lore raw

Re: [PATCH v6 01/11] usb: ch9: Add USB 3.2 SSP attributes

Quoted 34 lines · Thinh Nguyen

In preparation for USB 3.2 dual-lane support, add sublink speed attribute macros and enum usb_ssp_rate. A USB device that operates in SuperSpeed Plus may operate at different speed and lane count. These additional macros and enum values help specifying that.
Signed-off-by Thinh Nguyen <[email protected]>
---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Convert the sublink speed attribute enum to macros and move it to uapi
- Remove usb_sublink_speed struct
- To simplify things, use usb_ssp_rate enum to specify the signaling rate generation and lane count
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- None
Changes in v3:
- None
Changes in v2:
- Move to include/linux/usb/ch9.h instead of under uapi
 include/linux/usb/ch9.h      |  9 +++++++++
 include/uapi/linux/usb/ch9.h | 13 +++++++++++++
 2 files changed, 22 insertions(+)
include/linux/usb/ch9.h +0 -0
index 604c6c514a50..86c50907634e 100644
@@ -36,6 +36,15 @@
 #include <linux/device.h>

include/linux/usb/ch9.h line 46

Quoted 10 lines

include/linux/usb/ch9.h +8 -0

 #include <uapi/linux/usb/ch9.h>
 
+/* USB 3.2 SuperSpeed Plus phy signaling rate generation and lane count */
+
+enum usb_ssp_rate {
+	USB_SSP_GEN_UNKNOWN = 0,
+	USB_SSP_GEN_2x1,
+	USB_SSP_GEN_1x2,
+	USB_SSP_GEN_2x2,
+};

note that xHCI has some private definitions for USB 3.2 support. Maybe add a patch converting xHCI to the generic versions?

Signature · 1 line

balbi

Thinh Nguyen

author maintainer

2021-01-14 18:59 UTC #3 lore raw

Re: [PATCH v6 01/11] usb: ch9: Add USB 3.2 SSP attributes

Quoted 38 lines · Felipe Balbi

Quoted 36 lines · Thinh Nguyen
In preparation for USB 3.2 dual-lane support, add sublink speed attribute macros and enum usb_ssp_rate. A USB device that operates in SuperSpeed Plus may operate at different speed and lane count. These additional macros and enum values help specifying that.
Signed-off-by Thinh Nguyen <[email protected]>
---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Convert the sublink speed attribute enum to macros and move it to uapi
- Remove usb_sublink_speed struct
- To simplify things, use usb_ssp_rate enum to specify the signaling rate generation and lane count
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- None
Changes in v3:
- None
Changes in v2:
- Move to include/linux/usb/ch9.h instead of under uapi
 include/linux/usb/ch9.h      |  9 +++++++++
 include/uapi/linux/usb/ch9.h | 13 +++++++++++++
 2 files changed, 22 insertions(+)
include/linux/usb/ch9.h +0 -0
index 604c6c514a50..86c50907634e 100644
@@ -36,6 +36,15 @@
 #include <linux/device.h>
 #include <uapi/linux/usb/ch9.h>
 

Quoted 11 lines

include/linux/usb/ch9.h line 46
Quoted 8 lines
include/linux/usb/ch9.h +8 -0
+/* USB 3.2 SuperSpeed Plus phy signaling rate generation and lane count */
+
+enum usb_ssp_rate {
+	USB_SSP_GEN_UNKNOWN = 0,
+	USB_SSP_GEN_2x1,
+	USB_SSP_GEN_1x2,
+	USB_SSP_GEN_2x2,
+};
note that xHCI has some private definitions for USB 3.2 support. Maybe add a patch converting xHCI to the generic versions?

Should it be part of this series? I plan to do that after this series is merged to help minimize the review effort.

Thanks,
Thinh

Felipe Balbi

2021-01-15 11:09 UTC #4 lore raw

Re: [PATCH v6 01/11] usb: ch9: Add USB 3.2 SSP attributes

Hi,

Quoted 51 lines · Thinh Nguyen

Quoted 38 lines · Felipe Balbi
Quoted 36 lines · Thinh Nguyen
In preparation for USB 3.2 dual-lane support, add sublink speed attribute macros and enum usb_ssp_rate. A USB device that operates in SuperSpeed Plus may operate at different speed and lane count. These additional macros and enum values help specifying that.
Signed-off-by Thinh Nguyen <[email protected]>
---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Convert the sublink speed attribute enum to macros and move it to uapi
- Remove usb_sublink_speed struct
- To simplify things, use usb_ssp_rate enum to specify the signaling rate generation and lane count
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- None
Changes in v3:
- None
Changes in v2:
- Move to include/linux/usb/ch9.h instead of under uapi
 include/linux/usb/ch9.h      |  9 +++++++++
 include/uapi/linux/usb/ch9.h | 13 +++++++++++++
 2 files changed, 22 insertions(+)
include/linux/usb/ch9.h +0 -0
index 604c6c514a50..86c50907634e 100644
@@ -36,6 +36,15 @@
 #include <linux/device.h>
 #include <uapi/linux/usb/ch9.h>
 
Quoted 11 lines
include/linux/usb/ch9.h line 46
Quoted 8 lines
include/linux/usb/ch9.h +8 -0
+/* USB 3.2 SuperSpeed Plus phy signaling rate generation and lane count */
+
+enum usb_ssp_rate {
+	USB_SSP_GEN_UNKNOWN = 0,
+	USB_SSP_GEN_2x1,
+	USB_SSP_GEN_1x2,
+	USB_SSP_GEN_2x2,
+};
note that xHCI has some private definitions for USB 3.2 support. Maybe add a patch converting xHCI to the generic versions?

Quoted 2 lines

Should it be part of this series? I plan to do that after this series is merged to help minimize the review effort.

As long as it's part of your TODO list, should be good :-)

Signature · 1 line

balbi

Thinh Nguyen

author maintainer

2021-01-16 04:04 UTC #5 lore raw

Re: [PATCH v6 01/11] usb: ch9: Add USB 3.2 SSP attributes

Quoted 53 lines · Felipe Balbi

Hi,
Quoted 50 lines · Thinh Nguyen
Quoted 49 lines · Felipe Balbi
Quoted 34 lines · Thinh Nguyen
In preparation for USB 3.2 dual-lane support, add sublink speed attribute macros and enum usb_ssp_rate. A USB device that operates in SuperSpeed Plus may operate at different speed and lane count. These additional macros and enum values help specifying that.
Signed-off-by Thinh Nguyen <[email protected]>
---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Convert the sublink speed attribute enum to macros and move it to uapi
- Remove usb_sublink_speed struct
- To simplify things, use usb_ssp_rate enum to specify the signaling rate generation and lane count
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- None
Changes in v3:
- None
Changes in v2:
- Move to include/linux/usb/ch9.h instead of under uapi
 include/linux/usb/ch9.h      |  9 +++++++++
 include/uapi/linux/usb/ch9.h | 13 +++++++++++++
 2 files changed, 22 insertions(+)
include/linux/usb/ch9.h +0 -0
index 604c6c514a50..86c50907634e 100644
@@ -36,6 +36,15 @@
 #include <linux/device.h>
include/linux/usb/ch9.h line 46
Quoted 10 lines
include/linux/usb/ch9.h +8 -0
 #include <uapi/linux/usb/ch9.h>
 
+/* USB 3.2 SuperSpeed Plus phy signaling rate generation and lane count */
+
+enum usb_ssp_rate {
+	USB_SSP_GEN_UNKNOWN = 0,
+	USB_SSP_GEN_2x1,
+	USB_SSP_GEN_1x2,
+	USB_SSP_GEN_2x2,
+};
note that xHCI has some private definitions for USB 3.2 support. Maybe add a patch converting xHCI to the generic versions?

Quoted 4 lines

Quoted 2 lines
Should it be part of this series? I plan to do that after this series is merged to help minimize the review effort.
As long as it's part of your TODO list, should be good :-)

Yeah, it's on my list. Currently Linux xHCI driver doesn't really check for USB 3.2. It's missing the root hub sublink speed capability descriptors for gen1x2 and gen2x2. So it's missing some xHCI defined default port speed ID as SSID for gen1x2 and gen2x2. The Linux xHCI right now may think that the connected dual-lane device may not be a SSP device because it doesn't match the SSID from the extended port status.

There are more patches for dwc3 and some for xHCI on queue that hopefully get cleaned up and pushed out eventually.

BR,
Thinh

Felipe Balbi

2021-01-18 11:33 UTC #6 lore raw

Re: [PATCH v6 01/11] usb: ch9: Add USB 3.2 SSP attributes

Hi,

Quoted 59 lines · Thinh Nguyen

Quoted 47 lines
Quoted 47 lines
Quoted 47 lines
Quoted 34 lines
In preparation for USB 3.2 dual-lane support, add sublink speed attribute macros and enum usb_ssp_rate. A USB device that operates in SuperSpeed Plus may operate at different speed and lane count. These additional macros and enum values help specifying that.
Signed-off-by Thinh Nguyen <[email protected]>
---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Convert the sublink speed attribute enum to macros and move it to uapi
- Remove usb_sublink_speed struct
- To simplify things, use usb_ssp_rate enum to specify the signaling rate generation and lane count
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- None
Changes in v3:
- None
Changes in v2:
- Move to include/linux/usb/ch9.h instead of under uapi
 include/linux/usb/ch9.h      |  9 +++++++++
 include/uapi/linux/usb/ch9.h | 13 +++++++++++++
 2 files changed, 22 insertions(+)
include/linux/usb/ch9.h +0 -0
index 604c6c514a50..86c50907634e 100644
@@ -36,6 +36,15 @@
 #include <linux/device.h>
include/linux/usb/ch9.h line 46
Quoted 10 lines
include/linux/usb/ch9.h +8 -0
 #include <uapi/linux/usb/ch9.h>
 
+/* USB 3.2 SuperSpeed Plus phy signaling rate generation and lane count */
+
+enum usb_ssp_rate {
+	USB_SSP_GEN_UNKNOWN = 0,
+	USB_SSP_GEN_2x1,
+	USB_SSP_GEN_1x2,
+	USB_SSP_GEN_2x2,
+};
note that xHCI has some private definitions for USB 3.2 support. Maybe add a patch converting xHCI to the generic versions?
Quoted 4 lines
Quoted 2 lines
Should it be part of this series? I plan to do that after this series is merged to help minimize the review effort.
As long as it's part of your TODO list, should be good :-)
Yeah, it's on my list. Currently Linux xHCI driver doesn't really check for USB 3.2. It's missing the root hub sublink speed capability descriptors for gen1x2 and gen2x2. So it's missing some xHCI defined default port speed ID as SSID for gen1x2 and gen2x2. The Linux xHCI right now may think that the connected dual-lane device may not be a SSP device because it doesn't match the SSID from the extended port status.

Quoted 2 lines

There are more patches for dwc3 and some for xHCI on queue that hopefully get cleaned up and pushed out eventually.

Sounds great, Thinh. Thanks for all the effort you've been putting here :-)

Signature · 1 line

balbi

Thinh Nguyen

author maintainer

2021-01-14 02:53 UTC #7 lore raw

PATCH v6 2/11 usb: gadget: composite: Use SSP sublink speed macros

In reply to Thinh Nguyen

Use SuperSpeed Plus sublink speed macros to fill the BOS descriptor sublink speed attributes in the composite driver. They're self-documented so we can remove some of the comments, and this helps with readability by removing the magic numbers.

Signed-off-by Thinh Nguyen <[email protected]>

---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Use the updated macros
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- None
Changes in v3:
- None
Changes in v2:
- None

 drivers/usb/gadget/composite.c | 45 ++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 21 deletions(-)

drivers/usb/gadget/composite.c +24 ~~-21~~

index 9a7cc08d6593..bc17302a9e85 100644
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/utsname.h>
+#include <linux/bitfield.h>
 
 #include <linux/usb/composite.h>
 #include <linux/usb/otg.h>

@@ -749,32 +750,34 @@ static int bos_desc(struct usb_composite_dev *cdev)
 		ssp_cap->bReserved = 0;
 		ssp_cap->wReserved = 0;
 
-		/* SSAC = 1 (2 attributes) */
-		ssp_cap->bmAttributes = cpu_to_le32(1);
+		ssp_cap->bmAttributes =
+			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_ATTRIBS, 1) |
+				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_IDS, 0));
 
-		/* Min RX/TX Lane Count = 1 */
 		ssp_cap->wFunctionalitySupport =
-			cpu_to_le16((1 << 8) | (1 << 12));
+			cpu_to_le16(FIELD_PREP(USB_SSP_MIN_SUBLINK_SPEED_ATTRIBUTE_ID, 0) |
+				    FIELD_PREP(USB_SSP_MIN_RX_LANE_COUNT, 1) |
+				    FIELD_PREP(USB_SSP_MIN_TX_LANE_COUNT, 1));
 
-		/*
-		 * bmSublinkSpeedAttr[0]:
-		 *   ST  = Symmetric, RX
-		 *   LSE =  3 (Gbps)
-		 *   LP  =  1 (SuperSpeedPlus)
-		 *   LSM = 10 (10 Gbps)
-		 */
 		ssp_cap->bmSublinkSpeedAttr[0] =
-			cpu_to_le32((3 << 4) | (1 << 14) | (0xa << 16));
-		/*
-		 * bmSublinkSpeedAttr[1] =
-		 *   ST  = Symmetric, TX
-		 *   LSE =  3 (Gbps)
-		 *   LP  =  1 (SuperSpeedPlus)
-		 *   LSM = 10 (10 Gbps)
-		 */
+			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, 0) |
+				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE,
+					       USB_SSP_SUBLINK_SPEED_LSE_GBPS) |
+				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
+					       USB_SSP_SUBLINK_SPEED_ST_SYM_RX) |
+				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP,
+					       USB_SSP_SUBLINK_SPEED_LP_SSP) |
+				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, 10));
+
 		ssp_cap->bmSublinkSpeedAttr[1] =
-			cpu_to_le32((3 << 4) | (1 << 14) |
-				    (0xa << 16) | (1 << 7));
+			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, 0) |
+				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE,
+					       USB_SSP_SUBLINK_SPEED_LSE_GBPS) |
+				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
+					       USB_SSP_SUBLINK_SPEED_ST_SYM_TX) |
+				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP,
+					       USB_SSP_SUBLINK_SPEED_LP_SSP) |
+				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, 10));
 	}
 
 	return le16_to_cpu(bos->wTotalLength);

Signature · 1 line

2.28.0

Thinh Nguyen

author maintainer

2021-01-14 02:54 UTC #8 lore raw

PATCH v6 3/11 usb: gadget: Introduce SSP rates and lanes

In reply to Thinh Nguyen

A USB device controller operating in SuperSpeed Plus may support gen2x1, gen1x2, and/or gen2x2. Introduce SuperSpeed Plus signaling rate generation and lane count to usb_gadget with the fields ssp_rate and max_ssp_rate. The gadget driver can use these to setup the device BOS descriptor and select the desire operating speed and number of lanes.

Signed-off-by Thinh Nguyen <[email protected]>

---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Remove all the sublink speed fields and use only usb_ssp_rate enums
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- Change unsigned fields to unsigned int
Changes in v3:
- None
Changes in v2:
- None

 include/linux/usb/gadget.h | 9 +++++++++
 1 file changed, 9 insertions(+)

include/linux/usb/gadget.h +9 -0

index e7351d64f11f..02483c862444 100644
@@ -339,6 +339,10 @@ struct usb_gadget_ops {
  * @speed: Speed of current connection to USB host.
  * @max_speed: Maximal speed the UDC can handle.  UDC must support this
  *      and all slower speeds.
+ * @ssp_rate: Current connected SuperSpeed Plus signaling rate and lane count.
+ * @max_ssp_rate: Maximum SuperSpeed Plus signaling rate and lane count the UDC
+ *	can handle. The UDC must support this and all slower speeds and lower
+ *	number of lanes.
  * @state: the state we are now (attached, suspended, configured, etc)
  * @name: Identifies the controller hardware type.  Used in diagnostics
  *	and sometimes configuration.

@@ -406,6 +410,11 @@ struct usb_gadget {
 	struct list_head		ep_list;	/* of usb_ep */
 	enum usb_device_speed		speed;
 	enum usb_device_speed		max_speed;
+
+	/* USB SuperSpeed Plus only */
+	enum usb_ssp_rate		ssp_rate;
+	enum usb_ssp_rate		max_ssp_rate;
+
 	enum usb_device_state		state;
 	const char			*name;
 	struct device			dev;

Signature · 1 line

2.28.0

Thinh Nguyen

author maintainer

2021-01-14 02:54 UTC #9 lore raw

PATCH v6 4/11 usb: gadget: Introduce udc_set_ssp_rate() for SSP

In reply to Thinh Nguyen

A SuperSpeed Plus device may operate at different speed and lane count (i.e. gen2x2, gen1x2, or gen2x1). Introduce gadget ops udc_set_ssp_rate() to set the desire corresponding usb_ssp_rate for SuperSpeed Plus capable devices.

If the USB device supports different speeds at SuperSpeed Plus, set the device to operate with the maximum number of lanes and speed.

Signed-off-by Thinh Nguyen <[email protected]>

---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Rename udc_set_num_lanes_and_speed() to udc_set_ssp_rate()
- Use usb_ssp_rate enum as a parameter instead of SSID
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- Add identifier name for usb_gadget in gadget ops (*udc_set_num_lanes_and_speed) to avoid checkpatch warning
Changes in v3:
- None
Changes in v2:
- None

 drivers/usb/gadget/udc/core.c | 16 +++++++++++-----
 include/linux/usb/gadget.h    |  2 ++
 2 files changed, 13 insertions(+), 5 deletions(-)

drivers/usb/gadget/udc/core.c +11 -5

index 98cf9216f3cb..4173acdcd35b 100644
@@ -1133,12 +1133,18 @@ static inline void usb_gadget_udc_stop(struct usb_udc *udc)
 static inline void usb_gadget_udc_set_speed(struct usb_udc *udc,
 					    enum usb_device_speed speed)
 {
-	if (udc->gadget->ops->udc_set_speed) {
-		enum usb_device_speed s;
+	struct usb_gadget *gadget = udc->gadget;
+	enum usb_device_speed s;
 
-		s = min(speed, udc->gadget->max_speed);
-		udc->gadget->ops->udc_set_speed(udc->gadget, s);
-	}
+	if (speed == USB_SPEED_UNKNOWN)
+		s = gadget->max_speed;
+	else
+		s = min(speed, gadget->max_speed);
+
+	if (s == USB_SPEED_SUPER_PLUS && gadget->ops->udc_set_ssp_rate)
+		gadget->ops->udc_set_ssp_rate(gadget, gadget->max_ssp_rate);
+	else if (gadget->ops->udc_set_speed)
+		gadget->ops->udc_set_speed(gadget, s);
 }
 
 /**

include/linux/usb/gadget.h +2 -0

index 02483c862444..ee04ef214ce8 100644
@@ -323,6 +323,8 @@ struct usb_gadget_ops {
 			struct usb_gadget_driver *);
 	int	(*udc_stop)(struct usb_gadget *);
 	void	(*udc_set_speed)(struct usb_gadget *, enum usb_device_speed);
+	void	(*udc_set_ssp_rate)(struct usb_gadget *gadget,
+			enum usb_ssp_rate rate);
 	struct usb_ep *(*match_ep)(struct usb_gadget *,
 			struct usb_endpoint_descriptor *,
 			struct usb_ss_ep_comp_descriptor *);

Signature · 1 line

2.28.0

Thinh Nguyen

author maintainer

2021-01-14 02:54 UTC #10 lore raw

PATCH v6 5/11 usb: gadget: composite: Report various SSP sublink speeds

In reply to Thinh Nguyen

If a gadget supports SuperSpeed Plus, then it may operate in different sublink speeds. For example, if the gadget supports SuperSpeed Plus gen2x2, then it can support 2 sublink speeds gen1 and gen2. Inform the host of these speeds in the BOS descriptor.

Use 1 SSID if the gadget supports up to gen2x1, or not specified:
- SSID 0 for symmetric RX/TX sublink speed of 10 Gbps.

Use 1 SSID if the gadget supports up to gen1x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.

Use 2 SSIDs if the gadget supports up to gen2x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.

Signed-off-by Thinh Nguyen <[email protected]>

---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Use gadget->max_ssp_rate instead of all the sublink attribute fields (now removed) in usb_gadget
- Use the updated macros
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- None
Changes in v3:
- None
Changes in v2:
- None

 drivers/usb/gadget/composite.c | 80 +++++++++++++++++++++++-----------
 1 file changed, 54 insertions(+), 26 deletions(-)

drivers/usb/gadget/composite.c +54 ~~-26~~

index bc17302a9e85..72a9797dbbae 100644
@@ -735,49 +735,77 @@ static int bos_desc(struct usb_composite_dev *cdev)
 	/* The SuperSpeedPlus USB Device Capability descriptor */
 	if (gadget_is_superspeed_plus(cdev->gadget)) {
 		struct usb_ssp_cap_descriptor *ssp_cap;
+		u8 ssac = 1;
+		u8 ssic;
+		int i;
 
-		ssp_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
-		bos->bNumDeviceCaps++;
+		if (cdev->gadget->max_ssp_rate == USB_SSP_GEN_2x2)
+			ssac = 3;
 
 		/*
-		 * Report typical values.
+		 * Paired RX and TX sublink speed attributes share
+		 * the same SSID.
 		 */
+		ssic = (ssac + 1) / 2 - 1;
 
-		le16_add_cpu(&bos->wTotalLength, USB_DT_USB_SSP_CAP_SIZE(1));
-		ssp_cap->bLength = USB_DT_USB_SSP_CAP_SIZE(1);
+		ssp_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
+		bos->bNumDeviceCaps++;
+
+		le16_add_cpu(&bos->wTotalLength, USB_DT_USB_SSP_CAP_SIZE(ssac));
+		ssp_cap->bLength = USB_DT_USB_SSP_CAP_SIZE(ssac);
 		ssp_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
 		ssp_cap->bDevCapabilityType = USB_SSP_CAP_TYPE;
 		ssp_cap->bReserved = 0;
 		ssp_cap->wReserved = 0;
 
 		ssp_cap->bmAttributes =
-			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_ATTRIBS, 1) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_IDS, 0));
+			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_ATTRIBS, ssac) |
+				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_IDS, ssic));
 
 		ssp_cap->wFunctionalitySupport =
 			cpu_to_le16(FIELD_PREP(USB_SSP_MIN_SUBLINK_SPEED_ATTRIBUTE_ID, 0) |
 				    FIELD_PREP(USB_SSP_MIN_RX_LANE_COUNT, 1) |
 				    FIELD_PREP(USB_SSP_MIN_TX_LANE_COUNT, 1));
 
-		ssp_cap->bmSublinkSpeedAttr[0] =
-			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, 0) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE,
-					       USB_SSP_SUBLINK_SPEED_LSE_GBPS) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
-					       USB_SSP_SUBLINK_SPEED_ST_SYM_RX) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP,
-					       USB_SSP_SUBLINK_SPEED_LP_SSP) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, 10));
-
-		ssp_cap->bmSublinkSpeedAttr[1] =
-			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, 0) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE,
-					       USB_SSP_SUBLINK_SPEED_LSE_GBPS) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
-					       USB_SSP_SUBLINK_SPEED_ST_SYM_TX) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP,
-					       USB_SSP_SUBLINK_SPEED_LP_SSP) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, 10));
+		/*
+		 * Use 1 SSID if the gadget supports up to gen2x1 or not
+		 * specified:
+		 * - SSID 0 for symmetric RX/TX sublink speed of 10 Gbps.
+		 *
+		 * Use 1 SSID if the gadget supports up to gen1x2:
+		 * - SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
+		 *
+		 * Use 2 SSIDs if the gadget supports up to gen2x2:
+		 * - SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
+		 * - SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.
+		 */
+		for (i = 0; i < ssac + 1; i++) {
+			u8 ssid;
+			u8 mantissa;
+			u8 type;
+
+			ssid = i >> 1;
+
+			if (cdev->gadget->max_ssp_rate == USB_SSP_GEN_2x1 ||
+			    cdev->gadget->max_ssp_rate == USB_SSP_GEN_UNKNOWN)
+				mantissa = 10;
+			else
+				mantissa = 5 << ssid;
+
+			if (i % 2)
+				type = USB_SSP_SUBLINK_SPEED_ST_SYM_TX;
+			else
+				type = USB_SSP_SUBLINK_SPEED_ST_SYM_RX;
+
+			ssp_cap->bmSublinkSpeedAttr[i] =
+				cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, ssid) |
+					    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE,
+						       USB_SSP_SUBLINK_SPEED_LSE_GBPS) |
+					    FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST, type) |
+					    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP,
+						       USB_SSP_SUBLINK_SPEED_LP_SSP) |
+					    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, mantissa));
+		}
 	}
 
 	return le16_to_cpu(bos->wTotalLength);

Signature · 1 line

2.28.0

Peter Chen

2021-01-14 05:48 UTC #11 lore raw

Re: [PATCH v6 05/11] usb: gadget: composite: Report various SSP sublink speeds

Quoted 5 lines · Thinh Nguyen

If a gadget supports SuperSpeed Plus, then it may operate in different sublink speeds. For example, if the gadget supports SuperSpeed Plus gen2x2, then it can support 2 sublink speeds gen1 and gen2. Inform the host of these speeds in the BOS descriptor.

Hi Thinh,

I read USB 3.2 spec: ch9.6.2.5 SuperSpeedPlus USB Device Capability

Symmetric. Rx and Tx Sublinks have the same number of lanes and operate at the same speed.
Asymmetric. Rx and Tx Sublink have different number of lanes and/or operate at different speeds.

Why your below cases are all for symmetric, at least, the example 3 is asymmetric, it has different speed for sublink pairs? Does your below cases are specification defined or user defined?

Quoted 8 lines

Use 1 SSID if the gadget supports up to gen2x1, or not specified:
- SSID 0 for symmetric RX/TX sublink speed of 10 Gbps.
Use 1 SSID if the gadget supports up to gen1x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
Use 2 SSIDs if the gadget supports up to gen2x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.

Why SSID 0 is not 10Gbps?

Quoted 1 line

- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.

Besides, would you give me an example what kinds of system design will use below sublink speed?
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.

Peter

Quoted 133 lines

Signed-off-by Thinh Nguyen <[email protected]>

 drivers/usb/gadget/composite.c | 80 +++++++++++++++++++++++-----------
 1 file changed, 54 insertions(+), 26 deletions(-)

drivers/usb/gadget/composite.c +54 ~~-26~~

index bc17302a9e85..72a9797dbbae 100644
@@ -735,49 +735,77 @@ static int bos_desc(struct usb_composite_dev *cdev)
 	/* The SuperSpeedPlus USB Device Capability descriptor */
 	if (gadget_is_superspeed_plus(cdev->gadget)) {
 		struct usb_ssp_cap_descriptor *ssp_cap;
+		u8 ssac = 1;
+		u8 ssic;
+		int i;
 
-		ssp_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
-		bos->bNumDeviceCaps++;
+		if (cdev->gadget->max_ssp_rate == USB_SSP_GEN_2x2)
+			ssac = 3;
 
 		/*
-		 * Report typical values.
+		 * Paired RX and TX sublink speed attributes share
+		 * the same SSID.
 		 */
+		ssic = (ssac + 1) / 2 - 1;
 
-		le16_add_cpu(&bos->wTotalLength, USB_DT_USB_SSP_CAP_SIZE(1));
-		ssp_cap->bLength = USB_DT_USB_SSP_CAP_SIZE(1);
+		ssp_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
+		bos->bNumDeviceCaps++;
+
+		le16_add_cpu(&bos->wTotalLength, USB_DT_USB_SSP_CAP_SIZE(ssac));
+		ssp_cap->bLength = USB_DT_USB_SSP_CAP_SIZE(ssac);
 		ssp_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
 		ssp_cap->bDevCapabilityType = USB_SSP_CAP_TYPE;
 		ssp_cap->bReserved = 0;
 		ssp_cap->wReserved = 0;
 
 		ssp_cap->bmAttributes =
-			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_ATTRIBS, 1) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_IDS, 0));
+			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_ATTRIBS, ssac) |
+				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_IDS, ssic));
 
 		ssp_cap->wFunctionalitySupport =
 			cpu_to_le16(FIELD_PREP(USB_SSP_MIN_SUBLINK_SPEED_ATTRIBUTE_ID, 0) |
 				    FIELD_PREP(USB_SSP_MIN_RX_LANE_COUNT, 1) |
 				    FIELD_PREP(USB_SSP_MIN_TX_LANE_COUNT, 1));
 
-		ssp_cap->bmSublinkSpeedAttr[0] =
-			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, 0) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE,
-					       USB_SSP_SUBLINK_SPEED_LSE_GBPS) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
-					       USB_SSP_SUBLINK_SPEED_ST_SYM_RX) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP,
-					       USB_SSP_SUBLINK_SPEED_LP_SSP) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, 10));
-
-		ssp_cap->bmSublinkSpeedAttr[1] =
-			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, 0) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE,
-					       USB_SSP_SUBLINK_SPEED_LSE_GBPS) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
-					       USB_SSP_SUBLINK_SPEED_ST_SYM_TX) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP,
-					       USB_SSP_SUBLINK_SPEED_LP_SSP) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, 10));
+		/*
+		 * Use 1 SSID if the gadget supports up to gen2x1 or not
+		 * specified:
+		 * - SSID 0 for symmetric RX/TX sublink speed of 10 Gbps.
+		 *
+		 * Use 1 SSID if the gadget supports up to gen1x2:
+		 * - SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
+		 *
+		 * Use 2 SSIDs if the gadget supports up to gen2x2:
+		 * - SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
+		 * - SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.
+		 */
+		for (i = 0; i < ssac + 1; i++) {
+			u8 ssid;
+			u8 mantissa;
+			u8 type;
+
+			ssid = i >> 1;
+
+			if (cdev->gadget->max_ssp_rate == USB_SSP_GEN_2x1 ||
+			    cdev->gadget->max_ssp_rate == USB_SSP_GEN_UNKNOWN)
+				mantissa = 10;
+			else
+				mantissa = 5 << ssid;
+
+			if (i % 2)
+				type = USB_SSP_SUBLINK_SPEED_ST_SYM_TX;
+			else
+				type = USB_SSP_SUBLINK_SPEED_ST_SYM_RX;
+
+			ssp_cap->bmSublinkSpeedAttr[i] =
+				cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, ssid) |
+					    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE,
+						       USB_SSP_SUBLINK_SPEED_LSE_GBPS) |
+					    FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST, type) |
+					    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP,
+						       USB_SSP_SUBLINK_SPEED_LP_SSP) |
+					    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, mantissa));
+		}
 	}
 
 	return le16_to_cpu(bos->wTotalLength);

Signature · 1 line

2.28.0

Signature · 2 lines

Thanks,
Peter Chen

Thinh Nguyen

author maintainer

2021-01-14 06:17 UTC #12 lore raw

Re: [PATCH v6 05/11] usb: gadget: composite: Report various SSP sublink speeds

Hi Peter,

Quoted 10 lines · Peter Chen

Quoted 5 lines · Thinh Nguyen
If a gadget supports SuperSpeed Plus, then it may operate in different sublink speeds. For example, if the gadget supports SuperSpeed Plus gen2x2, then it can support 2 sublink speeds gen1 and gen2. Inform the host of these speeds in the BOS descriptor.
Hi Thinh,
I read USB 3.2 spec: ch9.6.2.5 SuperSpeedPlus USB Device Capability

Quoted 8 lines

Symmetric. Rx and Tx Sublinks have the same number of lanes and operate at the same speed.
Asymmetric. Rx and Tx Sublink have different number of lanes and/or operate at different speeds.
Why your below cases are all for symmetric, at least, the example 3 is asymmetric, it has different speed for sublink pairs? Does your below cases are specification defined or user defined?

USB 3.2 spec section 8.5.6.7:
Asymmetric lane types are only for SuperSpeed Interchip (SSIC). IMO, It's unlikely that SSIC user will use Linux kernel. We can extend and update the gadget framework if there's any use case for that.

Quoted 10 lines

Quoted 9 lines
Use 1 SSID if the gadget supports up to gen2x1, or not specified:
- SSID 0 for symmetric RX/TX sublink speed of 10 Gbps.
Use 1 SSID if the gadget supports up to gen1x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
Use 2 SSIDs if the gadget supports up to gen2x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
Why SSID 0 is not 10Gbps?

SSID 0 and 1 are arbitrary, we can do 0 for 10Gbps. There's no constraint or standard from the USB 3.2 spec. However, you need to set the descriptor wFunctionalitySupport.SSID to be the minimum lane speed SSID it supports. Using SSID 0 makes it easier since we don't have to condition it for multiple SSIDs.

Quoted 7 lines

Quoted 1 line
- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.
Besides, would you give me an example what kinds of system design will use below sublink speed?
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.
Peter

These 2 SSIDs indicate that the device is capable of running in gen1 and gen2 in SSP.

Thank you for reviewing it.

BR,
Thinh

Quoted 134 lines

Signed-off-by Thinh Nguyen <[email protected]>

 drivers/usb/gadget/composite.c | 80 +++++++++++++++++++++++-----------
 1 file changed, 54 insertions(+), 26 deletions(-)

drivers/usb/gadget/composite.c +54 ~~-26~~

index bc17302a9e85..72a9797dbbae 100644
@@ -735,49 +735,77 @@ static int bos_desc(struct usb_composite_dev *cdev)
 	/* The SuperSpeedPlus USB Device Capability descriptor */
 	if (gadget_is_superspeed_plus(cdev->gadget)) {
 		struct usb_ssp_cap_descriptor *ssp_cap;
+		u8 ssac = 1;
+		u8 ssic;
+		int i;
 
-		ssp_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
-		bos->bNumDeviceCaps++;
+		if (cdev->gadget->max_ssp_rate == USB_SSP_GEN_2x2)
+			ssac = 3;
 
 		/*
-		 * Report typical values.
+		 * Paired RX and TX sublink speed attributes share
+		 * the same SSID.
 		 */
+		ssic = (ssac + 1) / 2 - 1;
 
-		le16_add_cpu(&bos->wTotalLength, USB_DT_USB_SSP_CAP_SIZE(1));
-		ssp_cap->bLength = USB_DT_USB_SSP_CAP_SIZE(1);
+		ssp_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
+		bos->bNumDeviceCaps++;
+
+		le16_add_cpu(&bos->wTotalLength, USB_DT_USB_SSP_CAP_SIZE(ssac));
+		ssp_cap->bLength = USB_DT_USB_SSP_CAP_SIZE(ssac);
 		ssp_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
 		ssp_cap->bDevCapabilityType = USB_SSP_CAP_TYPE;
 		ssp_cap->bReserved = 0;
 		ssp_cap->wReserved = 0;
 
 		ssp_cap->bmAttributes =
-			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_ATTRIBS, 1) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_IDS, 0));
+			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_ATTRIBS, ssac) |
+				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_IDS, ssic));
 
 		ssp_cap->wFunctionalitySupport =
 			cpu_to_le16(FIELD_PREP(USB_SSP_MIN_SUBLINK_SPEED_ATTRIBUTE_ID, 0) |
 				    FIELD_PREP(USB_SSP_MIN_RX_LANE_COUNT, 1) |
 				    FIELD_PREP(USB_SSP_MIN_TX_LANE_COUNT, 1));
 
-		ssp_cap->bmSublinkSpeedAttr[0] =
-			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, 0) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE,
-					       USB_SSP_SUBLINK_SPEED_LSE_GBPS) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
-					       USB_SSP_SUBLINK_SPEED_ST_SYM_RX) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP,
-					       USB_SSP_SUBLINK_SPEED_LP_SSP) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, 10));
-
-		ssp_cap->bmSublinkSpeedAttr[1] =
-			cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, 0) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE,
-					       USB_SSP_SUBLINK_SPEED_LSE_GBPS) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
-					       USB_SSP_SUBLINK_SPEED_ST_SYM_TX) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP,
-					       USB_SSP_SUBLINK_SPEED_LP_SSP) |
-				    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, 10));
+		/*
+		 * Use 1 SSID if the gadget supports up to gen2x1 or not
+		 * specified:
+		 * - SSID 0 for symmetric RX/TX sublink speed of 10 Gbps.
+		 *
+		 * Use 1 SSID if the gadget supports up to gen1x2:
+		 * - SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
+		 *
+		 * Use 2 SSIDs if the gadget supports up to gen2x2:
+		 * - SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
+		 * - SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.
+		 */
+		for (i = 0; i < ssac + 1; i++) {
+			u8 ssid;
+			u8 mantissa;
+			u8 type;
+
+			ssid = i >> 1;
+
+			if (cdev->gadget->max_ssp_rate == USB_SSP_GEN_2x1 ||
+			    cdev->gadget->max_ssp_rate == USB_SSP_GEN_UNKNOWN)
+				mantissa = 10;
+			else
+				mantissa = 5 << ssid;
+
+			if (i % 2)
+				type = USB_SSP_SUBLINK_SPEED_ST_SYM_TX;
+			else
+				type = USB_SSP_SUBLINK_SPEED_ST_SYM_RX;
+
+			ssp_cap->bmSublinkSpeedAttr[i] =
+				cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, ssid) |
+					    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE,
+						       USB_SSP_SUBLINK_SPEED_LSE_GBPS) |
+					    FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST, type) |
+					    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP,
+						       USB_SSP_SUBLINK_SPEED_LP_SSP) |
+					    FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, mantissa));
+		}
 	}
 
 	return le16_to_cpu(bos->wTotalLength);

Signature · 1 line

2.28.0

Peter Chen

2021-01-15 00:52 UTC #13 lore raw

Re: [PATCH v6 05/11] usb: gadget: composite: Report various SSP sublink speeds

Quoted 51 lines · Thinh Nguyen

Hi Peter,
Quoted 10 lines · Peter Chen
Quoted 5 lines · Thinh Nguyen
If a gadget supports SuperSpeed Plus, then it may operate in different sublink speeds. For example, if the gadget supports SuperSpeed Plus gen2x2, then it can support 2 sublink speeds gen1 and gen2. Inform the host of these speeds in the BOS descriptor.
Hi Thinh,
I read USB 3.2 spec: ch9.6.2.5 SuperSpeedPlus USB Device Capability
Quoted 8 lines
Symmetric. Rx and Tx Sublinks have the same number of lanes and operate at the same speed.
Asymmetric. Rx and Tx Sublink have different number of lanes and/or operate at different speeds.
Why your below cases are all for symmetric, at least, the example 3 is asymmetric, it has different speed for sublink pairs? Does your below cases are specification defined or user defined?
USB 3.2 spec section 8.5.6.7:
Asymmetric lane types are only for SuperSpeed Interchip (SSIC). IMO, It's unlikely that SSIC user will use Linux kernel. We can extend and update the gadget framework if there's any use case for that.
Quoted 10 lines
Quoted 9 lines
Use 1 SSID if the gadget supports up to gen2x1, or not specified:
- SSID 0 for symmetric RX/TX sublink speed of 10 Gbps.
Use 1 SSID if the gadget supports up to gen1x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
Use 2 SSIDs if the gadget supports up to gen2x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
Why SSID 0 is not 10Gbps?
SSID 0 and 1 are arbitrary, we can do 0 for 10Gbps. There's no constraint or standard from the USB 3.2 spec. However, you need to set the descriptor wFunctionalitySupport.SSID to be the minimum lane speed SSID it supports. Using SSID 0 makes it easier since we don't have to condition it for multiple SSIDs.
Quoted 6 lines
Quoted 1 line
- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.
Besides, would you give me an example what kinds of system design will use below sublink speed?
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.

Quoted 5 lines

Quoted 1 line
Peter
These 2 SSIDs indicate that the device is capable of running in gen1 and gen2 in SSP.

Hi Thinh,

I am puzzled, GEN2 is back compatible for GEN1. Then, what's the usage of this descriptor, what kinds of specific information the host wants to get? I think the host wants to get if two pairs of tx/rx are supported, that is what USB 3.2 adds.

Signature · 2 lines

Thanks,
Peter Chen

Thinh Nguyen

author maintainer

2021-01-15 02:42 UTC #14 lore raw

Re: [PATCH v6 05/11] usb: gadget: composite: Report various SSP sublink speeds

Quoted 53 lines · Peter Chen

Quoted 43 lines · Thinh Nguyen
Hi Peter,
Quoted 10 lines · Peter Chen
Quoted 5 lines · Thinh Nguyen
If a gadget supports SuperSpeed Plus, then it may operate in different sublink speeds. For example, if the gadget supports SuperSpeed Plus gen2x2, then it can support 2 sublink speeds gen1 and gen2. Inform the host of these speeds in the BOS descriptor.
Hi Thinh,
I read USB 3.2 spec: ch9.6.2.5 SuperSpeedPlus USB Device Capability
Quoted 8 lines
Symmetric. Rx and Tx Sublinks have the same number of lanes and operate at the same speed.
Asymmetric. Rx and Tx Sublink have different number of lanes and/or operate at different speeds.
Why your below cases are all for symmetric, at least, the example 3 is asymmetric, it has different speed for sublink pairs? Does your below cases are specification defined or user defined?
USB 3.2 spec section 8.5.6.7:
Asymmetric lane types are only for SuperSpeed Interchip (SSIC). IMO, It's unlikely that SSIC user will use Linux kernel. We can extend and update the gadget framework if there's any use case for that.
Quoted 9 lines
Quoted 8 lines
Use 1 SSID if the gadget supports up to gen2x1, or not specified:
- SSID 0 for symmetric RX/TX sublink speed of 10 Gbps.
Use 1 SSID if the gadget supports up to gen1x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
Use 2 SSIDs if the gadget supports up to gen2x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
Why SSID 0 is not 10Gbps?
SSID 0 and 1 are arbitrary, we can do 0 for 10Gbps. There's no constraint or standard from the USB 3.2 spec. However, you need to set the descriptor wFunctionalitySupport.SSID to be the minimum lane speed SSID it supports. Using SSID 0 makes it easier since we don't have to condition it for multiple SSIDs.
Quoted 1 line
Quoted 1 line
- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.
Quoted 9 lines
Quoted 6 lines
Besides, would you give me an example what kinds of system design will use below sublink speed?
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.
Peter
These 2 SSIDs indicate that the device is capable of running in gen1 and gen2 in SSP.

Quoted 7 lines

Hi Thinh,
I am puzzled, GEN2 is back compatible for GEN1. Then, what's the usage of this descriptor, what kinds of specific information the host wants to get? I think the host wants to get if two pairs of tx/rx are supported, that is what USB 3.2 adds.

Hi Peter,

Yes, it's backward compatible. However Gen1x2 uses SuperSpeed Plus Link Protocol. Single lane Gen 1 doesn't. This additional RX/TX sublink speed attribute pair is telling the host that the device is capable of running at Gen1x2 and Gen2x2. Host can use this information to know that the device supports Gen 1 as SSP, and it can infer that the device supports dual-lane. Otherwise, the host can only check for dual-lane support after it receives a port status or sublink speed device notification TP. Regardless, the gadget should describe all the sublink speeds the device is capable of in the SSP capability descriptor.

BR,
Thinh

Thinh Nguyen

author maintainer

2021-01-15 02:57 UTC #15 lore raw

Re: [PATCH v6 05/11] usb: gadget: composite: Report various SSP sublink speeds

Quoted 63 lines · Thinh Nguyen

Quoted 53 lines · Peter Chen
Quoted 43 lines · Thinh Nguyen
Hi Peter,
Quoted 10 lines · Peter Chen
Quoted 5 lines · Thinh Nguyen
If a gadget supports SuperSpeed Plus, then it may operate in different sublink speeds. For example, if the gadget supports SuperSpeed Plus gen2x2, then it can support 2 sublink speeds gen1 and gen2. Inform the host of these speeds in the BOS descriptor.
Hi Thinh,
I read USB 3.2 spec: ch9.6.2.5 SuperSpeedPlus USB Device Capability
Quoted 8 lines
Symmetric. Rx and Tx Sublinks have the same number of lanes and operate at the same speed.
Asymmetric. Rx and Tx Sublink have different number of lanes and/or operate at different speeds.
Why your below cases are all for symmetric, at least, the example 3 is asymmetric, it has different speed for sublink pairs? Does your below cases are specification defined or user defined?
USB 3.2 spec section 8.5.6.7:
Asymmetric lane types are only for SuperSpeed Interchip (SSIC). IMO, It's unlikely that SSIC user will use Linux kernel. We can extend and update the gadget framework if there's any use case for that.
Quoted 9 lines
Quoted 8 lines
Use 1 SSID if the gadget supports up to gen2x1, or not specified:
- SSID 0 for symmetric RX/TX sublink speed of 10 Gbps.
Use 1 SSID if the gadget supports up to gen1x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
Use 2 SSIDs if the gadget supports up to gen2x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
Why SSID 0 is not 10Gbps?
SSID 0 and 1 are arbitrary, we can do 0 for 10Gbps. There's no constraint or standard from the USB 3.2 spec. However, you need to set the descriptor wFunctionalitySupport.SSID to be the minimum lane speed SSID it supports. Using SSID 0 makes it easier since we don't have to condition it for multiple SSIDs.
Quoted 1 line
Quoted 1 line
- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.
Quoted 9 lines
Quoted 6 lines
Besides, would you give me an example what kinds of system design will use below sublink speed?
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.
Peter
These 2 SSIDs indicate that the device is capable of running in gen1 and gen2 in SSP.
Quoted 7 lines
Hi Thinh,
I am puzzled, GEN2 is back compatible for GEN1. Then, what's the usage of this descriptor, what kinds of specific information the host wants to get? I think the host wants to get if two pairs of tx/rx are supported, that is what USB 3.2 adds.
Hi Peter,

Quoted 10 lines

Yes, it's backward compatible. However Gen1x2 uses SuperSpeed Plus Link Protocol. Single lane Gen 1 doesn't. This additional RX/TX sublink speed attribute pair is telling the host that the device is capable of running at Gen1x2 and Gen2x2. Host can use this information to know that the device supports Gen 1 as SSP, and it can infer that the device supports dual-lane. Otherwise, the host can only check for dual-lane support after it receives a port status or sublink speed device notification TP. Regardless, the gadget should describe all the sublink speeds the device is capable of in the SSP capability descriptor.

Regarding checking port status and sublink speed device notification, I meant it as "connected" status and not capability.

Thinh

Peter Chen

2021-01-16 17:11 UTC #16 lore raw

Re: [PATCH v6 05/11] usb: gadget: composite: Report various SSP sublink speeds

Quoted 75 lines · Thinh Nguyen

Quoted 63 lines · Thinh Nguyen
Quoted 53 lines · Peter Chen
Quoted 43 lines · Thinh Nguyen
Hi Peter,
Quoted 10 lines · Peter Chen
Quoted 5 lines · Thinh Nguyen
If a gadget supports SuperSpeed Plus, then it may operate in different sublink speeds. For example, if the gadget supports SuperSpeed Plus gen2x2, then it can support 2 sublink speeds gen1 and gen2. Inform the host of these speeds in the BOS descriptor.
Hi Thinh,
I read USB 3.2 spec: ch9.6.2.5 SuperSpeedPlus USB Device Capability
Quoted 8 lines
Symmetric. Rx and Tx Sublinks have the same number of lanes and operate at the same speed.
Asymmetric. Rx and Tx Sublink have different number of lanes and/or operate at different speeds.
Why your below cases are all for symmetric, at least, the example 3 is asymmetric, it has different speed for sublink pairs? Does your below cases are specification defined or user defined?
USB 3.2 spec section 8.5.6.7:
Asymmetric lane types are only for SuperSpeed Interchip (SSIC). IMO, It's unlikely that SSIC user will use Linux kernel. We can extend and update the gadget framework if there's any use case for that.
Quoted 9 lines
Quoted 8 lines
Use 1 SSID if the gadget supports up to gen2x1, or not specified:
- SSID 0 for symmetric RX/TX sublink speed of 10 Gbps.
Use 1 SSID if the gadget supports up to gen1x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
Use 2 SSIDs if the gadget supports up to gen2x2:
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
Why SSID 0 is not 10Gbps?
SSID 0 and 1 are arbitrary, we can do 0 for 10Gbps. There's no constraint or standard from the USB 3.2 spec. However, you need to set the descriptor wFunctionalitySupport.SSID to be the minimum lane speed SSID it supports. Using SSID 0 makes it easier since we don't have to condition it for multiple SSIDs.
Quoted 1 line
Quoted 1 line
- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.
Quoted 9 lines
Quoted 6 lines
Besides, would you give me an example what kinds of system design will use below sublink speed?
- SSID 0 for symmetric RX/TX sublink speed of 5 Gbps.
- SSID 1 for symmetric RX/TX sublink speed of 10 Gbps.
Peter
These 2 SSIDs indicate that the device is capable of running in gen1 and gen2 in SSP.
Quoted 7 lines
Hi Thinh,
I am puzzled, GEN2 is back compatible for GEN1. Then, what's the usage of this descriptor, what kinds of specific information the host wants to get? I think the host wants to get if two pairs of tx/rx are supported, that is what USB 3.2 adds.
Hi Peter,
Quoted 10 lines
Yes, it's backward compatible. However Gen1x2 uses SuperSpeed Plus Link Protocol. Single lane Gen 1 doesn't. This additional RX/TX sublink speed attribute pair is telling the host that the device is capable of running at Gen1x2 and Gen2x2. Host can use this information to know that the device supports Gen 1 as SSP, and it can infer that the device supports dual-lane. Otherwise, the host can only check for dual-lane support after it receives a port status or sublink speed device notification TP. Regardless, the gadget should describe all the sublink speeds the device is capable of in the SSP capability descriptor.

Quoted 3 lines

Regarding checking port status and sublink speed device notification, I meant it as "connected" status and not capability.

Thanks for explaining it, Thinh

Signature · 2 lines

Thanks,
Peter Chen

Thinh Nguyen

author maintainer

2021-01-14 02:54 UTC #17 lore raw

PATCH v6 6/11 dt-binding: usb: Include USB SSP rates in GenXxY

In reply to Thinh Nguyen

According to the USB 3.2 spec, a SuperSpeed Plus device can operate at gen2x2, gen2x1, or gen1x2. If the USB controller device supports multiple lanes at different transfer rates, the user can specify the HW capability via these new speed strings:

"super-speed-plus-gen2x2"
"super-speed-plus-gen2x1"
"super-speed-plus-gen1x2"

If the argument is simply "super-speed-plus", USB controllers should default to their maximum transfer rate and number of lanes.

Signed-off-by Thinh Nguyen <[email protected]>

---
Changes in v6:
- Update the different maximum_speed enums to the usb.yaml
- Remove Reviewed-by: Rob Herring <[email protected]> because the commit is updated
- Rebase on Greg's usb-testing branch
- Update commit message
Changes in v5:
- Add Reviewed-by: Rob Herring <[email protected]>
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header
Changes in v4:
- None
Changes in v3:
- Use "maximum-speed" to include both the num-lane and transfer rate for SSP
- Remove "num-lanes" and "lane-speed-mantissa-gbps" properties Changes in v2:
- Make "num-lanes" and "lane-speed-mantissa-gbps" common USB properties

 Documentation/devicetree/bindings/usb/usb.yaml | 3 +++
 1 file changed, 3 insertions(+)

Documentation/devicetree/bindings/usb/usb.yaml +3 -0

index ebe7f4275c59..78491e66ed24 100644
@@ -54,6 +54,9 @@ properties:
       - high-speed
       - super-speed
       - super-speed-plus
+      - super-speed-plus-gen2x1
+      - super-speed-plus-gen1x2
+      - super-speed-plus-gen2x2
 
 additionalProperties: true

Signature · 1 line

2.28.0

Thinh Nguyen

author maintainer

2021-01-14 02:54 UTC #18 lore raw

PATCH v6 7/11 usb: common: Parse for USB SSP genXxY

In reply to Thinh Nguyen

The USB "maximum-speed" property can now take the SSP signaling rate generation and lane count with these new strings:

"super-speed-plus-gen2x2"
"super-speed-plus-gen2x1"
"super-speed-plus-gen1x2"

Introduce usb_get_maximum_ssp_rate() to parse for the corresponding usb_ssp_rate enum. The original usb_get_maximum_speed() will return USB_SPEED_SUPER_PLUS if it matches one of these new strings.

Signed-off-by Thinh Nguyen <[email protected]>

---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Remove usb_get_ssp_num_lanes() and usb_get_ssp_phy_gen() and use usb_get_ssp_rate() to return the corresponding usb_ssp_rate enum instead
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header
Changes in v4:
- Create 2 functions to get the SSP gen and number of lanes from "maximum-speed" property
- Update usb_get_maximum_speed() to check new SSP strings with genXxY
- Update commit message and subject title to reflect the new changes Changes in v3:
- Add new function to parse "maximum-speed" for lanes and transfer rate
- Remove separate functions getting num_lanes and transfer rate properties Changes in v2:
- New commit

 drivers/usb/common/common.c | 26 +++++++++++++++++++++++++-
 include/linux/usb/ch9.h     | 11 +++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

drivers/usb/common/common.c +25 -1

index 1433260d99b4..fc21cf2d36f6 100644
@@ -69,6 +69,13 @@ static const char *const speed_names[] = {
 	[USB_SPEED_SUPER_PLUS] = "super-speed-plus",
 };
 
+static const char *const ssp_rate[] = {
+	[USB_SSP_GEN_UNKNOWN] = "UNKNOWN",
+	[USB_SSP_GEN_2x1] = "super-speed-plus-gen2x1",
+	[USB_SSP_GEN_1x2] = "super-speed-plus-gen1x2",
+	[USB_SSP_GEN_2x2] = "super-speed-plus-gen2x2",
+};
+
 const char *usb_speed_string(enum usb_device_speed speed)
 {
 	if (speed < 0 || speed >= ARRAY_SIZE(speed_names))

@@ -86,12 +93,29 @@ enum usb_device_speed usb_get_maximum_speed(struct device *dev)
 	if (ret < 0)
 		return USB_SPEED_UNKNOWN;
 
-	ret = match_string(speed_names, ARRAY_SIZE(speed_names), maximum_speed);
+	ret = match_string(ssp_rate, ARRAY_SIZE(ssp_rate), maximum_speed);
+	if (ret > 0)
+		return USB_SPEED_SUPER_PLUS;
 
+	ret = match_string(speed_names, ARRAY_SIZE(speed_names), maximum_speed);
 	return (ret < 0) ? USB_SPEED_UNKNOWN : ret;
 }
 EXPORT_SYMBOL_GPL(usb_get_maximum_speed);
 
+enum usb_ssp_rate usb_get_maximum_ssp_rate(struct device *dev)
+{
+	const char *maximum_speed;
+	int ret;
+
+	ret = device_property_read_string(dev, "maximum-speed", &maximum_speed);
+	if (ret < 0)
+		return USB_SSP_GEN_UNKNOWN;
+
+	ret = match_string(ssp_rate, ARRAY_SIZE(ssp_rate), maximum_speed);
+	return (ret < 0) ? USB_SSP_GEN_UNKNOWN : ret;
+}
+EXPORT_SYMBOL_GPL(usb_get_maximum_ssp_rate);
+
 const char *usb_state_string(enum usb_device_state state)
 {
 	static const char *const names[] = {

include/linux/usb/ch9.h +11 -0

index 86c50907634e..abdd310c77f0 100644
@@ -71,6 +71,17 @@ extern const char *usb_speed_string(enum usb_device_speed speed);
  */
 extern enum usb_device_speed usb_get_maximum_speed(struct device *dev);
 
+/**
+ * usb_get_maximum_ssp_rate - Get the signaling rate generation and lane count
+ *	of a SuperSpeed Plus capable device.
+ * @dev: Pointer to the given USB controller device
+ *
+ * If the string from "maximum-speed" property is super-speed-plus-genXxY where
+ * 'X' is the generation number and 'Y' is the number of lanes, then this
+ * function returns the corresponding enum usb_ssp_rate.
+ */
+extern enum usb_ssp_rate usb_get_maximum_ssp_rate(struct device *dev);
+
 /**
  * usb_state_string - Returns human readable name for the state.
  * @state: The state to return a human-readable name for. If it's not

Signature · 1 line

2.28.0

Thinh Nguyen

author maintainer

2021-01-14 02:54 UTC #19 lore raw

PATCH v6 8/11 usb: dwc3: core: Check maximum_speed SSP genXxY

In reply to Thinh Nguyen

The DWC_usb32 controller supports dual-lane SuperSpeed Plus. Check the maximum_speed property for any limitation in the HW to initialize and validate the maximum number of lanes and speed the device will operate.

Currently the controller has no visibility into the HW parameter to determine the maximum number of lanes the HW supports. If the number of lanes is not specified for SSP, then set the default rate to gen2x2 for DWC_usb32 and gen2x1 for DWC_usb31.

Signed-off-by Thinh Nguyen <[email protected]>

---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Update to check for lane count and gen# via usb_ssp_rate enum
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- Use new common funtions to get SSP Gen and number of lanes Changes in v3:
- Use new common function to get maximum-speed
- Remove num_lanes and lsm validation since they are no longer separate properties
- Replace dwc->maxmum_lsm field with dwc->maximum_ssp_rate for gen1/gen2 Changes in v2:
- Use common functions to get num_lanes and lsm properties

 drivers/usb/dwc3/core.c   | 37 +++++++++++++++++++++++++++++++++++++
 drivers/usb/dwc3/core.h   |  2 ++
 drivers/usb/dwc3/gadget.c |  1 +
 3 files changed, 40 insertions(+)

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

index 6969196fccd6..931ccf93eabd 100644
@@ -1252,6 +1252,7 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 	hird_threshold = 12;
 
 	dwc->maximum_speed = usb_get_maximum_speed(dev);
+	dwc->max_ssp_rate = usb_get_maximum_ssp_rate(dev);
 	dwc->dr_mode = usb_get_dr_mode(dev);
 	dwc->hsphy_mode = of_usb_get_phy_mode(dev->of_node);

@@ -1423,6 +1424,42 @@ static void dwc3_check_params(struct dwc3 *dwc)
 		}
 		break;
 	}
+
+	/*
+	 * Currently the controller does not have visibility into the HW
+	 * parameter to determine the maximum number of lanes the HW supports.
+	 * If the number of lanes is not specified in the device property, then
+	 * set the default to support dual-lane for DWC_usb32 and single-lane
+	 * for DWC_usb31 for super-speed-plus.
+	 */
+	if (dwc->maximum_speed == USB_SPEED_SUPER_PLUS) {
+		switch (dwc->max_ssp_rate) {
+		case USB_SSP_GEN_2x1:
+			if (hwparam_gen == DWC3_GHWPARAMS3_SSPHY_IFC_GEN1)
+				dev_warn(dev, "UDC only supports Gen 1\n");
+			break;
+		case USB_SSP_GEN_1x2:
+		case USB_SSP_GEN_2x2:
+			if (DWC3_IP_IS(DWC31))
+				dev_warn(dev, "UDC only supports single lane\n");
+			break;
+		case USB_SSP_GEN_UNKNOWN:
+		default:
+			switch (hwparam_gen) {
+			case DWC3_GHWPARAMS3_SSPHY_IFC_GEN2:
+				if (DWC3_IP_IS(DWC32))
+					dwc->max_ssp_rate = USB_SSP_GEN_2x2;
+				else
+					dwc->max_ssp_rate = USB_SSP_GEN_2x1;
+				break;
+			case DWC3_GHWPARAMS3_SSPHY_IFC_GEN1:
+				if (DWC3_IP_IS(DWC32))
+					dwc->max_ssp_rate = USB_SSP_GEN_1x2;
+				break;
+			}
+			break;
+		}
+	}
 }
 
 static int dwc3_probe(struct platform_device *pdev)

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

index ac290d896638..363c1b84f45f 100644
@@ -964,6 +964,7 @@ struct dwc3_scratchpad_array {
  * @nr_scratch: number of scratch buffers
  * @u1u2: only used on revisions <1.83a for workaround
  * @maximum_speed: maximum speed requested (mainly for testing purposes)
+ * @max_ssp_rate: SuperSpeed Plus maximum signaling rate and lane count
  * @ip: controller's ID
  * @revision: controller's version of an IP
  * @version_type: VERSIONTYPE register contents, a sub release of a revision

@@ -1127,6 +1128,7 @@ struct dwc3 {
 	u32			u1u2;
 	u32			maximum_speed;
 	u32			gadget_max_speed;
+	enum usb_ssp_rate	max_ssp_rate;
 
 	u32			ip;

drivers/usb/dwc3/gadget.c +1 -0

index ff14e5bbd152..494e4eca5460 100644
@@ -3889,6 +3889,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 				dwc->revision);
 
 	dwc->gadget->max_speed		= dwc->maximum_speed;
+	dwc->gadget->max_ssp_rate	= dwc->max_ssp_rate;
 
 	/*
 	 * REVISIT: Here we should clear all pending IRQs to be

Signature · 1 line

2.28.0

Thinh Nguyen

author maintainer

2021-01-14 02:54 UTC #20 lore raw

PATCH v6 9/11 usb: dwc3: gadget: Implement setting of SSP rate

In reply to Thinh Nguyen

Implement gadget ops udc_set_ssp_rate(). This allows the gadget/core driver to select SSP signaling rate and number of lanes to for DWC_usb32 controller.

Signed-off-by Thinh Nguyen <[email protected]>

---
Changes in v6:
- Update to use usb_ssp_rate enum
- Rebase on Greg's usb-testing branch
- Update to match with the latest flow in dwc3 for setting speed Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- None
Changes in v3:
- None
Changes in v2:
- None

 drivers/usb/dwc3/core.h   |  5 +++++
 drivers/usb/dwc3/gadget.c | 47 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 1 deletion(-)

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

index 363c1b84f45f..8f6e71052c6c 100644
@@ -386,6 +386,8 @@
 #define DWC3_GUCTL3_SPLITDISABLE		BIT(14)
 
 /* Device Configuration Register */
+#define DWC3_DCFG_NUMLANES(n)	(((n) & 0x3) << 30) /* DWC_usb32 only */
+
 #define DWC3_DCFG_DEVADDR(addr)	((addr) << 3)
 #define DWC3_DCFG_DEVADDR_MASK	DWC3_DCFG_DEVADDR(0x7f)

@@ -965,6 +967,8 @@ struct dwc3_scratchpad_array {
  * @u1u2: only used on revisions <1.83a for workaround
  * @maximum_speed: maximum speed requested (mainly for testing purposes)
  * @max_ssp_rate: SuperSpeed Plus maximum signaling rate and lane count
+ * @gadget_ssp_rate: Gadget driver's maximum supported SuperSpeed Plus signaling
+ *			rate and lane count.
  * @ip: controller's ID
  * @revision: controller's version of an IP
  * @version_type: VERSIONTYPE register contents, a sub release of a revision

@@ -1129,6 +1133,7 @@ struct dwc3 {
 	u32			maximum_speed;
 	u32			gadget_max_speed;
 	enum usb_ssp_rate	max_ssp_rate;
+	enum usb_ssp_rate	gadget_ssp_rate;
 
 	u32			ip;

drivers/usb/dwc3/gadget.c +46 -1

75 lines not on this page · raw message

Signature · 1 line

2.28.0

Felipe Balbi

2021-01-14 10:03 UTC #21 lore raw

Re: [PATCH v6 09/11] usb: dwc3: gadget: Implement setting of SSP rate

Hi,

Quoted 4 lines · Thinh Nguyen

drivers/usb/dwc3/gadget.c +0 -0

@@ -2476,6 +2506,17 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
 	spin_unlock_irqrestore(&dwc->lock, flags);
 }

drivers/usb/dwc3/gadget.c static void dwc3_gadget_set_speed(struct usb_gadget *g, · line 2518

Quoted 10 lines

drivers/usb/dwc3/gadget.c +10 -0

+static void dwc3_gadget_set_ssp_rate(struct usb_gadget *g,
+				     enum usb_ssp_rate rate)
+{
+	struct dwc3		*dwc = gadget_to_dwc(g);
+	unsigned long		flags;
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	dwc->gadget_ssp_rate = rate;
+	spin_unlock_irqrestore(&dwc->lock, flags);
+}

it would be best to make this return a value. If udc_set_ssp_rate() is called with invalid rate, UDC can notify core.

Signature · 1 line

balbi

Thinh Nguyen

author maintainer

2021-01-14 18:18 UTC #22 lore raw

Re: [PATCH v6 09/11] usb: dwc3: gadget: Implement setting of SSP rate

Quoted 7 lines · Felipe Balbi

Hi,

Quoted 4 lines · Thinh Nguyen

drivers/usb/dwc3/gadget.c +0 -0

@@ -2476,6 +2506,17 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
 	spin_unlock_irqrestore(&dwc->lock, flags);
 }

Quoted 13 lines

drivers/usb/dwc3/gadget.c static void dwc3_gadget_set_speed(struct usb_gadget *g, · line 2518
Quoted 10 lines
drivers/usb/dwc3/gadget.c +10 -0
+static void dwc3_gadget_set_ssp_rate(struct usb_gadget *g,
+				     enum usb_ssp_rate rate)
+{
+	struct dwc3		*dwc = gadget_to_dwc(g);
+	unsigned long		flags;
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	dwc->gadget_ssp_rate = rate;
+	spin_unlock_irqrestore(&dwc->lock, flags);
+}
it would be best to make this return a value. If udc_set_ssp_rate() is called with invalid rate, UDC can notify core.

The core should know what ssp rate the gadget supports via the gadget->max_ssp_rate capability field. Any rate beyond that is invalid. Is it necessary to have a return value here? This uses the same logic as udc_set_speed()

BR,
Thinh

Felipe Balbi

2021-01-15 11:14 UTC #23 lore raw

Re: [PATCH v6 09/11] usb: dwc3: gadget: Implement setting of SSP rate

Hi,

Quoted 22 lines · Thinh Nguyen

Quoted 7 lines · Felipe Balbi
Hi,
Quoted 4 lines · Thinh Nguyen
drivers/usb/dwc3/gadget.c +0 -0
@@ -2476,6 +2506,17 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
 	spin_unlock_irqrestore(&dwc->lock, flags);
 }
 
Quoted 13 lines
drivers/usb/dwc3/gadget.c static void dwc3_gadget_set_speed(struct usb_gadget *g, · line 2518
Quoted 10 lines
drivers/usb/dwc3/gadget.c +10 -0
+static void dwc3_gadget_set_ssp_rate(struct usb_gadget *g,
+				     enum usb_ssp_rate rate)
+{
+	struct dwc3		*dwc = gadget_to_dwc(g);
+	unsigned long		flags;
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	dwc->gadget_ssp_rate = rate;
+	spin_unlock_irqrestore(&dwc->lock, flags);
+}
it would be best to make this return a value. If udc_set_ssp_rate() is called with invalid rate, UDC can notify core.

Quoted 4 lines

The core should know what ssp rate the gadget supports via the gadget->max_ssp_rate capability field. Any rate beyond that is invalid. Is it necessary to have a return value here? This uses the same logic as udc_set_speed()

Yeah, I don't know what I had in mind when I made ->udc_set_speed() void. Then again, we know exactly who's calling it and we can guarantee that no invalid values will be passed. There's no way for, for example, userspace (via ffs) to call it with a bogus value.

Perhaps it's okay, but something to keep an eye for both ->udc_set_ssp_rate() and ->udc_set_speed().

Thanks for pointing me back at ->udc_set_speed().

Signature · 1 line

balbi

Thinh Nguyen

author maintainer

2021-01-14 02:54 UTC #24 lore raw

PATCH v6 10/11 usb: dwc3: gadget: Track connected SSP rate and lane count

In reply to Thinh Nguyen

Track the number of connected lanes and speed in corresponding enum usb_ssp_rate for SuperSpeed Plus capable device.

Signed-off-by Thinh Nguyen <[email protected]>

---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Update commmit message
- Update to report the rate and lane count in usb_ssp_rate enum Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- None
Changes in v3:
- Update commit with updated field name
- No longer use DWC3_LSM_5/10_GBPS macros
Changes in v2:
- Fix missing check for gen1x2 when writing to sublink speed attributes
- Minor fix in commit message (first commit sentence ended with comma)

 drivers/usb/dwc3/core.h   |  2 ++
 drivers/usb/dwc3/gadget.c | 23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+)

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

index 8f6e71052c6c..304912e84053 100644
@@ -461,6 +461,8 @@
 #define DWC3_DEVTEN_USBRSTEN		BIT(1)
 #define DWC3_DEVTEN_DISCONNEVTEN	BIT(0)
 
+#define DWC3_DSTS_CONNLANES(n)		(((n) >> 30) & 0x3) /* DWC_usb32 only */
+
 /* Device Status Register */
 #define DWC3_DSTS_DCNRD			BIT(29)

drivers/usb/dwc3/gadget.c +23 -0

index 7f06baac8b62..f46498f6d794 100644
@@ -2120,6 +2120,12 @@ static void __dwc3_gadget_set_speed(struct dwc3 *dwc)
 				reg |= DWC3_DCFG_SUPERSPEED_PLUS;
 		}
 	}
+
+	if (DWC3_IP_IS(DWC32) &&
+	    dwc->gadget_max_speed > USB_SPEED_UNKNOWN &&
+	    dwc->gadget_max_speed < USB_SPEED_SUPER_PLUS)
+		reg &= ~DWC3_DCFG_NUMLANES(~0);
+
 	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
 }

@@ -3370,12 +3376,18 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 	struct dwc3_ep		*dep;
 	int			ret;
 	u32			reg;
+	u8			lanes = 1;
 	u8			speed;
 
 	reg = dwc3_readl(dwc->regs, DWC3_DSTS);
 	speed = reg & DWC3_DSTS_CONNECTSPD;
 	dwc->speed = speed;
 
+	if (DWC3_IP_IS(DWC32))
+		lanes = DWC3_DSTS_CONNLANES(reg) + 1;
+
+	dwc->gadget->ssp_rate = USB_SSP_GEN_UNKNOWN;
+
 	/*
 	 * RAMClkSel is reset to 0 after USB reset, so it must be reprogrammed
 	 * each time on Connect Done.

@@ -3390,6 +3402,11 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 		dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
 		dwc->gadget->ep0->maxpacket = 512;
 		dwc->gadget->speed = USB_SPEED_SUPER_PLUS;
+
+		if (lanes > 1)
+			dwc->gadget->ssp_rate = USB_SSP_GEN_2x2;
+		else
+			dwc->gadget->ssp_rate = USB_SSP_GEN_2x1;
 		break;
 	case DWC3_DSTS_SUPERSPEED:
 		/*

@@ -3411,6 +3428,11 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 		dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
 		dwc->gadget->ep0->maxpacket = 512;
 		dwc->gadget->speed = USB_SPEED_SUPER;
+
+		if (lanes > 1) {
+			dwc->gadget->speed = USB_SPEED_SUPER_PLUS;
+			dwc->gadget->ssp_rate = USB_SSP_GEN_1x2;
+		}
 		break;
 	case DWC3_DSTS_HIGHSPEED:
 		dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(64);

@@ -3905,6 +3927,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 	dev->platform_data		= dwc;
 	dwc->gadget->ops		= &dwc3_gadget_ops;
 	dwc->gadget->speed		= USB_SPEED_UNKNOWN;
+	dwc->gadget->ssp_rate		= USB_SSP_GEN_UNKNOWN;
 	dwc->gadget->sg_supported	= true;
 	dwc->gadget->name		= "dwc3-gadget";
 	dwc->gadget->lpm_capable	= true;

Signature · 1 line

2.28.0

Thinh Nguyen

author maintainer

2021-01-14 02:54 UTC #25 lore raw

PATCH v6 11/11 usb: dwc3: gadget: Set speed only up to the max supported

In reply to Thinh Nguyen

The setting of the device speed should be limited by the device's maximum_speed. Check and prevent the driver from attempting to configure higher than the maximum_speed.

Signed-off-by Thinh Nguyen <[email protected]>

---
Changes in v6:
- Rebase on Greg's usb-testing branch
- Update to follow the latest change of dwc3 on Greg's usb-testing branch
- Update commit message
Changes in v5:
- Rebase on Felipe's testing/next branch
- Changed Signed-off-by email to match From: email header Changes in v4:
- None
Changes in v3:
- None
Changes in v2:
- None

 drivers/usb/dwc3/gadget.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

drivers/usb/dwc3/gadget.c +10 -5

index f46498f6d794..c9abc5e762b6 100644
@@ -2064,9 +2064,14 @@ static void __dwc3_gadget_set_ssp_rate(struct dwc3 *dwc)
 
 static void __dwc3_gadget_set_speed(struct dwc3 *dwc)
 {
+	enum usb_device_speed	speed;
 	u32			reg;
 
-	if (dwc->gadget_max_speed == USB_SPEED_SUPER_PLUS &&
+	speed = dwc->gadget_max_speed;
+	if (speed > dwc->maximum_speed)
+		speed = dwc->maximum_speed;
+
+	if (speed == USB_SPEED_SUPER_PLUS &&
 	    DWC3_IP_IS(DWC32)) {
 		__dwc3_gadget_set_ssp_rate(dwc);
 		return;

@@ -2092,7 +2097,7 @@ static void __dwc3_gadget_set_speed(struct dwc3 *dwc)
 	    !dwc->dis_metastability_quirk) {
 		reg |= DWC3_DCFG_SUPERSPEED;
 	} else {
-		switch (dwc->gadget_max_speed) {
+		switch (speed) {
 		case USB_SPEED_LOW:
 			reg |= DWC3_DCFG_LOWSPEED;
 			break;

@@ -2112,7 +2117,7 @@ static void __dwc3_gadget_set_speed(struct dwc3 *dwc)
 				reg |= DWC3_DCFG_SUPERSPEED_PLUS;
 			break;
 		default:
-			dev_err(dwc->dev, "invalid speed (%d)\n", dwc->gadget_max_speed);
+			dev_err(dwc->dev, "invalid speed (%d)\n", speed);
 
 			if (DWC3_IP_IS(DWC3))
 				reg |= DWC3_DCFG_SUPERSPEED;

@@ -2122,8 +2127,8 @@ static void __dwc3_gadget_set_speed(struct dwc3 *dwc)
 	}
 
 	if (DWC3_IP_IS(DWC32) &&
-	    dwc->gadget_max_speed > USB_SPEED_UNKNOWN &&
-	    dwc->gadget_max_speed < USB_SPEED_SUPER_PLUS)
+	    speed > USB_SPEED_UNKNOWN &&
+	    speed < USB_SPEED_SUPER_PLUS)
 		reg &= ~DWC3_DCFG_NUMLANES(~0);
 
 	dwc3_writel(dwc->regs, DWC3_DCFG, reg);

Signature · 1 line

2.28.0

Greg Kroah-Hartman

maintainer

2021-01-18 17:43 UTC #26 lore raw

In reply to Thinh Nguyen

Quoted 5 lines · Thinh Nguyen

A USB SuperSpeed Plus device may operate at different speed and lane count (i.e. gen2x2, gen1x2, or gen2x1). The DWC_usb32 IP supports SuperSpeed Plus gen2x2. To support this, this series update a few things to the USB gadget stack and dwc3 driver:

Quoted 8 lines

* Accept and parse new maximum_speed devicetree property strings
* Introduce enum usb_ssp_rate to describe the speed in SuperSpeed Plus genXxY
* Capture the connected and max supported usb_ssp_rate
* Report the device sublink speeds base on the usb_ssp_rate in the BOS descriptor
* Introduce gadget ops to select SuperSpeed Plus various transfer rate and lane count
* Update dwc3 driver to support the above changes

I've taken the first 5 patches now, I'll wait for the DT maintainers and dwc3 maintainers to review the rest before I can take them. Feel free to rebase and resend the smaller set of patches if that makes it easier for them to review.

thanks,

greg k-h

Thinh Nguyen

author maintainer

2021-01-20 01:41 UTC #27 lore raw

Quoted 14 lines · Greg Kroah-Hartman

Quoted 5 lines · Thinh Nguyen
A USB SuperSpeed Plus device may operate at different speed and lane count (i.e. gen2x2, gen1x2, or gen2x1). The DWC_usb32 IP supports SuperSpeed Plus gen2x2. To support this, this series update a few things to the USB gadget stack and dwc3 driver:
Quoted 8 lines
* Accept and parse new maximum_speed devicetree property strings
* Introduce enum usb_ssp_rate to describe the speed in SuperSpeed Plus genXxY
* Capture the connected and max supported usb_ssp_rate
* Report the device sublink speeds base on the usb_ssp_rate in the BOS descriptor
* Introduce gadget ops to select SuperSpeed Plus various transfer rate and lane count
* Update dwc3 driver to support the above changes

Quoted 8 lines

I've taken the first 5 patches now, I'll wait for the DT maintainers and dwc3 maintainers to review the rest before I can take them. Feel free to rebase and resend the smaller set of patches if that makes it easier for them to review.
thanks,
greg k-h

Just rebased and resent.

Thanks!
Thinh