* [PATCH v6 1/2] HID: logitech: Add MX Master over Bluetooth
@ 2019-10-14 18:36 Mazin Rezk
2019-10-18 15:36 ` Benjamin Tissoires
0 siblings, 1 reply; 3+ messages in thread
From: Mazin Rezk @ 2019-10-14 18:36 UTC (permalink / raw)
To: linux-input
Cc: benjamin.tissoires, jikos, linux-kernel, Filipe Laíns, mnrzk
This patch adds support for the MX Master (b01e and b012) and also adds
foundational code for other Bluetooth LE HID++ devices to be added.
Some devices do not support short reports and thus have a quirk
(HIDPP_QUIRK_MISSING_SHORT_REPORTS) that forces short reports to be sent as
long reports. Since all Bluetooth LE HID++ devices seem to act this way,
HIDPP_QUIRK_CLASS_BLUETOOTH_LE aliases this quirk.
To allow for some space for future quirks, I changed the comment that
defines the bits reserved for classes from 2...20 to 2..15.
Signed-off-by: Mazin Rezk <mnrzk@protonmail.com>
---
drivers/hid/hid-logitech-hidpp.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 0179f7ed77e5..997b1056850a 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -59,7 +59,8 @@ MODULE_PARM_DESC(disable_tap_to_click,
#define HIDPP_QUIRK_CLASS_G920 BIT(3)
#define HIDPP_QUIRK_CLASS_K750 BIT(4)
-/* bits 2..20 are reserved for classes */
+/* bits 2..15 are reserved for classes */
+#define HIDPP_QUIRK_MISSING_SHORT_REPORTS BIT(20)
/* #define HIDPP_QUIRK_CONNECT_EVENTS BIT(21) disabled */
#define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS BIT(22)
#define HIDPP_QUIRK_NO_HIDINPUT BIT(23)
@@ -81,6 +82,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_DELAYED_INIT HIDPP_QUIRK_NO_HIDINPUT
#define HIDPP_CAPABILITY_HIDPP10_BATTERY BIT(0)
@@ -340,6 +343,12 @@ static int hidpp_send_rap_command_sync(struct hidpp_device *hidpp_dev,
struct hidpp_report *message;
int ret, max_count;
+ /* Force long reports on devices that do not support short reports */
+ if (hidpp_dev->quirks & HIDPP_QUIRK_MISSING_SHORT_REPORTS &&
+ report_id == REPORT_ID_HIDPP_SHORT)
+ report_id = REPORT_ID_HIDPP_LONG;
+
+
switch (report_id) {
case REPORT_ID_HIDPP_SHORT:
max_count = HIDPP_REPORT_SHORT_LENGTH - 4;
@@ -3482,6 +3491,12 @@ static bool hidpp_validate_report(struct hid_device *hdev, int id,
static bool hidpp_validate_device(struct hid_device *hdev)
{
+ struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+ /* Skip the short report check if the device does not support it */
+ if (hidpp->quirks & HIDPP_QUIRK_MISSING_SHORT_REPORTS)
+ return hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
+ HIDPP_REPORT_LONG_LENGTH, false);
+
return hidpp_validate_report(hdev, REPORT_ID_HIDPP_SHORT,
HIDPP_REPORT_SHORT_LENGTH, false) &&
hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
@@ -3773,6 +3788,13 @@ static const struct hid_device_id hidpp_devices[] = {
{ /* MX5500 keyboard over Bluetooth */
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb30b),
.driver_data = HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS },
+ { /* MX Master mouse over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb012),
+ .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
+ HIDPP_QUIRK_CLASS_BLUETOOTH_LE },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb01e),
+ .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
+ HIDPP_QUIRK_CLASS_BLUETOOTH_LE },
{}
};
--
2.23.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v6 1/2] HID: logitech: Add MX Master over Bluetooth
2019-10-14 18:36 [PATCH v6 1/2] HID: logitech: Add MX Master over Bluetooth Mazin Rezk
@ 2019-10-18 15:36 ` Benjamin Tissoires
2019-10-19 1:59 ` Mazin Rezk
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Tissoires @ 2019-10-18 15:36 UTC (permalink / raw)
To: Mazin Rezk; +Cc: linux-input, jikos, linux-kernel, Filipe Laíns
Hi Mazin,
On Mon, Oct 14, 2019 at 8:36 PM Mazin Rezk <mnrzk@protonmail.com> wrote:
>
> This patch adds support for the MX Master (b01e and b012) and also adds
> foundational code for other Bluetooth LE HID++ devices to be added.
>
> Some devices do not support short reports and thus have a quirk
> (HIDPP_QUIRK_MISSING_SHORT_REPORTS) that forces short reports to be sent as
> long reports. Since all Bluetooth LE HID++ devices seem to act this way,
> HIDPP_QUIRK_CLASS_BLUETOOTH_LE aliases this quirk.
>
> To allow for some space for future quirks, I changed the comment that
> defines the bits reserved for classes from 2...20 to 2..15.
>
> Signed-off-by: Mazin Rezk <mnrzk@protonmail.com>
> ---
> drivers/hid/hid-logitech-hidpp.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 0179f7ed77e5..997b1056850a 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -59,7 +59,8 @@ MODULE_PARM_DESC(disable_tap_to_click,
> #define HIDPP_QUIRK_CLASS_G920 BIT(3)
> #define HIDPP_QUIRK_CLASS_K750 BIT(4)
>
> -/* bits 2..20 are reserved for classes */
> +/* bits 2..15 are reserved for classes */
> +#define HIDPP_QUIRK_MISSING_SHORT_REPORTS BIT(20)
> /* #define HIDPP_QUIRK_CONNECT_EVENTS BIT(21) disabled */
> #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS BIT(22)
> #define HIDPP_QUIRK_NO_HIDINPUT BIT(23)
> @@ -81,6 +82,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_DELAYED_INIT HIDPP_QUIRK_NO_HIDINPUT
>
> #define HIDPP_CAPABILITY_HIDPP10_BATTERY BIT(0)
> @@ -340,6 +343,12 @@ static int hidpp_send_rap_command_sync(struct hidpp_device *hidpp_dev,
> struct hidpp_report *message;
> int ret, max_count;
>
> + /* Force long reports on devices that do not support short reports */
> + if (hidpp_dev->quirks & HIDPP_QUIRK_MISSING_SHORT_REPORTS &&
> + report_id == REPORT_ID_HIDPP_SHORT)
> + report_id = REPORT_ID_HIDPP_LONG;
Wouldn't it be faster to just store which report needs to be put here
in struct hidpp_device?
> +
> +
> switch (report_id) {
> case REPORT_ID_HIDPP_SHORT:
> max_count = HIDPP_REPORT_SHORT_LENGTH - 4;
> @@ -3482,6 +3491,12 @@ static bool hidpp_validate_report(struct hid_device *hdev, int id,
>
> static bool hidpp_validate_device(struct hid_device *hdev)
> {
> + struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> + /* Skip the short report check if the device does not support it */
> + if (hidpp->quirks & HIDPP_QUIRK_MISSING_SHORT_REPORTS)
> + return hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
> + HIDPP_REPORT_LONG_LENGTH, false);
> +
I just merged Andrey's report detection, which means you will need to
update this hunk:
https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/commit/?h=for-5.4/upstream-fixes&id=905d754c53a522aacf806ea1d3e7c929148c1910
The good thing, is that now you can simply auto-detect if the short
report is missing. If the returned report_length is null, you know
that the report is missing (and thus you can remember to set the
quirk/which report id is needed).
Cheers,
Benjamin
> return hidpp_validate_report(hdev, REPORT_ID_HIDPP_SHORT,
> HIDPP_REPORT_SHORT_LENGTH, false) &&
> hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
> @@ -3773,6 +3788,13 @@ static const struct hid_device_id hidpp_devices[] = {
> { /* MX5500 keyboard over Bluetooth */
> HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb30b),
> .driver_data = HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS },
> + { /* MX Master mouse over Bluetooth */
> + HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb012),
> + .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
> + HIDPP_QUIRK_CLASS_BLUETOOTH_LE },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb01e),
> + .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
> + HIDPP_QUIRK_CLASS_BLUETOOTH_LE },
> {}
> };
>
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v6 1/2] HID: logitech: Add MX Master over Bluetooth
2019-10-18 15:36 ` Benjamin Tissoires
@ 2019-10-19 1:59 ` Mazin Rezk
0 siblings, 0 replies; 3+ messages in thread
From: Mazin Rezk @ 2019-10-19 1:59 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input, jikos, linux-kernel, Filipe Laíns, mnrzk
On Friday, October 18, 2019 11:36 AM, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
> Hi Mazin,
>
> On Mon, Oct 14, 2019 at 8:36 PM Mazin Rezk mnrzk@protonmail.com wrote:
>
> > This patch adds support for the MX Master (b01e and b012) and also adds
> > foundational code for other Bluetooth LE HID++ devices to be added.
> > Some devices do not support short reports and thus have a quirk
> > (HIDPP_QUIRK_MISSING_SHORT_REPORTS) that forces short reports to be sent as
> > long reports. Since all Bluetooth LE HID++ devices seem to act this way,
> > HIDPP_QUIRK_CLASS_BLUETOOTH_LE aliases this quirk.
> > To allow for some space for future quirks, I changed the comment that
> > defines the bits reserved for classes from 2...20 to 2..15.
> >
> > Signed-off-by: Mazin Rezk mnrzk@protonmail.com
> >
> > -----------------------------------------------
> >
> > drivers/hid/hid-logitech-hidpp.c | 24 +++++++++++++++++++++++-
> > 1 file changed, 23 insertions(+), 1 deletion(-)
> > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> > index 0179f7ed77e5..997b1056850a 100644
> > --- a/drivers/hid/hid-logitech-hidpp.c
> > +++ b/drivers/hid/hid-logitech-hidpp.c
> > @@ -59,7 +59,8 @@ MODULE_PARM_DESC(disable_tap_to_click,
> > #define HIDPP_QUIRK_CLASS_G920 BIT(3)
> > #define HIDPP_QUIRK_CLASS_K750 BIT(4)
> > -/* bits 2..20 are reserved for classes /
> > +/ bits 2..15 are reserved for classes /
> > +#define HIDPP_QUIRK_MISSING_SHORT_REPORTS BIT(20)
> > / #define HIDPP_QUIRK_CONNECT_EVENTS BIT(21) disabled */#define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS BIT(22)
> > #define HIDPP_QUIRK_NO_HIDINPUT BIT(23)
> > @@ -81,6 +82,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_DELAYED_INIT HIDPP_QUIRK_NO_HIDINPUT
> > #define HIDPP_CAPABILITY_HIDPP10_BATTERY BIT(0)
> > @@ -340,6 +343,12 @@ static int hidpp_send_rap_command_sync(struct hidpp_device *hidpp_dev,
> > struct hidpp_report *message;
> > int ret, max_count;
> >
> > - /* Force long reports on devices that do not support short reports */
> >
> >
> > - if (hidpp_dev->quirks & HIDPP_QUIRK_MISSING_SHORT_REPORTS &&
> >
> >
> > - report_id == REPORT_ID_HIDPP_SHORT)
> >
> >
> > - report_id = REPORT_ID_HIDPP_LONG;
> >
> >
>
> Wouldn't it be faster to just store which report needs to be put here
> in struct hidpp_device?
>
> > -
> > - switch (report_id) {
> > case REPORT_ID_HIDPP_SHORT:
> > max_count = HIDPP_REPORT_SHORT_LENGTH - 4;
> >
> >
> >
> > @@ -3482,6 +3491,12 @@ static bool hidpp_validate_report(struct hid_device *hdev, int id,
> > static bool hidpp_validate_device(struct hid_device *hdev)
> > {
> >
> > - struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> >
> >
> > - /* Skip the short report check if the device does not support it */
> >
> >
> > - if (hidpp->quirks & HIDPP_QUIRK_MISSING_SHORT_REPORTS)
> >
> >
> > - return hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
> >
> >
> > - HIDPP_REPORT_LONG_LENGTH, false);
> >
> >
> > -
>
> I just merged Andrey's report detection, which means you will need to
> update this hunk:
> https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/commit/?h=for-5.4/upstream-fixes&id=905d754c53a522aacf806ea1d3e7c929148c1910
>
> The good thing, is that now you can simply auto-detect if the short
> report is missing. If the returned report_length is null, you know
> that the report is missing (and thus you can remember to set the
> quirk/which report id is needed).
>
> Cheers,
> Benjamin
Thank you, I'll try to rework this patch based on the changes introduced in
that commit.
Thanks,
Mazin
>
> > return hidpp_validate_report(hdev, REPORT_ID_HIDPP_SHORT,
> > HIDPP_REPORT_SHORT_LENGTH, false) &&
> > hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
> >
> >
> > @@ -3773,6 +3788,13 @@ static const struct hid_device_id hidpp_devices[] = {
> > { /* MX5500 keyboard over Bluetooth */
> > HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb30b),
> > .driver_data = HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS },
> >
> > - { /* MX Master mouse over Bluetooth */
> >
> >
> > - HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb012),
> >
> >
> > - .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
> >
> >
> > - HIDPP_QUIRK_CLASS_BLUETOOTH_LE },
> >
> >
> > - { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb01e),
> >
> >
> > - .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
> >
> >
> > - HIDPP_QUIRK_CLASS_BLUETOOTH_LE },
> > {}
> >
> >
> >
> > };
> > --
> > 2.23.0
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-19 1:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-14 18:36 [PATCH v6 1/2] HID: logitech: Add MX Master over Bluetooth Mazin Rezk
2019-10-18 15:36 ` Benjamin Tissoires
2019-10-19 1:59 ` Mazin Rezk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).