Re: [PATCH] Input: applespi - turn the keyboard backlight off across suspend

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] input: applespi: backlight level saved as 0 during hibernation
- [Medium] input: applespi: caps-lock LED fails to turn off during suspend
--

commit 37ebd40953a1312d5d67af7477063eae746c320a
Author: Michal Szpakowski <michi.szpakowski@gmail.com>

Input: applespi - turn the keyboard backlight off across suspend

The patch sets the LED_CORE_SUSPENDRESUME flag for the keyboard backlight,
allowing the LED core to blank it during suspend and restore it on resume.
This resolves an issue where the backlight stayed lit through s2idle.

Fixes: 038b1a05eae6 ("Input: add Apple SPI keyboard and trackpad driver")
Cc: stable@vger.kernel.org

> diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
> index b5ff71cd5a701..838b380bde6e1 100644
> --- a/drivers/input/keyboard/applespi.c
> +++ b/drivers/input/keyboard/applespi.c
> @@ -1772,6 +1772,7 @@ static int applespi_probe(struct spi_device *spi)
>  	applespi->backlight_info.name            = "spi::kbd_backlight";
>  	applespi->backlight_info.default_trigger = "kbd-backlight";
>  	applespi->backlight_info.brightness_set  = applespi_set_bl_level;
> +	applespi->backlight_info.flags           = LED_CORE_SUSPENDRESUME;

[Severity: Medium]
During hibernation, the PM core calls led_suspend(), which updates
applespi->have_bl_level to 0. Does this cause the user's preferred backlight
level to be permanently overwritten with 0 when applespi_poweroff_late()
later saves the state?

For example, in drivers/input/keyboard/applespi.c:

applespi_poweroff_late() {
    ...
    applespi_save_bl_level(applespi, applespi->have_bl_level);
}

[Severity: Medium]
Could adding this flag also block applespi_suspend() from successfully turning
off the caps-lock LED?

If the LED core initiates an asynchronous backlight SPI write to dim the
keyboard before suspend, applespi->cmd_msg_queued will be set.

When applespi_suspend() then calls applespi_set_capsl_led(), the call to
applespi_send_cmd_msg() returns early without queueing the caps-lock command
because the backlight command is still in flight.

applespi_suspend() then calls applespi_drain_writes(), which sets
applespi->drain = true. When the backlight write eventually completes, the
pending caps-lock off command is permanently aborted because the drain flag
is set, potentially leaving the caps-lock LED in an incorrect state.

>  
>  	sts = devm_led_classdev_register(&spi->dev, &applespi->backlight_info);
>  	if (sts)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001084154.4703-1-michi.szpakowski@gmail.com?part=1