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

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Alan,

On Fri, Aug 08, 2025 at 11:16:22AM -0400, Alan Stern wrote:
> On Fri, Aug 08, 2025 at 08:18:58AM +0000, Sakari Ailus wrote:
> > > >  
> > > > +/**
> > > > + * 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.
> > > > + */
> > > > +static inline 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);
> > > > +	}
> > > > +}
> > > 
> > > This function is complicated enough that it probably should not be an 
> > > inline routine.  Not unless it's used in only one place (in which case 
> > > why define it in a .h file?).
> > 
> > The single user currently (and probably will be for some time at least) is
> > the UVC driver but this code is better located in the USB tree. I'd keep it
> > this way until we have more users, unless Mathias can suggest where the
> > function should be located.
> 
> Okay.  But in the meantime, how about putting just a declaration for 
> this function in include/linux/usb.h and putting the actual (non-inline) 
> definition in drivers/usb/core/usb.c, or in drivers/usb/common/common.c 
> if you think the function would be used in gadget-side drivers as well 
> as host-side drivers?

I'll move it to usb.c in v4.

-- 
Regards,

Sakari Ailus