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

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

[Jason A. Donenfeld]

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 <Jason@zx2c4.com>
---
 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

[PATCH 2/3] thinkpad_acpi: Support micmute LED

[Jason A. Donenfeld]

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

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 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

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

[Henrique de Moraes Holschuh]

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 <Jason@zx2c4.com>
> ---
>  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

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

[Jason A. Donenfeld]

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 <Jason@zx2c4.com>
---
 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

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

[Matthew Garrett]

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 | mjg59@srcf.ucam.org

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

[Jason A. Donenfeld]

On Thu, Aug 22, 2013 at 5:39 PM, Matthew Garrett <mjg59@srcf.ucam.org> 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

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

[Henrique de Moraes Holschuh]

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

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

[Jason A. Donenfeld]

On Fri, Aug 23, 2013 at 8:18 PM, Henrique de Moraes Holschuh
<hmh@hmh.eng.br> 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.

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

[Pavel Machek]

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