| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b7ad97009fa0a49b9229ade0af5f526057869a36.camel@rong.moe>
Date: Wed, 06 Aug 2025 21:28:39 +0800
From: Rong Zhang <i@...g.moe>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: Ike Panhc <ikepanhc@...il.com>, "Derek J. Clark"
<derekjohn.clark@...il.com>, Mark Pearson <mpearson-lenovo@...ebb.ca>,
Hans de Goede <hansg@...nel.org>, platform-driver-x86@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] platform/x86: ideapad-laptop: Fully support auto
kbd backlight
Hi Ilpo,
On Wed, 2025-08-06 at 14:26 +0300, Ilpo Järvinen wrote:
> 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@...g.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.
How time flies! It is embarrassing that I wrote this several weeks ago
but forgot to update it before finishing the patch. Thanks for your
reminder and I will update this in v2.
>
> > +Contact: platform-driver-x86@...r.kernel.org
> > +Description:
> > + This file allows to control the automatic keyboard
>
> Please avoid using "This file" entirely in the description.
>
> Start with "Controls ..."
Sure.
> > + 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,"
> ?
It does sound more understandable to describe it that way. Thanks for
your suggestion and I will improve the wording in v2.
> > + 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);
> >
Thanks for your review,
Rong
Powered by blists - more mailing lists