diff mbox series

[1/2] platform/chrome: cros_ec_framework_laptop: introduce driver

Message ID 20240505-cros_ec-framework-v1-1-402662d6276b@weissschuh.net
State New
Headers show
Series platform/chrome: cros_ec_framework_laptop: new driver | expand

Commit Message

Thomas Weißschuh May 5, 2024, 8:56 p.m. UTC
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(+)

Comments

Mario Limonciello May 6, 2024, 1:10 p.m. UTC | #1
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");
>
Thomas Weißschuh May 6, 2024, 5:48 p.m. UTC | #2
On 2024-05-06 08:10:58+0000, Limonciello, Mario wrote:
> 
> 
> 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.

I equate "standard" with "mainline".

> 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.

The commands identifiers are just numbers.
So if multiple vendors use the same numbers there could be collisions.
By loading the subdriver only for the correct vendors devices we avoid those.

If other vendors or mainline CrOS EC implements the same commands we can
revisit this and move the functionality to a generic mfd cell.

But after the discussions on the cover letter, let's first see if a
vendor-specific driver is needed at all.

Thomas
Lee Jones May 7, 2024, 8:25 a.m. UTC | #3
On Sun, 05 May 2024, 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 ++++++

I do not see any build-time deps here.

Please split this change out.

>  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 mbox series

Patch

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 */
+	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");