Gustavo A. R. Silva wrote:
> Hi,
>
> On 2/1/21 21:42, Thinh Nguyen wrote:
>> Introduce ssp_rate field to usb_device structure to capture the
>> connected SuperSpeed Plus signaling rate generation and lane count with
>> the corresponding usb_ssp_rate enum.
>>
>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>
>> ---
>> drivers/usb/core/hcd.c | 6 +++-
>> drivers/usb/core/hub.c | 78 ++++++++++++++++++++++++++++++++++++++++++
>> include/linux/usb.h | 2 ++
>> 3 files changed, 85 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index ad5a0f405a75..55de04df3022 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -2721,6 +2721,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
>>
>> rhdev->rx_lanes = 1;
>> rhdev->tx_lanes = 1;
>> + rhdev->ssp_rate = USB_SSP_GEN_UNKNOWN;
>>
>> switch (hcd->speed) {
>> case HCD_USB11:
>> @@ -2738,8 +2739,11 @@ int usb_add_hcd(struct usb_hcd *hcd,
>> case HCD_USB32:
>> rhdev->rx_lanes = 2;
>> rhdev->tx_lanes = 2;
>> - fallthrough;
>> + rhdev->ssp_rate = USB_SSP_GEN_2x2;
>> + rhdev->speed = USB_SPEED_SUPER_PLUS;
>> + break;
>> case HCD_USB31:
>> + rhdev->ssp_rate = USB_SSP_GEN_2x1;
>> rhdev->speed = USB_SPEED_SUPER_PLUS;
>> break;
>> default:
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index 7f71218cc1e5..e78b2dd7801a 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -31,6 +31,7 @@
>> #include <linux/pm_qos.h>
>> #include <linux/kobject.h>
>>
>> +#include <linux/bitfield.h>
>> #include <linux/uaccess.h>
>> #include <asm/byteorder.h>
>>
>> @@ -2668,6 +2669,81 @@ int usb_authorize_device(struct usb_device *usb_dev)
>> return result;
>> }
>>
>> +/**
>> + * get_port_ssp_rate - Match the extended port status to SSP rate
>> + * @hdev: The hub device
>> + * @ext_portstatus: extended port status
>> + *
>> + * Match the extended port status speed id to the SuperSpeed Plus sublink speed
>> + * capability attributes. Base on the number of connected lanes and speed,
>> + * return the corresponding enum usb_ssp_rate.
>> + */
>> +static enum usb_ssp_rate get_port_ssp_rate(struct usb_device *hdev,
>> + u32 ext_portstatus)
>> +{
>> + struct usb_ssp_cap_descriptor *ssp_cap = hdev->bos->ssp_cap;
>> + u32 attr;
>> + u8 speed_id;
>> + u8 ssac;
>> + u8 lanes;
>> + int i;
>> +
>> + if (!ssp_cap)
>> + goto out;
>> +
>> + speed_id = ext_portstatus & USB_EXT_PORT_STAT_RX_SPEED_ID;
>> + lanes = USB_EXT_PORT_RX_LANES(ext_portstatus) + 1;
>> +
>> + ssac = le32_to_cpu(ssp_cap->bmAttributes) &
>> + USB_SSP_SUBLINK_SPEED_ATTRIBS;
>> +
>> + for (i = 0; i <= ssac; i++) {
> Why a less than or equal to comparison here?
> Why not just a less than comparison (i < ssac) ?
>
> Thanks
> --
> Gustavo
The SSAC here matches with the SSAC (Sublink Speed Attribute Count) from
the USB 3.2 spec. It's zero based. E.g. so SSAC of 3 is 4 number of
Sublink Speed Attributes. That's why "<=".
Thinh
>
>> + u8 ssid;
>> +
>> + attr = le32_to_cpu(ssp_cap->bmSublinkSpeedAttr[i]);
>> + ssid = FIELD_GET(USB_SSP_SUBLINK_SPEED_SSID, attr);
>> + if (speed_id == ssid) {
>> + u16 mantissa;
>> + u8 lse;
>> + u8 type;
>> +
>> + /*
>> + * Note: currently asymmetric lane types are only
>> + * applicable for SSIC operate in SuperSpeed protocol
>> + */
>> + type = FIELD_GET(USB_SSP_SUBLINK_SPEED_ST, attr);
>> + if (type == USB_SSP_SUBLINK_SPEED_ST_ASYM_RX ||
>> + type == USB_SSP_SUBLINK_SPEED_ST_ASYM_TX)
>> + goto out;
>> +
>> + if (FIELD_GET(USB_SSP_SUBLINK_SPEED_LP, attr) !=
>> + USB_SSP_SUBLINK_SPEED_LP_SSP)
>> + goto out;
>> +
>> + lse = FIELD_GET(USB_SSP_SUBLINK_SPEED_LSE, attr);
>> + mantissa = FIELD_GET(USB_SSP_SUBLINK_SPEED_LSM, attr);
>> +
>> + /* Convert to Gbps */
>> + for (; lse < USB_SSP_SUBLINK_SPEED_LSE_GBPS; lse++)
>> + mantissa /= 1000;
>> +
>> + if (mantissa >= 10 && lanes == 1)
>> + return USB_SSP_GEN_2x1;
>> +
>> + if (mantissa >= 10 && lanes == 2)
>> + return USB_SSP_GEN_2x2;
>> +
>> + if (mantissa >= 5 && lanes == 2)
>> + return USB_SSP_GEN_1x2;
>> +
>> + goto out;
>> + }
>> + }
>> +
>> +out:
>> + return USB_SSP_GEN_UNKNOWN;
>> +}
>> +
>> /*
>> * Return 1 if port speed is SuperSpeedPlus, 0 otherwise
>> * check it from the link protocol field of the current speed ID attribute.
>> @@ -2850,9 +2926,11 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1,
>> /* extended portstatus Rx and Tx lane count are zero based */
>> udev->rx_lanes = USB_EXT_PORT_RX_LANES(ext_portstatus) + 1;
>> udev->tx_lanes = USB_EXT_PORT_TX_LANES(ext_portstatus) + 1;
>> + udev->ssp_rate = get_port_ssp_rate(hub->hdev, ext_portstatus);
>> } else {
>> udev->rx_lanes = 1;
>> udev->tx_lanes = 1;
>> + udev->ssp_rate = USB_SSP_GEN_UNKNOWN;
>> }
>> if (hub_is_wusb(hub))
>> udev->speed = USB_SPEED_WIRELESS;
>> diff --git a/include/linux/usb.h b/include/linux/usb.h
>> index 7d72c4e0713c..c334387f950e 100644
>> --- a/include/linux/usb.h
>> +++ b/include/linux/usb.h
>> @@ -560,6 +560,7 @@ struct usb3_lpm_parameters {
>> * @speed: device speed: high/full/low (or error)
>> * @rx_lanes: number of rx lanes in use, USB 3.2 adds dual-lane support
>> * @tx_lanes: number of tx lanes in use, USB 3.2 adds dual-lane support
>> + * @ssp_rate: SuperSpeed Plus phy signaling rate and lane count
>> * @tt: Transaction Translator info; used with low/full speed dev, highspeed hub
>> * @ttport: device port on that tt hub
>> * @toggle: one bit for each endpoint, with ([0] = IN, [1] = OUT) endpoints
>> @@ -636,6 +637,7 @@ struct usb_device {
>> enum usb_device_speed speed;
>> unsigned int rx_lanes;
>> unsigned int tx_lanes;
>> + enum usb_ssp_rate ssp_rate;
>>
>> struct usb_tt *tt;
>> int ttport;
>>
