mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Mark Pearson" <mpearson-lenovo@squebb.ca>
To: "Vishnu Sankar" <vishnuocv@gmail.com>,
	"Derek J . Clark" <derekjohn.clark@gmail.com>,
	"Jiri Kosina" <jikos@kernel.org>,
	"Benjamin Tissoires" <bentiss@kernel.org>
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Vishnu Sankar" <vsankar@lenovo.com>
Subject: Re: [PATCH] HID: lenovo: Add keyboard backlight support for the X13 detachable folio
Date: Fri, 30 Oct 2026 09:01:44 -0400	[thread overview]
Message-ID: <6e562b92-afbe-4c01-a703-81d55f2fdc65@app.fastmail.com> (raw)
In-Reply-To: <20261021200841.75370-1-vishnuocv@gmail.com>

Thanks Vishnu,

On Wed, Oct 21, 2026, at 4:08 PM, Vishnu Sankar wrote:
> The keyboard backlight of the ThinkPad X13 detachable folio is reached
> over HID rather than through the EC, so thinkpad_acpi exposes nothing
> for it and userspace can neither read the level nor set it.
>
> The folio takes and reports the level over the same vendor reports this
> driver already uses for the Fn-lock, mute and mic-mute indicators, so
> register a kbd_backlight LED class device on top of them.
>
> The firmware cycles the level on Fn+Space by itself.  Report the
> resulting state to the LED class device instead of the keypress, which
> userspace would otherwise act on a second time.
>
> Tested on a ThinkPad X13 Detachable (17ef:62af), BIOS R3IET32WM.
>
> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> ---
>  drivers/hid/hid-lenovo.c | 122 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 119 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 3976d7b53b14..956c26128f40 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -24,6 +24,7 @@
>  /*
>   */
> 
> +#include <linux/cleanup.h>
>  #include <linux/module.h>
>  #include <linux/sysfs.h>
>  #include <linux/device.h>
> @@ -51,6 +52,8 @@ struct lenovo_drvdata {
>  	struct mutex led_report_mutex;
>  	struct led_classdev led_mute;
>  	struct led_classdev led_micmute;
> +	struct led_classdev led_kbd_bl;
> +	struct work_struct kbd_bl_sync_work;
>  	struct work_struct fn_lock_sync_work;
>  	struct hid_device *hdev;
>  	int press_to_select;
> @@ -75,10 +78,19 @@ struct lenovo_drvdata {
>  #define TP10UBKBD_FN_LOCK_LED		0x54
>  #define TP10UBKBD_MUTE_LED		0x64
>  #define TP10UBKBD_MICMUTE_LED		0x74
> +#define TP10UBKBD_KBD_BL_LED		0xc4
> 
>  #define TP10UBKBD_LED_OFF		1
>  #define TP10UBKBD_LED_ON		2
> 
> +/* Above the levels, 0x04 selects the firmware's own automatic mode. */
> +#define TP10UBKBD_KBD_BL_MAX		2
> +
> +static bool lenovo_has_kbd_backlight(struct hid_device *hdev)
> +{
> +	return hdev->product == USB_DEVICE_ID_LENOVO_X13_TAB;
> +}
> +
>  /* Function to report raw_events as key events*/
>  static inline void report_key_event(struct input_dev *input, int 
> keycode)
>  {
> @@ -87,8 +99,8 @@ static inline void report_key_event(struct input_dev 
> *input, int keycode)
>  	input_sync(input);
>  }
> 
> -static int lenovo_led_set_tp10ubkbd(struct hid_device *hdev, u8 
> led_code,
> -				    enum led_brightness value)
> +static int lenovo_send_led_report_tp10ubkbd(struct hid_device *hdev, 
> u8 led_code,
> +					    u8 value)
>  {
>  	struct lenovo_drvdata *data = hid_get_drvdata(hdev);
>  	int ret;
> @@ -97,7 +109,7 @@ static int lenovo_led_set_tp10ubkbd(struct 
> hid_device *hdev, u8 led_code,
> 
>  	data->led_report[0] = TP10UBKBD_LED_OUTPUT_REPORT;
>  	data->led_report[1] = led_code;
> -	data->led_report[2] = value ? TP10UBKBD_LED_ON : TP10UBKBD_LED_OFF;
> +	data->led_report[2] = value;
>  	ret = hid_hw_raw_request(hdev, data->led_report[0], data->led_report, 
> 3,
>  				 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
>  	if (ret != 3) {
> @@ -114,6 +126,33 @@ static int lenovo_led_set_tp10ubkbd(struct 
> hid_device *hdev, u8 led_code,
>  	return ret;
>  }
> 
> +static int lenovo_led_set_tp10ubkbd(struct hid_device *hdev, u8 
> led_code,
> +				    enum led_brightness value)
> +{
> +	return lenovo_send_led_report_tp10ubkbd(hdev, led_code,
> +			value ? TP10UBKBD_LED_ON : TP10UBKBD_LED_OFF);
> +}
> +
> +static int lenovo_read_led_state_tp10ubkbd(struct hid_device *hdev, u8 
> led_code,
> +					   u8 *state)
> +{
> +	struct lenovo_drvdata *data = hid_get_drvdata(hdev);
> +	int ret;
> +
> +	guard(mutex)(&data->led_report_mutex);
> +
> +	data->led_report[0] = led_code;
> +	ret = hid_hw_raw_request(hdev, data->led_report[0], data->led_report, 
> 2,
> +				 HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> +
> +	if (ret != 2)
> +		return ret < 0 ? ret : -EIO;
> +
> +	*state = data->led_report[1];
> +
> +	return 0;
> +}
> +
>  static void lenovo_tp10ubkbd_sync_fn_lock(struct work_struct *work)
>  {
>  	struct lenovo_drvdata *data =
> @@ -725,6 +764,7 @@ static const struct attribute_group 
> lenovo_attr_group_cptkbd = {
>  /* Function to handle Lenovo Thinkpad TAB X12's HID raw inputs for fn 
> keys*/
>  static int lenovo_raw_event_TP_X12_tab(struct hid_device *hdev, u32 
> raw_data)
>  {
> +	struct lenovo_drvdata *data = hid_get_drvdata(hdev);
>  	struct hid_input *hidinput;
>  	struct input_dev *input = NULL;
> 
> @@ -764,6 +804,15 @@ static int lenovo_raw_event_TP_X12_tab(struct 
> hid_device *hdev, u32 raw_data)
>  			return 1;
>  		case TP_X12_RAW_HOTKEY_FN_SPACE:
>  			/* Keyboard LED backlight toggle*/
> +			if (data && lenovo_has_kbd_backlight(hdev)) {
> +				/*
> +				 * The firmware has already cycled the
> +				 * backlight; reporting the key as well would
> +				 * have userspace change the level again.
> +				 */
> +				schedule_work(&data->kbd_bl_sync_work);
> +				return 1;
> +			}
>  			report_key_event(input, KEY_KBDILLUMTOGGLE);
>  			return 1;
>  		default:
> @@ -1178,6 +1227,60 @@ static int lenovo_led_brightness_set(struct 
> led_classdev *led_cdev,
>  	return ret;
>  }
> 
> +static int lenovo_kbd_bl_brightness_set(struct led_classdev *led_cdev,
> +					enum led_brightness value)
> +{
> +	struct hid_device *hdev = to_hid_device(led_cdev->dev->parent);
> +
> +	return lenovo_send_led_report_tp10ubkbd(hdev, TP10UBKBD_KBD_BL_LED,
> +						value);
> +}
> +
> +static enum led_brightness
> +lenovo_kbd_bl_brightness_get(struct led_classdev *led_cdev)
> +{
> +	struct hid_device *hdev = to_hid_device(led_cdev->dev->parent);
> +	u8 state;
> +
> +	if (lenovo_read_led_state_tp10ubkbd(hdev, TP10UBKBD_KBD_BL_LED, &state))
> +		return led_cdev->brightness;
> +
> +	if (state > TP10UBKBD_KBD_BL_MAX)
> +		return led_cdev->brightness;
> +
> +	return state;
> +}
> +
> +/* Reading the state sleeps, so it cannot be done from the report handler. */
> +static void lenovo_kbd_bl_sync(struct work_struct *work)
> +{
> +	struct lenovo_drvdata *data =
> +		container_of(work, struct lenovo_drvdata, kbd_bl_sync_work);
> +	enum led_brightness value;
> +
> +	value = lenovo_kbd_bl_brightness_get(&data->led_kbd_bl);
> +	data->led_kbd_bl.brightness = value;
> +	led_classdev_notify_brightness_hw_changed(&data->led_kbd_bl, value);
> +}
> +
> +static int lenovo_register_kbd_backlight(struct hid_device *hdev)
> +{
> +	struct lenovo_drvdata *data = hid_get_drvdata(hdev);
> +
> +	data->led_kbd_bl.name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s::%s",
> +					       dev_name(&hdev->dev),
> +					       LED_FUNCTION_KBD_BACKLIGHT);
> +	if (!data->led_kbd_bl.name)
> +		return -ENOMEM;
> +
> +	data->led_kbd_bl.max_brightness = TP10UBKBD_KBD_BL_MAX;
> +	data->led_kbd_bl.brightness_set_blocking = lenovo_kbd_bl_brightness_set;
> +	data->led_kbd_bl.brightness_get = lenovo_kbd_bl_brightness_get;
> +	data->led_kbd_bl.flags = LED_HW_PLUGGABLE | LED_BRIGHT_HW_CHANGED;
> +
> +	return led_classdev_register(&hdev->dev, &data->led_kbd_bl);
> +}
> +
>  static int lenovo_register_leds(struct hid_device *hdev)
>  {
>  	struct lenovo_drvdata *data = hid_get_drvdata(hdev);
> @@ -1344,6 +1447,7 @@ static int lenovo_probe_tp10ubkbd(struct hid_device *hdev)
>  		return -ENOMEM;
> 
>  	mutex_init(&data->led_report_mutex);
> +	INIT_WORK(&data->kbd_bl_sync_work, lenovo_kbd_bl_sync);
>  	INIT_WORK(&data->fn_lock_sync_work, lenovo_tp10ubkbd_sync_fn_lock);
>  	data->hdev = hdev;
> 
> @@ -1374,7 +1478,16 @@ static int lenovo_probe_tp10ubkbd(struct 
> hid_device *hdev)
>  	if (ret)
>  		goto err;
> 
> +	if (lenovo_has_kbd_backlight(hdev)) {
> +		ret = lenovo_register_kbd_backlight(hdev);
> +		if (ret)
> +			goto err_leds;
> +	}
> +
>  	return 0;
> +err_leds:
> +	led_classdev_unregister(&data->led_micmute);
> +	led_classdev_unregister(&data->led_mute);
>  err:
>  	sysfs_remove_group(&hdev->dev.kobj, &lenovo_attr_group_tp10ubkbd);
>  	return ret;
> @@ -1477,6 +1590,9 @@ static void lenovo_remove_tp10ubkbd(struct 
> hid_device *hdev)
>  	if (data == NULL)
>  		return;
> 
> +	cancel_work_sync(&data->kbd_bl_sync_work);
> +
> +	led_classdev_unregister(&data->led_kbd_bl);
>  	led_classdev_unregister(&data->led_micmute);
>  	led_classdev_unregister(&data->led_mute);
> 
>
> base-commit: 0fcb05b72344f40806cbe810de8872f1698bf053
> -- 
> 2.56.0

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Mark

      reply	other threads:[~2026-10-30 13:02 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-21 20:08 Vishnu Sankar
2026-10-30 13:01 ` Mark Pearson [this message]

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=6e562b92-afbe-4c01-a703-81d55f2fdc65@app.fastmail.com \
    --to=mpearson-lenovo@squebb.ca \
    --cc=bentiss@kernel.org \
    --cc=derekjohn.clark@gmail.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vishnuocv@gmail.com \
    --cc=vsankar@lenovo.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®