On Mon, Mar 06, 2017 at 10:54:44AM +0100, Oliver Neukum wrote: > Am Montag, den 06.03.2017, 10:23 +0100 schrieb Johan Hovold: > > [ +CC: Greg ] > > > > On Fri, Mar 03, 2017 at 07:29:02PM +0100, Oliver Neukum wrote: > > > > > > Am Donnerstag, den 02.03.2017, 12:51 +0100 schrieb Johan Hovold: > > > > > > > > This series refactors the endpoint sanity checks by allowing > > > > subdrivers > > > > to specify a minimum number of endpoints required per type and > > > > letting > > > > core verify this during probe. > > > > > > Hi, > > > > > > this is very good, but is it at the right place? I see nothing > > > specific to serial drivers here. > > > > There are some usb-serial specifics here, most notably that the > > constraints are specified per usb-serial type (driver) of which there > > can be several per usb driver, and all with different ports counts > > and varying endpoint requirements. > > Yes, it would be wrong to see this as an attribute per driver. > It needs to apply when you match. AFAICS every serial subdriver > has a device table. Requirements need to go there. Every USB-serial-bus driver has a device table, but devices are matched against these tables by USB-serial core and not USB core. And then there's also a "combined" device table, for all USB-serial bus drivers implemented by the same USB interface driver. Are you suggesting the requirements should be specified per device, rather than per driver (device table)? That does not seem right to me, as it is really the driver which needs to verify the resources it needs for its implementation (e.g. before allocating a read urb). This means that all devices in a table would have the same (minimum) constraints anyway. So I do think this should be a driver attribute, but in the USB-serial case, several classes of devices can be handled by the same USB driver, so the constraints then needs to be specified at the USB-serial-bus driver level instead. > > As mentioned in the cover letter, a follow-on series will also use > > the descriptor arrays to allow subdrivers to manipulate the > > port-endpoint mapping. We have a couple of driver for which a single > > endpoint pair is shared, but bulk-out resources are allocated per > > port anyway. To be able to get rid of some related hacks, the array > > sizes are therefore large enough to express such mappings rather > > than having 15 elements each (i.e. the maximum number of in or out > > endpoints). > > Yes, but could you explain how this is connected to filtering at probe > time other than happening at probe time? Only in that this functionality will still be needed in USB-serial to implement the port-endpoint remap functionality even if USB core were to gain something similar. > > Also note that the constraints are verified only after the subdriver > > probe callback returns (but before calc_num_ports and attach) > > allowing the drivers to change altsettings, download firmware, etc, > > before verifying that the endpoints required for normal operation > > are present. > > True, but again not specific to serial drivers. DVB-T will face > the same issues. Certainly, and I should have elaborated a bit more on the differences here. For USB-serial endpoint-resources are allocated only after subdriver probe returns, while for most (all) other USB driver this would need to be done in the USB probe callback directly. In that case, checking before probe might be too soon (fw download, altsetting) and checking after would be too late (endpoint resources would already need to have been setup). > > Now, something similar to this could be added to USB core as well, > > and I believe Greg have been looking into that lately (as time > > permitted). But given the varying subdriver constraints, and the > > need for port- endpoint remapping, we'd still need these checks and > > features in USB serial. > > It looks like a duplicated effort to me and your code can already do > most of the job. Just add a flag to check the requirements after > probe() I realise that it may look like a duplicated effort, but I really don't think it is for the reasons given in this thread. This does not mean that for some USB-serial drivers, some of these checks could be taken care of by USB core later (e.g. when the endpoint constraints are the same for all device classes managed by a USB-serial driver). Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
