[PATCH 0/6] Add support for common USB HID headset features

[PATCH 0/6] Add support for common USB HID headset features

[Maxim Mikityanskiy]

This patch series extends hid-input to support features commonly present
in USB HID headsets:

1. Mic mute button and LED.

2. Answer/hangup button and ring/offhook LED(s).

These buttons and LEDs are exposed using standard event device
interfaces. It allows the VoIP applications in userspace to use the new
functionality in a generic device-independent fashion.

This series also contains patches for hid-plantronics and hid-jabra to
expose the new functionality and work around some hardware quirks, and a
fix to hid-input for an issue that prevents the new LED functionality to
work with certain headsets.

Although checkpatch compains about a few lines, they preserve the same
formatting that already exists in hid-input.c.

Maxim Mikityanskiy (6):
  HID: hid-input: Add offhook and ring LEDs for headsets
  HID: hid-input: Add phone hook and mic mute buttons for headsets
  HID: plantronics: Expose headset LEDs
  HID: plantronics: Expose headset telephony buttons
  HID: hid-input: Update LEDs in all HID reports
  HID: jabra: Change mute LED state to avoid missing key press events

 drivers/hid/hid-input.c                | 183 +++++++++++++++++++++++--
 drivers/hid/hid-jabra.c                |  35 +++++
 drivers/hid/hid-plantronics.c          |   6 +
 drivers/input/input-leds.c             |   2 +
 include/uapi/linux/input-event-codes.h |  10 ++
 5 files changed, 225 insertions(+), 11 deletions(-)

-- 
2.32.0

[PATCH 1/6] HID: hid-input: Add offhook and ring LEDs for headsets

[Maxim Mikityanskiy]

A lot of USBHID headsets available on the market have LEDs that indicate
ringing and off-hook states when used with VoIP applications. This
commit exposes these LEDs via the standard sysfs interface.

Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 drivers/hid/hid-input.c                | 2 ++
 drivers/input/input-leds.c             | 2 ++
 include/uapi/linux/input-event-codes.h | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 4286a51f7f16..44b8243f9924 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -798,6 +798,8 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 		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 0x18:  map_led (LED_RING);     break;    /*   "Ring"                     */
 
 		default: goto ignore;
 		}
diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
index 0b11990ade46..bc6e25b9af25 100644
--- a/drivers/input/input-leds.c
+++ b/drivers/input/input-leds.c
@@ -33,6 +33,8 @@ static const struct {
 	[LED_MISC]	= { "misc" },
 	[LED_MAIL]	= { "mail" },
 	[LED_CHARGING]	= { "charging" },
+	[LED_OFFHOOK]	= { "offhook" },
+	[LED_RING]	= { "ring" },
 };
 
 struct input_led {
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 225ec87d4f22..dd785a5b5076 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -925,6 +925,8 @@
 #define LED_MISC		0x08
 #define LED_MAIL		0x09
 #define LED_CHARGING		0x0a
+#define LED_OFFHOOK		0x0b
+#define LED_RING		0x0c
 #define LED_MAX			0x0f
 #define LED_CNT			(LED_MAX+1)
 
-- 
2.32.0

Re: [PATCH 1/6] HID: hid-input: Add offhook and ring LEDs for headsets

[Benjamin Tissoires]

Hi Maxim,

On Sun, Jul 4, 2021 at 12:02 AM Maxim Mikityanskiy <maxtram95@gmail.com> wrote:
>
> A lot of USBHID headsets available on the market have LEDs that indicate
> ringing and off-hook states when used with VoIP applications. This
> commit exposes these LEDs via the standard sysfs interface.
>
> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
> ---
>  drivers/hid/hid-input.c                | 2 ++
>  drivers/input/input-leds.c             | 2 ++
>  include/uapi/linux/input-event-codes.h | 2 ++
>  3 files changed, 6 insertions(+)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 4286a51f7f16..44b8243f9924 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -798,6 +798,8 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>                 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 0x18:  map_led (LED_RING);     break;    /*   "Ring"                     */
>
>                 default: goto ignore;
>                 }
> diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
> index 0b11990ade46..bc6e25b9af25 100644
> --- a/drivers/input/input-leds.c
> +++ b/drivers/input/input-leds.c
> @@ -33,6 +33,8 @@ static const struct {
>         [LED_MISC]      = { "misc" },
>         [LED_MAIL]      = { "mail" },
>         [LED_CHARGING]  = { "charging" },
> +       [LED_OFFHOOK]   = { "offhook" },

I am pretty sure this also needs to be reviewed by the led folks.
Adding them in Cc.

Cheers,
Benjamin

> +       [LED_RING]      = { "ring" },
>  };
>
>  struct input_led {
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 225ec87d4f22..dd785a5b5076 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -925,6 +925,8 @@
>  #define LED_MISC               0x08
>  #define LED_MAIL               0x09
>  #define LED_CHARGING           0x0a
> +#define LED_OFFHOOK            0x0b
> +#define LED_RING               0x0c
>  #define LED_MAX                        0x0f
>  #define LED_CNT                        (LED_MAX+1)
>
> --
> 2.32.0
>

Re: [PATCH 1/6] HID: hid-input: Add offhook and ring LEDs for headsets

[Jiri Kosina]

On Tue, 6 Jul 2021, Benjamin Tissoires wrote:

> > A lot of USBHID headsets available on the market have LEDs that indicate
> > ringing and off-hook states when used with VoIP applications. This
> > commit exposes these LEDs via the standard sysfs interface.
> >
> > Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
> > ---
> >  drivers/hid/hid-input.c                | 2 ++
> >  drivers/input/input-leds.c             | 2 ++
> >  include/uapi/linux/input-event-codes.h | 2 ++
> >  3 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > index 4286a51f7f16..44b8243f9924 100644
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -798,6 +798,8 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> >                 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 0x18:  map_led (LED_RING);     break;    /*   "Ring"                     */
> >
> >                 default: goto ignore;
> >                 }
> > diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
> > index 0b11990ade46..bc6e25b9af25 100644
> > --- a/drivers/input/input-leds.c
> > +++ b/drivers/input/input-leds.c
> > @@ -33,6 +33,8 @@ static const struct {
> >         [LED_MISC]      = { "misc" },
> >         [LED_MAIL]      = { "mail" },
> >         [LED_CHARGING]  = { "charging" },
> > +       [LED_OFFHOOK]   = { "offhook" },
> 
> I am pretty sure this also needs to be reviewed by the led folks.
> Adding them in Cc.

Can we please get Ack from the LED maintainers? Thanks.

> 
> Cheers,
> Benjamin
> 
> > +       [LED_RING]      = { "ring" },
> >  };
> >
> >  struct input_led {
> > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > index 225ec87d4f22..dd785a5b5076 100644
> > --- a/include/uapi/linux/input-event-codes.h
> > +++ b/include/uapi/linux/input-event-codes.h
> > @@ -925,6 +925,8 @@
> >  #define LED_MISC               0x08
> >  #define LED_MAIL               0x09
> >  #define LED_CHARGING           0x0a
> > +#define LED_OFFHOOK            0x0b
> > +#define LED_RING               0x0c
> >  #define LED_MAX                        0x0f
> >  #define LED_CNT                        (LED_MAX+1)
> >
> > --
> > 2.32.0
> >
> 

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 1/6] HID: hid-input: Add offhook and ring LEDs for headsets

[Dmitry Torokhov]

On Thu, Jul 15, 2021 at 08:57:44PM +0200, Jiri Kosina wrote:
> On Tue, 6 Jul 2021, Benjamin Tissoires wrote:
> 
> > > A lot of USBHID headsets available on the market have LEDs that indicate
> > > ringing and off-hook states when used with VoIP applications. This
> > > commit exposes these LEDs via the standard sysfs interface.
> > >
> > > Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
> > > ---
> > >  drivers/hid/hid-input.c                | 2 ++
> > >  drivers/input/input-leds.c             | 2 ++
> > >  include/uapi/linux/input-event-codes.h | 2 ++
> > >  3 files changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > > index 4286a51f7f16..44b8243f9924 100644
> > > --- a/drivers/hid/hid-input.c
> > > +++ b/drivers/hid/hid-input.c
> > > @@ -798,6 +798,8 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> > >                 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 0x18:  map_led (LED_RING);     break;    /*   "Ring"                     */
> > >
> > >                 default: goto ignore;
> > >                 }
> > > diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
> > > index 0b11990ade46..bc6e25b9af25 100644
> > > --- a/drivers/input/input-leds.c
> > > +++ b/drivers/input/input-leds.c
> > > @@ -33,6 +33,8 @@ static const struct {
> > >         [LED_MISC]      = { "misc" },
> > >         [LED_MAIL]      = { "mail" },
> > >         [LED_CHARGING]  = { "charging" },
> > > +       [LED_OFFHOOK]   = { "offhook" },
> > 
> > I am pretty sure this also needs to be reviewed by the led folks.
> > Adding them in Cc.
> 
> Can we please get Ack from the LED maintainers? Thanks.

I do not think we should be adding more LED bits to the input
subsystem/events; this functionality should be routed purely though LED
subsystem. input-leds is a bridge for legacy input functionality
reflecting it onto the newer LED subsystem.

Thanks.

-- 
Dmitry

Re: [PATCH 1/6] HID: hid-input: Add offhook and ring LEDs for headsets

[Pavel Machek]

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

On Thu 2021-07-15 13:39:26, Dmitry Torokhov wrote:
> On Thu, Jul 15, 2021 at 08:57:44PM +0200, Jiri Kosina wrote:
> > On Tue, 6 Jul 2021, Benjamin Tissoires wrote:
> > 
> > > > A lot of USBHID headsets available on the market have LEDs that indicate
> > > > ringing and off-hook states when used with VoIP applications. This
> > > > commit exposes these LEDs via the standard sysfs interface.

> > > > diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
> > > > index 0b11990ade46..bc6e25b9af25 100644
> > > > --- a/drivers/input/input-leds.c
> > > > +++ b/drivers/input/input-leds.c
> > > > @@ -33,6 +33,8 @@ static const struct {
> > > >         [LED_MISC]      = { "misc" },
> > > >         [LED_MAIL]      = { "mail" },
> > > >         [LED_CHARGING]  = { "charging" },
> > > > +       [LED_OFFHOOK]   = { "offhook" },
> > > 
> > > I am pretty sure this also needs to be reviewed by the led folks.
> > > Adding them in Cc.
> > 
> > Can we please get Ack from the LED maintainers? Thanks.
> 
> I do not think we should be adding more LED bits to the input
> subsystem/events; this functionality should be routed purely though LED
> subsystem. input-leds is a bridge for legacy input functionality
> reflecting it onto the newer LED subsystem.

If we do it purely through the LED subsystem, will it get trickier to
associate the devices?

Anyway, it is a headset. What does headset have to do with input
subsystem? Sounds like sound device to me... And we already have a
"micmute" LED which sounds quite similar to the "offhook" LED... no?

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH 1/6] HID: hid-input: Add offhook and ring LEDs for headsets

[Maxim Mikityanskiy]

On Fri, Jul 16, 2021 at 1:49 AM Pavel Machek <pavel@ucw.cz> wrote:
>
> On Thu 2021-07-15 13:39:26, Dmitry Torokhov wrote:
> > On Thu, Jul 15, 2021 at 08:57:44PM +0200, Jiri Kosina wrote:
> > > On Tue, 6 Jul 2021, Benjamin Tissoires wrote:
> > >
> > > > > A lot of USBHID headsets available on the market have LEDs that indicate
> > > > > ringing and off-hook states when used with VoIP applications. This
> > > > > commit exposes these LEDs via the standard sysfs interface.
>
> > > > > diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
> > > > > index 0b11990ade46..bc6e25b9af25 100644
> > > > > --- a/drivers/input/input-leds.c
> > > > > +++ b/drivers/input/input-leds.c
> > > > > @@ -33,6 +33,8 @@ static const struct {
> > > > >         [LED_MISC]      = { "misc" },
> > > > >         [LED_MAIL]      = { "mail" },
> > > > >         [LED_CHARGING]  = { "charging" },
> > > > > +       [LED_OFFHOOK]   = { "offhook" },
> > > >
> > > > I am pretty sure this also needs to be reviewed by the led folks.
> > > > Adding them in Cc.
> > >
> > > Can we please get Ack from the LED maintainers? Thanks.
> >
> > I do not think we should be adding more LED bits to the input
> > subsystem/events; this functionality should be routed purely though LED
> > subsystem. input-leds is a bridge for legacy input functionality
> > reflecting it onto the newer LED subsystem.

I'm a bit confused by this answer. I wasn't aware that input-leds was
some legacy stuff. Moreover, hid-input only handles LEDs through
input-leds, it doesn't use any modern replacement. So, does your
answer mean I need to implement this replacement? If so, I anticipate
some issues with this approach:

1. hid-input will handle different LEDs in different ways, which will
make code complicated and error-prone. There will be two parallel
implementations for LEDs.

2. A lot of code will be similar with input-leds, but not shared/reused.

3. New driver callbacks may be needed if drivers want to override the
default behavior, like they do with input_mapping/input_mapped.

4. If some hypothetical input device is a headset, but not HID, it
won't be able to reuse the LED handling code. With input-leds it
wouldn't be a problem.

5. (Minor) LED_MUTE is already there. If we handle LED_OFFHOOK and
LED_RING in a different way, it would be confusing.

Let's discuss the architecture before I write any new code, if we are
going to take this way. However, to me, input-leds looks like a better
fit: the implementation is much simpler and follows existing patterns,
and it integrates well with drivers and hid-input. If we throw away
input-leds, we'll have to do its job ourselves, and if we throw it
away only for part of LEDs, the code will likely be ugly.

> If we do it purely through the LED subsystem, will it get trickier to
> associate the devices?

I agree with this point. With the current approach, it's easy to look
up all LEDs of an input device. If the suggested approach makes it
hard, it's a serious drawback.

> Anyway, it is a headset. What does headset have to do with input
> subsystem? Sounds like sound device to me...

That's right, the main function of a headset is of course sound, but
such headsets also have buttons (vol up/down, mic mute, answer the
call) and LEDs (mic muted, ringing, offhook). The sound "subdevice"
(sorry, I'm not really familiar with USB terminology) is handled by
snd-usb-audio, and the buttons/LEDs are handled by usbhid.

Two examples of such headsets are mentioned in commit messages in this
patch series. Such headsets are usually "certified for skype for
business", but of course can be used with a variety of other VoIP
applications. The goal of this series is to provide a standard
interface between headsets and userspace applications, so that VoIP
applications could react to buttons and set LED state, making Linux
more ready for desktop.

> And we already have a
> "micmute" LED which sounds quite similar to the "offhook" LED... no?

These two are different. A typical headset has three LEDs: micmute,
ring and offhook (ring and offhook are often one physical LED, which
blinks in the ring state and glows constantly in the offhook state).

Offhook indicates that a call is in progress, while micmute shows that
the microphone is muted. These two states are orthogonal and may
happen in any combination. On the tested devices, offhook state didn't
affect sound, it was just a logical indication of an active call.

If you are interested in more details, I can describe the behavior of
the headsets that I tested (some info is actually in the commit
messages), but the short answer is that micmute and offhook are two
different physical LEDs with completely different functions.

>
> Best regards,
>                                                                 Pavel
> --
> http://www.livejournal.com/~pavelmachek

Re: [PATCH 1/6] HID: hid-input: Add offhook and ring LEDs for headsets

[Maxim Mikityanskiy]

Dmitry, what's your opinion on the points that I raised? I would like
to progress with this patch set, let's discuss the direction and sum
up the requirements.

On Fri, Jul 16, 2021 at 8:23 PM Maxim Mikityanskiy <maxtram95@gmail.com> wrote:
>
> On Fri, Jul 16, 2021 at 1:49 AM Pavel Machek <pavel@ucw.cz> wrote:
> >
> > On Thu 2021-07-15 13:39:26, Dmitry Torokhov wrote:
> > > On Thu, Jul 15, 2021 at 08:57:44PM +0200, Jiri Kosina wrote:
> > > > On Tue, 6 Jul 2021, Benjamin Tissoires wrote:
> > > >
> > > > > > A lot of USBHID headsets available on the market have LEDs that indicate
> > > > > > ringing and off-hook states when used with VoIP applications. This
> > > > > > commit exposes these LEDs via the standard sysfs interface.
> >
> > > > > > diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
> > > > > > index 0b11990ade46..bc6e25b9af25 100644
> > > > > > --- a/drivers/input/input-leds.c
> > > > > > +++ b/drivers/input/input-leds.c
> > > > > > @@ -33,6 +33,8 @@ static const struct {
> > > > > >         [LED_MISC]      = { "misc" },
> > > > > >         [LED_MAIL]      = { "mail" },
> > > > > >         [LED_CHARGING]  = { "charging" },
> > > > > > +       [LED_OFFHOOK]   = { "offhook" },
> > > > >
> > > > > I am pretty sure this also needs to be reviewed by the led folks.
> > > > > Adding them in Cc.
> > > >
> > > > Can we please get Ack from the LED maintainers? Thanks.
> > >
> > > I do not think we should be adding more LED bits to the input
> > > subsystem/events; this functionality should be routed purely though LED
> > > subsystem. input-leds is a bridge for legacy input functionality
> > > reflecting it onto the newer LED subsystem.
>
> I'm a bit confused by this answer. I wasn't aware that input-leds was
> some legacy stuff. Moreover, hid-input only handles LEDs through
> input-leds, it doesn't use any modern replacement. So, does your
> answer mean I need to implement this replacement? If so, I anticipate
> some issues with this approach:
>
> 1. hid-input will handle different LEDs in different ways, which will
> make code complicated and error-prone. There will be two parallel
> implementations for LEDs.
>
> 2. A lot of code will be similar with input-leds, but not shared/reused.
>
> 3. New driver callbacks may be needed if drivers want to override the
> default behavior, like they do with input_mapping/input_mapped.
>
> 4. If some hypothetical input device is a headset, but not HID, it
> won't be able to reuse the LED handling code. With input-leds it
> wouldn't be a problem.
>
> 5. (Minor) LED_MUTE is already there. If we handle LED_OFFHOOK and
> LED_RING in a different way, it would be confusing.
>
> Let's discuss the architecture before I write any new code, if we are
> going to take this way. However, to me, input-leds looks like a better
> fit: the implementation is much simpler and follows existing patterns,
> and it integrates well with drivers and hid-input. If we throw away
> input-leds, we'll have to do its job ourselves, and if we throw it
> away only for part of LEDs, the code will likely be ugly.
>
> > If we do it purely through the LED subsystem, will it get trickier to
> > associate the devices?
>
> I agree with this point. With the current approach, it's easy to look
> up all LEDs of an input device. If the suggested approach makes it
> hard, it's a serious drawback.
>
> > Anyway, it is a headset. What does headset have to do with input
> > subsystem? Sounds like sound device to me...
>
> That's right, the main function of a headset is of course sound, but
> such headsets also have buttons (vol up/down, mic mute, answer the
> call) and LEDs (mic muted, ringing, offhook). The sound "subdevice"
> (sorry, I'm not really familiar with USB terminology) is handled by
> snd-usb-audio, and the buttons/LEDs are handled by usbhid.
>
> Two examples of such headsets are mentioned in commit messages in this
> patch series. Such headsets are usually "certified for skype for
> business", but of course can be used with a variety of other VoIP
> applications. The goal of this series is to provide a standard
> interface between headsets and userspace applications, so that VoIP
> applications could react to buttons and set LED state, making Linux
> more ready for desktop.
>
> > And we already have a
> > "micmute" LED which sounds quite similar to the "offhook" LED... no?
>
> These two are different. A typical headset has three LEDs: micmute,
> ring and offhook (ring and offhook are often one physical LED, which
> blinks in the ring state and glows constantly in the offhook state).
>
> Offhook indicates that a call is in progress, while micmute shows that
> the microphone is muted. These two states are orthogonal and may
> happen in any combination. On the tested devices, offhook state didn't
> affect sound, it was just a logical indication of an active call.
>
> If you are interested in more details, I can describe the behavior of
> the headsets that I tested (some info is actually in the commit
> messages), but the short answer is that micmute and offhook are two
> different physical LEDs with completely different functions.
>
> >
> > Best regards,
> >                                                                 Pavel
> > --
> > http://www.livejournal.com/~pavelmachek

Re: [PATCH 1/6] HID: hid-input: Add offhook and ring LEDs for headsets

[Jiri Kosina]

On Mon, 9 Aug 2021, Maxim Mikityanskiy wrote:

> Dmitry, what's your opinion on the points that I raised? I would like
> to progress with this patch set, let's discuss the direction and sum
> up the requirements.

Friendly ping on this one :) Thanks.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 1/6] HID: hid-input: Add offhook and ring LEDs for headsets

[Dmitry Torokhov]

Hi,

On Fri, Jul 16, 2021 at 08:23:02PM +0300, Maxim Mikityanskiy wrote:

Sorry for disappearing for a while.

> On Fri, Jul 16, 2021 at 1:49 AM Pavel Machek <pavel@ucw.cz> wrote:
> >
> > On Thu 2021-07-15 13:39:26, Dmitry Torokhov wrote:
> > > On Thu, Jul 15, 2021 at 08:57:44PM +0200, Jiri Kosina wrote:
> > > > On Tue, 6 Jul 2021, Benjamin Tissoires wrote:
> > > >
> > > > > > A lot of USBHID headsets available on the market have LEDs that indicate
> > > > > > ringing and off-hook states when used with VoIP applications. This
> > > > > > commit exposes these LEDs via the standard sysfs interface.
> >
> > > > > > diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
> > > > > > index 0b11990ade46..bc6e25b9af25 100644
> > > > > > --- a/drivers/input/input-leds.c
> > > > > > +++ b/drivers/input/input-leds.c
> > > > > > @@ -33,6 +33,8 @@ static const struct {
> > > > > >         [LED_MISC]      = { "misc" },
> > > > > >         [LED_MAIL]      = { "mail" },
> > > > > >         [LED_CHARGING]  = { "charging" },
> > > > > > +       [LED_OFFHOOK]   = { "offhook" },
> > > > >
> > > > > I am pretty sure this also needs to be reviewed by the led folks.
> > > > > Adding them in Cc.
> > > >
> > > > Can we please get Ack from the LED maintainers? Thanks.
> > >
> > > I do not think we should be adding more LED bits to the input
> > > subsystem/events; this functionality should be routed purely though LED
> > > subsystem. input-leds is a bridge for legacy input functionality
> > > reflecting it onto the newer LED subsystem.
> 
> I'm a bit confused by this answer. I wasn't aware that input-leds was
> some legacy stuff.

Yes, input-leds provides bridge from legacy leds defined in input
subsystem over to proper LED subsystem that we have now. Now that we
have proper LED subsystem all new LEDs should be introduced via it.
Especially given that some of the LEDs defined in HID have little
relation to input (Printer Out of Paper, Battery OK, RGB LEDs, etc).

> Moreover, hid-input only handles LEDs through
> input-leds, it doesn't use any modern replacement. So, does your
> answer mean I need to implement this replacement?

Yes.

> If so, I anticipate
> some issues with this approach:
> 
> 1. hid-input will handle different LEDs in different ways, which will
> make code complicated and error-prone. There will be two parallel
> implementations for LEDs.

Yes you will need to route currently defined input LEDs as they are now
to keep compatibility with existing userspace, and new LEDs should be
registered directly.

> 
> 2. A lot of code will be similar with input-leds, but not shared/reused.

Hmm, not really. I mean you will need to call to register LEDs and
toggle them, but that's the same as any other driver registering LEDs.

> 
> 3. New driver callbacks may be needed if drivers want to override the
> default behavior, like they do with input_mapping/input_mapped.

OK.

> 
> 4. If some hypothetical input device is a headset, but not HID, it
> won't be able to reuse the LED handling code. With input-leds it
> wouldn't be a problem.

We have a lot of non-HID touchpads, touchscreens, mice, joysticks, etc
that do it for all other data. LEDs are not different.

> 
> 5. (Minor) LED_MUTE is already there. If we handle LED_OFFHOOK and
> LED_RING in a different way, it would be confusing.

Not to userspace that uses LED subsystem. And I admit that adding LEDs
beyond keyboard indicators was a mistake (but we did not have proper LED
subsystem back then).

> 
> Let's discuss the architecture before I write any new code, if we are
> going to take this way. However, to me, input-leds looks like a better
> fit: the implementation is much simpler and follows existing patterns,
> and it integrates well with drivers and hid-input. If we throw away
> input-leds, we'll have to do its job ourselves, and if we throw it
> away only for part of LEDs, the code will likely be ugly.

I think it will be OK. You just need to note how each led should be
routed, and then call appropriate API when handling events.

> 
> > If we do it purely through the LED subsystem, will it get trickier to
> > associate the devices?
> 
> I agree with this point. With the current approach, it's easy to look
> up all LEDs of an input device. If the suggested approach makes it
> hard, it's a serious drawback.

You already need to deal with composite devices and figure a way to
associate all parts of them. And you already need to locate LED devices
associated with the input device because you are supposed to interface
via LED subsystem and not input (i.e. new applications should not be
using EVIOCGLED and writing EV_LED to evdev to control LEDs).

> 
> > Anyway, it is a headset. What does headset have to do with input
> > subsystem? Sounds like sound device to me...
> 
> That's right, the main function of a headset is of course sound, but
> such headsets also have buttons (vol up/down, mic mute, answer the
> call) and LEDs (mic muted, ringing, offhook). The sound "subdevice"
> (sorry, I'm not really familiar with USB terminology) is handled by
> snd-usb-audio, and the buttons/LEDs are handled by usbhid.
> 
> Two examples of such headsets are mentioned in commit messages in this
> patch series. Such headsets are usually "certified for skype for
> business", but of course can be used with a variety of other VoIP
> applications. The goal of this series is to provide a standard
> interface between headsets and userspace applications, so that VoIP
> applications could react to buttons and set LED state, making Linux
> more ready for desktop.
> 
> > And we already have a
> > "micmute" LED which sounds quite similar to the "offhook" LED... no?
> 
> These two are different. A typical headset has three LEDs: micmute,
> ring and offhook (ring and offhook are often one physical LED, which
> blinks in the ring state and glows constantly in the offhook state).
> 
> Offhook indicates that a call is in progress, while micmute shows that
> the microphone is muted. These two states are orthogonal and may
> happen in any combination. On the tested devices, offhook state didn't
> affect sound, it was just a logical indication of an active call.
> 
> If you are interested in more details, I can describe the behavior of
> the headsets that I tested (some info is actually in the commit
> messages), but the short answer is that micmute and offhook are two
> different physical LEDs with completely different functions.
> 
> >
> > Best regards,
> >                                                                 Pavel
> > --
> > http://www.livejournal.com/~pavelmachek

Thanks.

-- 
Dmitry

[PATCH 2/6] HID: hid-input: Add phone hook and mic mute buttons for headsets

[Maxim Mikityanskiy]

A lot of USBHID headsets available on the market have buttons to toggle
microphone mute and to answer the call/hang up.

According to the HID Usage Tables specification, these usages are on/off
controls, which may be presented by either two buttons, a single toggle
button or a mechanical switch. This commit adds a function called
hidinput_handle_onoff that handles all these cases in a compliant way.

Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 drivers/hid/hid-input.c                | 140 +++++++++++++++++++++++++
 include/uapi/linux/input-event-codes.h |   8 ++
 2 files changed, 148 insertions(+)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 44b8243f9924..533a7f429a5f 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -579,6 +579,43 @@ static bool hidinput_field_in_collection(struct hid_device *device, struct hid_f
 	return collection->type == type && collection->usage == usage;
 }
 
+/**
+ * hidinput_get_onoff_keycodes - Gets on and off keycodes for OOC usages.
+ * @usage: HID usage.
+ * @code_on: Output parameter for the on keycode.
+ * @code_off: Output parameter for the off keycode.
+ *
+ * Returns true if @usage is a supported on/off control (OOC), as defined by HID
+ * Usage Tables 1.21 (3.4.1.2).
+ *
+ * Depending on the OOC type, we need to send either a toggle keycode or
+ * separate on/off keycodes. This function detects whether @usage is an OOC. If
+ * yes, and if this OOC is supported, it returns the on and off keycodes
+ * corresponding to the toggle keycode stored in usage->code.
+ */
+static bool hidinput_get_onoff_keycodes(struct hid_usage *usage,
+					u16 *code_on, u16 *code_off)
+{
+	if (usage->type != EV_KEY)
+		return false;
+
+	if ((usage->hid & HID_USAGE_PAGE) != HID_UP_TELEPHONY)
+		return false;
+
+	switch (usage->code) {
+	case KEY_TOGGLE_PHONE:
+		*code_on = KEY_PICKUP_PHONE;
+		*code_off = KEY_HANGUP_PHONE;
+		return true;
+	case KEY_MICMUTE:
+		*code_on = KEY_MICMUTE_ON;
+		*code_off = KEY_MICMUTE_OFF;
+		return true;
+	}
+
+	return false;
+}
+
 static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_field *field,
 				     struct hid_usage *usage)
 {
@@ -586,6 +623,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 	struct hid_device *device = input_get_drvdata(input);
 	int max = 0, code;
 	unsigned long *bit = NULL;
+	u16 code_on, code_off;
 
 	field->hidinput = hidinput;
 
@@ -887,6 +925,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 
 	case HID_UP_TELEPHONY:
 		switch (usage->hid & HID_USAGE) {
+		case 0x20: map_key_clear(KEY_TOGGLE_PHONE);	break;
 		case 0x2f: map_key_clear(KEY_MICMUTE);		break;
 		case 0xb0: map_key_clear(KEY_NUMERIC_0);	break;
 		case 0xb1: map_key_clear(KEY_NUMERIC_1);	break;
@@ -1198,6 +1237,11 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 
 	set_bit(usage->type, input->evbit);
 
+	if (hidinput_get_onoff_keycodes(usage, &code_on, &code_off)) {
+		set_bit(code_on, bit);
+		set_bit(code_off, bit);
+	}
+
 	/*
 	 * This part is *really* controversial:
 	 * - HID aims at being generic so we should do our best to export
@@ -1314,6 +1358,92 @@ static void hidinput_handle_scroll(struct hid_usage *usage,
 	input_event(input, EV_REL, usage->code, hi_res);
 }
 
+/**
+ * hidinput_handle_onoff - Handle on/off control (OOC).
+ * @field: HID field that corresponds to the event.
+ * @value: HID value that corresponds to the event.
+ * @code_toggle: Key code to send when toggling state of the on/off control.
+ * @code_on: Key code to send when turning on the on/off control.
+ * @code_off: Key code to send when turning off the on/off control.
+ *
+ * Returns true if the event was handled, false if the @field flags are invalid.
+ *
+ * Handles on/off control (OOC), as defined by HID Usage Tables 1.21 (3.4.1.2).
+ * Determines the type of the OOC by looking at @field and sends one of the key
+ * codes accordingly. Whenever it's possible to distinguish on and off states,
+ * different key strokes (@code_on, @code_off) are sent, otherwise @code_toggle
+ * is sent.
+ */
+static bool hidinput_handle_onoff(struct hid_field *field, __s32 value, unsigned int scan,
+				  __u16 code_toggle, __u16 code_on, __u16 code_off)
+{
+	struct input_dev *input = field->hidinput->input;
+	__u16 code = 0;
+
+	/* Two buttons, on and off */
+	if ((field->flags & HID_MAIN_ITEM_RELATIVE) &&
+	    (field->flags & HID_MAIN_ITEM_NO_PREFERRED) &&
+	    (field->logical_minimum == -1) &&
+	    (field->logical_maximum == 1)) {
+		if (value != 1 && value != -1)
+			return true;
+
+		code = value == 1 ? code_on : code_off;
+	}
+
+	/* A single button that toggles the on/off state each time it is pressed */
+	if ((field->flags & HID_MAIN_ITEM_RELATIVE) &&
+	    !(field->flags & HID_MAIN_ITEM_NO_PREFERRED) &&
+	    (field->logical_minimum == 0) &&
+	    (field->logical_maximum == 1)) {
+		if (value != 1)
+			return true;
+
+		code = code_toggle;
+	}
+
+	/* A toggle switch that maintains the on/off state mechanically */
+	if (!(field->flags & HID_MAIN_ITEM_RELATIVE) &&
+	    (field->flags & HID_MAIN_ITEM_NO_PREFERRED) &&
+	    (field->logical_minimum == 0) &&
+	    (field->logical_maximum == 1))
+		code = value ? code_on : code_off;
+
+	if (!code)
+		return false;
+
+	input_event(input, EV_MSC, MSC_SCAN, scan);
+	input_event(input, EV_KEY, code, 1);
+	input_sync(input);
+	input_event(input, EV_KEY, code, 0);
+
+	return true;
+}
+
+/**
+ * hidinput_handle_onoffs - Handles an OOC event if the HID usage type is OOC.
+ * @usage: HID usage to check.
+ * @field: HID field that corresponds to the event.
+ * @value: HID value that corresponds to the event.
+ *
+ * Returns: 1 if @usage is a supported on/off control (OOC), as defined by HID
+ *          Usage Tables 1.21 (3.4.1.2).
+ *          0 if @usage is not a supported OOC.
+ *          -EINVAL if @usage is not a valid OOC (@field is invalid).
+ */
+static int hidinput_handle_onoffs(struct hid_usage *usage, struct hid_field *field, __s32 value)
+{
+	u16 code_on, code_off;
+
+	if (!hidinput_get_onoff_keycodes(usage, &code_on, &code_off))
+		return 0;
+
+	if (!hidinput_handle_onoff(field, value, usage->hid, usage->code, code_on, code_off))
+		return -EINVAL;
+
+	return 1;
+}
+
 void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct hid_usage *usage, __s32 value)
 {
 	struct input_dev *input;
@@ -1438,6 +1568,16 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 	    value == field->value[usage->usage_index])
 		return;
 
+	switch (hidinput_handle_onoffs(usage, field, value)) {
+	case 1:
+		return;
+	case -EINVAL:
+		hid_warn_once(hid, "Invalid OOC usage: code %u, flags %#x, min %d, max %d\n",
+			      usage->code, field->flags,
+			      field->logical_minimum, field->logical_maximum);
+		return;
+	}
+
 	/* report the usage code as scancode if the key status has changed */
 	if (usage->type == EV_KEY &&
 	    (!test_bit(usage->code, input->key)) == value)
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index dd785a5b5076..d490de9ce7fe 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -518,6 +518,7 @@
 #define KEY_NOTIFICATION_CENTER	0x1bc	/* Show/hide the notification center */
 #define KEY_PICKUP_PHONE	0x1bd	/* Answer incoming call */
 #define KEY_HANGUP_PHONE	0x1be	/* Decline incoming call */
+#define KEY_TOGGLE_PHONE	0x1bf	/* Toggle phone hook */
 
 #define KEY_DEL_EOL		0x1c0
 #define KEY_DEL_EOS		0x1c1
@@ -660,6 +661,13 @@
 /* Select an area of screen to be copied */
 #define KEY_SELECTIVE_SCREENSHOT	0x27a
 
+/*
+ * In contrast to KEY_MICMUTE (that toggles the mute state), these set specific
+ * (on/off) states.
+ */
+#define KEY_MICMUTE_ON			0x280
+#define KEY_MICMUTE_OFF			0x281
+
 /*
  * Some keyboards have keys which do not have a defined meaning, these keys
  * are intended to be programmed / bound to macros by the user. For most
-- 
2.32.0

[PATCH 3/6] HID: plantronics: Expose headset LEDs

[Maxim Mikityanskiy]

hid-plantronics uses a custom input mapping, where unhandled usages get
ignored. Although these headsets have LEDs, they aren't handled in
plantronics_input_mapping, hence not exposed to the userspace. This
commit fixes it by adding a case for HID_UP_LED.

Tested with Plantronics Blackwire 3220 Series (047f:c056).

Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 drivers/hid/hid-plantronics.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hid/hid-plantronics.c b/drivers/hid/hid-plantronics.c
index e81b7cec2d12..ea056235a591 100644
--- a/drivers/hid/hid-plantronics.c
+++ b/drivers/hid/hid-plantronics.c
@@ -61,6 +61,10 @@ static int plantronics_input_mapping(struct hid_device *hdev,
 	if (field->application == HID_GD_JOYSTICK)
 		goto defaulted;
 
+	/* expose LEDs */
+	if ((usage->hid & HID_USAGE_PAGE) == HID_UP_LED)
+		goto defaulted;
+
 	/* handle volume up/down mapping */
 	/* non-standard types or multi-HID interfaces - plt_type is PID */
 	if (!(plt_type & HID_USAGE_PAGE)) {
-- 
2.32.0

[PATCH 4/6] HID: plantronics: Expose headset telephony buttons

[Maxim Mikityanskiy]

hid-plantronics uses a custom input mapping, where unhandled usages get
ignored. Although these headsets have telephony buttons (microphone mute
and answer/hangup), they are not handled in plantronics_input_mapping,
hence not exposed to the userspace. This commit fixes it by adding a
case for HID_UP_TELEPHONY to the "basic telephony compliant" devices.

Tested with Plantronics Blackwire 3220 Series (047f:c056).

Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 drivers/hid/hid-plantronics.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/hid-plantronics.c b/drivers/hid/hid-plantronics.c
index ea056235a591..19d6cddff86a 100644
--- a/drivers/hid/hid-plantronics.c
+++ b/drivers/hid/hid-plantronics.c
@@ -84,6 +84,8 @@ static int plantronics_input_mapping(struct hid_device *hdev,
 		 (plt_type & HID_USAGE) != PLT_BASIC_EXCEPTION) {
 		if (PLT_ALLOW_CONSUMER)
 			goto defaulted;
+		if ((usage->hid & HID_USAGE_PAGE) == HID_UP_TELEPHONY)
+			goto defaulted;
 	}
 	/* not 'basic telephony' - apply legacy mapping */
 	/* only map if the field is in the device's primary vendor page */
-- 
2.32.0

[PATCH 5/6] HID: hid-input: Update LEDs in all HID reports

[Maxim Mikityanskiy]

hidinput_led_worker is scheduled on a work queue to update all LEDs in a
batch. However, it uses hidinput_get_led_field which gets the first LED
field found, and updates only the report this field belongs to. There
are devices that expose multiple LEDs in multiple reports. The current
implementation of the worker fails to update some LEDs on such devices.

Plantronics Blackwire 3220 Series (047f:c056) is an example of such
device. Only mute LED works, but offhook and ring LEDs don't work.

This commit fixes hidinput_led_worker by making it go over all reports
that contain at least one LED field.

Fixes: 4371ea8202e9 ("HID: usbhid: defer LED setting to a workqueue")
Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 drivers/hid/hid-input.c | 41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 533a7f429a5f..29f59208b34c 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1663,22 +1663,29 @@ unsigned int hidinput_count_leds(struct hid_device *hid)
 }
 EXPORT_SYMBOL_GPL(hidinput_count_leds);
 
-static void hidinput_led_worker(struct work_struct *work)
+static bool hidinput_is_led_report(struct hid_report *report)
 {
-	struct hid_device *hid = container_of(work, struct hid_device,
-					      led_work);
 	struct hid_field *field;
-	struct hid_report *report;
+	int i, j;
+
+	for (i = 0; i < report->maxfield; i++) {
+		field = report->field[i];
+		for (j = 0; j < field->maxusage; j++)
+			if (field->usage[j].type == EV_LED)
+				return true;
+	}
+
+	return false;
+}
+
+static void hidinput_led_update(struct hid_device *hid, struct hid_report *report)
+{
 	int ret;
 	u32 len;
 	__u8 *buf;
 
-	field = hidinput_get_led_field(hid);
-	if (!field)
-		return;
-
 	/*
-	 * field->report is accessed unlocked regarding HID core. So there might
+	 * report is accessed unlocked regarding HID core. So there might
 	 * be another incoming SET-LED request from user-space, which changes
 	 * the LED state while we assemble our outgoing buffer. However, this
 	 * doesn't matter as hid_output_report() correctly converts it into a
@@ -1690,8 +1697,6 @@ static void hidinput_led_worker(struct work_struct *work)
 	 * correct value, guaranteed!
 	 */
 
-	report = field->report;
-
 	/* use custom SET_REPORT request if possible (asynchronous) */
 	if (hid->ll_driver->request)
 		return hid->ll_driver->request(hid, report, HID_REQ_SET_REPORT);
@@ -1711,6 +1716,20 @@ static void hidinput_led_worker(struct work_struct *work)
 	kfree(buf);
 }
 
+static void hidinput_led_worker(struct work_struct *work)
+{
+	struct hid_device *hid = container_of(work, struct hid_device,
+					      led_work);
+	struct hid_report *report;
+
+	list_for_each_entry(report,
+			    &hid->report_enum[HID_OUTPUT_REPORT].report_list,
+			    list) {
+		if (hidinput_is_led_report(report))
+			hidinput_led_update(hid, report);
+	}
+}
+
 static int hidinput_input_event(struct input_dev *dev, unsigned int type,
 				unsigned int code, int value)
 {
-- 
2.32.0

[PATCH 6/6] HID: jabra: Change mute LED state to avoid missing key press events

[Maxim Mikityanskiy]

Jabra devices use their discretion regarding when to send the mute key
press events. Although every press on the mute button changes the LED
and actual mute state, key press events are only generated in the
offhook state and only if the mute state set by the host matches the
mute state of the headset.

Without the host's help, every second mute key press will be missed.
This patch addresses it by making the driver update the mute state every
time the mute button is pressed.

Tested with GN Netcom Jabra EVOLVE 20 MS (0b0e:0300). If some other
Jabra device doesn't suffer from this behavior, this workaround
shouldn't hurt.

Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 drivers/hid/hid-jabra.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/hid/hid-jabra.c b/drivers/hid/hid-jabra.c
index 41dc30fe2d16..818c174cd544 100644
--- a/drivers/hid/hid-jabra.c
+++ b/drivers/hid/hid-jabra.c
@@ -37,16 +37,51 @@ static int jabra_input_mapping(struct hid_device *hdev,
 	return is_vendor_defined ? -1 : 0;
 }
 
+static int jabra_event(struct hid_device *hdev, struct hid_field *field,
+		       struct hid_usage *usage, __s32 value)
+{
+	struct hid_field *mute_led_field;
+	int offset;
+
+	/* Usages are filtered in jabra_usages. */
+
+	if (!value) /* Handle key presses only. */
+		return 0;
+
+	offset = hidinput_find_field(hdev, EV_LED, LED_MUTE, &mute_led_field);
+	if (offset == -1)
+		return 0; /* No mute LED, proceed. */
+
+	/*
+	 * The device changes the LED state automatically on the mute key press,
+	 * however, it still expects the host to change the LED state. If there
+	 * is a mismatch (i.e. the host didn't change the LED state), the next
+	 * mute key press won't generate an event. To avoid missing every second
+	 * mute key press, change the LED state here.
+	 */
+	input_event(mute_led_field->hidinput->input, EV_LED, LED_MUTE,
+		    !mute_led_field->value[offset]);
+
+	return 0;
+}
+
 static const struct hid_device_id jabra_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_JABRA, HID_ANY_ID) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, jabra_devices);
 
+static const struct hid_usage_id jabra_usages[] = {
+	{ 0x000b002f, EV_KEY, HID_ANY_ID }, /* Mic mute */
+	{ HID_TERMINATOR, HID_TERMINATOR, HID_TERMINATOR }
+};
+
 static struct hid_driver jabra_driver = {
 	.name = "jabra",
 	.id_table = jabra_devices,
+	.usage_table = jabra_usages,
 	.input_mapping = jabra_input_mapping,
+	.event = jabra_event,
 };
 module_hid_driver(jabra_driver);
 
-- 
2.32.0