diff mbox series

HID: input: Add support for Programmable Buttons

Message ID 20210519160349.609690-1-linux@weissschuh.net (mailing list archive)
State Superseded
Delegated to: Jiri Kosina
Headers show
Series HID: input: Add support for Programmable Buttons | expand

Commit Message

Thomas Weißschuh May 19, 2021, 4:03 p.m. UTC 
From: Thomas Weißschuh <thomas@t-8ch.de>

Map them to KEY_MACRO# event codes.

These buttons are defined by HID as follows:
"The user defines the function of these buttons to control software
applications or GUI objects."

This matches the semantics of the KEY_MACRO# input event codes that
Linux supports.

Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
---
 drivers/hid/hid-debug.c | 11 +++++++++++
 drivers/hid/hid-input.c |  1 +
 2 files changed, 12 insertions(+)


base-commit: efd8929b9eec1cde120abb36d76dd00ff6711023

Comments

Hans de Goede May 19, 2021, 4:13 p.m. UTC | #1 
Hi,

On 5/19/21 6:03 PM, Thomas Weißschuh wrote:
> From: Thomas Weißschuh <thomas@t-8ch.de>
> 
> Map them to KEY_MACRO# event codes.
> 
> These buttons are defined by HID as follows:
> "The user defines the function of these buttons to control software
> applications or GUI objects."
> 
> This matches the semantics of the KEY_MACRO# input event codes that
> Linux supports.
> 
> Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
> ---
>  drivers/hid/hid-debug.c | 11 +++++++++++
>  drivers/hid/hid-input.c |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index 59f8d716d78f..0e76d9b4530a 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -122,6 +122,7 @@ static const struct hid_usage_entry hid_usage_table[] = {
>    {  9, 0, "Button" },
>    { 10, 0, "Ordinal" },
>    { 12, 0, "Consumer" },
> +      {0, 0x003, "ProgrammableButtons"},
>        {0, 0x238, "HorizontalWheel"},
>    { 13, 0, "Digitizers" },
>      {0, 0x01, "Digitizer"},
> @@ -939,6 +940,16 @@ static const char *keys[KEY_MAX + 1] = {
>  	[KEY_KBDINPUTASSIST_NEXTGROUP] = "KbdInputAssistNextGroup",
>  	[KEY_KBDINPUTASSIST_ACCEPT] = "KbdInputAssistAccept",
>  	[KEY_KBDINPUTASSIST_CANCEL] = "KbdInputAssistCancel",
> +	[KEY_MACRO1] = "Macro1", [KEY_MACRO2] = "Macro2", [KEY_MACRO3] = "Macro3",
> +	[KEY_MACRO4] = "Macro4", [KEY_MACRO5] = "Macro5", [KEY_MACRO6] = "Macro6",
> +	[KEY_MACRO7] = "Macro7", [KEY_MACRO8] = "Macro8", [KEY_MACRO9] = "Macro9",
> +	[KEY_MACRO10] = "Macro10", [KEY_MACRO11] = "Macro11", [KEY_MACRO12] = "Macro12",
> +	[KEY_MACRO13] = "Macro13", [KEY_MACRO14] = "Macro14", [KEY_MACRO15] = "Macro15",
> +	[KEY_MACRO16] = "Macro16", [KEY_MACRO17] = "Macro17", [KEY_MACRO18] = "Macro18",
> +	[KEY_MACRO19] = "Macro19", [KEY_MACRO20] = "Macro20", [KEY_MACRO21] = "Macro21",
> +	[KEY_MACRO22] = "Macro22", [KEY_MACRO23] = "Macro23", [KEY_MACRO24] = "Macro24",
> +	[KEY_MACRO25] = "Macro25", [KEY_MACRO26] = "Macro26", [KEY_MACRO27] = "Macro27",
> +	[KEY_MACRO28] = "Macro28", [KEY_MACRO29] = "Macro29", [KEY_MACRO30] = "Macro30",
>  };
>  
>  static const char *relatives[REL_MAX + 1] = {
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 18f5e28d475c..7d4dee58d869 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -632,6 +632,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>  				else
>  					code += BTN_TRIGGER_HAPPY - 0x10;
>  				break;
> +		case HID_CP_CONSUMER_CONTROL: code += KEY_MACRO1; break;

Shouldn't there be a check here to ensure that we don't map things above KEY_MACRO30 ?
if we do that then we start hitting other codes like KEY_MACRO_RECORD_START and eventually
BTN_TRIGGER_HAPPY and after the BTN_TRIGGER_HAPPY range we go over KEY_MAX which I think
is not supported ?

Regards,

Hans



>  		default:
>  			switch (field->physical) {
>  			case HID_GD_MOUSE:
> 
> base-commit: efd8929b9eec1cde120abb36d76dd00ff6711023
>
Thomas Weißschuh May 19, 2021, 5:11 p.m. UTC | #2 
Hi,

On Mi, 2021-05-19T18:13+0200, Hans de Goede wrote:
> Hi,
> 
> On 5/19/21 6:03 PM, Thomas Weißschuh wrote:
> > From: Thomas Weißschuh <thomas@t-8ch.de>
> > 
> > Map them to KEY_MACRO# event codes.
> > 
> > These buttons are defined by HID as follows:
> > "The user defines the function of these buttons to control software
> > applications or GUI objects."
> > 
> > This matches the semantics of the KEY_MACRO# input event codes that
> > Linux supports.
> > 
> > Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
> > ---
> >  drivers/hid/hid-debug.c | 11 +++++++++++
> >  drivers/hid/hid-input.c |  1 +
> >  2 files changed, 12 insertions(+)
> > 
> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > index 18f5e28d475c..7d4dee58d869 100644
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -632,6 +632,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> >  				else
> >  					code += BTN_TRIGGER_HAPPY - 0x10;
> >  				break;
> > +		case HID_CP_CONSUMER_CONTROL: code += KEY_MACRO1; break;
> 
> Shouldn't there be a check here to ensure that we don't map things above KEY_MACRO30 ?
> if we do that then we start hitting other codes like KEY_MACRO_RECORD_START and eventually
> BTN_TRIGGER_HAPPY and after the BTN_TRIGGER_HAPPY range we go over KEY_MAX which I think
> is not supported ?
> 
> Regards,
> 
> Hans

I thought all the other chunks of logic around this one would be affected by
this issue, too.
But actually it seems all the overflowing keys get first assigned to the
BTN_TRIGGER_HAPPY range and after that will be clipped directly by
map_key()/hid_map_usage().

I'll resend the patch.

Thanks,
Thomas
diff mbox series

Patch

diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 59f8d716d78f..0e76d9b4530a 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -122,6 +122,7 @@  static const struct hid_usage_entry hid_usage_table[] = {
   {  9, 0, "Button" },
   { 10, 0, "Ordinal" },
   { 12, 0, "Consumer" },
+      {0, 0x003, "ProgrammableButtons"},
       {0, 0x238, "HorizontalWheel"},
   { 13, 0, "Digitizers" },
     {0, 0x01, "Digitizer"},
@@ -939,6 +940,16 @@  static const char *keys[KEY_MAX + 1] = {
 	[KEY_KBDINPUTASSIST_NEXTGROUP] = "KbdInputAssistNextGroup",
 	[KEY_KBDINPUTASSIST_ACCEPT] = "KbdInputAssistAccept",
 	[KEY_KBDINPUTASSIST_CANCEL] = "KbdInputAssistCancel",
+	[KEY_MACRO1] = "Macro1", [KEY_MACRO2] = "Macro2", [KEY_MACRO3] = "Macro3",
+	[KEY_MACRO4] = "Macro4", [KEY_MACRO5] = "Macro5", [KEY_MACRO6] = "Macro6",
+	[KEY_MACRO7] = "Macro7", [KEY_MACRO8] = "Macro8", [KEY_MACRO9] = "Macro9",
+	[KEY_MACRO10] = "Macro10", [KEY_MACRO11] = "Macro11", [KEY_MACRO12] = "Macro12",
+	[KEY_MACRO13] = "Macro13", [KEY_MACRO14] = "Macro14", [KEY_MACRO15] = "Macro15",
+	[KEY_MACRO16] = "Macro16", [KEY_MACRO17] = "Macro17", [KEY_MACRO18] = "Macro18",
+	[KEY_MACRO19] = "Macro19", [KEY_MACRO20] = "Macro20", [KEY_MACRO21] = "Macro21",
+	[KEY_MACRO22] = "Macro22", [KEY_MACRO23] = "Macro23", [KEY_MACRO24] = "Macro24",
+	[KEY_MACRO25] = "Macro25", [KEY_MACRO26] = "Macro26", [KEY_MACRO27] = "Macro27",
+	[KEY_MACRO28] = "Macro28", [KEY_MACRO29] = "Macro29", [KEY_MACRO30] = "Macro30",
 };
 
 static const char *relatives[REL_MAX + 1] = {
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 18f5e28d475c..7d4dee58d869 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -632,6 +632,7 @@  static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 				else
 					code += BTN_TRIGGER_HAPPY - 0x10;
 				break;
+		case HID_CP_CONSUMER_CONTROL: code += KEY_MACRO1; break;
 		default:
 			switch (field->physical) {
 			case HID_GD_MOUSE: