| Message ID | 20220523090822.3035189-1-tzungbi@kernel.org |
|---|---|
| Headers | show |
| Series | platform/chrome: cros_kbd_led_backlight: add EC PWM backend | expand |
Hello: This series was applied to chrome-platform/linux.git (for-kernelci) by Tzung-Bi Shih <tzungbi@kernel.org>: 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!
On Tue, May 24, 2022 at 8:50 AM <patchwork-bot+chrome-platform@kernel.org> wrote: > > Hello: > > This series was applied to chrome-platform/linux.git (for-kernelci) > by Tzung-Bi Shih <tzungbi@kernel.org>: > > 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.
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 <linux@roeck-us.net> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org> > --- > 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/20220214053646.3088298-5-tzungbi@google.com/) > - 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.
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 <tzungbi@kernel.org> Reviewed-by: Matthias Kaehlcke <mka@chromium.org> Tested-by: Matthias Kaehlcke <mka@chromium.org>
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.
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 <mka@chromium.org>
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 <linux@roeck-us.net> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org> > --- > 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/20220214053646.3088298-5-tzungbi@google.com/) > - 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 > >
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?
On Mon, May 23, 2022 at 2:08 AM Tzung-Bi Shih <tzungbi@kernel.org> 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 <tzungbi@kernel.org> Reviewed-by: Guenter Roeck <groeck@chromium.org> > --- > 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 <lkp@intel.com>" 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 >
Hello: This series was applied to chrome-platform/linux.git (for-kernelci) by Tzung-Bi Shih <tzungbi@kernel.org>: 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!
Hello: This series was applied to chrome-platform/linux.git (for-next) by Tzung-Bi Shih <tzungbi@kernel.org>: 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!