Re: [PATCH 2/2] platform/x86: ideapad-laptop: Fully support auto kbd backlight

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Tue, 5 Aug 2025, Rong Zhang wrote:

> Currently, the auto brightness mode of keyboard backlight maps to
> brightness=0 in LED classdev. The only method to switch to such a mode
> is by pressing the manufacturer-defined shortcut (Fn+Space). However, 0
> is a multiplexed brightness value; writing 0 simply results in the
> backlight being turned off.
> 
> With brightness processing code decoupled from LED classdev, we can now
> fully support the auto brightness mode. In this mode, the keyboard
> backlight is controlled by the EC according to the ambient light sensor
> (ALS).
> 
> To utilize this, a sysfs node is exposed to the userspace:
> /sys/class/leds/platform::kbd_backlight/als_enabled. The name is chosen
> to align with dell-laptop, which provides a similar feature.
> 
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  .../ABI/testing/sysfs-platform-ideapad-laptop | 12 ++++
>  drivers/platform/x86/lenovo/ideapad-laptop.c  | 65 ++++++++++++++++++-
>  2 files changed, 75 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> index 5ec0dee9e707..a2b78aa60aaa 100644
> --- a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> +++ b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> @@ -50,3 +50,15 @@ Description:
>  		Controls whether the "always on USB charging" feature is
>  		enabled or not. This feature enables charging USB devices
>  		even if the computer is not turned on.
> +
> +What:		/sys/class/leds/platform::kbd_backlight/als_enabled
> +Date:		July 2025
> +KernelVersion:	6.17

This ship has sailed.

> +Contact:	platform-driver-x86@vger.kernel.org
> +Description:
> +		This file allows to control the automatic keyboard

Please avoid using "This file" entirely in the description.

Start with "Controls ..."

> +		illumination mode on some systems that have an ambient
> +		light sensor. Write 1 to this file to enable the auto
> +		mode, 0 to disable it. In this mode, the actual

What is "this mode" ? Did you mean, e.g., "When the auto mode is enabled,"
?

> +		brightness level is not available and reading the
> +		"brightness" file always returns 0.
> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
> index 5014c1d0b633..49f2fc68add4 100644
> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> @@ -1712,6 +1712,57 @@ static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
>  	ideapad_kbd_bl_notify_known(priv, brightness);
>  }
>  
> +static ssize_t als_enabled_show(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
> +	int hw_brightness;
> +
> +	hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
> +	if (hw_brightness < 0)
> +		return hw_brightness;
> +
> +	return sysfs_emit(buf, "%d\n", hw_brightness == KBD_BL_AUTO_MODE_HW_BRIGHTNESS);
> +}
> +
> +static ssize_t als_enabled_store(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf, size_t count)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
> +	bool state;
> +	int err;
> +
> +	err = kstrtobool(buf, &state);
> +	if (err)
> +		return err;
> +
> +	/*
> +	 * Auto (ALS) mode uses a predefined HW brightness value. It is
> +	 * impossible to disable it without setting another brightness value.
> +	 * Set the brightness to 0 when disabling is requested.
> +	 */
> +	err = ideapad_kbd_bl_hw_brightness_set(priv, state ? KBD_BL_AUTO_MODE_HW_BRIGHTNESS : 0);
> +	if (err)
> +		return err;
> +
> +	/* Both HW brightness values map to 0 in the LED classdev. */
> +	ideapad_kbd_bl_notify_known(priv, 0);
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RW(als_enabled);
> +
> +static struct attribute *ideapad_kbd_bl_als_attrs[] = {
> +	&dev_attr_als_enabled.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(ideapad_kbd_bl_als);
> +
>  static int ideapad_kbd_bl_init(struct ideapad_private *priv)
>  {
>  	int brightness, err;
> @@ -1722,10 +1773,20 @@ static int ideapad_kbd_bl_init(struct ideapad_private *priv)
>  	if (WARN_ON(priv->kbd_bl.initialized))
>  		return -EEXIST;
>  
> -	if (ideapad_kbd_bl_check_tristate(priv->kbd_bl.type)) {
> +	switch (priv->kbd_bl.type) {
> +	case KBD_BL_TRISTATE_AUTO:
> +		/* The sysfs node will be /sys/class/leds/platform::kbd_backlight/als_enabled */
> +		priv->kbd_bl.led.groups = ideapad_kbd_bl_als_groups;
> +		fallthrough;
> +	case KBD_BL_TRISTATE:
>  		priv->kbd_bl.led.max_brightness = 2;
> -	} else {
> +		break;
> +	case KBD_BL_STANDARD:
>  		priv->kbd_bl.led.max_brightness = 1;
> +		break;
> +	default:
> +		/* This has already been validated by ideapad_check_features(). */
> +		unreachable();
>  	}
>  
>  	brightness = ideapad_kbd_bl_brightness_get(priv);
> 

-- 
 i.