LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator)
@ 2007-01-14 23:11 Simon Budig
  2007-01-15  8:42 ` Jiri Kosina
  2007-01-15 16:28 ` [PATCH 2.6.20-rc5] " Simon Budig
  0 siblings, 2 replies; 12+ messages in thread
From: Simon Budig @ 2007-01-14 23:11 UTC (permalink / raw)
  To: vojtech; +Cc: linux-kernel

From: Simon Budig <simon@budig.de>

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 <simon@budig.de>

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:

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator)
  2007-01-14 23:11 [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator) Simon Budig
@ 2007-01-15  8:42 ` Jiri Kosina
  2007-01-15 16:25   ` Simon Budig
  2007-01-15 16:28 ` [PATCH 2.6.20-rc5] " Simon Budig
  1 sibling, 1 reply; 12+ messages in thread
From: Jiri Kosina @ 2007-01-15  8:42 UTC (permalink / raw)
  To: Simon Budig; +Cc: Vojtech Pavlik, linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator)
  2007-01-15  8:42 ` Jiri Kosina
@ 2007-01-15 16:25   ` Simon Budig
  2007-01-15 16:44     ` Jiri Kosina
  0 siblings, 1 reply; 12+ messages in thread
From: Simon Budig @ 2007-01-15 16:25 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Vojtech Pavlik, linux-kernel

Jiri Kosina (jikos@jikos.cz) 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
-- 
              simon@budig.de              http://simon.budig.de/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2.6.20-rc5] USB HID: proper LED-mapping (support for SpaceNavigator)
  2007-01-14 23:11 [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator) Simon Budig
  2007-01-15  8:42 ` Jiri Kosina
@ 2007-01-15 16:28 ` Simon Budig
  2007-01-15 17:34   ` Simon Budig
  1 sibling, 1 reply; 12+ messages in thread
From: Simon Budig @ 2007-01-15 16:28 UTC (permalink / raw)
  To: vojtech; +Cc: linux-kernel, Jiri Kosina

From: Simon Budig <simon@budig.de>

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 <simon@budig.de>


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:

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator)
  2007-01-15 16:25   ` Simon Budig
@ 2007-01-15 16:44     ` Jiri Kosina
  2007-01-15 17:32       ` Simon Budig
  2007-01-15 18:32       ` Vojtech Pavlik
  0 siblings, 2 replies; 12+ messages in thread
From: Jiri Kosina @ 2007-01-15 16:44 UTC (permalink / raw)
  To: Simon Budig; +Cc: Vojtech Pavlik, linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator)
  2007-01-15 16:44     ` Jiri Kosina
@ 2007-01-15 17:32       ` Simon Budig
  2007-01-15 21:14         ` Jiri Kosina
  2007-01-15 18:32       ` Vojtech Pavlik
  1 sibling, 1 reply; 12+ messages in thread
From: Simon Budig @ 2007-01-15 17:32 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Vojtech Pavlik, linux-kernel

Jiri Kosina (jikos@jikos.cz) 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
-- 
              simon@budig.de              http://simon.budig.de/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6.20-rc5] USB HID: proper LED-mapping (support for SpaceNavigator)
  2007-01-15 16:28 ` [PATCH 2.6.20-rc5] " Simon Budig
@ 2007-01-15 17:34   ` Simon Budig
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Budig @ 2007-01-15 17:34 UTC (permalink / raw)
  To: vojtech; +Cc: linux-kernel, Jiri Kosina

From: Simon Budig <simon@budig.de>

This trivial change adds some missing enum values to the hid-debug output.

Signed-off-by: Simon Budig <simon@budig.de>


--- 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] = {

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator)
  2007-01-15 16:44     ` Jiri Kosina
  2007-01-15 17:32       ` Simon Budig
@ 2007-01-15 18:32       ` Vojtech Pavlik
  2007-01-15 18:38         ` Marcel Holtmann
  1 sibling, 1 reply; 12+ messages in thread
From: Vojtech Pavlik @ 2007-01-15 18:32 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Simon Budig, linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator)
  2007-01-15 18:32       ` Vojtech Pavlik
@ 2007-01-15 18:38         ` Marcel Holtmann
  2007-01-15 18:56           ` Vojtech Pavlik
  0 siblings, 1 reply; 12+ messages in thread
From: Marcel Holtmann @ 2007-01-15 18:38 UTC (permalink / raw)
  To: Vojtech Pavlik; +Cc: Jiri Kosina, Simon Budig, linux-kernel

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator)
  2007-01-15 18:38         ` Marcel Holtmann
@ 2007-01-15 18:56           ` Vojtech Pavlik
  0 siblings, 0 replies; 12+ messages in thread
From: Vojtech Pavlik @ 2007-01-15 18:56 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Jiri Kosina, Simon Budig, linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator)
  2007-01-15 17:32       ` Simon Budig
@ 2007-01-15 21:14         ` Jiri Kosina
  2007-01-15 22:55           ` Simon Budig
  0 siblings, 1 reply; 12+ messages in thread
From: Jiri Kosina @ 2007-01-15 21:14 UTC (permalink / raw)
  To: Simon Budig; +Cc: Vojtech Pavlik, linux-kernel, Marcel Holtmann

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator)
  2007-01-15 21:14         ` Jiri Kosina
@ 2007-01-15 22:55           ` Simon Budig
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Budig @ 2007-01-15 22:55 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-kernel

Jiri Kosina (jikos@jikos.cz) 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
-- 
              simon@budig.de              http://simon.budig.de/

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2007-01-15 22:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-01-14 23:11 [PATCH 2.6.19] USB HID: proper LED-mapping (support for SpaceNavigator) Simon Budig
2007-01-15  8:42 ` Jiri Kosina
2007-01-15 16:25   ` Simon Budig
2007-01-15 16:44     ` Jiri Kosina
2007-01-15 17:32       ` Simon Budig
2007-01-15 21:14         ` Jiri Kosina
2007-01-15 22:55           ` Simon Budig
2007-01-15 18:32       ` Vojtech Pavlik
2007-01-15 18:38         ` Marcel Holtmann
2007-01-15 18:56           ` Vojtech Pavlik
2007-01-15 16:28 ` [PATCH 2.6.20-rc5] " Simon Budig
2007-01-15 17:34   ` Simon Budig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).