Received-SPF: pass (google.com: domain of linux-kernel+bounces-169869-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <84c58078-93a4-406a-8abb-9054854e54a0@amd.com>
Date: Mon, 6 May 2024 08:10:58 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] platform/chrome: cros_ec_framework_laptop: introduce
 driver
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: linux-kernel@vger.kernel.org, chrome-platform@lists.linux.dev,
 "Dustin L. Howett" <dustin@howett.net>, Sebastian Reichel <sre@kernel.org>,
 linux-pm@vger.kernel.org
References: <20240505-cros_ec-framework-v1-0-402662d6276b@weissschuh.net>
 <20240505-cros_ec-framework-v1-1-402662d6276b@weissschuh.net>
Content-Language: en-US
From: "Limonciello, Mario" <mario.limonciello@amd.com>
In-Reply-To: <20240505-cros_ec-framework-v1-1-402662d6276b@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?cVNvVEt6dEMzYjJrSHg3ZG1QRXVEd29UaFI2VVZkTU5ZUDV1dm00WUpBVGor?=
 =?utf-8?B?V20wOXZ4V3NNNzVGaGduNDVLaGsrWHR5R2ZaTEtxQ1E3c2hwbDU3ME9QYjFL?=
 =?utf-8?B?YkZ0ajU4L0pETCtrVnJKZkk1QkE2NktpaVBCQUpXdmRYZWw2Z1RPa2tRM0JH?=
 =?utf-8?B?MnlVSmVJZWxad0ZlMkZ4RElxQklDUHl2YTMyNFVsRk02bG1FRUltTXRVY2JD?=
 =?utf-8?B?Q0FwWkN2c3ViY2FCTWpDN3VQaVliSnlNVm0yK0xjU2xGZU0zQm5XUlpMS2RR?=
 =?utf-8?B?aTJOM1ZGTEkzZFJqT3RENkNKZVZtY1RDd1VnbUltZzVYelFRdXV2Rk9sM2sx?=
 =?utf-8?B?cVpHYmZNNnlHT2xDaUVCa1dSUStsL0hjMnd1cUNwUUNBMTZxMU5PaTdWTnJ0?=
 =?utf-8?B?aGhhWkJ6Z3hBUVhCQ1YwVmFIOHBMQXhadklNU1dDM09PRnhTN0Y3VDBXSVk4?=
 =?utf-8?B?SE9vaUhQUFlPM1FHMGxDejhhRmIyaHYyaWlXQnVRa3pPM084WjZCUjRMM0dU?=
 =?utf-8?B?U3V5ZXowMFc3S2JuVFJ4Z2Z6THZmZW5WZXpuaVlZUlhRUXQ5R1pzUkczb3Aw?=
 =?utf-8?B?YmcvS0kvbGlXdTFXOXZubkxpczg1cGMybDkrSlRtVTFLN002VFNkRDhNb2Jy?=
 =?utf-8?B?YWlESEpvMGtoUlVyMEZtdTlnWVJWMmNucVJySERtRVo1K0FMWUwzc2U5K1FH?=
 =?utf-8?B?b0lHczhnc1ErNUdGS2lqbGUvSXo2cWE0cHJtbjEzeWJFdjNiNHJsbThkcmZ3?=
 =?utf-8?B?WW9xL0xidG9NNGUrRDE3TDNJNGRPbFBuUG9tMTlzTHFuTjZTMHowMW5zd2hD?=
 =?utf-8?B?dnU5UjN2MTM2bG42TENHMXBKLzhNOHRPaHU2N1R2NHI5NTFGMnQ4QmtYay9K?=
 =?utf-8?B?alpUU3ZjUytLeTFoQytYSDJQTnZaVnZaL2orNi96UFZEaDB5SmZSalRidjRR?=
 =?utf-8?B?RFdSY2QzUHpvdGxnWHJWd1ZIZnQybHEyN08zVmRKSXhEYU1BRlh5Q1lYYk1D?=
 =?utf-8?B?UkppOCtwTExlbUU3NDkyOStOOW9EYS9vT296YmN6RDBNdXZlenlLREFBemhX?=
 =?utf-8?B?a2lucTdYaldYK0JCRk9JNkRJNU11T3hqc2JYY3phcHAvNzVUaGxDN1dWU0hO?=
 =?utf-8?B?Y0tGbHZPY043aDUwK0hrYTFZak5vT0RWMXlTbXNRNms5RlRDSHF1VmNodTdl?=
 =?utf-8?B?U1ZqcmlLeHhYS3VFUkNCRlVwQ0dFUXpTNVhhaUVXTDM4a2lUalBQc3ljaGRF?=
 =?utf-8?B?bXBWdkZtdW01TkJjWkFJM2ZyWTQvL3dvMmlzUU81bzUzVHM5dXJHTVRQVFo0?=
 =?utf-8?B?RER2ZFVIblhLM1JCOTNrc1lsbndNclBJRjVpU3lrWDZXdTE2bCtvbjdNZE5z?=
 =?utf-8?B?dGRKWkgrL2E4NXZyY2JjcEE2eFBkaGZvRnBObU5HSlNJbVVhck44OSs1Y3Z4?=
 =?utf-8?B?MFhoNWdhNkN1eG1qK2FWT1Jla3FISzMrUEFkaGkya1p5bzZFNWNWVWYxWEY2?=
 =?utf-8?B?YllOaW85bUFwS2d5OFVXNmQxL2NaTVF1clJGMHhYMktYQ3ZNbFlEeHNpWUtP?=
 =?utf-8?B?V2RQSUVROXlWSkFHYlFmTnlDVFFTbkJRYmU4WUR4VTZrRzR0OGMzQ0RhMTg1?=
 =?utf-8?B?dnFTNmhiMmRoNDRFL1FCZ3laZ2NPSjZ4am9Ma1Z4cUtKNk5nRzkvb0IvOUhQ?=
 =?utf-8?B?cFNGMFR4eEJJSFRYRTBJQ2FqaGRIT0ZTcTR0NzFURjN6R2JQUkVrdmo5OUlH?=
 =?utf-8?B?dFZFR1djMnhxc29SdGZRM1d5TVY3VWkrbHdqT3ZiZEozSnErbnRmdmdtc0xY?=
 =?utf-8?B?SGtvRTdIYlFuNWJKcDZiTWltclFiOHlqQWtPTXJwWFNWU0t2NlVSNnlaUFRu?=
 =?utf-8?B?Q0YyRkllWVJldmZtdnNMSklIU0FGMW94Y1lKa0ZhUklrdXU4eHlMdUdTdDhs?=
 =?utf-8?B?OXYwTlRJTG5MVXZNSUxsenRzcDNrRkhFelRMNVhkZDF5UG9GSzBQRU1CV3N2?=
 =?utf-8?B?VytaS0QzQjNWcWY2MjFFVmVuaWJwdVlFM2RUOEM4OXoxZ1lqSXgzMHpqdXhn?=
 =?utf-8?B?Y2ZWNUNHVS9RWmw2QkdsWmsvbFlVSnNTS0lEOU51Z3pHQ3NrYlJscDMwaVpE?=
 =?utf-8?Q?g0fNKeFKk/AelA5aoeUQpHMeZ?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4a950818-c05b-4b43-894e-08dc6dcdf99d
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 May 2024 13:11:00.4991
 (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: 4CpCdjcunnMgg8p+rZV7Hs9S5lC8iETWtXdcZtCGmx29+tbWqHx0Mxk/1S0Ic1G4t/pUVoPtySgqtq24s+mV9w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR12MB5609


On 5/5/2024 3:56 PM, Thomas Weißschuh wrote:
> Framework Laptops are using embedded controller firmware based on the
> ChromeOS EC project.
> In addition to the standard upstream commands, some vendor-specific
> ones are implemented.
> 
> Add a driver for those custom EC commands.
> 
> At first, provide an empty driver that only takes care of scaffolding and
> device binding.
> Further patches will add functionality to the driver.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>   MAINTAINERS                                        |  5 ++
>   drivers/mfd/cros_ec_dev.c                          | 13 ++++++
>   drivers/platform/chrome/Kconfig                    | 11 +++++
>   drivers/platform/chrome/Makefile                   |  1 +
>   drivers/platform/chrome/cros_ec_framework_laptop.c | 53 ++++++++++++++++++++++
>   5 files changed, 83 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c23fda1aa1f0..60699c289757 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4988,6 +4988,11 @@ S:	Maintained
>   F:	Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml
>   F:	sound/soc/codecs/cros_ec_codec.*
>   
> +CHROMEOS EC FRAMEWORK LAPTOP EXTENSIONS
> +M:	Thomas Weißschuh <linux@weissschuh.net>
> +S:	Maintained
> +F:	drivers/platform/chrome/cros_ec_framework_laptop.c
> +
>   CHROMEOS EC SUBDRIVERS
>   M:	Benson Leung <bleung@chromium.org>
>   R:	Guenter Roeck <groeck@chromium.org>
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index a52d59cc2b1e..0a36e77e5039 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -145,6 +145,10 @@ static const struct mfd_cell cros_ec_vbc_cells[] = {
>   	{ .name = "cros-ec-vbc", }
>   };
>   
> +static const struct mfd_cell cros_ec_framework_cells[] = {
> +	{ .name = "cros-ec-framework", }
> +};
> +
>   static void cros_ec_class_release(struct device *dev)
>   {
>   	kfree(to_cros_ec_dev(dev));
> @@ -299,6 +303,15 @@ static int ec_device_probe(struct platform_device *pdev)
>   				 retval);
>   	}
>   
> +	 /* The EC on Framework laptops implements some nonstandard features */

I don't think there is a spec really for cros_ec is there?  I think it 
will depend upon what features you're talking about if this is the right 
way to go.

The reason I say this is that maybe some of the same kinds of features 
will make sense for chromebooks that use cros_ec in the future and thus 
they should be "generic" cros_ec mfd cells to probe for in some way.

> +	if (dmi_match(DMI_SYS_VENDOR, "Framework")) {
> +		retval = mfd_add_hotplug_devices(ec->dev, cros_ec_framework_cells,
> +						 ARRAY_SIZE(cros_ec_framework_cells));
> +		if (retval)
> +			dev_warn(ec->dev, "failed to add framework laptop devices: %d\n",
> +				 retval);
> +	}
> +
>   	return 0;
>   
>   failed:
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 073616b5b5a0..ff69ee226606 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -239,6 +239,17 @@ config CROS_EC_TYPEC
>   	  To compile this driver as a module, choose M here: the module will be
>   	  called cros-ec-typec.
>   
> +config CROS_EC_FRAMEWORK_LAPTOP
> +	tristate "ChromeOS EC Framework Laptop extensions"
> +	depends on MFD_CROS_EC_DEV
> +	default MFD_CROS_EC_DEV
> +	help
> +	  If you say Y here, you get support for using Framework Laptop-specific extensions
> +	  of the Chrome OS EC.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called cros_ec_framework_laptop.
> +
>   config CROS_HPS_I2C
>   	tristate "ChromeOS HPS device"
>   	depends on HID && I2C && PM
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index 2dcc6ccc2302..ce6aac620086 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_CHROMEOS_PRIVACY_SCREEN)	+= chromeos_privacy_screen.o
>   obj-$(CONFIG_CHROMEOS_PSTORE)		+= chromeos_pstore.o
>   obj-$(CONFIG_CHROMEOS_TBMC)		+= chromeos_tbmc.o
>   obj-$(CONFIG_CROS_EC)			+= cros_ec.o
> +obj-$(CONFIG_CROS_EC_FRAMEWORK_LAPTOP)	+= cros_ec_framework_laptop.o
>   obj-$(CONFIG_CROS_EC_I2C)		+= cros_ec_i2c.o
>   obj-$(CONFIG_CROS_EC_ISHTP)		+= cros_ec_ishtp.o
>   obj-$(CONFIG_CROS_TYPEC_SWITCH)		+= cros_typec_switch.o
> diff --git a/drivers/platform/chrome/cros_ec_framework_laptop.c b/drivers/platform/chrome/cros_ec_framework_laptop.c
> new file mode 100644
> index 000000000000..8a8bf039fa9c
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_framework_laptop.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *  ChromesOS EC driver for Framework laptop
> + *
> + *  Copyright (C) 2024 Thomas Weißschuh <linux@weissschuh.net>
> + */
> +#include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/platform_device.h>
> +
> +#define DRV_NAME	"cros-ec-framework"
> +
> +struct cros_fwk_priv {
> +	struct cros_ec_device *cros_ec;
> +};
> +
> +static int cros_fwk_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
> +	struct cros_ec_device *cros_ec = ec_dev->ec_dev;
> +	struct cros_fwk_priv *priv;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->cros_ec = cros_ec;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	return 0;
> +}
> +
> +static const struct platform_device_id cros_fwk_id[] = {
> +	{ DRV_NAME, 0 },
> +	{ }
> +};
> +
> +static struct platform_driver cros_fwk_driver = {
> +	.driver.name	= DRV_NAME,
> +	.probe		= cros_fwk_probe,
> +	.id_table	= cros_fwk_id,
> +};
> +
> +module_platform_driver(cros_fwk_driver);
> +
> +MODULE_DEVICE_TABLE(platform, cros_fwk_id);
> +MODULE_DESCRIPTION("ChromeOS EC Framework Laptop extensions");
> +MODULE_AUTHOR("Thomas Weißschuh <linux@weissschuh.net");
> +MODULE_LICENSE("GPL");
>