HID: Support telephony devices

Message ID	1470214307-29441-1-git-send-email-nolsen@jabra.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <linux-input-owner@kernel.org> From: nolsen@jabra.com To: linux-input@vger.kernel.org Cc: jikos@kernel.org, benjamin.tissoires@redhat.com, dmitry.torokhov@gmail.com, Niels Skou Olsen <nolsen@jabra.com> Subject: [PATCH] HID: Support telephony devices Date: Wed, 3 Aug 2016 10:51:47 +0200 Message-Id: <1470214307-29441-1-git-send-email-nolsen@jabra.com> Sender: linux-input-owner@vger.kernel.org Precedence: bulk

nolsen@jabra.com Aug. 3, 2016, 8:51 a.m. UTC

From: Niels Skou Olsen <nolsen@jabra.com>

Add support for telephony device buttons and LEDs as described in the USB
HID Usage Tables specification.

This allows user mode applications convenient access to telephony devices
such as headsets, speaker phones and desk phones.

Signed-off-by: Niels Skou Olsen <nolsen@jabra.com>
---
 drivers/hid/hid-debug.c                | 39 ++++++++++++++++++++-
 drivers/hid/hid-input.c                | 63 +++++++++++++++++++++++++++++++---
 drivers/input/input-leds.c             |  5 +++
 include/linux/hid.h                    |  8 ++++-
 include/linux/mod_devicetable.h        |  2 +-
 include/uapi/linux/input-event-codes.h | 18 +++++++++-
 6 files changed, 126 insertions(+), 9 deletions(-)

Jiri Kosina Aug. 5, 2016, 11:43 a.m. UTC | #1

On Wed, 3 Aug 2016, nolsen@jabra.com wrote:

> From: Niels Skou Olsen <nolsen@jabra.com>
> 
> Add support for telephony device buttons and LEDs as described in the USB
> HID Usage Tables specification.
> 
> This allows user mode applications convenient access to telephony devices
> such as headsets, speaker phones and desk phones.
> 
> Signed-off-by: Niels Skou Olsen <nolsen@jabra.com>
> ---
>  drivers/hid/hid-debug.c                | 39 ++++++++++++++++++++-
>  drivers/hid/hid-input.c                | 63 +++++++++++++++++++++++++++++++---
>  drivers/input/input-leds.c             |  5 +++
>  include/linux/hid.h                    |  8 ++++-
>  include/linux/mod_devicetable.h        |  2 +-
>  include/uapi/linux/input-event-codes.h | 18 +++++++++-

I'd like (at least) the input-event-codes.h be acked by Dmitry. Dmitry, 
please?

Thanks,

Junge, Terry Aug. 11, 2016, 11:31 p.m. UTC | #2

Good to get telephony support into the kernel.

My only concerns are the duplicate functionalities of LED_RING::LED_RINGER and LED_MUTE::LED_MICROPHONE.

Please see below.

>-----Original Message-----
>From: linux-input-owner@vger.kernel.org [mailto:linux-input-owner@vger.kernel.org] On Behalf Of nolsen@jabra.com
>Sent: Wednesday, August 03, 2016 1:52 AM
>To: linux-input@vger.kernel.org
>Cc: jikos@kernel.org; benjamin.tissoires@redhat.com; dmitry.torokhov@gmail.com; Niels Skou Olsen
>Subject: [PATCH] HID: Support telephony devices
>
>From: Niels Skou Olsen <nolsen@jabra.com>
>
>Add support for telephony device buttons and LEDs as described in the USB
>HID Usage Tables specification.
>
>This allows user mode applications convenient access to telephony devices
>such as headsets, speaker phones and desk phones.
>
>Signed-off-by: Niels Skou Olsen <nolsen@jabra.com>
>---
> drivers/hid/hid-debug.c                | 39 ++++++++++++++++++++-
> drivers/hid/hid-input.c                | 63 +++++++++++++++++++++++++++++++---
> drivers/input/input-leds.c             |  5 +++
> include/linux/hid.h                    |  8 ++++-
> include/linux/mod_devicetable.h        |  2 +-
> include/uapi/linux/input-event-codes.h | 18 +++++++++-
> 6 files changed, 126 insertions(+), 9 deletions(-)
>
>diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
>index acfb522..9f109c3 100644
>--- a/drivers/hid/hid-debug.c
>+++ b/drivers/hid/hid-debug.c
>@@ -129,9 +129,42 @@ static const struct hid_usage_entry hid_usage_table[] = {
>       {0, 0x03, "ScrollLock"},
>       {0, 0x04, "Compose"},
>       {0, 0x05, "Kana"},
>+	{0, 0x09, "Mute"},
>+	{0, 0x17, "OffHook"},
>+	{0, 0x18, "Ring"},
>+	{0, 0x19, "MessageWaiting"},
>+	{0, 0x20, "Hold"},
>+	{0, 0x21, "Microphone"},
>+	{0, 0x27, "Standby"},
>+	{0, 0x2a, "Online"},
>+	{0, 0x4c, "SystemSuspend"},
>+	{0, 0x4d, "ExtPowerConnected"},
>       {0, 0x4b, "GenericIndicator"},
>   {  9, 0, "Button" },
>   { 10, 0, "Ordinal" },
>+	{ 11, 0, "Telephony" },
>+	{0, 0x20, "KeyHookSwitch"},
>+	{0, 0x21, "KeyFlash"},
>+	{0, 0x23, "KeyHold"},
>+	{0, 0x24, "KeyRedial"},
>+	{0, 0x2f, "KeyMicMute"},
>+	{0, 0x9e, "LedRinger"},

This is not an LED usage, it's a Locally Generated Tone usage, maybe "RingerTone"?

>+	{0, 0xb0, "KeyNumeric0"},
>+	{0, 0xb1, "KeyNumeric1"},
>+	{0, 0xb2, "KeyNumeric2"},
>+	{0, 0xb3, "KeyNumeric3"},
>+	{0, 0xb4, "KeyNumeric4"},
>+	{0, 0xb5, "KeyNumeric5"},
>+	{0, 0xb6, "KeyNumeric6"},
>+	{0, 0xb7, "KeyNumeric7"},
>+	{0, 0xb8, "KeyNumeric8"},
>+	{0, 0xb9, "KeyNumeric9"},
>+	{0, 0xba, "KeyNumericStar"},
>+	{0, 0xbb, "KeyNumericPound"},
>+	{0, 0xbc, "KeyNumericA"},
>+	{0, 0xbd, "KeyNumericB"},
>+	{0, 0xbe, "KeyNumericC"},
>+	{0, 0xbf, "KeyNumericD"},
>   { 12, 0, "Consumer" },
>       {0, 0x238, "HorizontalWheel"},
>   { 13, 0, "Digitizers" },
>@@ -998,7 +1031,11 @@ static const char *leds[LED_MAX + 1] = {
> 	[LED_SCROLLL] = "ScrollLock",	[LED_COMPOSE] = "Compose",
> 	[LED_KANA] = "Kana",		[LED_SLEEP] = "Sleep",
> 	[LED_SUSPEND] = "Suspend",	[LED_MUTE] = "Mute",
>-	[LED_MISC] = "Misc",
>+	[LED_MISC] = "Misc",		[LED_MAIL] = "Mail",
>+	[LED_CHARGING] = "Charging",	[LED_OFF_HOOK] = "Off-hook",
>+	[LED_RING] = "Ring",		[LED_RINGER] = "Ringer",

LED_RINGER duplicates the functionality of LED_RING to the user.
Multiple vendor products in the field already respond to the proposed
mapping of LED_RING by indicating inbound ring to the user.
Adding LED_RINGER to the core and mapping it to 0x000b009e
will require the user to decide which one to use, LED_RING or LED_RINGER.
If needed, the 0x000b009e usage should be mapped by a vendor driver
to LED_RING so the user will only have to deal with LED_RING.

>+	[LED_HOLD] = "Hold",		[LED_MICROPHONE] = "Microphone",

LED_MICROPHONE duplicates the functionality of LED_MUTE to the user.
Multiple vendor products in the field already respond to the core
mapping of LED_MUTE by muting the microphone.
Adding LED_MICROPHONE to the core and mapping it to 0x00080021
will require the user to decide which one to use, LED_MUTE or LED_MICROPHONE.
If needed, the 0x00080021 usage should be mapped by a vendor driver
to LED_MUTE so the user will only have to deal with LED_MUTE.

>+	[LED_ONLINE] = "Online"
> };
>
> static const char *repeats[REP_MAX + 1] = {
>diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
>index bcfaf32..b2a4136 100644
>--- a/drivers/hid/hid-input.c
>+++ b/drivers/hid/hid-input.c
>@@ -509,9 +509,11 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> 	if (field->report_count < 1)
> 		goto ignore;
>
>-	/* only LED usages are supported in output fields */
>+	/* only LED and TELEPHONY usages are supported in output fields */
> 	if (field->report_type == HID_OUTPUT_REPORT &&
>-			(usage->hid & HID_USAGE_PAGE) != HID_UP_LED) {
>+			(usage->hid & HID_USAGE_PAGE) != HID_UP_LED &&
>+			(usage->hid & HID_USAGE_PAGE) != HID_UP_TELEPHONY) {
>+
> 		goto ignore;
> 	}

This allowance for telephony page output mapping in order to map LED_RINGER
would not be needed if the 0x000b009e mapping were done by a vendor driver and
the vendor driver was allowed access before the currently preemptive LED page test, like this:

	if (device->driver->input_mapping) {
		int ret = device->driver->input_mapping(device, hidinput, field,
				usage, &bit, &max);
		if (ret > 0)
			goto mapped;
		if (ret < 0)
			goto ignore;
	}

	/* only LED usages are supported in output fields */
	if (field->report_type == HID_OUTPUT_REPORT &&
			(usage->hid & HID_USAGE_PAGE) != HID_UP_LED) {
		goto ignore;
	}

>
>@@ -658,11 +660,32 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> 		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 0x17: /* "Off hook" */
>+			map_led(LED_OFF_HOOK);
>+			break;
>+		case 0x18: /* "Ring" */
>+			map_led(LED_RING);
>+			break;
> 		case 0x19:  map_led (LED_MAIL);     break;    /*   "Message Waiting"          */
>+		case 0x20: /* "Hold" */
>+			map_led(LED_HOLD);
>+			break;
>+		case 0x21: /* "Microphone" */
>+			map_led(LED_MICROPHONE);
>+			break;

As previous comment on duplicate functionality of LED_MICROPHONE and LED_MUTE.
Usage 0x00080021 could be mapped to LED_MUTE in a vendor driver.

>+		case 0x27: /* "Stand-By" */
>+			map_led(LED_SLEEP);
>+			break;
>+		case 0x2a: /* "Online" */
>+			map_led(LED_ONLINE);
>+			break;
>+		case 0x4b: /* "Generic Indicator" */
>+			map_led(LED_MISC);
>+			break;
>+		case 0x4c: /* "System Suspend" */
>+			map_led(LED_SUSPEND);
>+			break;
> 		case 0x4d:  map_led (LED_CHARGING); break;    /*   "External Power Connected" */
>
> 		default: goto ignore;
>@@ -732,7 +755,37 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>
> 	case HID_UP_TELEPHONY:
> 		switch (usage->hid & HID_USAGE) {
>+		case 0x07:
>+			map_key_clear(KEY_PROGRAMMABLE);
>+			break;

----
>+		case 0x1e:
>+			map_led(LED_SPEAKER);
>+			break;
----

This is in the wrong place, it should be in the HID_UP_LED case switch statement block.
Speaker usage 0x1e is in the LED page not the telephony page.

>+		case 0x20:
>+			map_key_clear(KEY_HOOK_SWITCH);
>+			break;
>+		case 0x21:
>+			map_key_clear(KEY_FLASH);
>+			break;
>+		case 0x23:
>+			map_key_clear(KEY_HOLD);
>+			break;
>+		case 0x24:
>+			map_key_clear(KEY_REDIAL);
>+			break;
>+		case 0x2a:
>+			map_key_clear(KEY_LINE);
>+			break;
>+		case 0x2b:
>+			map_key_clear(KEY_SPEAKER_PHONE);
>+			break;
> 		case 0x2f: map_key_clear(KEY_MICMUTE);		break;
>+		case 0x50:
>+			map_key_clear(KEY_SPEED_DIAL);
>+			break;
>+		case 0x9e:
>+			map_led(LED_RINGER);
>+			break;

As previous comment on duplicate functionality of LED_RINGER and LED_RING.
Usage 0x000b009e could be mapped to LED_RING in a vendor driver.

> 		case 0xb0: map_key_clear(KEY_NUMERIC_0);	break;
> 		case 0xb1: map_key_clear(KEY_NUMERIC_1);	break;
> 		case 0xb2: map_key_clear(KEY_NUMERIC_2);	break;
>diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
>index 766bf26..cf692df 100644
>--- a/drivers/input/input-leds.c
>+++ b/drivers/input/input-leds.c
>@@ -36,6 +36,11 @@ static const struct {
> 	[LED_MISC]	= { "misc" },
> 	[LED_MAIL]	= { "mail" },
> 	[LED_CHARGING]	= { "charging" },
>+	[LED_OFF_HOOK]	= { "off-hook" },
>+	[LED_RING]	= { "ring" },
>+	[LED_HOLD]	= { "hold" },
>+	[LED_MICROPHONE] = { "microphone" },

As previous comments on duplicate functionality of LED_MICROPHONE and LED_MUTE.
Is LED_RINGER missing here?

>+	[LED_ONLINE]	= { "online" },
> };
>
> struct input_led {
>diff --git a/include/linux/hid.h b/include/linux/hid.h
>index 75b66ec..0c91c89 100644
>--- a/include/linux/hid.h
>+++ b/include/linux/hid.h
>@@ -764,7 +764,13 @@ struct hid_ll_driver {
>
> /* Applications from HID Usage Tables 4/8/99 Version 1.1 */
> /* We ignore a few input applications that are not widely used */
>-#define IS_INPUT_APPLICATION(a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || (a == 0x000c0001) || ((a
>>= 0x000d0002) && (a <= 0x000d0006)))
>+#define IS_INPUT_APPLICATION(a) (				\
>+		((a >= 0x00010000) && (a <= 0x00010008)) ||	\
>+		(a == 0x00010080) ||				\
>+		(a == 0x000b0005) ||				\

This should be a range from 0x000b0001 through 0x000b0005 to include
Phone (1), Answering Machine (2), Message Controls (3), Handset (4),
as well as Headset (5) collections.

>+		(a == 0x000c0001) ||				\
>+		((a >= 0x000d0002) && (a <= 0x000d0006)))
>+
>
> /* HID core API */
>
>diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
>index ed84c07..e342d3f 100644
>--- a/include/linux/mod_devicetable.h
>+++ b/include/linux/mod_devicetable.h
>@@ -288,7 +288,7 @@ struct pcmcia_device_id {
> #define INPUT_DEVICE_ID_REL_MAX		0x0f
> #define INPUT_DEVICE_ID_ABS_MAX		0x3f
> #define INPUT_DEVICE_ID_MSC_MAX		0x07
>-#define INPUT_DEVICE_ID_LED_MAX		0x0f
>+#define INPUT_DEVICE_ID_LED_MAX		0x11
> #define INPUT_DEVICE_ID_SND_MAX		0x07
> #define INPUT_DEVICE_ID_FF_MAX		0x7f
> #define INPUT_DEVICE_ID_SW_MAX		0x0f
>diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
>index d6d071f..eaa368b 100644
>--- a/include/uapi/linux/input-event-codes.h
>+++ b/include/uapi/linux/input-event-codes.h
>@@ -593,6 +593,15 @@
>
> #define KEY_ALS_TOGGLE		0x230	/* Ambient light sensor */
>
>+#define KEY_HOOK_SWITCH	0x231
>+#define KEY_FLASH		0x232
>+#define KEY_HOLD		0x233
>+#define KEY_REDIAL		0x234
>+#define KEY_PROGRAMMABLE	0x235
>+#define KEY_LINE		0x236
>+#define KEY_SPEAKER_PHONE	0x237
>+#define KEY_SPEED_DIAL		0x238
>+
> #define KEY_BUTTONCONFIG		0x240	/* AL Button Configuration */
> #define KEY_TASKMANAGER		0x241	/* AL Task/Project Manager */
> #define KEY_JOURNAL		0x242	/* AL Log/Journal/Timecard */
>@@ -812,7 +821,14 @@
> #define LED_MISC		0x08
> #define LED_MAIL		0x09
> #define LED_CHARGING		0x0a
>-#define LED_MAX			0x0f
>+#define LED_OFF_HOOK		0x0b
>+#define LED_RING		0x0c
>+#define LED_RINGER		0x0d

As previous comments on duplicate functionality of LED_RINGER and LED_RING.

>+#define LED_HOLD		0x0e
>+#define LED_MICROPHONE		0x0f

As previous comments on duplicate functionality of LED_MICROPHONE and LED_MUTE.

>+#define LED_SPEAKER		0x10
>+#define LED_ONLINE		0x11
>+#define LED_MAX		0x11
> #define LED_CNT			(LED_MAX+1)
>
> /*
>--
>2.7.4
>

Thanks,

Terry Junge
Plantronics

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

nolsen@jabra.com Aug. 18, 2016, 9 a.m. UTC | #3

First of all thank you for review and good feedback!

>Good to get telephony support into the kernel.
>
>My only concerns are the duplicate functionalities of LED_RING::LED_RINGER and LED_MUTE::LED_MICROPHONE.

By adding RINGER and MICROPHONE we are trying to achieve finer grained
telephony control from user space. We understand your concern about the
user having to handle additional values that may be hard to distinguish.

RINGER is a functionality that has a different meaning than RING in the HUT
spec. So, if a user space application really wants RINGER (or needs both)
then it should have the option. It is more flexible to make the choice
available in user space, than it is to require vendor specific drivers. Also,
having only RING would preclude applications from using both RING and
RINGER against devices that support both.

As you point out, RINGER is not an LED usage, so changing the name will
help decrease confusion. But the telephony related usages do not fit well
with the current event types EV_KEY, EV_LED etc. The problem is that we are
trying to extend the existing event types. Maybe we need a new EV_TEL event
type, and a new TEL_ prefix for telephony related values: TEL_RINGER,
TEL_REDIAL etc.?

>
>Please see below.
>

Further comments interleaved below.

>>-----Original Message-----
>>From: linux-input-owner@vger.kernel.org
>>[mailto:linux-input-owner@vger.kernel.org] On Behalf Of
>>nolsen@jabra.com
>>Sent: Wednesday, August 03, 2016 1:52 AM
>>To: linux-input@vger.kernel.org
>>Cc: jikos@kernel.org; benjamin.tissoires@redhat.com;
>>dmitry.torokhov@gmail.com; Niels Skou Olsen
>>Subject: [PATCH] HID: Support telephony devices
>>
>>From: Niels Skou Olsen <nolsen@jabra.com>
>>
>>Add support for telephony device buttons and LEDs as described in the
>>USB HID Usage Tables specification.
>>
>>This allows user mode applications convenient access to telephony
>>devices such as headsets, speaker phones and desk phones.
>>
>>Signed-off-by: Niels Skou Olsen <nolsen@jabra.com>
>>---
>> drivers/hid/hid-debug.c                | 39 ++++++++++++++++++++-
>> drivers/hid/hid-input.c                | 63 +++++++++++++++++++++++++++++++---
>> drivers/input/input-leds.c             |  5 +++
>> include/linux/hid.h                    |  8 ++++-
>> include/linux/mod_devicetable.h        |  2 +-
>> include/uapi/linux/input-event-codes.h | 18 +++++++++-
>> 6 files changed, 126 insertions(+), 9 deletions(-)
>>
>>diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c index
>>acfb522..9f109c3 100644
>>--- a/drivers/hid/hid-debug.c
>>+++ b/drivers/hid/hid-debug.c
>>@@ -129,9 +129,42 @@ static const struct hid_usage_entry hid_usage_table[] = {
>>       {0, 0x03, "ScrollLock"},
>>       {0, 0x04, "Compose"},
>>       {0, 0x05, "Kana"},
>>+{0, 0x09, "Mute"},
>>+{0, 0x17, "OffHook"},
>>+{0, 0x18, "Ring"},
>>+{0, 0x19, "MessageWaiting"},
>>+{0, 0x20, "Hold"},
>>+{0, 0x21, "Microphone"},
>>+{0, 0x27, "Standby"},
 >>+{0, 0x2a, "Online"},
>>+{0, 0x4c, "SystemSuspend"},
>>+{0, 0x4d, "ExtPowerConnected"},
>>       {0, 0x4b, "GenericIndicator"},
>>   {  9, 0, "Button" },
>>   { 10, 0, "Ordinal" },
>>+{ 11, 0, "Telephony" },
>>+{0, 0x20, "KeyHookSwitch"},
>>+{0, 0x21, "KeyFlash"},
>>+{0, 0x23, "KeyHold"},
>>+{0, 0x24, "KeyRedial"},
>>+{0, 0x2f, "KeyMicMute"},
>>+{0, 0x9e, "LedRinger"},
>
>This is not an LED usage, it's a Locally Generated Tone usage, maybe "RingerTone"?

Yes, you are right. A V2 patch is forthcoming, once we agree how to proceed.

>>+{0, 0xb0, "KeyNumeric0"},
>>+{0, 0xb1, "KeyNumeric1"},
>>+{0, 0xb2, "KeyNumeric2"},
>>+{0, 0xb3, "KeyNumeric3"},
>>+{0, 0xb4, "KeyNumeric4"},
>>+{0, 0xb5, "KeyNumeric5"},
>>+{0, 0xb6, "KeyNumeric6"},
>>+{0, 0xb7, "KeyNumeric7"},
>>+{0, 0xb8, "KeyNumeric8"},
>>+{0, 0xb9, "KeyNumeric9"},
>>+{0, 0xba, "KeyNumericStar"},
>>+{0, 0xbb, "KeyNumericPound"},
>>+{0, 0xbc, "KeyNumericA"},
>>+{0, 0xbd, "KeyNumericB"},
>>+{0, 0xbe, "KeyNumericC"},
>>+{0, 0xbf, "KeyNumericD"},
>>   { 12, 0, "Consumer" },
>>       {0, 0x238, "HorizontalWheel"},
>>   { 13, 0, "Digitizers" },
>>@@ -998,7 +1031,11 @@ static const char *leds[LED_MAX + 1] = {
>> [LED_SCROLLL] = "ScrollLock",[LED_COMPOSE] = "Compose",
>> [LED_KANA] = "Kana",[LED_SLEEP] = "Sleep",
>> [LED_SUSPEND] = "Suspend",[LED_MUTE] = "Mute",
>>-[LED_MISC] = "Misc",
>>+[LED_MISC] = "Misc",[LED_MAIL] = "Mail",
>>+[LED_CHARGING] = "Charging",[LED_OFF_HOOK] = "Off-hook",
>>+[LED_RING] = "Ring",[LED_RINGER] = "Ringer",
>
>LED_RINGER duplicates the functionality of LED_RING to the user.
>Multiple vendor products in the field already respond to the proposed mapping of LED_RING by indicating inbound ring to the user.
>Adding LED_RINGER to the core and mapping it to 0x000b009e will require the user to decide which one to use, LED_RING or LED_RINGER.
>If needed, the 0x000b009e usage should be mapped by a vendor driver to LED_RING so the user will only have to deal with LED_RING.
>

See reply at the top.

>>+[LED_HOLD] = "Hold",[LED_MICROPHONE] = "Microphone",
>
>LED_MICROPHONE duplicates the functionality of LED_MUTE to the user.
>Multiple vendor products in the field already respond to the core mapping of LED_MUTE by muting the microphone.
>Adding LED_MICROPHONE to the core and mapping it to 0x00080021 will require the user to decide which one to use, LED_MUTE or LED_MICROPHONE.
>If needed, the 0x00080021 usage should be mapped by a vendor driver to LED_MUTE so the user will only have to deal with LED_MUTE.
>

See reply at the top.

>>+[LED_ONLINE] = "Online"
>> };
>>
>> static const char *repeats[REP_MAX + 1] = { diff --git
>>a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index
>>bcfaf32..b2a4136 100644
>>--- a/drivers/hid/hid-input.c
>>+++ b/drivers/hid/hid-input.c
>>@@ -509,9 +509,11 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>> if (field->report_count < 1)
>> goto ignore;
>>
>>-/* only LED usages are supported in output fields */
>>+/* only LED and TELEPHONY usages are supported in output fields */
>> if (field->report_type == HID_OUTPUT_REPORT &&
>>-(usage->hid & HID_USAGE_PAGE) != HID_UP_LED) {
>>+(usage->hid & HID_USAGE_PAGE) != HID_UP_LED &&
>>+(usage->hid & HID_USAGE_PAGE) != HID_UP_TELEPHONY) {
>>+
>> goto ignore;
>> }
>
>This allowance for telephony page output mapping in order to map LED_RINGER would not be needed if the 0x000b009e mapping were done by a vendor driver and the vendor driver was allowed access before the currently preemptive LED page test, like this:
>
>if (device->driver->input_mapping) {
>int ret = device->driver->input_mapping(device, hidinput, field,
>usage, &bit, &max);
>if (ret > 0)
>goto mapped;
>if (ret < 0)
>goto ignore;
>}
>
>/* only LED usages are supported in output fields */
>if (field->report_type == HID_OUTPUT_REPORT &&
>(usage->hid & HID_USAGE_PAGE) != HID_UP_LED) {
>goto ignore;
>}
>

See reply at the top.

>>
>>@@ -658,11 +660,32 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>> 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 0x17: /* "Off hook" */
>>+map_led(LED_OFF_HOOK);
>>+break;
>>+case 0x18: /* "Ring" */
>>+map_led(LED_RING);
>>+break;
>> case 0x19:  map_led (LED_MAIL);     break;    /*   "Message Waiting"          */
>>+case 0x20: /* "Hold" */
>>+map_led(LED_HOLD);
>>+break;
>>+case 0x21: /* "Microphone" */
>>+map_led(LED_MICROPHONE);
>>+break;
>
>As previous comment on duplicate functionality of LED_MICROPHONE and LED_MUTE.
>Usage 0x00080021 could be mapped to LED_MUTE in a vendor driver.

See reply at the top.

>
>>+case 0x27: /* "Stand-By" */
>>+map_led(LED_SLEEP);
>>+break;
>>+case 0x2a: /* "Online" */
>>+map_led(LED_ONLINE);
>>+break;
>>+case 0x4b: /* "Generic Indicator" */
>>+map_led(LED_MISC);
>>+break;
>>+case 0x4c: /* "System Suspend" */
>>+map_led(LED_SUSPEND);
>>+break;
>> case 0x4d:  map_led (LED_CHARGING); break;    /*   "External Power Connected" */
>>
>> default: goto ignore;
>>@@ -732,7 +755,37 @@ static void hidinput_configure_usage(struct
>>hid_input *hidinput, struct hid_fiel
>>
>> case HID_UP_TELEPHONY:
>> switch (usage->hid & HID_USAGE) {
>>+case 0x07:
>>+map_key_clear(KEY_PROGRAMMABLE);
>>+break;
>
>----
>>+case 0x1e:
>>+map_led(LED_SPEAKER);
>>+break;
>----
>
>This is in the wrong place, it should be in the HID_UP_LED case switch statement block.
>Speaker usage 0x1e is in the LED page not the telephony page.
>

Yes, you are right. A V2 patch is forthcoming, once we agree how to proceed.

>>+case 0x20:
>>+map_key_clear(KEY_HOOK_SWITCH);
>>+break;
>>+case 0x21:
>>+map_key_clear(KEY_FLASH);
>>+break;
>>+case 0x23:
>>+map_key_clear(KEY_HOLD);
>>+break;
>>+case 0x24:
>>+map_key_clear(KEY_REDIAL);
>>+break;
>>+case 0x2a:
>>+map_key_clear(KEY_LINE);
>>+break;
>>+case 0x2b:
>>+map_key_clear(KEY_SPEAKER_PHONE);
>>+break;
>> case 0x2f: map_key_clear(KEY_MICMUTE);break;
>>+case 0x50:
>>+map_key_clear(KEY_SPEED_DIAL);
>>+break;
>>+case 0x9e:
>>+map_led(LED_RINGER);
>>+break;
>
>As previous comment on duplicate functionality of LED_RINGER and LED_RING.
>Usage 0x000b009e could be mapped to LED_RING in a vendor driver.

See reply at the top.

>
>> case 0xb0: map_key_clear(KEY_NUMERIC_0);break;
>> case 0xb1: map_key_clear(KEY_NUMERIC_1);break;
>> case 0xb2: map_key_clear(KEY_NUMERIC_2);break;
>>diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
>>index 766bf26..cf692df 100644
>>--- a/drivers/input/input-leds.c
>>+++ b/drivers/input/input-leds.c
>>@@ -36,6 +36,11 @@ static const struct {
>> [LED_MISC]= { "misc" },
>> [LED_MAIL]= { "mail" },
>> [LED_CHARGING]= { "charging" },
>>+[LED_OFF_HOOK]= { "off-hook" },
>>+[LED_RING]= { "ring" },
>>+[LED_HOLD]= { "hold" },
>>+[LED_MICROPHONE] = { "microphone" },
>
>As previous comments on duplicate functionality of LED_MICROPHONE and LED_MUTE.
>Is LED_RINGER missing here?

If we change LED_RINGER to something else (maybe TEL_RINGER), then it doesn't belong here.

>
>>+[LED_ONLINE]= { "online" },
>> };
>>
>> struct input_led {
>>diff --git a/include/linux/hid.h b/include/linux/hid.h index
>>75b66ec..0c91c89 100644
>>--- a/include/linux/hid.h
>>+++ b/include/linux/hid.h
>>@@ -764,7 +764,13 @@ struct hid_ll_driver {
>>
>> /* Applications from HID Usage Tables 4/8/99 Version 1.1 */
>> /* We ignore a few input applications that are not widely used */
>>-#define IS_INPUT_APPLICATION(a) (((a >= 0x00010000) && (a <=
>>0x00010008)) || (a == 0x00010080) || (a == 0x000c0001) || ((a
>>>= 0x000d0002) && (a <= 0x000d0006)))
>>+#define IS_INPUT_APPLICATION(a) (\
>>+((a >= 0x00010000) && (a <= 0x00010008)) ||\
>>+(a == 0x00010080) ||\
>>+(a == 0x000b0005) ||\
>
>This should be a range from 0x000b0001 through 0x000b0005 to include Phone (1), Answering Machine (2), Message Controls (3), Handset (4), as well as Headset (5) collections.

Agreed. Coming in V2.

>
>>+(a == 0x000c0001) ||\
>>+((a >= 0x000d0002) && (a <= 0x000d0006)))
>>+
>>
>> /* HID core API */
>>
>>diff --git a/include/linux/mod_devicetable.h
>>b/include/linux/mod_devicetable.h index ed84c07..e342d3f 100644
>>--- a/include/linux/mod_devicetable.h
>>+++ b/include/linux/mod_devicetable.h
>>@@ -288,7 +288,7 @@ struct pcmcia_device_id {
>> #define INPUT_DEVICE_ID_REL_MAX0x0f
>> #define INPUT_DEVICE_ID_ABS_MAX0x3f
>> #define INPUT_DEVICE_ID_MSC_MAX0x07
>>-#define INPUT_DEVICE_ID_LED_MAX0x0f
>>+#define INPUT_DEVICE_ID_LED_MAX0x11
>> #define INPUT_DEVICE_ID_SND_MAX0x07
>> #define INPUT_DEVICE_ID_FF_MAX0x7f
>> #define INPUT_DEVICE_ID_SW_MAX0x0f
>>diff --git a/include/uapi/linux/input-event-codes.h
>>b/include/uapi/linux/input-event-codes.h
>>index d6d071f..eaa368b 100644
>>--- a/include/uapi/linux/input-event-codes.h
>>+++ b/include/uapi/linux/input-event-codes.h
>>@@ -593,6 +593,15 @@
>>
>> #define KEY_ALS_TOGGLE0x230/* Ambient light sensor */
>>
>>+#define KEY_HOOK_SWITCH0x231
>>+#define KEY_FLASH0x232
>>+#define KEY_HOLD0x233
>>+#define KEY_REDIAL0x234
>>+#define KEY_PROGRAMMABLE0x235
>>+#define KEY_LINE0x236
>>+#define KEY_SPEAKER_PHONE0x237
>>+#define KEY_SPEED_DIAL0x238
>>+
>> #define KEY_BUTTONCONFIG0x240/* AL Button Configuration */
>> #define KEY_TASKMANAGER0x241/* AL Task/Project Manager */
>> #define KEY_JOURNAL0x242/* AL Log/Journal/Timecard */
>>@@ -812,7 +821,14 @@
>> #define LED_MISC0x08
>> #define LED_MAIL0x09
>> #define LED_CHARGING0x0a
>>-#define LED_MAX0x0f
>>+#define LED_OFF_HOOK0x0b
>>+#define LED_RING0x0c
>>+#define LED_RINGER0x0d
>
>As previous comments on duplicate functionality of LED_RINGER and LED_RING.

See reply at the top.

>
>>+#define LED_HOLD0x0e
>>+#define LED_MICROPHONE0x0f
>
>As previous comments on duplicate functionality of LED_MICROPHONE and LED_MUTE.

See reply at the top.

>
>>+#define LED_SPEAKER0x10
>>+#define LED_ONLINE0x11
>>+#define LED_MAX0x11
>> #define LED_CNT(LED_MAX+1)
>>
>> /*
>>--
>>2.7.4
>>
>
>Thanks,
>
>Terry Junge
>Plantronics

Thanks,

Niels Skou Olsen
Jabra

**** GN GROUP NOTICE - AUTOMATICALLY INSERTED**** The information in this e-mail (including attachments, if any) is considered confidential and is intended only for the recipient(s) listed above. Any review, use, disclosure, distribution or copying of this e-mail is prohibited except by or on behalf of the intended recipient. If you have received this email in error, please notify me immediately by reply e-mail, delete this e-mail, and do not disclose its contents to anyone. Any opinions expressed in this e-mail are those of the individual and not necessarily the GN group. Thank you. ******************** DISCLAIMER END ************************
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Junge, Terry Sept. 15, 2016, 12:01 a.m. UTC | #4

Niels,

Sorry, just got back from holiday and am working my way through email.

Is there a V2 for this? Maybe I missed it?

If the maintainers have no issue with the duplicate functionalities then I have no further issues and am anxious to see V2 get applied.

Regards,
Terry

>-----Original Message-----
>From: Niels Skou Olsen [mailto:nolsen@jabra.com]
>Sent: Thursday, August 18, 2016 2:00 AM
>To: Junge, Terry; linux-input@vger.kernel.org
>Cc: jikos@kernel.org; benjamin.tissoires@redhat.com; dmitry.torokhov@gmail.com
>Subject: RE: [PATCH] HID: Support telephony devices
>
>First of all thank you for review and good feedback!
>
>>Good to get telephony support into the kernel.
>>
>>My only concerns are the duplicate functionalities of LED_RING::LED_RINGER and LED_MUTE::LED_MICROPHONE.
>
>By adding RINGER and MICROPHONE we are trying to achieve finer grained
>telephony control from user space. We understand your concern about the
>user having to handle additional values that may be hard to distinguish.
>
>RINGER is a functionality that has a different meaning than RING in the HUT
>spec. So, if a user space application really wants RINGER (or needs both)
>then it should have the option. It is more flexible to make the choice
>available in user space, than it is to require vendor specific drivers. Also,
>having only RING would preclude applications from using both RING and
>RINGER against devices that support both.
>
>As you point out, RINGER is not an LED usage, so changing the name will
>help decrease confusion. But the telephony related usages do not fit well
>with the current event types EV_KEY, EV_LED etc. The problem is that we are
>trying to extend the existing event types. Maybe we need a new EV_TEL event
>type, and a new TEL_ prefix for telephony related values: TEL_RINGER,
>TEL_REDIAL etc.?
>
>>
>>Please see below.
>>
>
>Further comments interleaved below.
>
>>>-----Original Message-----
>>>From: linux-input-owner@vger.kernel.org
>>>[mailto:linux-input-owner@vger.kernel.org] On Behalf Of
>>>nolsen@jabra.com
>>>Sent: Wednesday, August 03, 2016 1:52 AM
>>>To: linux-input@vger.kernel.org
>>>Cc: jikos@kernel.org; benjamin.tissoires@redhat.com;
>>>dmitry.torokhov@gmail.com; Niels Skou Olsen
>>>Subject: [PATCH] HID: Support telephony devices
>>>
>>>From: Niels Skou Olsen <nolsen@jabra.com>
>>>
>>>Add support for telephony device buttons and LEDs as described in the
>>>USB HID Usage Tables specification.
>>>
>>>This allows user mode applications convenient access to telephony
>>>devices such as headsets, speaker phones and desk phones.
>>>
>>>Signed-off-by: Niels Skou Olsen <nolsen@jabra.com>
>>>---
>>> drivers/hid/hid-debug.c                | 39 ++++++++++++++++++++-
>>> drivers/hid/hid-input.c                | 63 +++++++++++++++++++++++++++++++---
>>> drivers/input/input-leds.c             |  5 +++
>>> include/linux/hid.h                    |  8 ++++-
>>> include/linux/mod_devicetable.h        |  2 +-
>>> include/uapi/linux/input-event-codes.h | 18 +++++++++-
>>> 6 files changed, 126 insertions(+), 9 deletions(-)
>>>
>>>diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c index
>>>acfb522..9f109c3 100644
>>>--- a/drivers/hid/hid-debug.c
>>>+++ b/drivers/hid/hid-debug.c
>>>@@ -129,9 +129,42 @@ static const struct hid_usage_entry hid_usage_table[] = {
>>>       {0, 0x03, "ScrollLock"},
>>>       {0, 0x04, "Compose"},
>>>       {0, 0x05, "Kana"},
>>>+{0, 0x09, "Mute"},
>>>+{0, 0x17, "OffHook"},
>>>+{0, 0x18, "Ring"},
>>>+{0, 0x19, "MessageWaiting"},
>>>+{0, 0x20, "Hold"},
>>>+{0, 0x21, "Microphone"},
>>>+{0, 0x27, "Standby"},
> >>+{0, 0x2a, "Online"},
>>>+{0, 0x4c, "SystemSuspend"},
>>>+{0, 0x4d, "ExtPowerConnected"},
>>>       {0, 0x4b, "GenericIndicator"},
>>>   {  9, 0, "Button" },
>>>   { 10, 0, "Ordinal" },
>>>+{ 11, 0, "Telephony" },
>>>+{0, 0x20, "KeyHookSwitch"},
>>>+{0, 0x21, "KeyFlash"},
>>>+{0, 0x23, "KeyHold"},
>>>+{0, 0x24, "KeyRedial"},
>>>+{0, 0x2f, "KeyMicMute"},
>>>+{0, 0x9e, "LedRinger"},
>>
>>This is not an LED usage, it's a Locally Generated Tone usage, maybe "RingerTone"?
>
>Yes, you are right. A V2 patch is forthcoming, once we agree how to proceed.
>
>>>+{0, 0xb0, "KeyNumeric0"},
>>>+{0, 0xb1, "KeyNumeric1"},
>>>+{0, 0xb2, "KeyNumeric2"},
>>>+{0, 0xb3, "KeyNumeric3"},
>>>+{0, 0xb4, "KeyNumeric4"},
>>>+{0, 0xb5, "KeyNumeric5"},
>>>+{0, 0xb6, "KeyNumeric6"},
>>>+{0, 0xb7, "KeyNumeric7"},
>>>+{0, 0xb8, "KeyNumeric8"},
>>>+{0, 0xb9, "KeyNumeric9"},
>>>+{0, 0xba, "KeyNumericStar"},
>>>+{0, 0xbb, "KeyNumericPound"},
>>>+{0, 0xbc, "KeyNumericA"},
>>>+{0, 0xbd, "KeyNumericB"},
>>>+{0, 0xbe, "KeyNumericC"},
>>>+{0, 0xbf, "KeyNumericD"},
>>>   { 12, 0, "Consumer" },
>>>       {0, 0x238, "HorizontalWheel"},
>>>   { 13, 0, "Digitizers" },
>>>@@ -998,7 +1031,11 @@ static const char *leds[LED_MAX + 1] = {
>>> [LED_SCROLLL] = "ScrollLock",[LED_COMPOSE] = "Compose",
>>> [LED_KANA] = "Kana",[LED_SLEEP] = "Sleep",
>>> [LED_SUSPEND] = "Suspend",[LED_MUTE] = "Mute",
>>>-[LED_MISC] = "Misc",
>>>+[LED_MISC] = "Misc",[LED_MAIL] = "Mail",
>>>+[LED_CHARGING] = "Charging",[LED_OFF_HOOK] = "Off-hook",
>>>+[LED_RING] = "Ring",[LED_RINGER] = "Ringer",
>>
>>LED_RINGER duplicates the functionality of LED_RING to the user.
>>Multiple vendor products in the field already respond to the proposed mapping of LED_RING by indicating inbound ring to the user.
>>Adding LED_RINGER to the core and mapping it to 0x000b009e will require the user to decide which one to use, LED_RING or
>LED_RINGER.
>>If needed, the 0x000b009e usage should be mapped by a vendor driver to LED_RING so the user will only have to deal with
>LED_RING.
>>
>
>See reply at the top.
>
>>>+[LED_HOLD] = "Hold",[LED_MICROPHONE] = "Microphone",
>>
>>LED_MICROPHONE duplicates the functionality of LED_MUTE to the user.
>>Multiple vendor products in the field already respond to the core mapping of LED_MUTE by muting the microphone.
>>Adding LED_MICROPHONE to the core and mapping it to 0x00080021 will require the user to decide which one to use, LED_MUTE
>or LED_MICROPHONE.
>>If needed, the 0x00080021 usage should be mapped by a vendor driver to LED_MUTE so the user will only have to deal with
>LED_MUTE.
>>
>
>See reply at the top.
>
>>>+[LED_ONLINE] = "Online"
>>> };
>>>
>>> static const char *repeats[REP_MAX + 1] = { diff --git
>>>a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index
>>>bcfaf32..b2a4136 100644
>>>--- a/drivers/hid/hid-input.c
>>>+++ b/drivers/hid/hid-input.c
>>>@@ -509,9 +509,11 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>>> if (field->report_count < 1)
>>> goto ignore;
>>>
>>>-/* only LED usages are supported in output fields */
>>>+/* only LED and TELEPHONY usages are supported in output fields */
>>> if (field->report_type == HID_OUTPUT_REPORT &&
>>>-(usage->hid & HID_USAGE_PAGE) != HID_UP_LED) {
>>>+(usage->hid & HID_USAGE_PAGE) != HID_UP_LED &&
>>>+(usage->hid & HID_USAGE_PAGE) != HID_UP_TELEPHONY) {
>>>+
>>> goto ignore;
>>> }
>>
>>This allowance for telephony page output mapping in order to map LED_RINGER would not be needed if the 0x000b009e mapping
>were done by a vendor driver and the vendor driver was allowed access before the currently preemptive LED page test, like this:
>>
>>if (device->driver->input_mapping) {
>>int ret = device->driver->input_mapping(device, hidinput, field,
>>usage, &bit, &max);
>>if (ret > 0)
>>goto mapped;
>>if (ret < 0)
>>goto ignore;
>>}
>>
>>/* only LED usages are supported in output fields */
>>if (field->report_type == HID_OUTPUT_REPORT &&
>>(usage->hid & HID_USAGE_PAGE) != HID_UP_LED) {
>>goto ignore;
>>}
>>
>
>See reply at the top.
>
>>>
>>>@@ -658,11 +660,32 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>>> 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 0x17: /* "Off hook" */
>>>+map_led(LED_OFF_HOOK);
>>>+break;
>>>+case 0x18: /* "Ring" */
>>>+map_led(LED_RING);
>>>+break;
>>> case 0x19:  map_led (LED_MAIL);     break;    /*   "Message Waiting"          */
>>>+case 0x20: /* "Hold" */
>>>+map_led(LED_HOLD);
>>>+break;
>>>+case 0x21: /* "Microphone" */
>>>+map_led(LED_MICROPHONE);
>>>+break;
>>
>>As previous comment on duplicate functionality of LED_MICROPHONE and LED_MUTE.
>>Usage 0x00080021 could be mapped to LED_MUTE in a vendor driver.
>
>See reply at the top.
>
>>
>>>+case 0x27: /* "Stand-By" */
>>>+map_led(LED_SLEEP);
>>>+break;
>>>+case 0x2a: /* "Online" */
>>>+map_led(LED_ONLINE);
>>>+break;
>>>+case 0x4b: /* "Generic Indicator" */
>>>+map_led(LED_MISC);
>>>+break;
>>>+case 0x4c: /* "System Suspend" */
>>>+map_led(LED_SUSPEND);
>>>+break;
>>> case 0x4d:  map_led (LED_CHARGING); break;    /*   "External Power Connected" */
>>>
>>> default: goto ignore;
>>>@@ -732,7 +755,37 @@ static void hidinput_configure_usage(struct
>>>hid_input *hidinput, struct hid_fiel
>>>
>>> case HID_UP_TELEPHONY:
>>> switch (usage->hid & HID_USAGE) {
>>>+case 0x07:
>>>+map_key_clear(KEY_PROGRAMMABLE);
>>>+break;
>>
>>----
>>>+case 0x1e:
>>>+map_led(LED_SPEAKER);
>>>+break;
>>----
>>
>>This is in the wrong place, it should be in the HID_UP_LED case switch statement block.
>>Speaker usage 0x1e is in the LED page not the telephony page.
>>
>
>Yes, you are right. A V2 patch is forthcoming, once we agree how to proceed.
>
>>>+case 0x20:
>>>+map_key_clear(KEY_HOOK_SWITCH);
>>>+break;
>>>+case 0x21:
>>>+map_key_clear(KEY_FLASH);
>>>+break;
>>>+case 0x23:
>>>+map_key_clear(KEY_HOLD);
>>>+break;
>>>+case 0x24:
>>>+map_key_clear(KEY_REDIAL);
>>>+break;
>>>+case 0x2a:
>>>+map_key_clear(KEY_LINE);
>>>+break;
>>>+case 0x2b:
>>>+map_key_clear(KEY_SPEAKER_PHONE);
>>>+break;
>>> case 0x2f: map_key_clear(KEY_MICMUTE);break;
>>>+case 0x50:
>>>+map_key_clear(KEY_SPEED_DIAL);
>>>+break;
>>>+case 0x9e:
>>>+map_led(LED_RINGER);
>>>+break;
>>
>>As previous comment on duplicate functionality of LED_RINGER and LED_RING.
>>Usage 0x000b009e could be mapped to LED_RING in a vendor driver.
>
>See reply at the top.
>
>>
>>> case 0xb0: map_key_clear(KEY_NUMERIC_0);break;
>>> case 0xb1: map_key_clear(KEY_NUMERIC_1);break;
>>> case 0xb2: map_key_clear(KEY_NUMERIC_2);break;
>>>diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
>>>index 766bf26..cf692df 100644
>>>--- a/drivers/input/input-leds.c
>>>+++ b/drivers/input/input-leds.c
>>>@@ -36,6 +36,11 @@ static const struct {
>>> [LED_MISC]= { "misc" },
>>> [LED_MAIL]= { "mail" },
>>> [LED_CHARGING]= { "charging" },
>>>+[LED_OFF_HOOK]= { "off-hook" },
>>>+[LED_RING]= { "ring" },
>>>+[LED_HOLD]= { "hold" },
>>>+[LED_MICROPHONE] = { "microphone" },
>>
>>As previous comments on duplicate functionality of LED_MICROPHONE and LED_MUTE.
>>Is LED_RINGER missing here?
>
>If we change LED_RINGER to something else (maybe TEL_RINGER), then it doesn't belong here.
>
>>
>>>+[LED_ONLINE]= { "online" },
>>> };
>>>
>>> struct input_led {
>>>diff --git a/include/linux/hid.h b/include/linux/hid.h index
>>>75b66ec..0c91c89 100644
>>>--- a/include/linux/hid.h
>>>+++ b/include/linux/hid.h
>>>@@ -764,7 +764,13 @@ struct hid_ll_driver {
>>>
>>> /* Applications from HID Usage Tables 4/8/99 Version 1.1 */
>>> /* We ignore a few input applications that are not widely used */
>>>-#define IS_INPUT_APPLICATION(a) (((a >= 0x00010000) && (a <=
>>>0x00010008)) || (a == 0x00010080) || (a == 0x000c0001) || ((a
>>>>= 0x000d0002) && (a <= 0x000d0006)))
>>>+#define IS_INPUT_APPLICATION(a) (\
>>>+((a >= 0x00010000) && (a <= 0x00010008)) ||\
>>>+(a == 0x00010080) ||\
>>>+(a == 0x000b0005) ||\
>>
>>This should be a range from 0x000b0001 through 0x000b0005 to include Phone (1), Answering Machine (2), Message Controls (3),
>Handset (4), as well as Headset (5) collections.
>
>Agreed. Coming in V2.
>
>>
>>>+(a == 0x000c0001) ||\
>>>+((a >= 0x000d0002) && (a <= 0x000d0006)))
>>>+
>>>
>>> /* HID core API */
>>>
>>>diff --git a/include/linux/mod_devicetable.h
>>>b/include/linux/mod_devicetable.h index ed84c07..e342d3f 100644
>>>--- a/include/linux/mod_devicetable.h
>>>+++ b/include/linux/mod_devicetable.h
>>>@@ -288,7 +288,7 @@ struct pcmcia_device_id {
>>> #define INPUT_DEVICE_ID_REL_MAX0x0f
>>> #define INPUT_DEVICE_ID_ABS_MAX0x3f
>>> #define INPUT_DEVICE_ID_MSC_MAX0x07
>>>-#define INPUT_DEVICE_ID_LED_MAX0x0f
>>>+#define INPUT_DEVICE_ID_LED_MAX0x11
>>> #define INPUT_DEVICE_ID_SND_MAX0x07
>>> #define INPUT_DEVICE_ID_FF_MAX0x7f
>>> #define INPUT_DEVICE_ID_SW_MAX0x0f
>>>diff --git a/include/uapi/linux/input-event-codes.h
>>>b/include/uapi/linux/input-event-codes.h
>>>index d6d071f..eaa368b 100644
>>>--- a/include/uapi/linux/input-event-codes.h
>>>+++ b/include/uapi/linux/input-event-codes.h
>>>@@ -593,6 +593,15 @@
>>>
>>> #define KEY_ALS_TOGGLE0x230/* Ambient light sensor */
>>>
>>>+#define KEY_HOOK_SWITCH0x231
>>>+#define KEY_FLASH0x232
>>>+#define KEY_HOLD0x233
>>>+#define KEY_REDIAL0x234
>>>+#define KEY_PROGRAMMABLE0x235
>>>+#define KEY_LINE0x236
>>>+#define KEY_SPEAKER_PHONE0x237
>>>+#define KEY_SPEED_DIAL0x238
>>>+
>>> #define KEY_BUTTONCONFIG0x240/* AL Button Configuration */
>>> #define KEY_TASKMANAGER0x241/* AL Task/Project Manager */
>>> #define KEY_JOURNAL0x242/* AL Log/Journal/Timecard */
>>>@@ -812,7 +821,14 @@
>>> #define LED_MISC0x08
>>> #define LED_MAIL0x09
>>> #define LED_CHARGING0x0a
>>>-#define LED_MAX0x0f
>>>+#define LED_OFF_HOOK0x0b
>>>+#define LED_RING0x0c
>>>+#define LED_RINGER0x0d
>>
>>As previous comments on duplicate functionality of LED_RINGER and LED_RING.
>
>See reply at the top.
>
>>
>>>+#define LED_HOLD0x0e
>>>+#define LED_MICROPHONE0x0f
>>
>>As previous comments on duplicate functionality of LED_MICROPHONE and LED_MUTE.
>
>See reply at the top.
>
>>
>>>+#define LED_SPEAKER0x10
>>>+#define LED_ONLINE0x11
>>>+#define LED_MAX0x11
>>> #define LED_CNT(LED_MAX+1)
>>>
>>> /*
>>>--
>>>2.7.4
>>>
>>
>>Thanks,
>>
>>Terry Junge
>>Plantronics
>
>Thanks,
>
>Niels Skou Olsen
>Jabra
>
>**** GN GROUP NOTICE - AUTOMATICALLY INSERTED**** The information in this e-mail (including attachments, if any) is
>considered confidential and is intended only for the recipient(s) listed above. Any review, use, disclosure, distribution or copying of
>this e-mail is prohibited except by or on behalf of the intended recipient. If you have received this email in error, please notify me
>immediately by reply e-mail, delete this e-mail, and do not disclose its contents to anyone. Any opinions expressed in this e-mail are
>those of the individual and not necessarily the GN group. Thank you. ******************** DISCLAIMER END
>************************
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

nolsen@jabra.com Oct. 6, 2016, 6:30 a.m. UTC | #5

I am sorry for the long delay. I too have been on vacation, and also other projects have pulled me away from this. :-/

I will be posting a V2 patch during the next few hours.

Best regards,
Niels

-----Original Message-----
From: Junge, Terry [mailto:Terry.Junge@plantronics.com]
Sent: 15. september 2016 02:01
To: Niels Skou Olsen <nolsen@jabra.com>; linux-input@vger.kernel.org
Cc: jikos@kernel.org; benjamin.tissoires@redhat.com; dmitry.torokhov@gmail.com
Subject: RE: [PATCH] HID: Support telephony devices

Niels,

Sorry, just got back from holiday and am working my way through email.

Is there a V2 for this? Maybe I missed it?

If the maintainers have no issue with the duplicate functionalities then I have no further issues and am anxious to see V2 get applied.

Regards,
Terry

>-----Original Message-----
>From: Niels Skou Olsen [mailto:nolsen@jabra.com]
>Sent: Thursday, August 18, 2016 2:00 AM
>To: Junge, Terry; linux-input@vger.kernel.org
>Cc: jikos@kernel.org; benjamin.tissoires@redhat.com;
>dmitry.torokhov@gmail.com
>Subject: RE: [PATCH] HID: Support telephony devices
>
>First of all thank you for review and good feedback!
>
>>Good to get telephony support into the kernel.
>>
>>My only concerns are the duplicate functionalities of LED_RING::LED_RINGER and LED_MUTE::LED_MICROPHONE.
>
>By adding RINGER and MICROPHONE we are trying to achieve finer grained
>telephony control from user space. We understand your concern about the
>user having to handle additional values that may be hard to distinguish.
>
>RINGER is a functionality that has a different meaning than RING in the
>HUT spec. So, if a user space application really wants RINGER (or needs
>both) then it should have the option. It is more flexible to make the
>choice available in user space, than it is to require vendor specific
>drivers. Also, having only RING would preclude applications from using
>both RING and RINGER against devices that support both.
>
>As you point out, RINGER is not an LED usage, so changing the name will
>help decrease confusion. But the telephony related usages do not fit
>well with the current event types EV_KEY, EV_LED etc. The problem is
>that we are trying to extend the existing event types. Maybe we need a
>new EV_TEL event type, and a new TEL_ prefix for telephony related
>values: TEL_RINGER, TEL_REDIAL etc.?
>
>>
>>Please see below.
>>
>
>Further comments interleaved below.
>
>>>-----Original Message-----
>>>From: linux-input-owner@vger.kernel.org
>>>[mailto:linux-input-owner@vger.kernel.org] On Behalf Of
>>>nolsen@jabra.com
>>>Sent: Wednesday, August 03, 2016 1:52 AM
>>>To: linux-input@vger.kernel.org
>>>Cc: jikos@kernel.org; benjamin.tissoires@redhat.com;
>>>dmitry.torokhov@gmail.com; Niels Skou Olsen
>>>Subject: [PATCH] HID: Support telephony devices
>>>
>>>From: Niels Skou Olsen <nolsen@jabra.com>
>>>
>>>Add support for telephony device buttons and LEDs as described in the
>>>USB HID Usage Tables specification.
>>>
>>>This allows user mode applications convenient access to telephony
>>>devices such as headsets, speaker phones and desk phones.
>>>
>>>Signed-off-by: Niels Skou Olsen <nolsen@jabra.com>
>>>---
>>> drivers/hid/hid-debug.c                | 39 ++++++++++++++++++++-
>>> drivers/hid/hid-input.c                | 63 +++++++++++++++++++++++++++++++---
>>> drivers/input/input-leds.c             |  5 +++
>>> include/linux/hid.h                    |  8 ++++-
>>> include/linux/mod_devicetable.h        |  2 +-
>>> include/uapi/linux/input-event-codes.h | 18 +++++++++-
>>> 6 files changed, 126 insertions(+), 9 deletions(-)
>>>
>>>diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c index
>>>acfb522..9f109c3 100644
>>>--- a/drivers/hid/hid-debug.c
>>>+++ b/drivers/hid/hid-debug.c
>>>@@ -129,9 +129,42 @@ static const struct hid_usage_entry hid_usage_table[] = {
>>>       {0, 0x03, "ScrollLock"},
>>>       {0, 0x04, "Compose"},
>>>       {0, 0x05, "Kana"},
>>>+{0, 0x09, "Mute"},
>>>+{0, 0x17, "OffHook"},
>>>+{0, 0x18, "Ring"},
>>>+{0, 0x19, "MessageWaiting"},
>>>+{0, 0x20, "Hold"},
>>>+{0, 0x21, "Microphone"},
>>>+{0, 0x27, "Standby"},
> >>+{0, 0x2a, "Online"},
>>>+{0, 0x4c, "SystemSuspend"},
>>>+{0, 0x4d, "ExtPowerConnected"},
>>>       {0, 0x4b, "GenericIndicator"},
>>>   {  9, 0, "Button" },
>>>   { 10, 0, "Ordinal" },
>>>+{ 11, 0, "Telephony" },
>>>+{0, 0x20, "KeyHookSwitch"},
>>>+{0, 0x21, "KeyFlash"},
>>>+{0, 0x23, "KeyHold"},
>>>+{0, 0x24, "KeyRedial"},
>>>+{0, 0x2f, "KeyMicMute"},
>>>+{0, 0x9e, "LedRinger"},
>>
>>This is not an LED usage, it's a Locally Generated Tone usage, maybe "RingerTone"?
>
>Yes, you are right. A V2 patch is forthcoming, once we agree how to proceed.
>
>>>+{0, 0xb0, "KeyNumeric0"},
>>>+{0, 0xb1, "KeyNumeric1"},
>>>+{0, 0xb2, "KeyNumeric2"},
>>>+{0, 0xb3, "KeyNumeric3"},
>>>+{0, 0xb4, "KeyNumeric4"},
>>>+{0, 0xb5, "KeyNumeric5"},
>>>+{0, 0xb6, "KeyNumeric6"},
>>>+{0, 0xb7, "KeyNumeric7"},
>>>+{0, 0xb8, "KeyNumeric8"},
>>>+{0, 0xb9, "KeyNumeric9"},
>>>+{0, 0xba, "KeyNumericStar"},
>>>+{0, 0xbb, "KeyNumericPound"},
>>>+{0, 0xbc, "KeyNumericA"},
>>>+{0, 0xbd, "KeyNumericB"},
>>>+{0, 0xbe, "KeyNumericC"},
>>>+{0, 0xbf, "KeyNumericD"},
>>>   { 12, 0, "Consumer" },
>>>       {0, 0x238, "HorizontalWheel"},
>>>   { 13, 0, "Digitizers" },
>>>@@ -998,7 +1031,11 @@ static const char *leds[LED_MAX + 1] = {
>>>[LED_SCROLLL] = "ScrollLock",[LED_COMPOSE] = "Compose",  [LED_KANA] =
>>>"Kana",[LED_SLEEP] = "Sleep",  [LED_SUSPEND] = "Suspend",[LED_MUTE] =
>>>"Mute", -[LED_MISC] = "Misc",
>>>+[LED_MISC] = "Misc",[LED_MAIL] = "Mail", [LED_CHARGING] =
>>>+"Charging",[LED_OFF_HOOK] = "Off-hook", [LED_RING] =
>>>+"Ring",[LED_RINGER] = "Ringer",
>>
>>LED_RINGER duplicates the functionality of LED_RING to the user.
>>Multiple vendor products in the field already respond to the proposed mapping of LED_RING by indicating inbound ring to the user.
>>Adding LED_RINGER to the core and mapping it to 0x000b009e will
>>require the user to decide which one to use, LED_RING or
>LED_RINGER.
>>If needed, the 0x000b009e usage should be mapped by a vendor driver to
>>LED_RING so the user will only have to deal with
>LED_RING.
>>
>
>See reply at the top.
>
>>>+[LED_HOLD] = "Hold",[LED_MICROPHONE] = "Microphone",
>>
>>LED_MICROPHONE duplicates the functionality of LED_MUTE to the user.
>>Multiple vendor products in the field already respond to the core mapping of LED_MUTE by muting the microphone.
>>Adding LED_MICROPHONE to the core and mapping it to 0x00080021 will
>>require the user to decide which one to use, LED_MUTE
>or LED_MICROPHONE.
>>If needed, the 0x00080021 usage should be mapped by a vendor driver to
>>LED_MUTE so the user will only have to deal with
>LED_MUTE.
>>
>
>See reply at the top.
>
>>>+[LED_ONLINE] = "Online"
>>> };
>>>
>>> static const char *repeats[REP_MAX + 1] = { diff --git
>>>a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index
>>>bcfaf32..b2a4136 100644
>>>--- a/drivers/hid/hid-input.c
>>>+++ b/drivers/hid/hid-input.c
>>>@@ -509,9 +509,11 @@ static void hidinput_configure_usage(struct
>>>hid_input *hidinput, struct hid_fiel  if (field->report_count < 1)
>>>goto ignore;
>>>
>>>-/* only LED usages are supported in output fields */
>>>+/* only LED and TELEPHONY usages are supported in output fields */
>>> if (field->report_type == HID_OUTPUT_REPORT && -(usage->hid &
>>>HID_USAGE_PAGE) != HID_UP_LED) {
>>>+(usage->hid & HID_USAGE_PAGE) != HID_UP_LED && (usage->hid &
>>>+HID_USAGE_PAGE) != HID_UP_TELEPHONY) {
>>>+
>>> goto ignore;
>>> }
>>
>>This allowance for telephony page output mapping in order to map
>>LED_RINGER would not be needed if the 0x000b009e mapping
>were done by a vendor driver and the vendor driver was allowed access before the currently preemptive LED page test, like this:
>>
>>if (device->driver->input_mapping) {
>>int ret = device->driver->input_mapping(device, hidinput, field,
>>usage, &bit, &max); if (ret > 0) goto mapped; if (ret < 0) goto
>>ignore; }
>>
>>/* only LED usages are supported in output fields */ if
>>(field->report_type == HID_OUTPUT_REPORT && (usage->hid &
>>HID_USAGE_PAGE) != HID_UP_LED) { goto ignore; }
>>
>
>See reply at the top.
>
>>>
>>>@@ -658,11 +660,32 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>>> 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 0x17: /* "Off hook" */
>>>+map_led(LED_OFF_HOOK);
>>>+break;
>>>+case 0x18: /* "Ring" */
>>>+map_led(LED_RING);
>>>+break;
>>> case 0x19:  map_led (LED_MAIL);     break;    /*   "Message Waiting"          */
>>>+case 0x20: /* "Hold" */
>>>+map_led(LED_HOLD);
>>>+break;
>>>+case 0x21: /* "Microphone" */
>>>+map_led(LED_MICROPHONE);
>>>+break;
>>
>>As previous comment on duplicate functionality of LED_MICROPHONE and LED_MUTE.
>>Usage 0x00080021 could be mapped to LED_MUTE in a vendor driver.
>
>See reply at the top.
>
>>
>>>+case 0x27: /* "Stand-By" */
>>>+map_led(LED_SLEEP);
>>>+break;
>>>+case 0x2a: /* "Online" */
>>>+map_led(LED_ONLINE);
>>>+break;
>>>+case 0x4b: /* "Generic Indicator" */
>>>+map_led(LED_MISC);
>>>+break;
>>>+case 0x4c: /* "System Suspend" */
>>>+map_led(LED_SUSPEND);
>>>+break;
>>> case 0x4d:  map_led (LED_CHARGING); break;    /*   "External Power Connected" */
>>>
>>> default: goto ignore;
>>>@@ -732,7 +755,37 @@ static void hidinput_configure_usage(struct
>>>hid_input *hidinput, struct hid_fiel
>>>
>>> case HID_UP_TELEPHONY:
>>> switch (usage->hid & HID_USAGE) {
>>>+case 0x07:
>>>+map_key_clear(KEY_PROGRAMMABLE);
>>>+break;
>>
>>----
>>>+case 0x1e:
>>>+map_led(LED_SPEAKER);
>>>+break;
>>----
>>
>>This is in the wrong place, it should be in the HID_UP_LED case switch statement block.
>>Speaker usage 0x1e is in the LED page not the telephony page.
>>
>
>Yes, you are right. A V2 patch is forthcoming, once we agree how to proceed.
>
>>>+case 0x20:
>>>+map_key_clear(KEY_HOOK_SWITCH);
>>>+break;
>>>+case 0x21:
>>>+map_key_clear(KEY_FLASH);
>>>+break;
>>>+case 0x23:
>>>+map_key_clear(KEY_HOLD);
>>>+break;
>>>+case 0x24:
>>>+map_key_clear(KEY_REDIAL);
>>>+break;
>>>+case 0x2a:
>>>+map_key_clear(KEY_LINE);
>>>+break;
>>>+case 0x2b:
>>>+map_key_clear(KEY_SPEAKER_PHONE);
>>>+break;
>>> case 0x2f: map_key_clear(KEY_MICMUTE);break;
>>>+case 0x50:
>>>+map_key_clear(KEY_SPEED_DIAL);
>>>+break;
>>>+case 0x9e:
>>>+map_led(LED_RINGER);
>>>+break;
>>
>>As previous comment on duplicate functionality of LED_RINGER and LED_RING.
>>Usage 0x000b009e could be mapped to LED_RING in a vendor driver.
>
>See reply at the top.
>
>>
>>> case 0xb0: map_key_clear(KEY_NUMERIC_0);break;
>>> case 0xb1: map_key_clear(KEY_NUMERIC_1);break;
>>> case 0xb2: map_key_clear(KEY_NUMERIC_2);break;
>>>diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
>>>index 766bf26..cf692df 100644
>>>--- a/drivers/input/input-leds.c
>>>+++ b/drivers/input/input-leds.c
>>>@@ -36,6 +36,11 @@ static const struct {
>>> [LED_MISC]= { "misc" },
>>> [LED_MAIL]= { "mail" },
>>> [LED_CHARGING]= { "charging" },
>>>+[LED_OFF_HOOK]= { "off-hook" },
>>>+[LED_RING]= { "ring" },
>>>+[LED_HOLD]= { "hold" },
>>>+[LED_MICROPHONE] = { "microphone" },
>>
>>As previous comments on duplicate functionality of LED_MICROPHONE and LED_MUTE.
>>Is LED_RINGER missing here?
>
>If we change LED_RINGER to something else (maybe TEL_RINGER), then it doesn't belong here.
>
>>
>>>+[LED_ONLINE]= { "online" },
>>> };
>>>
>>> struct input_led {
>>>diff --git a/include/linux/hid.h b/include/linux/hid.h index
>>>75b66ec..0c91c89 100644
>>>--- a/include/linux/hid.h
>>>+++ b/include/linux/hid.h
>>>@@ -764,7 +764,13 @@ struct hid_ll_driver {
>>>
>>> /* Applications from HID Usage Tables 4/8/99 Version 1.1 */
>>> /* We ignore a few input applications that are not widely used */
>>>-#define IS_INPUT_APPLICATION(a) (((a >= 0x00010000) && (a <=
>>>0x00010008)) || (a == 0x00010080) || (a == 0x000c0001) || ((a
>>>>= 0x000d0002) && (a <= 0x000d0006)))
>>>+#define IS_INPUT_APPLICATION(a) (\
>>>+((a >= 0x00010000) && (a <= 0x00010008)) ||\
>>>+(a == 0x00010080) ||\
>>>+(a == 0x000b0005) ||\
>>
>>This should be a range from 0x000b0001 through 0x000b0005 to include Phone (1), Answering Machine (2), Message Controls (3),
>Handset (4), as well as Headset (5) collections.
>
>Agreed. Coming in V2.
>
>>
>>>+(a == 0x000c0001) ||\
>>>+((a >= 0x000d0002) && (a <= 0x000d0006)))
>>>+
>>>
>>> /* HID core API */
>>>
>>>diff --git a/include/linux/mod_devicetable.h
>>>b/include/linux/mod_devicetable.h index ed84c07..e342d3f 100644
>>>--- a/include/linux/mod_devicetable.h
>>>+++ b/include/linux/mod_devicetable.h
>>>@@ -288,7 +288,7 @@ struct pcmcia_device_id {
>>> #define INPUT_DEVICE_ID_REL_MAX0x0f
>>> #define INPUT_DEVICE_ID_ABS_MAX0x3f
>>> #define INPUT_DEVICE_ID_MSC_MAX0x07
>>>-#define INPUT_DEVICE_ID_LED_MAX0x0f
>>>+#define INPUT_DEVICE_ID_LED_MAX0x11
>>> #define INPUT_DEVICE_ID_SND_MAX0x07
>>> #define INPUT_DEVICE_ID_FF_MAX0x7f
>>> #define INPUT_DEVICE_ID_SW_MAX0x0f
>>>diff --git a/include/uapi/linux/input-event-codes.h
>>>b/include/uapi/linux/input-event-codes.h
>>>index d6d071f..eaa368b 100644
>>>--- a/include/uapi/linux/input-event-codes.h
>>>+++ b/include/uapi/linux/input-event-codes.h
>>>@@ -593,6 +593,15 @@
>>>
>>> #define KEY_ALS_TOGGLE0x230/* Ambient light sensor */
>>>
>>>+#define KEY_HOOK_SWITCH0x231
>>>+#define KEY_FLASH0x232
>>>+#define KEY_HOLD0x233
>>>+#define KEY_REDIAL0x234
>>>+#define KEY_PROGRAMMABLE0x235
>>>+#define KEY_LINE0x236
>>>+#define KEY_SPEAKER_PHONE0x237
>>>+#define KEY_SPEED_DIAL0x238
>>>+
>>> #define KEY_BUTTONCONFIG0x240/* AL Button Configuration */
>>> #define KEY_TASKMANAGER0x241/* AL Task/Project Manager */
>>> #define KEY_JOURNAL0x242/* AL Log/Journal/Timecard */
>>>@@ -812,7 +821,14 @@
>>> #define LED_MISC0x08
>>> #define LED_MAIL0x09
>>> #define LED_CHARGING0x0a
>>>-#define LED_MAX0x0f
>>>+#define LED_OFF_HOOK0x0b
>>>+#define LED_RING0x0c
>>>+#define LED_RINGER0x0d
>>
>>As previous comments on duplicate functionality of LED_RINGER and LED_RING.
>
>See reply at the top.
>
>>
>>>+#define LED_HOLD0x0e
>>>+#define LED_MICROPHONE0x0f
>>
>>As previous comments on duplicate functionality of LED_MICROPHONE and LED_MUTE.
>
>See reply at the top.
>
>>
>>>+#define LED_SPEAKER0x10
>>>+#define LED_ONLINE0x11
>>>+#define LED_MAX0x11
>>> #define LED_CNT(LED_MAX+1)
>>>
>>> /*
>>>--
>>>2.7.4
>>>
>>
>>Thanks,
>>
>>Terry Junge
>>Plantronics
>
>Thanks,
>
>Niels Skou Olsen
>Jabra
>
>**** GN GROUP NOTICE - AUTOMATICALLY INSERTED**** The information in this e-mail (including attachments, if any) is
>considered confidential and is intended only for the recipient(s) listed above. Any review, use, disclosure, distribution or copying of
>this e-mail is prohibited except by or on behalf of the intended recipient. If you have received this email in error, please notify me
>immediately by reply e-mail, delete this e-mail, and do not disclose its contents to anyone. Any opinions expressed in this e-mail are
>those of the individual and not necessarily the GN group. Thank you. ******************** DISCLAIMER END
>************************
**** GN GROUP NOTICE - AUTOMATICALLY INSERTED**** The information in this e-mail (including attachments, if any) is considered confidential and is intended only for the recipient(s) listed above. Any review, use, disclosure, distribution or copying of this e-mail is prohibited except by or on behalf of the intended recipient. If you have received this email in error, please notify me immediately by reply e-mail, delete this e-mail, and do not disclose its contents to anyone. Any opinions expressed in this e-mail are those of the individual and not necessarily the GN group. Thank you. ******************** DISCLAIMER END ************************
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

HID: Support telephony devices

Commit Message

Comments

Patch