Hi Bastien
On Tue, Sep 20, 2011 at 3:35 AM, Bastien Nocera <hadess@xxxxxxxxxx> wrote:
> Heya,
>
> I'm pretty certain that's already been discussed, but can't seem to find
> the thread in question, or whether somebody sent patches for the issue.
>
> The problem I see is that the input device is advertised really early in
> its creation, and seems to be missing information when udev (and layers
> above it in the stack) probes it.
>
> Using Linux 3.1.0 rc6
>
> Turning on a Bluetooth Wacom tablet I see the following udev events:
> UDEV [1890.604848] add /devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.1/1-1.1:1.0/bluetooth/hci0/hci0:42/input10 (input)
> UDEV_LOG=3
> ACTION=add
> DEVPATH=/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.1/1-1.1:1.0/bluetooth/hci0/hci0:42/input10
> SUBSYSTEM=input
> PRODUCT=5/56a/81/100
> NAME="WACOM Pen Tablet"
> PHYS="00:13:EF:F1:42:B7"
> UNIQ="00:13:C2:03:3D:D6"
> PROP=0
> EV=17
> KEY=70000 0 0 0 0
> REL=103
> MSC=10
> MODALIAS=input:b0005v056Ap0081e0100-e0,1,2,4,k110,111,112,r0,1,8,am4,lsfw
> SEQNUM=2391
> ID_INPUT=1
> ID_INPUT_MOUSE=1
> ID_PATH=pci-0000:00:1a.0-usb-0:1.1:1.0
> ID_PATH_TAG=pci-0000_00_1a_0-usb-0_1_1_1_0
> ID_FOR_SEAT=input-pci-0000_00_1a_0-usb-0_1_1_1_0
> TAGS=:seat:
>
> If I do a udev database dump I see:
> P: /devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.1/1-1.1:1.0/bluetooth/hci0/hci0:42/input10
> E: UDEV_LOG=3
> E: DEVPATH=/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.1/1-1.1:1.0/bluetooth/hci0/hci0:42/input10
> E: PRODUCT=5/56a/81/100
> E: NAME="WACOM Pen Tablet"
> E: PHYS="00:13:EF:F1:42:B7"
> E: UNIQ="00:13:C2:03:3D:D6"
> E: PROP=0
> E: EV=1f
> E: KEY=1c63 70003 0 0 0 0
> E: REL=103
> E: ABS=3000003
> E: MSC=11
> E: MODALIAS=input:b0005v056Ap0081e0100-e0,1,2,3,4,k100,101,110,111,112,140,141,145,146,14A,14B,14C,r0,1,8,a0,1,18,19,m0,4,lsfw
> E: SUBSYSTEM=input
> E: ID_INPUT=1
> E: ID_INPUT_MOUSE=1
> E: ID_PATH=pci-0000:00:1a.0-usb-0:1.1:1.0
> E: ID_PATH_TAG=pci-0000_00_1a_0-usb-0_1_1_1_0
> E: ID_FOR_SEAT=input-pci-0000_00_1a_0-usb-0_1_1_1_0
> E: TAGS=:seat:
>
> See the different KEY= value, and the presence of the ABS= key. If the
> tablet was probed at this point it would have the ID_INPUT_TABLET=1
> property (as running input_id by hand shows).
>
> Any ideas?
The hid-wacom.c driver should probably use the input_mapped() callback
instead of setting the input flags in wacom_probe(). input_mapped() is
called for every descriptor field so there must at least be one valid
field to make it work. I don't have the device so I can't test it, but
try something like this:
>From 2f91a998b1bca4b654903e41394fae19dfed1a3c Mon Sep 17 00:00:00 2001
From: David Herrmann <dh.herrmann@xxxxxxxxxxxxxx>
Date: Tue, 20 Sep 2011 10:54:06 +0200
Subject: [PATCH] HID: wacom: Set input bits before registration
We shouldn't change the event flags of input devices after they get registered.
Otherwise, udev will not get notified of these flags and cannot setup the
devices properly.
This fixes the probing to set the input event flags on the input_mapped callback
instead of the probe function.
Reported-by: Bastien Nocera <hadess@xxxxxxxxxx>
Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxxxxxxx>
---
drivers/hid/hid-wacom.c | 76 +++++++++++++++++++++++++---------------------
1 files changed, 41 insertions(+), 35 deletions(-)
diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index 0688832..bc00ef0 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -304,11 +304,49 @@ static int wacom_raw_event(struct hid_device
*hdev, struct hid_report *report,
return 1;
}
+static int wacom_input_mapped(struct hid_device *hdev, struct hid_input *hi,
+ struct hid_field *field, struct hid_usage *usage, unsigned long **bit,
+ int *max)
+{
+ struct input_dev *input = hi->input;
+
+ /* Basics */
+ input->evbit[0] |= BIT(EV_KEY) | BIT(EV_ABS) | BIT(EV_REL);
+
+ __set_bit(REL_WHEEL, input->relbit);
+
+ __set_bit(BTN_TOOL_PEN, input->keybit);
+ __set_bit(BTN_TOUCH, input->keybit);
+ __set_bit(BTN_STYLUS, input->keybit);
+ __set_bit(BTN_STYLUS2, input->keybit);
+ __set_bit(BTN_LEFT, input->keybit);
+ __set_bit(BTN_RIGHT, input->keybit);
+ __set_bit(BTN_MIDDLE, input->keybit);
+
+ /* Pad */
+ input->evbit[0] |= BIT(EV_MSC);
+
+ __set_bit(MSC_SERIAL, input->mscbit);
+
+ __set_bit(BTN_0, input->keybit);
+ __set_bit(BTN_1, input->keybit);
+ __set_bit(BTN_TOOL_FINGER, input->keybit);
+
+ /* Distance, rubber and mouse */
+ __set_bit(BTN_TOOL_RUBBER, input->keybit);
+ __set_bit(BTN_TOOL_MOUSE, input->keybit);
+
+ input_set_abs_params(input, ABS_X, 0, 16704, 4, 0);
+ input_set_abs_params(input, ABS_Y, 0, 12064, 4, 0);
+ input_set_abs_params(input, ABS_PRESSURE, 0, 511, 0, 0);
+ input_set_abs_params(input, ABS_DISTANCE, 0, 32, 0, 0);
+
+ return 0;
+}
+
static int wacom_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
- struct hid_input *hidinput;
- struct input_dev *input;
struct wacom_data *wdata;
int ret;
@@ -380,39 +418,6 @@ static int wacom_probe(struct hid_device *hdev,
move_on:
#endif
- hidinput = list_entry(hdev->inputs.next, struct hid_input, list);
- input = hidinput->input;
-
- /* Basics */
- input->evbit[0] |= BIT(EV_KEY) | BIT(EV_ABS) | BIT(EV_REL);
-
- __set_bit(REL_WHEEL, input->relbit);
-
- __set_bit(BTN_TOOL_PEN, input->keybit);
- __set_bit(BTN_TOUCH, input->keybit);
- __set_bit(BTN_STYLUS, input->keybit);
- __set_bit(BTN_STYLUS2, input->keybit);
- __set_bit(BTN_LEFT, input->keybit);
- __set_bit(BTN_RIGHT, input->keybit);
- __set_bit(BTN_MIDDLE, input->keybit);
-
- /* Pad */
- input->evbit[0] |= BIT(EV_MSC);
-
- __set_bit(MSC_SERIAL, input->mscbit);
-
- __set_bit(BTN_0, input->keybit);
- __set_bit(BTN_1, input->keybit);
- __set_bit(BTN_TOOL_FINGER, input->keybit);
-
- /* Distance, rubber and mouse */
- __set_bit(BTN_TOOL_RUBBER, input->keybit);
- __set_bit(BTN_TOOL_MOUSE, input->keybit);
-
- input_set_abs_params(input, ABS_X, 0, 16704, 4, 0);
- input_set_abs_params(input, ABS_Y, 0, 12064, 4, 0);
- input_set_abs_params(input, ABS_PRESSURE, 0, 511, 0, 0);
- input_set_abs_params(input, ABS_DISTANCE, 0, 32, 0, 0);
return 0;
@@ -448,6 +453,7 @@ static struct hid_driver wacom_driver = {
.probe = wacom_probe,
.remove = wacom_remove,
.raw_event = wacom_raw_event,
+ .input_mapped = wacom_input_mapped,
};
static int __init wacom_init(void)
--
1.7.6.1
This calls input_mapped for every hid-desc field so this is kind of an
overhead but should be enough for a first quick fix. If it works we
could modify it further.
Looking at the other hid drivers that use these quirks, we could
probably need some kind of callback that is called only once for each
input device that is registered instead of once for every field. On
the other hand, the input_mapped callback works quite well for this,
even though it is called multiple times.
Reagards
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
