| Message ID | 20220523090822.3035189-3-tzungbi@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6b1e5ba39c44442c5ca115d89056823e064f1796 |
| Headers | show |
| Series | platform/chrome: cros_kbd_led_backlight: add EC PWM backend | expand |
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 >
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, };
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> --- 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(-)