Re: [PATCH v3 2/7] platform/x86: thinkpad_acpi: Extract kbdlight_notify_change() helpers

[Rong Zhang <i@rong.moe>]

Hi Qiling,

You are extracting two helpers with different names. Let's retitle:

   Extract kbdlight notify helpers

On Sun, 2026-11-15 at 01:48 +0800, Qiling Zhu wrote:
> TP_HKEY_EV_KBD_LIGHT handling in tpacpi_driver_event() reads back the EC
> level, updates the cached brightness and notifies the LED core inline.
> Extract this block into separate kbdlight_notify_level() and
> kbdlight_notify_change() helpers, in preparation for reusing the former
> when writing a level through the proc interface in a subsequent change.
> 
> kbdlight_notify_level() expects the caller to hold kbdlight_mutex;
> kbdlight_notify_change() queries the EC and notifies under the same
> mutex, like the original inline code did.
> 
> No functional change intended.
> 
> Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
> ---
>  drivers/platform/x86/lenovo/thinkpad_acpi.c | 40 +++++++++++++--------
>  1 file changed, 26 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> index 2c3f8cf32..c8098f317 100644
> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> @@ -5317,6 +5317,31 @@ static void kbdlight_resume(void)
>  	led_classdev_resume(&tpacpi_led_kbdlight.led_classdev);
>  }
>  
> +/*
> + * Caller must hold kbdlight_mutex.

Annotate it with lockdep instead of comments.

> + */
> +static void kbdlight_notify_level(int level)
> +{
> +	struct led_classdev *led_cdev = &tpacpi_led_kbdlight.led_classdev;
> +
> +	/*
> +	 * Check the brightness actually changed, setting the brightness
> +	 * through kbdlight_set_level() also triggers this event.
> +	 */
> +	if (level == kbdlight_brightness)
> +		return;
> +
> +	kbdlight_brightness = level;
> +	led_classdev_notify_brightness_hw_changed(led_cdev, level);
> +}
> +
> +static void kbdlight_notify_change(void)
> +{
> +	guard(mutex)(&kbdlight_mutex);
> +
> +	kbdlight_notify_level(kbdlight_sysfs_get(NULL));
> +}
> +
>  static struct ibm_struct kbdlight_driver_data = {
>  	.name = "kbdlight",
>  	.read = kbdlight_read,
> @@ -11586,20 +11611,7 @@ static bool tpacpi_driver_event(const unsigned int hkey_event)
>  		return false;
>  	case TP_HKEY_EV_KBD_LIGHT:
>  		if (tp_features.kbdlight) {
> -			enum led_brightness brightness;
> -
> -			guard(mutex)(&kbdlight_mutex);
> -
> -			/*
> -			 * Check the brightness actually changed, setting the brightness
> -			 * through kbdlight_set_level() also triggers this event.
> -			 */
> -			brightness = kbdlight_sysfs_get(NULL);
> -			if (kbdlight_brightness != brightness) {
> -				kbdlight_brightness = brightness;
> -				led_classdev_notify_brightness_hw_changed(
> -					&tpacpi_led_kbdlight.led_classdev, brightness);
> -			}
> +			kbdlight_notify_change();
>  		}

Remove the {}.

With them fixed,

Reviewed-by: Rong Zhang <i@rong.moe>

Thanks,
Rong

>  		/* Key events are suppressed by default hotkey_user_mask */
>  		return false;