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

[sashiko-bot@kernel.org]

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

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

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;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-leds-trigger-hw-changed-v6-0-55693cd78877@rong.moe?part=11