2022-05-23 09:09:04

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH v4 0/5] platform/chrome: cros_kbd_led_backlight: add EC PWM backend

The series adds EC PWM as an backend option for ChromeOS keyboard LED
backlight.

The 1st patch reorder the headers alphabetically.

The 2nd patch separates the ACPI backend.

The 3rd patch is the DT binding document for the proposed compatible string.

The 4th patch supports OF match.

The 5th patch adds EC PWM as another backend.

Changes from v3:
(https://patchwork.kernel.org/project/chrome-platform/cover/[email protected]/)
- Fix review comments on 5th patch.

Changes from v2:
(https://patchwork.kernel.org/project/chrome-platform/cover/[email protected]/)
- Fix per review comments.

Changes from v1:
(https://patchwork.kernel.org/project/chrome-platform/cover/[email protected]/)
- Update email address accordingly.

Tzung-Bi Shih (5):
platform/chrome: cros_kbd_led_backlight: sort headers alphabetically
platform/chrome: cros_kbd_led_backlight: separate ACPI backend
dt-bindings: add google,cros-kbd-led-backlight
platform/chrome: cros_kbd_led_backlight: support OF match
platform/chrome: cros_kbd_led_backlight: support EC PWM backend

.../chrome/google,cros-kbd-led-backlight.yaml | 35 ++++
.../bindings/mfd/google,cros-ec.yaml | 3 +
drivers/platform/chrome/Kconfig | 2 +-
.../platform/chrome/cros_kbd_led_backlight.c | 196 ++++++++++++++++--
4 files changed, 213 insertions(+), 23 deletions(-)
create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-kbd-led-backlight.yaml

--
2.36.1.124.g0e6072fb45-goog



2022-05-23 09:09:12

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH v4 1/5] platform/chrome: cros_kbd_led_backlight: sort headers alphabetically

To be neat and reduce conflict possibility, sort the headers
alphabetically.

Reviewed-by: Guenter Roeck <[email protected]>
Signed-off-by: Tzung-Bi Shih <[email protected]>
---
No changes from v3.

Changes from v2:
- Add commit message.
- Add R-b tag.

Changes from v1:
- Update email address accordingly.

drivers/platform/chrome/cros_kbd_led_backlight.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_kbd_led_backlight.c b/drivers/platform/chrome/cros_kbd_led_backlight.c
index aa409f0201fb..f9587a562bb7 100644
--- a/drivers/platform/chrome/cros_kbd_led_backlight.c
+++ b/drivers/platform/chrome/cros_kbd_led_backlight.c
@@ -4,12 +4,12 @@
// Copyright (C) 2012 Google, Inc.

#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/leds.h>
+#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/slab.h>

--
2.36.1.124.g0e6072fb45-goog


2022-05-23 09:09:12

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH v4 2/5] platform/chrome: cros_kbd_led_backlight: separate ACPI backend

cros_kbd_led_backlight uses ACPI_KEYBOARD_BACKLIGHT_WRITE and
ACPI_KEYBOARD_BACKLIGHT_READ for setting and getting the brightness
respectively.

Separate ACPI operations for preparing the driver to support other
backends.

Signed-off-by: Tzung-Bi Shih <[email protected]>
---
Changes from v3:
- Remove CROS_KBD_LED_BACKLIGHT_ACPI Kconfig.
- Remove stub function.

Changes from v2:
- Use #ifdef for boolean CONFIG_CROS_KBD_LED_BACKLIGHT_ACPI.

Changes from v1:
- Update email address accordingly.
- Use CONFIG_ACPI guard per "kernel test robot <[email protected]>" reported an
unused variable issue.

.../platform/chrome/cros_kbd_led_backlight.c | 82 ++++++++++++++++---
1 file changed, 69 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/chrome/cros_kbd_led_backlight.c b/drivers/platform/chrome/cros_kbd_led_backlight.c
index f9587a562bb7..a86d664854ae 100644
--- a/drivers/platform/chrome/cros_kbd_led_backlight.c
+++ b/drivers/platform/chrome/cros_kbd_led_backlight.c
@@ -13,6 +13,33 @@
#include <linux/platform_device.h>
#include <linux/slab.h>

+/**
+ * struct keyboard_led_drvdata - keyboard LED driver data.
+ * @init: Init function.
+ * @brightness_get: Get LED brightness level.
+ * @brightness_set: Set LED brightness level. Must not sleep.
+ * @brightness_set_blocking: Set LED brightness level. It can block the
+ * caller for the time required for accessing a
+ * LED device register
+ * @max_brightness: Maximum brightness.
+ *
+ * See struct led_classdev in include/linux/leds.h for more details.
+ */
+struct keyboard_led_drvdata {
+ int (*init)(struct platform_device *pdev);
+
+ enum led_brightness (*brightness_get)(struct led_classdev *led_cdev);
+
+ void (*brightness_set)(struct led_classdev *led_cdev,
+ enum led_brightness brightness);
+ int (*brightness_set_blocking)(struct led_classdev *led_cdev,
+ enum led_brightness brightness);
+
+ enum led_brightness max_brightness;
+};
+
+#ifdef CONFIG_ACPI
+
/* 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"
@@ -20,8 +47,8 @@

#define ACPI_KEYBOARD_BACKLIGHT_MAX 100

-static void keyboard_led_set_brightness(struct led_classdev *cdev,
- enum led_brightness brightness)
+static void keyboard_led_set_brightness_acpi(struct led_classdev *cdev,
+ enum led_brightness brightness)
{
union acpi_object param;
struct acpi_object_list input;
@@ -40,7 +67,7 @@ static void keyboard_led_set_brightness(struct led_classdev *cdev,
}

static enum led_brightness
-keyboard_led_get_brightness(struct led_classdev *cdev)
+keyboard_led_get_brightness_acpi(struct led_classdev *cdev)
{
unsigned long long brightness;
acpi_status status;
@@ -56,12 +83,10 @@ keyboard_led_get_brightness(struct led_classdev *cdev)
return brightness;
}

-static int keyboard_led_probe(struct platform_device *pdev)
+static int keyboard_led_init_acpi(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,
@@ -73,15 +98,44 @@ static int keyboard_led_probe(struct platform_device *pdev)
return -ENXIO;
}

+ return 0;
+}
+
+static const struct keyboard_led_drvdata keyboard_led_drvdata_acpi = {
+ .init = keyboard_led_init_acpi,
+ .brightness_set = keyboard_led_set_brightness_acpi,
+ .brightness_get = keyboard_led_get_brightness_acpi,
+ .max_brightness = ACPI_KEYBOARD_BACKLIGHT_MAX,
+};
+
+#endif /* CONFIG_ACPI */
+
+static int keyboard_led_probe(struct platform_device *pdev)
+{
+ struct led_classdev *cdev;
+ const struct keyboard_led_drvdata *drvdata;
+ int error;
+
+ drvdata = acpi_device_get_match_data(&pdev->dev);
+ if (!drvdata)
+ return -EINVAL;
+
+ if (drvdata->init) {
+ error = drvdata->init(pdev);
+ if (error)
+ return error;
+ }
+
cdev = devm_kzalloc(&pdev->dev, sizeof(*cdev), GFP_KERNEL);
if (!cdev)
return -ENOMEM;

cdev->name = "chromeos::kbd_backlight";
- cdev->max_brightness = ACPI_KEYBOARD_BACKLIGHT_MAX;
cdev->flags |= LED_CORE_SUSPENDRESUME;
- cdev->brightness_set = keyboard_led_set_brightness;
- cdev->brightness_get = keyboard_led_get_brightness;
+ cdev->max_brightness = drvdata->max_brightness;
+ cdev->brightness_set = drvdata->brightness_set;
+ cdev->brightness_set_blocking = drvdata->brightness_set_blocking;
+ cdev->brightness_get = drvdata->brightness_get;

error = devm_led_classdev_register(&pdev->dev, cdev);
if (error)
@@ -90,16 +144,18 @@ static int keyboard_led_probe(struct platform_device *pdev)
return 0;
}

-static const struct acpi_device_id keyboard_led_id[] = {
- { "GOOG0002", 0 },
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id keyboard_led_acpi_match[] = {
+ { "GOOG0002", (kernel_ulong_t)&keyboard_led_drvdata_acpi },
{ }
};
-MODULE_DEVICE_TABLE(acpi, keyboard_led_id);
+MODULE_DEVICE_TABLE(acpi, keyboard_led_acpi_match);
+#endif

static struct platform_driver keyboard_led_driver = {
.driver = {
.name = "chromeos-keyboard-leds",
- .acpi_match_table = ACPI_PTR(keyboard_led_id),
+ .acpi_match_table = ACPI_PTR(keyboard_led_acpi_match),
},
.probe = keyboard_led_probe,
};
--
2.36.1.124.g0e6072fb45-goog


2022-05-23 09:09:15

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH v4 3/5] dt-bindings: add google,cros-kbd-led-backlight

Acked-by: Rob Herring <[email protected]>
Signed-off-by: Tzung-Bi Shih <[email protected]>
---
No content changes from v3 but rebase to the latest
Documentation/devicetree/bindings/mfd/google,cros-ec.yaml.

No changes from v2.

Changes from v1:
(https://patchwork.kernel.org/project/chrome-platform/patch/[email protected]/)
- Update email address accordingly.
- Add A-b tag.

.../chrome/google,cros-kbd-led-backlight.yaml | 35 +++++++++++++++++++
.../bindings/mfd/google,cros-ec.yaml | 3 ++
2 files changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-kbd-led-backlight.yaml

diff --git a/Documentation/devicetree/bindings/chrome/google,cros-kbd-led-backlight.yaml b/Documentation/devicetree/bindings/chrome/google,cros-kbd-led-backlight.yaml
new file mode 100644
index 000000000000..5b875af6a95a
--- /dev/null
+++ b/Documentation/devicetree/bindings/chrome/google,cros-kbd-led-backlight.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/chrome/google,cros-kbd-led-backlight.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ChromeOS keyboard backlight LED driver.
+
+maintainers:
+ - Tzung-Bi Shih <[email protected]>
+
+properties:
+ compatible:
+ const: google,cros-kbd-led-backlight
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ spi0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cros_ec: ec@0 {
+ compatible = "google,cros-ec-spi";
+ reg = <0>;
+
+ kbd-led-backlight {
+ compatible = "google,cros-kbd-led-backlight";
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index afec0bd2f1de..09e00adf445e 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -90,6 +90,9 @@ properties:
pwm:
$ref: "/schemas/pwm/google,cros-ec-pwm.yaml#"

+ kbd-led-backlight:
+ $ref: "/schemas/chrome/google,cros-kbd-led-backlight.yaml#"
+
keyboard-controller:
$ref: "/schemas/input/google,cros-ec-keyb.yaml#"

--
2.36.1.124.g0e6072fb45-goog


2022-05-23 09:09:28

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH v4 4/5] platform/chrome: cros_kbd_led_backlight: support OF match

For letting device tree based machines to use the driver, support OF match.

Reviewed-by: Guenter Roeck <[email protected]>
Signed-off-by: Tzung-Bi Shih <[email protected]>
---
No changes from v3.

Changes from v2:
- Add commit message.
- Add R-b tag.

Changes from v1:
(https://patchwork.kernel.org/project/chrome-platform/patch/[email protected]/)
- Update email address accordingly.
- Use device_get_match_data() per review comment in v1.

drivers/platform/chrome/cros_kbd_led_backlight.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_kbd_led_backlight.c b/drivers/platform/chrome/cros_kbd_led_backlight.c
index a86d664854ae..4bca880d7721 100644
--- a/drivers/platform/chrome/cros_kbd_led_backlight.c
+++ b/drivers/platform/chrome/cros_kbd_led_backlight.c
@@ -10,7 +10,9 @@
#include <linux/kernel.h>
#include <linux/leds.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/slab.h>

/**
@@ -116,7 +118,7 @@ static int keyboard_led_probe(struct platform_device *pdev)
const struct keyboard_led_drvdata *drvdata;
int error;

- drvdata = acpi_device_get_match_data(&pdev->dev);
+ drvdata = device_get_match_data(&pdev->dev);
if (!drvdata)
return -EINVAL;

@@ -152,10 +154,21 @@ static const struct acpi_device_id keyboard_led_acpi_match[] = {
MODULE_DEVICE_TABLE(acpi, keyboard_led_acpi_match);
#endif

+#ifdef CONFIG_OF
+static const struct of_device_id keyboard_led_of_match[] = {
+ {
+ .compatible = "google,cros-kbd-led-backlight",
+ },
+ {}
+};
+MODULE_DEVICE_TABLE(of, keyboard_led_of_match);
+#endif
+
static struct platform_driver keyboard_led_driver = {
.driver = {
.name = "chromeos-keyboard-leds",
.acpi_match_table = ACPI_PTR(keyboard_led_acpi_match),
+ .of_match_table = of_match_ptr(keyboard_led_of_match),
},
.probe = keyboard_led_probe,
};
--
2.36.1.124.g0e6072fb45-goog


2022-05-23 09:09:47

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH v4 5/5] platform/chrome: cros_kbd_led_backlight: support EC PWM backend

EC PWM backend uses EC_CMD_PWM_SET_KEYBOARD_BACKLIGHT and
EC_CMD_PWM_GET_KEYBOARD_BACKLIGHT for setting and getting the brightness
respectively.

Signed-off-by: Tzung-Bi Shih <[email protected]>
---
Changes from v3:
- Remove CROS_KBD_LED_BACKLIGHT_EC_PWM Kconfig.
- Remove stub function.
- Remove redundant variable assignments.
- Remove the "private" term.

Changes from v2:
- Turn CROS_KBD_LED_BACKLIGHT_EC_PWM to boolean.
- Use #ifdef for boolean CROS_KBD_LED_BACKLIGHT_EC_PWM.

Changes from v1:
- Update email address accordingly.

drivers/platform/chrome/Kconfig | 2 +-
.../platform/chrome/cros_kbd_led_backlight.c | 113 +++++++++++++++---
2 files changed, 99 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 717299cbccac..3caf3194e3f5 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -139,7 +139,7 @@ config CROS_EC_PROTO

config CROS_KBD_LED_BACKLIGHT
tristate "Backlight LED support for Chrome OS keyboards"
- depends on LEDS_CLASS && ACPI
+ depends on LEDS_CLASS && (ACPI || CROS_EC)
help
This option enables support for the keyboard backlight LEDs on
select Chrome OS systems.
diff --git a/drivers/platform/chrome/cros_kbd_led_backlight.c b/drivers/platform/chrome/cros_kbd_led_backlight.c
index 4bca880d7721..5ad41c10412d 100644
--- a/drivers/platform/chrome/cros_kbd_led_backlight.c
+++ b/drivers/platform/chrome/cros_kbd_led_backlight.c
@@ -11,10 +11,17 @@
#include <linux/leds.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/property.h>
#include <linux/slab.h>

+struct keyboard_led {
+ struct led_classdev cdev;
+ struct cros_ec_device *ec;
+};
+
/**
* struct keyboard_led_drvdata - keyboard LED driver data.
* @init: Init function.
@@ -40,6 +47,8 @@ struct keyboard_led_drvdata {
enum led_brightness max_brightness;
};

+#define KEYBOARD_BACKLIGHT_MAX 100
+
#ifdef CONFIG_ACPI

/* Keyboard LED ACPI Device must be defined in firmware */
@@ -47,8 +56,6 @@ struct keyboard_led_drvdata {
#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_acpi(struct led_classdev *cdev,
enum led_brightness brightness)
{
@@ -107,39 +114,114 @@ static const struct keyboard_led_drvdata keyboard_led_drvdata_acpi = {
.init = keyboard_led_init_acpi,
.brightness_set = keyboard_led_set_brightness_acpi,
.brightness_get = keyboard_led_get_brightness_acpi,
- .max_brightness = ACPI_KEYBOARD_BACKLIGHT_MAX,
+ .max_brightness = KEYBOARD_BACKLIGHT_MAX,
};

#endif /* CONFIG_ACPI */

+#ifdef CONFIG_CROS_EC
+
+static int
+keyboard_led_set_brightness_ec_pwm(struct led_classdev *cdev,
+ enum led_brightness brightness)
+{
+ struct {
+ struct cros_ec_command msg;
+ struct ec_params_pwm_set_keyboard_backlight params;
+ } __packed buf;
+ struct ec_params_pwm_set_keyboard_backlight *params = &buf.params;
+ struct cros_ec_command *msg = &buf.msg;
+ struct keyboard_led *keyboard_led = container_of(cdev, struct keyboard_led, cdev);
+
+ memset(&buf, 0, sizeof(buf));
+
+ msg->command = EC_CMD_PWM_SET_KEYBOARD_BACKLIGHT;
+ msg->outsize = sizeof(*params);
+
+ params->percent = brightness;
+
+ return cros_ec_cmd_xfer_status(keyboard_led->ec, msg);
+}
+
+static enum led_brightness
+keyboard_led_get_brightness_ec_pwm(struct led_classdev *cdev)
+{
+ struct {
+ struct cros_ec_command msg;
+ struct ec_response_pwm_get_keyboard_backlight resp;
+ } __packed buf;
+ struct ec_response_pwm_get_keyboard_backlight *resp = &buf.resp;
+ struct cros_ec_command *msg = &buf.msg;
+ struct keyboard_led *keyboard_led = container_of(cdev, struct keyboard_led, cdev);
+ int ret;
+
+ memset(&buf, 0, sizeof(buf));
+
+ msg->command = EC_CMD_PWM_GET_KEYBOARD_BACKLIGHT;
+ msg->insize = sizeof(*resp);
+
+ ret = cros_ec_cmd_xfer_status(keyboard_led->ec, msg);
+ if (ret < 0)
+ return ret;
+
+ return resp->percent;
+}
+
+static int keyboard_led_init_ec_pwm(struct platform_device *pdev)
+{
+ struct keyboard_led *keyboard_led = platform_get_drvdata(pdev);
+
+ keyboard_led->ec = dev_get_drvdata(pdev->dev.parent);
+ if (!keyboard_led->ec) {
+ dev_err(&pdev->dev, "no parent EC device\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static const struct keyboard_led_drvdata keyboard_led_drvdata_ec_pwm = {
+ .init = keyboard_led_init_ec_pwm,
+ .brightness_set_blocking = keyboard_led_set_brightness_ec_pwm,
+ .brightness_get = keyboard_led_get_brightness_ec_pwm,
+ .max_brightness = KEYBOARD_BACKLIGHT_MAX,
+};
+
+#else /* CONFIG_CROS_EC */
+
+static const struct keyboard_led_drvdata keyboard_led_drvdata_ec_pwm = {};
+
+#endif /* CONFIG_CROS_EC */
+
static int keyboard_led_probe(struct platform_device *pdev)
{
- struct led_classdev *cdev;
const struct keyboard_led_drvdata *drvdata;
+ struct keyboard_led *keyboard_led;
int error;

drvdata = device_get_match_data(&pdev->dev);
if (!drvdata)
return -EINVAL;

+ keyboard_led = devm_kzalloc(&pdev->dev, sizeof(*keyboard_led), GFP_KERNEL);
+ if (!keyboard_led)
+ return -ENOMEM;
+ platform_set_drvdata(pdev, keyboard_led);
+
if (drvdata->init) {
error = drvdata->init(pdev);
if (error)
return error;
}

- cdev = devm_kzalloc(&pdev->dev, sizeof(*cdev), GFP_KERNEL);
- if (!cdev)
- return -ENOMEM;
-
- cdev->name = "chromeos::kbd_backlight";
- cdev->flags |= LED_CORE_SUSPENDRESUME;
- cdev->max_brightness = drvdata->max_brightness;
- cdev->brightness_set = drvdata->brightness_set;
- cdev->brightness_set_blocking = drvdata->brightness_set_blocking;
- cdev->brightness_get = drvdata->brightness_get;
+ keyboard_led->cdev.name = "chromeos::kbd_backlight";
+ keyboard_led->cdev.flags |= LED_CORE_SUSPENDRESUME;
+ keyboard_led->cdev.max_brightness = drvdata->max_brightness;
+ keyboard_led->cdev.brightness_set = drvdata->brightness_set;
+ keyboard_led->cdev.brightness_set_blocking = drvdata->brightness_set_blocking;
+ keyboard_led->cdev.brightness_get = drvdata->brightness_get;

- error = devm_led_classdev_register(&pdev->dev, cdev);
+ error = devm_led_classdev_register(&pdev->dev, &keyboard_led->cdev);
if (error)
return error;

@@ -158,6 +240,7 @@ MODULE_DEVICE_TABLE(acpi, keyboard_led_acpi_match);
static const struct of_device_id keyboard_led_of_match[] = {
{
.compatible = "google,cros-kbd-led-backlight",
+ .data = &keyboard_led_drvdata_ec_pwm,
},
{}
};
--
2.36.1.124.g0e6072fb45-goog


Subject: Re: [PATCH v4 0/5] platform/chrome: cros_kbd_led_backlight: add EC PWM backend

Hello:

This series was applied to chrome-platform/linux.git (for-kernelci)
by Tzung-Bi Shih <[email protected]>:

On Mon, 23 May 2022 17:08:17 +0800 you wrote:
> The series adds EC PWM as an backend option for ChromeOS keyboard LED
> backlight.
>
> The 1st patch reorder the headers alphabetically.
>
> The 2nd patch separates the ACPI backend.
>
> [...]

Here is the summary with links:
- [v4,1/5] platform/chrome: cros_kbd_led_backlight: sort headers alphabetically
https://git.kernel.org/chrome-platform/c/a4da30150ab4
- [v4,2/5] platform/chrome: cros_kbd_led_backlight: separate ACPI backend
(no matching commit)
- [v4,3/5] dt-bindings: add google,cros-kbd-led-backlight
(no matching commit)
- [v4,4/5] platform/chrome: cros_kbd_led_backlight: support OF match
(no matching commit)
- [v4,5/5] platform/chrome: cros_kbd_led_backlight: support EC PWM backend
(no matching commit)

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



2022-05-24 04:34:21

by Tzung-Bi Shih

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] platform/chrome: cros_kbd_led_backlight: add EC PWM backend

On Tue, May 24, 2022 at 8:50 AM
<[email protected]> wrote:
>
> Hello:
>
> This series was applied to chrome-platform/linux.git (for-kernelci)
> by Tzung-Bi Shih <[email protected]>:
>
> On Mon, 23 May 2022 17:08:17 +0800 you wrote:
> > The series adds EC PWM as an backend option for ChromeOS keyboard LED
> > backlight.
> >
> > The 1st patch reorder the headers alphabetically.
> >
> > The 2nd patch separates the ACPI backend.
> >
> > [...]
>
> Here is the summary with links:
> - [v4,1/5] platform/chrome: cros_kbd_led_backlight: sort headers alphabetically
> https://git.kernel.org/chrome-platform/c/a4da30150ab4
> - [v4,2/5] platform/chrome: cros_kbd_led_backlight: separate ACPI backend
> (no matching commit)
> - [v4,3/5] dt-bindings: add google,cros-kbd-led-backlight
> (no matching commit)
> - [v4,4/5] platform/chrome: cros_kbd_led_backlight: support OF match
> (no matching commit)
> - [v4,5/5] platform/chrome: cros_kbd_led_backlight: support EC PWM backend
> (no matching commit)

No. They haven't all got R-b tags and one of the commits was pushed
mistakenly. I have updated the for-kernelci branch again to fix it
up.

2022-05-25 05:37:32

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 4/5] platform/chrome: cros_kbd_led_backlight: support OF match

On Mon, May 23, 2022 at 05:08:21PM +0800, Tzung-Bi Shih wrote:
> For letting device tree based machines to use the driver, support OF match.
>
> Reviewed-by: Guenter Roeck <[email protected]>
> Signed-off-by: Tzung-Bi Shih <[email protected]>
> ---
> No changes from v3.
>
> Changes from v2:
> - Add commit message.
> - Add R-b tag.
>
> Changes from v1:
> (https://patchwork.kernel.org/project/chrome-platform/patch/[email protected]/)
> - Update email address accordingly.
> - Use device_get_match_data() per review comment in v1.
>
> drivers/platform/chrome/cros_kbd_led_backlight.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/cros_kbd_led_backlight.c b/drivers/platform/chrome/cros_kbd_led_backlight.c
> index a86d664854ae..4bca880d7721 100644
> --- a/drivers/platform/chrome/cros_kbd_led_backlight.c
> +++ b/drivers/platform/chrome/cros_kbd_led_backlight.c
> @@ -10,7 +10,9 @@
> #include <linux/kernel.h>
> #include <linux/leds.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/property.h>
> #include <linux/slab.h>
>
> /**
> @@ -116,7 +118,7 @@ static int keyboard_led_probe(struct platform_device *pdev)
> const struct keyboard_led_drvdata *drvdata;
> int error;
>
> - drvdata = acpi_device_get_match_data(&pdev->dev);
> + drvdata = device_get_match_data(&pdev->dev);
> if (!drvdata)
> return -EINVAL;
>
> @@ -152,10 +154,21 @@ static const struct acpi_device_id keyboard_led_acpi_match[] = {
> MODULE_DEVICE_TABLE(acpi, keyboard_led_acpi_match);
> #endif
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id keyboard_led_of_match[] = {
> + {
> + .compatible = "google,cros-kbd-led-backlight",
> + },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, keyboard_led_of_match);
> +#endif
> +
> static struct platform_driver keyboard_led_driver = {
> .driver = {
> .name = "chromeos-keyboard-leds",
> .acpi_match_table = ACPI_PTR(keyboard_led_acpi_match),
> + .of_match_table = of_match_ptr(keyboard_led_of_match),

You need to put this assignment inside an '#ifdef CONFIG_OF' block,
otherwise the compiler won't find 'keyboard_led_of_match' when
CONFIG_OF isn't set.

2022-05-25 08:20:38

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] platform/chrome: cros_kbd_led_backlight: support EC PWM backend

On Mon, May 23, 2022 at 05:08:22PM +0800, Tzung-Bi Shih wrote:
> EC PWM backend uses EC_CMD_PWM_SET_KEYBOARD_BACKLIGHT and
> EC_CMD_PWM_GET_KEYBOARD_BACKLIGHT for setting and getting the brightness
> respectively.
>
> Signed-off-by: Tzung-Bi Shih <[email protected]>

Reviewed-by: Matthias Kaehlcke <[email protected]>
Tested-by: Matthias Kaehlcke <[email protected]>

2022-05-25 15:41:22

by Tzung-Bi Shih

[permalink] [raw]
Subject: Re: [PATCH v4 4/5] platform/chrome: cros_kbd_led_backlight: support OF match

On Tue, May 24, 2022 at 01:08:00PM -0700, Matthias Kaehlcke wrote:
> On Mon, May 23, 2022 at 05:08:21PM +0800, Tzung-Bi Shih wrote:
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id keyboard_led_of_match[] = {
> > + {
> > + .compatible = "google,cros-kbd-led-backlight",
> > + },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, keyboard_led_of_match);
> > +#endif
> > +
> > static struct platform_driver keyboard_led_driver = {
> > .driver = {
> > .name = "chromeos-keyboard-leds",
> > .acpi_match_table = ACPI_PTR(keyboard_led_acpi_match),
> > + .of_match_table = of_match_ptr(keyboard_led_of_match),
>
> You need to put this assignment inside an '#ifdef CONFIG_OF' block,
> otherwise the compiler won't find 'keyboard_led_of_match' when
> CONFIG_OF isn't set.

It doesn't need as of_match_ptr() already guarded it.

2022-05-25 19:17:10

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 4/5] platform/chrome: cros_kbd_led_backlight: support OF match

On Wed, May 25, 2022 at 11:33:25AM +0800, Tzung-Bi Shih wrote:
> On Tue, May 24, 2022 at 01:08:00PM -0700, Matthias Kaehlcke wrote:
> > On Mon, May 23, 2022 at 05:08:21PM +0800, Tzung-Bi Shih wrote:
> > > +#ifdef CONFIG_OF
> > > +static const struct of_device_id keyboard_led_of_match[] = {
> > > + {
> > > + .compatible = "google,cros-kbd-led-backlight",
> > > + },
> > > + {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, keyboard_led_of_match);
> > > +#endif
> > > +
> > > static struct platform_driver keyboard_led_driver = {
> > > .driver = {
> > > .name = "chromeos-keyboard-leds",
> > > .acpi_match_table = ACPI_PTR(keyboard_led_acpi_match),
> > > + .of_match_table = of_match_ptr(keyboard_led_of_match),
> >
> > You need to put this assignment inside an '#ifdef CONFIG_OF' block,
> > otherwise the compiler won't find 'keyboard_led_of_match' when
> > CONFIG_OF isn't set.
>
> It doesn't need as of_match_ptr() already guarded it.

I learned something new today :)

Reviewed-by: Matthias Kaehlcke <[email protected]>

2022-05-26 06:00:47

by Prashant Malani

[permalink] [raw]
Subject: Re: [PATCH v4 4/5] platform/chrome: cros_kbd_led_backlight: support OF match

Hi Tzung-Bi,

On May 23 17:08, Tzung-Bi Shih wrote:
> For letting device tree based machines to use the driver, support OF match.
>
> Reviewed-by: Guenter Roeck <[email protected]>
> Signed-off-by: Tzung-Bi Shih <[email protected]>
> ---
> No changes from v3.
>
> Changes from v2:
> - Add commit message.
> - Add R-b tag.
>
> Changes from v1:
> (https://patchwork.kernel.org/project/chrome-platform/patch/[email protected]/)
> - Update email address accordingly.
> - Use device_get_match_data() per review comment in v1.
>
> drivers/platform/chrome/cros_kbd_led_backlight.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/cros_kbd_led_backlight.c b/drivers/platform/chrome/cros_kbd_led_backlight.c
> index a86d664854ae..4bca880d7721 100644
> --- a/drivers/platform/chrome/cros_kbd_led_backlight.c
> +++ b/drivers/platform/chrome/cros_kbd_led_backlight.c
> @@ -10,7 +10,9 @@
> #include <linux/kernel.h>
> #include <linux/leds.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/property.h>

linux/of.h includes linux/property.h [1]

[1] https://elixir.bootlin.com/linux/v5.18/source/include/linux/of.h#L22

> #include <linux/slab.h>
>
> /**
> @@ -116,7 +118,7 @@ static int keyboard_led_probe(struct platform_device *pdev)
> const struct keyboard_led_drvdata *drvdata;
> int error;
>
> - drvdata = acpi_device_get_match_data(&pdev->dev);
> + drvdata = device_get_match_data(&pdev->dev);
> if (!drvdata)
> return -EINVAL;
>
> @@ -152,10 +154,21 @@ static const struct acpi_device_id keyboard_led_acpi_match[] = {
> MODULE_DEVICE_TABLE(acpi, keyboard_led_acpi_match);
> #endif
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id keyboard_led_of_match[] = {
> + {
> + .compatible = "google,cros-kbd-led-backlight",
> + },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, keyboard_led_of_match);
> +#endif
> +
> static struct platform_driver keyboard_led_driver = {
> .driver = {
> .name = "chromeos-keyboard-leds",
> .acpi_match_table = ACPI_PTR(keyboard_led_acpi_match),
> + .of_match_table = of_match_ptr(keyboard_led_of_match),
> },
> .probe = keyboard_led_probe,
> };
> --
> 2.36.1.124.g0e6072fb45-goog
>
>

2022-06-09 08:56:27

by Tzung-Bi Shih

[permalink] [raw]
Subject: Re: [PATCH v4 2/5] platform/chrome: cros_kbd_led_backlight: separate ACPI backend

On Mon, May 23, 2022 at 05:08:19PM +0800, Tzung-Bi Shih wrote:
> cros_kbd_led_backlight uses ACPI_KEYBOARD_BACKLIGHT_WRITE and
> ACPI_KEYBOARD_BACKLIGHT_READ for setting and getting the brightness
> respectively.
>
> Separate ACPI operations for preparing the driver to support other
> backends.

The patch is the last one in the series that hasn't got a R-b tag.

I have tested the patch on an ADL-P-based chromebook and the keyboard
backlight works. Could anyone on the list help to review the patch?

2022-06-09 13:22:45

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v4 2/5] platform/chrome: cros_kbd_led_backlight: separate ACPI backend

On Mon, May 23, 2022 at 2:08 AM Tzung-Bi Shih <[email protected]> wrote:
>
> cros_kbd_led_backlight uses ACPI_KEYBOARD_BACKLIGHT_WRITE and
> ACPI_KEYBOARD_BACKLIGHT_READ for setting and getting the brightness
> respectively.
>
> Separate ACPI operations for preparing the driver to support other
> backends.
>
> Signed-off-by: Tzung-Bi Shih <[email protected]>

Reviewed-by: Guenter Roeck <[email protected]>

> ---
> Changes from v3:
> - Remove CROS_KBD_LED_BACKLIGHT_ACPI Kconfig.
> - Remove stub function.
>
> Changes from v2:
> - Use #ifdef for boolean CONFIG_CROS_KBD_LED_BACKLIGHT_ACPI.
>
> Changes from v1:
> - Update email address accordingly.
> - Use CONFIG_ACPI guard per "kernel test robot <[email protected]>" reported an
> unused variable issue.
>
> .../platform/chrome/cros_kbd_led_backlight.c | 82 ++++++++++++++++---
> 1 file changed, 69 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_kbd_led_backlight.c b/drivers/platform/chrome/cros_kbd_led_backlight.c
> index f9587a562bb7..a86d664854ae 100644
> --- a/drivers/platform/chrome/cros_kbd_led_backlight.c
> +++ b/drivers/platform/chrome/cros_kbd_led_backlight.c
> @@ -13,6 +13,33 @@
> #include <linux/platform_device.h>
> #include <linux/slab.h>
>
> +/**
> + * struct keyboard_led_drvdata - keyboard LED driver data.
> + * @init: Init function.
> + * @brightness_get: Get LED brightness level.
> + * @brightness_set: Set LED brightness level. Must not sleep.
> + * @brightness_set_blocking: Set LED brightness level. It can block the
> + * caller for the time required for accessing a
> + * LED device register
> + * @max_brightness: Maximum brightness.
> + *
> + * See struct led_classdev in include/linux/leds.h for more details.
> + */
> +struct keyboard_led_drvdata {
> + int (*init)(struct platform_device *pdev);
> +
> + enum led_brightness (*brightness_get)(struct led_classdev *led_cdev);
> +
> + void (*brightness_set)(struct led_classdev *led_cdev,
> + enum led_brightness brightness);
> + int (*brightness_set_blocking)(struct led_classdev *led_cdev,
> + enum led_brightness brightness);
> +
> + enum led_brightness max_brightness;
> +};
> +
> +#ifdef CONFIG_ACPI
> +
> /* 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"
> @@ -20,8 +47,8 @@
>
> #define ACPI_KEYBOARD_BACKLIGHT_MAX 100
>
> -static void keyboard_led_set_brightness(struct led_classdev *cdev,
> - enum led_brightness brightness)
> +static void keyboard_led_set_brightness_acpi(struct led_classdev *cdev,
> + enum led_brightness brightness)
> {
> union acpi_object param;
> struct acpi_object_list input;
> @@ -40,7 +67,7 @@ static void keyboard_led_set_brightness(struct led_classdev *cdev,
> }
>
> static enum led_brightness
> -keyboard_led_get_brightness(struct led_classdev *cdev)
> +keyboard_led_get_brightness_acpi(struct led_classdev *cdev)
> {
> unsigned long long brightness;
> acpi_status status;
> @@ -56,12 +83,10 @@ keyboard_led_get_brightness(struct led_classdev *cdev)
> return brightness;
> }
>
> -static int keyboard_led_probe(struct platform_device *pdev)
> +static int keyboard_led_init_acpi(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,
> @@ -73,15 +98,44 @@ static int keyboard_led_probe(struct platform_device *pdev)
> return -ENXIO;
> }
>
> + return 0;
> +}
> +
> +static const struct keyboard_led_drvdata keyboard_led_drvdata_acpi = {
> + .init = keyboard_led_init_acpi,
> + .brightness_set = keyboard_led_set_brightness_acpi,
> + .brightness_get = keyboard_led_get_brightness_acpi,
> + .max_brightness = ACPI_KEYBOARD_BACKLIGHT_MAX,
> +};
> +
> +#endif /* CONFIG_ACPI */
> +
> +static int keyboard_led_probe(struct platform_device *pdev)
> +{
> + struct led_classdev *cdev;
> + const struct keyboard_led_drvdata *drvdata;
> + int error;
> +
> + drvdata = acpi_device_get_match_data(&pdev->dev);
> + if (!drvdata)
> + return -EINVAL;
> +
> + if (drvdata->init) {
> + error = drvdata->init(pdev);
> + if (error)
> + return error;
> + }
> +
> cdev = devm_kzalloc(&pdev->dev, sizeof(*cdev), GFP_KERNEL);
> if (!cdev)
> return -ENOMEM;
>
> cdev->name = "chromeos::kbd_backlight";
> - cdev->max_brightness = ACPI_KEYBOARD_BACKLIGHT_MAX;
> cdev->flags |= LED_CORE_SUSPENDRESUME;
> - cdev->brightness_set = keyboard_led_set_brightness;
> - cdev->brightness_get = keyboard_led_get_brightness;
> + cdev->max_brightness = drvdata->max_brightness;
> + cdev->brightness_set = drvdata->brightness_set;
> + cdev->brightness_set_blocking = drvdata->brightness_set_blocking;
> + cdev->brightness_get = drvdata->brightness_get;
>
> error = devm_led_classdev_register(&pdev->dev, cdev);
> if (error)
> @@ -90,16 +144,18 @@ static int keyboard_led_probe(struct platform_device *pdev)
> return 0;
> }
>
> -static const struct acpi_device_id keyboard_led_id[] = {
> - { "GOOG0002", 0 },
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id keyboard_led_acpi_match[] = {
> + { "GOOG0002", (kernel_ulong_t)&keyboard_led_drvdata_acpi },
> { }
> };
> -MODULE_DEVICE_TABLE(acpi, keyboard_led_id);
> +MODULE_DEVICE_TABLE(acpi, keyboard_led_acpi_match);
> +#endif
>
> static struct platform_driver keyboard_led_driver = {
> .driver = {
> .name = "chromeos-keyboard-leds",
> - .acpi_match_table = ACPI_PTR(keyboard_led_id),
> + .acpi_match_table = ACPI_PTR(keyboard_led_acpi_match),
> },
> .probe = keyboard_led_probe,
> };
> --
> 2.36.1.124.g0e6072fb45-goog
>

Subject: Re: [PATCH v4 0/5] platform/chrome: cros_kbd_led_backlight: add EC PWM backend

Hello:

This series was applied to chrome-platform/linux.git (for-kernelci)
by Tzung-Bi Shih <[email protected]>:

On Mon, 23 May 2022 17:08:17 +0800 you wrote:
> The series adds EC PWM as an backend option for ChromeOS keyboard LED
> backlight.
>
> The 1st patch reorder the headers alphabetically.
>
> The 2nd patch separates the ACPI backend.
>
> [...]

Here is the summary with links:
- [v4,1/5] platform/chrome: cros_kbd_led_backlight: sort headers alphabetically
https://git.kernel.org/chrome-platform/c/337eac8f8499
- [v4,2/5] platform/chrome: cros_kbd_led_backlight: separate ACPI backend
https://git.kernel.org/chrome-platform/c/6b1e5ba39c44
- [v4,3/5] dt-bindings: add google,cros-kbd-led-backlight
https://git.kernel.org/chrome-platform/c/20f370efddb5
- [v4,4/5] platform/chrome: cros_kbd_led_backlight: support OF match
https://git.kernel.org/chrome-platform/c/fd1e8054ff69
- [v4,5/5] platform/chrome: cros_kbd_led_backlight: support EC PWM backend
https://git.kernel.org/chrome-platform/c/40f58143745e

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html


Subject: Re: [PATCH v4 0/5] platform/chrome: cros_kbd_led_backlight: add EC PWM backend

Hello:

This series was applied to chrome-platform/linux.git (for-next)
by Tzung-Bi Shih <[email protected]>:

On Mon, 23 May 2022 17:08:17 +0800 you wrote:
> The series adds EC PWM as an backend option for ChromeOS keyboard LED
> backlight.
>
> The 1st patch reorder the headers alphabetically.
>
> The 2nd patch separates the ACPI backend.
>
> [...]

Here is the summary with links:
- [v4,1/5] platform/chrome: cros_kbd_led_backlight: sort headers alphabetically
https://git.kernel.org/chrome-platform/c/337eac8f8499
- [v4,2/5] platform/chrome: cros_kbd_led_backlight: separate ACPI backend
https://git.kernel.org/chrome-platform/c/6b1e5ba39c44
- [v4,3/5] dt-bindings: add google,cros-kbd-led-backlight
https://git.kernel.org/chrome-platform/c/20f370efddb5
- [v4,4/5] platform/chrome: cros_kbd_led_backlight: support OF match
https://git.kernel.org/chrome-platform/c/fd1e8054ff69
- [v4,5/5] platform/chrome: cros_kbd_led_backlight: support EC PWM backend
https://git.kernel.org/chrome-platform/c/40f58143745e

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html