Re: [PATCH] usb: core: Fix bandwidth for devices with invalid wBytesPerInterval

["Xuetao (kirin)" <xuetao09@huawei.com>]

在 2026/4/2 11:51, Greg KH 写道:
> On Thu, Apr 02, 2026 at 10:14:00AM +0800, Tao Xue wrote:
>> As specified in Section 4.14.2 of the xHCI Specification, the xHC
>> reserves bandwidth for periodic endpoints according to bInterval and
>> wBytesPerInterval (Max ESIT Payload).
>>
>> Some peripherals report an invalid wBytesPerInterval in their device
>> descriptor, which is either 0 or smaller than the actual data length
>> transmitted. This issue is observed on ASIX AX88179 series USB 3.0
>> Ethernet adapters.
>>
>> These errors may lead to unexpected behavior on certain USB host
>> controllers, causing USB peripherals to malfunction.
>>
>> To address the issue, return max(wBytesPerInterval, max_payload) when
>> calculating bandwidth reservation.
>>
>> Fixes: 9238f25d5d32 ("USB: xhci: properly set endpoint context fields for periodic eps.")
>> Cc: <stable@kernel.org>
>> Signed-off-by: Tao Xue <xuetao09@huawei.com>
>> ---
>>   drivers/usb/core/usb.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
>> index e9a10a33534c..8f2e05a5a015 100644
>> --- a/drivers/usb/core/usb.c
>> +++ b/drivers/usb/core/usb.c
>> @@ -1125,6 +1125,8 @@ EXPORT_SYMBOL_GPL(usb_free_noncoherent);
>>   u32 usb_endpoint_max_periodic_payload(struct usb_device *udev,
>>   				      const struct usb_host_endpoint *ep)
>>   {
>> +	u32 max_payload;
>> +
>>   	if (!usb_endpoint_xfer_isoc(&ep->desc) &&
>>   	    !usb_endpoint_xfer_int(&ep->desc))
>>   		return 0;
>> @@ -1135,7 +1137,12 @@ u32 usb_endpoint_max_periodic_payload(struct usb_device *udev,
>>   			return le32_to_cpu(ep->ssp_isoc_ep_comp.dwBytesPerInterval);
>>   		fallthrough;
>>   	case USB_SPEED_SUPER:
>> -		return le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval);
>> +		max_payload = usb_endpoint_maxp(&ep->desc) * (ep->ss_ep_comp.bMaxBurst + 1);
>> +		if (usb_endpoint_xfer_isoc(&ep->desc))
>> +			return max_t(u32, max_payload * USB_SS_MULT(ep->ss_ep_comp.bmAttributes),
>> +					ep->ss_ep_comp.wBytesPerInterval);
>> +		else
>> +			return max_t(u32, max_payload, ep->ss_ep_comp.wBytesPerInterval);
> 
> You dropped the conversion from le16 to cpu?  Why?
> 
> thanks,
> 
> greg k-h

Hi Greg,

Thank you for the review.

1、You're right, that was an oversight. I should keep the le16_to_cpu().
Here's the corrected version:

     max_payload = usb_endpoint_maxp(&ep->desc) * 
(ep->ss_ep_comp.bMaxBurst + 1);
     if (usb_endpoint_xfer_isoc(&ep->desc))
         return max_t(u32, max_payload * 
USB_SS_MULT(ep->ss_ep_comp.bmAttributes),
                         le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval));
     else
         return max_t(u32, max_payload, 
le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval));

2、Following Alan's suggestion in another email, should I check whether 
wBytesPerInterval is a valid value and handle it in the 
usb_parse_ss_endpoint_companion() ?

However, when parsing the device descriptor, we do not know whether the 
actual data length transmitted by the peripheral is greater than 
wBytesPerInterval.

Therefore, would it be sufficient to only add a check for whether 
wBytesPerInterval is 0 in the existing flow, and if it is 0, set 
wBytesPerInterval to cpu_to_le16(max_tx) by default?

For example, modify it in the following way：

      if (le16_to_cpu(desc->wBytesPerInterval) > max_tx || 
le16_to_cpu(desc->wBytesPerInterval) == 0) {
         dev_notice(ddev, "%s endpoint with wBytesPerInterval of %d in "
                 "config %d interface %d altsetting %d ep %d: "
                 "setting to %d\n",
                 usb_endpoint_xfer_isoc(&ep->desc) ? "Isoc" : "Int",
                 le16_to_cpu(desc->wBytesPerInterval),
                 cfgno, inum, asnum, ep->desc.bEndpointAddress,
                 max_tx);
         ep->ss_ep_comp.wBytesPerInterval = cpu_to_le16(max_tx);
     }

  Could you please give me some advice? Thanks.