Re: [PATCH] HID: input: Add support for Programmable Buttons

[Thomas Weißschuh]

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@xxxxxxxx>
> > 
> > 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@xxxxxxxx>
> > ---
> >  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