| Message ID | 20170119172132.36029-1-agoode@google.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Andy Shevchenko |
| Headers | show |
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.
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
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
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
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" ;-)
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 ;-).
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?
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) {
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>