Re: [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume

["Pali Rohár" <pali.rohar@gmail.com>]

On Thursday 19 May 2016 03:23:02 Marco Trevisan wrote:
> From: Marco Trevisan (Treviño) <mail@3v1n0.net>
> 
> Override default LED class suspend/resume handles, by keeping track of
> the brightness level before suspending so that it can be automatically
> restored on resume by calling default resume handler.
> 
> Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 43 +++++++++++++++++++++++++++++++++---
>  1 file changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 1f9783c..10111c2 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -5041,6 +5041,8 @@ static int kbdlight_set_level(int level)
>  	return 0;
>  }
>  
> +static int kbdlight_set_level_and_update(int level);
> +
>  static int kbdlight_get_level(void)
>  {
>  	int status = 0;
> @@ -5108,7 +5110,7 @@ static void kbdlight_set_worker(struct work_struct *work)
>  			container_of(work, struct tpacpi_led_classdev, work);
>  
>  	if (likely(tpacpi_lifecycle == TPACPI_LIFE_RUNNING))
> -		kbdlight_set_level(data->new_state);
> +		kbdlight_set_level_and_update(data->new_state);
>  }
>  
>  static void kbdlight_sysfs_set(struct led_classdev *led_cdev,
> @@ -5139,7 +5141,6 @@ static struct tpacpi_led_classdev tpacpi_led_kbdlight = {
>  		.max_brightness	= 2,
>  		.brightness_set	= &kbdlight_sysfs_set,
>  		.brightness_get	= &kbdlight_sysfs_get,
> -		.flags		= LED_CORE_SUSPENDRESUME,
>  	}
>  };
>  
> @@ -5177,6 +5178,20 @@ static void kbdlight_exit(void)
>  	flush_workqueue(tpacpi_wq);
>  }
>  
> +static int kbdlight_set_level_and_update(int level)
> +{
> +	int ret;
> +	struct led_classdev *led_cdev;
> +
> +	ret = kbdlight_set_level(level);
> +	led_cdev = &tpacpi_led_kbdlight.led_classdev;
> +
> +	if (ret == 0 && !(led_cdev->flags & LED_SUSPENDED))
> +		led_cdev->brightness = level;
> +
> +	return ret;
> +}
> +
>  static int kbdlight_read(struct seq_file *m)
>  {
>  	int level;
> @@ -5217,13 +5232,35 @@ static int kbdlight_write(char *buf)
>  	if (level == -1)
>  		return -EINVAL;
>  
> -	return kbdlight_set_level(level);
> +	return kbdlight_set_level_and_update(level);
> +}
> +
> +static void kbdlight_suspend(void)
> +{
> +	struct led_classdev *led_cdev;
> +
> +	if (!tp_features.kbdlight)
> +		return;
> +
> +	led_cdev = &tpacpi_led_kbdlight.led_classdev;
> +	led_update_brightness(led_cdev);
> +	led_classdev_suspend(led_cdev);
> +}
> +
> +static void kbdlight_resume(void)
> +{
> +	if (!tp_features.kbdlight)
> +		return;
> +
> +	led_classdev_resume(&tpacpi_led_kbdlight.led_classdev);
>  }
>  
>  static struct ibm_struct kbdlight_driver_data = {
>  	.name = "kbdlight",
>  	.read = kbdlight_read,
>  	.write = kbdlight_write,
> +	.suspend = kbdlight_suspend,
> +	.resume = kbdlight_resume,
>  	.exit = kbdlight_exit,
>  };
>  

For me whole patch looks like a big hack because LED_CORE_SUSPENDRESUME
does not work correctly... I would rather see fixed support for flag
LED_CORE_SUSPENDRESUME, not adding another suspend/resume hook if
possible. Any idea?

-- 
Pali Rohár
pali.rohar@gmail.com