All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/9] HID: lenovo: Mute LED handling fixes and improvements
@ 2021-03-04 19:21 Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 1/9] HID: lenovo: Use brightness_set_blocking callback for setting LEDs brightness Hans de Goede
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Hans de Goede @ 2021-03-04 19:21 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Alexander Kobel
  Cc: Hans de Goede, linux-input

Hi All,

Here is v4 of my series with mute LED handling fixes and improvements
for the hid-lenovo driver.

This is the same as v3, with 2 new patches added. I'm sending this out as
a v4 because the 2 new patches depend on the previous patches.

Changes in v4:
- Add 2 new patches to add support for the mute-LEDs and special media-keys
  on the Thinkpad X1 Tablet Thin Keyboard

Changes in v3:
- Address the review-remarks from Marek Behún, thank you for all the
  reviews Marek.

Regards,

Hans


Hans de Goede (9):
  HID: lenovo: Use brightness_set_blocking callback for setting LEDs
    brightness
  HID: lenovo: Fix lenovo_led_set_tp10ubkbd() error handling
  HID: lenovo: Check hid_get_drvdata() returns non NULL in
    lenovo_event()
  HID: lenovo: Remove lenovo_led_brightness_get()
  HID: lenovo: Set LEDs max_brightness value
  HID: lenovo: Map mic-mute button to KEY_F20 instead of KEY_MICMUTE
  HID: lenovo: Set default_triggers for the mute and micmute LEDs
  HID: lenovo: Rework how the tp10ubkbd code decides which USB interface
    to use
  HID: lenovo: Add support for Thinkpad X1 Tablet Thin keyboard

 drivers/hid/hid-lenovo.c | 147 ++++++++++++++++++++++++++++++---------
 1 file changed, 114 insertions(+), 33 deletions(-)

-- 
2.30.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/9] HID: lenovo: Use brightness_set_blocking callback for setting LEDs brightness
  2021-03-04 19:21 [PATCH v4 0/9] HID: lenovo: Mute LED handling fixes and improvements Hans de Goede
@ 2021-03-04 19:21 ` Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 2/9] HID: lenovo: Fix lenovo_led_set_tp10ubkbd() error handling Hans de Goede
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2021-03-04 19:21 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Alexander Kobel
  Cc: Hans de Goede, linux-input, Marek Behún, Pavel Machek

The lenovo_led_brightness_set function may sleep, so we should have the
the led_class_dev's brightness_set_blocking callback point to it, rather
then the regular brightness_set callback.

When toggled through sysfs this is not a problem, but the brightness_set
callback may be called from atomic context when using LED-triggers.

Fixes: bc04b37ea0ec ("HID: lenovo: Add ThinkPad 10 Ultrabook Keyboard support")
Reviewed-by: Marek Behún <kabel@kernel.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-lenovo.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index c6c8e20f3e8d..4dc5e5f932ed 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -777,7 +777,7 @@ static enum led_brightness lenovo_led_brightness_get(
 				: LED_OFF;
 }
 
-static void lenovo_led_brightness_set(struct led_classdev *led_cdev,
+static int lenovo_led_brightness_set(struct led_classdev *led_cdev,
 			enum led_brightness value)
 {
 	struct device *dev = led_cdev->dev->parent;
@@ -802,6 +802,8 @@ static void lenovo_led_brightness_set(struct led_classdev *led_cdev,
 		lenovo_led_set_tp10ubkbd(hdev, tp10ubkbd_led[led_nr], value);
 		break;
 	}
+
+	return 0;
 }
 
 static int lenovo_register_leds(struct hid_device *hdev)
@@ -822,7 +824,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 = lenovo_led_brightness_set;
+	data->led_mute.brightness_set_blocking = lenovo_led_brightness_set;
 	data->led_mute.dev = &hdev->dev;
 	ret = led_classdev_register(&hdev->dev, &data->led_mute);
 	if (ret < 0)
@@ -830,7 +832,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 = lenovo_led_brightness_set;
+	data->led_micmute.brightness_set_blocking = lenovo_led_brightness_set;
 	data->led_micmute.dev = &hdev->dev;
 	ret = led_classdev_register(&hdev->dev, &data->led_micmute);
 	if (ret < 0) {
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 2/9] HID: lenovo: Fix lenovo_led_set_tp10ubkbd() error handling
  2021-03-04 19:21 [PATCH v4 0/9] HID: lenovo: Mute LED handling fixes and improvements Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 1/9] HID: lenovo: Use brightness_set_blocking callback for setting LEDs brightness Hans de Goede
@ 2021-03-04 19:21 ` Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 3/9] HID: lenovo: Check hid_get_drvdata() returns non NULL in lenovo_event() Hans de Goede
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2021-03-04 19:21 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Alexander Kobel
  Cc: Hans de Goede, linux-input, Marek Behún

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.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 3/9] HID: lenovo: Check hid_get_drvdata() returns non NULL in lenovo_event()
  2021-03-04 19:21 [PATCH v4 0/9] HID: lenovo: Mute LED handling fixes and improvements Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 1/9] HID: lenovo: Use brightness_set_blocking callback for setting LEDs brightness Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 2/9] HID: lenovo: Fix lenovo_led_set_tp10ubkbd() error handling Hans de Goede
@ 2021-03-04 19:21 ` Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 4/9] HID: lenovo: Remove lenovo_led_brightness_get() Hans de Goede
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2021-03-04 19:21 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Alexander Kobel
  Cc: Hans de Goede, linux-input, Marek Behún

The HID lenovo probe function only attaches drvdata to one of the
USB interfaces, but lenovo_event() will get called for all USB interfaces
to which hid-lenovo is bound.

This allows a malicious device to fake being a device handled by
hid-lenovo, which generates events for which lenovo_event() has
special handling (and thus dereferences hid_get_drvdata()) on another
interface triggering a NULL pointer exception.

Add a check for hid_get_drvdata() returning NULL, avoiding this
possible NULL pointer exception.

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>
---
 drivers/hid/hid-lenovo.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index ee175ab54281..b2596ed37880 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -508,6 +508,9 @@ static int lenovo_event_cptkbd(struct hid_device *hdev,
 static int lenovo_event(struct hid_device *hdev, struct hid_field *field,
 		struct hid_usage *usage, __s32 value)
 {
+	if (!hid_get_drvdata(hdev))
+		return 0;
+
 	switch (hdev->product) {
 	case USB_DEVICE_ID_LENOVO_CUSBKBD:
 	case USB_DEVICE_ID_LENOVO_CBTKBD:
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 4/9] HID: lenovo: Remove lenovo_led_brightness_get()
  2021-03-04 19:21 [PATCH v4 0/9] HID: lenovo: Mute LED handling fixes and improvements Hans de Goede
                   ` (2 preceding siblings ...)
  2021-03-04 19:21 ` [PATCH v4 3/9] HID: lenovo: Check hid_get_drvdata() returns non NULL in lenovo_event() Hans de Goede
@ 2021-03-04 19:21 ` Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 5/9] HID: lenovo: Set LEDs max_brightness value Hans de Goede
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2021-03-04 19:21 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Alexander Kobel
  Cc: Hans de Goede, linux-input, Marek Behún, Pavel Machek

The led_classdev already contains a cached value of the last set
brightness, the brightness_get callback is only meant for LED drivers
which can read back the actual / current brightness from the hardware.

Since lenovo_led_brightness_get() just returns the last set value
it does not add any functionality, so we can just remove it.

Reviewed-by: Marek Behún <kabel@kernel.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- New patch in v2 of this patch-set
---
 drivers/hid/hid-lenovo.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index b2596ed37880..1b8dd85ceb05 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -774,22 +774,6 @@ static void lenovo_led_set_tpkbd(struct hid_device *hdev)
 	hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
 }
 
-static enum led_brightness lenovo_led_brightness_get(
-			struct led_classdev *led_cdev)
-{
-	struct device *dev = led_cdev->dev->parent;
-	struct hid_device *hdev = to_hid_device(dev);
-	struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
-	int led_nr = 0;
-
-	if (led_cdev == &data_pointer->led_micmute)
-		led_nr = 1;
-
-	return data_pointer->led_state & (1 << led_nr)
-				? LED_FULL
-				: LED_OFF;
-}
-
 static int lenovo_led_brightness_set(struct led_classdev *led_cdev,
 			enum led_brightness value)
 {
@@ -837,7 +821,6 @@ static int lenovo_register_leds(struct hid_device *hdev)
 	snprintf(name_micm, name_sz, "%s:amber:micmute", dev_name(&hdev->dev));
 
 	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;
@@ -846,7 +829,6 @@ static int lenovo_register_leds(struct hid_device *hdev)
 		return ret;
 
 	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;
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 5/9] HID: lenovo: Set LEDs max_brightness value
  2021-03-04 19:21 [PATCH v4 0/9] HID: lenovo: Mute LED handling fixes and improvements Hans de Goede
                   ` (3 preceding siblings ...)
  2021-03-04 19:21 ` [PATCH v4 4/9] HID: lenovo: Remove lenovo_led_brightness_get() Hans de Goede
@ 2021-03-04 19:21 ` Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 6/9] HID: lenovo: Map mic-mute button to KEY_F20 instead of KEY_MICMUTE Hans de Goede
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2021-03-04 19:21 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Alexander Kobel
  Cc: Hans de Goede, linux-input, Marek Behún, Pavel Machek

The LEDs can only by turned on/off, so max_brightness should be set to 1.
Without this the max_brightness sysfs-attribute will report 255 which is
wrong.

Reviewed-by: Marek Behún <kabel@kernel.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
- Use 1 instead of LED_ON

Changes in v2:
- New patch in v2 of this patch-set
---
 drivers/hid/hid-lenovo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 1b8dd85ceb05..668762663f69 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -822,6 +822,7 @@ static int lenovo_register_leds(struct hid_device *hdev)
 
 	data->led_mute.name = name_mute;
 	data->led_mute.brightness_set_blocking = lenovo_led_brightness_set;
+	data->led_mute.max_brightness = 1;
 	data->led_mute.flags = LED_HW_PLUGGABLE;
 	data->led_mute.dev = &hdev->dev;
 	ret = led_classdev_register(&hdev->dev, &data->led_mute);
@@ -830,6 +831,7 @@ static int lenovo_register_leds(struct hid_device *hdev)
 
 	data->led_micmute.name = name_micm;
 	data->led_micmute.brightness_set_blocking = lenovo_led_brightness_set;
+	data->led_micmute.max_brightness = 1;
 	data->led_micmute.flags = LED_HW_PLUGGABLE;
 	data->led_micmute.dev = &hdev->dev;
 	ret = led_classdev_register(&hdev->dev, &data->led_micmute);
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 6/9] HID: lenovo: Map mic-mute button to KEY_F20 instead of KEY_MICMUTE
  2021-03-04 19:21 [PATCH v4 0/9] HID: lenovo: Mute LED handling fixes and improvements Hans de Goede
                   ` (4 preceding siblings ...)
  2021-03-04 19:21 ` [PATCH v4 5/9] HID: lenovo: Set LEDs max_brightness value Hans de Goede
@ 2021-03-04 19:21 ` Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 7/9] HID: lenovo: Set default_triggers for the mute and micmute LEDs Hans de Goede
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2021-03-04 19:21 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Alexander Kobel
  Cc: Hans de Goede, linux-input, Marek Behún

Mapping the mic-mute button to KEY_MICMUTE is technically correct but
KEY_MICMUTE translates to a scancode of 256 (248 + 8) under X,
which does not fit in 8 bits, so it does not work.

Because of this userspace is expecting KEY_F20 instead,
theoretically KEY_MICMUTE should work under Wayland but even
there it does not work, because the desktop-environment is
listening only for KEY_F20 and not for KEY_MICMUTE.

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>
---
 drivers/hid/hid-lenovo.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 668762663f69..2cf89b880ac5 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -33,6 +33,9 @@
 
 #include "hid-ids.h"
 
+/* Userspace expects F20 for mic-mute KEY_MICMUTE does not work */
+#define LENOVO_KEY_MICMUTE KEY_F20
+
 struct lenovo_drvdata {
 	u8 led_report[3]; /* Must be first for proper alignment */
 	int led_state;
@@ -134,7 +137,7 @@ static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
 	if (usage->hid == (HID_UP_BUTTON | 0x0010)) {
 		/* This sub-device contains trackpoint, mark it */
 		hid_set_drvdata(hdev, (void *)1);
-		map_key_clear(KEY_MICMUTE);
+		map_key_clear(LENOVO_KEY_MICMUTE);
 		return 1;
 	}
 	return 0;
@@ -149,7 +152,7 @@ static int lenovo_input_mapping_cptkbd(struct hid_device *hdev,
 	    (usage->hid & HID_USAGE_PAGE) == HID_UP_LNVENDOR) {
 		switch (usage->hid & HID_USAGE) {
 		case 0x00f1: /* Fn-F4: Mic mute */
-			map_key_clear(KEY_MICMUTE);
+			map_key_clear(LENOVO_KEY_MICMUTE);
 			return 1;
 		case 0x00f2: /* Fn-F5: Brightness down */
 			map_key_clear(KEY_BRIGHTNESSDOWN);
@@ -239,7 +242,7 @@ static int lenovo_input_mapping_tp10_ultrabook_kbd(struct hid_device *hdev,
 			map_key_clear(KEY_FN_ESC);
 			return 1;
 		case 9: /* Fn-F4: Mic mute */
-			map_key_clear(KEY_MICMUTE);
+			map_key_clear(LENOVO_KEY_MICMUTE);
 			return 1;
 		case 10: /* Fn-F7: Control panel */
 			map_key_clear(KEY_CONFIG);
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 7/9] HID: lenovo: Set default_triggers for the mute and micmute LEDs
  2021-03-04 19:21 [PATCH v4 0/9] HID: lenovo: Mute LED handling fixes and improvements Hans de Goede
                   ` (5 preceding siblings ...)
  2021-03-04 19:21 ` [PATCH v4 6/9] HID: lenovo: Map mic-mute button to KEY_F20 instead of KEY_MICMUTE Hans de Goede
@ 2021-03-04 19:21 ` Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 8/9] HID: lenovo: Rework how the tp10ubkbd code decides which USB interface to use Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 9/9] HID: lenovo: Add support for Thinkpad X1 Tablet Thin keyboard Hans de Goede
  8 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2021-03-04 19:21 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Alexander Kobel
  Cc: Hans de Goede, linux-input, Marek Behún, Pavel Machek

The mute and mic-mute LEDs should be automatically turned on/off based
on the audio-card's mixer settings.

Add the standardized default-trigger names for this, so that the alsa
code can turn the LEDs on/off as appropriate (on supported audio cards).

This brings the mute/mic-mute LED support inline with the thinkpad_acpi
support for the same LEDs in keyboards directly connected to the
laptop's embedded-controller.

Reviewed-by: Marek Behún <kabel@kernel.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-lenovo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 2cf89b880ac5..2287142116b9 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -824,6 +824,7 @@ static int lenovo_register_leds(struct hid_device *hdev)
 	snprintf(name_micm, name_sz, "%s:amber:micmute", dev_name(&hdev->dev));
 
 	data->led_mute.name = name_mute;
+	data->led_mute.default_trigger = "audio-mute";
 	data->led_mute.brightness_set_blocking = lenovo_led_brightness_set;
 	data->led_mute.max_brightness = 1;
 	data->led_mute.flags = LED_HW_PLUGGABLE;
@@ -833,6 +834,7 @@ static int lenovo_register_leds(struct hid_device *hdev)
 		return ret;
 
 	data->led_micmute.name = name_micm;
+	data->led_micmute.default_trigger = "audio-micmute";
 	data->led_micmute.brightness_set_blocking = lenovo_led_brightness_set;
 	data->led_micmute.max_brightness = 1;
 	data->led_micmute.flags = LED_HW_PLUGGABLE;
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 8/9] HID: lenovo: Rework how the tp10ubkbd code decides which USB interface to use
  2021-03-04 19:21 [PATCH v4 0/9] HID: lenovo: Mute LED handling fixes and improvements Hans de Goede
                   ` (6 preceding siblings ...)
  2021-03-04 19:21 ` [PATCH v4 7/9] HID: lenovo: Set default_triggers for the mute and micmute LEDs Hans de Goede
@ 2021-03-04 19:21 ` Hans de Goede
  2021-03-04 19:21 ` [PATCH v4 9/9] HID: lenovo: Add support for Thinkpad X1 Tablet Thin keyboard Hans de Goede
  8 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2021-03-04 19:21 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Alexander Kobel
  Cc: Hans de Goede, linux-input

Instead of looking for a hdev with a type of HID_TYPE_USBMOUSE find
the interface for the mute/mic-mute/fn-lock LEDs by checking for the
output-report which is used to set them.

This is a preparation patch for adding support for the LEDs on the
X1 tablet thin keyboard which uses the same output-report, but has
a separate (third) USB interface for the touchpad/mouse functionality.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-lenovo.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 2287142116b9..a33de5022ec3 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -959,11 +959,24 @@ static const struct attribute_group lenovo_attr_group_tp10ubkbd = {
 
 static int lenovo_probe_tp10ubkbd(struct hid_device *hdev)
 {
+	struct hid_report_enum *rep_enum;
 	struct lenovo_drvdata *data;
+	struct hid_report *rep;
+	bool found;
 	int ret;
 
-	/* All the custom action happens on the USBMOUSE device for USB */
-	if (hdev->type != HID_TYPE_USBMOUSE)
+	/*
+	 * The LEDs and the Fn-lock functionality use output report 9,
+	 * with an application of 0xffa0001, add the LEDs on the interface
+	 * with this output report.
+	 */
+	found = false;
+	rep_enum = &hdev->report_enum[HID_OUTPUT_REPORT];
+	list_for_each_entry(rep, &rep_enum->report_list, list) {
+		if (rep->application == 0xffa00001)
+			found = true;
+	}
+	if (!found)
 		return 0;
 
 	data = devm_kzalloc(&hdev->dev, sizeof(*data), GFP_KERNEL);
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 9/9] HID: lenovo: Add support for Thinkpad X1 Tablet Thin keyboard
  2021-03-04 19:21 [PATCH v4 0/9] HID: lenovo: Mute LED handling fixes and improvements Hans de Goede
                   ` (7 preceding siblings ...)
  2021-03-04 19:21 ` [PATCH v4 8/9] HID: lenovo: Rework how the tp10ubkbd code decides which USB interface to use Hans de Goede
@ 2021-03-04 19:21 ` Hans de Goede
  8 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2021-03-04 19:21 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Alexander Kobel
  Cc: Hans de Goede, linux-input

The Thinkpad X1 Tablet Thin keyboard's HID interface for the media-keys
and other special functions, is quite similar to the Thinkpad 10 ultrabook
keyboard's mouse/media-keys HID interface.

The only difference is that it needs a bit different key mappings.

Add support for the mute-LED and the non-standard media-keys on this
keyboard by re-using the tp10_ultrabook_kbd code combined with a new
lenovo_input_mapping_x1_tab_kbd() function.

Co-authored-by: Alexander Kobel <a-kobel@a-kobel.de>
Tested-by: Alexander Kobel <a-kobel@a-kobel.de>
Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-lenovo.c | 61 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index a33de5022ec3..93b1f935e526 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -266,6 +266,54 @@ static int lenovo_input_mapping_tp10_ultrabook_kbd(struct hid_device *hdev,
 	return 0;
 }
 
+static int lenovo_input_mapping_x1_tab_kbd(struct hid_device *hdev,
+		struct hid_input *hi, struct hid_field *field,
+		struct hid_usage *usage, unsigned long **bit, int *max)
+{
+	/*
+	 * The ThinkPad X1 Tablet Thin Keyboard uses 0x000c0001 usage for
+	 * a bunch of keys which have no standard consumer page code.
+	 */
+	if (usage->hid == 0x000c0001) {
+		switch (usage->usage_index) {
+		case 0: /* Fn-F10: Enable/disable bluetooth */
+			map_key_clear(KEY_BLUETOOTH);
+			return 1;
+		case 1: /* Fn-F11: Keyboard settings */
+			map_key_clear(KEY_KEYBOARD);
+			return 1;
+		case 2: /* Fn-F12: User function / Cortana */
+			map_key_clear(KEY_MACRO1);
+			return 1;
+		case 3: /* Fn-PrtSc: Snipping tool */
+			map_key_clear(KEY_SELECTIVE_SCREENSHOT);
+			return 1;
+		case 8: /* Fn-Esc: Fn-lock toggle */
+			map_key_clear(KEY_FN_ESC);
+			return 1;
+		case 9: /* Fn-F4: Mute/unmute microphone */
+			map_key_clear(KEY_MICMUTE);
+			return 1;
+		case 10: /* Fn-F9: Settings */
+			map_key_clear(KEY_CONFIG);
+			return 1;
+		case 13: /* Fn-F7: Manage external displays */
+			map_key_clear(KEY_SWITCHVIDEOMODE);
+			return 1;
+		case 14: /* Fn-F8: Enable/disable wifi */
+			map_key_clear(KEY_WLAN);
+			return 1;
+		}
+	}
+
+	if (usage->hid == (HID_UP_KEYBOARD | 0x009a)) {
+		map_key_clear(KEY_SYSRQ);
+		return 1;
+	}
+
+	return 0;
+}
+
 static int lenovo_input_mapping(struct hid_device *hdev,
 		struct hid_input *hi, struct hid_field *field,
 		struct hid_usage *usage, unsigned long **bit, int *max)
@@ -289,6 +337,8 @@ static int lenovo_input_mapping(struct hid_device *hdev,
 	case USB_DEVICE_ID_LENOVO_TP10UBKBD:
 		return lenovo_input_mapping_tp10_ultrabook_kbd(hdev, hi, field,
 							       usage, bit, max);
+	case USB_DEVICE_ID_LENOVO_X1_TAB:
+		return lenovo_input_mapping_x1_tab_kbd(hdev, hi, field, usage, bit, max);
 	default:
 		return 0;
 	}
@@ -375,6 +425,7 @@ static ssize_t attr_fn_lock_store(struct device *dev,
 		lenovo_features_set_cptkbd(hdev);
 		break;
 	case USB_DEVICE_ID_LENOVO_TP10UBKBD:
+	case USB_DEVICE_ID_LENOVO_X1_TAB:
 		ret = lenovo_led_set_tp10ubkbd(hdev, TP10UBKBD_FN_LOCK_LED, value);
 		if (ret)
 			return ret;
@@ -519,6 +570,7 @@ static int lenovo_event(struct hid_device *hdev, struct hid_field *field,
 	case USB_DEVICE_ID_LENOVO_CBTKBD:
 		return lenovo_event_cptkbd(hdev, field, usage, value);
 	case USB_DEVICE_ID_LENOVO_TP10UBKBD:
+	case USB_DEVICE_ID_LENOVO_X1_TAB:
 		return lenovo_event_tp10ubkbd(hdev, field, usage, value);
 	default:
 		return 0;
@@ -800,6 +852,7 @@ static int lenovo_led_brightness_set(struct led_classdev *led_cdev,
 		lenovo_led_set_tpkbd(hdev);
 		break;
 	case USB_DEVICE_ID_LENOVO_TP10UBKBD:
+	case USB_DEVICE_ID_LENOVO_X1_TAB:
 		ret = lenovo_led_set_tp10ubkbd(hdev, tp10ubkbd_led[led_nr], value);
 		break;
 	}
@@ -1038,6 +1091,7 @@ static int lenovo_probe(struct hid_device *hdev,
 		ret = lenovo_probe_cptkbd(hdev);
 		break;
 	case USB_DEVICE_ID_LENOVO_TP10UBKBD:
+	case USB_DEVICE_ID_LENOVO_X1_TAB:
 		ret = lenovo_probe_tp10ubkbd(hdev);
 		break;
 	default:
@@ -1103,6 +1157,7 @@ static void lenovo_remove(struct hid_device *hdev)
 		lenovo_remove_cptkbd(hdev);
 		break;
 	case USB_DEVICE_ID_LENOVO_TP10UBKBD:
+	case USB_DEVICE_ID_LENOVO_X1_TAB:
 		lenovo_remove_tp10ubkbd(hdev);
 		break;
 	}
@@ -1142,6 +1197,12 @@ static const struct hid_device_id lenovo_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_IBM, USB_DEVICE_ID_IBM_SCROLLPOINT_800DPI_OPTICAL_PRO) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_SCROLLPOINT_OPTICAL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TP10UBKBD) },
+	/*
+	 * Note bind to the HID_GROUP_GENERIC group, so that we only bind to the keyboard
+	 * part, while letting hid-multitouch.c handle the touchpad and trackpoint.
+	 */
+	{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
+		     USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_X1_TAB) },
 	{ }
 };
 
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-03-04 19:24 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-04 19:21 [PATCH v4 0/9] HID: lenovo: Mute LED handling fixes and improvements Hans de Goede
2021-03-04 19:21 ` [PATCH v4 1/9] HID: lenovo: Use brightness_set_blocking callback for setting LEDs brightness Hans de Goede
2021-03-04 19:21 ` [PATCH v4 2/9] HID: lenovo: Fix lenovo_led_set_tp10ubkbd() error handling Hans de Goede
2021-03-04 19:21 ` [PATCH v4 3/9] HID: lenovo: Check hid_get_drvdata() returns non NULL in lenovo_event() Hans de Goede
2021-03-04 19:21 ` [PATCH v4 4/9] HID: lenovo: Remove lenovo_led_brightness_get() Hans de Goede
2021-03-04 19:21 ` [PATCH v4 5/9] HID: lenovo: Set LEDs max_brightness value Hans de Goede
2021-03-04 19:21 ` [PATCH v4 6/9] HID: lenovo: Map mic-mute button to KEY_F20 instead of KEY_MICMUTE Hans de Goede
2021-03-04 19:21 ` [PATCH v4 7/9] HID: lenovo: Set default_triggers for the mute and micmute LEDs Hans de Goede
2021-03-04 19:21 ` [PATCH v4 8/9] HID: lenovo: Rework how the tp10ubkbd code decides which USB interface to use Hans de Goede
2021-03-04 19:21 ` [PATCH v4 9/9] HID: lenovo: Add support for Thinkpad X1 Tablet Thin keyboard Hans de Goede

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.