Re: [PATCH v2] HID: google: modify HID device groups of eel

From: Stephen Boyd <swboyd@chromium.org>
To: Jiri Kosina <jikos@kernel.org>,
	Zhengqiao Xia <xiazhengqiao@huaqin.corp-partner.google.com>
Cc: benjamin.tissoires@redhat.com, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, dianders@chromium.org,
	Wei-Ning Huang <wnhuang@google.com>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Nicolas Boichat <drinkcat@chromium.org>,
	"Sean O'Brien" <seobrien@chromium.org>,
	phoenixshen@chromium.org
Subject: Re: [PATCH v2] HID: google: modify HID device groups of eel
Date: Fri, 14 Jan 2022 13:25:12 -0600	[thread overview]
Message-ID: <CAE-0n53M723sZ7H-f0SF=AoTrwznmTRhKPapgHe5H7Mw6bPb7Q@mail.gmail.com> (raw)
In-Reply-To: <nycvar.YFH.7.76.2201140935460.28059@cbobk.fhfr.pm>

Quoting Jiri Kosina (2022-01-14 00:38:23)
> On Fri, 7 Jan 2022, Zhengqiao Xia wrote:
>
> > If HID_GROUP of eel is set to HID_GROUP_GENERIC, Whiskers Tablet
> > Mode Switch of eel hammer will not be detected by system. when it
> > is set to HID_GROUP_VIVALDI, system will detect Whiskers Tablet
> > Mode Switch successfully.
> >
> > Signed-off-by: Zhengqiao Xia <xiazhengqiao@huaqin.corp-partner.google.com>
> > ---
> >  drivers/hid/hid-google-hammer.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
> > index 0403beb3104b..e5acd15f4a55 100644
> > --- a/drivers/hid/hid-google-hammer.c
> > +++ b/drivers/hid/hid-google-hammer.c
> > @@ -585,7 +585,7 @@ static void hammer_remove(struct hid_device *hdev)
> >  static const struct hid_device_id hammer_devices[] = {
> >       { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
> >                    USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_DON) },
> > -     { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
> > +     { HID_DEVICE(BUS_USB, HID_GROUP_VIVALDI,
> >                    USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_EEL) },
> >       { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>
> Color me confused, but anything with HID_GROUP_VIVALDI should be matched
> by hid-vivaldi driver, so what is this about?
>

My understanding is that 'vivaldi' is mostly a keyboard layout and
'hammer' is a detachable keyboard. We want to prevent the hid-vivaldi
driver from probing this particular device because the hid-vivaldi
driver doesn't know about detachable keyboards. Hammer devices also
support 360 degree wraparound so we know that the keyboard has been put
behind the screen or that it's being used to stand up the device on a
table.

Given all that, I'm still confused. If we make the hid-google-hammer
driver probe this device and the keyboard layout is vivaldi then we'd
want the part of the vivaldi driver that exposes the
function_row_physmap through sysfs. Otherwise userspace won't know how
to handle the function row properly. I think we need the device to stack
two drivers here. Does that happen with HID?