All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
@ 2016-05-19  1:23 Marco Trevisan
  2016-05-19  7:52 ` Pali Rohár
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Marco Trevisan @ 2016-05-19  1:23 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: Henrique de Moraes Holschuh, Darren Hart, Pali Rohár,
	Marco Trevisan (Treviño)

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,
 };
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-19  1:23 [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume Marco Trevisan
@ 2016-05-19  7:52 ` Pali Rohár
  2016-05-20  3:03   ` Marco Trevisan (Treviño)
  2016-05-23 21:57 ` Darren Hart
  2016-05-23 22:39 ` [PATCH v2] " Marco Trevisan
  2 siblings, 1 reply; 15+ messages in thread
From: Pali Rohár @ 2016-05-19  7:52 UTC (permalink / raw)
  To: Marco Trevisan
  Cc: platform-driver-x86, Henrique de Moraes Holschuh, Darren Hart

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-19  7:52 ` Pali Rohár
@ 2016-05-20  3:03   ` Marco Trevisan (Treviño)
  2016-05-23 22:09     ` Darren Hart
  0 siblings, 1 reply; 15+ messages in thread
From: Marco Trevisan (Treviño) @ 2016-05-20  3:03 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Marco Trevisan, platform-driver-x86, Henrique de Moraes Holschuh,
	Darren Hart

2016-05-19 9:52 GMT+02:00 Pali Rohár <pali.rohar@gmail.com>:
> 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?

As I wrote you in private mail I think the only way we have is to keep
the led_cdev->brightness in sync with actual state. And to do this, we
can listen the ACPI event, keeping this like a private event that we
just consume to update led brightness value, without delivering it to
userspace.

However, the custom code needed for getting this done, is probably way
more than the one I added here.
Maybe it could be useful to others to read the brightness level before
suspending, and in this case we could add a new flag to the led
subsystem, but not sure it's really worth.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-19  1:23 [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume Marco Trevisan
  2016-05-19  7:52 ` Pali Rohár
@ 2016-05-23 21:57 ` Darren Hart
  2016-05-23 22:04   ` Pali Rohár
                     ` (2 more replies)
  2016-05-23 22:39 ` [PATCH v2] " Marco Trevisan
  2 siblings, 3 replies; 15+ messages in thread
From: Darren Hart @ 2016-05-23 21:57 UTC (permalink / raw)
  To: Marco Trevisan
  Cc: platform-driver-x86, Henrique de Moraes Holschuh, Pali Rohár

On Thu, May 19, 2016 at 03:23:02AM +0200, 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>

The patch requires a Signed-off-by. I presume that should be from Marco?

Henrique, is this Acked-by from you accurate? I need to see maintainers respond
on list as they would likely object to me just taking other people's word for it
in other scenarios ;-)

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-23 21:57 ` Darren Hart
@ 2016-05-23 22:04   ` Pali Rohár
  2016-05-23 22:10     ` Marco Trevisan (Treviño)
  2016-05-23 22:08   ` Marco Trevisan (Treviño)
  2016-05-24 15:27   ` Henrique de Moraes Holschuh
  2 siblings, 1 reply; 15+ messages in thread
From: Pali Rohár @ 2016-05-23 22:04 UTC (permalink / raw)
  To: Darren Hart
  Cc: Marco Trevisan, platform-driver-x86, Henrique de Moraes Holschuh

[-- Attachment #1: Type: Text/Plain, Size: 972 bytes --]

On Monday 23 May 2016 23:57:52 Darren Hart wrote:
> On Thu, May 19, 2016 at 03:23:02AM +0200, 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>
> 
> The patch requires a Signed-off-by. I presume that should be from
> Marco?
> 
> Henrique, is this Acked-by from you accurate? I need to see
> maintainers respond on list as they would likely object to me just
> taking other people's word for it in other scenarios ;-)

Darren, I'm not sure if this is really correct approach in current 
patch... Anyway as Marco wrote, there is ACPI event when keyboard 
backlight change and maybe we should handle it...

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-23 21:57 ` Darren Hart
  2016-05-23 22:04   ` Pali Rohár
@ 2016-05-23 22:08   ` Marco Trevisan (Treviño)
  2016-05-23 22:20     ` Darren Hart
  2016-05-24 15:27   ` Henrique de Moraes Holschuh
  2 siblings, 1 reply; 15+ messages in thread
From: Marco Trevisan (Treviño) @ 2016-05-23 22:08 UTC (permalink / raw)
  To: Darren Hart
  Cc: platform-driver-x86, Henrique de Moraes Holschuh, Pali Rohár

2016-05-23 23:57 GMT+02:00 Darren Hart <dvhart@infradead.org>:
> On Thu, May 19, 2016 at 03:23:02AM +0200, 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>
>
> The patch requires a Signed-off-by. I presume that should be from Marco?

I'm quite new to this, so please let me know how I should proceed.

> Henrique, is this Acked-by from you accurate? I need to see maintainers respond
> on list as they would likely object to me just taking other people's word for it
> in other scenarios ;-)

I've added it based on this reply (on ibm-acpi ML):
 - http://thread.gmane.org/gmane.linux.acpi.ibm-acpi.devel/4006/focus=4007

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-20  3:03   ` Marco Trevisan (Treviño)
@ 2016-05-23 22:09     ` Darren Hart
  0 siblings, 0 replies; 15+ messages in thread
From: Darren Hart @ 2016-05-23 22:09 UTC (permalink / raw)
  To: Marco Trevisan (Treviño)
  Cc: Pali Rohár, platform-driver-x86, Henrique de Moraes Holschuh

On Fri, May 20, 2016 at 05:03:52AM +0200, Marco Trevisan (Treviño) wrote:
> 2016-05-19 9:52 GMT+02:00 Pali Rohár <pali.rohar@gmail.com>:
> > 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?
> 
> As I wrote you in private mail I think the only way we have is to keep
> the led_cdev->brightness in sync with actual state. And to do this, we
> can listen the ACPI event, keeping this like a private event that we
> just consume to update led brightness value, without delivering it to
> userspace.
> 
> However, the custom code needed for getting this done, is probably way
> more than the one I added here.
> Maybe it could be useful to others to read the brightness level before
> suspending, and in this case we could add a new flag to the led
> subsystem, but not sure it's really worth.
> 

At the risk of contributing to "the platform platform", I agree with a local fix
here in the near term. If this becomes a pattern, we can look to move this into
the LED subsystem. As it is, this is working as designed, overriding when
necessary.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-23 22:04   ` Pali Rohár
@ 2016-05-23 22:10     ` Marco Trevisan (Treviño)
  2016-05-23 22:21       ` Darren Hart
  0 siblings, 1 reply; 15+ messages in thread
From: Marco Trevisan (Treviño) @ 2016-05-23 22:10 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Darren Hart, platform-driver-x86, Henrique de Moraes Holschuh

2016-05-24 0:04 GMT+02:00 Pali Rohár <pali.rohar@gmail.com>:
> Darren, I'm not sure if this is really correct approach in current
> patch... Anyway as Marco wrote, there is ACPI event when keyboard
> backlight change and maybe we should handle it...

I'll be happy to work on that as well, but I've not much time at the moment.
However I think this could be an ok way to handle things in the mean time.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-23 22:08   ` Marco Trevisan (Treviño)
@ 2016-05-23 22:20     ` Darren Hart
  0 siblings, 0 replies; 15+ messages in thread
From: Darren Hart @ 2016-05-23 22:20 UTC (permalink / raw)
  To: Marco Trevisan (Treviño)
  Cc: platform-driver-x86, Henrique de Moraes Holschuh, Pali Rohár

On Tue, May 24, 2016 at 12:08:46AM +0200, Marco Trevisan (Treviño) wrote:
> 2016-05-23 23:57 GMT+02:00 Darren Hart <dvhart@infradead.org>:
> > On Thu, May 19, 2016 at 03:23:02AM +0200, 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>
> >
> > The patch requires a Signed-off-by. I presume that should be from Marco?
> 
> I'm quite new to this, so please let me know how I should proceed.

Any time you submit a patch, you must provide a Signed-off-by in the commit
message. Git will do this for you with "git commit --signoff". See:

Documentation/SubmittingPatches, Section 11: Sign your work

> 
> > Henrique, is this Acked-by from you accurate? I need to see maintainers respond
> > on list as they would likely object to me just taking other people's word for it
> > in other scenarios ;-)
> 
> I've added it based on this reply (on ibm-acpi ML):
>  - http://thread.gmane.org/gmane.linux.acpi.ibm-acpi.devel/4006/focus=4007
> 

Ah, excellent. I didn't have the original since I wasn't on Cc initially. That's
all I need from Henrique.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-23 22:10     ` Marco Trevisan (Treviño)
@ 2016-05-23 22:21       ` Darren Hart
  0 siblings, 0 replies; 15+ messages in thread
From: Darren Hart @ 2016-05-23 22:21 UTC (permalink / raw)
  To: Marco Trevisan (Treviño)
  Cc: Pali Rohár, platform-driver-x86, Henrique de Moraes Holschuh

On Tue, May 24, 2016 at 12:10:44AM +0200, Marco Trevisan (Treviño) wrote:
> 2016-05-24 0:04 GMT+02:00 Pali Rohár <pali.rohar@gmail.com>:
> > Darren, I'm not sure if this is really correct approach in current
> > patch... Anyway as Marco wrote, there is ACPI event when keyboard
> > backlight change and maybe we should handle it...
> 
> I'll be happy to work on that as well, but I've not much time at the moment.
> However I think this could be an ok way to handle things in the mean time.
> 

I believe so, and Henrique has approved.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-19  1:23 [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume Marco Trevisan
  2016-05-19  7:52 ` Pali Rohár
  2016-05-23 21:57 ` Darren Hart
@ 2016-05-23 22:39 ` Marco Trevisan
  2016-05-24 15:49   ` Darren Hart
  2 siblings, 1 reply; 15+ messages in thread
From: Marco Trevisan @ 2016-05-23 22:39 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: Henrique de Moraes Holschuh, Darren Hart, Pali Rohár,
	Marco Trevisan (Treviño)

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>
Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
---
 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,
 };
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-23 21:57 ` Darren Hart
  2016-05-23 22:04   ` Pali Rohár
  2016-05-23 22:08   ` Marco Trevisan (Treviño)
@ 2016-05-24 15:27   ` Henrique de Moraes Holschuh
  2 siblings, 0 replies; 15+ messages in thread
From: Henrique de Moraes Holschuh @ 2016-05-24 15:27 UTC (permalink / raw)
  To: Darren Hart, Marco Trevisan; +Cc: platform-driver-x86, Pali Rohár

On Mon, May 23, 2016, at 18:57, Darren Hart wrote:
> On Thu, May 19, 2016 at 03:23:02AM +0200, 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>
> 
> The patch requires a Signed-off-by. I presume that should be from Marco?
> 
> Henrique, is this Acked-by from you accurate? I need to see maintainers
> respond
> on list as they would likely object to me just taking other people's word
> for it
> in other scenarios ;-)

Yeah, I acked it.  Sorry it took a few days to reply to this email.

This is not the "best" fix, but it is by far the less troublesome one:
hooking to the internal event from the thinkpad DSDT and updating the
brightness shadow register in the driver should work, but that will
require testing as we cannot really take for granted that we will get
that event on every firmware out there without going over the DSDT AML
of a lot of models.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-23 22:39 ` [PATCH v2] " Marco Trevisan
@ 2016-05-24 15:49   ` Darren Hart
  0 siblings, 0 replies; 15+ messages in thread
From: Darren Hart @ 2016-05-24 15:49 UTC (permalink / raw)
  To: Marco Trevisan
  Cc: platform-driver-x86, Henrique de Moraes Holschuh, Pali Rohár

On Tue, May 24, 2016 at 12:39:51AM +0200, 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>
> Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>

Just a nit (I fixed it), your Signed-off-by comes first. Reviewers and testers
follow, then the sub maintainers Signed-off-by, then Linus' Signed-off-by. It
tracks the path the patch took from authorship, testing, review, pull, and
merge.

Queued, thanks.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
@ 2016-05-31 17:02 Marco Trevisan
  2016-06-18 16:53 ` Greg KH
  0 siblings, 1 reply; 15+ messages in thread
From: Marco Trevisan @ 2016-05-31 17:02 UTC (permalink / raw)
  To: stable

Backport for v4.5 and v4.6 of
commit afcedebc6a094224973534f43b396bbbf33fe44e upstream.

Fixes resuming of keyboard backlight for ThinkPad laptops after suspension


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.

Signed-off-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Signed-off-by: Darren Hart <dvhart@linux.intel.com>
---
 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 e305ab5..5f09f16 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5001,6 +5001,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;
@@ -5068,7 +5070,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,
@@ -5099,7 +5101,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,
 	}
 };
 
@@ -5137,6 +5138,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;
@@ -5177,13 +5192,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,
 };
 

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume
  2016-05-31 17:02 [PATCH] " Marco Trevisan
@ 2016-06-18 16:53 ` Greg KH
  0 siblings, 0 replies; 15+ messages in thread
From: Greg KH @ 2016-06-18 16:53 UTC (permalink / raw)
  To: Marco Trevisan; +Cc: stable

On Tue, May 31, 2016 at 07:02:53PM +0200, Marco Trevisan wrote:
> Backport for v4.5 and v4.6 of
> commit afcedebc6a094224973534f43b396bbbf33fe44e upstream.
> 
> Fixes resuming of keyboard backlight for ThinkPad laptops after suspension
> 
> 
> 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.
> 
> Signed-off-by: Marco Trevisan (Trevi�o) <mail@3v1n0.net>
> Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 43 +++++++++++++++++++++++++++++++++---
>  1 file changed, 40 insertions(+), 3 deletions(-)

This looks like a new feature to me, which isn't really stable material,
sorry.

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-06-18 16:53 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-19  1:23 [PATCH] thinkpad_acpi: save kbdlight state on suspend and restore it on resume Marco Trevisan
2016-05-19  7:52 ` Pali Rohár
2016-05-20  3:03   ` Marco Trevisan (Treviño)
2016-05-23 22:09     ` Darren Hart
2016-05-23 21:57 ` Darren Hart
2016-05-23 22:04   ` Pali Rohár
2016-05-23 22:10     ` Marco Trevisan (Treviño)
2016-05-23 22:21       ` Darren Hart
2016-05-23 22:08   ` Marco Trevisan (Treviño)
2016-05-23 22:20     ` Darren Hart
2016-05-24 15:27   ` Henrique de Moraes Holschuh
2016-05-23 22:39 ` [PATCH v2] " Marco Trevisan
2016-05-24 15:49   ` Darren Hart
  -- strict thread matches above, loose matches on Subject: below --
2016-05-31 17:02 [PATCH] " Marco Trevisan
2016-06-18 16:53 ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.