linux-kernel - Re: [PATCH] platform/chrome: cros_kbd_led_backlight: enable probing through EC_FEATURE_PWM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sun, 5 May 2024 08:42:21 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: Thomas Weißschuh <linux@...ssschuh.net>,
 Lee Jones <lee@...nel.org>, Benson Leung <bleung@...omium.org>,
 Guenter Roeck <groeck@...omium.org>, Tzung-Bi Shih <tzungbi@...nel.org>
Cc: chrome-platform@...ts.linux.dev, linux-kernel@...r.kernel.org,
 Dustin Howett <dustin@...ett.net>
Subject: Re: [PATCH] platform/chrome: cros_kbd_led_backlight: enable probing
 through EC_FEATURE_PWM_KEYB

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@...ssschuh.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,