Received-SPF: pass (google.com: domain of linux-kernel+bounces-168989-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <cd9c5b6a-0155-434a-b868-a9ea52e878c9@amd.com>
Date: Sun, 5 May 2024 08:42:21 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] platform/chrome: cros_kbd_led_backlight: enable probing
 through EC_FEATURE_PWM_KEYB
To: =?UTF-8?Q?Thomas_Wei=C3=9Fschuh?= <linux@weissschuh.net>,
 Lee Jones <lee@kernel.org>, Benson Leung <bleung@chromium.org>,
 Guenter Roeck <groeck@chromium.org>, Tzung-Bi Shih <tzungbi@kernel.org>
Cc: chrome-platform@lists.linux.dev, linux-kernel@vger.kernel.org,
 Dustin Howett <dustin@howett.net>
References: <20240505-cros_ec-kbd-led-framework-v1-1-bfcca69013d2@weissschuh.net>
Content-Language: en-US
From: Mario Limonciello <mario.limonciello@amd.com>
In-Reply-To: <20240505-cros_ec-kbd-led-framework-v1-1-bfcca69013d2@weissschuh.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?M2tFZzB5Q2VISE1rcHYxeXRKTWNlOU8rU3B4VXZKTVFwYVZMNkZQRU1EUWRI?=
 =?utf-8?B?YTN6allwS01OdzIyQVhORmVvQ09qTEo1QjIybUtQd1FHTDZiOEdLcEZyZ2Yz?=
 =?utf-8?B?eXUxTFVzQW5lbTFHS1YydnBkZGp5c1k2ZDdhT1AxNDJ0dVJFQndFY0diQWFs?=
 =?utf-8?B?MDhOWlluTVVYV0JKTTlPb0RkSVJHY3hXVXRrY1FKbHY2OUtQRTZQRDRFMkR0?=
 =?utf-8?B?SFRJWlphd0NabUtSc0dHUXF4NWVCcGhIL05JNTZMMVE3UjU4aUJIVncyc25a?=
 =?utf-8?B?dXM0ZUpONTBHV3F2d1dQejF5N2owbTJJWmhza3BTRVB2V2JHeWk3RnFJYkdW?=
 =?utf-8?B?SWpQQmpwbjVCZ2NNNS8vNlc2MDJJdjBXV3d2Q1lNT2thaWFRdGRpQ05Yem94?=
 =?utf-8?B?dVZEejRRc1NIU1I1c3QvbW5rbnlrbmFvbHFXN1pFUEdSVThDY0hYVWlvV1lo?=
 =?utf-8?B?RjFiQzNHYUpMeVNQUVMzVnFBbUFiYzdiaTB6VFhlMzNFUXpPRW51WlI0dzlT?=
 =?utf-8?B?ckQyR0JTMklEVmNjVXpXdE9rc015anVUQkwyRXFkcU9ydnZNNnNLY2dhYVpI?=
 =?utf-8?B?ZHpGazdlZFoxMUcza2k3SVZXazFaaTBVeXUvQ3MzYW1leVo2dWZ2cW5hZVJi?=
 =?utf-8?B?L3RJWDJ3cy9JTFUzSEdaNnhZcUlkTTk5dXprVHlUcjNOK0J0eDJtOW85SURZ?=
 =?utf-8?B?cXUxZjAwNXU3WjhsRGY2V3Z4bXFzWVp5QWtuZjAzL0dEb0VRK3NINUprNGF4?=
 =?utf-8?B?RGN0SGxodVhQbnIrT2JLV3ROcUhVOFZwd0RHMyt1Q0EzRWRzZHNnTlAzaGRo?=
 =?utf-8?B?NDY2M2Q3OHlpK3RYTU8xZWxWTk8wRCsvTERoQTlCclhGeW5NcG9JL2JtSmZi?=
 =?utf-8?B?aXFOVklNUnRma0FsMXRXdi8wTC9mdVVKdXIrNEI3UEFjMXNobythSXBqeGxE?=
 =?utf-8?B?cVFvTytmc3pZNW9aSVZmb05NNTdSdkdRc283eE1JaG52MVIwR0NEdnRMd3pI?=
 =?utf-8?B?U1pQVU5zV05Fd0RXUjVpRjRySkV5UnBscldreGQ2ZlkxcWN1SGppVU5JYXlW?=
 =?utf-8?B?YzJEd1RGWlBKeDEwN1ZCTlY2WUlEZldFNHhQQldCNm85NHRJblEwaGJqK0gr?=
 =?utf-8?B?ZEZwU0JNY1U5ZUw4T1hkMExDYTg4RzFUZ2lMdUNmbXIzc3FDTzRja2ZEdnZE?=
 =?utf-8?B?Z1YxY2Q1SkI2VGJxdVVmQVVhKzNDaEtmZHlkVENDNlZScWRaTHlSQnY2UDhT?=
 =?utf-8?B?OXRoWTJxRWQxWjZUY0I4OHpBRWVVb1hoelJCUVlCbTNEN0JpU3pRSnNjYURH?=
 =?utf-8?B?VERpYy8yRm9pTTBiR01SWThHamJ2SmdqZmtuREQxZlEza1IxdjJsZkFpLzJT?=
 =?utf-8?B?bGJsaW01WTdVRHVtaWRlMVBGSFR1L25WcmVwbnowcHlsSm52M2pTNmZNWkVp?=
 =?utf-8?B?SU9jdHFMZGZidFc2SWN3YjM5RDNKVW5JSStnSFpCTG8zSmxHMS9uSDZIREdj?=
 =?utf-8?B?Vk1Tc29ZTmZPTWE4aTlCSnpHcnRNVzRVR3JuV1M5bkh6aGhKRkVHMW9GMVBr?=
 =?utf-8?B?bjI0T0lpRmdRMzdVenRNT0tGR1prOE01TnV1dVpWV09mZ1ZoN2t2MjhaL1B6?=
 =?utf-8?B?aFhTdDZjREhaVmtSMXV0cVNBTk5UVm5wK1IwQmxjbmVWVGlrT0tldUUxbE1W?=
 =?utf-8?B?T3B5RWNoNWpSdlJiUUx0QU82enh0N0JBV2EwMGErZFgycnFaekMrYWF2YWJq?=
 =?utf-8?B?OHV6QlQwbTN1bGhNdmhwb3I3anlwdUNlWWQvQzRPYU9sRWc3MnVISkZYYkhO?=
 =?utf-8?B?VjlzNTFpMzlwdW5lZUdzMGNKclFEaEpvaHN6cXFNVG5uVkJYc1hGRjRVWHNw?=
 =?utf-8?B?MUcveHl0cmJzSkNEb3JxTko1cmxwRlJpSUFGeE1rQlF2RDd3T0FXclJVSFh6?=
 =?utf-8?B?V3hpWVNNUitmYzB2bm9DWUl2WEhtczJKNGZFeU1Tak5hZjg4ckIvZE9QSWJk?=
 =?utf-8?B?bGdhaE1oMmgvMFhoVmsxT3RicFpna2Fra2duOHVHQ3dFMXN3bDZhRm95cFZZ?=
 =?utf-8?B?eVk0UllCQUtkSlN2cGhrak5OUmRFeUExT0dvdzMxOUdmY0xCNmoyRjBGYUZW?=
 =?utf-8?Q?g/Qi9ZrW+xaAqkkjWtdQ9SqFs?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: aa7da996-6f12-464a-d2f8-08dc6d093202
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 May 2024 13:42:24.2276
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: PBZFcWjGkLlY2sAKso/oVdn0LNH1R/5oGAtIDUw6flAFtRnaofngL4jVdNoKRCtgr7QNRVJxTuPg5g/EaFucAA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR12MB5792

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,