| 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
| ||
|
Message-ID: <CAO-hwJK3Q1Bu+6JVbXmbZ2-JqaPzX=dEVPfue=v2LRJsU7FmGA@mail.gmail.com>
Date: Fri, 18 Oct 2019 17:38:14 +0200
From: Benjamin Tissoires <benjamin.tissoires@...hat.com>
To: Mazin Rezk <mnrzk@...tonmail.com>
Cc: "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
"jikos@...nel.org" <jikos@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Filipe Laíns <lains@...hlinux.org>
Subject: Re: [PATCH v6 2/2] HID: logitech: Support WirelessDeviceStatus
connect events
On Mon, Oct 14, 2019 at 8:36 PM Mazin Rezk <mnrzk@...tonmail.com> wrote:
>
> This patch allows WirelessDeviceStatus (0x1d4b) events to be detected as
> connection events in the hid-logitech-hidpp module.
>
> Devices with HIDPP_QUIRK_WIRELESS_DEVICE_STATUS use WirelessDeviceStatus
> instead of traditional connect events. Since all Bluetooth LE devices seem
> to act this way, HIDPP_QUIRK_CLASS_BLUETOOTH_LE aliases this quirk.
>
> Signed-off-by: Mazin Rezk <mnrzk@...tonmail.com>
> ---
> drivers/hid/hid-logitech-hidpp.c | 42 ++++++++++++++++++++++++++++----
> 1 file changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 997b1056850a..9b3df57ca857 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -60,6 +60,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
> #define HIDPP_QUIRK_CLASS_K750 BIT(4)
>
> /* bits 2..15 are reserved for classes */
> +#define HIDPP_QUIRK_WIRELESS_DEVICE_STATUS BIT(19)
> #define HIDPP_QUIRK_MISSING_SHORT_REPORTS BIT(20)
> /* #define HIDPP_QUIRK_CONNECT_EVENTS BIT(21) disabled */
> #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS BIT(22)
> @@ -82,7 +83,8 @@ MODULE_PARM_DESC(disable_tap_to_click,
> HIDPP_QUIRK_HI_RES_SCROLL_X2120 | \
> HIDPP_QUIRK_HI_RES_SCROLL_X2121)
>
> -#define HIDPP_QUIRK_CLASS_BLUETOOTH_LE HIDPP_QUIRK_MISSING_SHORT_REPORTS
> +#define HIDPP_QUIRK_CLASS_BLUETOOTH_LE (HIDPP_QUIRK_MISSING_SHORT_REPORTS | \
> + HIDPP_QUIRK_WIRELESS_DEVICE_STATUS)
>
> #define HIDPP_QUIRK_DELAYED_INIT HIDPP_QUIRK_NO_HIDINPUT
>
> @@ -189,6 +191,8 @@ struct hidpp_device {
>
> struct hidpp_battery battery;
> struct hidpp_scroll_counter vertical_wheel_counter;
> +
> + u8 wireless_feature_index;
> };
>
> /* HID++ 1.0 error codes */
> @@ -402,10 +406,14 @@ static inline bool hidpp_match_error(struct hidpp_report *question,
> (answer->fap.params[0] == question->fap.funcindex_clientid);
> }
>
> -static inline bool hidpp_report_is_connect_event(struct hidpp_report *report)
> +static inline bool hidpp_report_is_connect_event(struct hidpp_device *hidpp,
> + struct hidpp_report *report)
> {
> - return (report->report_id == REPORT_ID_HIDPP_SHORT) &&
> - (report->rap.sub_id == 0x41);
> + return ((hidpp->quirks & HIDPP_QUIRK_WIRELESS_DEVICE_STATUS) &&
> + (report->fap.feature_index == hidpp->wireless_feature_index)) ||
> + (((report->report_id == REPORT_ID_HIDPP_SHORT) ||
> + (hidpp->quirks & HIDPP_QUIRK_MISSING_SHORT_REPORTS)) &&
> + (report->rap.sub_id == 0x41));
I wonder if we need a quirk after all: if
hidpp->wireless_feature_index is non null, that means that we have the
quirk.
Unless the feature is present for non BLE devices, in which case, we
might want to enable them one by one, for now.
Cheers,
Benjamin
> }
>
> /**
> @@ -1282,6 +1290,24 @@ static int hidpp_battery_get_property(struct power_supply *psy,
> return ret;
> }
>
> +/* -------------------------------------------------------------------------- */
> +/* 0x1d4b: Wireless device status */
> +/* -------------------------------------------------------------------------- */
> +#define HIDPP_PAGE_WIRELESS_DEVICE_STATUS 0x1d4b
> +
> +static int hidpp_set_wireless_feature_index(struct hidpp_device *hidpp)
> +{
> + u8 feature_type;
> + int ret;
> +
> + ret = hidpp_root_get_feature(hidpp,
> + HIDPP_PAGE_WIRELESS_DEVICE_STATUS,
> + &hidpp->wireless_feature_index,
> + &feature_type);
> +
> + return ret;
> +}
> +
> /* -------------------------------------------------------------------------- */
> /* 0x2120: Hi-resolution scrolling */
> /* -------------------------------------------------------------------------- */
> @@ -3077,7 +3103,7 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
> }
> }
>
> - if (unlikely(hidpp_report_is_connect_event(report))) {
> + if (unlikely(hidpp_report_is_connect_event(hidpp, report))) {
> atomic_set(&hidpp->connected,
> !(report->rap.params[0] & (1 << 6)));
> if (schedule_work(&hidpp->work) == 0)
> @@ -3624,6 +3650,12 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
> hidpp_overwrite_name(hdev);
> }
>
> + if (connected && (hidpp->quirks & HIDPP_QUIRK_WIRELESS_DEVICE_STATUS)) {
> + ret = hidpp_set_wireless_feature_index(hidpp);
> + if (ret)
> + goto hid_hw_init_fail;
> + }
> +
> if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
> ret = wtp_get_config(hidpp);
> if (ret)
> --
> 2.23.0
>
Powered by blists - more mailing lists