linux-kernel - Re: [PATCH] HID: Add Wireless Radio Control feature for Chicony devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPpJ_edREMzaNMm=D7OPFLcP-MJASvvs9wDfF4qzbpjhcG9FVw@mail.gmail.com>
Date:   Wed, 23 Dec 2020 12:02:22 +0800
From:   Jian-Hong Pan <jhp@...lessos.org>
To:     Chris Chiu <chiu@...lessos.org>
Cc:     Jiri Kosina <jikos@...nel.org>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        linux-input <linux-input@...r.kernel.org>,
        Linux Kernel <linux-kernel@...r.kernel.org>,
        linux@...lessos.org
Subject: Re: [PATCH] HID: Add Wireless Radio Control feature for Chicony devices

Chris Chiu <chiu@...lessos.org> 於 2020年12月23日 週三 上午12:41寫道：
>
> On Tue, Dec 22, 2020 at 3:41 PM Jian-Hong Pan <jhp@...lessos.org> wrote:
> >
> > Some Chicony's keyboards support airplane mode hotkey (Fn+F2) with
> > "Wireless Radio Control" feature. For example, the wireless keyboard
> > [04f2:1236] shipped with ASUS all-in-one desktop.
> >
> > After consulting Chicony for this hotkey, learned the device will send
> > with 0x11 as the report ID and 0x1 as the value when the key is pressed
> > down.
> >
> > This patch maps the event as KEY_RFKILL.
> >
> > Signed-off-by: Jian-Hong Pan <jhp@...lessos.org>
> > ---
> >  drivers/hid/hid-chicony.c | 58 +++++++++++++++++++++++++++++++++++++++
> >  drivers/hid/hid-ids.h     |  1 +
> >  2 files changed, 59 insertions(+)
> >
> > diff --git a/drivers/hid/hid-chicony.c b/drivers/hid/hid-chicony.c
> > index 3f0ed6a95223..aca963aa0f1e 100644
> > --- a/drivers/hid/hid-chicony.c
> > +++ b/drivers/hid/hid-chicony.c
> > @@ -21,6 +21,42 @@
> >
> >  #include "hid-ids.h"
> >
> > +#define KEY_PRESSED                    0x01
> > +#define CH_WIRELESS_CTL_REPORT_ID      0x11
> > +
> > +static int ch_report_wireless(struct hid_report *report, u8 *data, int size)
> > +{
> > +       struct hid_device *hdev = report->device;
> > +       struct input_dev *input;
> > +
> > +       if (report->id != CH_WIRELESS_CTL_REPORT_ID ||
> > +           report->maxfield != 1 ||
> > +           *report->field[0]->value != KEY_PRESSED)
>
> Maybe replace this line with hid_check_keys_pressed() and the KEY_PRESSED
> is not required.

Thanks for your suggestion!

I tried hid_check_keys_pressed().  But, it always returns no key is
pressed in this case.
However, if the idea is: Since there is already a report, there must
be an event from the input.  So, the key press checking is duplicated.
This idea makes sense.  I will have a modification for this.

Thanks!
Jian-Hong Pan

> > +               return 0;
> > +
> > +       input = report->field[0]->hidinput->input;
> > +       if (!input) {
> > +               hid_warn(hdev, "can't find wireless radio control's input");
> > +               return 0;
> > +       }
> > +
> > +       input_report_key(input, KEY_RFKILL, 1);
> > +       input_sync(input);
> > +       input_report_key(input, KEY_RFKILL, 0);
> > +       input_sync(input);
> > +
> > +       return 1;
> > +}
> > +
> > +static int ch_raw_event(struct hid_device *hdev,
> > +               struct hid_report *report, u8 *data, int size)
> > +{
> > +       if (report->application == HID_GD_WIRELESS_RADIO_CTLS)
> > +               return ch_report_wireless(report, data, size);
> > +
> > +       return 0;
> > +}
> > +
> >  #define ch_map_key_clear(c)    hid_map_usage_clear(hi, usage, bit, max, \
> >                                         EV_KEY, (c))
> >  static int ch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> > @@ -77,10 +113,30 @@ static __u8 *ch_switch12_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> >         return rdesc;
> >  }
> >
> > +static int ch_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > +{
> > +       int ret;
> > +
> > +       hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
> > +       ret = hid_parse(hdev);
> > +       if (ret) {
> > +               hid_err(hdev, "Chicony hid parse failed: %d\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> > +       if (ret) {
> > +               hid_err(hdev, "Chicony hw start failed: %d\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       return 0;
> > +}
> >
> >  static const struct hid_device_id ch_devices[] = {
> >         { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_TACTICAL_PAD) },
> >         { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_WIRELESS2) },
> > +       { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_WIRELESS3) },
> >         { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_ACER_SWITCH12) },
> >         { }
> >  };
> > @@ -91,6 +147,8 @@ static struct hid_driver ch_driver = {
> >         .id_table = ch_devices,
> >         .report_fixup = ch_switch12_report_fixup,
> >         .input_mapping = ch_input_mapping,
> > +       .probe = ch_probe,
> > +       .raw_event = ch_raw_event,
> >  };
> >  module_hid_driver(ch_driver);
> >
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index 4c5f23640f9c..06d90301a3dc 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -270,6 +270,7 @@
> >  #define USB_DEVICE_ID_CHICONY_PIXART_USB_OPTICAL_MOUSE 0x1053
> >  #define USB_DEVICE_ID_CHICONY_PIXART_USB_OPTICAL_MOUSE2        0x0939
> >  #define USB_DEVICE_ID_CHICONY_WIRELESS2        0x1123
> > +#define USB_DEVICE_ID_CHICONY_WIRELESS3        0x1236
> >  #define USB_DEVICE_ID_ASUS_AK1D                0x1125
> >  #define USB_DEVICE_ID_CHICONY_TOSHIBA_WT10A    0x1408
> >  #define USB_DEVICE_ID_CHICONY_ACER_SWITCH12    0x1421
> > --
> > 2.29.2
> >