2013-08-22 14:09:41

by Jason A. Donenfeld

[permalink] [raw]
Subject: [PATCH 1/3] Input: atkbd - add LED triggers for keyboard state

Many new laptop keyboards aren't shipping with LEDs in the keys for
caps lock, num lock, and scroll lock. They do, however, ship with many LEDs
for specialized functions that mostly go non-utilized by any current
Linux drivers. Having a caps lock LED is very helpful in early boot full
disk encryption, where a fancy GUI is not available to show that caps
lock is activated.

This patch wires in the caps, num, and scroll lock states of the
keyboard into the generic LED trigger subsystem, so that integrators can
have different LEDs activated on caps/num/scroll lock state changes.

Signed-off-by: Jason A. Donenfeld <[email protected]>
---
drivers/input/keyboard/atkbd.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 2626773..15061bf 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -28,6 +28,7 @@
#include <linux/libps2.h>
#include <linux/mutex.h>
#include <linux/dmi.h>
+#include <linux/leds.h>

#define DRIVER_DESC "AT and PS/2 keyboard driver"

@@ -302,6 +303,12 @@ static const unsigned int xl_table[] = {
ATKBD_RET_NAK, ATKBD_RET_HANJA, ATKBD_RET_HANGEUL,
};

+#ifdef CONFIG_LEDS_TRIGGERS
+struct led_trigger *capsl_led_trigger = 0;
+struct led_trigger *numl_led_trigger = 0;
+struct led_trigger *scrolll_led_trigger = 0;
+#endif
+
/*
* Checks if we should mangle the scancode to extract 'release' bit
* in translated mode.
@@ -559,6 +566,12 @@ static int atkbd_set_leds(struct atkbd *atkbd)
if (ps2_command(&atkbd->ps2dev, param, ATKBD_CMD_SETLEDS))
return -1;

+#ifdef CONFIG_LEDS_TRIGGERS
+ led_trigger_event(capsl_led_trigger, test_bit(LED_CAPSL, dev->led) ? LED_FULL : LED_OFF);
+ led_trigger_event(numl_led_trigger, test_bit(LED_NUML, dev->led) ? LED_FULL : LED_OFF);
+ led_trigger_event(scrolll_led_trigger, test_bit(LED_SCROLLL, dev->led) ? LED_FULL : LED_OFF);
+#endif
+
if (atkbd->extra) {
param[0] = 0;
param[1] = (test_bit(LED_COMPOSE, dev->led) ? 0x01 : 0)
@@ -1781,12 +1794,25 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
static int __init atkbd_init(void)
{
dmi_check_system(atkbd_dmi_quirk_table);
+#ifdef CONFIG_LEDS_TRIGGERS
+ led_trigger_register_simple("caps-lock", &capsl_led_trigger);
+ led_trigger_register_simple("num-lock", &numl_led_trigger);
+ led_trigger_register_simple("scroll-lock", &scrolll_led_trigger);
+#endif

return serio_register_driver(&atkbd_drv);
}

static void __exit atkbd_exit(void)
{
+#ifdef CONFIG_LEDS_TRIGGERS
+ led_trigger_unregister_simple(capsl_led_trigger);
+ capsl_led_trigger = 0;
+ led_trigger_unregister_simple(numl_led_trigger);
+ numl_led_trigger = 0;
+ led_trigger_unregister_simple(scrolll_led_trigger);
+ scrolll_led_trigger = 0;
+#endif
serio_unregister_driver(&atkbd_drv);
}

--
1.8.3.2


2013-08-22 14:09:45

by Jason A. Donenfeld

[permalink] [raw]
Subject: [PATCH 2/3] thinkpad_acpi: Support micmute LED

The micmute LED is currently unused. This patch allows it to be hooked
up to various LED triggers.

Signed-off-by: Jason A. Donenfeld <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 54d31c0..33b23cb 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5050,8 +5050,10 @@ static const char * const tpacpi_led_names[TPACPI_LED_NUMLEDS] = {
"tpacpi::unknown_led2",
"tpacpi::unknown_led3",
"tpacpi::thinkvantage",
+ "tpacpi::unknown_led4",
+ "tpacpi::micmute"
};
-#define TPACPI_SAFE_LEDS 0x1081U
+#define TPACPI_SAFE_LEDS 0x5081U

static inline bool tpacpi_is_led_restricted(const unsigned int led)
{
@@ -5274,7 +5276,7 @@ static const struct tpacpi_quirk led_useful_qtable[] __initconst = {
{ /* Lenovo */
.vendor = PCI_VENDOR_ID_LENOVO,
.bios = TPACPI_MATCH_ANY, .ec = TPACPI_MATCH_ANY,
- .quirks = 0x1fffU,
+ .quirks = 0x5fffU,
},
{ /* IBM ThinkPads with no EC version string */
.vendor = PCI_VENDOR_ID_IBM,
--
1.8.3.2

2013-08-22 14:09:49

by Jason A. Donenfeld

[permalink] [raw]
Subject: [PATCH 3/3] thinkpad_acpi: Wire unused micmute LED to capslock

Thinkpads with a micmute LED do not have a capslock LED. The micmute LED
is currently not used by any piece of Linux kernel land or user land. It
seems reasonable to hook it up to caps lock, at least by default, so
users can have some degree of functionality.

Signed-off-by: Jason A. Donenfeld <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 33b23cb..708fdb8 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5235,6 +5235,10 @@ static int __init tpacpi_init_led(unsigned int led)

tpacpi_leds[led].led_classdev.name = tpacpi_led_names[led];

+ /* Special case wiring micmute to caps-lock by default. */
+ if (led == 14)
+ tpacpi_leds[led].led_classdev.default_trigger = "caps-lock";
+
INIT_WORK(&tpacpi_leds[led].work, led_set_status_worker);

rc = led_classdev_register(&tpacpi_pdev->dev,
--
1.8.3.2

2013-08-22 15:39:11

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 3/3] thinkpad_acpi: Wire unused micmute LED to capslock

On Thu, Aug 22, 2013 at 04:02:12PM +0200, Jason A. Donenfeld wrote:
> Thinkpads with a micmute LED do not have a capslock LED. The micmute LED
> is currently not used by any piece of Linux kernel land or user land. It
> seems reasonable to hook it up to caps lock, at least by default, so
> users can have some degree of functionality.

I think there's a risk of user confusion here, in that it's now possible
for the mic mute light to be lit despite the internal microphone still
being active. That seems like surprising behaviour.

--
Matthew Garrett | [email protected]

2013-08-22 17:30:21

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [PATCH 3/3] thinkpad_acpi: Wire unused micmute LED to capslock

On Thu, Aug 22, 2013 at 5:39 PM, Matthew Garrett <[email protected]> wrote:
> I think there's a risk of user confusion here, in that it's now possible
> for the mic mute light to be lit despite the internal microphone still
> being active. That seems like surprising behaviour.

Yeah, that is a legitimate concern. Patches 1/3 [1] and 2/3 [2]
clearly make sense, as they're generic and don't really suggest any
new behavior.

But setting this behavior here as the default could indeed be a bit
unexpected. I think it's fair to reject this patch (3/3), but accept
the first two. Tweakers who have the same early boot caps-lock LED
needs as me can simply put this in their initramfs after 1/3 and 2/3
are applied:

# echo caps-lock >
/sys/devices/platform/thinkpad_acpi/leds/tpacpi\:\:micmute/trigger

Jason

[1] https://lkml.org/lkml/2013/8/22/310
[2] https://lkml.org/lkml/2013/8/22/313

Subject: Re: [PATCH 2/3] thinkpad_acpi: Support micmute LED

Hi Jason!

On Thu, 22 Aug 2013, Jason A. Donenfeld wrote:

> The micmute LED is currently unused. This patch allows it to be hooked
> up to various LED triggers.
>
> Signed-off-by: Jason A. Donenfeld <[email protected]>
> ---
> drivers/platform/x86/thinkpad_acpi.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 54d31c0..33b23cb 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -5050,8 +5050,10 @@ static const char * const tpacpi_led_names[TPACPI_LED_NUMLEDS] = {
> "tpacpi::unknown_led2",
> "tpacpi::unknown_led3",
> "tpacpi::thinkvantage",
> + "tpacpi::unknown_led4",
> + "tpacpi::micmute"
> };
> -#define TPACPI_SAFE_LEDS 0x1081U
> +#define TPACPI_SAFE_LEDS 0x5081U

Micmute is not a "safe" LED by thinkpad-acpi's definition.

Besides, I want to see that LED hooked to alsa, any other use should be
protected by a module parameter (like fan-control is) or kernel config (like
unsafe LEDs are).

--
"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

Subject: Re: [PATCH 3/3] thinkpad_acpi: Wire unused micmute LED to capslock

On Thu, 22 Aug 2013, Jason A. Donenfeld wrote:
> Thinkpads with a micmute LED do not have a capslock LED. The micmute LED
> is currently not used by any piece of Linux kernel land or user land. It
> seems reasonable to hook it up to caps lock, at least by default, so
> users can have some degree of functionality.

NACK. This we won't do. It is a LED misuse, and it will get in the way
when we finally put that LED to its proper use.

--
"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

2013-08-26 00:11:27

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [PATCH 3/3] thinkpad_acpi: Wire unused micmute LED to capslock

On Fri, Aug 23, 2013 at 8:18 PM, Henrique de Moraes Holschuh
<[email protected]> wrote:
> NACK. This we won't do. It is a LED misuse, and it will get in the way
> when we finally put that LED to its proper use.

Agreed. Please see my response to mjg.

2013-09-22 14:26:33

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 1/3] Input: atkbd - add LED triggers for keyboard state

Hi!

> Many new laptop keyboards aren't shipping with LEDs in the keys for
> caps lock, num lock, and scroll lock. They do, however, ship with many LEDs
> for specialized functions that mostly go non-utilized by any current
> Linux drivers. Having a caps lock LED is very helpful in early boot full
> disk encryption, where a fancy GUI is not available to show that caps
> lock is activated.
>
> This patch wires in the caps, num, and scroll lock states of the
> keyboard into the generic LED trigger subsystem, so that integrators can
> have different LEDs activated on caps/num/scroll lock state changes.

There's another patch floating around that properly integrates input
with LED subsystem; it already contains this functionality IIRC.
Pavel

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