usb: Handle different sublink speeds
Subsystems (3)
Thinh Nguyen
maintainercover usb: Handle different sublink speeds
A USB super-speed-plus device may operate at different sublink speed and lane count (e.g. gen2x2, gen1x2, or gen2x1). The usb gadget stack needs to be able to handle a couple things:
1) Report the sublink speed attributes the device support
2) Select the sublink speed attribute
This series introduces sublink speed attribute structure to ch9.h to capture the device capability of the gadget. It also introduces a new gadget ops udc_set_num_lanes_and_speed to select a specific sublink speed.
DWC3 needs this support for DWC_usb32 IP. Implement the new changes for DWC3.
Thinh Nguyen (11):
usb: ch9: Add sublink speed struct
usb: gadget: composite: Avoid using magic numbers
usb: gadget: Expose sublink speed attributes
usb: gadget: Set max speed for SSP devices
usb: composite: Properly report sublink speed
usb: devicetree: dwc3: Introduce num-lanes and lsm
usb: dwc3: Initialize lane count and sublink speed
usb: dwc3: gadget: Report sublink speed capability
usb: dwc3: gadget: Implement setting of sublink speed
usb: dwc3: gadget: Track connected lane and sublink speed
usb: dwc3: gadget: Set speed only up to the max supported
Documentation/devicetree/bindings/usb/dwc3.txt | 9 ++ drivers/usb/dwc3/core.c | 64 ++++++++++++ drivers/usb/dwc3/core.h | 18 ++++ drivers/usb/dwc3/gadget.c | 135 ++++++++++++++++++++++++- drivers/usb/gadget/composite.c | 81 ++++++++++----- drivers/usb/gadget/udc/core.c | 24 ++++- include/linux/usb/gadget.h | 23 +++++ include/uapi/linux/usb/ch9.h | 42 ++++++++ 8 files changed, 360 insertions(+), 36 deletions(-)
Signature · 1 line
2.11.0
Thinh Nguyen
maintainerPATCH 1/11 usb: ch9: Add sublink speed struct
USB 3.2 specification supports dual-lane for super-speed-plus. USB devices may operate at different sublink speeds. To avoid using magic numbers and capture the sublink speed better, introduce the usb_sublink_speed structure and various sublink speed attribute enum.
See SSP BOS descriptor in USB 3.2 specification section 9.6.2.5
--- include/uapi/linux/usb/ch9.h | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
include/uapi/linux/usb/ch9.h +42 -0
@@ -1145,6 +1145,48 @@ enum usb_device_speed { USB_SPEED_SUPER_PLUS, /* usb 3.1 */ }; +/* USB 3.2 sublink speed attributes */ + +enum usb_lane_speed_exponent { +USB_LSE_BPS =0 , +USB_LSE_KBPS =1 , +USB_LSE_MBPS =2 , +USB_LSE_GBPS =3 , +}; + +enum usb_sublink_type { +USB_ST_SYMMETRIC_RX =0 , +USB_ST_ASYMMETRIC_RX =1 , +USB_ST_SYMMETRIC_TX =2 , +USB_ST_ASYMMETRIC_TX =3 , +}; + +enum usb_link_protocol { +USB_LP_SS =0 , +USB_LP_SSP =1 , +}; + +/** +* struct usb_sublink_speed - sublink speed attribute +* @id: sublink speed attribute ID (SSID) +* @mantissa: lane speed mantissa +* @exponent: lane speed exponent +* @sublink type: sublink type +* @protocol: sublink protocol +* +* Super-speed-plus supports multiple lanes. Use the sublink speed attributes to +* describe the sublink speed. +* +* See USB 3.2 spec section 9.6.2.6 for super-speed-plus capability for more +* information. +*/ +struct usb_sublink_speed { +u8 id; +u16 mantissa; +enum usb_lane_speed_exponent exponent; +enum usb_sublink_type type; +enum usb_link_protocol protocol; +};enum usb_device_state {/* NOTATTACHED isn't in the USB spec, and this state acts
Signature · 1 line
2.11.0
Greg Kroah-Hartman
maintainerRe: [PATCH 01/11] usb: ch9: Add sublink speed struct
Quoted 48 lines · Thinh Nguyen
USB 3.2 specification supports dual-lane for super-speed-plus. USB devices may operate at different sublink speeds. To avoid using magic numbers and capture the sublink speed better, introduce the
usb_sublink_speedstructure and various sublink speed attribute enum.See SSP BOS descriptor in USB 3.2 specification section 9.6.2.5
Signed-off-by Thinh Nguyen <[email protected]>--- include/uapi/linux/usb/ch9.h | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)include/uapi/linux/usb/ch9.h +28
-0@@ -1145,6 +1145,48 @@ enum usb_device_speed { USB_SPEED_SUPER_PLUS, /* usb 3.1 */ }; +/* USB 3.2 sublink speed attributes */ + +enum usb_lane_speed_exponent { +USB_LSE_BPS =0 , +USB_LSE_KBPS =1 , +USB_LSE_MBPS =2 , +USB_LSE_GBPS =3 , +}; + +enum usb_sublink_type { +USB_ST_SYMMETRIC_RX =0 , +USB_ST_ASYMMETRIC_RX =1 , +USB_ST_SYMMETRIC_TX =2 , +USB_ST_ASYMMETRIC_TX =3 , +}; + +enum usb_link_protocol { +USB_LP_SS =0 , +USB_LP_SSP =1 , +}; + +/** +* struct usb_sublink_speed - sublink speed attribute +* @id: sublink speed attribute ID (SSID) +* @mantissa: lane speed mantissa +* @exponent: lane speed exponent +* @sublink type: sublink type +* @protocol: sublink protocol
include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1185
Quoted 10 lines
include/uapi/linux/usb/ch9.h +10
-0+* +* Super-speed-plus supports multiple lanes. Use the sublink speed attributes to +* describe the sublink speed. +* +* See USB 3.2 spec section 9.6.2.6 for super-speed-plus capability for more +* information. +*/ +struct usb_sublink_speed { +u8 id; +u16 mantissa;
You have to use the proper data types for crossing the user/kernel boundry here. That would be __u8 and __u16, right?
include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1188
Quoted 3 lines
include/uapi/linux/usb/ch9.h +3
-0+enum usb_lane_speed_exponent exponent; +enum usb_sublink_type type; +enum usb_link_protocol protocol;
Are you _sure_ that an enum is the correct size for these fields? How can you guarantee this? We do not use enums in this way for any other field in this file for a reason...
And did you look at the layout of this structure to verify it actually matches what is on the wire with USB? I think you need to add a packed attribute to guarantee it.
thanks,
greg k-h
Quoted 7 lines
include/uapi/linux/usb/ch9.h +1
-0+};enum usb_device_state {/* NOTATTACHED isn't in the USB spec, and this state acts Signature · 1 line
2.11.0
Thinh Nguyen
maintainerRe: [PATCH 01/11] usb: ch9: Add sublink speed struct
Quoted 69 lines · Greg Kroah-Hartman
Quoted 48 lines · Thinh Nguyen
USB 3.2 specification supports dual-lane for super-speed-plus. USB devices may operate at different sublink speeds. To avoid using magic numbers and capture the sublink speed better, introduce the
usb_sublink_speedstructure and various sublink speed attribute enum.See SSP BOS descriptor in USB 3.2 specification section 9.6.2.5
Signed-off-by Thinh Nguyen <[email protected]>--- include/uapi/linux/usb/ch9.h | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)include/uapi/linux/usb/ch9.h +28
-0@@ -1145,6 +1145,48 @@ enum usb_device_speed { USB_SPEED_SUPER_PLUS, /* usb 3.1 */ }; +/* USB 3.2 sublink speed attributes */ + +enum usb_lane_speed_exponent { +USB_LSE_BPS =0 , +USB_LSE_KBPS =1 , +USB_LSE_MBPS =2 , +USB_LSE_GBPS =3 , +}; + +enum usb_sublink_type { +USB_ST_SYMMETRIC_RX =0 , +USB_ST_ASYMMETRIC_RX =1 , +USB_ST_SYMMETRIC_TX =2 , +USB_ST_ASYMMETRIC_TX =3 , +}; + +enum usb_link_protocol { +USB_LP_SS =0 , +USB_LP_SSP =1 , +}; + +/** +* struct usb_sublink_speed - sublink speed attribute +* @id: sublink speed attribute ID (SSID) +* @mantissa: lane speed mantissa +* @exponent: lane speed exponent +* @sublink type: sublink type +* @protocol: sublink protocol include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1185
Quoted 10 lines
include/uapi/linux/usb/ch9.h +10
-0+* +* Super-speed-plus supports multiple lanes. Use the sublink speed attributes to +* describe the sublink speed. +* +* See USB 3.2 spec section 9.6.2.6 for super-speed-plus capability for more +* information. +*/ +struct usb_sublink_speed { +u8 id; +u16 mantissa;You have to use the proper data types for crossing the user/kernel boundry here. That would be
__u8and__u16, right?include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1188
Quoted 3 lines
include/uapi/linux/usb/ch9.h +3
-0+enum usb_lane_speed_exponent exponent; +enum usb_sublink_type type; +enum usb_link_protocol protocol;Are you _sure_ that an enum is the correct size for these fields? How can you guarantee this? We do not use enums in this way for any other field in this file for a reason...
Quoted 3 lines
And did you look at the layout of this structure to verify it actually matches what is on the wire with USB? I think you need to add a packed attribute to guarantee it.
This struct is not intended to be packed to be sent over the bus. It's a simple struct for host and gadget driver use only. I intended to use enum to make it more clear not to be used that way. From your question, it's obviously not clear.
Otherwise, it may look something like this:
struct usb_sublink_speed {
__u8 ssid:4;
__u8 lse:2;
__u8 st:2;
__u8 rsvd:6;
__u8 lp:2;
__le16 lsm;
} __attribute__((packed));
Let me know how you'd like to handle it.
Thanks,
Thinh
Greg Kroah-Hartman
maintainerRe: [PATCH 01/11] usb: ch9: Add sublink speed struct
Quoted 70 lines · Thinh Nguyen
Quoted 69 lines · Greg Kroah-Hartman
Quoted 48 lines · Thinh Nguyen
USB 3.2 specification supports dual-lane for super-speed-plus. USB devices may operate at different sublink speeds. To avoid using magic numbers and capture the sublink speed better, introduce the
usb_sublink_speedstructure and various sublink speed attribute enum.See SSP BOS descriptor in USB 3.2 specification section 9.6.2.5
Signed-off-by Thinh Nguyen <[email protected]>--- include/uapi/linux/usb/ch9.h | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)include/uapi/linux/usb/ch9.h +28
-0@@ -1145,6 +1145,48 @@ enum usb_device_speed { USB_SPEED_SUPER_PLUS, /* usb 3.1 */ }; +/* USB 3.2 sublink speed attributes */ + +enum usb_lane_speed_exponent { +USB_LSE_BPS =0 , +USB_LSE_KBPS =1 , +USB_LSE_MBPS =2 , +USB_LSE_GBPS =3 , +}; + +enum usb_sublink_type { +USB_ST_SYMMETRIC_RX =0 , +USB_ST_ASYMMETRIC_RX =1 , +USB_ST_SYMMETRIC_TX =2 , +USB_ST_ASYMMETRIC_TX =3 , +}; + +enum usb_link_protocol { +USB_LP_SS =0 , +USB_LP_SSP =1 , +}; + +/** +* struct usb_sublink_speed - sublink speed attribute +* @id: sublink speed attribute ID (SSID) +* @mantissa: lane speed mantissa +* @exponent: lane speed exponent +* @sublink type: sublink type +* @protocol: sublink protocol include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1185
Quoted 10 lines
include/uapi/linux/usb/ch9.h +10
-0+* +* Super-speed-plus supports multiple lanes. Use the sublink speed attributes to +* describe the sublink speed. +* +* See USB 3.2 spec section 9.6.2.6 for super-speed-plus capability for more +* information. +*/ +struct usb_sublink_speed { +u8 id; +u16 mantissa;You have to use the proper data types for crossing the user/kernel boundry here. That would be
__u8and__u16, right?include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1188
Quoted 3 lines
include/uapi/linux/usb/ch9.h +3
-0+enum usb_lane_speed_exponent exponent; +enum usb_sublink_type type; +enum usb_link_protocol protocol;Are you _sure_ that an enum is the correct size for these fields? How can you guarantee this? We do not use enums in this way for any other field in this file for a reason...
Quoted 8 lines
Quoted 3 lines
And did you look at the layout of this structure to verify it actually matches what is on the wire with USB? I think you need to add a packed attribute to guarantee it.
This struct is not intended to be packed to be sent over the bus. It's a simple struct for host and gadget driver use only. I intended to use enum to make it more clear not to be used that way. From your question, it's obviously not clear.
Then why are you putting it in this file? This file is only for things that are described in the USB spec that need to cross the user/kernel boundry.
Quoted 7 lines
Otherwise, it may look something like this:
structusb_sublink_speed{
__u8 ssid:4;
__u8 lse:2;
__u8 st:2;
__u8 rsvd:6;
__u8 lp:2;
Are you sure those bit fields will work on big-endian systems?
Quoted 2 lines
__le16 lsm;
} __attribute__((packed));
Do you need to read this from a device?
thanks,
greg k-h
Thinh Nguyen
maintainerRe: [PATCH 01/11] usb: ch9: Add sublink speed struct
Quoted 74 lines · Greg Kroah-Hartman
Quoted 73 lines · Thinh Nguyen
Quoted 72 lines · Greg Kroah-Hartman
Quoted 48 lines · Thinh Nguyen
USB 3.2 specification supports dual-lane for super-speed-plus. USB devices may operate at different sublink speeds. To avoid using magic numbers and capture the sublink speed better, introduce the
usb_sublink_speedstructure and various sublink speed attribute enum.See SSP BOS descriptor in USB 3.2 specification section 9.6.2.5
Signed-off-by Thinh Nguyen <[email protected]>--- include/uapi/linux/usb/ch9.h | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)include/uapi/linux/usb/ch9.h +28
-0@@ -1145,6 +1145,48 @@ enum usb_device_speed { USB_SPEED_SUPER_PLUS, /* usb 3.1 */ }; +/* USB 3.2 sublink speed attributes */ + +enum usb_lane_speed_exponent { +USB_LSE_BPS =0 , +USB_LSE_KBPS =1 , +USB_LSE_MBPS =2 , +USB_LSE_GBPS =3 , +}; + +enum usb_sublink_type { +USB_ST_SYMMETRIC_RX =0 , +USB_ST_ASYMMETRIC_RX =1 , +USB_ST_SYMMETRIC_TX =2 , +USB_ST_ASYMMETRIC_TX =3 , +}; + +enum usb_link_protocol { +USB_LP_SS =0 , +USB_LP_SSP =1 , +}; + +/** +* struct usb_sublink_speed - sublink speed attribute +* @id: sublink speed attribute ID (SSID) +* @mantissa: lane speed mantissa +* @exponent: lane speed exponent +* @sublink type: sublink type +* @protocol: sublink protocol include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1185
Quoted 10 lines
include/uapi/linux/usb/ch9.h +10
-0+* +* Super-speed-plus supports multiple lanes. Use the sublink speed attributes to +* describe the sublink speed. +* +* See USB 3.2 spec section 9.6.2.6 for super-speed-plus capability for more +* information. +*/ +struct usb_sublink_speed { +u8 id; +u16 mantissa;You have to use the proper data types for crossing the user/kernel boundry here. That would be
__u8and__u16, right?include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1188
Quoted 3 lines
include/uapi/linux/usb/ch9.h +3
-0+enum usb_lane_speed_exponent exponent; +enum usb_sublink_type type; +enum usb_link_protocol protocol;Are you _sure_ that an enum is the correct size for these fields? How can you guarantee this? We do not use enums in this way for any other field in this file for a reason...
And did you look at the layout of this structure to verify it actually matches what is on the wire with USB? I think you need to add a packed attribute to guarantee it.
Quoted 7 lines
Quoted 4 lines
This struct is not intended to be packed to be sent over the bus. It's a simple struct for host and gadget driver use only. I intended to use enum to make it more clear not to be used that way. From your question, it's obviously not clear.
Then why are you putting it in this file? This file is only for things that are described in the USB spec that need to cross the user/kernel boundry.
Ok, it seemed to fit here. I'll place it under /include/linux/usb.h then?
Quoted 9 lines
Quoted 8 lines
Otherwise, it may look something like this:
structusb_sublink_speed{
__u8 ssid:4;
__u8 lse:2;
__u8 st:2;
__u8 rsvd:6;
__u8 lp:2;Are you sure those bit fields will work on big-endian systems?
No. Because of the way the bitfields are placed, it's a path to unnecessary headache/bugs with boundary and endianness checks. That's why I decided to go with the other solution.
Thanks,
Thinh
Greg Kroah-Hartman
maintainerRe: [PATCH 01/11] usb: ch9: Add sublink speed struct
Quoted 79 lines · Thinh Nguyen
Quoted 78 lines · Greg Kroah-Hartman
Quoted 77 lines · Thinh Nguyen
Quoted 69 lines · Greg Kroah-Hartman
Quoted 48 lines · Thinh Nguyen
USB 3.2 specification supports dual-lane for super-speed-plus. USB devices may operate at different sublink speeds. To avoid using magic numbers and capture the sublink speed better, introduce the
usb_sublink_speedstructure and various sublink speed attribute enum.See SSP BOS descriptor in USB 3.2 specification section 9.6.2.5
Signed-off-by Thinh Nguyen <[email protected]>--- include/uapi/linux/usb/ch9.h | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)include/uapi/linux/usb/ch9.h +28
-0@@ -1145,6 +1145,48 @@ enum usb_device_speed { USB_SPEED_SUPER_PLUS, /* usb 3.1 */ }; +/* USB 3.2 sublink speed attributes */ + +enum usb_lane_speed_exponent { +USB_LSE_BPS =0 , +USB_LSE_KBPS =1 , +USB_LSE_MBPS =2 , +USB_LSE_GBPS =3 , +}; + +enum usb_sublink_type { +USB_ST_SYMMETRIC_RX =0 , +USB_ST_ASYMMETRIC_RX =1 , +USB_ST_SYMMETRIC_TX =2 , +USB_ST_ASYMMETRIC_TX =3 , +}; + +enum usb_link_protocol { +USB_LP_SS =0 , +USB_LP_SSP =1 , +}; + +/** +* struct usb_sublink_speed - sublink speed attribute +* @id: sublink speed attribute ID (SSID) +* @mantissa: lane speed mantissa +* @exponent: lane speed exponent +* @sublink type: sublink type +* @protocol: sublink protocol include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1185
Quoted 10 lines
include/uapi/linux/usb/ch9.h +10
-0+* +* Super-speed-plus supports multiple lanes. Use the sublink speed attributes to +* describe the sublink speed. +* +* See USB 3.2 spec section 9.6.2.6 for super-speed-plus capability for more +* information. +*/ +struct usb_sublink_speed { +u8 id; +u16 mantissa;You have to use the proper data types for crossing the user/kernel boundry here. That would be
__u8and__u16, right?include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1188
Quoted 3 lines
include/uapi/linux/usb/ch9.h +3
-0+enum usb_lane_speed_exponent exponent; +enum usb_sublink_type type; +enum usb_link_protocol protocol;Are you _sure_ that an enum is the correct size for these fields? How can you guarantee this? We do not use enums in this way for any other field in this file for a reason...
Quoted 3 lines
And did you look at the layout of this structure to verify it actually matches what is on the wire with USB? I think you need to add a packed attribute to guarantee it.
This struct is not intended to be packed to be sent over the bus. It's a simple struct for host and gadget driver use only. I intended to use enum to make it more clear not to be used that way. From your question, it's obviously not clear.
Quoted 5 lines
Quoted 3 lines
Then why are you putting it in this file? This file is only for things that are described in the USB spec that need to cross the user/kernel boundry.
Ok, it seemed to fit here. I'll place it under /include/linux/usb.h then?
include/linux/usb/ch9.h perhaps?
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Otherwise, it may look something like this:
structusb_sublink_speed{
__u8 ssid:4;
__u8 lse:2;
__u8 st:2;
__u8 rsvd:6;
__u8 lp:2;
Quoted 5 lines
Quoted 1 line
Are you sure those bit fields will work on big-endian systems?
No. Because of the way the bitfields are placed, it's a path to unnecessary headache/bugs with boundary and endianness checks. That's why I decided to go with the other solution.
That's good, but again, this is a uapi file, not a "normal" include file in the kernel, you have to be careful about this.
greg k-h
Thinh Nguyen
maintainerRe: [PATCH 01/11] usb: ch9: Add sublink speed struct
Quoted 80 lines · Greg Kroah-Hartman
Quoted 79 lines · Thinh Nguyen
Quoted 78 lines · Greg Kroah-Hartman
Quoted 77 lines · Thinh Nguyen
Quoted 69 lines · Greg Kroah-Hartman
Quoted 48 lines · Thinh Nguyen
USB 3.2 specification supports dual-lane for super-speed-plus. USB devices may operate at different sublink speeds. To avoid using magic numbers and capture the sublink speed better, introduce the
usb_sublink_speedstructure and various sublink speed attribute enum.See SSP BOS descriptor in USB 3.2 specification section 9.6.2.5
Signed-off-by Thinh Nguyen <[email protected]>--- include/uapi/linux/usb/ch9.h | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)include/uapi/linux/usb/ch9.h +28
-0@@ -1145,6 +1145,48 @@ enum usb_device_speed { USB_SPEED_SUPER_PLUS, /* usb 3.1 */ }; +/* USB 3.2 sublink speed attributes */ + +enum usb_lane_speed_exponent { +USB_LSE_BPS =0 , +USB_LSE_KBPS =1 , +USB_LSE_MBPS =2 , +USB_LSE_GBPS =3 , +}; + +enum usb_sublink_type { +USB_ST_SYMMETRIC_RX =0 , +USB_ST_ASYMMETRIC_RX =1 , +USB_ST_SYMMETRIC_TX =2 , +USB_ST_ASYMMETRIC_TX =3 , +}; + +enum usb_link_protocol { +USB_LP_SS =0 , +USB_LP_SSP =1 , +}; + +/** +* struct usb_sublink_speed - sublink speed attribute +* @id: sublink speed attribute ID (SSID) +* @mantissa: lane speed mantissa +* @exponent: lane speed exponent +* @sublink type: sublink type +* @protocol: sublink protocol include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1185
Quoted 10 lines
include/uapi/linux/usb/ch9.h +10
-0+* +* Super-speed-plus supports multiple lanes. Use the sublink speed attributes to +* describe the sublink speed. +* +* See USB 3.2 spec section 9.6.2.6 for super-speed-plus capability for more +* information. +*/ +struct usb_sublink_speed { +u8 id; +u16 mantissa;You have to use the proper data types for crossing the user/kernel boundry here. That would be
__u8and__u16, right?include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1188
Quoted 3 lines
include/uapi/linux/usb/ch9.h +3
-0+enum usb_lane_speed_exponent exponent; +enum usb_sublink_type type; +enum usb_link_protocol protocol;Are you _sure_ that an enum is the correct size for these fields? How can you guarantee this? We do not use enums in this way for any other field in this file for a reason...
Quoted 3 lines
And did you look at the layout of this structure to verify it actually matches what is on the wire with USB? I think you need to add a packed attribute to guarantee it.
This struct is not intended to be packed to be sent over the bus. It's a simple struct for host and gadget driver use only. I intended to use enum to make it more clear not to be used that way. From your question, it's obviously not clear.
Quoted 5 lines
Quoted 4 lines
Quoted 3 lines
Then why are you putting it in this file? This file is only for things that are described in the USB spec that need to cross the user/kernel boundry.
Ok, it seemed to fit here. I'll place it under /include/linux/usb.h then?
include/linux/usb/ch9.hperhaps?
Sure. I'll place it there.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Otherwise, it may look something like this:
struct usb_sublink_speed { __u8 ssid:4; __u8 lse:2; __u8 st:2; __u8 rsvd:6; __u8 lp:2;
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Are you sure those bit fields will work on big-endian systems?
No. Because of the way the bitfields are placed, it's a path to unnecessary headache/bugs with boundary and endianness checks. That's why I decided to go with the other solution.
That's good, but again, this is a uapi file, not a "normal" include file in the kernel, you have to be careful about this.
Will do. Thanks for the quick responses.
BR,
Thinh
kernel test robot
build botRe: [PATCH 01/11] usb: ch9: Add sublink speed struct
In reply to Thinh NguyenThank you for the patch! Yet something to improve:
Hi Thinh,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on balbi-usb/testing/next]
[also build test ERROR on usb/usb-testing peter.chen-usb/ci-for-usb-next linus/master v5.8-rc5 next-20200716] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Thinh-Nguyen/usb-Handle-different-sublink-speeds/20200717-060144 base: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next config: x86_64-randconfig-a005-20200717 (attached as .config) compiler: gcc-9 (Debian 9.3.0-14) 9.3.0 reproduce (this is a W=1 build): # save the attached .config to linux build tree make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
In file included from ./usr/include/linux/usb/g_uvc.h:13,
from <command-line>:32:
>> ./usr/include/linux/usb/ch9.h:1184:2: error: unknown type name 'u8'
1184 | u8 id;
| ^~
>> ./usr/include/linux/usb/ch9.h:1185:2: error: unknown type name 'u16'
1185 | u16 mantissa;
| ^~~---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Re: [PATCH 01/11] usb: ch9: Add sublink speed struct
In reply to Thinh NguyenHello!
Quoted 37 lines · Thinh Nguyen
USB 3.2 specification supports dual-lane for super-speed-plus. USB devices may operate at different sublink speeds. To avoid using magic numbers and capture the sublink speed better, introduce the
usb_sublink_speedstructure and various sublink speed attribute enum.See SSP BOS descriptor in USB 3.2 specification section 9.6.2.5
Signed-off-by Thinh Nguyen <[email protected]>--- include/uapi/linux/usb/ch9.h | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)include/uapi/linux/usb/ch9.h +17
-0@@ -1145,6 +1145,48 @@ enum usb_device_speed { USB_SPEED_SUPER_PLUS, /* usb 3.1 */ }; +/* USB 3.2 sublink speed attributes */ + +enum usb_lane_speed_exponent { +USB_LSE_BPS =0 , +USB_LSE_KBPS =1 , +USB_LSE_MBPS =2 , +USB_LSE_GBPS =3 , +}; + +enum usb_sublink_type { +USB_ST_SYMMETRIC_RX =0 , +USB_ST_ASYMMETRIC_RX =1 , +USB_ST_SYMMETRIC_TX =2 , +USB_ST_ASYMMETRIC_TX =3 , +}; + +enum usb_link_protocol {
include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1174
Quoted 10 lines
include/uapi/linux/usb/ch9.h +10
-0+USB_LP_SS =0 , + USB_LP_SSP = 1, +}; + +/** +* struct usb_sublink_speed - sublink speed attribute +* @id: sublink speed attribute ID (SSID) +* @mantissa: lane speed mantissa +* @exponent: lane speed exponent +* @sublink type: sublink type
It's called just 'type' below.
Quoted 8 lines
include/uapi/linux/usb/ch9.h +8
-0+* @protocol: sublink protocol +* +* Super-speed-plus supports multiple lanes. Use the sublink speed attributes to +* describe the sublink speed. +* +* See USB 3.2 spec section 9.6.2.6 for super-speed-plus capability for more +* information. +*/
include/uapi/linux/usb/ch9.h enum usb_device_speed { · line 1192
Quoted 10 lines
include/uapi/linux/usb/ch9.h +7
-0+struct usb_sublink_speed { +u8 id; +u16 mantissa; +enum usb_lane_speed_exponent exponent; +enum usb_sublink_type type; +enum usb_link_protocol protocol; +};enum usb_device_state {/* NOTATTACHED isn't in the USB spec, and this state acts
MBR, Sergei
Thinh Nguyen
maintainerRe: [PATCH 01/11] usb: ch9: Add sublink speed struct
Quoted 45 lines · Sergei Shtylyov
Hello!
Quoted 41 lines · Thinh Nguyen
USB 3.2 specification supports dual-lane for super-speed-plus. USB devices may operate at different sublink speeds. To avoid using magic numbers and capture the sublink speed better, introduce the
usb_sublink_speedstructure and various sublink speed attribute enum.See SSP BOS descriptor in USB 3.2 specification section 9.6.2.5
Signed-off-by Thinh Nguyen <[email protected]>---
include/uapi/linux/usb/ch9.h| 42
++++++++++++++++++++++++++++++++++++++++++1 file changed, 42 insertions(+)include/uapi/linux/usb/ch9.h +0
-0@@ -1145,6 +1145,48 @@ enum usb_device_speed {USB_SPEED_SUPER_PLUS, /* usb 3.1 */ };
+/* USB 3.2 sublink speed attributes */include/uapi/linux/usb/ch9.h +20
-0+ +enum usb_lane_speed_exponent { + USB_LSE_BPS = 0, + USB_LSE_KBPS = 1, + USB_LSE_MBPS = 2, + USB_LSE_GBPS = 3, +}; + +enum usb_sublink_type { + USB_ST_SYMMETRIC_RX = 0, + USB_ST_ASYMMETRIC_RX = 1, + USB_ST_SYMMETRIC_TX = 2, + USB_ST_ASYMMETRIC_TX = 3, +}; + +enum usb_link_protocol { + USB_LP_SS = 0, + USB_LP_SSP = 1, +}; +
Quoted 9 lines
Quoted 6 lines
+/**
+ * structusb_sublink_speed- sublink speed attribute
+ * @id: sublink speed attribute ID (SSID)
+ * @mantissa: lane speed mantissa
+ * @exponent: lane speed exponent
+ * @sublink type: sublink typeIt's called just 'type' below.
Will fix.
Thanks,
Thinh
kernel test robot
build botRe: [PATCH 01/11] usb: ch9: Add sublink speed struct
In reply to Thinh NguyenThank you for the patch! Yet something to improve:
Hi Thinh,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on balbi-usb/testing/next]
[also build test ERROR on usb/usb-testing peter.chen-usb/ci-for-usb-next linus/master v5.8-rc5 next-20200717] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Thinh-Nguyen/usb-Handle-different-sublink-speeds/20200717-060144 base: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next config: x86_64-allyesconfig (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project ed6b578040a85977026c93bf4188f996148f3218)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
In file included from <built-in>:1:
>> ./usr/include/linux/usb/ch9.h:1184:2: error: unknown type name 'u8'
u8 id;
^
>> ./usr/include/linux/usb/ch9.h:1185:2: error: unknown type name 'u16'
u16 mantissa;
^2 errors generated.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Thinh Nguyen
maintainerPATCH 2/11 usb: gadget: composite: Avoid using magic numbers
In reply to Thinh NguyenMagic numbers are difficult to read. Use macros for super-speed-plus BOS descriptor attributes in the composite driver. They're self-documented. So there's no need to provide comments as we did previously for the magic numbers.
--- drivers/usb/gadget/composite.c | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-)
drivers/usb/gadget/composite.c +20 -21
@@ -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>
@@ -743,32 +744,30 @@ 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_LSE_GBPS ) | +FIELD_PREP (USB_SSP_SUBLINK_SPEED_ST , +USB_ST_SYMMETRIC_RX ) | +FIELD_PREP (USB_SSP_SUBLINK_SPEED_LP ,USB_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_LSE_GBPS ) | +FIELD_PREP (USB_SSP_SUBLINK_SPEED_ST , +USB_ST_SYMMETRIC_TX ) | +FIELD_PREP (USB_SSP_SUBLINK_SPEED_LP ,USB_LP_SSP ) | +FIELD_PREP (USB_SSP_SUBLINK_SPEED_LSM ,10 )); }return le16_to_cpu (bos->wTotalLength);
Signature · 1 line
2.11.0
Thinh Nguyen
maintainerPATCH 3/11 usb: gadget: Expose sublink speed attributes
In reply to Thinh NguyenThe USB 3.2 specification supports dual-lane and different transfer rates for super-speed-plus. Devices operating in super-speed-plus can be gen2x1, gen1x2, or gen2x2.
A gadget driver may need to know the gadget's sublink speeds to properly setup its transfer requests and describe its capability in its descriptors. To describe the transfer rate in super-speed-plus fully, let's expose the lane count and sublink speed attributes when operating in super-speed-plus.
--- include/linux/usb/gadget.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
include/linux/usb/gadget.h +20 -0
@@ -338,6 +338,15 @@ 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. +* @num_lanes: Number of lanes in use. +* @max_num_lanes: Maximum number of lanes the UDC supports. +* @ssac: Sublink speed attribute count. The number of sublink speed +* attributes is ssac + 1. +* @sublink_speed: Array of sublink speed attributes the UDC supports. Sublink +* speed attributes are paired, and an RX followed by a TX attribute. +* @speed_ssid: Current sublink speed attribute ID in use. +* @min_speed_ssid: Sublink speed attribute ID with the minimum speed. +* @max_speed_ssid: Sublink speed attribute ID with the maximum speed. * @state: the state we are now (attached, suspended, configured, etc) * @name: Identifies the controller hardware type. Used in diagnostics * and sometimes configuration.
@@ -405,6 +414,17 @@ struct usb_gadget {struct list_head ep_list;/* of usb_ep */ enum usb_device_speed speed;enum usb_device_speed max_speed; + +/* SSP only */ +unsigned num_lanes; +unsigned max_num_lanes; +unsigned ssac; +#define USB_GADGET_MAX_SSAC 3 +struct usb_sublink_speed sublink_speed[USB_GADGET_MAX_SSAC +1 ]; +unsigned speed_ssid; +unsigned min_speed_ssid; +unsigned max_speed_ssid; +enum usb_device_state state;const char *name;struct device dev;
Signature · 1 line
2.11.0
Thinh Nguyen
maintainerPATCH 4/11 usb: gadget: Set max speed for SSP devices
In reply to Thinh NguyenA super-speed-plus device may operate at different sublink speeds (e.g. gen2x2, gen1x2, or gen2x1). If the USB device supports different sublink speeds at super-speed-plus, set the device to operate at the maximum number of lanes and sublink speed possible. Introduce gadget ops udc_set_num_lanes_and_speed to set the lane count and sublink speed for super-speed-plus capable devices.
--- drivers/usb/gadget/udc/core.c | 24 +++++++++++++++++++----- include/linux/usb/gadget.h | 3 +++ 2 files changed, 22 insertions(+), 5 deletions(-)
drivers/usb/gadget/udc/core.c +19 -5
@@ -1112,12 +1112,26 @@ 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 the UDC supports super-speed-plus and different sublink speeds, +* then set the gadget to the max possible sublink speed for +* super-speed-plus symmetric lanes. +*/ +if (s ==USB_SPEED_SUPER_PLUS && + gadget->ops->udc_set_num_lanes_and_speed) + gadget->ops->udc_set_num_lanes_and_speed (gadget, + gadget->max_num_lanes, + gadget->max_speed_ssid); +else if (gadget->ops->udc_set_speed) + gadget->ops->udc_set_speed (gadget, s); }/**
include/linux/usb/gadget.h +3 -0
@@ -322,6 +322,9 @@ 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_num_lanes_and_speed)(struct usb_gadget *, +unsigned int num_lanes, +unsigned int ssid);struct usb_ep *(*match_ep)(struct usb_gadget *,struct usb_endpoint_descriptor *,struct usb_ss_ep_comp_descriptor *);
Signature · 1 line
2.11.0
Thinh Nguyen
maintainerPATCH 5/11 usb: composite: Properly report sublink speed
In reply to Thinh NguyenUse the max sublink speed attributes reported in the gadget structure to write to the super-speed-plus BOS descriptor if available.
--- drivers/usb/gadget/composite.c | 76 +++++++++++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 24 deletions(-)
drivers/usb/gadget/composite.c +52 -24
@@ -729,45 +729,73 @@ 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; +unsigned int ssac =1 ; +unsigned int ssic =0 ; +unsigned int min_ssid =0 ; +int i; + +if (cdev->gadget->ssac) { + ssac = cdev->gadget->ssac; + +/* +* Paired RX and TX sublink speed attributes share +* the same SSID. +*/ + ssic = (ssac +1 ) /2 -1 ; + min_ssid = cdev->gadget->min_speed_ssid; + } ssp_cap = cdev->req->buf +le16_to_cpu (bos->wTotalLength); bos->bNumDeviceCaps++; -/* -* Report typical values. -*/ - -le16_add_cpu (&bos->wTotalLength,USB_DT_USB_SSP_CAP_SIZE (1 )); - ssp_cap->bLength =USB_DT_USB_SSP_CAP_SIZE (1 ); +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 ) | +cpu_to_le16 (FIELD_PREP (USB_SSP_MIN_SUBLINK_SPEED_ATTRIBUTE_ID , min_ssid) |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_LSE_GBPS ) | -FIELD_PREP (USB_SSP_SUBLINK_SPEED_ST , -USB_ST_SYMMETRIC_RX ) | -FIELD_PREP (USB_SSP_SUBLINK_SPEED_LP ,USB_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_LSE_GBPS ) | -FIELD_PREP (USB_SSP_SUBLINK_SPEED_ST , -USB_ST_SYMMETRIC_TX ) | -FIELD_PREP (USB_SSP_SUBLINK_SPEED_LP ,USB_LP_SSP ) | -FIELD_PREP (USB_SSP_SUBLINK_SPEED_LSM ,10 )); +/* +* If the sublink speed attributes are not specified, then the +* default will be a pair symmetric RX/TX sublink speed +* attributes of 10 Gbps. +*/ +for (i =0 ; i < ssac +1 ; i++) { +struct usb_sublink_speed default_ssa; +struct usb_sublink_speed *ptr; + +if (cdev->gadget->ssac) { + ptr = &cdev->gadget->sublink_speed[i]; + }else { + default_ssa.id = i /2 ; + default_ssa.protocol =USB_LP_SSP ; + default_ssa.exponent =USB_LSE_GBPS ; + default_ssa.mantissa =10 ; + +if (i %2 ) + default_ssa.type =USB_ST_SYMMETRIC_TX ; +else + default_ssa.type =USB_ST_SYMMETRIC_RX ; + + ptr = &default_ssa; + } + + ssp_cap->bmSublinkSpeedAttr[i] = +cpu_to_le32 (FIELD_PREP (USB_SSP_SUBLINK_SPEED_SSID , ptr->id) | +FIELD_PREP (USB_SSP_SUBLINK_SPEED_LSE , ptr->exponent) | +FIELD_PREP (USB_SSP_SUBLINK_SPEED_ST , ptr->type) | +FIELD_PREP (USB_SSP_SUBLINK_SPEED_LP , ptr->protocol) | +FIELD_PREP (USB_SSP_SUBLINK_SPEED_LSM , ptr->mantissa)); + } }return le16_to_cpu (bos->wTotalLength);
Signature · 1 line
2.11.0
Thinh Nguyen
maintainerPATCH 6/11 usb: devicetree: dwc3: Introduce num-lanes and lsm
In reply to Thinh NguyenIntroduce num-lanes and lane-speed-mantissa-gbps for devices operating in super-speed-plus. DWC_usb32 IP supports multiple lanes and can operate in different sublink speeds. Currently the device controller does not have the information of the phy's number of lanes supported. As a result, the user can specify them through these properties if they are different than the default setting.
--- Documentation/devicetree/bindings/usb/dwc3.txt | 9 +++++++++ 1 file changed, 9 insertions(+)
Documentation/devicetree/bindings/usb/dwc3.txt +9 -0
@@ -86,6 +86,15 @@ Optional properties: - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ register for post-silicon frame length adjustment when the fladj_30mhz_sdbnd signal is invalid or incorrect. + - snps,num-lanes: set to specify the number of lanes to use. Valid inputs are + 1 or 2. Apply if the maximum-speed is super-speed-plus + only. Default value is 2 for DWC_usb32. For DWC_usb31, + it is always 1 at super-speed-plus. + - snps,lane-speed-mantissa-gbps: set to specify the symmetric lane speed + mantissa in Gbps. Valid inputs are 5 or 10. Apply if + the maximum-speed is super-speed-plus only. Default + value is 10. For DWC_usb31, it's always 10 at + super-speed-plus. - snps,rx-thr-num-pkt-prd: periodic ESS RX packet threshold count - host mode only. Set this and rx-max-burst-prd to a valid, non-zero value 1-16 (DWC_usb31 programming guide
Signature · 1 line
2.11.0
Rob Herring
maintainerRe: [PATCH 06/11] usb: devicetree: dwc3: Introduce num-lanes and lsm
Quoted 19 lines · Thinh Nguyen
Introduce num-lanes and lane-speed-mantissa-gbps for devices operating in super-speed-plus.
DWC_usb32IP supports multiple lanes and can operate in different sublink speeds. Currently the device controller does not have the information of the phy's number of lanes supported. As a result, the user can specify them through these properties if they are different than the default setting.Signed-off-by Thinh Nguyen <[email protected]>--- Documentation/devicetree/bindings/usb/dwc3.txt | 9 +++++++++ 1 file changed, 9 insertions(+)Documentation/devicetree/bindings/usb/dwc3.txt +0
-0@@ -86,6 +86,15 @@ Optional properties: - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ register for post-silicon frame length adjustment when the
Documentation/devicetree/bindings/usb/dwc3.txt Optional properties: · line 97
Quoted 10 lines
Documentation/devicetree/bindings/usb/dwc3.txt +9
-0fladj_30mhz_sdbnd signal is invalid or incorrect. + - snps,num-lanes: set to specify the number of lanes to use. Valid inputs are + 1 or 2. Apply if the maximum-speed is super-speed-plus + only. Default value is 2 for DWC_usb32. For DWC_usb31, + it is always 1 at super-speed-plus. + - snps,lane-speed-mantissa-gbps: set to specify the symmetric lane speed + mantissa in Gbps. Valid inputs are 5 or 10. Apply if + the maximum-speed is super-speed-plus only. Default + value is 10. For DWC_usb31, it's always 10 at + super-speed-plus.
This is all common USB things and should be common properties (which we may already have).
Rob
Thinh Nguyen
maintainerRe: [PATCH 06/11] usb: devicetree: dwc3: Introduce num-lanes and lsm
Quoted 24 lines · Rob Herring
Quoted 23 lines · Thinh Nguyen
Introduce num-lanes and lane-speed-mantissa-gbps for devices operating in super-speed-plus.
DWC_usb32IP supports multiple lanes and can operate in different sublink speeds. Currently the device controller does not have the information of the phy's number of lanes supported. As a result, the user can specify them through these properties if they are different than the default setting.Signed-off-by Thinh Nguyen <[email protected]>--- Documentation/devicetree/bindings/usb/dwc3.txt | 9 +++++++++ 1 file changed, 9 insertions(+)Documentation/devicetree/bindings/usb/dwc3.txt +3
-0@@ -86,6 +86,15 @@ Optional properties: - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ register for post-silicon frame length adjustment when the fladj_30mhz_sdbnd signal is invalid or incorrect. + - snps,num-lanes: set to specify the number of lanes to use. Valid inputs are + 1 or 2. Apply if the maximum-speed is super-speed-plus + only. Default value is 2 for DWC_usb32. For DWC_usb31,
Quoted 8 lines
Documentation/devicetree/bindings/usb/dwc3.txt Optional properties: · line 97
Quoted 6 lines
Documentation/devicetree/bindings/usb/dwc3.txt +6
-0+ it is always 1 at super-speed-plus. + - snps,lane-speed-mantissa-gbps: set to specify the symmetric lane speed + mantissa in Gbps. Valid inputs are 5 or 10. Apply if + the maximum-speed is super-speed-plus only. Default + value is 10. For DWC_usb31, it's always 10 at + super-speed-plus.This is all common USB things and should be common properties (which we may already have).
Sure. For "num-lanes" is simple, any objection if we use "lane-speed-mantissa-gbps"? Or should we add "lane-speed-exponent"?
Thanks,
Thinh
Rob Herring
maintainerRe: [PATCH 06/11] usb: devicetree: dwc3: Introduce num-lanes and lsm
Quoted 35 lines · Thinh Nguyen
Quoted 24 lines · Rob Herring
Quoted 23 lines · Thinh Nguyen
Introduce num-lanes and lane-speed-mantissa-gbps for devices operating in super-speed-plus.
DWC_usb32IP supports multiple lanes and can operate in different sublink speeds. Currently the device controller does not have the information of the phy's number of lanes supported. As a result, the user can specify them through these properties if they are different than the default setting.Signed-off-by Thinh Nguyen <[email protected]>--- Documentation/devicetree/bindings/usb/dwc3.txt | 9 +++++++++ 1 file changed, 9 insertions(+)Documentation/devicetree/bindings/usb/dwc3.txt +3
-0@@ -86,6 +86,15 @@ Optional properties: - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ register for post-silicon frame length adjustment when the fladj_30mhz_sdbnd signal is invalid or incorrect. + - snps,num-lanes: set to specify the number of lanes to use. Valid inputs are + 1 or 2. Apply if the maximum-speed is super-speed-plus + only. Default value is 2 for DWC_usb32. For DWC_usb31,Quoted 8 lines
Documentation/devicetree/bindings/usb/dwc3.txt Optional properties: · line 97
Quoted 6 lines
Documentation/devicetree/bindings/usb/dwc3.txt +6
-0+ it is always 1 at super-speed-plus. + - snps,lane-speed-mantissa-gbps: set to specify the symmetric lane speed + mantissa in Gbps. Valid inputs are 5 or 10. Apply if + the maximum-speed is super-speed-plus only. Default + value is 10. For DWC_usb31, it's always 10 at + super-speed-plus.This is all common USB things and should be common properties (which we may already have).
Quoted 2 lines
Sure. For "num-lanes" is simple, any objection if we use "lane-speed-mantissa-gbps"? Or should we add "lane-speed-exponent"?
'num-lanes' is good as that's what PCIe uses. Document that with 'maximum-speed'.
I think 'super-speed-plus' should mean gen 2 10G per lane. Then between num-lanes and maximum-speed you can define all 4 possible rates.
Rob
Thinh Nguyen
maintainerRe: [PATCH 06/11] usb: devicetree: dwc3: Introduce num-lanes and lsm
Quoted 34 lines · Rob Herring
Quoted 33 lines · Thinh Nguyen
Quoted 32 lines · Rob Herring
Quoted 19 lines · Thinh Nguyen
Introduce num-lanes and lane-speed-mantissa-gbps for devices operating in super-speed-plus.
DWC_usb32IP supports multiple lanes and can operate in different sublink speeds. Currently the device controller does not have the information of the phy's number of lanes supported. As a result, the user can specify them through these properties if they are different than the default setting.Signed-off-by Thinh Nguyen <[email protected]>--- Documentation/devicetree/bindings/usb/dwc3.txt | 9 +++++++++ 1 file changed, 9 insertions(+)Documentation/devicetree/bindings/usb/dwc3.txt +0
-0@@ -86,6 +86,15 @@ Optional properties: - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ register for post-silicon frame length adjustment when theDocumentation/devicetree/bindings/usb/dwc3.txt Optional properties: · line 97
Quoted 10 lines
Documentation/devicetree/bindings/usb/dwc3.txt +9
-0fladj_30mhz_sdbnd signal is invalid or incorrect. + - snps,num-lanes: set to specify the number of lanes to use. Valid inputs are + 1 or 2. Apply if the maximum-speed is super-speed-plus + only. Default value is 2 for DWC_usb32. For DWC_usb31, + it is always 1 at super-speed-plus. + - snps,lane-speed-mantissa-gbps: set to specify the symmetric lane speed + mantissa in Gbps. Valid inputs are 5 or 10. Apply if + the maximum-speed is super-speed-plus only. Default + value is 10. For DWC_usb31, it's always 10 at + super-speed-plus.This is all common USB things and should be common properties (which we may already have).
Quoted 8 lines
Quoted 2 lines
Sure. For "num-lanes" is simple, any objection if we use "lane-speed-mantissa-gbps"? Or should we add "lane-speed-exponent"?
'num-lanes' is good as that's what PCIe uses. Document that with 'maximum-speed'.
I think 'super-speed-plus' should mean gen 2 10G per lane. Then between num-lanes and maximum-speed you can define all 4 possible rates.
That may confuse the user because now we'd use 'super-speed-plus' to define the speed of the lane rather than the device itself.
According to the USB 3.2 spec, super-speed-plus can mean gen2x1, gen1x2, or gen2x2.
BR,
Thinh
Re: [PATCH 06/11] usb: devicetree: dwc3: Introduce num-lanes and lsm
Hi,
Quoted 40 lines · Thinh Nguyen
Quoted 39 lines · Rob Herring
Quoted 33 lines · Thinh Nguyen
Quoted 24 lines · Rob Herring
Quoted 23 lines · Thinh Nguyen
Introduce num-lanes and lane-speed-mantissa-gbps for devices operating in super-speed-plus.
DWC_usb32IP supports multiple lanes and can operate in different sublink speeds. Currently the device controller does not have the information of the phy's number of lanes supported. As a result, the user can specify them through these properties if they are different than the default setting.Signed-off-by Thinh Nguyen <[email protected]>--- Documentation/devicetree/bindings/usb/dwc3.txt | 9 +++++++++ 1 file changed, 9 insertions(+)Documentation/devicetree/bindings/usb/dwc3.txt +3
-0@@ -86,6 +86,15 @@ Optional properties: - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ register for post-silicon frame length adjustment when the fladj_30mhz_sdbnd signal is invalid or incorrect. + - snps,num-lanes: set to specify the number of lanes to use. Valid inputs are + 1 or 2. Apply if the maximum-speed is super-speed-plus + only. Default value is 2 for DWC_usb32. For DWC_usb31,Quoted 8 lines
Documentation/devicetree/bindings/usb/dwc3.txt Optional properties: · line 97
Quoted 6 lines
Documentation/devicetree/bindings/usb/dwc3.txt +6
-0+ it is always 1 at super-speed-plus. + - snps,lane-speed-mantissa-gbps: set to specify the symmetric lane speed + mantissa in Gbps. Valid inputs are 5 or 10. Apply if + the maximum-speed is super-speed-plus only. Default + value is 10. For DWC_usb31, it's always 10 at + super-speed-plus.This is all common USB things and should be common properties (which we may already have).
Quoted 2 lines
Sure. For "num-lanes" is simple, any objection if we use "lane-speed-mantissa-gbps"? Or should we add "lane-speed-exponent"?
'num-lanes' is good as that's what PCIe uses. Document that with 'maximum-speed'.
Quoted 6 lines
Quoted 3 lines
I think 'super-speed-plus' should mean gen 2 10G per lane. Then between num-lanes and maximum-speed you can define all 4 possible rates.
That may confuse the user because now we'd use 'super-speed-plus' to define the speed of the lane rather than the device itself.
I agree. In USB land we should refer solely to the USB specification naming schemes.
Signature · 1 line
balbi
Rob Herring
maintainerRe: [PATCH 06/11] usb: devicetree: dwc3: Introduce num-lanes and lsm
In reply to Thinh NguyenQuoted 45 lines · Thinh Nguyen
Quoted 34 lines · Rob Herring
Quoted 33 lines · Thinh Nguyen
Quoted 32 lines · Rob Herring
Quoted 19 lines · Thinh Nguyen
Introduce num-lanes and lane-speed-mantissa-gbps for devices operating in super-speed-plus.
DWC_usb32IP supports multiple lanes and can operate in different sublink speeds. Currently the device controller does not have the information of the phy's number of lanes supported. As a result, the user can specify them through these properties if they are different than the default setting.Signed-off-by Thinh Nguyen <[email protected]>--- Documentation/devicetree/bindings/usb/dwc3.txt | 9 +++++++++ 1 file changed, 9 insertions(+)Documentation/devicetree/bindings/usb/dwc3.txt +0
-0@@ -86,6 +86,15 @@ Optional properties: - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ register for post-silicon frame length adjustment when theDocumentation/devicetree/bindings/usb/dwc3.txt Optional properties: · line 97
Quoted 10 lines
Documentation/devicetree/bindings/usb/dwc3.txt +9
-0fladj_30mhz_sdbnd signal is invalid or incorrect. + - snps,num-lanes: set to specify the number of lanes to use. Valid inputs are + 1 or 2. Apply if the maximum-speed is super-speed-plus + only. Default value is 2 for DWC_usb32. For DWC_usb31, + it is always 1 at super-speed-plus. + - snps,lane-speed-mantissa-gbps: set to specify the symmetric lane speed + mantissa in Gbps. Valid inputs are 5 or 10. Apply if + the maximum-speed is super-speed-plus only. Default + value is 10. For DWC_usb31, it's always 10 at + super-speed-plus.This is all common USB things and should be common properties (which we may already have).
Quoted 8 lines
Quoted 2 lines
Sure. For "num-lanes" is simple, any objection if we use "lane-speed-mantissa-gbps"? Or should we add "lane-speed-exponent"?
'num-lanes' is good as that's what PCIe uses. Document that with 'maximum-speed'.
I think 'super-speed-plus' should mean gen 2 10G per lane. Then between num-lanes and maximum-speed you can define all 4 possible rates.
Quoted 5 lines
That may confuse the user because now we'd use 'super-speed-plus' to define the speed of the lane rather than the device itself.
According to the USB 3.2 spec, super-speed-plus can mean gen2x1, gen1x2, or gen2x2.
Then add new strings as needed to make it clear: super-speed-plus-gen1x2
It's obvious that what 'super-speed-plus' means is not clear since USB-IF extended its meaning.
Rob
Thinh Nguyen
maintainerRe: [PATCH 06/11] usb: devicetree: dwc3: Introduce num-lanes and lsm
Quoted 47 lines · Rob Herring
Quoted 46 lines · Thinh Nguyen
Quoted 34 lines · Rob Herring
Quoted 33 lines · Thinh Nguyen
Quoted 32 lines · Rob Herring
Quoted 19 lines · Thinh Nguyen
Introduce num-lanes and lane-speed-mantissa-gbps for devices operating in super-speed-plus.
DWC_usb32IP supports multiple lanes and can operate in different sublink speeds. Currently the device controller does not have the information of the phy's number of lanes supported. As a result, the user can specify them through these properties if they are different than the default setting.Signed-off-by Thinh Nguyen <[email protected]>--- Documentation/devicetree/bindings/usb/dwc3.txt | 9 +++++++++ 1 file changed, 9 insertions(+)Documentation/devicetree/bindings/usb/dwc3.txt +0
-0@@ -86,6 +86,15 @@ Optional properties: - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ register for post-silicon frame length adjustment when theDocumentation/devicetree/bindings/usb/dwc3.txt Optional properties: · line 97
Quoted 10 lines
Documentation/devicetree/bindings/usb/dwc3.txt +9
-0fladj_30mhz_sdbnd signal is invalid or incorrect. + - snps,num-lanes: set to specify the number of lanes to use. Valid inputs are + 1 or 2. Apply if the maximum-speed is super-speed-plus + only. Default value is 2 for DWC_usb32. For DWC_usb31, + it is always 1 at super-speed-plus. + - snps,lane-speed-mantissa-gbps: set to specify the symmetric lane speed + mantissa in Gbps. Valid inputs are 5 or 10. Apply if + the maximum-speed is super-speed-plus only. Default + value is 10. For DWC_usb31, it's always 10 at + super-speed-plus.This is all common USB things and should be common properties (which we may already have).
Quoted 8 lines
Quoted 2 lines
Sure. For "num-lanes" is simple, any objection if we use "lane-speed-mantissa-gbps"? Or should we add "lane-speed-exponent"?
'num-lanes' is good as that's what PCIe uses. Document that with 'maximum-speed'.
I think 'super-speed-plus' should mean gen 2 10G per lane. Then between num-lanes and maximum-speed you can define all 4 possible rates.
That may confuse the user because now we'd use 'super-speed-plus' to define the speed of the lane rather than the device itself.
Quoted 8 lines
Quoted 2 lines
According to the USB 3.2 spec, super-speed-plus can mean gen2x1, gen1x2, or gen2x2.
Then add new strings as needed to make it clear: super-speed-plus-gen1x2
It's obvious that what 'super-speed-plus' means is not clear since USB-IF extended its meaning.
Rob
If we introduce a new enum for gen1x2, now we'd have to go back and inspect all the checks for all the drivers where for example speed == USB_SPEED_SUPER_PLUS. It seems to be more clunky and may introduce more bugs.
BR,
Thinh
Thinh Nguyen
maintainerRe: [PATCH 06/11] usb: devicetree: dwc3: Introduce num-lanes and lsm
Quoted 52 lines · Thinh Nguyen
Quoted 51 lines · Rob Herring
Quoted 40 lines · Thinh Nguyen
Quoted 39 lines · Rob Herring
Quoted 33 lines · Thinh Nguyen
Quoted 24 lines · Rob Herring
Quoted 23 lines · Thinh Nguyen
Introduce num-lanes and lane-speed-mantissa-gbps for devices operating in super-speed-plus.
DWC_usb32IP supports multiple lanes and can operate in different sublink speeds. Currently the device controller does not have the information of the phy's number of lanes supported. As a result, the user can specify them through these properties if they are different than the default setting.Signed-off-by Thinh Nguyen <[email protected]>--- Documentation/devicetree/bindings/usb/dwc3.txt | 9 +++++++++ 1 file changed, 9 insertions(+)Documentation/devicetree/bindings/usb/dwc3.txt +3
-0@@ -86,6 +86,15 @@ Optional properties: - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ register for post-silicon frame length adjustment when the fladj_30mhz_sdbnd signal is invalid or incorrect. + - snps,num-lanes: set to specify the number of lanes to use. Valid inputs are + 1 or 2. Apply if the maximum-speed is super-speed-plus + only. Default value is 2 for DWC_usb32. For DWC_usb31,Quoted 8 lines
Documentation/devicetree/bindings/usb/dwc3.txt Optional properties: · line 97
Quoted 6 lines
Documentation/devicetree/bindings/usb/dwc3.txt +6
-0+ it is always 1 at super-speed-plus. + - snps,lane-speed-mantissa-gbps: set to specify the symmetric lane speed + mantissa in Gbps. Valid inputs are 5 or 10. Apply if + the maximum-speed is super-speed-plus only. Default + value is 10. For DWC_usb31, it's always 10 at + super-speed-plus.This is all common USB things and should be common properties (which we may already have).
Quoted 2 lines
Sure. For "num-lanes" is simple, any objection if we use "lane-speed-mantissa-gbps"? Or should we add "lane-speed-exponent"?
'num-lanes' is good as that's what PCIe uses. Document that with 'maximum-speed'.
Quoted 8 lines
Quoted 3 lines
I think 'super-speed-plus' should mean gen 2 10G per lane. Then between num-lanes and maximum-speed you can define all 4 possible rates.
That may confuse the user because now we'd use 'super-speed-plus' to define the speed of the lane rather than the device itself.
According to the USB 3.2 spec, super-speed-plus can mean gen2x1, gen1x2, or gen2x2.
Then add new strings as needed to make it clear: super-speed-plus-gen1x2
Quoted 9 lines
Quoted 4 lines
It's obvious that what 'super-speed-plus' means is not clear since USB-IF extended its meaning.
Rob
If we introduce a new enum for gen1x2, now we'd have to go back and inspect all the checks for all the drivers where for example speed ==
USB_SPEED_SUPER_PLUS. It seems to be more clunky and may introduce more bugs.
In my opinion, the better option would be to introduce a new property for lane speed such as "lane-speed-mantissa-gbps" because:
1) It still follows the spec, easier for the user to understand
2) We only need to update the drivers where the number of lanes and lane speed matter
3) Easier speed comparison between usb_device_speed enum
4) Easier to backport new code where there's speed comparison
5) Easily extendable to new/different lane speeds
BR,
Thinh
Thinh Nguyen
maintainerRe: [PATCH 06/11] usb: devicetree: dwc3: Introduce num-lanes and lsm
Quoted 65 lines · Thinh Nguyen
Quoted 52 lines · Thinh Nguyen
Quoted 51 lines · Rob Herring
Quoted 40 lines · Thinh Nguyen
Quoted 39 lines · Rob Herring
Quoted 33 lines · Thinh Nguyen
Quoted 24 lines · Rob Herring
Quoted 23 lines · Thinh Nguyen
Introduce num-lanes and lane-speed-mantissa-gbps for devices operating in super-speed-plus.
DWC_usb32IP supports multiple lanes and can operate in different sublink speeds. Currently the device controller does not have the information of the phy's number of lanes supported. As a result, the user can specify them through these properties if they are different than the default setting.Signed-off-by Thinh Nguyen <[email protected]>--- Documentation/devicetree/bindings/usb/dwc3.txt | 9 +++++++++ 1 file changed, 9 insertions(+)Documentation/devicetree/bindings/usb/dwc3.txt +3
-0@@ -86,6 +86,15 @@ Optional properties: - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ register for post-silicon frame length adjustment when the fladj_30mhz_sdbnd signal is invalid or incorrect. + - snps,num-lanes: set to specify the number of lanes to use. Valid inputs are + 1 or 2. Apply if the maximum-speed is super-speed-plus + only. Default value is 2 for DWC_usb32. For DWC_usb31,Quoted 8 lines
Documentation/devicetree/bindings/usb/dwc3.txt Optional properties: · line 97
Quoted 6 lines
Documentation/devicetree/bindings/usb/dwc3.txt +6
-0+ it is always 1 at super-speed-plus. + - snps,lane-speed-mantissa-gbps: set to specify the symmetric lane speed + mantissa in Gbps. Valid inputs are 5 or 10. Apply if + the maximum-speed is super-speed-plus only. Default + value is 10. For DWC_usb31, it's always 10 at + super-speed-plus.This is all common USB things and should be common properties (which we may already have).
Quoted 2 lines
Sure. For "num-lanes" is simple, any objection if we use "lane-speed-mantissa-gbps"? Or should we add "lane-speed-exponent"?
'num-lanes' is good as that's what PCIe uses. Document that with 'maximum-speed'.
Quoted 8 lines
Quoted 3 lines
I think 'super-speed-plus' should mean gen 2 10G per lane. Then between num-lanes and maximum-speed you can define all 4 possible rates.
That may confuse the user because now we'd use 'super-speed-plus' to define the speed of the lane rather than the device itself.
According to the USB 3.2 spec, super-speed-plus can mean gen2x1, gen1x2, or gen2x2.
Then add new strings as needed to make it clear: super-speed-plus-gen1x2
Quoted 9 lines
Quoted 4 lines
It's obvious that what 'super-speed-plus' means is not clear since USB-IF extended its meaning.
Rob
If we introduce a new enum for gen1x2, now we'd have to go back and inspect all the checks for all the drivers where for example speed ==
USB_SPEED_SUPER_PLUS. It seems to be more clunky and may introduce more bugs.In my opinion, the better option would be to introduce a new property for lane speed such as "lane-speed-mantissa-gbps" because:
Quoted 7 lines
1) It still follows the spec, easier for the user to understand
2) We only need to update the drivers where the number of lanes and lane speed matter
3) Easier speed comparison betweenusb_device_speedenum
4) Easier to backport new code where there's speed comparison
5) Easily extendable to new/different lane speeds
Let me send out v2 of this series so that others can also provide more feedback on other patches. We can continue with this discussion as needed in the meanwhile.
BR,
Thinh
Thinh Nguyen
maintainerPATCH 7/11 usb: dwc3: Initialize lane count and sublink speed
In reply to Thinh NguyenDWC_usb32 supports dual-lane operating at different sublink speeds. Initialize and validate the maximum number of lanes and speed the controller supports captured from the dwc3 device properties.
Currently the controller has no visibility into the HW parameter to determine the maximum number of lanes the phy supports. If the number of lanes is not specified, then set the default to 2 for DWC_usb32 and 1 for DWC_usb31 if operate in SSP.
--- drivers/usb/dwc3/core.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 10 ++++++++ 2 files changed, 74 insertions(+)
drivers/usb/dwc3/core.c +64 -0
@@ -1299,6 +1299,10 @@ static void dwc3_get_properties(struct dwc3 *dwc) "snps,usb3_lpm_capable"); dwc->usb2_lpm_disable =device_property_read_bool (dev,"snps,usb2-lpm-disable" ); +device_property_read_u8 (dev,"snps,num-lanes" , + &dwc->maximum_num_lanes); +device_property_read_u8 (dev,"snps,lane-speed-mantissa-gbps" , + &dwc->maximum_lsm);device_property_read_u8 (dev,"snps,rx-thr-num-pkt-prd" , &rx_thr_num_pkt_prd); device_property_read_u8(dev, "snps,rx-max-burst-prd",
@@ -1426,6 +1430,66 @@ static void dwc3_check_params(struct dwc3 *dwc)break ; } + +/* +* If it's not DWC_usb32 IP or speed is less than SSP, no need to +* validate the number of lanes or lane speed mantissa. +* +* Currently the controller does not have visibility into the HW +* parameter to determine the maximum number of lanes the phy supports. +* If the number of lanes is not specified in the device property, then +* set the default to 2 for DWC_usb32 and 1 for DWC_usb31 if the device +* is capable of super-speed-plus. +*/ +if (!DWC3_IP_IS (DWC32 ) || + dwc->maximum_speed <USB_SPEED_SUPER_PLUS ) { +if (dwc->maximum_lsm || dwc->maximum_num_lanes) +dev_warn (dev,"Ignore num_lanes and LSM properties\n" ); + +if (DWC3_IP_IS (DWC31 ) && + dwc->maximum_speed ==USB_SPEED_SUPER_PLUS ) { + dwc->maximum_lsm =DWC3_LSM_10_GBPS ; + dwc->maximum_num_lanes =1 ; + }else { + dwc->maximum_lsm =DWC3_LSM_UNSPECIFIED ; + dwc->maximum_num_lanes =0 ; + } + +return ; + } + +switch (dwc->maximum_lsm) { +case DWC3_LSM_10_GBPS : +case DWC3_LSM_5_GBPS : +break ; +default : +dev_err (dev,"Invalid LSM (%dGbps)\n" , dwc->maximum_lsm); + fallthrough; +case DWC3_LSM_UNSPECIFIED : + dwc->maximum_lsm =DWC3_LSM_10_GBPS ; +break ; + } + +switch (dwc->maximum_num_lanes) { +case 2 : +break ; +case 1 : +if (dwc->maximum_lsm ==DWC3_LSM_5_GBPS ) { +dev_err (dev,"Invalid num_lanes (%d) and LSM (%dGbps) for %s\n" , + dwc->maximum_num_lanes, + dwc->maximum_lsm, +usb_speed_string (dwc->maximum_speed)); + dwc->maximum_lsm =DWC3_LSM_10_GBPS ; + } +break ; +default : +dev_err (dev,"Invalid number of lanes (%d)\n" , + dwc->maximum_num_lanes); + fallthrough; +case 0 : + dwc->maximum_num_lanes =2 ; +break ; + } }static int dwc3_probe (struct platform_device *pdev)
drivers/usb/dwc3/core.h +10 -0
@@ -40,6 +40,10 @@#define DWC3_XHCI_RESOURCES_NUM 2#define DWC3_ISOC_MAX_RETRIES 5 +#define DWC3_LSM_UNSPECIFIED 0 +#define DWC3_LSM_5_GBPS 5 +#define DWC3_LSM_10_GBPS 10 +#define DWC3_SCRATCHBUF_SIZE 4096/* each buffer is assumed to be 4KiB */ #define DWC3_EVENT_BUFFERS_SIZE 4096#define DWC3_EVENT_TYPE_MASK 0xfe
@@ -958,6 +962,8 @@ 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) +* @maximum_lsm: maximum lane speed mantissa in Gbps +* @maximum_num_lanes: maximum number of lanes * @ip: controller's ID * @revision: controller's version of an IP * @version_type: VERSIONTYPE register contents, a sub release of a revision
@@ -988,6 +994,7 @@ struct dwc3_scratchpad_array {* @ep0state: state of endpoint zero * @link_state: link state * @speed: device speed (super, high, full, low) +* @num_lanes: number of connected lanes * @hwparams: copy of hwparams registers * @root: debugfs root folder pointer * @regset: debugfs pointer to regdump file
@@ -1119,6 +1126,8 @@ struct dwc3 {u32 nr_scratch;u32 u1u2;u32 maximum_speed; +u8 maximum_lsm; +u8 maximum_num_lanes;u32 ip;
@@ -1184,6 +1193,7 @@ struct dwc3 {u8 u1pel;u8 speed; +u8 num_lanes;u8 num_eps;
Signature · 1 line
2.11.0
Thinh Nguyen
maintainerPATCH 8/11 usb: dwc3: gadget: Report sublink speed capability
In reply to Thinh NguyenReport the sublink speed attributes to the usb_gadget structure based on the dwc3 device capability maximum_lsm and maximum_num_lanes,
Only DWC_usb32 supports 2 sublink speeds if it can operate with 2 lanes. (i.e. at SSP, it can operate as gen1x2)
Note: the SSID DWC3_SSP_SSID_GEN2 and DWC3_SSP_SSID_GEN1 are arbitrary. There's no standard according to the USB 3.2 spec as long as they are unique and within 0-15.
--- drivers/usb/dwc3/core.h | 4 ++++ drivers/usb/dwc3/gadget.c | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+)
drivers/usb/dwc3/core.h +4 -0
@@ -44,6 +44,10 @@#define DWC3_LSM_5_GBPS 5#define DWC3_LSM_10_GBPS 10 +/* Sublink Speed Attribute ID */ +#define DWC3_SSP_SSID_GEN2 2 +#define DWC3_SSP_SSID_GEN1 1 +#define DWC3_SCRATCHBUF_SIZE 4096/* each buffer is assumed to be 4KiB */ #define DWC3_EVENT_BUFFERS_SIZE 4096#define DWC3_EVENT_TYPE_MASK 0xfe
drivers/usb/dwc3/gadget.c +41 -0
47 lines not on this page · raw message
Signature · 1 line
2.11.0
Thinh Nguyen
maintainerPATCH 9/11 usb: dwc3: gadget: Implement setting of sublink speed
In reply to Thinh NguyenImplement gadget ops udc_set_num_lanes_and_speed. This allows the gadget/core driver to select number of lanes to use and the sublink speed the controller supports.
--- drivers/usb/dwc3/core.h | 2 ++ drivers/usb/dwc3/gadget.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 1 deletion(-)
drivers/usb/dwc3/core.h +2 -0
@@ -389,6 +389,8 @@#define DWC3_GUCTL2_RST_ACTBITLATER 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)
drivers/usb/dwc3/gadget.c +63 -1
82 lines not on this page · raw message
Signature · 1 line
2.11.0
Thinh Nguyen
maintainerPATCH 10/11 usb: dwc3: gadget: Track connected lane and sublink speed
In reply to Thinh NguyenTrack the number of lanes connected in gadget->num_lanes and track the current sublink speed attribute ID for super-speed-plus operations.
Note: if the device is running in gen1x2, set the gadget->speed to USB_SPEED_SUPER_PLUS.
--- drivers/usb/dwc3/core.h | 2 ++ drivers/usb/dwc3/gadget.c | 21 ++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-)
drivers/usb/dwc3/core.h +2 -0
@@ -464,6 +464,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 +20 -1
@@ -2306,6 +2306,10 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g, reg |=DWC3_DCFG_SUPERSPEED_PLUS ; } } + +if (DWC3_IP_IS (DWC32 ) && speed <USB_SPEED_SUPER_PLUS ) + reg &= ~DWC3_DCFG_NUMLANES (~0 ); +dwc3_writel (dwc->regs,DWC3_DCFG , reg);spin_unlock_irqrestore (&dwc->lock, flags);
@@ -3175,12 +3179,19 @@ 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.num_lanes = lanes; + dwc->gadget.speed_ssid =0 ; +/* * RAMClkSel is reset to 0 after USB reset, so it must be reprogrammed * each time on Connect Done.
@@ -3195,6 +3206,7 @@ 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 ; + dwc->gadget.speed_ssid =DWC3_SSP_SSID_GEN2 ;break ;case DWC3_DSTS_SUPERSPEED :/*
@@ -3215,7 +3227,13 @@ 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.speed_ssid =DWC3_SSP_SSID_GEN1 ; + }else { + dwc->gadget.speed =USB_SPEED_SUPER ; + }break ;case DWC3_DSTS_HIGHSPEED : dwc3_gadget_ep0_desc.wMaxPacketSize =cpu_to_le16 (64 );
@@ -3696,6 +3714,7 @@ int dwc3_gadget_init(struct dwc3 *dwc) dwc->gadget.sg_supported = true; dwc->gadget.name ="dwc3-gadget" ; dwc->gadget.lpm_capable = true; + dwc->gadget.num_lanes =1 ;/* * FIXME We might be setting max_speed to <SUPER, however versions
Signature · 1 line
2.11.0
Thinh Nguyen
maintainerPATCH 11/11 usb: dwc3: gadget: Set speed only up to the max supported
In reply to Thinh NguyenThe setting of device speed should be limited by the device's maximum_speed. This patch adds a check and prevent the driver from attempting to configure higher than the maximum_speed.
--- drivers/usb/dwc3/gadget.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/usb/dwc3/gadget.c +8 -3
@@ -2254,6 +2254,7 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g, enum usb_device_speed speed) {struct dwc3 *dwc =gadget_to_dwc (g); +enum usb_device_speed selected_speed = speed;unsigned long flags;u32 reg;
@@ -2278,7 +2279,10 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g, !dwc->dis_metastability_quirk) { reg |= DWC3_DCFG_SUPERSPEED; } else { -switch (speed) { +if (speed > dwc->maximum_speed) + selected_speed = dwc->maximum_speed; + +switch (selected_speed) {case USB_SPEED_LOW : reg |=DWC3_DCFG_LOWSPEED ;break ;
@@ -2298,7 +2302,8 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g, reg |=DWC3_DCFG_SUPERSPEED_PLUS ;break ;default : -dev_err (dwc->dev,"invalid speed (%d)\n" , speed); +dev_err (dwc->dev,"invalid speed (%d)\n" , + selected_speed);if (DWC3_IP_IS (DWC3 )) reg |=DWC3_DCFG_SUPERSPEED ;
@@ -2307,7 +2312,7 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g, } } -if (DWC3_IP_IS (DWC32 ) && speed <USB_SPEED_SUPER_PLUS ) +if (DWC3_IP_IS (DWC32 ) && selected_speed <USB_SPEED_SUPER_PLUS ) reg &= ~DWC3_DCFG_NUMLANES (~0 );dwc3_writel (dwc->regs,DWC3_DCFG , reg);
Signature · 1 line
2.11.0