On Wed, 13 Feb 2013 21:08:20 -0800 Andrew de los Reyes wrote: > > > > /** > > > > + * hid_device_io_start - enable HID input during probe, remove > > > > + * > > > > + * @hid - the device > > > > + * > > > > + * This should only be called during probe or remove. It will allow > > > > + * incoming packets to be delivered to the driver. > > > > + */ > > > > +static inline void hid_device_io_start(struct hid_device *hid) { > > > > + up(&hid->driver_input_lock); > > > > + hid->io_started = true; > > > > > > Shouldn't these lines be swapped? Doesn't matter but it looks weird to > > > me this way. > > > > > > But more importantly, we must go sure this is called from the same > > > thread that probe() is called on. Other use-cases are not supported by > > > semaphores and might break due to missing barriers. So maybe the > > > comment could include that? > > > > Maybe even check what value hid->io_started has and WARN() [+skip > > up()/down()] > > in case hid_device_io_start()/stop() was not called in a balanced way. > > It is a goal to support hid_device_io_start()/stop() not being called > in a balanced way. This is specifically needed by a change I'm making > to hid-logitech-dj.c: During probe(), the driver will send out a > request to the USB dongle to list any attached mice/keyboards. The > reply packets aren't needed during probe(), so probe will return, but > the driver wants packets to flow in, and it doesn't mind if they come > in while probe() is still running or not. In this case, during > probe(), the driver will call hid_device_io_start() to allow incoming > packets, but then not call hid_device_io_stop(). With unbalanced I meant calling hid_device_io_start()/stop() twice or more in a row in _probe() (or _remove()) without corresponding _stop()/_start() call. That kind of cases might happen (mostly) with error paths. Like the following (simplified) example when both if() match: int driver_probe(...) { ... if (...) { ... hid_device_io_start(); ... } ... if (...) { ... hid_device_io_start(); ... } ... } Bruno -- 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