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

Guenter Roeck
2022-06-09T12:45:01Z

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
>