Re: [PATCH] platform/chrome: cros_kbd_led_backlight: enable probing through EC_FEATURE_PWM_KEYB

Mario Limonciello
2024-05-05T13:42:26Z

On 5/5/2024 04:41, Thomas Weißschuh wrote:
> The ChromeOS EC used in Framework laptops supports the standard cros
> keyboard backlight protocol.
> However the firmware on these laptops don't implement the ACPI ID
> GOOG0002 that is recognized by cros_kbd_led_backlight and they also
> don't use device tree.

Something I'd wonder is if the GOOG0002 ACPI ID can go away entirely
with this type of change. Presumably the Chromebooks with ChromeOS EC
/also/ advertise EC_FEATURE_PWM_KEYB.

>
> Extend the cros_ec MFD device to also load cros_kbd_led_backlight
> when the EC reports EC_FEATURE_PWM_KEYB.
>
> Tested on a Framework 13 AMD, Bios 3.05.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> This is based on
> https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
>
> The helper keyboard_led_is_mfd_device is a bit iffy, but I couldn't find
> a nicer way.
>
> * driver_data from platform_device_id is overwritten by the mfd platform data
> * Setting the driver_data in drivers/mfd/cros_ec_dev.c would expose the
> internals of cros_kbd_led_backlight
> ---
> drivers/mfd/cros_ec_dev.c | 9 ++++++
> drivers/platform/chrome/cros_kbd_led_backlight.c | 41 +++++++++++++++++++++++-
> 2 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index a52d59cc2b1e..4444b361aeae 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -99,6 +99,10 @@ static const struct mfd_cell cros_ec_wdt_cells[] = {
> { .name = "cros-ec-wdt", }
> };
>
> +static const struct mfd_cell cros_ec_keyboard_leds_cells[] = {
> + { .name = "cros-keyboard-leds", },
> +};
> +
> static const struct cros_feature_to_cells cros_subdevices[] = {
> {
> .id = EC_FEATURE_CEC,
> @@ -125,6 +129,11 @@ static const struct cros_feature_to_cells cros_subdevices[] = {
> .mfd_cells = cros_ec_wdt_cells,
> .num_cells = ARRAY_SIZE(cros_ec_wdt_cells),
> },
> + {
> + .id = EC_FEATURE_PWM_KEYB,
> + .mfd_cells = cros_ec_keyboard_leds_cells,
> + .num_cells = ARRAY_SIZE(cros_ec_keyboard_leds_cells),
> + },
> };
>
> static const struct mfd_cell cros_ec_platform_cells[] = {
> diff --git a/drivers/platform/chrome/cros_kbd_led_backlight.c b/drivers/platform/chrome/cros_kbd_led_backlight.c
> index b83e4f328620..88dd3848e5da 100644
> --- a/drivers/platform/chrome/cros_kbd_led_backlight.c
> +++ b/drivers/platform/chrome/cros_kbd_led_backlight.c
> @@ -194,13 +194,52 @@ static const __maybe_unused struct keyboard_led_drvdata keyboard_led_drvdata_ec_
>
> #endif /* IS_ENABLED(CONFIG_CROS_EC) */
>
> +#if IS_ENABLED(CONFIG_MFD_CROS_EC_DEV)
> +static int keyboard_led_init_ec_pwm_mfd(struct platform_device *pdev)
> +{
> + struct cros_ec_dev *ec_dev = dev_get_drvdata(pdev->dev.parent);
> + struct cros_ec_device *cros_ec = ec_dev->ec_dev;
> + struct keyboard_led *keyboard_led = platform_get_drvdata(pdev);
> +
> + keyboard_led->ec = cros_ec;
> +
> + return 0;
> +}
> +
> +static const struct keyboard_led_drvdata keyboard_led_drvdata_ec_pwm_mfd = {
> + .init = keyboard_led_init_ec_pwm_mfd,
> + .brightness_set_blocking = keyboard_led_set_brightness_ec_pwm,
> + .brightness_get = keyboard_led_get_brightness_ec_pwm,
> + .max_brightness = KEYBOARD_BACKLIGHT_MAX,
> +};
> +
> +#else /* IS_ENABLED(CONFIG_MFD_CROS_EC_DEV) */
> +
> +static const struct keyboard_led_drvdata keyboard_led_drvdata_ec_pwm = {};
> +
> +#endif /* IS_ENABLED(CONFIG_MFD_CROS_EC_DEV) */
> +
> +static int keyboard_led_is_mfd_device(struct platform_device *pdev)
> +{
> + if (!IS_ENABLED(CONFIG_MFD_CROS_EC_DEV))
> + return 0;
> +
> + if (!pdev->dev.parent)
> + return 0;
> +
> + return strcmp(pdev->dev.parent->driver->name, "cros-ec-dev") == 0;
> +}
> +
> static int keyboard_led_probe(struct platform_device *pdev)
> {
> const struct keyboard_led_drvdata *drvdata;
> struct keyboard_led *keyboard_led;
> int error;
>
> - drvdata = device_get_match_data(&pdev->dev);
> + if (keyboard_led_is_mfd_device(pdev))
> + drvdata = &keyboard_led_drvdata_ec_pwm_mfd;
> + else
> + drvdata = device_get_match_data(&pdev->dev);
> if (!drvdata)
> return -EINVAL;
>
>
> ---
> base-commit: 2fbe479c0024e1c6b992184a799055e19932aa48
> change-id: 20240505-cros_ec-kbd-led-framework-7e2e831bc79c
>
> Best regards,