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

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

[Yaraslau Furman]

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 <yaro330@gmail.com>
---
 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

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

[Yaraslau Furman]

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

Signed-off-by: Yaraslau Furman <yaro330@gmail.com>
---
 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

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

[Yaraslau Furman]

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

Signed-off-by: Yaraslau Furman <yaro330@gmail.com>
---
 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

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

[Jiri Kosina]

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 <yaro330@gmail.com>
> ---
>  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

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

[Hans de Goede]

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 <yaro330@gmail.com>
>> ---
>>  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 <hdegoede@redhat.com>

Regards,

Hans

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

[Yaroslav Furman]

From: Yaraslau Furman <yaro330@gmail.com>

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

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

[Yaroslav Furman]

From: Yaraslau Furman <yaro330@gmail.com>

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

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

[Hans de Goede]

Hi Yaroslav,

On 4/3/24 5:06 PM, Yaroslav Furman wrote:
> From: Yaraslau Furman <yaro330@gmail.com>
> 
> 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/20240128214906.60606-1-yaro330@gmail.com/

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;
>  	}
>  }

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

[Yaroslav Furman]

From: Yaraslau Furman <yaro330@gmail.com>

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 <yaro330@gmail.com>
---
 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

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

[Hans de Goede]

Hi,

On 4/3/24 6:54 PM, Yaroslav Furman wrote:
> From: Yaraslau Furman <yaro330@gmail.com>
> 
> 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 <yaro330@gmail.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

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;
>  	}
>  }

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

[Jiri Kosina]

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

> > From: Yaraslau Furman <yaro330@gmail.com>
> > 
> > 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 <yaro330@gmail.com>
> 
> Thanks, patch looks good to me:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Thanks to both of you. I've added

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

and applied.

-- 
Jiri Kosina
SUSE Labs