[PATCH v4 0/4] eUSB2 Double Isochronous IN Bandwidth support

[PATCH v4 0/4] eUSB2 Double Isochronous IN Bandwidth support

[Sakari Ailus]

Hi all,

This series enables support for eUSB2 Double Isochronous IN Bandwidth UVC
devices specified in 'USB 2.0 Double Isochronous IN Bandwidth' ECN. In
short, it adds support for new integrated USB2 webcams that can send twice
the data compared to conventional USB2 webcams.

These devices are identified by the device descriptor bcdUSB 0x0220 value.
They have an additional eUSB2 Isochronous Endpoint Companion Descriptor,
and a zero max packet size in regular isoc endpoint descriptor. Support
for parsing that new descriptor was added in commit

c749f058b437 ("USB: core: Add eUSB2 descriptor and parsing in USB core")

This series adds support to UVC, USB core, and xHCI to identify eUSB2
double isoc devices, and allow and set proper max packet, iso frame desc
sizes, bytes per interval, and other values in URBs and xHCI endpoint
contexts needed to support the double data rates for eUSB2 double isoc
devices.

v1 can be found here
<URL:https://lore.kernel.org/linux-usb/20250616093730.2569328-2-mathias.nyman@linux.intel.com/>.

v2 can be found here
<URL:https://lore.kernel.org/linux-usb/20250711083413.1552423-1-sakari.ailus@linux.intel.com/>.

v3 can be found here
<URL:https://lore.kernel.org/linux-usb/20250807055355.1257029-1-sakari.ailus@linux.intel.com/>.

since v3:

- Use spaces in aligning macro body for HCC2_EUSB2_DIC() (1st patch).

- Move usb_endpoint_max_isoc_bpi() to drivers/usb/core/usb.c (3rd patch).

since v2:

- Use ep->eusb2_isoc_ep_comp.bDescriptorType to determined whether the
  eUSB2 isochronous endpoint companion descriptor exists.

- Clean up eUSB2 double isoc bw maxp calculation.

- Drop le16_to_cpu(udev->descriptor.bcdUSB) == 0x220 check from
  xhci_eusb2_is_isoc_bw_double() -- it's redundant as
  ep->eusb2_isoc_ep_comp.dwBytesPerInterval will be zero otherwise.

- Add kernel-doc documentation for usb_endpoint_max_isoc_bpi().

- Check the endpoint has IN direction in usb_endpoint_max_isoc_bpi() and
  usb_submit_urb() as a condition for eUSB2 isoc double bw.

since v1:

- Introduce uvc_endpoint_max_isoc_bpi() to obtain maximum bytes per
  interval value for an endpoint, in a new patch (3rd). This code has been
  slightly reworked from the instance in the UVC driver, including support
  for SuperSpeedPlus Isochronous Endpoint Companion.

- Use usb_endpoint_max_isoc_bpi() in the UVC driver instead of open-coding
  eUSB2 support there, also drop now-redundant uvc_endpoint_max_bpi().

- Use u32 for maximum bpi and related information in the UVC driver -- the
  value could be larger than a 16-bit type can hold.

- Assume max in usb_submit_urb() is a natural number as
  usb_endpoint_maxp() returns only natural numbers (2nd patch).

Rai, Amardeep (3):
  xhci: Add host support for eUSB2 double isochronous bandwidth devices
  USB: core: support eUSB2 double bandwidth large isoc URB frames
  USB: Add a function to obtain USB version independent maximum bpi
    value

Tao Q Tao (1):
  media: uvcvideo: eUSB2 double isochronous bandwidth support

 drivers/media/usb/uvc/uvc_driver.c |  4 +-
 drivers/media/usb/uvc/uvc_video.c  | 24 ++----------
 drivers/media/usb/uvc/uvcvideo.h   |  4 +-
 drivers/usb/core/urb.c             | 17 +++++++--
 drivers/usb/core/usb.c             | 32 ++++++++++++++++
 drivers/usb/host/xhci-caps.h       |  2 +
 drivers/usb/host/xhci-mem.c        | 60 ++++++++++++++++++++++++------
 drivers/usb/host/xhci-ring.c       |  6 +--
 drivers/usb/host/xhci.c            | 16 +++++++-
 drivers/usb/host/xhci.h            | 19 ++++++++++
 include/linux/usb.h                |  3 ++
 11 files changed, 141 insertions(+), 46 deletions(-)

-- 
2.39.5

[PATCH v4 1/4] xhci: Add host support for eUSB2 double isochronous bandwidth devices

[Sakari Ailus]

From: "Rai, Amardeep" <amardeep.rai@intel.com>

Detect eUSB2 double isoc bw capable hosts and devices, and set the proper
xhci endpoint context values such as 'Mult', 'Max Burst Size', and 'Max
ESIT Payload' to enable the double isochronous bandwidth endpoints.

Intel xHC uses the endpoint context 'Mult' field for eUSB2 isoc
endpoints even if hosts supporting Large ESIT Payload Capability should
normally ignore the mult field.

Signed-off-by: Rai, Amardeep <amardeep.rai@intel.com>
Co-developed-by: Kannappan R <r.kannappan@intel.com>
Signed-off-by: Kannappan R <r.kannappan@intel.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Co-developed-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/usb/host/xhci-caps.h |  2 ++
 drivers/usb/host/xhci-mem.c  | 60 ++++++++++++++++++++++++++++--------
 drivers/usb/host/xhci-ring.c |  6 ++--
 drivers/usb/host/xhci.c      | 16 +++++++++-
 drivers/usb/host/xhci.h      | 19 ++++++++++++
 5 files changed, 87 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
index 4b8ff4815644..89bc83e4f1eb 100644
--- a/drivers/usb/host/xhci-caps.h
+++ b/drivers/usb/host/xhci-caps.h
@@ -89,3 +89,5 @@
 #define HCC2_GSC(p)             ((p) & (1 << 8))
 /* true: HC support Virtualization Based Trusted I/O Capability */
 #define HCC2_VTC(p)             ((p) & (1 << 9))
+/* true: HC support Double BW on a eUSB2 HS ISOC EP */
+#define HCC2_EUSB2_DIC(p)       ((p) & (1 << 11))
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6680afa4f596..ea51434c80fa 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1328,18 +1328,36 @@ static unsigned int xhci_get_endpoint_interval(struct usb_device *udev,
 	return interval;
 }
 
-/* The "Mult" field in the endpoint context is only set for SuperSpeed isoc eps.
+/*
+ * xHCs without LEC use the "Mult" field in the endpoint context for SuperSpeed
+ * isoc eps, and High speed isoc eps that support bandwidth doubling. Standard
  * High speed endpoint descriptors can define "the number of additional
  * transaction opportunities per microframe", but that goes in the Max Burst
  * endpoint context field.
  */
-static u32 xhci_get_endpoint_mult(struct usb_device *udev,
-		struct usb_host_endpoint *ep)
+static u32 xhci_get_endpoint_mult(struct xhci_hcd *xhci,
+				  struct usb_device *udev,
+				  struct usb_host_endpoint *ep)
 {
-	if (udev->speed < USB_SPEED_SUPER ||
-			!usb_endpoint_xfer_isoc(&ep->desc))
+	bool lec;
+
+	/* xHCI 1.1 with LEC set does not use mult field, except intel eUSB2 */
+	lec = xhci->hci_version > 0x100 && HCC2_LEC(xhci->hcc_params2);
+
+	/* eUSB2 double isoc bw devices are the only USB2 devices using mult */
+	if (xhci_eusb2_is_isoc_bw_double(udev, ep)) {
+		if (!lec || xhci->quirks & XHCI_INTEL_HOST)
+			return 1;
+	}
+
+	/* Oherwise only isoc transfers on hosts without LEC uses mult field */
+	if (!usb_endpoint_xfer_isoc(&ep->desc) || lec)
 		return 0;
-	return ep->ss_ep_comp.bmAttributes;
+
+	if (udev->speed >= USB_SPEED_SUPER)
+		return ep->ss_ep_comp.bmAttributes;
+
+	return 0;
 }
 
 static u32 xhci_get_endpoint_max_burst(struct usb_device *udev,
@@ -1351,8 +1369,18 @@ static u32 xhci_get_endpoint_max_burst(struct usb_device *udev,
 
 	if (udev->speed == USB_SPEED_HIGH &&
 	    (usb_endpoint_xfer_isoc(&ep->desc) ||
-	     usb_endpoint_xfer_int(&ep->desc)))
+	     usb_endpoint_xfer_int(&ep->desc))) {
+		/*
+		 * eUSB2 double isoc bw endpoints max packet field service
+		 * opportunity bits 12:11 are not valid, so set the ctx burst to
+		 * max service opportunity "2" as these eps support transferring
+		 * over 3072 bytes per interval
+		 */
+		if (xhci_eusb2_is_isoc_bw_double(udev, ep))
+			return 2;
+
 		return usb_endpoint_maxp_mult(&ep->desc) - 1;
+	}
 
 	return 0;
 }
@@ -1400,6 +1428,10 @@ static u32 xhci_get_max_esit_payload(struct usb_device *udev,
 	if (udev->speed >= USB_SPEED_SUPER)
 		return le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval);
 
+	/* High speed eUSB2 double isoc bw with over 3072 bytes per esit */
+	if (xhci_eusb2_is_isoc_bw_double(udev, ep))
+		return le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval);
+
 	max_packet = usb_endpoint_maxp(&ep->desc);
 	max_burst = usb_endpoint_maxp_mult(&ep->desc);
 	/* A 0 in max burst means 1 transfer per ESIT */
@@ -1437,6 +1469,13 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
 
 	ring_type = usb_endpoint_type(&ep->desc);
 
+	/* Ensure host supports double isoc bandwidth for eUSB2 devices */
+	if (xhci_eusb2_is_isoc_bw_double(udev, ep) &&
+	    !HCC2_EUSB2_DIC(xhci->hcc_params2))	{
+		dev_dbg(&udev->dev, "Double Isoc Bandwidth not supported by xhci\n");
+		return -EINVAL;
+	}
+
 	/*
 	 * Get values to fill the endpoint context, mostly from ep descriptor.
 	 * The average TRB buffer lengt for bulk endpoints is unclear as we
@@ -1460,8 +1499,8 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
 		}
 	}
 
-	mult = xhci_get_endpoint_mult(udev, ep);
-	max_packet = usb_endpoint_maxp(&ep->desc);
+	mult = xhci_get_endpoint_mult(xhci, udev, ep);
+	max_packet = xhci_usb_endpoint_maxp(udev, ep);
 	max_burst = xhci_get_endpoint_max_burst(udev, ep);
 	avg_trb_len = max_esit_payload;
 
@@ -1482,9 +1521,6 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
 	/* xHCI 1.0 and 1.1 indicates that ctrl ep avg TRB Length should be 8 */
 	if (usb_endpoint_xfer_control(&ep->desc) && xhci->hci_version >= 0x100)
 		avg_trb_len = 8;
-	/* xhci 1.1 with LEC support doesn't use mult field, use RsvdZ */
-	if ((xhci->hci_version > 0x100) && HCC2_LEC(xhci->hcc_params2))
-		mult = 0;
 
 	/* Set up the endpoint ring */
 	virt_dev->eps[ep_index].new_ring =
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 94c9c9271658..9abc5bc4e1c0 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3542,7 +3542,7 @@ static u32 xhci_td_remainder(struct xhci_hcd *xhci, int transferred,
 	if ((xhci->quirks & XHCI_MTK_HOST) && (xhci->hci_version < 0x100))
 		trb_buff_len = 0;
 
-	maxp = usb_endpoint_maxp(&urb->ep->desc);
+	maxp = xhci_usb_endpoint_maxp(urb->dev, urb->ep);
 	total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
 
 	/* Queueing functions don't count the current TRB into transferred */
@@ -3559,7 +3559,7 @@ static int xhci_align_td(struct xhci_hcd *xhci, struct urb *urb, u32 enqd_len,
 	u32 new_buff_len;
 	size_t len;
 
-	max_pkt = usb_endpoint_maxp(&urb->ep->desc);
+	max_pkt = xhci_usb_endpoint_maxp(urb->dev, urb->ep);
 	unalign = (enqd_len + *trb_buff_len) % max_pkt;
 
 	/* we got lucky, last normal TRB data on segment is packet aligned */
@@ -4130,7 +4130,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 		addr = start_addr + urb->iso_frame_desc[i].offset;
 		td_len = urb->iso_frame_desc[i].length;
 		td_remain_len = td_len;
-		max_pkt = usb_endpoint_maxp(&urb->ep->desc);
+		max_pkt = xhci_usb_endpoint_maxp(urb->dev, urb->ep);
 		total_pkt_count = DIV_ROUND_UP(td_len, max_pkt);
 
 		/* A zero-length transfer still involves at least one packet. */
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 8a819e853288..1ea7d2fb5876 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1332,7 +1332,7 @@ static bool xhci_urb_temp_buffer_required(struct usb_hcd *hcd,
 	struct scatterlist *tail_sg;
 
 	tail_sg = urb->sg;
-	max_pkt = usb_endpoint_maxp(&urb->ep->desc);
+	max_pkt = xhci_usb_endpoint_maxp(urb->dev, urb->ep);
 
 	if (!urb->num_sgs)
 		return ret;
@@ -2920,6 +2920,20 @@ int xhci_stop_endpoint_sync(struct xhci_hcd *xhci, struct xhci_virt_ep *ep, int
 }
 EXPORT_SYMBOL_GPL(xhci_stop_endpoint_sync);
 
+/*
+ * xhci_usb_endpoint_maxp - get endpoint max packet size
+ * @host_ep: USB host endpoint to be checked
+ *
+ * Returns max packet from the correct descriptor
+ */
+int xhci_usb_endpoint_maxp(struct usb_device *udev,
+			   struct usb_host_endpoint *host_ep)
+{
+	if (xhci_eusb2_is_isoc_bw_double(udev, host_ep))
+		return le16_to_cpu(host_ep->eusb2_isoc_ep_comp.wMaxPacketSize);
+	return usb_endpoint_maxp(&host_ep->desc);
+}
+
 /* Issue a configure endpoint command or evaluate context command
  * and wait for it to finish.
  */
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index a20f4e7cd43a..9657a4deb87b 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1736,6 +1736,23 @@ static inline bool xhci_has_one_roothub(struct xhci_hcd *xhci)
 	       (!xhci->usb2_rhub.num_ports || !xhci->usb3_rhub.num_ports);
 }
 
+/*
+ * USB 2.0 specification, chapter 5.6.4 Isochronous Transfer Bus Access
+ * Constraint. A high speed endpoint can move up to 3072 bytes per microframe
+ * (or 192Mb/s).
+ */
+#define MAX_ISOC_XFER_SIZE_HS  3072
+
+static inline bool xhci_eusb2_is_isoc_bw_double(struct usb_device *udev,
+						struct usb_host_endpoint *ep)
+{
+	return udev->speed == USB_SPEED_HIGH &&
+		usb_endpoint_is_isoc_in(&ep->desc) &&
+		le16_to_cpu(ep->desc.wMaxPacketSize) == 0 &&
+		le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval) >
+		MAX_ISOC_XFER_SIZE_HS;
+}
+
 #define xhci_dbg(xhci, fmt, args...) \
 	dev_dbg(xhci_to_hcd(xhci)->self.controller , fmt , ## args)
 #define xhci_err(xhci, fmt, args...) \
@@ -1957,6 +1974,8 @@ void xhci_update_erst_dequeue(struct xhci_hcd *xhci,
 			      struct xhci_interrupter *ir,
 			      bool clear_ehb);
 void xhci_add_interrupter(struct xhci_hcd *xhci, unsigned int intr_num);
+int xhci_usb_endpoint_maxp(struct usb_device *udev,
+			   struct usb_host_endpoint *host_ep);
 
 /* xHCI roothub code */
 void xhci_set_link_state(struct xhci_hcd *xhci, struct xhci_port *port,
-- 
2.39.5

[PATCH v4 2/4] USB: core: support eUSB2 double bandwidth large isoc URB frames

[Sakari Ailus]

From: "Rai, Amardeep" <amardeep.rai@intel.com>

eUSB2 double isochronous in bandwidth devices support up to 6 transactions
per microframe, and thus doubles the total bytes possible to receive per
microframe.

Support larger URB isoc frame sizes for eUSB2 double isoc in endpoints.

Also usb_endpoint_maxp() returns a natural number so there's no need to
assume it could be < 0.

Signed-off-by: Rai, Amardeep <amardeep.rai@intel.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 drivers/usb/core/urb.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 7a76d5a62db1..6f8f3d751854 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -372,6 +372,7 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
 	struct usb_host_endpoint	*ep;
 	int				is_out;
 	unsigned int			allowed;
+	bool				is_eusb2_isoch_double;
 
 	if (!urb || !urb->complete)
 		return -EINVAL;
@@ -434,7 +435,11 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
 		return -ENODEV;
 
 	max = usb_endpoint_maxp(&ep->desc);
-	if (max <= 0) {
+	is_eusb2_isoch_double = dev->speed == USB_SPEED_HIGH &&
+				usb_endpoint_is_isoc_in(&ep->desc) &&
+				ep->eusb2_isoc_ep_comp.bDescriptorType &&
+				le16_to_cpu(ep->desc.wMaxPacketSize) == 0;
+	if (!max && !is_eusb2_isoch_double) {
 		dev_dbg(&dev->dev,
 			"bogus endpoint ep%d%s in %s (bad maxpacket %d)\n",
 			usb_endpoint_num(&ep->desc), is_out ? "out" : "in",
@@ -467,9 +472,13 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
 			max = le32_to_cpu(isoc_ep_comp->dwBytesPerInterval);
 		}
 
-		/* "high bandwidth" mode, 1-3 packets/uframe? */
-		if (dev->speed == USB_SPEED_HIGH)
-			max *= usb_endpoint_maxp_mult(&ep->desc);
+		/* High speed, 1-3 packets/uframe, max 6 for eUSB2 double bw */
+		if (dev->speed == USB_SPEED_HIGH) {
+			if (is_eusb2_isoch_double)
+				max = le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval);
+			else
+				max *= usb_endpoint_maxp_mult(&ep->desc);
+		}
 
 		if (urb->number_of_packets <= 0)
 			return -EINVAL;
-- 
2.39.5

[PATCH v4 3/4] USB: Add a function to obtain USB version independent maximum bpi value

[Sakari Ailus]

From: "Rai, Amardeep" <amardeep.rai@intel.com>

Add usb_endpoint_max_isoc_bpi() to obtain maximum bytes per interval for
isochronous endpoints in a USB version independent way.

Signed-off-by: Rai, Amardeep <amardeep.rai@intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Hans de Goede <hansg@kernel.org>
---
 drivers/usb/core/usb.c | 32 ++++++++++++++++++++++++++++++++
 include/linux/usb.h    |  3 +++
 2 files changed, 35 insertions(+)

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index fca7735fc660..3e80a5b3e41b 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1110,6 +1110,38 @@ void usb_free_noncoherent(struct usb_device *dev, size_t size,
 }
 EXPORT_SYMBOL_GPL(usb_free_noncoherent);
 
+/**
+ * usb_endpoint_max_isoc_bpi - Get maximum isochronous transfer bytes per interval
+ * @dev: The USB device
+ * @ep: The endpoint
+ *
+ * Returns: the maximum number of bytes isochronous endpoint @endpoint can
+ * transfer in during a service interval, or 0 for non-isochronous endpoints.
+ */
+u32 usb_endpoint_max_isoc_bpi(struct usb_device *dev,
+			      const struct usb_host_endpoint *ep)
+{
+	if (usb_endpoint_type(&ep->desc) != USB_ENDPOINT_XFER_ISOC)
+		return 0;
+
+	switch (dev->speed) {
+	case USB_SPEED_SUPER_PLUS:
+		if (USB_SS_SSP_ISOC_COMP(ep->ss_ep_comp.bmAttributes))
+			return le32_to_cpu(ep->ssp_isoc_ep_comp.dwBytesPerInterval);
+		fallthrough;
+	case USB_SPEED_SUPER:
+		return le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval);
+	case USB_SPEED_HIGH:
+		if (ep->eusb2_isoc_ep_comp.bDescriptorType &&
+		    !usb_endpoint_maxp(&ep->desc) && usb_endpoint_dir_in(&ep->desc))
+			return le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval);
+		fallthrough;
+	default:
+		return usb_endpoint_maxp(&ep->desc) * usb_endpoint_maxp_mult(&ep->desc);
+	}
+}
+EXPORT_SYMBOL_GPL(usb_endpoint_max_isoc_bpi);
+
 /*
  * Notifications of device and interface registration
  */
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 535ac37198a1..b38978abc3d6 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -2049,6 +2049,9 @@ static inline int usb_translate_errors(int error_code)
 	}
 }
 
+u32 usb_endpoint_max_isoc_bpi(struct usb_device *dev,
+			      const struct usb_host_endpoint *ep);
+
 /* Events from the usb core */
 #define USB_DEVICE_ADD		0x0001
 #define USB_DEVICE_REMOVE	0x0002
-- 
2.39.5

[PATCH v4 4/4] media: uvcvideo: eUSB2 double isochronous bandwidth support

[Sakari Ailus]

From: Tao Q Tao <tao.q.tao@intel.com>

Use usb_endpoint_max_isoc_bpi() from the USB framework to find the maximum
bytes per interval for the endpoint. Consequently this adds eUSB2
isochronous mode and SuperSpeedPlus Isochronous Endpoint Compaion support
where larger bpi values are possible.

Co-developed-by: Amardeep Rai <amardeep.rai@intel.com>
Signed-off-by: Amardeep Rai <amardeep.rai@intel.com>
Signed-off-by: Tao Q Tao <tao.q.tao@intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/usb/uvc/uvc_driver.c |  4 ++--
 drivers/media/usb/uvc/uvc_video.c  | 24 +++---------------------
 drivers/media/usb/uvc/uvcvideo.h   |  4 +---
 3 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index da24a655ab68..fde0bc95622c 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -536,7 +536,7 @@ static int uvc_parse_streaming(struct uvc_device *dev,
 	unsigned int nformats = 0, nframes = 0, nintervals = 0;
 	unsigned int size, i, n, p;
 	u32 *interval;
-	u16 psize;
+	u32 psize;
 	int ret = -EINVAL;
 
 	if (intf->cur_altsetting->desc.bInterfaceSubClass
@@ -772,7 +772,7 @@ static int uvc_parse_streaming(struct uvc_device *dev,
 				streaming->header.bEndpointAddress);
 		if (ep == NULL)
 			continue;
-		psize = uvc_endpoint_max_bpi(dev->udev, ep);
+		psize = usb_endpoint_max_isoc_bpi(dev->udev, ep);
 		if (psize > streaming->maxpsize)
 			streaming->maxpsize = psize;
 	}
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index a75af314e46b..335b1c4eff9b 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1860,24 +1860,6 @@ static void uvc_video_stop_transfer(struct uvc_streaming *stream,
 		uvc_free_urb_buffers(stream);
 }
 
-/*
- * Compute the maximum number of bytes per interval for an endpoint.
- */
-u16 uvc_endpoint_max_bpi(struct usb_device *dev, struct usb_host_endpoint *ep)
-{
-	u16 psize;
-
-	switch (dev->speed) {
-	case USB_SPEED_SUPER:
-	case USB_SPEED_SUPER_PLUS:
-		return le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval);
-	default:
-		psize = usb_endpoint_maxp(&ep->desc);
-		psize *= usb_endpoint_maxp_mult(&ep->desc);
-		return psize;
-	}
-}
-
 /*
  * Initialize isochronous URBs and allocate transfer buffers. The packet size
  * is given by the endpoint.
@@ -1888,10 +1870,10 @@ static int uvc_init_video_isoc(struct uvc_streaming *stream,
 	struct urb *urb;
 	struct uvc_urb *uvc_urb;
 	unsigned int npackets, i;
-	u16 psize;
+	u32 psize;
 	u32 size;
 
-	psize = uvc_endpoint_max_bpi(stream->dev->udev, ep);
+	psize = usb_endpoint_max_isoc_bpi(stream->dev->udev, ep);
 	size = stream->ctrl.dwMaxVideoFrameSize;
 
 	npackets = uvc_alloc_urb_buffers(stream, size, psize, gfp_flags);
@@ -2034,7 +2016,7 @@ static int uvc_video_start_transfer(struct uvc_streaming *stream,
 				continue;
 
 			/* Check if the bandwidth is high enough. */
-			psize = uvc_endpoint_max_bpi(stream->dev->udev, ep);
+			psize = usb_endpoint_max_isoc_bpi(stream->dev->udev, ep);
 			if (psize >= bandwidth && psize < best_psize) {
 				altsetting = alts->desc.bAlternateSetting;
 				best_psize = psize;
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index b9f8eb62ba1d..a77ba76e033a 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -450,7 +450,7 @@ struct uvc_streaming {
 
 	struct usb_interface *intf;
 	int intfnum;
-	u16 maxpsize;
+	u32 maxpsize;
 
 	struct uvc_streaming_header header;
 	enum v4l2_buf_type type;
@@ -818,8 +818,6 @@ void uvc_ctrl_cleanup_fh(struct uvc_fh *handle);
 /* Utility functions */
 struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts,
 					    u8 epaddr);
-u16 uvc_endpoint_max_bpi(struct usb_device *dev, struct usb_host_endpoint *ep);
-
 /* Quirks support */
 void uvc_video_decode_isight(struct uvc_urb *uvc_urb,
 			     struct uvc_buffer *buf,
-- 
2.39.5

Re: [PATCH v4 3/4] USB: Add a function to obtain USB version independent maximum bpi value

[Alan Stern]

On Tue, Aug 12, 2025 at 04:24:44PM +0300, Sakari Ailus wrote:
> From: "Rai, Amardeep" <amardeep.rai@intel.com>
> 
> Add usb_endpoint_max_isoc_bpi() to obtain maximum bytes per interval for
> isochronous endpoints in a USB version independent way.
> 
> Signed-off-by: Rai, Amardeep <amardeep.rai@intel.com>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Hans de Goede <hansg@kernel.org>
> ---
>  drivers/usb/core/usb.c | 32 ++++++++++++++++++++++++++++++++
>  include/linux/usb.h    |  3 +++
>  2 files changed, 35 insertions(+)

> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index 535ac37198a1..b38978abc3d6 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -2049,6 +2049,9 @@ static inline int usb_translate_errors(int error_code)
>  	}
>  }
>  
> +u32 usb_endpoint_max_isoc_bpi(struct usb_device *dev,
> +			      const struct usb_host_endpoint *ep);
> +

I would have put this declaration after usb_maxpacket() instead of after 
usb_translate_errors(), but no matter.

>  /* Events from the usb core */
>  #define USB_DEVICE_ADD		0x0001
>  #define USB_DEVICE_REMOVE	0x0002


Acked-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern

Re: [PATCH v4 3/4] USB: Add a function to obtain USB version independent maximum bpi value

[Michał Pecio]

On Tue, 12 Aug 2025 16:24:44 +0300, Sakari Ailus wrote:
> From: "Rai, Amardeep" <amardeep.rai@intel.com>
> 
> Add usb_endpoint_max_isoc_bpi() to obtain maximum bytes per interval for
> isochronous endpoints in a USB version independent way.
> 
> Signed-off-by: Rai, Amardeep <amardeep.rai@intel.com>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Hans de Goede <hansg@kernel.org>

Hi,

This is practically identical to xhci_get_max_esit_payload().

Couldn't xhci also use this helper now, to reduce duplication and
ensure that it has the same idea of ESIT payload as class drivers?

Note that this here would need to also accept interrupt EPs:
> +{
> +	if (usb_endpoint_type(&ep->desc) != USB_ENDPOINT_XFER_ISOC)
> +		return 0;

Regards,
Michal

Re: [PATCH v4 3/4] USB: Add a function to obtain USB version independent maximum bpi value

[Sakari Ailus]

Hi Michał,

Thank you for the review.

On Wed, Aug 13, 2025 at 04:49:58PM +0200, Michał Pecio wrote:
> On Tue, 12 Aug 2025 16:24:44 +0300, Sakari Ailus wrote:
> > From: "Rai, Amardeep" <amardeep.rai@intel.com>
> > 
> > Add usb_endpoint_max_isoc_bpi() to obtain maximum bytes per interval for
> > isochronous endpoints in a USB version independent way.
> > 
> > Signed-off-by: Rai, Amardeep <amardeep.rai@intel.com>
> > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Reviewed-by: Hans de Goede <hansg@kernel.org>
> 
> Hi,
> 
> This is practically identical to xhci_get_max_esit_payload().
> 
> Couldn't xhci also use this helper now, to reduce duplication and
> ensure that it has the same idea of ESIT payload as class drivers?
> 
> Note that this here would need to also accept interrupt EPs:
> > +{
> > +	if (usb_endpoint_type(&ep->desc) != USB_ENDPOINT_XFER_ISOC)
> > +		return 0;

Sounds reasonable, I'll see how to best take that into account in v5.

I wonder if I should adopt the name from the xHCI variant as the function
would be also used for interrupt endpoints.

-- 
Regards,

Sakari Ailus

Re: [PATCH v4 3/4] USB: Add a function to obtain USB version independent maximum bpi value

[Mathias Nyman]

Hi

On 18.8.2025 9.08, Sakari Ailus wrote:
> Hi Michał,
> 
> Thank you for the review.
> 
> On Wed, Aug 13, 2025 at 04:49:58PM +0200, Michał Pecio wrote:
>> On Tue, 12 Aug 2025 16:24:44 +0300, Sakari Ailus wrote:
>>> From: "Rai, Amardeep" <amardeep.rai@intel.com>
>>>
>>> Add usb_endpoint_max_isoc_bpi() to obtain maximum bytes per interval for
>>> isochronous endpoints in a USB version independent way.
>>>
>>> Signed-off-by: Rai, Amardeep <amardeep.rai@intel.com>
>>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>>> Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>> Reviewed-by: Hans de Goede <hansg@kernel.org>
>>
>> Hi,
>>
>> This is practically identical to xhci_get_max_esit_payload().
>>
>> Couldn't xhci also use this helper now, to reduce duplication and
>> ensure that it has the same idea of ESIT payload as class drivers?
>>
>> Note that this here would need to also accept interrupt EPs:
>>> +{
>>> +	if (usb_endpoint_type(&ep->desc) != USB_ENDPOINT_XFER_ISOC)
>>> +		return 0;
> 
> Sounds reasonable, I'll see how to best take that into account in v5.
> 
> I wonder if I should adopt the name from the xHCI variant as the function
> would be also used for interrupt endpoints.
> 

I think the "ESIT" acronym (Endpoint Service Interval Time) is very xHCI spec
specific. Usb spec usually refers to isoc and interrupt endpoints as "periodic"

how about one of these:
usb_endpoint_max_periodic_bytes()
usb_endpoint_max_periodic_payload()
usb_endpoint_max_periodic_bpi()

Thanks
Mathias

Re: [PATCH v4 3/4] USB: Add a function to obtain USB version independent maximum bpi value

[Michał Pecio]

On Mon, 18 Aug 2025 10:32:48 +0300, Mathias Nyman wrote:
> I think the "ESIT" acronym (Endpoint Service Interval Time) is very xHCI spec
> specific. Usb spec usually refers to isoc and interrupt endpoints as "periodic"
> 
> how about one of these:
> usb_endpoint_max_periodic_bytes()
> usb_endpoint_max_periodic_payload()
> usb_endpoint_max_periodic_bpi()

I was thinking just drop "isoch" and leave usb_endpoint_max_bpi(),
because USB specs call this "bytes per interval".

But Alan didn't like this 3LA so we can get creative. More ideas:

usb_endpoint_interval_payload()
usb_endpoint_interval_bytes()


Naming things and off-by-one errors...

Re: [PATCH v4 1/4] xhci: Add host support for eUSB2 double isochronous bandwidth devices

[Michał Pecio]

On Tue, 12 Aug 2025 16:24:42 +0300, Sakari Ailus wrote:
> From: "Rai, Amardeep" <amardeep.rai@intel.com>
> 
> Detect eUSB2 double isoc bw capable hosts and devices, and set the proper
> xhci endpoint context values such as 'Mult', 'Max Burst Size', and 'Max
> ESIT Payload' to enable the double isochronous bandwidth endpoints.
> 
> Intel xHC uses the endpoint context 'Mult' field for eUSB2 isoc
> endpoints even if hosts supporting Large ESIT Payload Capability should
> normally ignore the mult field.
> 
> Signed-off-by: Rai, Amardeep <amardeep.rai@intel.com>
> Co-developed-by: Kannappan R <r.kannappan@intel.com>
> Signed-off-by: Kannappan R <r.kannappan@intel.com>
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Co-developed-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/usb/host/xhci-caps.h |  2 ++
>  drivers/usb/host/xhci-mem.c  | 60 ++++++++++++++++++++++++++++--------
>  drivers/usb/host/xhci-ring.c |  6 ++--
>  drivers/usb/host/xhci.c      | 16 +++++++++-
>  drivers/usb/host/xhci.h      | 19 ++++++++++++
>  5 files changed, 87 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
> index 4b8ff4815644..89bc83e4f1eb 100644
> --- a/drivers/usb/host/xhci-caps.h
> +++ b/drivers/usb/host/xhci-caps.h
> @@ -89,3 +89,5 @@
>  #define HCC2_GSC(p)             ((p) & (1 << 8))
>  /* true: HC support Virtualization Based Trusted I/O Capability */
>  #define HCC2_VTC(p)             ((p) & (1 << 9))
> +/* true: HC support Double BW on a eUSB2 HS ISOC EP */
> +#define HCC2_EUSB2_DIC(p)       ((p) & (1 << 11))

I guess this bit is not defined in the original xHCI 1.2 spec which
predates BW doubling, any reference to where it is specified and what
it means exactly?

> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 6680afa4f596..ea51434c80fa 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -1328,18 +1328,36 @@ static unsigned int xhci_get_endpoint_interval(struct usb_device *udev,
>  	return interval;
>  }
>  
> -/* The "Mult" field in the endpoint context is only set for SuperSpeed isoc eps.
> +/*
> + * xHCs without LEC use the "Mult" field in the endpoint context for SuperSpeed
> + * isoc eps, and High speed isoc eps that support bandwidth doubling. Standard
>   * High speed endpoint descriptors can define "the number of additional
>   * transaction opportunities per microframe", but that goes in the Max Burst
>   * endpoint context field.
>   */
> -static u32 xhci_get_endpoint_mult(struct usb_device *udev,
> -		struct usb_host_endpoint *ep)
> +static u32 xhci_get_endpoint_mult(struct xhci_hcd *xhci,
> +				  struct usb_device *udev,
> +				  struct usb_host_endpoint *ep)
>  {
> -	if (udev->speed < USB_SPEED_SUPER ||
> -			!usb_endpoint_xfer_isoc(&ep->desc))
> +	bool lec;
> +
> +	/* xHCI 1.1 with LEC set does not use mult field, except intel eUSB2 */
> +	lec = xhci->hci_version > 0x100 && HCC2_LEC(xhci->hcc_params2);
> +
> +	/* eUSB2 double isoc bw devices are the only USB2 devices using mult */
> +	if (xhci_eusb2_is_isoc_bw_double(udev, ep)) {
> +		if (!lec || xhci->quirks & XHCI_INTEL_HOST)
> +			return 1;
> +	}
> +
> +	/* Oherwise only isoc transfers on hosts without LEC uses mult field */
> +	if (!usb_endpoint_xfer_isoc(&ep->desc) || lec)
>  		return 0;
> -	return ep->ss_ep_comp.bmAttributes;
> +
> +	if (udev->speed >= USB_SPEED_SUPER)
> +		return ep->ss_ep_comp.bmAttributes;
> +
> +	return 0;
>  }

That's a complicated control flow. I think it could just be:
> +	/* SuperSpeed isoc transfers on hosts without LEC uses mult field */
> +	if (udev->speed >= USB_SPEED_SUPER && usb_endpoint_xfer_isoc(&ep->desc) && !lec)
> +		return ep->ss_ep_comp.bmAttributes;
> +	return 0;
> }

>  
>  static u32 xhci_get_endpoint_max_burst(struct usb_device *udev,
> @@ -1351,8 +1369,18 @@ static u32 xhci_get_endpoint_max_burst(struct usb_device *udev,
>  
>  	if (udev->speed == USB_SPEED_HIGH &&
>  	    (usb_endpoint_xfer_isoc(&ep->desc) ||
> -	     usb_endpoint_xfer_int(&ep->desc)))
> +	     usb_endpoint_xfer_int(&ep->desc))) {
> +		/*
> +		 * eUSB2 double isoc bw endpoints max packet field service
> +		 * opportunity bits 12:11 are not valid, so set the ctx burst to
> +		 * max service opportunity "2" as these eps support transferring
> +		 * over 3072 bytes per interval
> +		 */

I think a shorter comment would suffice: eUSB2 BWD uses fixed burst
size and max packets bits 12:11 are invalid.

> +		if (xhci_eusb2_is_isoc_bw_double(udev, ep))
> +			return 2;
> +
>  		return usb_endpoint_maxp_mult(&ep->desc) - 1;
> +	}
>  
>  	return 0;
>  }
> @@ -1400,6 +1428,10 @@ static u32 xhci_get_max_esit_payload(struct usb_device *udev,
>  	if (udev->speed >= USB_SPEED_SUPER)
>  		return le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval);
>  
> +	/* High speed eUSB2 double isoc bw with over 3072 bytes per esit */
> +	if (xhci_eusb2_is_isoc_bw_double(udev, ep))
> +		return le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval);
> +
>  	max_packet = usb_endpoint_maxp(&ep->desc);
>  	max_burst = usb_endpoint_maxp_mult(&ep->desc);
>  	/* A 0 in max burst means 1 transfer per ESIT */
> @@ -1437,6 +1469,13 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
>  
>  	ring_type = usb_endpoint_type(&ep->desc);
>  
> +	/* Ensure host supports double isoc bandwidth for eUSB2 devices */
> +	if (xhci_eusb2_is_isoc_bw_double(udev, ep) &&
> +	    !HCC2_EUSB2_DIC(xhci->hcc_params2))	{
> +		dev_dbg(&udev->dev, "Double Isoc Bandwidth not supported by xhci\n");
> +		return -EINVAL;
> +	}
> +
>  	/*
>  	 * Get values to fill the endpoint context, mostly from ep descriptor.
>  	 * The average TRB buffer lengt for bulk endpoints is unclear as we
> @@ -1460,8 +1499,8 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
>  		}
>  	}
>  
> -	mult = xhci_get_endpoint_mult(udev, ep);
> -	max_packet = usb_endpoint_maxp(&ep->desc);
> +	mult = xhci_get_endpoint_mult(xhci, udev, ep);
> +	max_packet = xhci_usb_endpoint_maxp(udev, ep);
>  	max_burst = xhci_get_endpoint_max_burst(udev, ep);
>  	avg_trb_len = max_esit_payload;
>  
> @@ -1482,9 +1521,6 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
>  	/* xHCI 1.0 and 1.1 indicates that ctrl ep avg TRB Length should be 8 */
>  	if (usb_endpoint_xfer_control(&ep->desc) && xhci->hci_version >= 0x100)
>  		avg_trb_len = 8;
> -	/* xhci 1.1 with LEC support doesn't use mult field, use RsvdZ */
> -	if ((xhci->hci_version > 0x100) && HCC2_LEC(xhci->hcc_params2))
> -		mult = 0;
>  
>  	/* Set up the endpoint ring */
>  	virt_dev->eps[ep_index].new_ring =
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 94c9c9271658..9abc5bc4e1c0 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3542,7 +3542,7 @@ static u32 xhci_td_remainder(struct xhci_hcd *xhci, int transferred,
>  	if ((xhci->quirks & XHCI_MTK_HOST) && (xhci->hci_version < 0x100))
>  		trb_buff_len = 0;
>  
> -	maxp = usb_endpoint_maxp(&urb->ep->desc);
> +	maxp = xhci_usb_endpoint_maxp(urb->dev, urb->ep);
>  	total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
>  
>  	/* Queueing functions don't count the current TRB into transferred */
> @@ -3559,7 +3559,7 @@ static int xhci_align_td(struct xhci_hcd *xhci, struct urb *urb, u32 enqd_len,
>  	u32 new_buff_len;
>  	size_t len;
>  
> -	max_pkt = usb_endpoint_maxp(&urb->ep->desc);
> +	max_pkt = xhci_usb_endpoint_maxp(urb->dev, urb->ep);
>  	unalign = (enqd_len + *trb_buff_len) % max_pkt;
>  
>  	/* we got lucky, last normal TRB data on segment is packet aligned */
> @@ -4130,7 +4130,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
>  		addr = start_addr + urb->iso_frame_desc[i].offset;
>  		td_len = urb->iso_frame_desc[i].length;
>  		td_remain_len = td_len;
> -		max_pkt = usb_endpoint_maxp(&urb->ep->desc);
> +		max_pkt = xhci_usb_endpoint_maxp(urb->dev, urb->ep);
>  		total_pkt_count = DIV_ROUND_UP(td_len, max_pkt);
>  
>  		/* A zero-length transfer still involves at least one packet. */
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 8a819e853288..1ea7d2fb5876 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1332,7 +1332,7 @@ static bool xhci_urb_temp_buffer_required(struct usb_hcd *hcd,
>  	struct scatterlist *tail_sg;
>  
>  	tail_sg = urb->sg;
> -	max_pkt = usb_endpoint_maxp(&urb->ep->desc);
> +	max_pkt = xhci_usb_endpoint_maxp(urb->dev, urb->ep);
>  
>  	if (!urb->num_sgs)
>  		return ret;
> @@ -2920,6 +2920,20 @@ int xhci_stop_endpoint_sync(struct xhci_hcd *xhci, struct xhci_virt_ep *ep, int
>  }
>  EXPORT_SYMBOL_GPL(xhci_stop_endpoint_sync);
>  
> +/*
> + * xhci_usb_endpoint_maxp - get endpoint max packet size
> + * @host_ep: USB host endpoint to be checked
> + *
> + * Returns max packet from the correct descriptor
> + */
> +int xhci_usb_endpoint_maxp(struct usb_device *udev,
> +			   struct usb_host_endpoint *host_ep)
> +{
> +	if (xhci_eusb2_is_isoc_bw_double(udev, host_ep))
> +		return le16_to_cpu(host_ep->eusb2_isoc_ep_comp.wMaxPacketSize);
> +	return usb_endpoint_maxp(&host_ep->desc);
> +}
> +
>  /* Issue a configure endpoint command or evaluate context command
>   * and wait for it to finish.
>   */
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index a20f4e7cd43a..9657a4deb87b 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1736,6 +1736,23 @@ static inline bool xhci_has_one_roothub(struct xhci_hcd *xhci)
>  	       (!xhci->usb2_rhub.num_ports || !xhci->usb3_rhub.num_ports);
>  }
>  
> +/*
> + * USB 2.0 specification, chapter 5.6.4 Isochronous Transfer Bus Access
> + * Constraint. A high speed endpoint can move up to 3072 bytes per microframe
> + * (or 192Mb/s).
> + */
> +#define MAX_ISOC_XFER_SIZE_HS  3072
> +
> +static inline bool xhci_eusb2_is_isoc_bw_double(struct usb_device *udev,
> +						struct usb_host_endpoint *ep)
> +{
> +	return udev->speed == USB_SPEED_HIGH &&
> +		usb_endpoint_is_isoc_in(&ep->desc) &&
> +		le16_to_cpu(ep->desc.wMaxPacketSize) == 0 &&
> +		le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval) >
> +		MAX_ISOC_XFER_SIZE_HS;
> +}

It looks like eUSB2v2 is another spec which uses this descriptor for
larger isoc endpoints and this code might trigger on such devices once
USB core learns to parse them.

Would there be no issues with that? Or conversely, any chance that your
code could be trivially modified to support full 2v2, and "bw doubling"
removed from names and comments?

> +
>  #define xhci_dbg(xhci, fmt, args...) \
>  	dev_dbg(xhci_to_hcd(xhci)->self.controller , fmt , ## args)
>  #define xhci_err(xhci, fmt, args...) \
> @@ -1957,6 +1974,8 @@ void xhci_update_erst_dequeue(struct xhci_hcd *xhci,
>  			      struct xhci_interrupter *ir,
>  			      bool clear_ehb);
>  void xhci_add_interrupter(struct xhci_hcd *xhci, unsigned int intr_num);
> +int xhci_usb_endpoint_maxp(struct usb_device *udev,
> +			   struct usb_host_endpoint *host_ep);
>  
>  /* xHCI roothub code */
>  void xhci_set_link_state(struct xhci_hcd *xhci, struct xhci_port *port,
> -- 
> 2.39.5
> 

Re: [PATCH v4 3/4] USB: Add a function to obtain USB version independent maximum bpi value

[Sakari Ailus]

Hi Mathias,

On Mon, Aug 18, 2025 at 10:32:48AM +0300, Mathias Nyman wrote:
> Hi
> 
> On 18.8.2025 9.08, Sakari Ailus wrote:
> > Hi Michał,
> > 
> > Thank you for the review.
> > 
> > On Wed, Aug 13, 2025 at 04:49:58PM +0200, Michał Pecio wrote:
> > > On Tue, 12 Aug 2025 16:24:44 +0300, Sakari Ailus wrote:
> > > > From: "Rai, Amardeep" <amardeep.rai@intel.com>
> > > > 
> > > > Add usb_endpoint_max_isoc_bpi() to obtain maximum bytes per interval for
> > > > isochronous endpoints in a USB version independent way.
> > > > 
> > > > Signed-off-by: Rai, Amardeep <amardeep.rai@intel.com>
> > > > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > > > Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > Reviewed-by: Hans de Goede <hansg@kernel.org>
> > > 
> > > Hi,
> > > 
> > > This is practically identical to xhci_get_max_esit_payload().
> > > 
> > > Couldn't xhci also use this helper now, to reduce duplication and
> > > ensure that it has the same idea of ESIT payload as class drivers?
> > > 
> > > Note that this here would need to also accept interrupt EPs:
> > > > +{
> > > > +	if (usb_endpoint_type(&ep->desc) != USB_ENDPOINT_XFER_ISOC)
> > > > +		return 0;
> > 
> > Sounds reasonable, I'll see how to best take that into account in v5.
> > 
> > I wonder if I should adopt the name from the xHCI variant as the function
> > would be also used for interrupt endpoints.
> > 
> 
> I think the "ESIT" acronym (Endpoint Service Interval Time) is very xHCI
> spec specific. Usb spec usually refers to isoc and interrupt endpoints as
> "periodic"
> 
> how about one of these:
> usb_endpoint_max_periodic_bytes()
> usb_endpoint_max_periodic_payload()
> usb_endpoint_max_periodic_bpi()

How about usb_endpoint_max_si_payload() ("si" being for service interval)?

-- 
Regards,

Sakari Ailus

Re: [PATCH v4 3/4] USB: Add a function to obtain USB version independent maximum bpi value

[Sakari Ailus]

Hi Mathias,

On Mon, Aug 18, 2025 at 11:27:00AM +0000, Sakari Ailus wrote:
> Hi Mathias,
> 
> On Mon, Aug 18, 2025 at 10:32:48AM +0300, Mathias Nyman wrote:
> > Hi
> > 
> > On 18.8.2025 9.08, Sakari Ailus wrote:
> > > Hi Michał,
> > > 
> > > Thank you for the review.
> > > 
> > > On Wed, Aug 13, 2025 at 04:49:58PM +0200, Michał Pecio wrote:
> > > > On Tue, 12 Aug 2025 16:24:44 +0300, Sakari Ailus wrote:
> > > > > From: "Rai, Amardeep" <amardeep.rai@intel.com>
> > > > > 
> > > > > Add usb_endpoint_max_isoc_bpi() to obtain maximum bytes per interval for
> > > > > isochronous endpoints in a USB version independent way.
> > > > > 
> > > > > Signed-off-by: Rai, Amardeep <amardeep.rai@intel.com>
> > > > > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > > > > Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > > Reviewed-by: Hans de Goede <hansg@kernel.org>
> > > > 
> > > > Hi,
> > > > 
> > > > This is practically identical to xhci_get_max_esit_payload().
> > > > 
> > > > Couldn't xhci also use this helper now, to reduce duplication and
> > > > ensure that it has the same idea of ESIT payload as class drivers?
> > > > 
> > > > Note that this here would need to also accept interrupt EPs:
> > > > > +{
> > > > > +	if (usb_endpoint_type(&ep->desc) != USB_ENDPOINT_XFER_ISOC)
> > > > > +		return 0;
> > > 
> > > Sounds reasonable, I'll see how to best take that into account in v5.
> > > 
> > > I wonder if I should adopt the name from the xHCI variant as the function
> > > would be also used for interrupt endpoints.
> > > 
> > 
> > I think the "ESIT" acronym (Endpoint Service Interval Time) is very xHCI
> > spec specific. Usb spec usually refers to isoc and interrupt endpoints as
> > "periodic"
> > 
> > how about one of these:
> > usb_endpoint_max_periodic_bytes()
> > usb_endpoint_max_periodic_payload()
> > usb_endpoint_max_periodic_bpi()
> 
> How about usb_endpoint_max_si_payload() ("si" being for service interval)?

I somehow missed your latter sentence earlier. I'm totally fine with these,
perhaps I'm slightly leaning towards usb_endpoint_max_periodic_payload()
but let's see what others think.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v4 3/4] USB: Add a function to obtain USB version independent maximum bpi value

[Alan Stern]

On Mon, Aug 18, 2025 at 12:13:05PM +0000, Sakari Ailus wrote:
> > > how about one of these:
> > > usb_endpoint_max_periodic_bytes()
> > > usb_endpoint_max_periodic_payload()
> > > usb_endpoint_max_periodic_bpi()
> > 
> > How about usb_endpoint_max_si_payload() ("si" being for service interval)?
> 
> I somehow missed your latter sentence earlier. I'm totally fine with these,
> perhaps I'm slightly leaning towards usb_endpoint_max_periodic_payload()
> but let's see what others think.

I'm okay with either usb_endpoint_max_periodic_bytes() or 
usb_endpoint_max_periodic_payload().

Alan Stern

Re: [PATCH v4 1/4] xhci: Add host support for eUSB2 double isochronous bandwidth devices

[Mathias Nyman]

On 18.8.2025 12.50, Michał Pecio wrote:
> On Tue, 12 Aug 2025 16:24:42 +0300, Sakari Ailus wrote:
>> From: "Rai, Amardeep" <amardeep.rai@intel.com>
>>
>> Detect eUSB2 double isoc bw capable hosts and devices, and set the proper
>> xhci endpoint context values such as 'Mult', 'Max Burst Size', and 'Max
>> ESIT Payload' to enable the double isochronous bandwidth endpoints.
>>
>> Intel xHC uses the endpoint context 'Mult' field for eUSB2 isoc
>> endpoints even if hosts supporting Large ESIT Payload Capability should
>> normally ignore the mult field.
>>
>> Signed-off-by: Rai, Amardeep <amardeep.rai@intel.com>
>> Co-developed-by: Kannappan R <r.kannappan@intel.com>
>> Signed-off-by: Kannappan R <r.kannappan@intel.com>
>> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Co-developed-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>>   drivers/usb/host/xhci-caps.h |  2 ++
>>   drivers/usb/host/xhci-mem.c  | 60 ++++++++++++++++++++++++++++--------
>>   drivers/usb/host/xhci-ring.c |  6 ++--
>>   drivers/usb/host/xhci.c      | 16 +++++++++-
>>   drivers/usb/host/xhci.h      | 19 ++++++++++++
>>   5 files changed, 87 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
>> index 4b8ff4815644..89bc83e4f1eb 100644
>> --- a/drivers/usb/host/xhci-caps.h
>> +++ b/drivers/usb/host/xhci-caps.h
>> @@ -89,3 +89,5 @@
>>   #define HCC2_GSC(p)             ((p) & (1 << 8))
>>   /* true: HC support Virtualization Based Trusted I/O Capability */
>>   #define HCC2_VTC(p)             ((p) & (1 << 9))
>> +/* true: HC support Double BW on a eUSB2 HS ISOC EP */
>> +#define HCC2_EUSB2_DIC(p)       ((p) & (1 << 11))
> 
> I guess this bit is not defined in the original xHCI 1.2 spec which
> predates BW doubling, any reference to where it is specified and what
> it means exactly?

USB 2.0 Double Isochronous IN Bandwidth Capability (DIC) – RO.
This bit when set to 1, indicates that the xHC supports the USB 2.0 Double Isochronous IN
Bandwidth Capability on a eUSB2 HS Isochronous Endpoint.
This feature is only applicable to a directly connected inbox native eUSB2 device.

The xHCI specification with this addition is on its way, we got permission from
author(s) to start upstreaming the code already.

> 
>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>> index 6680afa4f596..ea51434c80fa 100644
>> --- a/drivers/usb/host/xhci-mem.c
>> +++ b/drivers/usb/host/xhci-mem.c
>> @@ -1328,18 +1328,36 @@ static unsigned int xhci_get_endpoint_interval(struct usb_device *udev,
>>   	return interval;
>>   }
>>   
>> -/* The "Mult" field in the endpoint context is only set for SuperSpeed isoc eps.
>> +/*
>> + * xHCs without LEC use the "Mult" field in the endpoint context for SuperSpeed
>> + * isoc eps, and High speed isoc eps that support bandwidth doubling. Standard
>>    * High speed endpoint descriptors can define "the number of additional
>>    * transaction opportunities per microframe", but that goes in the Max Burst
>>    * endpoint context field.
>>    */
>> -static u32 xhci_get_endpoint_mult(struct usb_device *udev,
>> -		struct usb_host_endpoint *ep)
>> +static u32 xhci_get_endpoint_mult(struct xhci_hcd *xhci,
>> +				  struct usb_device *udev,
>> +				  struct usb_host_endpoint *ep)
>>   {
>> -	if (udev->speed < USB_SPEED_SUPER ||
>> -			!usb_endpoint_xfer_isoc(&ep->desc))
>> +	bool lec;
>> +
>> +	/* xHCI 1.1 with LEC set does not use mult field, except intel eUSB2 */
>> +	lec = xhci->hci_version > 0x100 && HCC2_LEC(xhci->hcc_params2);
>> +
>> +	/* eUSB2 double isoc bw devices are the only USB2 devices using mult */
>> +	if (xhci_eusb2_is_isoc_bw_double(udev, ep)) {
>> +		if (!lec || xhci->quirks & XHCI_INTEL_HOST)
>> +			return 1;
>> +	}
>> +
>> +	/* Oherwise only isoc transfers on hosts without LEC uses mult field */
>> +	if (!usb_endpoint_xfer_isoc(&ep->desc) || lec)
>>   		return 0;
>> -	return ep->ss_ep_comp.bmAttributes;
>> +
>> +	if (udev->speed >= USB_SPEED_SUPER)
>> +		return ep->ss_ep_comp.bmAttributes;
>> +
>> +	return 0;
>>   }
> 
> That's a complicated control flow. I think it could just be:
>> +	/* SuperSpeed isoc transfers on hosts without LEC uses mult field */
>> +	if (udev->speed >= USB_SPEED_SUPER && usb_endpoint_xfer_isoc(&ep->desc) && !lec)
>> +		return ep->ss_ep_comp.bmAttributes;
>> +	return 0;

Possibly, not sure there's much difference, especially if you break that line at
80 columns

>> +/*
>> + * USB 2.0 specification, chapter 5.6.4 Isochronous Transfer Bus Access
>> + * Constraint. A high speed endpoint can move up to 3072 bytes per microframe
>> + * (or 192Mb/s).
>> + */
>> +#define MAX_ISOC_XFER_SIZE_HS  3072
>> +
>> +static inline bool xhci_eusb2_is_isoc_bw_double(struct usb_device *udev,
>> +						struct usb_host_endpoint *ep)
>> +{
>> +	return udev->speed == USB_SPEED_HIGH &&
>> +		usb_endpoint_is_isoc_in(&ep->desc) &&
>> +		le16_to_cpu(ep->desc.wMaxPacketSize) == 0 &&
>> +		le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval) >
>> +		MAX_ISOC_XFER_SIZE_HS;
>> +}
> 
> It looks like eUSB2v2 is another spec which uses this descriptor for
> larger isoc endpoints and this code might trigger on such devices once
> USB core learns to parse them.
> 

Could make sense to check that bcdUSB is 0x220 here to avoid that.

We could also check if ep->eusb2_isoc_ep_comp.bDescriptorType exists
like in PATCH 2/4, and could then remove the wMaxPacketSize == 0 check as
usb core descriptor parsing will already do that check for us, before copying
the descriptor.

The USB_SPEED_HIGH check could also be moved to descriptor parsing in usb core.
This way we could remove the speed check hare and from PATCH 2/3


> Would there be no issues with that? Or conversely, any chance that your
> code could be trivially modified to support full 2v2, and "bw doubling"
> removed from names and comments?

Proably not, eUSB2v2 may have some odd burst and mult fields while double
bandwidth has fixed values.

Thanks
Mathias

Re: [PATCH v4 1/4] xhci: Add host support for eUSB2 double isochronous bandwidth devices

[Sakari Ailus]

Hi Mathias, Michał,

On Mon, Aug 18, 2025 at 06:30:23PM +0300, Mathias Nyman wrote:
> On 18.8.2025 12.50, Michał Pecio wrote:
> > On Tue, 12 Aug 2025 16:24:42 +0300, Sakari Ailus wrote:
> > > From: "Rai, Amardeep" <amardeep.rai@intel.com>
> > > 
> > > Detect eUSB2 double isoc bw capable hosts and devices, and set the proper
> > > xhci endpoint context values such as 'Mult', 'Max Burst Size', and 'Max
> > > ESIT Payload' to enable the double isochronous bandwidth endpoints.
> > > 
> > > Intel xHC uses the endpoint context 'Mult' field for eUSB2 isoc
> > > endpoints even if hosts supporting Large ESIT Payload Capability should
> > > normally ignore the mult field.
> > > 
> > > Signed-off-by: Rai, Amardeep <amardeep.rai@intel.com>
> > > Co-developed-by: Kannappan R <r.kannappan@intel.com>
> > > Signed-off-by: Kannappan R <r.kannappan@intel.com>
> > > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > Co-developed-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > > Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > ---
> > >   drivers/usb/host/xhci-caps.h |  2 ++
> > >   drivers/usb/host/xhci-mem.c  | 60 ++++++++++++++++++++++++++++--------
> > >   drivers/usb/host/xhci-ring.c |  6 ++--
> > >   drivers/usb/host/xhci.c      | 16 +++++++++-
> > >   drivers/usb/host/xhci.h      | 19 ++++++++++++
> > >   5 files changed, 87 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
> > > index 4b8ff4815644..89bc83e4f1eb 100644
> > > --- a/drivers/usb/host/xhci-caps.h
> > > +++ b/drivers/usb/host/xhci-caps.h
> > > @@ -89,3 +89,5 @@
> > >   #define HCC2_GSC(p)             ((p) & (1 << 8))
> > >   /* true: HC support Virtualization Based Trusted I/O Capability */
> > >   #define HCC2_VTC(p)             ((p) & (1 << 9))
> > > +/* true: HC support Double BW on a eUSB2 HS ISOC EP */
> > > +#define HCC2_EUSB2_DIC(p)       ((p) & (1 << 11))
> > 
> > I guess this bit is not defined in the original xHCI 1.2 spec which
> > predates BW doubling, any reference to where it is specified and what
> > it means exactly?
> 
> USB 2.0 Double Isochronous IN Bandwidth Capability (DIC) – RO.
> This bit when set to 1, indicates that the xHC supports the USB 2.0 Double Isochronous IN
> Bandwidth Capability on a eUSB2 HS Isochronous Endpoint.
> This feature is only applicable to a directly connected inbox native eUSB2 device.
> 
> The xHCI specification with this addition is on its way, we got permission from
> author(s) to start upstreaming the code already.
> 
> > 
> > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> > > index 6680afa4f596..ea51434c80fa 100644
> > > --- a/drivers/usb/host/xhci-mem.c
> > > +++ b/drivers/usb/host/xhci-mem.c
> > > @@ -1328,18 +1328,36 @@ static unsigned int xhci_get_endpoint_interval(struct usb_device *udev,
> > >   	return interval;
> > >   }
> > > -/* The "Mult" field in the endpoint context is only set for SuperSpeed isoc eps.
> > > +/*
> > > + * xHCs without LEC use the "Mult" field in the endpoint context for SuperSpeed
> > > + * isoc eps, and High speed isoc eps that support bandwidth doubling. Standard
> > >    * High speed endpoint descriptors can define "the number of additional
> > >    * transaction opportunities per microframe", but that goes in the Max Burst
> > >    * endpoint context field.
> > >    */
> > > -static u32 xhci_get_endpoint_mult(struct usb_device *udev,
> > > -		struct usb_host_endpoint *ep)
> > > +static u32 xhci_get_endpoint_mult(struct xhci_hcd *xhci,
> > > +				  struct usb_device *udev,
> > > +				  struct usb_host_endpoint *ep)
> > >   {
> > > -	if (udev->speed < USB_SPEED_SUPER ||
> > > -			!usb_endpoint_xfer_isoc(&ep->desc))
> > > +	bool lec;
> > > +
> > > +	/* xHCI 1.1 with LEC set does not use mult field, except intel eUSB2 */
> > > +	lec = xhci->hci_version > 0x100 && HCC2_LEC(xhci->hcc_params2);
> > > +
> > > +	/* eUSB2 double isoc bw devices are the only USB2 devices using mult */
> > > +	if (xhci_eusb2_is_isoc_bw_double(udev, ep)) {
> > > +		if (!lec || xhci->quirks & XHCI_INTEL_HOST)
> > > +			return 1;
> > > +	}
> > > +
> > > +	/* Oherwise only isoc transfers on hosts without LEC uses mult field */
> > > +	if (!usb_endpoint_xfer_isoc(&ep->desc) || lec)
> > >   		return 0;
> > > -	return ep->ss_ep_comp.bmAttributes;
> > > +
> > > +	if (udev->speed >= USB_SPEED_SUPER)
> > > +		return ep->ss_ep_comp.bmAttributes;
> > > +
> > > +	return 0;
> > >   }
> > 
> > That's a complicated control flow. I think it could just be:
> > > +	/* SuperSpeed isoc transfers on hosts without LEC uses mult field */
> > > +	if (udev->speed >= USB_SPEED_SUPER && usb_endpoint_xfer_isoc(&ep->desc) && !lec)
> > > +		return ep->ss_ep_comp.bmAttributes;
> > > +	return 0;
> 
> Possibly, not sure there's much difference, especially if you break that line at
> 80 columns

I prefer Michał's suggestion, line breaks are fine, too...

> 
> > > +/*
> > > + * USB 2.0 specification, chapter 5.6.4 Isochronous Transfer Bus Access
> > > + * Constraint. A high speed endpoint can move up to 3072 bytes per microframe
> > > + * (or 192Mb/s).
> > > + */
> > > +#define MAX_ISOC_XFER_SIZE_HS  3072
> > > +
> > > +static inline bool xhci_eusb2_is_isoc_bw_double(struct usb_device *udev,
> > > +						struct usb_host_endpoint *ep)
> > > +{
> > > +	return udev->speed == USB_SPEED_HIGH &&
> > > +		usb_endpoint_is_isoc_in(&ep->desc) &&
> > > +		le16_to_cpu(ep->desc.wMaxPacketSize) == 0 &&
> > > +		le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval) >
> > > +		MAX_ISOC_XFER_SIZE_HS;
> > > +}
> > 
> > It looks like eUSB2v2 is another spec which uses this descriptor for
> > larger isoc endpoints and this code might trigger on such devices once
> > USB core learns to parse them.
> > 
> 
> Could make sense to check that bcdUSB is 0x220 here to avoid that.

I'll add the check for v5.

> 
> We could also check if ep->eusb2_isoc_ep_comp.bDescriptorType exists
> like in PATCH 2/4, and could then remove the wMaxPacketSize == 0 check as
> usb core descriptor parsing will already do that check for us, before copying
> the descriptor.

Ack.

> 
> The USB_SPEED_HIGH check could also be moved to descriptor parsing in usb core.
> This way we could remove the speed check hare and from PATCH 2/3

Ack.

> 
> 
> > Would there be no issues with that? Or conversely, any chance that your
> > code could be trivially modified to support full 2v2, and "bw doubling"
> > removed from names and comments?
> 
> Proably not, eUSB2v2 may have some odd burst and mult fields while double
> bandwidth has fixed values.

-- 
Regards,

Sakari Ailus

Re: [PATCH v4 1/4] xhci: Add host support for eUSB2 double isochronous bandwidth devices

[Sakari Ailus]

Hi Michał,

Thanks for the review.

On Mon, Aug 18, 2025 at 11:50:16AM +0200, Michał Pecio wrote:
> > @@ -1351,8 +1369,18 @@ static u32 xhci_get_endpoint_max_burst(struct usb_device *udev,
> >  
> >  	if (udev->speed == USB_SPEED_HIGH &&
> >  	    (usb_endpoint_xfer_isoc(&ep->desc) ||
> > -	     usb_endpoint_xfer_int(&ep->desc)))
> > +	     usb_endpoint_xfer_int(&ep->desc))) {
> > +		/*
> > +		 * eUSB2 double isoc bw endpoints max packet field service
> > +		 * opportunity bits 12:11 are not valid, so set the ctx burst to
> > +		 * max service opportunity "2" as these eps support transferring
> > +		 * over 3072 bytes per interval
> > +		 */
> 
> I think a shorter comment would suffice: eUSB2 BWD uses fixed burst
> size and max packets bits 12:11 are invalid.

I'll use this:

+                * eUSB2 double isochronous BW ECN uses fixed burst size and max
+                * packets bits 12:11 are invalid.

-- 
Sakari Ailus

Re: [PATCH v4 1/4] xhci: Add host support for eUSB2 double isochronous bandwidth devices

[Michał Pecio]

On Wed, 20 Aug 2025 11:24:24 +0300, Sakari Ailus wrote:
> Hi Michał,
> 
> Thanks for the review.
> 
> On Mon, Aug 18, 2025 at 11:50:16AM +0200, Michał Pecio wrote:
> > > @@ -1351,8 +1369,18 @@ static u32 xhci_get_endpoint_max_burst(struct usb_device *udev,
> > >  
> > >  	if (udev->speed == USB_SPEED_HIGH &&
> > >  	    (usb_endpoint_xfer_isoc(&ep->desc) ||
> > > -	     usb_endpoint_xfer_int(&ep->desc)))
> > > +	     usb_endpoint_xfer_int(&ep->desc))) {
> > > +		/*
> > > +		 * eUSB2 double isoc bw endpoints max packet field service
> > > +		 * opportunity bits 12:11 are not valid, so set the ctx burst to
> > > +		 * max service opportunity "2" as these eps support transferring
> > > +		 * over 3072 bytes per interval
> > > +		 */  
> > 
> > I think a shorter comment would suffice: eUSB2 BWD uses fixed burst
> > size and max packets bits 12:11 are invalid.  
> 
> I'll use this:
> 
> +                * eUSB2 double isochronous BW ECN uses fixed burst size and max
> +                * packets bits 12:11 are invalid.
>

Fine. And by the way, it may look like my comment was overly pedantic,
but I though that mentioning "more than 3072 bytes per interval" not
only isn't necessary here, but it adds confusion. It was something that
would be more relevant to 'mult' than 'burst.

Re: [PATCH v4 1/4] xhci: Add host support for eUSB2 double isochronous bandwidth devices

[Sakari Ailus]

On Wed, Aug 20, 2025 at 10:43:04AM +0200, Michał Pecio wrote:
> On Wed, 20 Aug 2025 11:24:24 +0300, Sakari Ailus wrote:
> > Hi Michał,
> > 
> > Thanks for the review.
> > 
> > On Mon, Aug 18, 2025 at 11:50:16AM +0200, Michał Pecio wrote:
> > > > @@ -1351,8 +1369,18 @@ static u32 xhci_get_endpoint_max_burst(struct usb_device *udev,
> > > >  
> > > >  	if (udev->speed == USB_SPEED_HIGH &&
> > > >  	    (usb_endpoint_xfer_isoc(&ep->desc) ||
> > > > -	     usb_endpoint_xfer_int(&ep->desc)))
> > > > +	     usb_endpoint_xfer_int(&ep->desc))) {
> > > > +		/*
> > > > +		 * eUSB2 double isoc bw endpoints max packet field service
> > > > +		 * opportunity bits 12:11 are not valid, so set the ctx burst to
> > > > +		 * max service opportunity "2" as these eps support transferring
> > > > +		 * over 3072 bytes per interval
> > > > +		 */  
> > > 
> > > I think a shorter comment would suffice: eUSB2 BWD uses fixed burst
> > > size and max packets bits 12:11 are invalid.  
> > 
> > I'll use this:
> > 
> > +                * eUSB2 double isochronous BW ECN uses fixed burst size and max
> > +                * packets bits 12:11 are invalid.
> >
> 
> Fine. And by the way, it may look like my comment was overly pedantic,
> but I though that mentioning "more than 3072 bytes per interval" not
> only isn't necessary here, but it adds confusion. It was something that
> would be more relevant to 'mult' than 'burst.

Further I corrected the ECN title, the result being:

+                * USB 2 Isochronous Double IN Bandwidth ECN uses fixed burst
+                * size and max packets bits 12:11 are invalid.

There are now quite a few other changes for v5 as well, I'll post it
soonish.

-- 
Sakari Ailus