From: Martin Pieuchot Subject: Re: Fix dumping ISOCHRONOUS IN transfers To: tech@openbsd.org Date: Fri, 20 Dec 2024 20:09:46 +0100 On 20/12/24(Fri) 20:04, Kirill A. Korinsky wrote: > On Fri, 20 Dec 2024 19:40:02 +0100, > Martin Pieuchot wrote: > > > > On 19/12/24(Thu) 14:18, Kirill A. Korinsky wrote: > > > [...] > > > However, the situation is different for IN transfers. The kernel reads > > > packets from the device, but they may be shorter than the expected size. As > > > a result, the kernel tracks the total length of packets read using > > > xfer->actlen and updates xfer->frlengths[i] accordingly. > > > > Understood. I'd prefer if we could find another solution than adding an > > array to every USB transfer descriptor. > > It seems to me that all isochronous USB drivers use frames of the same size, > > which mean we could calculate the offset with: > > > > (xfer->length / xfer->nframes) > > > > Could that work? > > > > On a related note, is their any OS and/or driver that use isochronous > > transfers with frames of different sizes? > > > > I made a quick grep through the Linux sources and was able to locate an > example with a different offset and length in a driver: > https://github.com/torvalds/linux/blob/v6.12/drivers/isdn/hardware/mISDN/hfcsusb.c#L1245-L1321 > > Linux uses the following structure to describe each isochronous packet: > > struct usb_iso_packet_descriptor { > unsigned int offset; > unsigned int length; /* expected length */ > unsigned int actual_length; > int status; > }; > > Since we have a few isochronous drivers, what do you think about migrating > the code to follow the same approach? I mean that the driver should allocate > an array of usb_iso_packet_descriptor instead of an array of uint16_t. Thanks for the digging! I'd rather fix usb_tap() with a comment explaining why the hack I described above work and that it should be revisited if we start supporting a device like the one you found.