LinuxLists.cc - [PATCH] HID: logitech-dj: allow mice to use all types of reports

2024-01-28 21:49:53

Subject: [PATCH] HID: logitech-dj: allow mice to use all types of reports

You can bind whatever action you want to the mouse's reprogrammable
buttons using Windows application. Allow Linux to receive those keycodes.

Signed-off-by: Yaraslau Furman <[email protected]>
---
drivers/hid/hid-logitech-dj.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index e6a8b6d8eab7..5730be1aa41c 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -948,11 +948,12 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
HIDPP_DEVICE_TYPE_MASK;
workitem->quad_id_msb = hidpp_report->params[HIDPP_PARAM_EQUAD_MSB];
workitem->quad_id_lsb = hidpp_report->params[HIDPP_PARAM_EQUAD_LSB];
+ workitem->reports_supported |= STD_KEYBOARD | MULTIMEDIA |
+ POWER_KEYS | MEDIA_CENTER |
+ HIDPP;
+
switch (workitem->device_type) {
case REPORT_TYPE_KEYBOARD:
- workitem->reports_supported |= STD_KEYBOARD | MULTIMEDIA |
- POWER_KEYS | MEDIA_CENTER |
- HIDPP;
id = (workitem->quad_id_msb << 8) | workitem->quad_id_lsb;
for (i = 0; i < ARRAY_SIZE(kbd_builtin_touchpad_ids); i++) {
if (id == kbd_builtin_touchpad_ids[i]) {
@@ -965,9 +966,6 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
}
break;
case REPORT_TYPE_MOUSE:
- workitem->reports_supported |= STD_MOUSE | HIDPP;
- if (djrcv_dev->type == recvr_type_mouse_only)
- workitem->reports_supported |= MULTIMEDIA;
break;
}
}
--
2.43.0

2024-01-30 11:51:37

by Yaroslav Furman

[permalink] [raw]

Subject: [PATCH v2] HID: logitech-dj: allow mice to report multimedia keycodes

Multimedia buttons can be bound to the mouse's extra keys in Windows application.
Let Linux receive those keycodes.

Signed-off-by: Yaraslau Furman <[email protected]>
---
drivers/hid/hid-logitech-dj.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index e6a8b6d8eab7..3c3c497b6b91 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -965,9 +965,7 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
}
break;
case REPORT_TYPE_MOUSE:
- workitem->reports_supported |= STD_MOUSE | HIDPP;
- if (djrcv_dev->type == recvr_type_mouse_only)
- workitem->reports_supported |= MULTIMEDIA;
+ workitem->reports_supported |= STD_MOUSE | HIDPP | MULTIMEDIA;
break;
}
}
--
2.43.0

2024-02-22 10:18:58

by Yaroslav Furman

[permalink] [raw]

Subject: [PATCH v2] HID: logitech-dj: allow mice to report multimedia keycodes

2024-04-03 11:38:23

by Jiri Kosina

[permalink] [raw]

Subject: Re: [PATCH v2] HID: logitech-dj: allow mice to report multimedia keycodes

On Thu, 22 Feb 2024, Yaraslau Furman wrote:

> Multimedia buttons can be bound to the mouse's extra keys in Windows application.
> Let Linux receive those keycodes.
>
> Signed-off-by: Yaraslau Furman <[email protected]>
> ---
> drivers/hid/hid-logitech-dj.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index e6a8b6d8eab7..3c3c497b6b91 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -965,9 +965,7 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
> }
> break;
> case REPORT_TYPE_MOUSE:
> - workitem->reports_supported |= STD_MOUSE | HIDPP;
> - if (djrcv_dev->type == recvr_type_mouse_only)
> - workitem->reports_supported |= MULTIMEDIA;
> + workitem->reports_supported |= STD_MOUSE | HIDPP | MULTIMEDIA;
> break;

CCing Hans who introduced this explicit check back in 3ed224e273ac
("HID: logitech-dj: Fix 064d:c52f receiver support").
Should it be made device-specific?

--
Jiri Kosina
SUSE Labs

2024-04-03 13:55:27

by Hans de Goede

[permalink] [raw]

Subject: Re: [PATCH v2] HID: logitech-dj: allow mice to report multimedia keycodes

Hi All,

On 4/3/24 1:38 PM, Jiri Kosina wrote:
> On Thu, 22 Feb 2024, Yaraslau Furman wrote:
>
>> Multimedia buttons can be bound to the mouse's extra keys in Windows application.
>> Let Linux receive those keycodes.
>>
>> Signed-off-by: Yaraslau Furman <[email protected]>
>> ---
>> drivers/hid/hid-logitech-dj.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
>> index e6a8b6d8eab7..3c3c497b6b91 100644
>> --- a/drivers/hid/hid-logitech-dj.c
>> +++ b/drivers/hid/hid-logitech-dj.c
>> @@ -965,9 +965,7 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
>> }
>> break;
>> case REPORT_TYPE_MOUSE:
>> - workitem->reports_supported |= STD_MOUSE | HIDPP;
>> - if (djrcv_dev->type == recvr_type_mouse_only)
>> - workitem->reports_supported |= MULTIMEDIA;
>> + workitem->reports_supported |= STD_MOUSE | HIDPP | MULTIMEDIA;
>> break;
>
> CCing Hans who introduced this explicit check back in 3ed224e273ac
> ("HID: logitech-dj: Fix 064d:c52f receiver support").
> Should it be made device-specific?

Jiri, thank you for bringing this to my attention. If mice attached
to other receivers can send multi-media key presses too, then I believe
that dropping the if (djrcv_dev->type == recvr_type_mouse_only) check
is fine.

IOW, the patch looks good to me:

Reviewed-by: Hans de Goede <[email protected]>

Regards,

Hans

2024-04-03 15:06:01

by Yaroslav Furman

[permalink] [raw]

Subject: [PATCH v2] HID: logitech-dj: allow mice to report multimedia keycodes

From: Yaraslau Furman <[email protected]>

So, I think this should work fine for now, but IMO we should see if it's
possible to allow the driver to receive all events that a keyboard can send,
since Windows application can assign pretty much any keycode to any mouse
button (perhaps not the main ones though), and then we might see the same situation again.

I tried doing that on my system but the driver stopped working completely.
---
drivers/hid/hid-logitech-dj.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index e6a8b6d8eab7..3c3c497b6b91 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -965,9 +965,7 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
}
break;
case REPORT_TYPE_MOUSE:
- workitem->reports_supported |= STD_MOUSE | HIDPP;
- if (djrcv_dev->type == recvr_type_mouse_only)
- workitem->reports_supported |= MULTIMEDIA;
+ workitem->reports_supported |= STD_MOUSE | HIDPP | MULTIMEDIA;
break;
}
}
--
2.44.0

2024-04-03 15:09:54

by Yaroslav Furman

[permalink] [raw]

Subject: [PATCH v2] HID: logitech-dj: allow mice to report multimedia keycodes

2024-04-03 15:38:52

by Hans de Goede

[permalink] [raw]

Subject: Re: [PATCH v2] HID: logitech-dj: allow mice to report multimedia keycodes

Hi Yaroslav,

On 4/3/24 5:06 PM, Yaroslav Furman wrote:
> From: Yaraslau Furman <[email protected]>
>
> So, I think this should work fine for now, but IMO we should see if it's
> possible to allow the driver to receive all events that a keyboard can send,
> since Windows application can assign pretty much any keycode to any mouse
> button (perhaps not the main ones though), and then we might see the same situation again.
>
> I tried doing that on my system but the driver stopped working completely.

The actual code change itself looks fine, but the above should have gone
into a cover-letter or something like that.

This is not a proper commit message and this is also lacking
your Signed-off-by (which you did correctly include in your
original version:

https://lore.kernel.org/linux-input/[email protected]/

Please send a v3 with a proper commit-msg + Signed-off-by.

Regards,

Hans

> ---
> drivers/hid/hid-logitech-dj.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index e6a8b6d8eab7..3c3c497b6b91 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -965,9 +965,7 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
> }
> break;
> case REPORT_TYPE_MOUSE:
> - workitem->reports_supported |= STD_MOUSE | HIDPP;
> - if (djrcv_dev->type == recvr_type_mouse_only)
> - workitem->reports_supported |= MULTIMEDIA;
> + workitem->reports_supported |= STD_MOUSE | HIDPP | MULTIMEDIA;
> break;
> }
> }

2024-04-03 16:55:00

by Yaroslav Furman

[permalink] [raw]

Subject: [PATCH v3] HID: logitech-dj: allow mice to use all types of reports

From: Yaraslau Furman <[email protected]>

You can bind whatever action you want to the mouse's reprogrammable
buttons using Windows application. Allow Linux to receive multimedia keycodes.

Signed-off-by: Yaraslau Furman <[email protected]>
---
drivers/hid/hid-logitech-dj.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index e6a8b6d8eab7..3c3c497b6b91 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -965,9 +965,7 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
}
break;
case REPORT_TYPE_MOUSE:
- workitem->reports_supported |= STD_MOUSE | HIDPP;
- if (djrcv_dev->type == recvr_type_mouse_only)
- workitem->reports_supported |= MULTIMEDIA;
+ workitem->reports_supported |= STD_MOUSE | HIDPP | MULTIMEDIA;
break;
}
}
--
2.44.0

2024-04-03 16:56:42

by Hans de Goede

[permalink] [raw]

Subject: Re: [PATCH v3] HID: logitech-dj: allow mice to use all types of reports

Hi,

On 4/3/24 6:54 PM, Yaroslav Furman wrote:
> From: Yaraslau Furman <[email protected]>
>
> You can bind whatever action you want to the mouse's reprogrammable
> buttons using Windows application. Allow Linux to receive multimedia keycodes.
>
> Signed-off-by: Yaraslau Furman <[email protected]>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <[email protected]>

Regards,

Hans

> ---
> drivers/hid/hid-logitech-dj.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index e6a8b6d8eab7..3c3c497b6b91 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -965,9 +965,7 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
> }
> break;
> case REPORT_TYPE_MOUSE:
> - workitem->reports_supported |= STD_MOUSE | HIDPP;
> - if (djrcv_dev->type == recvr_type_mouse_only)
> - workitem->reports_supported |= MULTIMEDIA;
> + workitem->reports_supported |= STD_MOUSE | HIDPP | MULTIMEDIA;
> break;
> }
> }

2024-04-03 17:50:41

by Jiri Kosina

[permalink] [raw]

Subject: Re: [PATCH v3] HID: logitech-dj: allow mice to use all types of reports

On Wed, 3 Apr 2024, Hans de Goede wrote:

> > From: Yaraslau Furman <[email protected]>
> >
> > You can bind whatever action you want to the mouse's reprogrammable
> > buttons using Windows application. Allow Linux to receive multimedia keycodes.
> >
> > Signed-off-by: Yaraslau Furman <[email protected]>
>
> Thanks, patch looks good to me:
>
> Reviewed-by: Hans de Goede <[email protected]>

Thanks to both of you. I've added

Fixes: 3ed224e273ac ("HID: logitech-dj: Fix 064d:c52f receiver support")

and applied.

--
Jiri Kosina
SUSE Labs