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

[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