On Wed, Apr 12, 2023 at 08:52:45PM +0200, Oliver Neukum wrote: > > > On 12.04.23 17:08, Alan Stern wrote: > > On Wed, Apr 12, 2023 at 01:54:12PM +0200, Oliver Neukum wrote: > > > > these will do the job. Yet this strikes me as unelegant. That is > > > if you define a data structure to match against, why not > > > add a pointer to it to struct usb_device_id and use that? > > > > Struct usb_device_id doesn't seem like the right place. Struct > > Conceptually it belongs there. It describes a device, not a driver. > I would say that the name is not well chosen, but it is the right place. > > > usb_driver would be more appropriate. The drivers that need this have > > only one entry in their match table, which means that drivers with large > > Why would that be the case? As far as I can see everything that is not > a class driver will need this or an equivalent and many of them > support multiple types of devices. In fact, I'm not sure where to find examples of drivers needing this. Apparently only one in drivers/usb/misc/ could benefit (uss720). The other already use usb_find_common_endpoints() or related functions. Some of the drivers in atm/ also could use some work. But there must be plenty of others; I just don't know where to look. The point about how many different devices a driver supports is irrelevant; what matters is how it checks the endpoints it will use. If a driver assumes that all the devices it supports will have three bulk-OUT endpoints numbered 1, 2, and 3 then it doesn't need a separate entry for each usb_device_id in its match table. > > match tables (which would require a lot of extra space for the new > > pointers) don't need it. > > A few dozen bytes. Ho, ho. Look at usb-storage or uas and think again: two useless 8-byte pointers added to each and every entry in the unusual_devs tables. > > True, the checks could be centralized in usb_probe_interface(). What do > > you think about doing it that way? > > That really is the best place to put the code for checking. > And you might put into a comment that the way USB works the endpoint > number includes the direction. It is not obvious to a casual reader. The patches already contain such comments, in the patch description and in the kerneldoc. Alan Stern
