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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to