leds: Add Chrome OS keyboard backlight LEDs driver

From: Simon Que <[email protected]>

This is a driver for ACPI-based keyboard backlight LEDs found on Chromebooks. The driver locates \\_SB.KBLT ACPI device and exports backlight as "chromeos::kbd_backlight" LED class device in sysfs.

Quoted 5 lines · Dmitry Torokhov

From: Simon Que <[email protected]>

This is a driver for ACPI-based keyboard backlight LEDs found on Chromebooks. The driver locates \\_SB.KBLT ACPI device and exports backlight as "chromeos::kbd_backlight" LED class device in sysfs.

Was it ever decided where this driver should live? I was planning on submitting to platform/chrome since most keyboard backlights seem to live over there but I don't think I got a response.

Hi Evan,

Quoted 11 lines · Evan McClain

Quoted 5 lines · Dmitry Torokhov

From: Simon Que <[email protected]>

This is a driver for ACPI-based keyboard backlight LEDs found on Chromebooks. The driver locates \\_SB.KBLT ACPI device and exports backlight as "chromeos::kbd_backlight" LED class device in sysfs.

Was it ever decided where this driver should live? I was planning on submitting to platform/chrome since most keyboard backlights seem to live over there but I don't think I got a response.

It hasn't been decided yet. I can take it, but could you submit one more version, without

'owner = THIS_MODULE' in struct platform_driver keyboard_led_driver ?

It is redundant, because the core will do it.

Also the line with devm_kzalloc has over 80 characters.

Quoted 30 lines · Jacek Anaszewski

Hi Evan,

Quoted 11 lines · Evan McClain

Quoted 9 lines · Dmitry Torokhov

From: Simon Que <[email protected]>

This is a driver for ACPI-based keyboard backlight LEDs found on
Chromebooks. The driver locates \\_SB.KBLT ACPI device and exports
backlight as "chromeos::kbd_backlight" LED class device in sysfs.

Quoted 6 lines

Was it ever decided where this driver should live? I was planning on
submitting to platform/chrome since most keyboard backlights seem to
live over there but I don't think I got a response.

It hasn't been decided yet. I can take it, but could you submit one more
version, without

'owner = THIS_MODULE' in struct platform_driver
keyboard_led_driver ?

It is redundant, because the core will do it.

Quoted 1 line

Also the line with devm_kzalloc has over 80 characters.

Also:

- preferably use sizeof(*cdev) instead of sizeof(struct ...)
- do not check cdev->flags & LED_SUSPENDED in keyboard_led_set_brightness() as it is not going to be called when led
  device is suspended anyway

- change the MODULE_LICENSE from "GPL v2" to "GPL" as to match the actual license notice

- report ACPI errors in error messages (since we clobber them)
- preferably use ENXIO instead of ENODEV

- maybe add "depends on CHROME_PLATFORMS || COMPILE_TEST" so that we do
  not prompt for it on non-Chrome platforms

Quoted 31 lines · Dmitry Torokhov

Quoted 30 lines · Jacek Anaszewski

Hi Evan,

Quoted 11 lines · Evan McClain

Quoted 9 lines · Dmitry Torokhov

From: Simon Que <[email protected]>

This is a driver for ACPI-based keyboard backlight LEDs found on
Chromebooks. The driver locates \\_SB.KBLT ACPI device and exports
backlight as "chromeos::kbd_backlight" LED class device in sysfs.

Quoted 6 lines

Was it ever decided where this driver should live? I was planning on
submitting to platform/chrome since most keyboard backlights seem to
live over there but I don't think I got a response.

It hasn't been decided yet. I can take it, but could you submit one more
version, without

'owner = THIS_MODULE' in struct platform_driver
keyboard_led_driver ?

It is redundant, because the core will do it.

Quoted 8 lines

Quoted 1 line

Also the line with devm_kzalloc has over 80 characters.

Also:

- preferably use sizeof(*cdev) instead of sizeof(struct ...)
- do not check cdev->flags & LED_SUSPENDED in keyboard_led_set_brightness() as it is not going to be called when led
  device is suspended anyway

Your patch is definitely better, I was only taking Simon's original submission and doing the minimal cleanup to help get it submitted (as someone using mainline linux on a pixel 2/samus).

Quoted 2 lines

- change the MODULE_LICENSE from "GPL v2" to "GPL" as to match the actual license notice

I think this got changed in one of the revisions in error.

Quoted 2 lines

- report ACPI errors in error messages (since we clobber them)
- preferably use ENXIO instead of ENODEV

Most other drivers seem to use ENODEV on probe, but I'm in the 'learn through grep' level of understanding for parts of linux.

Quoted 3 lines

- maybe add "depends on CHROME_PLATFORMS || COMPILE_TEST" so that we do
  not prompt for it on non-Chrome platforms

Adding depends on CHROME_PLATFORMS definitely makes sense, but also might support putting this driver in platform/chrome. Either way I just

selfishly want better mainline support for this laptop.

The only change I made (other than the changes suggested by Jacek) was to remove the line setting brightness to max_brightness on probe. I can

Quoted 56 lines · Evan McClain

Quoted 31 lines · Dmitry Torokhov

Quoted 30 lines · Jacek Anaszewski

Hi Evan,

Quoted 11 lines · Evan McClain

Quoted 9 lines · Dmitry Torokhov

From: Simon Que <[email protected]>

This is a driver for ACPI-based keyboard backlight LEDs found on
Chromebooks. The driver locates \\_SB.KBLT ACPI device and exports
backlight as "chromeos::kbd_backlight" LED class device in sysfs.

Quoted 6 lines

Was it ever decided where this driver should live? I was planning on
submitting to platform/chrome since most keyboard backlights seem to
live over there but I don't think I got a response.

It hasn't been decided yet. I can take it, but could you submit one more
version, without

'owner = THIS_MODULE' in struct platform_driver
keyboard_led_driver ?

It is redundant, because the core will do it.

Quoted 8 lines

Quoted 1 line

Also the line with devm_kzalloc has over 80 characters.

Also:

- preferably use sizeof(*cdev) instead of sizeof(struct ...)
- do not check cdev->flags & LED_SUSPENDED in keyboard_led_set_brightness() as it is not going to be called when
led
device is suspended anyway

Your patch is definitely better, I was only taking Simon's original submission and doing the minimal cleanup to help get it submitted (as someone using mainline linux on a pixel 2/samus).

Quoted 2 lines

- change the MODULE_LICENSE from "GPL v2" to "GPL" as to match the actual license notice

I think this got changed in one of the revisions in error.

Quoted 2 lines

- report ACPI errors in error messages (since we clobber them)
- preferably use ENXIO instead of ENODEV

Most other drivers seem to use ENODEV on probe, but I'm in the 'learn through grep' level of understanding for parts of linux.

Quoted 6 lines

Quoted 3 lines

- maybe add "depends on CHROME_PLATFORMS || COMPILE_TEST" so that we do
not prompt for it on non-Chrome platforms

Adding depends on CHROME_PLATFORMS definitely makes sense, but also might support putting this driver in platform/chrome. Either way I just

Olof, do you want it in platform/chrome?

Quoted 15 lines · Jacek Anaszewski

Hi Evan,

Quoted 11 lines · Evan McClain

Quoted 5 lines · Dmitry Torokhov

From: Simon Que <[email protected]>

This is a driver for ACPI-based keyboard backlight LEDs found on Chromebooks. The driver locates \\_SB.KBLT ACPI device and exports backlight as "chromeos::kbd_backlight" LED class device in sysfs.

Was it ever decided where this driver should live? I was planning on submitting to platform/chrome since most keyboard backlights seem to live over there but I don't think I got a response.

Quoted 8 lines

It hasn't been decided yet. I can take it, but could you submit one more version, without

'owner = THIS_MODULE' in struct platform_driver keyboard_led_driver ?

It is redundant, because the core will do it.

Also the line with devm_kzalloc has over 80 characters.

Also:

- preferably use sizeof(*cdev) instead of sizeof(struct ...)
- do not check cdev->flags & LED_SUSPENDED in keyboard_led_set_brightness() as it is not going to be called when led device is suspended anyway
- change the MODULE_LICENSE from "GPL v2" to "GPL" as to match the

actual license notice
- report ACPI errors in error messages (since we clobber them)
- preferably use ENXIO instead of ENODEV
- maybe add "depends on CHROME_PLATFORMS || COMPILE_TEST" so that we do not prompt for it on non-Chrome platforms

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines · Jacek Anaszewski

Hi Evan,

Quoted 11 lines · Evan McClain

Quoted 5 lines · Dmitry Torokhov

From: Simon Que <[email protected]>

This is a driver for ACPI-based keyboard backlight LEDs found on Chromebooks. The driver locates \\_SB.KBLT ACPI device and exports backlight as "chromeos::kbd_backlight" LED class device in sysfs.

Was it ever decided where this driver should live? I was planning on submitting to platform/chrome since most keyboard backlights seem to live over there but I don't think I got a response.

Quoted 8 lines

It hasn't been decided yet. I can take it, but could you submit one more version, without

'owner = THIS_MODULE' in struct platform_driver keyboard_led_driver ?

It is redundant, because the core will do it.

Also the line with devm_kzalloc has over 80 characters.

Quoted 7 lines

Also:

- preferably use sizeof(*cdev) instead of sizeof(struct ...)
- do not check cdev->flags & LED_SUSPENDED in keyboard_led_set_brightness() as it is not going to be called when led device is suspended anyway
- change the MODULE_LICENSE from "GPL v2" to "GPL" as to match the

I can see "either version 2 of the License" in the license notice.

From module.h:

Quoted 34 lines · Jacek Anaszewski

Quoted 32 lines · Dmitry Torokhov

Quoted 15 lines · Jacek Anaszewski

Hi Evan,

Quoted 11 lines · Evan McClain

Quoted 5 lines · Dmitry Torokhov

From: Simon Que <[email protected]>

This is a driver for ACPI-based keyboard backlight LEDs found on Chromebooks. The driver locates \\_SB.KBLT ACPI device and exports backlight as "chromeos::kbd_backlight" LED class device in sysfs.

Was it ever decided where this driver should live? I was planning on submitting to platform/chrome since most keyboard backlights seem to live over there but I don't think I got a response.

Quoted 8 lines

It hasn't been decided yet. I can take it, but could you submit one more version, without

'owner = THIS_MODULE' in struct platform_driver keyboard_led_driver ?

It is redundant, because the core will do it.

Also the line with devm_kzalloc has over 80 characters.

Also:

- preferably use sizeof(*cdev) instead of sizeof(struct ...)
- do not check cdev->flags & LED_SUSPENDED in keyboard_led_set_brightness() as it is not going to be called when led device is suspended anyway
- change the MODULE_LICENSE from "GPL v2" to "GPL" as to match the

Quoted 3 lines

I can see "either version 2 of the License" in the license notice.

From module.h:

* "GPL" [GNU Public License v2 or later]
* "GPL v2" [GNU Public License v2]

leds-chromeos-keyboard is GPL v2+ so module license should be "GPL".

From: Simon Que <[email protected]>

This is a driver for ACPI-based keyboard backlight LEDs found on Chromebooks. The driver locates \\_SB.KBLT ACPI device and exports backlight as "chromeos::kbd_backlight" LED class device in sysfs.

Signed-off-by Simon Que <[email protected]>

Signed-off-by Duncan Laurie <[email protected]>

Signed-off-by Dmitry Torokhov <[email protected]>

---
  drivers/leds/Kconfig                  |  11 ++++
  drivers/leds/Makefile                 |   1 +
  drivers/leds/leds-chromeos-keyboard.c | 106 ++++++++++++++++++++++++++++++++++
  3 files changed, 118 insertions(+)

drivers/leds/Kconfig +11 -0

index f86527cf..02aaaf1 100644
@@ -581,6 +581,17 @@ config LEDS_SN3218
  	  This driver can also be built as a module. If so the module
  	  will be called leds-sn3218.
 
+config LEDS_CHROMEOS_KEYBOARD
+	tristate "LED backlight support for Chrome OS keyboards"
+	depends on LEDS_CLASS && ACPI
+	depends on CHROME_PLATFORMS || COMPILE_TEST
+	help
+	  This option enables support for the keyboard backlight LEDs on
+	  select Chrome OS systems.
+
+	  This driver can also be built as a module. If so the module
+	  will be called leds-chromeos-keyboard.
+
  comment "LED driver for blink(1) USB RGB LED is under Special HID drivers (HID_THINGM)"
 
  config LEDS_BLINKM

drivers/leds/Makefile +1 -0

index 89c9b6f..5b96d15 100644
@@ -67,6 +67,7 @@ obj-$(CONFIG_LEDS_KTD2692)		+= leds-ktd2692.o
  obj-$(CONFIG_LEDS_POWERNV)		+= leds-powernv.o
  obj-$(CONFIG_LEDS_SEAD3)		+= leds-sead3.o
  obj-$(CONFIG_LEDS_SN3218)		+= leds-sn3218.o
+obj-$(CONFIG_LEDS_CHROMEOS_KEYBOARD)	+= leds-chromeos-keyboard.o
 
  # LED SPI Drivers
  obj-$(CONFIG_LEDS_DAC124S085)		+= leds-dac124s085.o

drivers/leds/leds-chromeos-keyboard.c new file +106 -0

new file mode 100644
index 0000000..3592f09
@@ -0,0 +1,106 @@
+/*
+ *  Keyboard backlight LED driver for Chrome OS.
+ *
+ *  Copyright (C) 2012 Google, Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#include <linux/acpi.h>
+#include <linux/leds.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/* Keyboard LED ACPI Device must be defined in firmware */
+#define ACPI_KEYBOARD_BACKLIGHT_DEVICE	"\\_SB.KBLT"
+#define ACPI_KEYBOARD_BACKLIGHT_READ	ACPI_KEYBOARD_BACKLIGHT_DEVICE ".KBQC"
+#define ACPI_KEYBOARD_BACKLIGHT_WRITE	ACPI_KEYBOARD_BACKLIGHT_DEVICE ".KBCM"
+
+#define ACPI_KEYBOARD_BACKLIGHT_MAX		100
+
+static void keyboard_led_set_brightness(struct led_classdev *cdev,
+					enum led_brightness brightness)
+{
+	union acpi_object param;
+	struct acpi_object_list input;
+	acpi_status status;
+
+	param.type = ACPI_TYPE_INTEGER;
+	param.integer.value = brightness;
+	input.count = 1;
+	input.pointer = &param;
+
+	status = acpi_evaluate_object(NULL, ACPI_KEYBOARD_BACKLIGHT_WRITE,
+				      &input, NULL);
+	if (ACPI_FAILURE(status))
+		dev_err(cdev->dev, "Error setting keyboard LED value: %d\n",
+			status);
+}
+
+static int keyboard_led_probe(struct platform_device *pdev)
+{
+	struct led_classdev *cdev;
+	acpi_handle handle;
+	acpi_status status;
+	int error;
+
+	/* Look for the keyboard LED ACPI Device */
+	status = acpi_get_handle(ACPI_ROOT_OBJECT,
+				 ACPI_KEYBOARD_BACKLIGHT_DEVICE,
+				 &handle);
+	if (ACPI_FAILURE(status)) {
+		dev_err(&pdev->dev, "Unable to find ACPI device %s: %d\n",
+			ACPI_KEYBOARD_BACKLIGHT_DEVICE, status);
+		return -ENXIO;
+	}
+
+	cdev = devm_kzalloc(&pdev->dev, sizeof(*cdev), GFP_KERNEL);
+	if (!cdev)
+		return -ENOMEM;
+
+	cdev->name = "chromeos::kbd_backlight";
+	cdev->brightness_set = keyboard_led_set_brightness;
+	cdev->max_brightness = ACPI_KEYBOARD_BACKLIGHT_MAX;
+	cdev->brightness = cdev->max_brightness;
+	cdev->flags |= LED_CORE_SUSPENDRESUME;
+
+	error = devm_led_classdev_register(&pdev->dev, cdev);
+	if (error)
+		return error;
+
+	platform_set_drvdata(pdev, cdev);
+	return 0;
+}
+
+static const struct acpi_device_id keyboard_led_id[] = {
+	{ "GOOG0002", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, keyboard_led_id);
+
+static struct platform_driver keyboard_led_driver = {
+	.driver		= {
+		.name	= "chromeos-keyboard-leds",
+		.acpi_match_table = ACPI_PTR(keyboard_led_id),
+	},
+	.probe		= keyboard_led_probe,
+};
+module_platform_driver(keyboard_led_driver);
+
+MODULE_AUTHOR("Simon Que <[email protected]>");
+MODULE_DESCRIPTION("ChromeOS Keyboard backlight LED Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:chromeos-keyboard-leds");

Hi Dmitry,

Thanks for the patch. There's already other pending version [1], still to be decided where it should live.

[1] http://comments.gmane.org/gmane.linux.leds/4523

Hi Jacek,

Quoted 2 lines · Jacek Anaszewski

Hi Dmitry,

Quoted 4 lines

Thanks for the patch. There's already other pending version [1], still to be decided where it should live.

[1] http://comments.gmane.org/gmane.linux.leds/4523

Yes, sorry, have not noticed it already being submitted while doing some spring cleaning.