On Thu, Dec 09, 2021 at 08:53:37AM +0100, Pavel Hofman wrote: > Hi, > > in > https://elixir.bootlin.com/linux/latest/source/drivers/usb/core/config.c#L409 > the initial value of maxp is obtained using function usb_endpoint_maxp. > > maxp = usb_endpoint_maxp(&endpoint->desc); > > This function https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/usb/ch9.h#L647 > returns only the bits 0 - 10 of the wMaxPacketSize field, i.e. dropping the > high-bandwidth bits 11 and 12. Yet the subsequent code extracts these bits > from maxp into variable i > https://elixir.bootlin.com/linux/latest/source/drivers/usb/core/config.c#L427 > , clears them in maxp, and re-sets back in one of the further checks > https://elixir.bootlin.com/linux/latest/source/drivers/usb/core/config.c#L445 > > IMO that means the code requires that initial value of maxp contains the > additional-transactions bits. IMO the code should be fixed with this trivial > patch (tested on my build): > > > =================================================================== > diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c > --- a/drivers/usb/core/config.c (revision > 018dd9dd80ab5f3bd988911b1f10255029ffa52d) > +++ b/drivers/usb/core/config.c (date 1638972286064) > @@ -406,7 +406,7 @@ > * the USB-2 spec requires such endpoints to have wMaxPacketSize = 0 > * (see the end of section 5.6.3), so don't warn about them. > */ > - maxp = usb_endpoint_maxp(&endpoint->desc); > + maxp = endpoint->desc.wMaxPacketSize; > if (maxp == 0 && !(usb_endpoint_xfer_isoc(d) && asnum == 0)) { > dev_warn(ddev, "config %d interface %d altsetting %d endpoint 0x%X has > invalid wMaxPacketSize 0\n", > cfgno, inum, asnum, d->bEndpointAddress); > > > ========================= > > I can send a proper patch should the change be approved. Please always just send a real patch, that makes it easier to discuss. Anyway, what problem is this solving? Do you have a device where the data is calculated incorrectly? What value in a device is being declared incorrect because of the existing code? thanks, greg k-h
