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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



HI,

On 01-03-17 13:00, Pali Rohár wrote:
On Wednesday 01 March 2017 12:57:42 Hans de Goede wrote:
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@xxxxxxxxxx>
---
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.

Ok, I wrote it because of adding Marco to discussion so he has chance to
comment this change -- as it is basically revert of his commit...

No it is not, it removes a one line change he made amongst many others,
the important parts of his commit are in other parts of it.

Regards,

Hans



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux