On Dec 12, 2014 3:05 AM, "David Herrmann" <dh.herrmann@xxxxxxxxx> wrote: > > Hi > > On Thu, Dec 11, 2014 at 6:36 PM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote: > > It happens during driver probe. I don't set GET_REPORT to my device > > -- the protocol consists solely of SET_REPORT from the driver followed > > by an asynchronous (but generally reasonably quick) report back from > > the device. > > > > During probe, I set INIT, and the device responds by saying "I'm a > > device. I speak this version of the protocol. > > > > Shouldn't that code be more like: > > > > mutex_lock(whatever); > > if (driver loaded) > > deliver the report; > > mutex_unlock(whatever); > > > > and then the core could hold the lock briefly as well when probing or > > in hid_hw_open/hid_hw_close. > > If we only lock around a boolean flag, we need a wait-queue to make > sure a driver-removal does not run before driver-probe is done. I > haven't looked at it in detail, maybe we're only called from > driver-core so we already have this guarantee. Not sure. Feel free to > send patches. Hmm. I'll look, but I may just stick with hid_device_io_start. > > So far, the solution is to use hid_device_io_start() and > hid_device_io_stop() in ->probe(). It's a hack to allow I/O during > ->probe(). Maybe your idea is the way to go, but unless someone > implements it, we're stuck with the current code. That should work. For normal HID drivers (those that don't start IO from probe), what happens if userspace starts IO before probe returns? Does the driver code prevent that somehow? Thanks, Andy > > Please have a look at some other drivers in drivers/hid/ to see how > hid_device_io_start() / hid_device_io_stop() is used. You should be > fine by just calling hid_device_io_start() from ->probe() after your > context setup is done and you called hid_hw_start(). > > > (Also, shouldn't hid_hw_open have some kind of reference counting to > > avoid interference between hidraw and real drivers? Or does that > > already work correctly somehow?) > > Kinda.. currently, hidraw is treated as a passive client. That is, the > hid-device-driver manages the device and hidraw does not interfere. If > no device-driver is loaded, hid_hw_start/stop is called by hid-core, > so hidraw will usually work fine. However, if a device-driver is > loaded, it gets exclusive access to the hardware so is also > responsible of hid_hw_start/stop. hidraw will follow its lead. Note > that this is _required_ by some devices as vendors are really crappy > in writing HID firmware. Think like devices which need an > initialization after transport-layer setup, or they will timeout... or > continuous beacon events... It's stuff like that.. Not sure whether we > still have drivers for broken devices like that, but afaik that's > where this originated. > > Thanks > David -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
