[PATCH] HID: Driver for Google Hangouts Meet Speakermic

[PATCH] HID: Driver for Google Hangouts Meet Speakermic

[Pablo Ceballos]

This driver works around a problem with the HID usage sent by this
device for the mute button. It prevents key events from being generated
for that HID usage since they would be incorrect.

Signed-off-by: Pablo Ceballos <pceballos@google.com>
---
 drivers/hid/Kconfig            | 12 ++++++++
 drivers/hid/Makefile           |  1 +
 drivers/hid/hid-google-atrus.c | 55 ++++++++++++++++++++++++++++++++++
 drivers/hid/hid-ids.h          |  1 +
 4 files changed, 69 insertions(+)
 create mode 100644 drivers/hid/hid-google-atrus.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index f5544157576c..d4b6be827d15 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -403,6 +403,18 @@ config HOLTEK_FF
 	  Say Y here if you have a Holtek On Line Grip based game controller
 	  and want to have force feedback support for it.
 
+config HID_GOOGLE_ATRUS
+	tristate "Google Hangouts Meet Speakermic"
+	depends on USB_HID
+	help
+	This selects a driver for the Google Hangouts Meet Speakermic.
+
+	This driver works around a problem with the HID usage sent by this
+	device for the mute button. It prevents key events from being generated
+	for that HID usage since they would be incorrect.
+
+	Say Y here if you have a Google Hangouts Meet Speakermic.
+
 config HID_GOOGLE_HAMMER
 	tristate "Google Hammer Keyboard"
 	depends on USB_HID && LEDS_CLASS && CROS_EC
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 6d3e630e81af..2ee446b5b953 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_HID_FT260)		+= hid-ft260.o
 obj-$(CONFIG_HID_GEMBIRD)	+= hid-gembird.o
 obj-$(CONFIG_HID_GFRM)		+= hid-gfrm.o
 obj-$(CONFIG_HID_GLORIOUS)  += hid-glorious.o
+obj-$(CONFIG_HID_GOOGLE_ATRUS)  += hid-google-atrus.o
 obj-$(CONFIG_HID_GOOGLE_HAMMER)	+= hid-google-hammer.o
 obj-$(CONFIG_HID_VIVALDI)	+= hid-vivaldi.o
 obj-$(CONFIG_HID_GT683R)	+= hid-gt683r.o
diff --git a/drivers/hid/hid-google-atrus.c b/drivers/hid/hid-google-atrus.c
new file mode 100644
index 000000000000..e136c70e9425
--- /dev/null
+++ b/drivers/hid/hid-google-atrus.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  HID driver for Google Hangouts Meet Speakermic
+ *
+ *  Copyright 2022 Google LLC.
+ */
+
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+/*
+ * This driver handles the telephony phone mute HID usage by ignoring it. This
+ * avoids the default handling by the hid-input driver which is to map this to
+ * a KEY_MICMUTE event. The issue is that this device implements the phone mute
+ * HID usage as a toggle switch, where 1 indicates muted, and 0 indicates
+ * unmuted. However, for an EV_KEY event 1 indicates the key has been pressed
+ * and 0 indicates it has been released.
+ */
+
+static int atrus_event(struct hid_device *hid, struct hid_field *field,
+		       struct hid_usage *usage, __s32 value)
+{
+	/*
+	 * Return 1 to indicate no further processing should be done for this
+	 * usage.
+	 */
+	return 1;
+}
+
+static const struct hid_device_id atrus_devices[] = {
+	{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
+		     USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_ATRUS) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, atrus_devices);
+
+static const struct hid_usage_id atrus_usages[] = {
+	/* Handle only the Telephony Phone Mute usage. */
+	{ HID_UP_TELEPHONY | 0x2f, EV_KEY, HID_ANY_ID },
+	{ HID_TERMINATOR, HID_TERMINATOR, HID_TERMINATOR }
+};
+
+static struct hid_driver atrus_driver = {
+	.name = "atrus",
+	.id_table = atrus_devices,
+	.usage_table = atrus_usages,
+	.event = atrus_event,
+};
+module_hid_driver(atrus_driver);
+
+MODULE_AUTHOR("Pablo Ceballos <pcebalos@google.com>");
+MODULE_DESCRIPTION("Google Hangouts Meet Speakermic USB HID Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 85975031389b..9f6fc5cfbeb9 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -506,6 +506,7 @@
 #define USB_DEVICE_ID_GOOGLE_MOONBALL	0x5044
 #define USB_DEVICE_ID_GOOGLE_DON	0x5050
 #define USB_DEVICE_ID_GOOGLE_EEL	0x5057
+#define USB_DEVICE_ID_GOOGLE_ATRUS	0x8001
 
 #define USB_VENDOR_ID_GOTOP		0x08f2
 #define USB_DEVICE_ID_SUPER_Q2		0x007f
-- 
2.35.1.1094.g7c7d902a7c-goog

Re: [PATCH] HID: Driver for Google Hangouts Meet Speakermic

[Pablo Ceballos]

On Tue, Apr 5, 2022 at 11:39 AM Pablo Ceballos <pceballos@google.com> wrote:
>
> This driver works around a problem with the HID usage sent by this
> device for the mute button. It prevents key events from being generated
> for that HID usage since they would be incorrect.
>
> Signed-off-by: Pablo Ceballos <pceballos@google.com>

Following up on this patch request. Please take a look.

Re: [PATCH] HID: Driver for Google Hangouts Meet Speakermic

[Jiri Kosina]

On Tue, 5 Apr 2022, Pablo Ceballos wrote:

> This driver works around a problem with the HID usage sent by this
> device for the mute button. It prevents key events from being generated
> for that HID usage since they would be incorrect.
> 
> Signed-off-by: Pablo Ceballos <pceballos@google.com>

Applied to hid.git#for-5.19/google. Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: Driver for Google Hangouts Meet Speakermic

[Dmitry Torokhov]

On Thu, Apr 21, 2022 at 10:28 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Tue, 5 Apr 2022, Pablo Ceballos wrote:
>
> > This driver works around a problem with the HID usage sent by this
> > device for the mute button. It prevents key events from being generated
> > for that HID usage since they would be incorrect.
> >
> > Signed-off-by: Pablo Ceballos <pceballos@google.com>
>
> Applied to hid.git#for-5.19/google. Thanks,

I am curious, could not this be achieved without a kernel driver by
simply using udev to map this usage code to KEY_RESERVED?

Thanks.

-- 
Dmitry

Re: [PATCH] HID: Driver for Google Hangouts Meet Speakermic

[Jiri Kosina]

On Thu, 12 May 2022, Dmitry Torokhov wrote:

> > > This driver works around a problem with the HID usage sent by this
> > > device for the mute button. It prevents key events from being generated
> > > for that HID usage since they would be incorrect.
> > >
> > > Signed-off-by: Pablo Ceballos <pceballos@google.com>
> >
> > Applied to hid.git#for-5.19/google. Thanks,
> 
> I am curious, could not this be achieved without a kernel driver by
> simply using udev to map this usage code to KEY_RESERVED?

Hmm, good point, using KEY_RESERVED mapping to achieve the key being 
actually ignored didn't immediately occur to me.

Pablo, could you please verify that it behaves in the expected way, and 
confirm that we could drop the 'driver' in favor of udev rule?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: Driver for Google Hangouts Meet Speakermic

[Pablo Ceballos]

On Thu, May 12, 2022 at 3:53 AM Jiri Kosina <jikos@kernel.org> wrote:
> On Thu, 12 May 2022, Dmitry Torokhov wrote:
> > I am curious, could not this be achieved without a kernel driver by
> > simply using udev to map this usage code to KEY_RESERVED?
>
> Hmm, good point, using KEY_RESERVED mapping to achieve the key being
> actually ignored didn't immediately occur to me.
>
> Pablo, could you please verify that it behaves in the expected way, and
> confirm that we could drop the 'driver' in favor of udev rule?

I think I've achieved the same result by adding the following to udev
hwdb. Dmitry, is this what you had in mind, or is there a better way
of doing this?

evdev:input:b0003v18D1p8001*
 KEYBOARD_KEY_b002f=reserved

Pablo

Re: [PATCH] HID: Driver for Google Hangouts Meet Speakermic

[Dmitry Torokhov]

On Thu, May 12, 2022 at 03:35:00PM -0700, Pablo Ceballos wrote:
> On Thu, May 12, 2022 at 3:53 AM Jiri Kosina <jikos@kernel.org> wrote:
> > On Thu, 12 May 2022, Dmitry Torokhov wrote:
> > > I am curious, could not this be achieved without a kernel driver by
> > > simply using udev to map this usage code to KEY_RESERVED?
> >
> > Hmm, good point, using KEY_RESERVED mapping to achieve the key being
> > actually ignored didn't immediately occur to me.
> >
> > Pablo, could you please verify that it behaves in the expected way, and
> > confirm that we could drop the 'driver' in favor of udev rule?
> 
> I think I've achieved the same result by adding the following to udev
> hwdb. Dmitry, is this what you had in mind, or is there a better way
> of doing this?
> 
> evdev:input:b0003v18D1p8001*
>  KEYBOARD_KEY_b002f=reserved

No, that is exactly what I had in mind, thank you. Please submit this
entry to upstream systemd/udev project (and we can cherry-pick it into
our udev as well).

In general I think we should try to avoid trivial "fixup" HID drivers if
it is possible. I also wondered if we could be supplying fixed-up HID
descriptors via request_firmware() for HID devices.

Thanks.

-- 
Dmitry

Re: [PATCH] HID: Driver for Google Hangouts Meet Speakermic

[Pablo Ceballos]

On Thu, May 12, 2022 at 3:48 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Thu, May 12, 2022 at 03:35:00PM -0700, Pablo Ceballos wrote:
> > On Thu, May 12, 2022 at 3:53 AM Jiri Kosina <jikos@kernel.org> wrote:
> > > On Thu, 12 May 2022, Dmitry Torokhov wrote:
> > > > I am curious, could not this be achieved without a kernel driver by
> > > > simply using udev to map this usage code to KEY_RESERVED?
> > >
> > > Hmm, good point, using KEY_RESERVED mapping to achieve the key being
> > > actually ignored didn't immediately occur to me.
> > >
> > > Pablo, could you please verify that it behaves in the expected way, and
> > > confirm that we could drop the 'driver' in favor of udev rule?
Jiri, this driver can be dropped from 5.19. The udev rule works just as well.

> >
> > I think I've achieved the same result by adding the following to udev
> > hwdb. Dmitry, is this what you had in mind, or is there a better way
> > of doing this?
> >
> > evdev:input:b0003v18D1p8001*
> >  KEYBOARD_KEY_b002f=reserved
>
> No, that is exactly what I had in mind, thank you. Please submit this
> entry to upstream systemd/udev project (and we can cherry-pick it into
> our udev as well).
The pull request is here: https://github.com/systemd/systemd/pull/23372

>
> In general I think we should try to avoid trivial "fixup" HID drivers if
> it is possible. I also wondered if we could be supplying fixed-up HID
> descriptors via request_firmware() for HID devices.

Pablo

Re: [PATCH] HID: Driver for Google Hangouts Meet Speakermic

[Benjamin Tissoires]

On Fri, May 13, 2022 at 12:48 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Thu, May 12, 2022 at 03:35:00PM -0700, Pablo Ceballos wrote:
> > On Thu, May 12, 2022 at 3:53 AM Jiri Kosina <jikos@kernel.org> wrote:
> > > On Thu, 12 May 2022, Dmitry Torokhov wrote:
> > > > I am curious, could not this be achieved without a kernel driver by
> > > > simply using udev to map this usage code to KEY_RESERVED?
> > >
> > > Hmm, good point, using KEY_RESERVED mapping to achieve the key being
> > > actually ignored didn't immediately occur to me.
> > >
> > > Pablo, could you please verify that it behaves in the expected way, and
> > > confirm that we could drop the 'driver' in favor of udev rule?
> >
> > I think I've achieved the same result by adding the following to udev
> > hwdb. Dmitry, is this what you had in mind, or is there a better way
> > of doing this?
> >
> > evdev:input:b0003v18D1p8001*
> >  KEYBOARD_KEY_b002f=reserved
>
> No, that is exactly what I had in mind, thank you. Please submit this
> entry to upstream systemd/udev project (and we can cherry-pick it into
> our udev as well).
>
> In general I think we should try to avoid trivial "fixup" HID drivers if
> it is possible. I also wondered if we could be supplying fixed-up HID
> descriptors via request_firmware() for HID devices.

Just FYI, in case you haven't noticed it :)
I am currently working on supporting exactly this kind of fixups
through eBPF. I had in the past a request_firmware() patch for wacom
devices, but there were way too much problems raised by that because
you basically need to have the API right from the first attempt.

With eBPF, you can programmatically change the report descriptor for
this particular use case, or you could blindly load a new one. But the
other advantage that eBPF provides is that we can also change the
stream of events to accommodate for device mishaps, like a bouncing
button or garbage in the data.

Greg KH requested that we embed such fixups in the kernels directly,
and so I'm going to try to add that feature too. But we can also have
udev rules in userspace that would fix devices based on the context.
And the long term plan is also to convert existing simple fixup
drivers into eBPF progs that would be shipped by the kernel so we
don't lose functionalities.

Cheers,
Benjamin

>
> Thanks.
>
> --
> Dmitry
>

Re: [PATCH] HID: Driver for Google Hangouts Meet Speakermic

[Pavel Machek]

Hi!

> > No, that is exactly what I had in mind, thank you. Please submit this
> > entry to upstream systemd/udev project (and we can cherry-pick it into
> > our udev as well).
> >
> > In general I think we should try to avoid trivial "fixup" HID drivers if
> > it is possible. I also wondered if we could be supplying fixed-up HID
> > descriptors via request_firmware() for HID devices.
> 
> Just FYI, in case you haven't noticed it :)
> I am currently working on supporting exactly this kind of fixups
> through eBPF. I had in the past a request_firmware() patch for wacom

Is that good idea? eBPF is fairly dangerous thing, so I'd preffer
basic functionality not depend on it...

Best regards,
							Pavel

Re: [PATCH] HID: Driver for Google Hangouts Meet Speakermic

[Jiri Kosina]

On Sun, 22 May 2022, Pavel Machek wrote:

> Is that good idea? eBPF is fairly dangerous thing, so I'd preffer
> basic functionality not depend on it...

Although I understand the concern, that ship has already sailed long time 
ago.

-- 
Jiri Kosina
SUSE Labs