[PATCH] hid: add two led codes to hid input mapping

[PATCH] hid: add two led codes to hid input mapping

[Dan Engel]

From: Daniel P. Engel <dengel@sourceharvest.com>

Add the "Off-hook" and "Speaker" LED codes 0xb and 0xc to the hid-input configuration,
mapping them to the 0x17 and 0x1e usages in the HID usage table.

Signed-off-by: Daniel P. Engel <dengel@sourceharvest.com>
---
This patch is really being offered because it's what's needed to make the operation
of the Belkin Flip USB KVM switch avaiable to user-space programs through the HID input
event interface. The Belkin Flip KVM overloads LED usages to give software control
over the device, providing options to flip either audio, video or both. However,
without an input mapping to the Off-hook and Speaker LED usages, this functionality
isn't available.

It's a minor patch, adding two led codes to the EV_LED type, and mapping them to
corresponding HID usages.

This patch was created against kernel version 2.6.20.4.

diff -uprN -X linux-2.6.20.4-vanilla/Documentation/dontdiff linux-2.6.20.4-vanilla/drivers/hid/hid-input.c linux-2.6/drivers/hid/hid-input.c
--- linux-2.6.20.4-vanilla/drivers/hid/hid-input.c	2007-03-23 15:52:51.000000000 -0400
+++ linux-2.6/drivers/hid/hid-input.c	2007-03-31 13:43:46.000000000 -0400
@@ -381,6 +381,8 @@ static void hidinput_configure_usage(str
 				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" */
+				case 0x17:  map_led (LED_OFFHOOK);  break;    /*   "Off Hook"                 */
+				case 0x1e:  map_led (LED_SPEAKER);  break;    /*   "Speaker"                  */
 
 				default: goto ignore;
 			}
diff -uprN -X linux-2.6.20.4-vanilla/Documentation/dontdiff linux-2.6.20.4-vanilla/include/linux/input.h linux-2.6/include/linux/input.h
--- linux-2.6.20.4-vanilla/include/linux/input.h	2007-03-23 15:52:51.000000000 -0400
+++ linux-2.6/include/linux/input.h	2007-03-31 13:42:22.000000000 -0400
@@ -630,6 +630,8 @@ struct input_absinfo {
 #define LED_MISC		0x08
 #define LED_MAIL		0x09
 #define LED_CHARGING		0x0a
+#define LED_OFFHOOK		0x0b
+#define LED_SPEAKER		0x0c
 #define LED_MAX			0x0f
 
 /*

Re: [PATCH] hid: add two led codes to hid input mapping

[Jiri Kosina]

On Sat, 31 Mar 2007, Dan Engel wrote:

> This patch is really being offered because it's what's needed to make the operation
> of the Belkin Flip USB KVM switch avaiable to user-space programs through the HID input
> event interface. The Belkin Flip KVM overloads LED usages to give software control
> over the device, providing options to flip either audio, video or both. However,
> without an input mapping to the Off-hook and Speaker LED usages, this functionality
> isn't available.

Dmitry, would adding these two LED_ constants to input.h be OK by you? 
(the coresponding usages are defined in HUT 1.12 on page 62).

Thanks,

-- 
Jiri Kosina

Re: [PATCH] hid: add two led codes to hid input mapping

[Dmitry Torokhov]

On Sunday 01 April 2007 05:01, Jiri Kosina wrote:
> On Sat, 31 Mar 2007, Dan Engel wrote:
> 
> > This patch is really being offered because it's what's needed to make the operation
> > of the Belkin Flip USB KVM switch avaiable to user-space programs through the HID input
> > event interface. The Belkin Flip KVM overloads LED usages to give software control
> > over the device, providing options to flip either audio, video or both. However,
> > without an input mapping to the Off-hook and Speaker LED usages, this functionality
> > isn't available.
> 
> Dmitry, would adding these two LED_ constants to input.h be OK by you? 
> (the coresponding usages are defined in HUT 1.12 on page 62).
> 

No, I do not want to add any more LED constants to input. In fact I think
that adding constants beyond keyboard indicators was a mistake. We have 
led subsystem that provides interface to control arbitrary leds and we
should use it.

-- 
Dmitry

Re: [PATCH] hid: add two led codes to hid input mapping

[Jiri Kosina]

On Sun, 1 Apr 2007, Dmitry Torokhov wrote:

> No, I do not want to add any more LED constants to input. In fact I 
> think that adding constants beyond keyboard indicators was a mistake. We 
> have led subsystem that provides interface to control arbitrary leds and 
> we should use it.

Ah, of course. Dan, could you please redo the patch to use the leds 
infrastructure and send it to me?

Thanks,

-- 
Jiri Kosina

Re: [PATCH] hid: add two led codes to hid input mapping

[Dan Engel]

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]

On Sun, 2007-04-01 at 19:43 +0200, Jiri Kosina wrote:
> On Sun, 1 Apr 2007, Dmitry Torokhov wrote:
> 
> > No, I do not want to add any more LED constants to input. In fact I 
> > think that adding constants beyond keyboard indicators was a mistake. We 
> > have led subsystem that provides interface to control arbitrary leds and 
> > we should use it.
> 
> Ah, of course. Dan, could you please redo the patch to use the leds 
> infrastructure and send it to me?

Perhaps I could find a way to do that, but it seem rather a round-about
way to go about it. The fact is that the input subsystem is already the
home of this device.

As far as I can tell, the led subsystem doesn't already provide any
framework for mapping LED's to HID input events. I could possibly create
a driver in the led subsystem that sends the corresponding events to the
input subsystem, but those events would be dropped by the input system,
anyway, since the event class would still have to be EV_LED and the code
would still be unrecognized without those two mappings.

Given that the HID or input subsystem still needs to be in the loop,
however the driver is written, can you give any advice for accomplishing
that without adding the extra LED mappings?

Thanks for any help,
-Dan Engel


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] hid: add two led codes to hid input mapping

[Dmitry Torokhov]

Hi Dan,

On Sunday 01 April 2007 23:28, Dan Engel wrote:
> On Sun, 2007-04-01 at 19:43 +0200, Jiri Kosina wrote:
> > On Sun, 1 Apr 2007, Dmitry Torokhov wrote:
> > 
> > > No, I do not want to add any more LED constants to input. In fact I 
> > > think that adding constants beyond keyboard indicators was a mistake. We 
> > > have led subsystem that provides interface to control arbitrary leds and 
> > > we should use it.
> > 
> > Ah, of course. Dan, could you please redo the patch to use the leds 
> > infrastructure and send it to me?
> 
> Perhaps I could find a way to do that, but it seem rather a round-about
> way to go about it. The fact is that the input subsystem is already the
> home of this device.
> 
> As far as I can tell, the led subsystem doesn't already provide any
> framework for mapping LED's to HID input events. I could possibly create
> a driver in the led subsystem that sends the corresponding events to the
> input subsystem, but those events would be dropped by the input system,
> anyway, since the event class would still have to be EV_LED and the code
> would still be unrecognized without those two mappings.
>

No, no, do not try to route the control through evdev. Since the LED
override is device-sepcific (i.e yours is the only device which will
react in that way) I'd recommend using sysfs attributes to control it.
Hm, we may need to wait for HID bus to clearly do that in a separate
driver...
 
> Given that the HID or input subsystem still needs to be in the loop,
> however the driver is written, can you give any advice for accomplishing
> that without adding the extra LED mappings?

Actually I want to keep input subystm out of the loop here, since LEDs
such as mail, charging, etc have nothing to do with user input but
rather reflect overall system/application state.

-- 
Dmitry

Re: [PATCH] hid: add two led codes to hid input mapping

[Dan Engel]

[-- Attachment #1: Type: text/plain, Size: 1187 bytes --]

On Mon, 2007-04-02 at 00:04 -0400, Dmitry Torokhov wrote:

> Actually I want to keep input subystm out of the loop here, since LEDs
> such as mail, charging, etc have nothing to do with user input but
> rather reflect overall system/application state.
> 

What if I just added a HID_QUIRK_HIDDEV for the vendor/product id's in
the blacklist table in hiddev.c. That would force the creation of a
hiddev device, through which a user-space program could access the HID
usages directly.

It would be awkward, though, since there would also be an input event
device, but one through which the full functionality is not accessible.
Is there a HID_QUIRK that would force creation of a hiddev *instead of*
(rather than in addition to) an event dev?

Two things to keep in mind:

1) The device in question (it's a Belkin USB Flip KVM switch, just to
restate from the OP) has nothing to do with LED's. It just overloads the
LED HID usage codes to receive control from the PC.

2) No matter what we do in terms of presenting this device to user-land,
under the hood it's a HID-class USB device, and it probably doesn't make
sense to not make use of that.

Thanks,
-Dan

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] hid: add two led codes to hid input mapping

[Dmitry Torokhov]

On 4/2/07, Dan Engel <dan@sourceharvest.com> wrote:
> On Mon, 2007-04-02 at 00:04 -0400, Dmitry Torokhov wrote:
>
> > Actually I want to keep input subystm out of the loop here, since LEDs
> > such as mail, charging, etc have nothing to do with user input but
> > rather reflect overall system/application state.
> >
>
> What if I just added a HID_QUIRK_HIDDEV for the vendor/product id's in
> the blacklist table in hiddev.c. That would force the creation of a
> hiddev device, through which a user-space program could access the HID
> usages directly.
>
> It would be awkward, though, since there would also be an input event
> device, but one through which the full functionality is not accessible.

What other evets/usages does this device support?

> Is there a HID_QUIRK that would force creation of a hiddev *instead of*
> (rather than in addition to) an event dev?

No but it could be added.

>
> Two things to keep in mind:
>
> 1) The device in question (it's a Belkin USB Flip KVM switch, just to
> restate from the OP) has nothing to do with LED's. It just overloads the
> LED HID usage codes to receive control from the PC.

Ah, OK.

> 2) No matter what we do in terms of presenting this device to user-land,
> under the hood it's a HID-class USB device, and it probably doesn't make
> sense to not make use of that.

It may be a HID-class device but it is definetly not an input device
and it would be wrong to present it to userspace as a device having 2
LEDs on it - it would be a lie. If we did that then some application
might mistake the device for something else and decide to switch that
LED off thus turning your KVM off. A random application should not be
aware how a random vendor decided to interpret the specs.

-- 
Dmitry

Re: [PATCH] hid: add two led codes to hid input mapping

[Dan Engel]

[-- Attachment #1: Type: text/plain, Size: 2096 bytes --]

On Mon, 2007-04-02 at 09:06 -0400, Dmitry Torokhov wrote:
> It may be a HID-class device but it is definetly not an input device
> and it would be wrong to present it to userspace as a device having 2
> LEDs on it - it would be a lie. If we did that then some application
> might mistake the device for something else and decide to switch that
> LED off thus turning your KVM off. A random application should not be
> aware how a random vendor decided to interpret the specs.

Okay, I did a little more exploration of the total device (I was
focusing on the PC control of switching up until now.)

This HID interface has two aspects:

One is the PC control over switching. This is represented by a single
report, with 3 fields, each with a single on/off usage. It can switch
KVM only, audio only, or both. This is the one that overloads the HID
LED usages, and is (as you say) not properly an input device.

The other one appears to have something to do with the case where you've
switched only audio. It's a single input report with four fields that
map to the key usages volume up, volume down, mute (I think), and
eject. 

So, I think the "right" thing to is to allow it to be driven as both a
hiddev and an evdev (which is what the HID_QUIRK_HIDDEV does for this
device.)

Then, the switch controls would be accessed only through hiddev, while
the key events, if they're ever actually generated, would still be
received by the evdev driver.

On a side note, I'm not sure why those input key events are there. The
basic idea of the device is that you can switch KVM control back and
forth, while keeping audio on a single computer (e.g., to listen to
music), and at first I thought those key events were to feed audio
controls through from the computer with KVM focus to the one with audio
focus, so that if you're working on either computer you can adjust the
volume, etc. However, the computer that does not have KVM focus will
never receive these events, since switching KVM focus effectively
disconnects the device completely from the computer.



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] hid: add two led codes to hid input mapping

[Dmitry Torokhov]

On 4/2/07, Dan Engel <dan@sourceharvest.com> wrote:
>
> On a side note, I'm not sure why those input key events are there. The
> basic idea of the device is that you can switch KVM control back and
> forth, while keeping audio on a single computer (e.g., to listen to
> music), and at first I thought those key events were to feed audio
> controls through from the computer with KVM focus to the one with audio
> focus, so that if you're working on either computer you can adjust the
> volume, etc. However, the computer that does not have KVM focus will
> never receive these events, since switching KVM focus effectively
> disconnects the device completely from the computer.
>

* Pulled out of my behind idea *
It could be that the KVM allows you to limit or normalize volume
coming out of your speakers so that you don't get blasted with full
sound if switch from one box to another.

-- 
Dmitry