Thread (10 messages) 10 messages, 6 authors, 2013-02-05

Re: [PATCH] HID: Separate struct hid_device's driver_lock into two locks.

From: Benjamin Tissoires <hidden>
Date: 2012-11-26 18:34:20

Hi Bruno,

On Mon, Nov 26, 2012 at 6:56 PM, Bruno Prémont
[off-list ref] wrote:
Hi Andrew,

[CCing David Herrmann]


On Sun, 25 November 2012 Andrew de los Reyes wrote:
quoted
Benjamin Tissoires and Nestor Lopez Casado have been helping me to add
Linux support for new Logitech Touch Mice (T620, T400). After running
into a road-block in hid-core, and solving it with this patch, we
thought it was good to show the community and see if this is okay, or
if there's a better solution that we've missed.

Thanks for your comments,
-andrew

This patch separates struct hid_device's driver_lock into two. The
goal is to allow hid device drivers to receive input during their
probe() function call. This is necessary because some drivers need to
communicate with the device to determine parameters needed during
probe (e.g., size of a multi-touch surface).

Historically, three functions used driver_lock:

- hid_device_probe: blocks to acquire lock
- hid_device_remove: blocks to acquire lock
- hid_input_report: if locked returns -EBUSY, else acquires lock

This patch adds another lock (driver_input_lock) which is used to
block input from occurring. The lock behavior is now:

- hid_device_probe: blocks to acq. driver_lock, then driver_input_lock
- hid_device_remove: blocks to acq. driver_lock, then driver_input_lock
- hid_input_report: if driver_input_lock locked returns -EBUSY, else
  acquires driver_input_lock

This results in no behavior change for existing devices and
drivers. However, during a probe() function call in a driver, that
driver may now selectively unlock driver_input_lock to let input
events come through, then re-lock.
That is more or less a new approach to release the restriction added in
commit 4ea5454203d991ec85264f64f89ca8855fce69b0
(HID: Fix race condition between driver core and ll-driver).

From your suggested change the question could be if releasing the input
lock should be connected to hw_start() / hw_stop() calls...

Though connecting those together would certainly require some review of
existing drivers in order not to reopen the can of worms closed by above
mentioned commit.
It is clear that the goal is to make commit
4ea5454203d991ec85264f64f89ca8855fce69b0 less restrictive.
The problem is that this commit stops any communication with the
device, even configuration communication.

Logitech devices use their own protocol on top of the HID standard
protocol. For touch devices, this proprietary protocol requires to ask
the device for axis ranges, etc...

So here, the idea is not to open the can of worm for every hid devices
through hw_start() / hw_stop() calls, I think the idea of Andrew is
just to allow hid-logitech-dj to get rid of this restriction in some
particular circumstances.
Consider this as a controlled backdoor of the can of worms :)

Cheers,
Benjamin
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help