Re: [PATCH v8 1/7] platform/x86/thinkpad_acpi: Stop setting led_classdev brightness directly

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 01-03-17 12:27, Pali Rohár wrote:
> On Thursday 09 February 2017 16:44:11 Hans de Goede wrote:
>> There is no need to set the led_classdev's brightness value from
>> its set_brightness callback, this is taken care of by the led-core and
>> thinkpad_acpi really should not be mucking with it.
>>
>> Note that kbdlight_set_level_and_update() is still used by the old
>> thinpad_acpi specific sysfs interface for the led, so we cannot
>> remove it.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v8:
>> -New patch in v8 of this patch-set
>> ---
>>  drivers/platform/x86/thinkpad_acpi.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>> index cacb43f..0680bb3 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -5095,8 +5095,6 @@ 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;
>> @@ -5164,7 +5162,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_and_update(data->new_state);
>> +		kbdlight_set_level(data->new_state);
>>  }
>>
>>  static void kbdlight_sysfs_set(struct led_classdev *led_cdev,
>
> Hi! This change was added in commit afcedebc6a0 (thinkpad_acpi: save
> kbdlight state on suspend and restore it on resume). It is really OK to
> revert that change?

Yes, as explained on the commit msg, kbdlight_set_worker is only used
from ledclass callbacks and the led core already update led_cdev->brightness
when calling those.

Regards,

Hans