* Re: [PATCH v2 7/8] usb: xhci: Rewrite xhci_create_usb3_bos_desc()
@ 2021-02-09 5:37 kernel test robot
0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2021-02-09 5:37 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 17881 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <2d9a3de7f6cfeada9a15ec0ec4683b49c62543c2.1612410491.git.Thinh.Nguyen@synopsys.com>
References: <2d9a3de7f6cfeada9a15ec0ec4683b49c62543c2.1612410491.git.Thinh.Nguyen@synopsys.com>
TO: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
TO: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
TO: Thinh.Nguyen(a)synopsys.com
TO: linux-usb(a)vger.kernel.org
TO: Mathias Nyman <mathias.nyman@intel.com>
Hi Thinh,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on d021e0694d77ee3cdc5d3fca2c8d53ae7575499a]
url: https://github.com/0day-ci/linux/commits/Thinh-Nguyen/usb-Check-for-genXxY-on-host/20210204-121545
base: d021e0694d77ee3cdc5d3fca2c8d53ae7575499a
:::::: branch date: 5 days ago
:::::: commit date: 5 days ago
config: x86_64-randconfig-m001-20210209 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
drivers/usb/host/xhci-hub.c:98 xhci_create_usb3x_bos_desc() error: uninitialized symbol 'bcdUSB'.
Old smatch warnings:
drivers/usb/host/xhci-hub.c:104 xhci_create_usb3x_bos_desc() error: uninitialized symbol 'bcdUSB'.
vim +/bcdUSB +98 drivers/usb/host/xhci-hub.c
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 67
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 68 static int xhci_create_usb3x_bos_desc(struct xhci_hcd *xhci, char *buf,
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 69 u16 wLength)
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 70 {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 71 struct usb_bos_descriptor *bos;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 72 struct usb_ss_cap_descriptor *ss_cap;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 73 struct usb_ssp_cap_descriptor *ssp_cap;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 74 struct xhci_port_cap *port_cap = NULL;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 75 u16 bcdUSB;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 76 u32 reg;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 77 u32 min_rate = 0;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 78 u8 min_ssid;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 79 u8 ssac;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 80 u8 ssic;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 81 int offset;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 82 int i;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 83
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 84 /* BOS descriptor */
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 85 bos = (struct usb_bos_descriptor *)buf;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 86 bos->bLength = USB_DT_BOS_SIZE;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 87 bos->bDescriptorType = USB_DT_BOS;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 88 bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE +
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 89 USB_DT_USB_SS_CAP_SIZE);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 90 bos->bNumDeviceCaps = 1;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 91
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 92 /* Create the descriptor for port with the highest revision */
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 93 for (i = 0; i < xhci->num_port_caps; i++) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 94 u8 major = xhci->port_caps[i].maj_rev;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 95 u8 minor = xhci->port_caps[i].min_rev;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 96 u16 rev = (major << 8) | minor;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 97
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 @98 if (i == 0 || bcdUSB < rev) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 99 bcdUSB = rev;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 100 port_cap = &xhci->port_caps[i];
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 101 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 102 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 103
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 104 if (bcdUSB >= 0x0310) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 105 if (port_cap->psi_count) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 106 u8 num_sym_ssa = 0;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 107
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 108 for (i = 0; i < port_cap->psi_count; i++) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 109 if ((port_cap->psi[i] & PLT_MASK) == PLT_SYM)
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 110 num_sym_ssa++;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 111 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 112
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 113 ssac = port_cap->psi_count + num_sym_ssa - 1;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 114 ssic = port_cap->psi_uid_count - 1;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 115 } else {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 116 if (bcdUSB >= 0x0320)
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 117 ssac = 7;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 118 else
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 119 ssac = 3;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 120
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 121 ssic = (ssac + 1) / 2 - 1;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 122 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 123
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 124 bos->bNumDeviceCaps++;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 125 bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE +
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 126 USB_DT_USB_SS_CAP_SIZE +
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 127 USB_DT_USB_SSP_CAP_SIZE(ssac));
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 128 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 129
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 130 if (wLength < USB_DT_BOS_SIZE + USB_DT_USB_SS_CAP_SIZE)
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 131 return wLength;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 132
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 133 /* SuperSpeed USB Device Capability */
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 134 ss_cap = (struct usb_ss_cap_descriptor *)&buf[USB_DT_BOS_SIZE];
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 135 ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 136 ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 137 ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 138 ss_cap->bmAttributes = 0; /* set later */
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 139 ss_cap->wSpeedSupported = cpu_to_le16(USB_5GBPS_OPERATION);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 140 ss_cap->bFunctionalitySupport = USB_LOW_SPEED_OPERATION;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 141 ss_cap->bU1devExitLat = 0; /* set later */
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 142 ss_cap->bU2DevExitLat = 0; /* set later */
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 143
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 144 reg = readl(&xhci->cap_regs->hcc_params);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 145 if (HCC_LTC(reg))
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 146 ss_cap->bmAttributes |= USB_LTM_SUPPORT;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 147
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 148 if ((xhci->quirks & XHCI_LPM_SUPPORT)) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 149 reg = readl(&xhci->cap_regs->hcs_params3);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 150 ss_cap->bU1devExitLat = HCS_U1_LATENCY(reg);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 151 ss_cap->bU2DevExitLat = cpu_to_le16(HCS_U2_LATENCY(reg));
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 152 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 153
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 154 if (wLength < le16_to_cpu(bos->wTotalLength))
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 155 return wLength;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 156
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 157 if (bcdUSB < 0x0310)
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 158 return le16_to_cpu(bos->wTotalLength);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 159
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 160 ssp_cap = (struct usb_ssp_cap_descriptor *)&buf[USB_DT_BOS_SIZE +
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 161 USB_DT_USB_SS_CAP_SIZE];
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 162 ssp_cap->bLength = USB_DT_USB_SSP_CAP_SIZE(ssac);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 163 ssp_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 164 ssp_cap->bDevCapabilityType = USB_SSP_CAP_TYPE;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 165 ssp_cap->bReserved = 0;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 166 ssp_cap->wReserved = 0;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 167 ssp_cap->bmAttributes =
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 168 cpu_to_le32(FIELD_PREP(USB_SSP_SUBLINK_SPEED_ATTRIBS, ssac) |
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 169 FIELD_PREP(USB_SSP_SUBLINK_SPEED_IDS, ssic));
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 170
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 171 if (!port_cap->psi_count) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 172 for (i = 0; i < ssac + 1; i++)
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 173 ssp_cap->bmSublinkSpeedAttr[i] =
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 174 cpu_to_le32(ssp_cap_default_ssa[i]);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 175
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 176 min_ssid = 4;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 177 goto out;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 178 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 179
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 180 offset = 0;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 181 for (i = 0; i < port_cap->psi_count; i++) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 182 u32 psi;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 183 u32 attr;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 184 u8 ssid;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 185 u8 lp;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 186 u8 lse;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 187 u8 psie;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 188 u16 lane_mantissa;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 189 u16 psim;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 190 u16 plt;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 191
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 192 psi = port_cap->psi[i];
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 193 ssid = XHCI_EXT_PORT_PSIV(psi);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 194 lp = XHCI_EXT_PORT_LP(psi);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 195 psie = XHCI_EXT_PORT_PSIE(psi);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 196 psim = XHCI_EXT_PORT_PSIM(psi);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 197 plt = psi & PLT_MASK;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 198
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 199 lse = psie;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 200 lane_mantissa = psim;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 201
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 202 /* Shift to Gbps and set SSP Link Protocol if 10Gpbs */
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 203 for (; psie < USB_SSP_SUBLINK_SPEED_LSE_GBPS; psie++)
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 204 psim /= 1000;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 205
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 206 if (!min_rate || psim < min_rate) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 207 min_ssid = ssid;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 208 min_rate = psim;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 209 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 210
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 211 /* Some host controllers don't set the link protocol for SSP */
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 212 if (psim >= 10)
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 213 lp = USB_SSP_SUBLINK_SPEED_LP_SSP;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 214
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 215 /*
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 216 * PSIM and PSIE represent the total speed of PSI. The BOS
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 217 * descriptor SSP sublink speed attribute lane mantissa
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 218 * describes the lane speed. E.g. PSIM and PSIE for gen2x2
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 219 * is 20Gbps, but the BOS descriptor lane speed mantissa is
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 220 * 10Gbps. Check and modify the mantissa value to match the
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 221 * lane speed.
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 222 */
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 223 if (bcdUSB == 0x0320 && plt == PLT_SYM) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 224 /*
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 225 * The PSI dword for gen1x2 and gen2x1 share the same
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 226 * values. But the lane speed for gen1x2 is 5Gbps while
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 227 * gen2x1 is 10Gbps. If the previous PSI dword SSID is
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 228 * 5 and the PSIE and PSIM match with SSID 6, let's
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 229 * assume that the controller follows the default speed
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 230 * id with SSID 6 for gen1x2.
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 231 */
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 232 if (ssid == 6 && psie == 3 && psim == 10 && i) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 233 u32 prev = port_cap->psi[i - 1];
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 234
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 235 if ((prev & PLT_MASK) == PLT_SYM &&
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 236 XHCI_EXT_PORT_PSIV(prev) == 5 &&
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 237 XHCI_EXT_PORT_PSIE(prev) == 3 &&
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 238 XHCI_EXT_PORT_PSIM(prev) == 10) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 239 lse = USB_SSP_SUBLINK_SPEED_LSE_GBPS;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 240 lane_mantissa = 5;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 241 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 242 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 243
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 244 if (psie == 3 && psim > 10) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 245 lse = USB_SSP_SUBLINK_SPEED_LSE_GBPS;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 246 lane_mantissa = 10;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 247 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 248 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 249
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 250 attr = (FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, ssid) |
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 251 FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP, lp) |
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 252 FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE, lse) |
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 253 FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, lane_mantissa));
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 254
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 255 switch (plt) {
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 256 case PLT_SYM:
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 257 attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 258 USB_SSP_SUBLINK_SPEED_ST_SYM_RX);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 259 ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 260
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 261 attr &= ~USB_SSP_SUBLINK_SPEED_ST;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 262 attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 263 USB_SSP_SUBLINK_SPEED_ST_SYM_TX);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 264 ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 265 break;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 266 case PLT_ASYM_RX:
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 267 attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 268 USB_SSP_SUBLINK_SPEED_ST_ASYM_RX);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 269 ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 270 break;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 271 case PLT_ASYM_TX:
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 272 attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 273 USB_SSP_SUBLINK_SPEED_ST_ASYM_TX);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 274 ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 275 break;
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 276 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 277 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 278 out:
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 279 ssp_cap->wFunctionalitySupport =
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 280 cpu_to_le16(FIELD_PREP(USB_SSP_MIN_SUBLINK_SPEED_ATTRIBUTE_ID,
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 281 min_ssid) |
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 282 FIELD_PREP(USB_SSP_MIN_RX_LANE_COUNT, 1) |
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 283 FIELD_PREP(USB_SSP_MIN_TX_LANE_COUNT, 1));
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 284
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 285 return le16_to_cpu(bos->wTotalLength);
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 286 }
e531a9f0c9c4b7 Thinh Nguyen 2021-02-03 287
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 29578 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 7/8] usb: xhci: Rewrite xhci_create_usb3_bos_desc()
2021-03-09 19:32 ` Thinh Nguyen
@ 2021-03-10 8:28 ` Mathias Nyman
0 siblings, 0 replies; 5+ messages in thread
From: Mathias Nyman @ 2021-03-10 8:28 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, linux-usb, Mathias Nyman
On 9.3.2021 21.32, Thinh Nguyen wrote:
> Mathias Nyman wrote:
>> On 4.2.2021 6.11, Thinh Nguyen wrote:
>>> The current xhci_create_usb3_bos_desc() uses a static bos u8 array and
>>> various magic numbers and offsets making it difficult to extend support
>>> for USB 3.2. Let's rewrite this entire function to support dual-lane in
>>> USB 3.2.
>>>
>>> The hub driver matches the port speed ID from the extended port status
>>> to the SSID of the sublink speed attributes to detect if the device
>>> supports SuperSpeed Plus. Currently we don't provide the default gen1x2
>>> and gen2x2 sublink speed capability descriptor for USB 3.2 roothub. The
>>> USB stack depends on this to detect and match the correct speed.
>>> In addition, if the xHCI host provides Protocol Speed ID (PSI)
>>> capability, then make sure to convert Protocol Speed ID Mantissa and
>>> Exponent (PSIM & PSIE) to lane speed for gen1x2 and gen2x2.
>>>
>>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>>> ---
>>> Changes in v2:
>>> - Use static array for SSA per Mathias suggestion
>>> - Check bcdUSB >= 0x320 instead of bcdUSB == 0x320 per Mathias suggestion
>>> - When host uses custom PSI, SSIC is the psi_uid_count and not SSAC. I missed
>>> this when transferring the previous logic over. Previous implementation
>>> was incorrect. Let's fix it here.
>>>
>>> drivers/usb/host/xhci-hub.c | 237 +++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 235 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
>>> index 74c497fd3476..eddd139c2260 100644
>>> --- a/drivers/usb/host/xhci-hub.c
>>> +++ b/drivers/usb/host/xhci-hub.c
>>> @@ -11,6 +11,7 @@
>>>
>>> #include <linux/slab.h>
>>> #include <asm/unaligned.h>
>>> +#include <linux/bitfield.h>
>>>
>>> #include "xhci.h"
>>> #include "xhci-trace.h"
>>> @@ -52,7 +53,239 @@ static u8 usb_bos_descriptor [] = {
>>> 0xb5, 0x40, 0x0a, 0x00, /* 10Gbps, SSP, symmetric, tx, ID = 5 */
>>> };
>>>
>>> -static int xhci_create_usb3_bos_desc(struct xhci_hcd *xhci, char *buf,
>>> +/* Default sublink speed attribute of each lane */
>>> +static u32 ssp_cap_default_ssa[] = {
>>> + 0x00050034, /* USB 3.0 SS Gen1x1 id:4 symmetric rx 5Gbps */
>>> + 0x000500b4, /* USB 3.0 SS Gen1x1 id:4 symmetric tx 5Gbps */
>>> + 0x000a4035, /* USB 3.1 SSP Gen2x1 id:5 symmetric rx 10Gbps */
>>> + 0x000a40b5, /* USB 3.1 SSP Gen2x1 id:5 symmetric tx 10Gbps */
>>> + 0x00054036, /* USB 3.2 SSP Gen1x2 id:6 symmetric rx 5Gbps */
>>> + 0x000540b6, /* USB 3.2 SSP Gen1x2 id:6 symmetric tx 5Gbps */
>>> + 0x000a4037, /* USB 3.2 SSP Gen2x2 id:7 symmetric rx 10Gbps */
>>> + 0x000a40b7, /* USB 3.2 SSP Gen2x2 id:7 symmetric tx 10Gbps */
>>> +};
>>> +
>>> +static int xhci_create_usb3x_bos_desc(struct xhci_hcd *xhci, char *buf,
>>> + u16 wLength)
>>> +{
>>> + struct usb_bos_descriptor *bos;
>>> + struct usb_ss_cap_descriptor *ss_cap;
>>> + struct usb_ssp_cap_descriptor *ssp_cap;
>>> + struct xhci_port_cap *port_cap = NULL;
>>> + u16 bcdUSB;
>>> + u32 reg;
>>> + u32 min_rate = 0;
>>> + u8 min_ssid;
>>> + u8 ssac;
>>> + u8 ssic;
>>> + int offset;
>>> + int i;
>>> +
>>> + /* BOS descriptor */
>>> + bos = (struct usb_bos_descriptor *)buf;
>>> + bos->bLength = USB_DT_BOS_SIZE;
>>> + bos->bDescriptorType = USB_DT_BOS;
>>> + bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE +
>>> + USB_DT_USB_SS_CAP_SIZE);
>>> + bos->bNumDeviceCaps = 1;
>>> +
>>> + /* Create the descriptor for port with the highest revision */
>>> + for (i = 0; i < xhci->num_port_caps; i++) {
>>> + u8 major = xhci->port_caps[i].maj_rev;
>>> + u8 minor = xhci->port_caps[i].min_rev;
>>> + u16 rev = (major << 8) | minor;
>>> +
>>> + if (i == 0 || bcdUSB < rev) {
>>> + bcdUSB = rev;
>>> + port_cap = &xhci->port_caps[i];
>>> + }
>>> + }
>>> +
>>> + if (bcdUSB >= 0x0310) {
>>> + if (port_cap->psi_count) {
>>> + u8 num_sym_ssa = 0;
>>> +
>>> + for (i = 0; i < port_cap->psi_count; i++) {
>>> + if ((port_cap->psi[i] & PLT_MASK) == PLT_SYM)
>>> + num_sym_ssa++;
>>> + }
>>> +
>>> + ssac = port_cap->psi_count + num_sym_ssa - 1;
>>> + ssic = port_cap->psi_uid_count - 1;
>>> + } else {
>>> + if (bcdUSB >= 0x0320)
>>> + ssac = 7;
>>> + else
>>> + ssac = 3;
>>> +
>>> + ssic = (ssac + 1) / 2 - 1;
>>> + }
>>> +
>>> + bos->bNumDeviceCaps++;
>>> + bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE +
>>> + USB_DT_USB_SS_CAP_SIZE +
>>> + USB_DT_USB_SSP_CAP_SIZE(ssac));
>>> + }
>>> +
>>> + if (wLength < USB_DT_BOS_SIZE + USB_DT_USB_SS_CAP_SIZE)
>>> + return wLength;
>>> +
>>> + /* SuperSpeed USB Device Capability */
>>> + ss_cap = (struct usb_ss_cap_descriptor *)&buf[USB_DT_BOS_SIZE];
>>> + ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
>>> + ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
>>> + ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
>>> + ss_cap->bmAttributes = 0; /* set later */
>>> + ss_cap->wSpeedSupported = cpu_to_le16(USB_5GBPS_OPERATION);
>>> + ss_cap->bFunctionalitySupport = USB_LOW_SPEED_OPERATION;
>>> + ss_cap->bU1devExitLat = 0; /* set later */
>>> + ss_cap->bU2DevExitLat = 0; /* set later */
>>> +
>>> + reg = readl(&xhci->cap_regs->hcc_params);
>>> + if (HCC_LTC(reg))
>>> + ss_cap->bmAttributes |= USB_LTM_SUPPORT;
>>> +
>>> + if ((xhci->quirks & XHCI_LPM_SUPPORT)) {
>>> + reg = readl(&xhci->cap_regs->hcs_params3);
>>> + ss_cap->bU1devExitLat = HCS_U1_LATENCY(reg);
>>> + ss_cap->bU2DevExitLat = cpu_to_le16(HCS_U2_LATENCY(reg));
>>> + }
>>> +
>>> + if (wLength < le16_to_cpu(bos->wTotalLength))
>>> + return wLength;
>>> +
>>> + if (bcdUSB < 0x0310)
>>> + return le16_to_cpu(bos->wTotalLength);
>>> +
>>> + ssp_cap = (struct usb_ssp_cap_descriptor *)&buf[USB_DT_BOS_SIZE +
>>> + USB_DT_USB_SS_CAP_SIZE];
>>> + 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, ssac) |
>>> + FIELD_PREP(USB_SSP_SUBLINK_SPEED_IDS, ssic));
>>> +
>>> + if (!port_cap->psi_count) {
>>> + for (i = 0; i < ssac + 1; i++)
>>> + ssp_cap->bmSublinkSpeedAttr[i] =
>>> + cpu_to_le32(ssp_cap_default_ssa[i]);
>>> +
>>> + min_ssid = 4;
>>> + goto out;
>>> + }
>>> +
>>> + offset = 0;
>>> + for (i = 0; i < port_cap->psi_count; i++) {
>>> + u32 psi;
>>> + u32 attr;
>>> + u8 ssid;
>>> + u8 lp;
>>> + u8 lse;
>>> + u8 psie;
>>> + u16 lane_mantissa;
>>> + u16 psim;
>>> + u16 plt;
>>> +
>>> + psi = port_cap->psi[i];
>>> + ssid = XHCI_EXT_PORT_PSIV(psi);
>>> + lp = XHCI_EXT_PORT_LP(psi);
>>> + psie = XHCI_EXT_PORT_PSIE(psi);
>>> + psim = XHCI_EXT_PORT_PSIM(psi);
>>> + plt = psi & PLT_MASK;
>>> +
>>> + lse = psie;
>>> + lane_mantissa = psim;
>>> +
>>> + /* Shift to Gbps and set SSP Link Protocol if 10Gpbs */
>>> + for (; psie < USB_SSP_SUBLINK_SPEED_LSE_GBPS; psie++)
>>> + psim /= 1000;
>>> +
>>> + if (!min_rate || psim < min_rate) {
>>> + min_ssid = ssid;
>>> + min_rate = psim;
>>> + }
>>> +
>>> + /* Some host controllers don't set the link protocol for SSP */
>>> + if (psim >= 10)
>>> + lp = USB_SSP_SUBLINK_SPEED_LP_SSP;
>> Maybe we should limit the above fix to older than USB 3.2 hosts.
>> xHCI supporting Gen 1x2 can in theory have psim==10 even if it only supports
>> SS link protocol, not SSP.
>
> Gen 1x2 uses SuperSpeed Plus link protocol. See USB 3.2 spec section 3.2
>
Yes, you're right. I got so used to Gen 1 always being SS, and Gen 2 SSP in USB 3.1.
Then those specific workarounds you made also makes sense.
From xhci POV this looks good.
Thanks
-Mathias
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 7/8] usb: xhci: Rewrite xhci_create_usb3_bos_desc()
2021-03-09 11:47 ` Mathias Nyman
@ 2021-03-09 19:32 ` Thinh Nguyen
2021-03-10 8:28 ` Mathias Nyman
0 siblings, 1 reply; 5+ messages in thread
From: Thinh Nguyen @ 2021-03-09 19:32 UTC (permalink / raw)
To: Mathias Nyman, Thinh Nguyen, Greg Kroah-Hartman, linux-usb,
Mathias Nyman
Mathias Nyman wrote:
> On 4.2.2021 6.11, Thinh Nguyen wrote:
>> The current xhci_create_usb3_bos_desc() uses a static bos u8 array and
>> various magic numbers and offsets making it difficult to extend support
>> for USB 3.2. Let's rewrite this entire function to support dual-lane in
>> USB 3.2.
>>
>> The hub driver matches the port speed ID from the extended port status
>> to the SSID of the sublink speed attributes to detect if the device
>> supports SuperSpeed Plus. Currently we don't provide the default gen1x2
>> and gen2x2 sublink speed capability descriptor for USB 3.2 roothub. The
>> USB stack depends on this to detect and match the correct speed.
>> In addition, if the xHCI host provides Protocol Speed ID (PSI)
>> capability, then make sure to convert Protocol Speed ID Mantissa and
>> Exponent (PSIM & PSIE) to lane speed for gen1x2 and gen2x2.
>>
>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>> ---
>> Changes in v2:
>> - Use static array for SSA per Mathias suggestion
>> - Check bcdUSB >= 0x320 instead of bcdUSB == 0x320 per Mathias suggestion
>> - When host uses custom PSI, SSIC is the psi_uid_count and not SSAC. I missed
>> this when transferring the previous logic over. Previous implementation
>> was incorrect. Let's fix it here.
>>
>> drivers/usb/host/xhci-hub.c | 237 +++++++++++++++++++++++++++++++++++-
>> 1 file changed, 235 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
>> index 74c497fd3476..eddd139c2260 100644
>> --- a/drivers/usb/host/xhci-hub.c
>> +++ b/drivers/usb/host/xhci-hub.c
>> @@ -11,6 +11,7 @@
>>
>> #include <linux/slab.h>
>> #include <asm/unaligned.h>
>> +#include <linux/bitfield.h>
>>
>> #include "xhci.h"
>> #include "xhci-trace.h"
>> @@ -52,7 +53,239 @@ static u8 usb_bos_descriptor [] = {
>> 0xb5, 0x40, 0x0a, 0x00, /* 10Gbps, SSP, symmetric, tx, ID = 5 */
>> };
>>
>> -static int xhci_create_usb3_bos_desc(struct xhci_hcd *xhci, char *buf,
>> +/* Default sublink speed attribute of each lane */
>> +static u32 ssp_cap_default_ssa[] = {
>> + 0x00050034, /* USB 3.0 SS Gen1x1 id:4 symmetric rx 5Gbps */
>> + 0x000500b4, /* USB 3.0 SS Gen1x1 id:4 symmetric tx 5Gbps */
>> + 0x000a4035, /* USB 3.1 SSP Gen2x1 id:5 symmetric rx 10Gbps */
>> + 0x000a40b5, /* USB 3.1 SSP Gen2x1 id:5 symmetric tx 10Gbps */
>> + 0x00054036, /* USB 3.2 SSP Gen1x2 id:6 symmetric rx 5Gbps */
>> + 0x000540b6, /* USB 3.2 SSP Gen1x2 id:6 symmetric tx 5Gbps */
>> + 0x000a4037, /* USB 3.2 SSP Gen2x2 id:7 symmetric rx 10Gbps */
>> + 0x000a40b7, /* USB 3.2 SSP Gen2x2 id:7 symmetric tx 10Gbps */
>> +};
>> +
>> +static int xhci_create_usb3x_bos_desc(struct xhci_hcd *xhci, char *buf,
>> + u16 wLength)
>> +{
>> + struct usb_bos_descriptor *bos;
>> + struct usb_ss_cap_descriptor *ss_cap;
>> + struct usb_ssp_cap_descriptor *ssp_cap;
>> + struct xhci_port_cap *port_cap = NULL;
>> + u16 bcdUSB;
>> + u32 reg;
>> + u32 min_rate = 0;
>> + u8 min_ssid;
>> + u8 ssac;
>> + u8 ssic;
>> + int offset;
>> + int i;
>> +
>> + /* BOS descriptor */
>> + bos = (struct usb_bos_descriptor *)buf;
>> + bos->bLength = USB_DT_BOS_SIZE;
>> + bos->bDescriptorType = USB_DT_BOS;
>> + bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE +
>> + USB_DT_USB_SS_CAP_SIZE);
>> + bos->bNumDeviceCaps = 1;
>> +
>> + /* Create the descriptor for port with the highest revision */
>> + for (i = 0; i < xhci->num_port_caps; i++) {
>> + u8 major = xhci->port_caps[i].maj_rev;
>> + u8 minor = xhci->port_caps[i].min_rev;
>> + u16 rev = (major << 8) | minor;
>> +
>> + if (i == 0 || bcdUSB < rev) {
>> + bcdUSB = rev;
>> + port_cap = &xhci->port_caps[i];
>> + }
>> + }
>> +
>> + if (bcdUSB >= 0x0310) {
>> + if (port_cap->psi_count) {
>> + u8 num_sym_ssa = 0;
>> +
>> + for (i = 0; i < port_cap->psi_count; i++) {
>> + if ((port_cap->psi[i] & PLT_MASK) == PLT_SYM)
>> + num_sym_ssa++;
>> + }
>> +
>> + ssac = port_cap->psi_count + num_sym_ssa - 1;
>> + ssic = port_cap->psi_uid_count - 1;
>> + } else {
>> + if (bcdUSB >= 0x0320)
>> + ssac = 7;
>> + else
>> + ssac = 3;
>> +
>> + ssic = (ssac + 1) / 2 - 1;
>> + }
>> +
>> + bos->bNumDeviceCaps++;
>> + bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE +
>> + USB_DT_USB_SS_CAP_SIZE +
>> + USB_DT_USB_SSP_CAP_SIZE(ssac));
>> + }
>> +
>> + if (wLength < USB_DT_BOS_SIZE + USB_DT_USB_SS_CAP_SIZE)
>> + return wLength;
>> +
>> + /* SuperSpeed USB Device Capability */
>> + ss_cap = (struct usb_ss_cap_descriptor *)&buf[USB_DT_BOS_SIZE];
>> + ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
>> + ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
>> + ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
>> + ss_cap->bmAttributes = 0; /* set later */
>> + ss_cap->wSpeedSupported = cpu_to_le16(USB_5GBPS_OPERATION);
>> + ss_cap->bFunctionalitySupport = USB_LOW_SPEED_OPERATION;
>> + ss_cap->bU1devExitLat = 0; /* set later */
>> + ss_cap->bU2DevExitLat = 0; /* set later */
>> +
>> + reg = readl(&xhci->cap_regs->hcc_params);
>> + if (HCC_LTC(reg))
>> + ss_cap->bmAttributes |= USB_LTM_SUPPORT;
>> +
>> + if ((xhci->quirks & XHCI_LPM_SUPPORT)) {
>> + reg = readl(&xhci->cap_regs->hcs_params3);
>> + ss_cap->bU1devExitLat = HCS_U1_LATENCY(reg);
>> + ss_cap->bU2DevExitLat = cpu_to_le16(HCS_U2_LATENCY(reg));
>> + }
>> +
>> + if (wLength < le16_to_cpu(bos->wTotalLength))
>> + return wLength;
>> +
>> + if (bcdUSB < 0x0310)
>> + return le16_to_cpu(bos->wTotalLength);
>> +
>> + ssp_cap = (struct usb_ssp_cap_descriptor *)&buf[USB_DT_BOS_SIZE +
>> + USB_DT_USB_SS_CAP_SIZE];
>> + 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, ssac) |
>> + FIELD_PREP(USB_SSP_SUBLINK_SPEED_IDS, ssic));
>> +
>> + if (!port_cap->psi_count) {
>> + for (i = 0; i < ssac + 1; i++)
>> + ssp_cap->bmSublinkSpeedAttr[i] =
>> + cpu_to_le32(ssp_cap_default_ssa[i]);
>> +
>> + min_ssid = 4;
>> + goto out;
>> + }
>> +
>> + offset = 0;
>> + for (i = 0; i < port_cap->psi_count; i++) {
>> + u32 psi;
>> + u32 attr;
>> + u8 ssid;
>> + u8 lp;
>> + u8 lse;
>> + u8 psie;
>> + u16 lane_mantissa;
>> + u16 psim;
>> + u16 plt;
>> +
>> + psi = port_cap->psi[i];
>> + ssid = XHCI_EXT_PORT_PSIV(psi);
>> + lp = XHCI_EXT_PORT_LP(psi);
>> + psie = XHCI_EXT_PORT_PSIE(psi);
>> + psim = XHCI_EXT_PORT_PSIM(psi);
>> + plt = psi & PLT_MASK;
>> +
>> + lse = psie;
>> + lane_mantissa = psim;
>> +
>> + /* Shift to Gbps and set SSP Link Protocol if 10Gpbs */
>> + for (; psie < USB_SSP_SUBLINK_SPEED_LSE_GBPS; psie++)
>> + psim /= 1000;
>> +
>> + if (!min_rate || psim < min_rate) {
>> + min_ssid = ssid;
>> + min_rate = psim;
>> + }
>> +
>> + /* Some host controllers don't set the link protocol for SSP */
>> + if (psim >= 10)
>> + lp = USB_SSP_SUBLINK_SPEED_LP_SSP;
> Maybe we should limit the above fix to older than USB 3.2 hosts.
> xHCI supporting Gen 1x2 can in theory have psim==10 even if it only supports
> SS link protocol, not SSP.
Gen 1x2 uses SuperSpeed Plus link protocol. See USB 3.2 spec section 3.2
>
> I'd guess newer USB 3.2 hosts have have fixed the LP bit.
>
>> +
>> + /*
>> + * PSIM and PSIE represent the total speed of PSI. The BOS
>> + * descriptor SSP sublink speed attribute lane mantissa
>> + * describes the lane speed. E.g. PSIM and PSIE for gen2x2
>> + * is 20Gbps, but the BOS descriptor lane speed mantissa is
>> + * 10Gbps. Check and modify the mantissa value to match the
>> + * lane speed.
>> + */
>> + if (bcdUSB == 0x0320 && plt == PLT_SYM) {
>> + /*
>> + * The PSI dword for gen1x2 and gen2x1 share the same
>> + * values. But the lane speed for gen1x2 is 5Gbps while
>> + * gen2x1 is 10Gbps. If the previous PSI dword SSID is
>> + * 5 and the PSIE and PSIM match with SSID 6, let's
>> + * assume that the controller follows the default speed
>> + * id with SSID 6 for gen1x2.
>> + */
>> + if (ssid == 6 && psie == 3 && psim == 10 && i) {
>> + u32 prev = port_cap->psi[i - 1];
>> +
>> + if ((prev & PLT_MASK) == PLT_SYM &&
>> + XHCI_EXT_PORT_PSIV(prev) == 5 &&
>> + XHCI_EXT_PORT_PSIE(prev) == 3 &&
>> + XHCI_EXT_PORT_PSIM(prev) == 10) {
>> + lse = USB_SSP_SUBLINK_SPEED_LSE_GBPS;
>> + lane_mantissa = 5;
> This is a very specific workaround designed for one default case.
Since we can't differentiate between gen1x2 or gen2x1 PSI dword, I want
to the driver to assume by default that the PSI represents gen2x1 unless
it falls into this very specific condition for gen1x2. It's not often
that a device is connected in gen1x2.
>
>> + }
>> + }
>> +
>> + if (psie == 3 && psim > 10) {
>> + lse = USB_SSP_SUBLINK_SPEED_LSE_GBPS;
>> + lane_mantissa = 10;
>> + }
> So is this.
This case assumes psim > 10 is for gen2x2 and each lane is capable of up
to 10Gbps. We may change this and set lane_mantissa = psim / 2. But it
complicates the GenXxY description check a bit since it means that we're
expecting some odd number between 5Gbps or 10Gbps.
> Maybe something more generic could be figured out.
> If we assume that bcdUSB >= 0x0320 cases have the Link Protocol (LP) bit is set correctly,
> we could do something like:
>
> if (bcdUSB >= 0x0320)
> if (LP && psim > 10) || (!LP && (psim > 5))
> psim = psim / 2 /* divide by lane count */
This won't work well because gen1x2 uses the same SSP link protocol, and
we don't have lane count in PSI dword.
>
> Not sure if above pseudocode works with SSIC cases
>
> We are also forcing the speeds to Gbps (current code does this as well)
> This is rounding down non-standard speeds. (SSIC at 2915 Mbps for example)
We're only forcing the speed to Gbps for PSI dwords that we determined
to be gen1x2 and gen2x2. We're not rounding down for others.
>
>> + }
>> +
>> + attr = (FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, ssid) |
>> + FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP, lp) |
>> + FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE, lse) |
>> + FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, lane_mantissa));
>> +
>> + switch (plt) {
>> + case PLT_SYM:
>> + attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
>> + USB_SSP_SUBLINK_SPEED_ST_SYM_RX);
>> + ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
>> +
>> + attr &= ~USB_SSP_SUBLINK_SPEED_ST;
>> + attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
>> + USB_SSP_SUBLINK_SPEED_ST_SYM_TX);
>> + ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
>> + break;
>> + case PLT_ASYM_RX:
>> + attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
>> + USB_SSP_SUBLINK_SPEED_ST_ASYM_RX);
>> + ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
>> + break;
>> + case PLT_ASYM_TX:
>> + attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
>> + USB_SSP_SUBLINK_SPEED_ST_ASYM_TX);
>> + ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
>> + break;
>> + }
>> + }
>> +out:
>> + ssp_cap->wFunctionalitySupport =
>> + 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));
>> +
>> + return le16_to_cpu(bos->wTotalLength);
>> +}
>> +
>> +static __maybe_unused int xhci_create_usb3_bos_desc(struct xhci_hcd *xhci, char *buf,
>> u16 wLength)
>> {
>> struct xhci_port_cap *port_cap = NULL;
>> @@ -1137,7 +1370,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
>> if (hcd->speed < HCD_USB3)
>> goto error;
>>
>> - retval = xhci_create_usb3_bos_desc(xhci, buf, wLength);
>> + retval = xhci_create_usb3x_bos_desc(xhci, buf, wLength);
>> spin_unlock_irqrestore(&xhci->lock, flags);
>> return retval;
>> case GetPortStatus:
>>
> This is anyways a lot better than what we currently have.
> Would be nice to get those xHCI PSIM values nicely converted to USB 3.x bmSuperSpeedAttr
> Lane Speed Mantissa.
>
> Otherwise everyting looks good
>
Yes, it would be nice if the PSI dwords can just be converted to
bmSublinkSpeedAttr. Thanks for the review.
BR,
Thinh
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 7/8] usb: xhci: Rewrite xhci_create_usb3_bos_desc()
2021-02-04 4:11 ` [PATCH v2 7/8] usb: xhci: Rewrite xhci_create_usb3_bos_desc() Thinh Nguyen
@ 2021-03-09 11:47 ` Mathias Nyman
2021-03-09 19:32 ` Thinh Nguyen
0 siblings, 1 reply; 5+ messages in thread
From: Mathias Nyman @ 2021-03-09 11:47 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, linux-usb, Mathias Nyman
On 4.2.2021 6.11, Thinh Nguyen wrote:
> The current xhci_create_usb3_bos_desc() uses a static bos u8 array and
> various magic numbers and offsets making it difficult to extend support
> for USB 3.2. Let's rewrite this entire function to support dual-lane in
> USB 3.2.
>
> The hub driver matches the port speed ID from the extended port status
> to the SSID of the sublink speed attributes to detect if the device
> supports SuperSpeed Plus. Currently we don't provide the default gen1x2
> and gen2x2 sublink speed capability descriptor for USB 3.2 roothub. The
> USB stack depends on this to detect and match the correct speed.
> In addition, if the xHCI host provides Protocol Speed ID (PSI)
> capability, then make sure to convert Protocol Speed ID Mantissa and
> Exponent (PSIM & PSIE) to lane speed for gen1x2 and gen2x2.
>
> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
> Changes in v2:
> - Use static array for SSA per Mathias suggestion
> - Check bcdUSB >= 0x320 instead of bcdUSB == 0x320 per Mathias suggestion
> - When host uses custom PSI, SSIC is the psi_uid_count and not SSAC. I missed
> this when transferring the previous logic over. Previous implementation
> was incorrect. Let's fix it here.
>
> drivers/usb/host/xhci-hub.c | 237 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 235 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 74c497fd3476..eddd139c2260 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -11,6 +11,7 @@
>
> #include <linux/slab.h>
> #include <asm/unaligned.h>
> +#include <linux/bitfield.h>
>
> #include "xhci.h"
> #include "xhci-trace.h"
> @@ -52,7 +53,239 @@ static u8 usb_bos_descriptor [] = {
> 0xb5, 0x40, 0x0a, 0x00, /* 10Gbps, SSP, symmetric, tx, ID = 5 */
> };
>
> -static int xhci_create_usb3_bos_desc(struct xhci_hcd *xhci, char *buf,
> +/* Default sublink speed attribute of each lane */
> +static u32 ssp_cap_default_ssa[] = {
> + 0x00050034, /* USB 3.0 SS Gen1x1 id:4 symmetric rx 5Gbps */
> + 0x000500b4, /* USB 3.0 SS Gen1x1 id:4 symmetric tx 5Gbps */
> + 0x000a4035, /* USB 3.1 SSP Gen2x1 id:5 symmetric rx 10Gbps */
> + 0x000a40b5, /* USB 3.1 SSP Gen2x1 id:5 symmetric tx 10Gbps */
> + 0x00054036, /* USB 3.2 SSP Gen1x2 id:6 symmetric rx 5Gbps */
> + 0x000540b6, /* USB 3.2 SSP Gen1x2 id:6 symmetric tx 5Gbps */
> + 0x000a4037, /* USB 3.2 SSP Gen2x2 id:7 symmetric rx 10Gbps */
> + 0x000a40b7, /* USB 3.2 SSP Gen2x2 id:7 symmetric tx 10Gbps */
> +};
> +
> +static int xhci_create_usb3x_bos_desc(struct xhci_hcd *xhci, char *buf,
> + u16 wLength)
> +{
> + struct usb_bos_descriptor *bos;
> + struct usb_ss_cap_descriptor *ss_cap;
> + struct usb_ssp_cap_descriptor *ssp_cap;
> + struct xhci_port_cap *port_cap = NULL;
> + u16 bcdUSB;
> + u32 reg;
> + u32 min_rate = 0;
> + u8 min_ssid;
> + u8 ssac;
> + u8 ssic;
> + int offset;
> + int i;
> +
> + /* BOS descriptor */
> + bos = (struct usb_bos_descriptor *)buf;
> + bos->bLength = USB_DT_BOS_SIZE;
> + bos->bDescriptorType = USB_DT_BOS;
> + bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE +
> + USB_DT_USB_SS_CAP_SIZE);
> + bos->bNumDeviceCaps = 1;
> +
> + /* Create the descriptor for port with the highest revision */
> + for (i = 0; i < xhci->num_port_caps; i++) {
> + u8 major = xhci->port_caps[i].maj_rev;
> + u8 minor = xhci->port_caps[i].min_rev;
> + u16 rev = (major << 8) | minor;
> +
> + if (i == 0 || bcdUSB < rev) {
> + bcdUSB = rev;
> + port_cap = &xhci->port_caps[i];
> + }
> + }
> +
> + if (bcdUSB >= 0x0310) {
> + if (port_cap->psi_count) {
> + u8 num_sym_ssa = 0;
> +
> + for (i = 0; i < port_cap->psi_count; i++) {
> + if ((port_cap->psi[i] & PLT_MASK) == PLT_SYM)
> + num_sym_ssa++;
> + }
> +
> + ssac = port_cap->psi_count + num_sym_ssa - 1;
> + ssic = port_cap->psi_uid_count - 1;
> + } else {
> + if (bcdUSB >= 0x0320)
> + ssac = 7;
> + else
> + ssac = 3;
> +
> + ssic = (ssac + 1) / 2 - 1;
> + }
> +
> + bos->bNumDeviceCaps++;
> + bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE +
> + USB_DT_USB_SS_CAP_SIZE +
> + USB_DT_USB_SSP_CAP_SIZE(ssac));
> + }
> +
> + if (wLength < USB_DT_BOS_SIZE + USB_DT_USB_SS_CAP_SIZE)
> + return wLength;
> +
> + /* SuperSpeed USB Device Capability */
> + ss_cap = (struct usb_ss_cap_descriptor *)&buf[USB_DT_BOS_SIZE];
> + ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
> + ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> + ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
> + ss_cap->bmAttributes = 0; /* set later */
> + ss_cap->wSpeedSupported = cpu_to_le16(USB_5GBPS_OPERATION);
> + ss_cap->bFunctionalitySupport = USB_LOW_SPEED_OPERATION;
> + ss_cap->bU1devExitLat = 0; /* set later */
> + ss_cap->bU2DevExitLat = 0; /* set later */
> +
> + reg = readl(&xhci->cap_regs->hcc_params);
> + if (HCC_LTC(reg))
> + ss_cap->bmAttributes |= USB_LTM_SUPPORT;
> +
> + if ((xhci->quirks & XHCI_LPM_SUPPORT)) {
> + reg = readl(&xhci->cap_regs->hcs_params3);
> + ss_cap->bU1devExitLat = HCS_U1_LATENCY(reg);
> + ss_cap->bU2DevExitLat = cpu_to_le16(HCS_U2_LATENCY(reg));
> + }
> +
> + if (wLength < le16_to_cpu(bos->wTotalLength))
> + return wLength;
> +
> + if (bcdUSB < 0x0310)
> + return le16_to_cpu(bos->wTotalLength);
> +
> + ssp_cap = (struct usb_ssp_cap_descriptor *)&buf[USB_DT_BOS_SIZE +
> + USB_DT_USB_SS_CAP_SIZE];
> + 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, ssac) |
> + FIELD_PREP(USB_SSP_SUBLINK_SPEED_IDS, ssic));
> +
> + if (!port_cap->psi_count) {
> + for (i = 0; i < ssac + 1; i++)
> + ssp_cap->bmSublinkSpeedAttr[i] =
> + cpu_to_le32(ssp_cap_default_ssa[i]);
> +
> + min_ssid = 4;
> + goto out;
> + }
> +
> + offset = 0;
> + for (i = 0; i < port_cap->psi_count; i++) {
> + u32 psi;
> + u32 attr;
> + u8 ssid;
> + u8 lp;
> + u8 lse;
> + u8 psie;
> + u16 lane_mantissa;
> + u16 psim;
> + u16 plt;
> +
> + psi = port_cap->psi[i];
> + ssid = XHCI_EXT_PORT_PSIV(psi);
> + lp = XHCI_EXT_PORT_LP(psi);
> + psie = XHCI_EXT_PORT_PSIE(psi);
> + psim = XHCI_EXT_PORT_PSIM(psi);
> + plt = psi & PLT_MASK;
> +
> + lse = psie;
> + lane_mantissa = psim;
> +
> + /* Shift to Gbps and set SSP Link Protocol if 10Gpbs */
> + for (; psie < USB_SSP_SUBLINK_SPEED_LSE_GBPS; psie++)
> + psim /= 1000;
> +
> + if (!min_rate || psim < min_rate) {
> + min_ssid = ssid;
> + min_rate = psim;
> + }
> +
> + /* Some host controllers don't set the link protocol for SSP */
> + if (psim >= 10)
> + lp = USB_SSP_SUBLINK_SPEED_LP_SSP;
Maybe we should limit the above fix to older than USB 3.2 hosts.
xHCI supporting Gen 1x2 can in theory have psim==10 even if it only supports
SS link protocol, not SSP.
I'd guess newer USB 3.2 hosts have have fixed the LP bit.
> +
> + /*
> + * PSIM and PSIE represent the total speed of PSI. The BOS
> + * descriptor SSP sublink speed attribute lane mantissa
> + * describes the lane speed. E.g. PSIM and PSIE for gen2x2
> + * is 20Gbps, but the BOS descriptor lane speed mantissa is
> + * 10Gbps. Check and modify the mantissa value to match the
> + * lane speed.
> + */
> + if (bcdUSB == 0x0320 && plt == PLT_SYM) {
> + /*
> + * The PSI dword for gen1x2 and gen2x1 share the same
> + * values. But the lane speed for gen1x2 is 5Gbps while
> + * gen2x1 is 10Gbps. If the previous PSI dword SSID is
> + * 5 and the PSIE and PSIM match with SSID 6, let's
> + * assume that the controller follows the default speed
> + * id with SSID 6 for gen1x2.
> + */
> + if (ssid == 6 && psie == 3 && psim == 10 && i) {
> + u32 prev = port_cap->psi[i - 1];
> +
> + if ((prev & PLT_MASK) == PLT_SYM &&
> + XHCI_EXT_PORT_PSIV(prev) == 5 &&
> + XHCI_EXT_PORT_PSIE(prev) == 3 &&
> + XHCI_EXT_PORT_PSIM(prev) == 10) {
> + lse = USB_SSP_SUBLINK_SPEED_LSE_GBPS;
> + lane_mantissa = 5;
This is a very specific workaround designed for one default case.
> + }
> + }
> +
> + if (psie == 3 && psim > 10) {
> + lse = USB_SSP_SUBLINK_SPEED_LSE_GBPS;
> + lane_mantissa = 10;
> + }
So is this.
Maybe something more generic could be figured out.
If we assume that bcdUSB >= 0x0320 cases have the Link Protocol (LP) bit is set correctly,
we could do something like:
if (bcdUSB >= 0x0320)
if (LP && psim > 10) || (!LP && (psim > 5))
psim = psim / 2 /* divide by lane count */
Not sure if above pseudocode works with SSIC cases
We are also forcing the speeds to Gbps (current code does this as well)
This is rounding down non-standard speeds. (SSIC at 2915 Mbps for example)
> + }
> +
> + attr = (FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, ssid) |
> + FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP, lp) |
> + FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE, lse) |
> + FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, lane_mantissa));
> +
> + switch (plt) {
> + case PLT_SYM:
> + attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
> + USB_SSP_SUBLINK_SPEED_ST_SYM_RX);
> + ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
> +
> + attr &= ~USB_SSP_SUBLINK_SPEED_ST;
> + attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
> + USB_SSP_SUBLINK_SPEED_ST_SYM_TX);
> + ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
> + break;
> + case PLT_ASYM_RX:
> + attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
> + USB_SSP_SUBLINK_SPEED_ST_ASYM_RX);
> + ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
> + break;
> + case PLT_ASYM_TX:
> + attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
> + USB_SSP_SUBLINK_SPEED_ST_ASYM_TX);
> + ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
> + break;
> + }
> + }
> +out:
> + ssp_cap->wFunctionalitySupport =
> + 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));
> +
> + return le16_to_cpu(bos->wTotalLength);
> +}
> +
> +static __maybe_unused int xhci_create_usb3_bos_desc(struct xhci_hcd *xhci, char *buf,
> u16 wLength)
> {
> struct xhci_port_cap *port_cap = NULL;
> @@ -1137,7 +1370,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
> if (hcd->speed < HCD_USB3)
> goto error;
>
> - retval = xhci_create_usb3_bos_desc(xhci, buf, wLength);
> + retval = xhci_create_usb3x_bos_desc(xhci, buf, wLength);
> spin_unlock_irqrestore(&xhci->lock, flags);
> return retval;
> case GetPortStatus:
>
This is anyways a lot better than what we currently have.
Would be nice to get those xHCI PSIM values nicely converted to USB 3.x bmSuperSpeedAttr
Lane Speed Mantissa.
Otherwise everyting looks good
Thanks
-Mathias
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 7/8] usb: xhci: Rewrite xhci_create_usb3_bos_desc()
2021-02-04 4:10 [PATCH v2 0/8] usb: Check for genXxY on host Thinh Nguyen
@ 2021-02-04 4:11 ` Thinh Nguyen
2021-03-09 11:47 ` Mathias Nyman
0 siblings, 1 reply; 5+ messages in thread
From: Thinh Nguyen @ 2021-02-04 4:11 UTC (permalink / raw)
To: Greg Kroah-Hartman, Thinh.Nguyen, linux-usb, Mathias Nyman
The current xhci_create_usb3_bos_desc() uses a static bos u8 array and
various magic numbers and offsets making it difficult to extend support
for USB 3.2. Let's rewrite this entire function to support dual-lane in
USB 3.2.
The hub driver matches the port speed ID from the extended port status
to the SSID of the sublink speed attributes to detect if the device
supports SuperSpeed Plus. Currently we don't provide the default gen1x2
and gen2x2 sublink speed capability descriptor for USB 3.2 roothub. The
USB stack depends on this to detect and match the correct speed.
In addition, if the xHCI host provides Protocol Speed ID (PSI)
capability, then make sure to convert Protocol Speed ID Mantissa and
Exponent (PSIM & PSIE) to lane speed for gen1x2 and gen2x2.
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
---
Changes in v2:
- Use static array for SSA per Mathias suggestion
- Check bcdUSB >= 0x320 instead of bcdUSB == 0x320 per Mathias suggestion
- When host uses custom PSI, SSIC is the psi_uid_count and not SSAC. I missed
this when transferring the previous logic over. Previous implementation
was incorrect. Let's fix it here.
drivers/usb/host/xhci-hub.c | 237 +++++++++++++++++++++++++++++++++++-
1 file changed, 235 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 74c497fd3476..eddd139c2260 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -11,6 +11,7 @@
#include <linux/slab.h>
#include <asm/unaligned.h>
+#include <linux/bitfield.h>
#include "xhci.h"
#include "xhci-trace.h"
@@ -52,7 +53,239 @@ static u8 usb_bos_descriptor [] = {
0xb5, 0x40, 0x0a, 0x00, /* 10Gbps, SSP, symmetric, tx, ID = 5 */
};
-static int xhci_create_usb3_bos_desc(struct xhci_hcd *xhci, char *buf,
+/* Default sublink speed attribute of each lane */
+static u32 ssp_cap_default_ssa[] = {
+ 0x00050034, /* USB 3.0 SS Gen1x1 id:4 symmetric rx 5Gbps */
+ 0x000500b4, /* USB 3.0 SS Gen1x1 id:4 symmetric tx 5Gbps */
+ 0x000a4035, /* USB 3.1 SSP Gen2x1 id:5 symmetric rx 10Gbps */
+ 0x000a40b5, /* USB 3.1 SSP Gen2x1 id:5 symmetric tx 10Gbps */
+ 0x00054036, /* USB 3.2 SSP Gen1x2 id:6 symmetric rx 5Gbps */
+ 0x000540b6, /* USB 3.2 SSP Gen1x2 id:6 symmetric tx 5Gbps */
+ 0x000a4037, /* USB 3.2 SSP Gen2x2 id:7 symmetric rx 10Gbps */
+ 0x000a40b7, /* USB 3.2 SSP Gen2x2 id:7 symmetric tx 10Gbps */
+};
+
+static int xhci_create_usb3x_bos_desc(struct xhci_hcd *xhci, char *buf,
+ u16 wLength)
+{
+ struct usb_bos_descriptor *bos;
+ struct usb_ss_cap_descriptor *ss_cap;
+ struct usb_ssp_cap_descriptor *ssp_cap;
+ struct xhci_port_cap *port_cap = NULL;
+ u16 bcdUSB;
+ u32 reg;
+ u32 min_rate = 0;
+ u8 min_ssid;
+ u8 ssac;
+ u8 ssic;
+ int offset;
+ int i;
+
+ /* BOS descriptor */
+ bos = (struct usb_bos_descriptor *)buf;
+ bos->bLength = USB_DT_BOS_SIZE;
+ bos->bDescriptorType = USB_DT_BOS;
+ bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE +
+ USB_DT_USB_SS_CAP_SIZE);
+ bos->bNumDeviceCaps = 1;
+
+ /* Create the descriptor for port with the highest revision */
+ for (i = 0; i < xhci->num_port_caps; i++) {
+ u8 major = xhci->port_caps[i].maj_rev;
+ u8 minor = xhci->port_caps[i].min_rev;
+ u16 rev = (major << 8) | minor;
+
+ if (i == 0 || bcdUSB < rev) {
+ bcdUSB = rev;
+ port_cap = &xhci->port_caps[i];
+ }
+ }
+
+ if (bcdUSB >= 0x0310) {
+ if (port_cap->psi_count) {
+ u8 num_sym_ssa = 0;
+
+ for (i = 0; i < port_cap->psi_count; i++) {
+ if ((port_cap->psi[i] & PLT_MASK) == PLT_SYM)
+ num_sym_ssa++;
+ }
+
+ ssac = port_cap->psi_count + num_sym_ssa - 1;
+ ssic = port_cap->psi_uid_count - 1;
+ } else {
+ if (bcdUSB >= 0x0320)
+ ssac = 7;
+ else
+ ssac = 3;
+
+ ssic = (ssac + 1) / 2 - 1;
+ }
+
+ bos->bNumDeviceCaps++;
+ bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE +
+ USB_DT_USB_SS_CAP_SIZE +
+ USB_DT_USB_SSP_CAP_SIZE(ssac));
+ }
+
+ if (wLength < USB_DT_BOS_SIZE + USB_DT_USB_SS_CAP_SIZE)
+ return wLength;
+
+ /* SuperSpeed USB Device Capability */
+ ss_cap = (struct usb_ss_cap_descriptor *)&buf[USB_DT_BOS_SIZE];
+ ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
+ ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
+ ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
+ ss_cap->bmAttributes = 0; /* set later */
+ ss_cap->wSpeedSupported = cpu_to_le16(USB_5GBPS_OPERATION);
+ ss_cap->bFunctionalitySupport = USB_LOW_SPEED_OPERATION;
+ ss_cap->bU1devExitLat = 0; /* set later */
+ ss_cap->bU2DevExitLat = 0; /* set later */
+
+ reg = readl(&xhci->cap_regs->hcc_params);
+ if (HCC_LTC(reg))
+ ss_cap->bmAttributes |= USB_LTM_SUPPORT;
+
+ if ((xhci->quirks & XHCI_LPM_SUPPORT)) {
+ reg = readl(&xhci->cap_regs->hcs_params3);
+ ss_cap->bU1devExitLat = HCS_U1_LATENCY(reg);
+ ss_cap->bU2DevExitLat = cpu_to_le16(HCS_U2_LATENCY(reg));
+ }
+
+ if (wLength < le16_to_cpu(bos->wTotalLength))
+ return wLength;
+
+ if (bcdUSB < 0x0310)
+ return le16_to_cpu(bos->wTotalLength);
+
+ ssp_cap = (struct usb_ssp_cap_descriptor *)&buf[USB_DT_BOS_SIZE +
+ USB_DT_USB_SS_CAP_SIZE];
+ 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, ssac) |
+ FIELD_PREP(USB_SSP_SUBLINK_SPEED_IDS, ssic));
+
+ if (!port_cap->psi_count) {
+ for (i = 0; i < ssac + 1; i++)
+ ssp_cap->bmSublinkSpeedAttr[i] =
+ cpu_to_le32(ssp_cap_default_ssa[i]);
+
+ min_ssid = 4;
+ goto out;
+ }
+
+ offset = 0;
+ for (i = 0; i < port_cap->psi_count; i++) {
+ u32 psi;
+ u32 attr;
+ u8 ssid;
+ u8 lp;
+ u8 lse;
+ u8 psie;
+ u16 lane_mantissa;
+ u16 psim;
+ u16 plt;
+
+ psi = port_cap->psi[i];
+ ssid = XHCI_EXT_PORT_PSIV(psi);
+ lp = XHCI_EXT_PORT_LP(psi);
+ psie = XHCI_EXT_PORT_PSIE(psi);
+ psim = XHCI_EXT_PORT_PSIM(psi);
+ plt = psi & PLT_MASK;
+
+ lse = psie;
+ lane_mantissa = psim;
+
+ /* Shift to Gbps and set SSP Link Protocol if 10Gpbs */
+ for (; psie < USB_SSP_SUBLINK_SPEED_LSE_GBPS; psie++)
+ psim /= 1000;
+
+ if (!min_rate || psim < min_rate) {
+ min_ssid = ssid;
+ min_rate = psim;
+ }
+
+ /* Some host controllers don't set the link protocol for SSP */
+ if (psim >= 10)
+ lp = USB_SSP_SUBLINK_SPEED_LP_SSP;
+
+ /*
+ * PSIM and PSIE represent the total speed of PSI. The BOS
+ * descriptor SSP sublink speed attribute lane mantissa
+ * describes the lane speed. E.g. PSIM and PSIE for gen2x2
+ * is 20Gbps, but the BOS descriptor lane speed mantissa is
+ * 10Gbps. Check and modify the mantissa value to match the
+ * lane speed.
+ */
+ if (bcdUSB == 0x0320 && plt == PLT_SYM) {
+ /*
+ * The PSI dword for gen1x2 and gen2x1 share the same
+ * values. But the lane speed for gen1x2 is 5Gbps while
+ * gen2x1 is 10Gbps. If the previous PSI dword SSID is
+ * 5 and the PSIE and PSIM match with SSID 6, let's
+ * assume that the controller follows the default speed
+ * id with SSID 6 for gen1x2.
+ */
+ if (ssid == 6 && psie == 3 && psim == 10 && i) {
+ u32 prev = port_cap->psi[i - 1];
+
+ if ((prev & PLT_MASK) == PLT_SYM &&
+ XHCI_EXT_PORT_PSIV(prev) == 5 &&
+ XHCI_EXT_PORT_PSIE(prev) == 3 &&
+ XHCI_EXT_PORT_PSIM(prev) == 10) {
+ lse = USB_SSP_SUBLINK_SPEED_LSE_GBPS;
+ lane_mantissa = 5;
+ }
+ }
+
+ if (psie == 3 && psim > 10) {
+ lse = USB_SSP_SUBLINK_SPEED_LSE_GBPS;
+ lane_mantissa = 10;
+ }
+ }
+
+ attr = (FIELD_PREP(USB_SSP_SUBLINK_SPEED_SSID, ssid) |
+ FIELD_PREP(USB_SSP_SUBLINK_SPEED_LP, lp) |
+ FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSE, lse) |
+ FIELD_PREP(USB_SSP_SUBLINK_SPEED_LSM, lane_mantissa));
+
+ switch (plt) {
+ case PLT_SYM:
+ attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
+ USB_SSP_SUBLINK_SPEED_ST_SYM_RX);
+ ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
+
+ attr &= ~USB_SSP_SUBLINK_SPEED_ST;
+ attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
+ USB_SSP_SUBLINK_SPEED_ST_SYM_TX);
+ ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
+ break;
+ case PLT_ASYM_RX:
+ attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
+ USB_SSP_SUBLINK_SPEED_ST_ASYM_RX);
+ ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
+ break;
+ case PLT_ASYM_TX:
+ attr |= FIELD_PREP(USB_SSP_SUBLINK_SPEED_ST,
+ USB_SSP_SUBLINK_SPEED_ST_ASYM_TX);
+ ssp_cap->bmSublinkSpeedAttr[offset++] = cpu_to_le32(attr);
+ break;
+ }
+ }
+out:
+ ssp_cap->wFunctionalitySupport =
+ 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));
+
+ return le16_to_cpu(bos->wTotalLength);
+}
+
+static __maybe_unused int xhci_create_usb3_bos_desc(struct xhci_hcd *xhci, char *buf,
u16 wLength)
{
struct xhci_port_cap *port_cap = NULL;
@@ -1137,7 +1370,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
if (hcd->speed < HCD_USB3)
goto error;
- retval = xhci_create_usb3_bos_desc(xhci, buf, wLength);
+ retval = xhci_create_usb3x_bos_desc(xhci, buf, wLength);
spin_unlock_irqrestore(&xhci->lock, flags);
return retval;
case GetPortStatus:
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-03-10 8:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-09 5:37 [PATCH v2 7/8] usb: xhci: Rewrite xhci_create_usb3_bos_desc() kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2021-02-04 4:10 [PATCH v2 0/8] usb: Check for genXxY on host Thinh Nguyen
2021-02-04 4:11 ` [PATCH v2 7/8] usb: xhci: Rewrite xhci_create_usb3_bos_desc() Thinh Nguyen
2021-03-09 11:47 ` Mathias Nyman
2021-03-09 19:32 ` Thinh Nguyen
2021-03-10 8:28 ` Mathias Nyman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.