Hi Jiri On Wed, Feb 6, 2013 at 4:10 PM, Jiri Kosina <jkosina@xxxxxxx> wrote: > 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? If you unbind a driver via sysfs, hid-wiimote could suspend several peripherals of a Wii Remote to safe energy. I don't know whether that's a use-case that we actively support, but I thought it would be nice to have if we are on it, anyway. I know that during normal device removal no I/O is possible (obviously), but during manual probe/remove this might happen. Regards 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
