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

Yaraslau Furman posted 1 patch 1 year, 10 months ago
There is a newer version of this series
drivers/hid/hid-logitech-dj.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
[PATCH v2] HID: logitech-dj: allow mice to report multimedia keycodes
Posted by Yaraslau Furman 1 year, 10 months ago
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
Posted by Yaraslau Furman 1 year, 10 months ago
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
Posted by Jiri Kosina 1 year, 8 months ago
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
Posted by Hans de Goede 1 year, 8 months ago
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
Posted by Yaroslav Furman 1 year, 8 months ago
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
Posted by Hans de Goede 1 year, 8 months ago
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
Posted by Yaroslav Furman 1 year, 8 months ago
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
Posted by Hans de Goede 1 year, 8 months ago
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
Posted by Jiri Kosina 1 year, 8 months ago
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
[PATCH v2] HID: logitech-dj: allow mice to report multimedia keycodes
Posted by Yaroslav Furman 1 year, 8 months ago
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