From: Hans de Goede <hdegoede@redhat.com>
To: Jiri Kosina <jikos@kernel.org>,
Benjamin Tissoires <benjamin.tissoires@redhat.com>,
Alexander Kobel <a-kobel@a-kobel.de>
Cc: "Hans de Goede" <hdegoede@redhat.com>,
"Pavel Machek" <pavel@ucw.cz>, "Marek Behún" <kabel@kernel.org>,
linux-input@vger.kernel.org, linux-leds@vger.kernel.org
Subject: [PATCH v2 resend 2/9] HID: lenovo: Fix lenovo_led_set_tp10ubkbd() error handling
Date: Sun, 4 Apr 2021 10:04:25 +0200 [thread overview]
Message-ID: <20210404080432.4322-3-hdegoede@redhat.com> (raw)
In-Reply-To: <20210404080432.4322-1-hdegoede@redhat.com>
Fix the following issues with lenovo_led_set_tp10ubkbd() error handling:
1. On success hid_hw_raw_request() returns the number of bytes sent.
So we should check for (ret != 3) rather then for (ret != 0).
2. Actually propagate errors to the caller.
3. Since the LEDs are part of an USB keyboard-dock the mute LEDs can go
away at any time. Don't log an error when ret == -ENODEV and set the
LED_HW_PLUGGABLE flag to avoid errors getting logged when the USB gets
disconnected.
Fixes: bc04b37ea0ec ("HID: lenovo: Add ThinkPad 10 Ultrabook Keyboard support")
Reviewed-by: Marek Behún <kabel@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
- Return -EIO when hid_hw_raw_request() returns a value != 3 and >= 0
Changes in v2:
- Rewrite to fix a bunch of other error-handling issues too
---
drivers/hid/hid-lenovo.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 4dc5e5f932ed..ee175ab54281 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -62,8 +62,8 @@ struct lenovo_drvdata {
#define TP10UBKBD_LED_OFF 1
#define TP10UBKBD_LED_ON 2
-static void lenovo_led_set_tp10ubkbd(struct hid_device *hdev, u8 led_code,
- enum led_brightness value)
+static int lenovo_led_set_tp10ubkbd(struct hid_device *hdev, u8 led_code,
+ enum led_brightness value)
{
struct lenovo_drvdata *data = hid_get_drvdata(hdev);
int ret;
@@ -75,10 +75,18 @@ static void lenovo_led_set_tp10ubkbd(struct hid_device *hdev, u8 led_code,
data->led_report[2] = value ? TP10UBKBD_LED_ON : TP10UBKBD_LED_OFF;
ret = hid_hw_raw_request(hdev, data->led_report[0], data->led_report, 3,
HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
- if (ret)
- hid_err(hdev, "Set LED output report error: %d\n", ret);
+ if (ret != 3) {
+ if (ret != -ENODEV)
+ hid_err(hdev, "Set LED output report error: %d\n", ret);
+
+ ret = ret < 0 ? ret : -EIO;
+ } else {
+ ret = 0;
+ }
mutex_unlock(&data->led_report_mutex);
+
+ return ret;
}
static void lenovo_tp10ubkbd_sync_fn_lock(struct work_struct *work)
@@ -349,7 +357,7 @@ static ssize_t attr_fn_lock_store(struct device *dev,
{
struct hid_device *hdev = to_hid_device(dev);
struct lenovo_drvdata *data = hid_get_drvdata(hdev);
- int value;
+ int value, ret;
if (kstrtoint(buf, 10, &value))
return -EINVAL;
@@ -364,7 +372,9 @@ static ssize_t attr_fn_lock_store(struct device *dev,
lenovo_features_set_cptkbd(hdev);
break;
case USB_DEVICE_ID_LENOVO_TP10UBKBD:
- lenovo_led_set_tp10ubkbd(hdev, TP10UBKBD_FN_LOCK_LED, value);
+ ret = lenovo_led_set_tp10ubkbd(hdev, TP10UBKBD_FN_LOCK_LED, value);
+ if (ret)
+ return ret;
break;
}
@@ -785,6 +795,7 @@ static int lenovo_led_brightness_set(struct led_classdev *led_cdev,
struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
u8 tp10ubkbd_led[] = { TP10UBKBD_MUTE_LED, TP10UBKBD_MICMUTE_LED };
int led_nr = 0;
+ int ret = 0;
if (led_cdev == &data_pointer->led_micmute)
led_nr = 1;
@@ -799,11 +810,11 @@ static int lenovo_led_brightness_set(struct led_classdev *led_cdev,
lenovo_led_set_tpkbd(hdev);
break;
case USB_DEVICE_ID_LENOVO_TP10UBKBD:
- lenovo_led_set_tp10ubkbd(hdev, tp10ubkbd_led[led_nr], value);
+ ret = lenovo_led_set_tp10ubkbd(hdev, tp10ubkbd_led[led_nr], value);
break;
}
- return 0;
+ return ret;
}
static int lenovo_register_leds(struct hid_device *hdev)
@@ -825,6 +836,7 @@ static int lenovo_register_leds(struct hid_device *hdev)
data->led_mute.name = name_mute;
data->led_mute.brightness_get = lenovo_led_brightness_get;
data->led_mute.brightness_set_blocking = lenovo_led_brightness_set;
+ data->led_mute.flags = LED_HW_PLUGGABLE;
data->led_mute.dev = &hdev->dev;
ret = led_classdev_register(&hdev->dev, &data->led_mute);
if (ret < 0)
@@ -833,6 +845,7 @@ static int lenovo_register_leds(struct hid_device *hdev)
data->led_micmute.name = name_micm;
data->led_micmute.brightness_get = lenovo_led_brightness_get;
data->led_micmute.brightness_set_blocking = lenovo_led_brightness_set;
+ data->led_micmute.flags = LED_HW_PLUGGABLE;
data->led_micmute.dev = &hdev->dev;
ret = led_classdev_register(&hdev->dev, &data->led_micmute);
if (ret < 0) {
--
2.30.2
next prev parent reply other threads:[~2021-04-04 8:04 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-04 8:04 [PATCH v2 resend 0/9] HID: lenovo: LED fixes and Thinkpad X1 Tablet kbd support Hans de Goede
2021-04-04 8:04 ` [PATCH v2 resend 1/9] HID: lenovo: Use brightness_set_blocking callback for setting LEDs brightness Hans de Goede
2021-04-04 8:04 ` Hans de Goede [this message]
2021-04-04 8:04 ` [PATCH v2 resend 3/9] HID: lenovo: Check hid_get_drvdata() returns non NULL in lenovo_event() Hans de Goede
2021-04-04 8:04 ` [PATCH v2 resend 4/9] HID: lenovo: Remove lenovo_led_brightness_get() Hans de Goede
2021-04-04 8:04 ` [PATCH v2 resend 5/9] HID: lenovo: Set LEDs max_brightness value Hans de Goede
2021-04-04 8:04 ` [PATCH v2 resend 6/9] HID: lenovo: Map mic-mute button to KEY_F20 instead of KEY_MICMUTE Hans de Goede
2021-04-04 8:04 ` [PATCH v2 resend 7/9] HID: lenovo: Set default_triggers for the mute and micmute LEDs Hans de Goede
2021-04-04 8:04 ` [PATCH v2 resend 8/9] HID: lenovo: Rework how the tp10ubkbd code decides which USB interface to use Hans de Goede
2021-04-04 8:04 ` [PATCH v2 resend 9/9] HID: lenovo: Add support for Thinkpad X1 Tablet Thin keyboard Hans de Goede
2021-04-07 12:56 ` [PATCH v2 resend 0/9] HID: lenovo: LED fixes and Thinkpad X1 Tablet kbd support Jiri Kosina
2021-04-07 13:42 ` Hans de Goede
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210404080432.4322-3-hdegoede@redhat.com \
--to=hdegoede@redhat.com \
--cc=a-kobel@a-kobel.de \
--cc=benjamin.tissoires@redhat.com \
--cc=jikos@kernel.org \
--cc=kabel@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=pavel@ucw.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.