diff mbox series

HID: lg-g15: Do not fail the probe when we fail to disable F# emulation

Message ID 20200315173449.24857-1-hdegoede@redhat.com (mailing list archive)
State Mainlined
Commit b8a75eaddae9410767c7d95a1c5f3a547aae7b81
Delegated to: Jiri Kosina
Headers show
Series HID: lg-g15: Do not fail the probe when we fail to disable F# emulation | expand

Commit Message

Hans de Goede March 15, 2020, 5:34 p.m. UTC
By default the G1-G12 keys on the Logitech gaming keyboards send
F1 - F12 when in "generic HID" mode.

The first thing the hid-lg-g15 driver does is disable this behavior.

We have received a bugreport that this does not work when the keyboard
is connected through an Aten KVM switch. Using a gaming keyboard with
a KVM is a bit weird setup, but still we can try to fail a bit more
gracefully here.

On the G510 keyboards the same USB-interface which is used for the gaming
keys is also used for the media-keys. Before this commit we would call
hid_hw_stop() on failure to disable the F# emulation and then exit the
probe method with an error code.

This not only causes us to not handle the gaming-keys, but this also
breaks the media keys which is a regression compared to the situation
when these keyboards where handled by the generic hidinput driver.

This commit changes the error handling to clear the hiddev drvdata
(to disable our .raw_event handler) and then returning from the probe
method with success.

The net result of this is that, when connected through a KVM, things
work as well as they did before the hid-lg-g15 driver was introduced.

Fixes: ad4203f5a243 ("HID: lg-g15: Add support for the G510 keyboards' gaming keys")
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1806321
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-lg-g15.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Jiri Kosina March 20, 2020, 11:12 p.m. UTC | #1
On Sun, 15 Mar 2020, Hans de Goede wrote:

> By default the G1-G12 keys on the Logitech gaming keyboards send
> F1 - F12 when in "generic HID" mode.
> 
> The first thing the hid-lg-g15 driver does is disable this behavior.
> 
> We have received a bugreport that this does not work when the keyboard
> is connected through an Aten KVM switch. Using a gaming keyboard with
> a KVM is a bit weird setup, but still we can try to fail a bit more
> gracefully here.
> 
> On the G510 keyboards the same USB-interface which is used for the gaming
> keys is also used for the media-keys. Before this commit we would call
> hid_hw_stop() on failure to disable the F# emulation and then exit the
> probe method with an error code.
> 
> This not only causes us to not handle the gaming-keys, but this also
> breaks the media keys which is a regression compared to the situation
> when these keyboards where handled by the generic hidinput driver.
> 
> This commit changes the error handling to clear the hiddev drvdata
> (to disable our .raw_event handler) and then returning from the probe
> method with success.
> 
> The net result of this is that, when connected through a KVM, things
> work as well as they did before the hid-lg-g15 driver was introduced.
> 
> Fixes: ad4203f5a243 ("HID: lg-g15: Add support for the G510 keyboards' gaming keys")
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1806321
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/hid/hid-lg-g15.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-lg-g15.c b/drivers/hid/hid-lg-g15.c
> index 8a9268a5c66a..ad4b5412a9f4 100644
> --- a/drivers/hid/hid-lg-g15.c
> +++ b/drivers/hid/hid-lg-g15.c
> @@ -803,8 +803,10 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	}
>  
>  	if (ret < 0) {
> -		hid_err(hdev, "Error disabling keyboard emulation for the G-keys\n");
> -		goto error_hw_stop;
> +		hid_err(hdev, "Error %d disabling keyboard emulation for the G-keys, falling back to generic hid-input driver\n",
> +			ret);
> +		hid_set_drvdata(hdev, NULL);
> +		return 0;
>  	}
>  
>  	/* Get initial brightness levels */

This is now (for a few days actually, but seems like I forgot to send the 
e-mail) queued in for-5.6/upstream-fixes (but it might go in only for 5.7 
in case there is nothing urgent popping up in that branch in the coming 
days ... we'll see).

Thanks Hans,
diff mbox series

Patch

diff --git a/drivers/hid/hid-lg-g15.c b/drivers/hid/hid-lg-g15.c
index 8a9268a5c66a..ad4b5412a9f4 100644
--- a/drivers/hid/hid-lg-g15.c
+++ b/drivers/hid/hid-lg-g15.c
@@ -803,8 +803,10 @@  static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	if (ret < 0) {
-		hid_err(hdev, "Error disabling keyboard emulation for the G-keys\n");
-		goto error_hw_stop;
+		hid_err(hdev, "Error %d disabling keyboard emulation for the G-keys, falling back to generic hid-input driver\n",
+			ret);
+		hid_set_drvdata(hdev, NULL);
+		return 0;
 	}
 
 	/* Get initial brightness levels */