From: Simon Budig <[email protected]>
This change introduces a mapping for LED indicators between the USB HID
specification and the Linux input subsystem. The previous code properly
mapped the LEDs relevant for Keyboards, but garbeled the remaining ones.
With this change all LED enums from the input system get mapped to more
or less equivalent LED numbers from the HID specification.
This patch also extends the debug output and ensures that the unused
bits in a HID report to the device are zeroed out. This makes the
3Dconnexion SpaceNavigator fully usable with the linux input system.
Signed-off-by: Simon Budig <[email protected]>
diff -uprN -X linux/Documentation/dontdiff linux/drivers/usb/input.orig/hid-core.c linux/drivers/usb/input/hid-core.c
--- linux/drivers/usb/input.orig/hid-core.c 2006-11-29 22:57:37.000000000 +0100
+++ linux/drivers/usb/input/hid-core.c 2007-01-15 00:01:25.000000000 +0100
@@ -1089,6 +1089,10 @@ static void hid_output_field(struct hid_
unsigned size = field->report_size;
unsigned n;
+ /* make sure the unused bits in the last byte are zeros */
+ if (count > 0 && size > 0)
+ data[(count*size-1)/8] = 0;
+
for (n = 0; n < count; n++) {
if (field->logical_minimum < 0) /* signed values */
implement(data, offset + n * size, size, s32ton(field->value[n], size));
diff -uprN -X linux/Documentation/dontdiff linux/drivers/usb/input.orig/hid-debug.h linux/drivers/usb/input/hid-debug.h
--- linux/drivers/usb/input.orig/hid-debug.h 2006-11-29 22:57:37.000000000 +0100
+++ linux/drivers/usb/input/hid-debug.h 2007-01-08 15:36:51.000000000 +0100
@@ -700,9 +700,10 @@ static char *keys[KEY_MAX + 1] = {
static char *relatives[REL_MAX + 1] = {
[REL_X] = "X", [REL_Y] = "Y",
- [REL_Z] = "Z", [REL_HWHEEL] = "HWheel",
- [REL_DIAL] = "Dial", [REL_WHEEL] = "Wheel",
- [REL_MISC] = "Misc",
+ [REL_Z] = "Z", [REL_RX] = "Rx",
+ [REL_RY] = "Ry", [REL_RZ] = "Rz",
+ [REL_HWHEEL] = "HWheel", [REL_DIAL] = "Dial",
+ [REL_WHEEL] = "Wheel", [REL_MISC] = "Misc",
};
static char *absolutes[ABS_MAX + 1] = {
diff -uprN -X linux/Documentation/dontdiff linux/drivers/usb/input.orig/hid-input.c linux/drivers/usb/input/hid-input.c
--- linux/drivers/usb/input.orig/hid-input.c 2006-11-29 22:57:37.000000000 +0100
+++ linux/drivers/usb/input/hid-input.c 2007-01-08 17:20:16.000000000 +0100
@@ -366,9 +366,22 @@ static void hidinput_configure_usage(str
break;
case HID_UP_LED:
- if (((usage->hid - 1) & 0xffff) >= LED_MAX)
- goto ignore;
- map_led((usage->hid - 1) & 0xffff);
+
+ switch (usage->hid & 0xffff) { /* HID-Value: */
+ case 0x01: map_led (LED_NUML); break; /* Num Lock */
+ case 0x02: map_led (LED_CAPSL); break; /* Caps Lock */
+ case 0x03: map_led (LED_SCROLLL); break; /* Scroll Lock */
+ case 0x04: map_led (LED_COMPOSE); break; /* Compose */
+ case 0x05: map_led (LED_KANA); break; /* Kana */
+ case 0x27: map_led (LED_SLEEP); break; /* Stand-By */
+ case 0x4c: map_led (LED_SUSPEND); break; /* System Suspend */
+ case 0x09: map_led (LED_MUTE); break; /* Mute */
+ case 0x4b: map_led (LED_MISC); break; /* Generic Indicator */
+ case 0x19: map_led (LED_MAIL); break; /* Message Waiting */
+ case 0x4d: map_led (LED_CHARGING); break; /* External Power Connected */
+
+ default: goto ignore;
+ }
break;
case HID_UP_DIGITIZER:
On Mon, 15 Jan 2007, Simon Budig wrote:
> This change introduces a mapping for LED indicators between the USB HID
> specification and the Linux input subsystem. The previous code properly
> mapped the LEDs relevant for Keyboards, but garbeled the remaining ones.
> With this change all LED enums from the input system get mapped to more
> or less equivalent LED numbers from the HID specification. This patch
> also extends the debug output and ensures that the unused bits in a HID
> report to the device are zeroed out. This makes the 3Dconnexion
> SpaceNavigator fully usable with the linux input system.
Hi Simon,
thanks for the patch. It seems that it is based on pre-2.6.20-rc1 kernel
(this is where the USBHID split happened and generic HID layer was
introduced). Could you please rebase it against newer version of kernel
and resend it?
All your changes happen to be in the transport-independent code, so it
seems that this would be rather trivial task - probably only pathnames
(and diff offsets) will change - your changes should now go to
drivers/hid/hid-*, not drivers/usb/input/hid-*.
Thanks,
--
Jiri Kosina
Jiri Kosina ([email protected]) wrote:
> thanks for the patch. It seems that it is based on pre-2.6.20-rc1 kernel
> (this is where the USBHID split happened and generic HID layer was
> introduced). Could you please rebase it against newer version of kernel
> and resend it?
I've updated the patch, will submit it to the list in a few minutes.
> All your changes happen to be in the transport-independent code, so it
> seems that this would be rather trivial task - probably only pathnames
> (and diff offsets) will change - your changes should now go to
> drivers/hid/hid-*, not drivers/usb/input/hid-*.
Yeah, it was easy to port over. Did the hid-debug stuff disappear
completely? What would I use instead?
Thanks,
Simon
--
[email protected] http://simon.budig.de/
From: Simon Budig <[email protected]>
This change introduces a mapping for LED indicators between the USB HID
specification and the Linux input subsystem. The previous code properly
mapped the LEDs relevant for Keyboards, but garbeled the remaining ones.
With this change all LED enums from the input system get mapped to more
or less equivalent LED numbers from the HID specification.
This patch also ensures that the unused bits in a HID report to the
device are zeroed out. This makes the 3Dconnexion SpaceNavigator fully
usable with the linux input system.
Signed-off-by: Simon Budig <[email protected]>
diff -uprN -X linux/Documentation/dontdiff linux-2.6.19/drivers/hid/hid-core.c linux/drivers/hid/hid-core.c
--- linux-2.6.19/drivers/hid/hid-core.c 2007-01-15 17:06:37.000000000 +0100
+++ linux/drivers/hid/hid-core.c 2007-01-15 16:59:16.000000000 +0100
@@ -880,6 +880,10 @@ static void hid_output_field(struct hid_
unsigned size = field->report_size;
unsigned n;
+ /* make sure the unused bits in the last byte are zeros */
+ if (count > 0 && size > 0)
+ data[(count*size-1)/8] = 0;
+
for (n = 0; n < count; n++) {
if (field->logical_minimum < 0) /* signed values */
implement(data, offset + n * size, size, s32ton(field->value[n], size));
diff -uprN -X linux/Documentation/dontdiff linux-2.6.19/drivers/hid/hid-input.c linux/drivers/hid/hid-input.c
--- linux-2.6.19/drivers/hid/hid-input.c 2007-01-15 17:06:37.000000000 +0100
+++ linux/drivers/hid/hid-input.c 2007-01-15 17:03:20.000000000 +0100
@@ -364,9 +364,22 @@ static void hidinput_configure_usage(str
break;
case HID_UP_LED:
- if (((usage->hid - 1) & 0xffff) >= LED_MAX)
- goto ignore;
- map_led((usage->hid - 1) & 0xffff);
+
+ switch (usage->hid & 0xffff) { /* HID-Value: */
+ case 0x01: map_led (LED_NUML); break; /* "Num Lock" */
+ case 0x02: map_led (LED_CAPSL); break; /* "Caps Lock" */
+ case 0x03: map_led (LED_SCROLLL); break; /* "Scroll Lock" */
+ case 0x04: map_led (LED_COMPOSE); break; /* "Compose" */
+ case 0x05: map_led (LED_KANA); break; /* "Kana" */
+ case 0x27: map_led (LED_SLEEP); break; /* "Stand-By" */
+ case 0x4c: map_led (LED_SUSPEND); break; /* "System Suspend" */
+ case 0x09: map_led (LED_MUTE); break; /* "Mute" */
+ case 0x4b: map_led (LED_MISC); break; /* "Generic Indicator" */
+ case 0x19: map_led (LED_MAIL); break; /* "Message Waiting" */
+ case 0x4d: map_led (LED_CHARGING); break; /* "External Power Connected" */
+
+ default: goto ignore;
+ }
break;
case HID_UP_DIGITIZER:
On Mon, 15 Jan 2007, Simon Budig wrote:
> > thanks for the patch. It seems that it is based on pre-2.6.20-rc1 kernel
> > (this is where the USBHID split happened and generic HID layer was
> > introduced). Could you please rebase it against newer version of kernel
> > and resend it?
> I've updated the patch, will submit it to the list in a few minutes.
I got it, thanks.
> > All your changes happen to be in the transport-independent code, so it
> > seems that this would be rather trivial task - probably only pathnames
> > (and diff offsets) will change - your changes should now go to
> > drivers/hid/hid-*, not drivers/usb/input/hid-*.
> Yeah, it was easy to port over. Did the hid-debug stuff disappear
> completely? What would I use instead?
No, it didn't disappear, it was just moved to include/linux/hid-debug.h.
Should I wait for an updated patch that uses hid-debug.h again?
Thanks,
--
Jiri Kosina
Jiri Kosina ([email protected]) wrote:
> On Mon, 15 Jan 2007, Simon Budig wrote:
> > Yeah, it was easy to port over. Did the hid-debug stuff disappear
> > completely? What would I use instead?
>
> No, it didn't disappear, it was just moved to include/linux/hid-debug.h.
> Should I wait for an updated patch that uses hid-debug.h again?
Thanks, I missed that. Since these issues are unrelated I'll just submit
a trivial patch for the hid-debug.h stuff.
Is it possible that there is a regression in the hid-debug stuff? The
mapping does not seem to appear in the dmesg-output. I unfortunately
don't have an earlier kernel available right now to verify, but now the
output on plugging in the device looks like this:
[...]
usbcore: registered new interface driver hiddev
hid-debug: input GenericDesktop.X = 0
hid-debug: input GenericDesktop.Y = 0
hid-debug: input GenericDesktop.Z = 0
hid-debug: input GenericDesktop.Rx = 0
hid-debug: input GenericDesktop.Ry = 0
hid-debug: input GenericDesktop.Rz = 0
[...]
which looks bogus to me, IIRC earlier versions really printed the
mapping to linux input events at this point.
Anyway, the patch is correct anyway, will submit it soon.
Bye,
Simon
--
[email protected] http://simon.budig.de/
From: Simon Budig <[email protected]>
This trivial change adds some missing enum values to the hid-debug output.
Signed-off-by: Simon Budig <[email protected]>
--- linux/include/linux/hid-debug.h.orig 2007-01-15 18:19:52.000000000 +0100
+++ linux/include/linux/hid-debug.h 2007-01-15 18:21:44.000000000 +0100
@@ -700,9 +700,10 @@ static char *keys[KEY_MAX + 1] = {
static char *relatives[REL_MAX + 1] = {
[REL_X] = "X", [REL_Y] = "Y",
- [REL_Z] = "Z", [REL_HWHEEL] = "HWheel",
- [REL_DIAL] = "Dial", [REL_WHEEL] = "Wheel",
- [REL_MISC] = "Misc",
+ [REL_Z] = "Z", [REL_RX] = "Rx",
+ [REL_RY] = "Ry", [REL_RZ] = "Rz",
+ [REL_HWHEEL] = "HWheel", [REL_DIAL] = "Dial",
+ [REL_WHEEL] = "Wheel", [REL_MISC] = "Misc",
};
static char *absolutes[ABS_MAX + 1] = {
On Mon, Jan 15, 2007 at 05:44:26PM +0100, Jiri Kosina wrote:
> On Mon, 15 Jan 2007, Simon Budig wrote:
>
> > > thanks for the patch. It seems that it is based on pre-2.6.20-rc1 kernel
> > > (this is where the USBHID split happened and generic HID layer was
> > > introduced). Could you please rebase it against newer version of kernel
> > > and resend it?
> > I've updated the patch, will submit it to the list in a few minutes.
>
> I got it, thanks.
>
> > > All your changes happen to be in the transport-independent code, so it
> > > seems that this would be rather trivial task - probably only pathnames
> > > (and diff offsets) will change - your changes should now go to
> > > drivers/hid/hid-*, not drivers/usb/input/hid-*.
> > Yeah, it was easy to port over. Did the hid-debug stuff disappear
> > completely? What would I use instead?
>
> No, it didn't disappear, it was just moved to include/linux/hid-debug.h.
Do you think that makes sense? It's code, not a header file.
> Should I wait for an updated patch that uses hid-debug.h again?
--
Vojtech Pavlik
Director SuSE Labs
Hi Vojtech,
> > No, it didn't disappear, it was just moved to include/linux/hid-debug.h.
>
> Do you think that makes sense? It's code, not a header file.
>
> > Should I wait for an updated patch that uses hid-debug.h again?
actually that code shouldn't be in a header file at all. It should be
drivers/hid/hid-debug.c and the Makefile should compile it conditionally
depending on a Kconfig option.
Regards
Marcel
On Mon, Jan 15, 2007 at 07:38:10PM +0100, Marcel Holtmann wrote:
> Hi Vojtech,
>
> > > No, it didn't disappear, it was just moved to include/linux/hid-debug.h.
> >
> > Do you think that makes sense? It's code, not a header file.
> >
> > > Should I wait for an updated patch that uses hid-debug.h again?
>
> actually that code shouldn't be in a header file at all. It should be
> drivers/hid/hid-debug.c and the Makefile should compile it conditionally
> depending on a Kconfig option.
Agreed. It was a .h file just because of me being lazy.
--
Vojtech Pavlik
Director SuSE Labs
On Mon, 15 Jan 2007, Simon Budig wrote:
> Is it possible that there is a regression in the hid-debug stuff? The
> mapping does not seem to appear in the dmesg-output. I unfortunately
> don't have an earlier kernel available right now to verify, but now the
> output on plugging in the device looks like this:
Hi Simon,
thanks, I queued the LED mapping fix for upstream.
I agree with Vojtech and Marcel that it doesn't make much sense having the
hid-debug as a header file - I will fix it, and apply your patch to it
(after I check why the debug output seems to be broken), you don't have to
resend it, thanks.
--
Jiri Kosina
Jiri Kosina ([email protected]) wrote:
> On Mon, 15 Jan 2007, Simon Budig wrote:
> > Is it possible that there is a regression in the hid-debug stuff? The
> > mapping does not seem to appear in the dmesg-output. I unfortunately
> > don't have an earlier kernel available right now to verify, but now the
> > output on plugging in the device looks like this:
>
[...]
> (after I check why the debug output seems to be broken),
Actually this might have been a false alarm. I remembered about
/var/log/messages and looked up how this looked like with earlier
kernels - turns out it looks exactly the same.
(the values dumped there seem to be the initial values of a given field
in a HID-Report)
So there is no regression there, sorry about the confusion.
Bye,
Simon
--
[email protected] http://simon.budig.de/