Re: [PATCH v6 11/12] platform/x86: ideapad-laptop: Decouple hardware & classdev brightness for keyboard backlight

[Rong Zhang <i@rong.moe>]

Hi Lee,

On Wed, 2026-09-02 at 18:15 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Low] Incorrect format string `%d` used for `unsigned int hw_brightness` in `dev_warn`.

Well, let me fix it in the next revision.

> 
> Pre-existing issues:
> - [High] The driver uses the unmanaged `led_classdev_register` API instead of the managed `devm_led_classdev_register` API.

This is yet another LLM hallucination. The driver needs to manage LED
unregistration itself to coordinate with ACPI notification handling.

Sashiko's concerns on other patches in the series are either pre-existing
issues that are out of the scope of the series, or LLM hallucinations.

Lee, I believe the series is ready for your review. Could you kindly
shine some light on the series? If you decide to apply PATCH v6 1-9 to
the LED tree, I will send a v7 solely for pdx86. Otherwise I will refresh
the whole series according to your insights.

Thanks,
Rong

> --
> 
> commit 227fca9f070999fc85599521adcec4975c6e441e
> Author: Rong Zhang <i@rong.moe>
> 
> platform/x86: ideapad-laptop: Decouple hardware & classdev brightness for keyboard backlight
> 
> This commit decouples hardware brightness from LED classdev brightness to
> better support models with an ambient light sensor (ALS). It also introduces
> a refactor of the initialization path to prepare for exposing the auto
> brightness mode.
> 
> > diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
> > index 684421ba53cbc..712999b6900df 100644
> > --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> > +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> 
> [ ... ]
> 
> > @@ -1594,7 +1619,24 @@ static int ideapad_kbd_bl_check_tristate(int type)
> >  	return (type == KBD_BL_TRISTATE) || (type == KBD_BL_TRISTATE_AUTO);
> >  }
> >  
> > -static int ideapad_kbd_bl_brightness_get(struct ideapad_private *priv)
> > +static int ideapad_kbd_bl_brightness_parse(struct ideapad_private *priv, unsigned int hw_brightness)
> > +{
> > +	/* Off, low or high */
> > +	if (hw_brightness <= priv->kbd_bl.led.max_brightness)
> > +		return hw_brightness;
> > +
> > +	/* Auto (controlled by EC according to ALS), report as off */
> > +	if (priv->kbd_bl.type == KBD_BL_TRISTATE_AUTO &&
> > +	    hw_brightness == KBD_BL_AUTO_MODE_HW_BRIGHTNESS)
> > +		return 0;
> > +
> > +	/* Unknown value */
> > +	dev_warn(&priv->platform_device->dev,
> > +		 "Unknown keyboard backlight value: %d", hw_brightness);
> 
> [Severity: Low]
> Does this dev_warn() use the correct format string for its argument?
> The hw_brightness variable is passed as an unsigned int, but the format
> string uses %d which expects a signed int.
> 
> > +	return -EINVAL;
> > +}
> 
> [ ... ]
> 
> > @@ -1710,21 +1758,35 @@ static int ideapad_kbd_bl_init(struct ideapad_private *priv)
> >  	if (err)
> >  		return err;
> 
> [ ... ]
> 
> > +	/* Reject insane values. */
> > +	err = ideapad_kbd_bl_brightness_parse(priv, hw_brightness);
> > +	if (err < 0)
> > +		return err;
> > +
> >  	err = led_classdev_register(&priv->platform_device->dev, &priv->kbd_bl.led);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but should this use the managed
> devm_led_classdev_register() API instead?
> 
> The LED subsystem guidelines mandate using managed registration APIs to
> prevent resource leaks and use-after-free hazards during driver removal.
> 
> >  	if (err)
> >  		return err;