[PATCH 0/3] thinkpad_acpi: avoid ACPI Warning while operating non-exist LEDs

[PATCH 0/3] thinkpad_acpi: avoid ACPI Warning while operating non-exist LEDs

[Adam Lee]

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

[PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs

[Adam Lee]

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

Re: [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs

[Bjørn Mork]

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

Re: [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs

[Adam Lee]

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

Re: [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs

[Henrique de Moraes Holschuh]

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

Re: [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs

[Henrique de Moraes Holschuh]

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

[PATCH 2/3] thinkpad_acpi: add the ability setting TPACPI_LED_NONE by quirk

[Adam Lee]

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

Re: [PATCH 2/3] thinkpad_acpi: add the ability setting TPACPI_LED_NONE by quirk

[Henrique de Moraes Holschuh]

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

[PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs

[Adam Lee]

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

Re: [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs

[Matthew Garrett]

[-- 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¥

Re: [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs

[Adam Lee]

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

Re: [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs

[Henrique de Moraes Holschuh]

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

Re: [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs

[Adam Lee]

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

Re: [PATCH 3/3] thinkpad_acpi: add LED quirks of models which don't have EC controllable LEDs

[Henrique de Moraes Holschuh]

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

[PATCH v2] thinkpad_acpi: return -NODEV while operating uninitialized LEDs

[Adam Lee]

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

Re: [PATCH v2] thinkpad_acpi: return -NODEV while operating uninitialized LEDs

[Amos Kong]

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

[PATCH 1/3 v3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs

[Adam Lee]

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

Re: [PATCH 1/3 v3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs

[Henrique de Moraes Holschuh]

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