diff mbox series

[v3,03/12] usb: gadget: Expose sublink speed attributes

Message ID 500284ccf0353ee17a6bee8fa55011f801e17630.1595631457.git.thinhn@synopsys.com (mailing list archive)
State Superseded
Headers show
Series usb: Handle different sublink speeds | expand

Commit Message

Thinh Nguyen July 24, 2020, 11:38 p.m. UTC
The 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.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 Changes in v3:
 - None
 Changes in v2:
 - None

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

Comments

Chunfeng Yun July 25, 2020, 3:14 a.m. UTC | #1
On Fri, 2020-07-24 at 16:38 -0700, Thinh Nguyen wrote:
> The 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.
> 
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
>  Changes in v3:
>  - None
>  Changes in v2:
>  - None
> 
>  include/linux/usb/gadget.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index 52ce1f6b8f83..bd982669609c 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -339,6 +339,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.
> @@ -406,6 +415,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;
checkpatch warning:

WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'

> +
>  	enum usb_device_state		state;
>  	const char			*name;
>  	struct device			dev;
Thinh Nguyen July 25, 2020, 3:33 a.m. UTC | #2
Chunfeng Yun wrote:
> On Fri, 2020-07-24 at 16:38 -0700, Thinh Nguyen wrote:
>> The 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.
>>
>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>> ---
>>   Changes in v3:
>>   - None
>>   Changes in v2:
>>   - None
>>
>>   include/linux/usb/gadget.h | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>> index 52ce1f6b8f83..bd982669609c 100644
>> --- a/include/linux/usb/gadget.h
>> +++ b/include/linux/usb/gadget.h
>> @@ -339,6 +339,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.
>> @@ -406,6 +415,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;
> checkpatch warning:
>
> WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'

Yes, but I'd like to keep them consistent with the rest of the fields in 
this structure.

>> +
>>   	enum usb_device_state		state;
>>   	const char			*name;
>>   	struct device			dev;

BR,
Thinh
Greg Kroah-Hartman July 25, 2020, 10:13 a.m. UTC | #3
On Sat, Jul 25, 2020 at 03:33:39AM +0000, Thinh Nguyen wrote:
> Chunfeng Yun wrote:
> > On Fri, 2020-07-24 at 16:38 -0700, Thinh Nguyen wrote:
> >> The 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.
> >>
> >> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> >> ---
> >>   Changes in v3:
> >>   - None
> >>   Changes in v2:
> >>   - None
> >>
> >>   include/linux/usb/gadget.h | 20 ++++++++++++++++++++
> >>   1 file changed, 20 insertions(+)
> >>
> >> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> >> index 52ce1f6b8f83..bd982669609c 100644
> >> --- a/include/linux/usb/gadget.h
> >> +++ b/include/linux/usb/gadget.h
> >> @@ -339,6 +339,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.
> >> @@ -406,6 +415,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;
> > checkpatch warning:
> >
> > WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
> 
> Yes, but I'd like to keep them consistent with the rest of the fields in 
> this structure.

No, do not do things that you know are wrong and will have to be cleaned
up in the future.  Unless you are trying to increase your patch count
for some reason, this is not ok.

thanks,

greg k-h
Thinh Nguyen July 25, 2020, 10:52 a.m. UTC | #4
Greg Kroah-Hartman wrote:
> On Sat, Jul 25, 2020 at 03:33:39AM +0000, Thinh Nguyen wrote:
>> Chunfeng Yun wrote:
>>> On Fri, 2020-07-24 at 16:38 -0700, Thinh Nguyen wrote:
>>>> The 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.
>>>>
>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>>> ---
>>>>    Changes in v3:
>>>>    - None
>>>>    Changes in v2:
>>>>    - None
>>>>
>>>>    include/linux/usb/gadget.h | 20 ++++++++++++++++++++
>>>>    1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>>>> index 52ce1f6b8f83..bd982669609c 100644
>>>> --- a/include/linux/usb/gadget.h
>>>> +++ b/include/linux/usb/gadget.h
>>>> @@ -339,6 +339,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.
>>>> @@ -406,6 +415,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;
>>> checkpatch warning:
>>>
>>> WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
>> Yes, but I'd like to keep them consistent with the rest of the fields in
>> this structure.
> No, do not do things that you know are wrong and will have to be cleaned
> up in the future.  Unless you are trying to increase your patch count
> for some reason, this is not ok.
>

Ok. Will fix this.

Thanks,
Thinh
diff mbox series

Patch

diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 52ce1f6b8f83..bd982669609c 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -339,6 +339,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.
@@ -406,6 +415,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;