Re: [PATCH v3 1/3] usb: xhci: Add support for eUSB2v2 1024-byte bulk packet size

[Oliver Neukum]

On 05.10.26 10:18, Pawel Laszczak via B4 Relay wrote:
> From: Pawel Laszczak <pawell@xxxxxxxxxxx>
>
> The eUSB2 v2 specification (bcdUSB 0x0230) introduces support for
> 1024-byte maximum packet sizes for Bulk endpoints in High-Speed mode.
> However, an eUSB2v2 peripheral will revert its internal maximum packet
> size back to 512 bytes after events like a bus reset, disconnect, or
> deconfiguration.
>
> To support 1024-byte bulk transfers on capable hosts, add a new
> is_eusb2v2 flag to the usb_bus structure, populated via the HCCPARAMS2
> E2V2C capability bit in the xHCI driver.
>
> When an eUSB2v2 host configures an eUSB2v2 device, issue a specific
> SET_FEATURE (USB_DEVICE_BULK_MAX_PACKET_UPDATE) request during device
> configuration to switch the peripheral to 1024-byte packet mode, and
> allow the xHCI endpoint initialization to accept up to 1024 bytes for
> HS bulk endpoints.

[..]
> +int eusb_update_max_packet(struct usb_device *udev, struct usb_host_config *cp)
> +{
> +	struct usb_host_config *config = cp ? cp : udev->actconfig;
> +	struct usb_hcd *hcd = bus_to_hcd(udev->bus);
> +	struct usb_interface_cache *intfc;
> +	struct usb_host_interface *alt;
> +	struct usb_host_endpoint *ep;
> +	bool has_bulk = false;
> +	int i, j, a;
> +	int ret;
> +
> +	if (le16_to_cpu(udev->descriptor.bcdUSB) != 0x0230 ||
> +	    !hcd->self.is_eusb2v2)
> +		return 0;
> +
> +	if (!config)
> +		return 0;
> +
> +	for (i = 0; i < config->desc.bNumInterfaces; i++) {
> +		intfc = config->intf_cache[i];
> +
> +		if (!intfc)
> +			continue;
> +
> +		for (a = 0; a < intfc->num_altsetting; a++) {
> +			alt = &intfc->altsetting[a];
> +
> +			for (j = 0; j < alt->desc.bNumEndpoints; j++) {
> +				ep = &alt->endpoint[j];
> +
> +				if (usb_endpoint_xfer_bulk(&ep->desc)) {
> +					has_bulk = true;
> +					goto found_bulk;
> +				}
> +			}
> +		}
> +	}
> +
> +	if (!has_bulk)
> +		return 0;
> +
> +found_bulk:
> +	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> +			      USB_REQ_SET_FEATURE, USB_RECIP_DEVICE,
> +			      USB_DEVICE_BULK_MAX_PACKET_UPDATE, 0, NULL, 0,
> +			      USB_CTRL_SET_TIMEOUT);
> +	if (ret < 0) {
> +		dev_warn(&udev->dev, "eUSB2v2 1KB update failed: %d\n", ret);
> +		return ret;
> +	}

Why not handle eusb2v2_mps_active here?

> +
> +	return 0;
> +}
> +
>   /*
>    * usb_set_configuration - Makes a particular device setting be current
>    * @dev: the device whose configuration is being updated
> @@ -2123,6 +2188,15 @@ int usb_set_configuration(struct usb_device *dev, int configuration)
>   	if (dev->state != USB_STATE_ADDRESS)
>   		usb_disable_device(dev, 1);	/* Skip ep0 */
>   
> +	ret = eusb_update_max_packet(dev, cp);
> +	if (ret < 0)
> +		dev->eusb2v2_mps_active = 0;
> +	else if (cp && le16_to_cpu(dev->descriptor.bcdUSB) == 0x0230 &&
> +		 hcd->self.is_eusb2v2)
> +		dev->eusb2v2_mps_active = 1;
> +	else
> +		dev->eusb2v2_mps_active = 0;
> +

You are effectively duplicating the logic of eusb_update_max_packet()

	Regards
		Oliver