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

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

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