On Tue, 5 Feb 2013, David Herrmann wrote: > > Yes, I'm hoping to upstream soon as part of a patch series to get WTP > > support in. The current version of the commit you are asking about is > > specifically here: > > https://github.com/adlr/linux/commit/2a4fae572bd88462c4a558a77dc53d9eb3f9e91c > > > > The main difference from before is that there is a new return value (1) > > which signifies that the event lock has been been up()ed, so the hid-core > > doesn't need to call up() itself. This way during probe(), a driver can > > allow incoming events continuously from midway through probe() indefinitely. > > The patch looks nice. Some comments: > > 1) I would prefer constants HID_IS_LOCKED and HID_IS_UNLOCKED or > similar instead of 1. This makes the code much more readable than > returning 0 or 1. Maybe you can find better names than my suggestions? > > 2) in hid_device_probe() you can use "ret = -EINTR; goto unlock;" > instead of "up(input_lock); return -EINTR;". I think this is more > consistent. > > 3) in hid_device_probe() maybe check that hid_hw_start() doesn't > return "1" accidentally? The current code deadlocks if hid_hw_start() > would be changed to return 1 (which would be bad as is, but maybe be > rather safe than sorry?). > > 4) Maybe we can add a similar logic to hid_device_remove? That is, > hid_device_remove() shouldn't lock input_lock if the driver wants to > perform I/O during remove, too. Because this may cause the driver to > drop input events while the lock is held. > At least the hid-wiimote driver could make use of this feature during removal. David, what use-case exactly are you thinking of here, please? Thanks, -- Jiri Kosina SUSE Labs -- 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
