linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] thinkpad_acpi: avoid ACPI Warning while operating non-exist LEDs
@ 2013-06-07  8:20 Adam Lee
  2013-06-07  8:20 ` [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs Adam Lee
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Adam Lee @ 2013-06-07  8:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: platform-driver-x86, ibm-acpi-devel, Matthew Garrett,
	Henrique de Moraes Holschuh, Alex Hung, Bjorn Mork

Some new Lenovo or ThinkPad laptops don't have EC controllable LEDs,
they control LEDs by BIOS functions, and return ACPI Warning while
operating LEDs by EC.

These patches set some quirks, optimize led_init() and led_write() to
avoid that warning.

Adam Lee (3):
  thinkpad_acpi: return -NODEV while operating uninitialized LEDs
  thinkpad_acpi: add the ability setting TPACPI_LED_NONE by quirk
  thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs

 drivers/platform/x86/thinkpad_acpi.c |   24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

-- 
1.7.10.4


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

* [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs
  2013-06-07  8:20 [PATCH 0/3] thinkpad_acpi: avoid ACPI Warning while operating non-exist LEDs Adam Lee
@ 2013-06-07  8:20 ` Adam Lee
  2013-06-07  8:53   ` Bjørn Mork
  2013-06-18  0:12   ` Henrique de Moraes Holschuh
  2013-06-07  8:20 ` [PATCH 2/3] thinkpad_acpi: add the ability setting TPACPI_LED_NONE by quirk Adam Lee
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 18+ messages in thread
From: Adam Lee @ 2013-06-07  8:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: platform-driver-x86, ibm-acpi-devel, Matthew Garrett,
	Henrique de Moraes Holschuh, Alex Hung, Bjorn Mork

Not all 0-15 LEDs are available for all models, sometimes it's even not
safe. This patch return -NODEV while operating uninitialized LEDs.

Signed-off-by: Adam Lee <adam.lee@canonical.com>
---
 drivers/platform/x86/thinkpad_acpi.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 54d31c0..bea57ae 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5401,9 +5401,12 @@ static int led_write(char *buf)
 		return -ENODEV;
 
 	while ((cmd = next_cmd(&buf))) {
-		if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > 15)
+		if (sscanf(cmd, "%d", &led) != 1)
 			return -EINVAL;
 
+		if (!tpacpi_leds[led].led)
+			return -ENODEV;
+
 		if (strstr(cmd, "off")) {
 			s = TPACPI_LED_OFF;
 		} else if (strstr(cmd, "on")) {
-- 
1.7.10.4


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

* [PATCH 2/3] thinkpad_acpi: add the ability setting TPACPI_LED_NONE by quirk
  2013-06-07  8:20 [PATCH 0/3] thinkpad_acpi: avoid ACPI Warning while operating non-exist LEDs Adam Lee
  2013-06-07  8:20 ` [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs Adam Lee
@ 2013-06-07  8:20 ` Adam Lee
  2013-06-18  0:19   ` Henrique de Moraes Holschuh
  2013-06-07  8:20 ` [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs Adam Lee
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Adam Lee @ 2013-06-07  8:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: platform-driver-x86, ibm-acpi-devel, Matthew Garrett,
	Henrique de Moraes Holschuh, Alex Hung, Bjorn Mork

Some new Lenovo or ThinkPad laptops don't have EC controllable LEDs,
their LED quirks are 0. This patch set led_supported=TPACPI_LED_NONE
when quirk equals 0.

Signed-off-by: Adam Lee <adam.lee@canonical.com>
---
 drivers/platform/x86/thinkpad_acpi.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index bea57ae..2fb1f1c 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5327,6 +5327,16 @@ static int __init led_init(struct ibm_init_struct *iibm)
 
 	led_supported = led_init_detect_mode();
 
+	if (led_supported != TPACPI_LED_NONE) {
+		useful_leds = tpacpi_check_quirks(led_useful_qtable,
+				ARRAY_SIZE(led_useful_qtable));
+
+		if (!useful_leds) {
+			led_handle = NULL;
+			led_supported = TPACPI_LED_NONE;
+		}
+	}
+
 	vdbg_printk(TPACPI_DBG_INIT, "LED commands are %s, mode %d\n",
 		str_supported(led_supported), led_supported);
 
@@ -5340,9 +5350,6 @@ static int __init led_init(struct ibm_init_struct *iibm)
 		return -ENOMEM;
 	}
 
-	useful_leds = tpacpi_check_quirks(led_useful_qtable,
-					  ARRAY_SIZE(led_useful_qtable));
-
 	for (i = 0; i < TPACPI_LED_NUMLEDS; i++) {
 		if (!tpacpi_is_led_restricted(i) &&
 		    test_bit(i, &useful_leds)) {
-- 
1.7.10.4


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

* [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs
  2013-06-07  8:20 [PATCH 0/3] thinkpad_acpi: avoid ACPI Warning while operating non-exist LEDs Adam Lee
  2013-06-07  8:20 ` [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs Adam Lee
  2013-06-07  8:20 ` [PATCH 2/3] thinkpad_acpi: add the ability setting TPACPI_LED_NONE by quirk Adam Lee
@ 2013-06-07  8:20 ` Adam Lee
  2013-06-07 13:22   ` Matthew Garrett
  2013-06-18  0:20   ` Henrique de Moraes Holschuh
  2013-06-07  9:05 ` [PATCH v2] thinkpad_acpi: return -NODEV while operating uninitialized LEDs Adam Lee
  2013-06-08  8:51 ` [PATCH 1/3 v3] " Adam Lee
  4 siblings, 2 replies; 18+ messages in thread
From: Adam Lee @ 2013-06-07  8:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: platform-driver-x86, ibm-acpi-devel, Matthew Garrett,
	Henrique de Moraes Holschuh, Alex Hung, Bjorn Mork

Some new Lenovo or ThinkPad laptops don't have EC controllable LEDs.
This patch adds their quirks.

Signed-off-by: Adam Lee <adam.lee@canonical.com>
---
 drivers/platform/x86/thinkpad_acpi.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 2fb1f1c..4cc7d73 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5268,6 +5268,12 @@ static const struct tpacpi_quirk led_useful_qtable[] __initconst = {
 	TPACPI_Q_IBM('7', 'F', 0x1f97), /* Z61* (1) */
 	TPACPI_Q_IBM('7', 'B', 0x1fb7), /* X60 (1) */
 
+	/* Models don't have EC controllable LEDs */
+	TPACPI_Q_LNV('G', 'D', 0), /* ThinkPad Twist */
+	TPACPI_Q_LNV('H', '0', 0), /* ThinkPad E430 */
+	TPACPI_Q_LNV('H', 'C', 0), /* Lenovo E49L */
+	TPACPI_Q_LNV('H', 'D', 0), /* Lenovo K4350 */
+
 	/* (1) - may have excess leds enabled on MSB */
 
 	/* Defaults (order matters, keep last, don't reorder!) */
-- 
1.7.10.4


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

* Re: [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs
  2013-06-07  8:20 ` [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs Adam Lee
@ 2013-06-07  8:53   ` Bjørn Mork
  2013-06-07  8:59     ` Adam Lee
  2013-06-18  0:12   ` Henrique de Moraes Holschuh
  1 sibling, 1 reply; 18+ messages in thread
From: Bjørn Mork @ 2013-06-07  8:53 UTC (permalink / raw)
  To: Adam Lee
  Cc: linux-kernel, platform-driver-x86, ibm-acpi-devel,
	Matthew Garrett, Henrique de Moraes Holschuh, Alex Hung

Adam Lee <adam.lee@canonical.com> writes:

> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -5401,9 +5401,12 @@ static int led_write(char *buf)
>  		return -ENODEV;
>  
>  	while ((cmd = next_cmd(&buf))) {
> -		if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > 15)
> +		if (sscanf(cmd, "%d", &led) != 1)
>  			return -EINVAL;
>  
> +		if (!tpacpi_leds[led].led)
> +			return -ENODEV;

This looks risky.  Why did you remove the index sanity check?  What will
happen now if the input is e.g "-1" or "42"?

BTW, the magic number 15 should probably be (TPACPI_LED_NUMLEDS - 1)
instead.



Bjørn

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

* Re: [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs
  2013-06-07  8:53   ` Bjørn Mork
@ 2013-06-07  8:59     ` Adam Lee
  0 siblings, 0 replies; 18+ messages in thread
From: Adam Lee @ 2013-06-07  8:59 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: linux-kernel, platform-driver-x86, ibm-acpi-devel,
	Matthew Garrett, Henrique de Moraes Holschuh, Alex Hung

On Fri, Jun 07, 2013 at 10:53:50AM +0200, Bjørn Mork wrote:
> Adam Lee <adam.lee@canonical.com> writes:
> 
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -5401,9 +5401,12 @@ static int led_write(char *buf)
> >  		return -ENODEV;
> >  
> >  	while ((cmd = next_cmd(&buf))) {
> > -		if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > 15)
> > +		if (sscanf(cmd, "%d", &led) != 1)
> >  			return -EINVAL;
> >  
> > +		if (!tpacpi_leds[led].led)
> > +			return -ENODEV;
> 
> This looks risky.  Why did you remove the index sanity check?  What will
> happen now if the input is e.g "-1" or "42"?
> 
> BTW, the magic number 15 should probably be (TPACPI_LED_NUMLEDS - 1)
> instead.

Oh, good point! I was thinking led_init() already checked the index.

Will submit patch v2, thanks.

-- 
Regards,
Adam Lee
Hardware Enablement

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

* [PATCH v2] thinkpad_acpi: return -NODEV while operating uninitialized LEDs
  2013-06-07  8:20 [PATCH 0/3] thinkpad_acpi: avoid ACPI Warning while operating non-exist LEDs Adam Lee
                   ` (2 preceding siblings ...)
  2013-06-07  8:20 ` [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs Adam Lee
@ 2013-06-07  9:05 ` Adam Lee
  2013-06-08  2:19   ` Amos Kong
  2013-06-08  8:51 ` [PATCH 1/3 v3] " Adam Lee
  4 siblings, 1 reply; 18+ messages in thread
From: Adam Lee @ 2013-06-07  9:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: platform-driver-x86, ibm-acpi-devel, Matthew Garrett,
	Henrique de Moraes Holschuh, Alex Hung, Bjorn Mork

Not all 0-15 LEDs are available for all models, sometimes it's even not
safe. This patch return -NODEV while operating uninitialized LEDs.

Signed-off-by: Adam Lee <adam.lee@canonical.com>
---
 drivers/platform/x86/thinkpad_acpi.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 54d31c0..d2ac4e8 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5401,9 +5401,12 @@ static int led_write(char *buf)
 		return -ENODEV;
 
 	while ((cmd = next_cmd(&buf))) {
-		if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > 15)
+		if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > (TPACPI_LED_NUMLEDS -1))
 			return -EINVAL;
 
+		if (!tpacpi_leds[led].led)
+			return -ENODEV;
+
 		if (strstr(cmd, "off")) {
 			s = TPACPI_LED_OFF;
 		} else if (strstr(cmd, "on")) {
-- 
1.7.10.4


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

* Re: [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs
  2013-06-07  8:20 ` [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs Adam Lee
@ 2013-06-07 13:22   ` Matthew Garrett
  2013-06-07 15:06     ` Adam Lee
  2013-06-18  0:20   ` Henrique de Moraes Holschuh
  1 sibling, 1 reply; 18+ messages in thread
From: Matthew Garrett @ 2013-06-07 13:22 UTC (permalink / raw)
  To: Adam Lee
  Cc: linux-kernel, platform-driver-x86, ibm-acpi-devel,
	Henrique de Moraes Holschuh, Alex Hung, Bjorn Mork

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 559 bytes --]

On Fri, 2013-06-07 at 16:20 +0800, Adam Lee wrote:
> Some new Lenovo or ThinkPad laptops don't have EC controllable LEDs.
> This patch adds their quirks.

Can you detect this case? If this is a new way of doing things then
you're going to have to add new entries to the quirk list every time
Lenovo release a new machine, which isn't really going to work.

-- 
Matthew Garrett | mjg59@srcf.ucam.org
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* Re: [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs
  2013-06-07 13:22   ` Matthew Garrett
@ 2013-06-07 15:06     ` Adam Lee
  2013-06-07 20:16       ` Henrique de Moraes Holschuh
  0 siblings, 1 reply; 18+ messages in thread
From: Adam Lee @ 2013-06-07 15:06 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-kernel, platform-driver-x86, ibm-acpi-devel,
	Henrique de Moraes Holschuh, Alex Hung, Bjorn Mork

On Fri, Jun 07, 2013 at 01:22:39PM +0000, Matthew Garrett wrote:
> On Fri, 2013-06-07 at 16:20 +0800, Adam Lee wrote:
> > Some new Lenovo or ThinkPad laptops don't have EC controllable LEDs.
> > This patch adds their quirks.
> 
> Can you detect this case? If this is a new way of doing things then
> you're going to have to add new entries to the quirk list every time
> Lenovo release a new machine, which isn't really going to work.

Thanks. The difference is their LED handle expects only one argument,
not two. Does kernel have a method detecting acpi_handle's arguments
number? (checking...)

About the new way:

Lenovo is going to assign a ACPI method to each operation. Like the
volume_toggle button and LED, hardware toggle is "SHDA", getting state
is "GSMS", setting state is "SSMS".

About the problem:

Some models only support the new way but containing internal EC LED
interfaces which makes our detecting failed at the same time. The new
interfaces accept only one argument(on, off, or blink), we couldn't
control LEDs by them because can't specify the LED number. If you give
them two arguments, it returns ACPI warning, so I wanna add these "0"
quirks.

-- 
Regards,
Adam Lee
Hardware Enablement

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

* Re: [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs
  2013-06-07 15:06     ` Adam Lee
@ 2013-06-07 20:16       ` Henrique de Moraes Holschuh
  2013-06-08  9:04         ` Adam Lee
  0 siblings, 1 reply; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2013-06-07 20:16 UTC (permalink / raw)
  To: Adam Lee, Matthew Garrett
  Cc: linux-kernel, platform-driver-x86, ibm-acpi-devel, Alex Hung, Bjorn Mork

Can you get me the ACPI interface documentation?  It should be simple to get it, but I haven't emailed the Lenovo BIOS engineers for some time.

Basically, create a new LED method in thinkpad-acpi, lock it down to vendor lenovo, detect the new interfaces, and only check the two-argument LED one if the newer ones aren't found.

Not supporting the old /proc interface on new thinkpads is fine.

-- 
  "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] 18+ messages in thread

* Re: [PATCH v2] thinkpad_acpi: return -NODEV while operating uninitialized LEDs
  2013-06-07  9:05 ` [PATCH v2] thinkpad_acpi: return -NODEV while operating uninitialized LEDs Adam Lee
@ 2013-06-08  2:19   ` Amos Kong
  0 siblings, 0 replies; 18+ messages in thread
From: Amos Kong @ 2013-06-08  2:19 UTC (permalink / raw)
  To: Adam Lee
  Cc: linux-kernel, platform-driver-x86, ibm-acpi-devel,
	Matthew Garrett, Henrique de Moraes Holschuh, Alex Hung,
	Bjorn Mork

On Fri, Jun 7, 2013 at 5:05 PM, Adam Lee <adam.lee@canonical.com> wrote:
>
> Not all 0-15 LEDs are available for all models, sometimes it's even not
> safe. This patch return -NODEV while operating uninitialized LEDs.
>
> Signed-off-by: Adam Lee <adam.lee@canonical.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 54d31c0..d2ac4e8 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -5401,9 +5401,12 @@ static int led_write(char *buf)
>                 return -ENODEV;
>
>         while ((cmd = next_cmd(&buf))) {
> -               if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > 15)
> +               if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > (TPACPI_LED_NUMLEDS -1))
>                         return -EINVAL;
>
> +               if (!tpacpi_leds[led].led)


The .led filed might be set to '0' in led_init(), how about set the
uninitiated .led to -1? and check by "if (tpacpi_leds[led].led < 0)"

        for (i = 0; i < TPACPI_LED_NUMLEDS; i++) {
                if (!tpacpi_is_led_restricted(i) &&
                    test_bit(i, &useful_leds)) {
                        rc = tpacpi_init_led(i);
                        if (rc < 0) {
                                led_exit();
                                return rc;
-                       }
+                       } else
+                               tpacpi_leds[led].led = -1;

                } else tpacpi_leds[led].led = -1;
        }


> +                       return -ENODEV;
> +
>                 if (strstr(cmd, "off")) {
>                         s = TPACPI_LED_OFF;
>                 } else if (strstr(cmd, "on")) {
> --
> 1.7.10.4

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

* [PATCH 1/3 v3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs
  2013-06-07  8:20 [PATCH 0/3] thinkpad_acpi: avoid ACPI Warning while operating non-exist LEDs Adam Lee
                   ` (3 preceding siblings ...)
  2013-06-07  9:05 ` [PATCH v2] thinkpad_acpi: return -NODEV while operating uninitialized LEDs Adam Lee
@ 2013-06-08  8:51 ` Adam Lee
  2013-06-18  0:17   ` Henrique de Moraes Holschuh
  4 siblings, 1 reply; 18+ messages in thread
From: Adam Lee @ 2013-06-08  8:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: platform-driver-x86, ibm-acpi-devel, Matthew Garrett,
	Henrique de Moraes Holschuh, Alex Hung, Bjorn Mork, Amos Kong

Not all 0-15 LEDs are available for all models, sometimes it's even not
safe. This patch return -NODEV while operating uninitialized LEDs.

Signed-off-by: Adam Lee <adam.lee@canonical.com>
---
 drivers/platform/x86/thinkpad_acpi.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 54d31c0..455384b 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -369,7 +369,7 @@ struct tpacpi_led_classdev {
 	struct led_classdev led_classdev;
 	struct work_struct work;
 	enum led_status_t new_state;
-	unsigned int led;
+	int led;
 };
 
 /* brightness level capabilities */
@@ -5344,6 +5344,8 @@ static int __init led_init(struct ibm_init_struct *iibm)
 					  ARRAY_SIZE(led_useful_qtable));
 
 	for (i = 0; i < TPACPI_LED_NUMLEDS; i++) {
+		tpacpi_leds[i].led = -1;
+
 		if (!tpacpi_is_led_restricted(i) &&
 		    test_bit(i, &useful_leds)) {
 			rc = tpacpi_init_led(i);
@@ -5401,9 +5403,13 @@ static int led_write(char *buf)
 		return -ENODEV;
 
 	while ((cmd = next_cmd(&buf))) {
-		if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > 15)
+		if (sscanf(cmd, "%d", &led) != 1)
 			return -EINVAL;
 
+		if (led < 0 || led > (TPACPI_LED_NUMLEDS - 1) ||
+				tpacpi_leds[led].led < 0)
+			return -ENODEV;
+
 		if (strstr(cmd, "off")) {
 			s = TPACPI_LED_OFF;
 		} else if (strstr(cmd, "on")) {
-- 
1.7.10.4


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

* Re: [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs
  2013-06-07 20:16       ` Henrique de Moraes Holschuh
@ 2013-06-08  9:04         ` Adam Lee
  0 siblings, 0 replies; 18+ messages in thread
From: Adam Lee @ 2013-06-08  9:04 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Matthew Garrett, linux-kernel, platform-driver-x86,
	ibm-acpi-devel, Alex Hung, Bjorn Mork

On Fri, Jun 07, 2013 at 05:16:15PM -0300, Henrique de Moraes Holschuh wrote:
> Can you get me the ACPI interface documentation?  It should be simple
> to get it, but I haven't emailed the Lenovo BIOS engineers for some
> time.
> 
> Basically, create a new LED method in thinkpad-acpi, lock it down to
> vendor lenovo, detect the new interfaces, and only check the
> two-argument LED one if the newer ones aren't found.
> 
> Not supporting the old /proc interface on new thinkpads is fine.

Thanks, I will request the documentation from Lenovo again.

Please apply "[PATCH 1/3 v3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs" and
"[PATCH 2/3] thinkpad_acpi: add the ability setting TPACPI_LED_NONE by quirk", but
hold "[PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs",
I will drop [PATCH 3/3] to discussion again when I got the documentation.

-- 
Regards,
Adam Lee
Hardware Enablement

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

* Re: [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs
  2013-06-07  8:20 ` [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs Adam Lee
  2013-06-07  8:53   ` Bjørn Mork
@ 2013-06-18  0:12   ` Henrique de Moraes Holschuh
  2013-06-18  0:18     ` Henrique de Moraes Holschuh
  1 sibling, 1 reply; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2013-06-18  0:12 UTC (permalink / raw)
  To: Adam Lee
  Cc: linux-kernel, platform-driver-x86, ibm-acpi-devel,
	Matthew Garrett, Henrique de Moraes Holschuh, Alex Hung,
	Bjorn Mork

On Fri, 07 Jun 2013, Adam Lee wrote:
> Not all 0-15 LEDs are available for all models, sometimes it's even not
> safe. This patch return -NODEV while operating uninitialized LEDs.
> 
> Signed-off-by: Adam Lee <adam.lee@canonical.com>
Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

> ---
>  drivers/platform/x86/thinkpad_acpi.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 54d31c0..bea57ae 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -5401,9 +5401,12 @@ static int led_write(char *buf)
>  		return -ENODEV;
>  
>  	while ((cmd = next_cmd(&buf))) {
> -		if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > 15)
> +		if (sscanf(cmd, "%d", &led) != 1)
>  			return -EINVAL;
>  
> +		if (!tpacpi_leds[led].led)
> +			return -ENODEV;
> +
>  		if (strstr(cmd, "off")) {
>  			s = TPACPI_LED_OFF;
>  		} else if (strstr(cmd, "on")) {

-- 
  "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] 18+ messages in thread

* Re: [PATCH 1/3 v3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs
  2013-06-08  8:51 ` [PATCH 1/3 v3] " Adam Lee
@ 2013-06-18  0:17   ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2013-06-18  0:17 UTC (permalink / raw)
  To: Adam Lee
  Cc: linux-kernel, platform-driver-x86, ibm-acpi-devel,
	Matthew Garrett, Henrique de Moraes Holschuh, Alex Hung,
	Bjorn Mork, Amos Kong

On Sat, 08 Jun 2013, Adam Lee wrote:
> Not all 0-15 LEDs are available for all models, sometimes it's even not
> safe. This patch return -NODEV while operating uninitialized LEDs.
> 
> Signed-off-by: Adam Lee <adam.lee@canonical.com>

Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

> ---
>  drivers/platform/x86/thinkpad_acpi.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 54d31c0..455384b 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -369,7 +369,7 @@ struct tpacpi_led_classdev {
>  	struct led_classdev led_classdev;
>  	struct work_struct work;
>  	enum led_status_t new_state;
> -	unsigned int led;
> +	int led;
>  };
>  
>  /* brightness level capabilities */
> @@ -5344,6 +5344,8 @@ static int __init led_init(struct ibm_init_struct *iibm)
>  					  ARRAY_SIZE(led_useful_qtable));
>  
>  	for (i = 0; i < TPACPI_LED_NUMLEDS; i++) {
> +		tpacpi_leds[i].led = -1;
> +
>  		if (!tpacpi_is_led_restricted(i) &&
>  		    test_bit(i, &useful_leds)) {
>  			rc = tpacpi_init_led(i);
> @@ -5401,9 +5403,13 @@ static int led_write(char *buf)
>  		return -ENODEV;
>  
>  	while ((cmd = next_cmd(&buf))) {
> -		if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > 15)
> +		if (sscanf(cmd, "%d", &led) != 1)
>  			return -EINVAL;
>  
> +		if (led < 0 || led > (TPACPI_LED_NUMLEDS - 1) ||
> +				tpacpi_leds[led].led < 0)
> +			return -ENODEV;
> +
>  		if (strstr(cmd, "off")) {
>  			s = TPACPI_LED_OFF;
>  		} else if (strstr(cmd, "on")) {

-- 
  "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] 18+ messages in thread

* Re: [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs
  2013-06-18  0:12   ` Henrique de Moraes Holschuh
@ 2013-06-18  0:18     ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2013-06-18  0:18 UTC (permalink / raw)
  To: Adam Lee
  Cc: linux-kernel, platform-driver-x86, ibm-acpi-devel,
	Matthew Garrett, Henrique de Moraes Holschuh, Alex Hung,
	Bjorn Mork

On Mon, 17 Jun 2013, Henrique de Moraes Holschuh wrote:
> On Fri, 07 Jun 2013, Adam Lee wrote:
> > Not all 0-15 LEDs are available for all models, sometimes it's even not
> > safe. This patch return -NODEV while operating uninitialized LEDs.
> > 
> > Signed-off-by: Adam Lee <adam.lee@canonical.com>
> Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

Matthew, please pick v3 of this patch, and not this one.

-- 
  "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] 18+ messages in thread

* Re: [PATCH 2/3] thinkpad_acpi: add the ability setting TPACPI_LED_NONE by quirk
  2013-06-07  8:20 ` [PATCH 2/3] thinkpad_acpi: add the ability setting TPACPI_LED_NONE by quirk Adam Lee
@ 2013-06-18  0:19   ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2013-06-18  0:19 UTC (permalink / raw)
  To: Adam Lee
  Cc: linux-kernel, platform-driver-x86, ibm-acpi-devel,
	Matthew Garrett, Henrique de Moraes Holschuh, Alex Hung,
	Bjorn Mork

On Fri, 07 Jun 2013, Adam Lee wrote:
> Some new Lenovo or ThinkPad laptops don't have EC controllable LEDs,
> their LED quirks are 0. This patch set led_supported=TPACPI_LED_NONE
> when quirk equals 0.
> 
> Signed-off-by: Adam Lee <adam.lee@canonical.com>

Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

> ---
>  drivers/platform/x86/thinkpad_acpi.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index bea57ae..2fb1f1c 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -5327,6 +5327,16 @@ static int __init led_init(struct ibm_init_struct *iibm)
>  
>  	led_supported = led_init_detect_mode();
>  
> +	if (led_supported != TPACPI_LED_NONE) {
> +		useful_leds = tpacpi_check_quirks(led_useful_qtable,
> +				ARRAY_SIZE(led_useful_qtable));
> +
> +		if (!useful_leds) {
> +			led_handle = NULL;
> +			led_supported = TPACPI_LED_NONE;
> +		}
> +	}
> +
>  	vdbg_printk(TPACPI_DBG_INIT, "LED commands are %s, mode %d\n",
>  		str_supported(led_supported), led_supported);
>  
> @@ -5340,9 +5350,6 @@ static int __init led_init(struct ibm_init_struct *iibm)
>  		return -ENOMEM;
>  	}
>  
> -	useful_leds = tpacpi_check_quirks(led_useful_qtable,
> -					  ARRAY_SIZE(led_useful_qtable));
> -
>  	for (i = 0; i < TPACPI_LED_NUMLEDS; i++) {
>  		if (!tpacpi_is_led_restricted(i) &&
>  		    test_bit(i, &useful_leds)) {

-- 
  "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] 18+ messages in thread

* Re: [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs
  2013-06-07  8:20 ` [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs Adam Lee
  2013-06-07 13:22   ` Matthew Garrett
@ 2013-06-18  0:20   ` Henrique de Moraes Holschuh
  1 sibling, 0 replies; 18+ messages in thread
From: Henrique de Moraes Holschuh @ 2013-06-18  0:20 UTC (permalink / raw)
  To: Adam Lee
  Cc: linux-kernel, platform-driver-x86, ibm-acpi-devel,
	Matthew Garrett, Henrique de Moraes Holschuh, Alex Hung,
	Bjorn Mork

On Fri, 07 Jun 2013, Adam Lee wrote:
> Some new Lenovo or ThinkPad laptops don't have EC controllable LEDs.
> This patch adds their quirks.
> 
> Signed-off-by: Adam Lee <adam.lee@canonical.com>

NAK.

Let's see if we can do this in a better way that is a bit more future-proof.

-- 
  "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] 18+ messages in thread

end of thread, other threads:[~2013-06-18  0:20 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-07  8:20 [PATCH 0/3] thinkpad_acpi: avoid ACPI Warning while operating non-exist LEDs Adam Lee
2013-06-07  8:20 ` [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs Adam Lee
2013-06-07  8:53   ` Bjørn Mork
2013-06-07  8:59     ` Adam Lee
2013-06-18  0:12   ` Henrique de Moraes Holschuh
2013-06-18  0:18     ` Henrique de Moraes Holschuh
2013-06-07  8:20 ` [PATCH 2/3] thinkpad_acpi: add the ability setting TPACPI_LED_NONE by quirk Adam Lee
2013-06-18  0:19   ` Henrique de Moraes Holschuh
2013-06-07  8:20 ` [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs Adam Lee
2013-06-07 13:22   ` Matthew Garrett
2013-06-07 15:06     ` Adam Lee
2013-06-07 20:16       ` Henrique de Moraes Holschuh
2013-06-08  9:04         ` Adam Lee
2013-06-18  0:20   ` Henrique de Moraes Holschuh
2013-06-07  9:05 ` [PATCH v2] thinkpad_acpi: return -NODEV while operating uninitialized LEDs Adam Lee
2013-06-08  2:19   ` Amos Kong
2013-06-08  8:51 ` [PATCH 1/3 v3] " Adam Lee
2013-06-18  0:17   ` Henrique de Moraes Holschuh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).