Re: [PATCH v5 01/12] usb: ch9: Add sublink speed struct

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Sep 24, 2020 at 07:41:49PM -0700, Thinh Nguyen wrote:
> 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
> 
> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
> 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 | 43 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h
> index 604c6c514a50..01191649a0ad 100644
> --- a/include/linux/usb/ch9.h
> +++ b/include/linux/usb/ch9.h
> @@ -36,6 +36,49 @@
>  #include <linux/device.h>
>  #include <uapi/linux/usb/ch9.h>
>  
> +/* 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
> + * @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;

__u8?

> +	u16				mantissa;

What endian is this?

> +	enum usb_lane_speed_exponent	exponent;
> +	enum usb_sublink_type		type;
> +	enum usb_link_protocol		protocol;


Crazy packing, is this really the way the field is set up?

thanks,

greg k-h