[PATCH 1/1] thinkpad_acpi: Add support for status (external cover) LED

[PATCH 1/1] thinkpad_acpi: Add support for status (external cover) LED

[Adam Goode]

This allows the control of the red status LED, which is the dot of the "i"
in the word "ThinkPad" on the outside cover of newer models.

In the manual, both this LED and the power LED are referred to as
the "system-status indicators" without distinction between the two, so
I chose "status" as the LED name.

Signed-off-by: Adam Goode <agoode@google.com>

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index cacb43fb1df7..6577bf8e5635 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5611,11 +5611,11 @@ static const char * const tpacpi_led_names[TPACPI_LED_NUMLEDS] = {
 	"tpacpi::standby",
 	"tpacpi::dock_status1",
 	"tpacpi::dock_status2",
-	"tpacpi::unknown_led2",
+	"tpacpi::status",
 	"tpacpi::unknown_led3",
 	"tpacpi::thinkvantage",
 };
-#define TPACPI_SAFE_LEDS	0x1081U
+#define TPACPI_SAFE_LEDS	0x1481U
 
 static inline bool tpacpi_is_led_restricted(const unsigned int led)
 {
-- 
2.11.0.390.gc69c2f50cf-goog

Re: [ibm-acpi-devel] [PATCH 1/1] thinkpad_acpi: Add support for status (external cover) LED

[Henrique de Moraes Holschuh]

Hello Adam,

I apologise for the delay on answering you.

On Tue, 31 Jan 2017, Adam Goode wrote:
> On Thu, Jan 19, 2017 at 12:21 PM, Adam Goode <agoode@google.com> wrote:
> > This allows the control of the red status LED, which is the dot of the "i"
> > in the word "ThinkPad" on the outside cover of newer models.
> >
> > In the manual, both this LED and the power LED are referred to as
> > the "system-status indicators" without distinction between the two, so
> > I chose "status" as the LED name.

I seem to recall this LED had an ACPI interface that was specific for
it, and allowed it to on/off/sine-wave?

> > Signed-off-by: Adam Goode <agoode@google.com>
> >
> > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/
> > thinkpad_acpi.c
> > index cacb43fb1df7..6577bf8e5635 100644
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -5611,11 +5611,11 @@ static const char * const
> > tpacpi_led_names[TPACPI_LED_NUMLEDS] = {
> >         "tpacpi::standby",
> >         "tpacpi::dock_status1",
> >         "tpacpi::dock_status2",
> > -       "tpacpi::unknown_led2",
> > +       "tpacpi::status",
> >         "tpacpi::unknown_led3",
> >         "tpacpi::thinkvantage",
> >  };
> > -#define TPACPI_SAFE_LEDS       0x1081U
> > +#define TPACPI_SAFE_LEDS       0x1481U

What happens on older Lenovo models (x00, x10, x20 series?)?  I think
the T410 already had it...

Also, please add code to not export it to userspace (as a led class) on
IBM.

-- 
  Henrique Holschuh

Re: [ibm-acpi-devel] [PATCH 1/1] thinkpad_acpi: Add support for status (external cover) LED

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2270 bytes --]

Hi,

On Tue, Feb 07, 2017 at 11:01:32PM -0200, Henrique de Moraes Holschuh wrote:
> Hello Adam,
> 
> I apologise for the delay on answering you.
> 
> On Tue, 31 Jan 2017, Adam Goode wrote:
> > On Thu, Jan 19, 2017 at 12:21 PM, Adam Goode <agoode@google.com> wrote:
> > > This allows the control of the red status LED, which is the dot of the "i"
> > > in the word "ThinkPad" on the outside cover of newer models.
> > >
> > > In the manual, both this LED and the power LED are referred to as
> > > the "system-status indicators" without distinction between the two, so
> > > I chose "status" as the LED name.
> 
> I seem to recall this LED had an ACPI interface that was specific for
> it, and allowed it to on/off/sine-wave?

I don't know what the ACPI interface looks like, but the lid status
led goes into sine-wave mode during suspend. Note, that the power-led
also goes into sine-wave mode during suspend and is already supported
by thinkpad-acpi's LED code (without the sine-wave feature as far
as I can tell).

> > > Signed-off-by: Adam Goode <agoode@google.com>
> > >
> > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/
> > > thinkpad_acpi.c
> > > index cacb43fb1df7..6577bf8e5635 100644
> > > --- a/drivers/platform/x86/thinkpad_acpi.c
> > > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > > @@ -5611,11 +5611,11 @@ static const char * const
> > > tpacpi_led_names[TPACPI_LED_NUMLEDS] = {
> > >         "tpacpi::standby",
> > >         "tpacpi::dock_status1",
> > >         "tpacpi::dock_status2",
> > > -       "tpacpi::unknown_led2",
> > > +       "tpacpi::status",

"status" looks a bit generic. I suggest "external_lid_status".

> > >         "tpacpi::unknown_led3",
> > >         "tpacpi::thinkvantage",
> > >  };
> > > -#define TPACPI_SAFE_LEDS       0x1081U
> > > +#define TPACPI_SAFE_LEDS       0x1481U
> 
> What happens on older Lenovo models (x00, x10, x20 series?)?  I think
> the T410 already had it...
> 
> Also, please add code to not export it to userspace (as a led class) on
> IBM.

Adam, can you prepare an updated patch? I would like to use this
led to notify myself about events (i.e. compilation finished)
while my thinkpad's lid is closed.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [ibm-acpi-devel] [PATCH 1/1] thinkpad_acpi: Add support for status (external cover) LED

[Adam Goode]

On Sun, Feb 12, 2017 at 1:22 PM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Tue, Feb 07, 2017 at 11:01:32PM -0200, Henrique de Moraes Holschuh wrote:
>> Hello Adam,
>>
>> I apologise for the delay on answering you.
>>
>> On Tue, 31 Jan 2017, Adam Goode wrote:
>> > On Thu, Jan 19, 2017 at 12:21 PM, Adam Goode <agoode@google.com> wrote:
>> > > This allows the control of the red status LED, which is the dot of the "i"
>> > > in the word "ThinkPad" on the outside cover of newer models.
>> > >
>> > > In the manual, both this LED and the power LED are referred to as
>> > > the "system-status indicators" without distinction between the two, so
>> > > I chose "status" as the LED name.
>>
>> I seem to recall this LED had an ACPI interface that was specific for
>> it, and allowed it to on/off/sine-wave?
>
> I don't know what the ACPI interface looks like, but the lid status
> led goes into sine-wave mode during suspend. Note, that the power-led
> also goes into sine-wave mode during suspend and is already supported
> by thinkpad-acpi's LED code (without the sine-wave feature as far
> as I can tell).
>
>> > > Signed-off-by: Adam Goode <agoode@google.com>
>> > >
>> > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/
>> > > thinkpad_acpi.c
>> > > index cacb43fb1df7..6577bf8e5635 100644
>> > > --- a/drivers/platform/x86/thinkpad_acpi.c
>> > > +++ b/drivers/platform/x86/thinkpad_acpi.c
>> > > @@ -5611,11 +5611,11 @@ static const char * const
>> > > tpacpi_led_names[TPACPI_LED_NUMLEDS] = {
>> > >         "tpacpi::standby",
>> > >         "tpacpi::dock_status1",
>> > >         "tpacpi::dock_status2",
>> > > -       "tpacpi::unknown_led2",
>> > > +       "tpacpi::status",
>
> "status" looks a bit generic. I suggest "external_lid_status".
>
>> > >         "tpacpi::unknown_led3",
>> > >         "tpacpi::thinkvantage",
>> > >  };
>> > > -#define TPACPI_SAFE_LEDS       0x1081U
>> > > +#define TPACPI_SAFE_LEDS       0x1481U
>>
>> What happens on older Lenovo models (x00, x10, x20 series?)?  I think
>> the T410 already had it...
>>
>> Also, please add code to not export it to userspace (as a led class) on
>> IBM.
>
> Adam, can you prepare an updated patch? I would like to use this
> led to notify myself about events (i.e. compilation finished)
> while my thinkpad's lid is closed.
>

Yes, I will definitely prepare a new patch at some point. The only
hardware I have to test this on is an old IBM X40 and the Lenovo X260.
(I have a T410 with a busted screen that I could try it on maybe.) But
not much else.

I don't know if I can get to this in the next few weeks, since I am
travelling. If someone would like to clean up the patch and move
forward without me, no problem. Otherwise I will get to it eventually.


Thanks,

Adam


> -- Sebastian

Re: [PATCH 1/1] thinkpad_acpi: Add support for status (external cover) LED

[Pavel Machek]

On Thu 2017-01-19 12:21:32, Adam Goode wrote:
> This allows the control of the red status LED, which is the dot of the "i"
> in the word "ThinkPad" on the outside cover of newer models.
> 
> In the manual, both this LED and the power LED are referred to as
> the "system-status indicators" without distinction between the two, so
> I chose "status" as the LED name.

Could we name it something like external_status or something? "status" is way too generic.

										Pavel
> 
> Signed-off-by: Adam Goode <agoode@google.com>
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index cacb43fb1df7..6577bf8e5635 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -5611,11 +5611,11 @@ static const char * const tpacpi_led_names[TPACPI_LED_NUMLEDS] = {
>  	"tpacpi::standby",
>  	"tpacpi::dock_status1",
>  	"tpacpi::dock_status2",
> -	"tpacpi::unknown_led2",
> +	"tpacpi::status",
>  	"tpacpi::unknown_led3",
>  	"tpacpi::thinkvantage",
>  };
> -#define TPACPI_SAFE_LEDS	0x1081U
> +#define TPACPI_SAFE_LEDS	0x1481U
>  
>  static inline bool tpacpi_is_led_restricted(const unsigned int led)
>  {
> -- 
> 2.11.0.390.gc69c2f50cf-goog

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/1] thinkpad_acpi: Add support for status (external cover) LED

[Henrique de Moraes Holschuh]

On Sun, 07 May 2017, Pavel Machek wrote:
> On Thu 2017-01-19 12:21:32, Adam Goode wrote:
> > This allows the control of the red status LED, which is the dot of the "i"
> > in the word "ThinkPad" on the outside cover of newer models.
> > 
> > In the manual, both this LED and the power LED are referred to as
> > the "system-status indicators" without distinction between the two, so
> > I chose "status" as the LED name.
> 
> Could we name it something like external_status or something? "status" is way too generic.

"thinkdot" ;-)

-- 
  Henrique Holschuh

Re: [PATCH 1/1] thinkpad_acpi: Add support for status (external cover) LED

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]

On Sun 2017-05-07 20:49:03, Henrique de Moraes Holschuh wrote:
> On Sun, 07 May 2017, Pavel Machek wrote:
> > On Thu 2017-01-19 12:21:32, Adam Goode wrote:
> > > This allows the control of the red status LED, which is the dot of the "i"
> > > in the word "ThinkPad" on the outside cover of newer models.
> > > 
> > > In the manual, both this LED and the power LED are referred to as
> > > the "system-status indicators" without distinction between the two, so
> > > I chose "status" as the LED name.
> > 
> > Could we name it something like external_status or something? "status" is way too generic.
> 
> "thinkdot" ;-)

Sounds good to me ;-).

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [ibm-acpi-devel] [PATCH 1/1] thinkpad_acpi: Add support for status (external cover) LED

[Henrique de Moraes Holschuh]

On Fri, 12 May 2017, Pavel Machek wrote:
> On Sun 2017-05-07 20:49:03, Henrique de Moraes Holschuh wrote:
> > On Sun, 07 May 2017, Pavel Machek wrote:
> > > On Thu 2017-01-19 12:21:32, Adam Goode wrote:
> > > > This allows the control of the red status LED, which is the dot of the "i"
> > > > in the word "ThinkPad" on the outside cover of newer models.
> > > > 
> > > > In the manual, both this LED and the power LED are referred to as
> > > > the "system-status indicators" without distinction between the two, so
> > > > I chose "status" as the LED name.
> > > 
> > > Could we name it something like external_status or something? "status" is way too generic.
> > 
> > "thinkdot" ;-)
> 
> Sounds good to me ;-).

Ok.  Adam, care to respin this when you have the time?

-- 
  Henrique Holschuh