-----Original Message-----
From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com]
Sent: Tuesday, September 23, 2014 8:12 AM
To: Jason Gerecke
Cc: linux-input@vger.kernel.org; jkosina@suse.cz; pinglinux@gmail.com;
Jason Gerecke
Subject: Re: [PATCH] HID: input: Finish TransducerSerialNumber
implementation

Hi Jason,

On Sep 22 2014 or thereabouts, Jason Gerecke wrote:

quoted

The commit which introduced TransducerSerialNumber (368c966) is
missing two crucial implementation details. Firstly, the commit does
not set the type/code/bit/max fields as expected later down the code
which can cause the driver to crash when a tablet with this usage is
connected. Secondly, the code to send a MSC_SERIAL event to userspace
when this usage is seen in a report is nowhere to be found. This commit

addresses both issues.

quoted

Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
 drivers/hid/hid-input.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index

2619f7f..abed624 100644

--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c

@@ -690,6 +690,10 @@ static void hidinput_configure_usage(struct

hid_input *hidinput, struct hid_fiel

 		case 0x5b: /* TransducerSerialNumber */
 			set_bit(MSC_SERIAL, input->mscbit);
+			usage->type = EV_MSC;
+			usage->code = MSC_SERIAL;
+			bit = input->mscbit;
+			max = MSC_MAX;
 			break;

 		default:  goto unknown;

@@ -1041,6 +1045,11 @@ void hidinput_hid_event(struct hid_device *hid,

struct hid_field *field, struct

quoted

 		input_event(input, EV_KEY, BTN_TOUCH, value > a + ((b - a)

quoted

3));

 	}

+	if (usage->hid == (HID_UP_DIGITIZER | 0x005b)) { /*

TransducerSerialNumber */

quoted

+		input_event(input, EV_MSC, MSC_SERIAL, value);
+		return;
+	}
+

Are you sure that the generic call to
"input_event(input, usage->type, usage->code, value);" in the end of
hidinput_hid_event() is not sufficient enough?

As long as the usage/code are set in the hid_usage struct, you should be fine.

Cheers,
Benjamin

Hmmm... This might be an issue with the HID descriptor, actually. It looks like my hardware doesn't set the logical minimum and maximum for the field, so the (quite out-of-range) value is ignored. I start seeing data after patching the descriptor to add these in, but the code is set to MSC_PULSELED instead of MSC_SERIAL. I suspect that the existing call to 'set_bit' in the TransducerSerialNumber case is to blame (which makes it act like 'map_foo' instead of 'map_foo_clear').

Updated patch forthcoming...

Jason

quoted

 	if (usage->hid == (HID_UP_PID | 0x83UL)) { /* Simultaneous Effects

Max */

quoted

 		dbg_hid("Maximum Effects - %d\n",value);
 		return;
--
2.1.0