Chrome platform driver development
 help / color / mirror / Atom feed
* [PATCH v2 0/2] mfd: platform/chrome: Catch EC feature read errors during probe
@ 2026-06-08 21:15 Andrei Kuchynski
  2026-06-08 21:15 ` [PATCH v2 1/2] platform/chrome: cros_ec_proto: Introduce cros_ec_read_features helper Andrei Kuchynski
  2026-06-08 21:15 ` [PATCH v2 2/2] mfd: cros_ec: Read EC features during probe to catch transfer error Andrei Kuchynski
  0 siblings, 2 replies; 6+ messages in thread
From: Andrei Kuchynski @ 2026-06-08 21:15 UTC (permalink / raw)
  To: Lee Jones, Benson Leung, Tzung-Bi Shih
  Cc: Guenter Roeck, Gwendal Grignou, chrome-platform, linux-kernel,
	Andrei Kuchynski

This series fixes a sysfs duplicate name collision that occurs when the
Fingerprint device fails to respond during probe, causing it to
incorrectly fall back to the generic 'cros_ec' name.

Changes in v2:
- Split into a 2-patch series.
- Carried over Acked-by from Tzung-Bi Shih for Patch 1.
- Fixed an ec->class_dev resource leak by utilizing 'goto failed;'.
- Moved class_dev initialization to prevent a missing release()
  callback warning on the error path.

Andrei Kuchynski (2):
  platform/chrome: cros_ec_proto: Introduce cros_ec_read_features helper
  mfd: cros_ec: Read EC features during probe to catch transfer error

 drivers/mfd/cros_ec_dev.c                   | 18 ++++++++-----
 drivers/platform/chrome/cros_ec_proto.c     | 30 +++++++++++++++------
 include/linux/platform_data/cros_ec_proto.h |  2 ++
 3 files changed, 35 insertions(+), 15 deletions(-)

-- 
2.54.0.1032.g2f8565e1d1-goog


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] platform/chrome: cros_ec_proto: Introduce cros_ec_read_features helper
  2026-06-08 21:15 [PATCH v2 0/2] mfd: platform/chrome: Catch EC feature read errors during probe Andrei Kuchynski
@ 2026-06-08 21:15 ` Andrei Kuchynski
  2026-06-08 21:15 ` [PATCH v2 2/2] mfd: cros_ec: Read EC features during probe to catch transfer error Andrei Kuchynski
  1 sibling, 0 replies; 6+ messages in thread
From: Andrei Kuchynski @ 2026-06-08 21:15 UTC (permalink / raw)
  To: Lee Jones, Benson Leung, Tzung-Bi Shih
  Cc: Guenter Roeck, Gwendal Grignou, chrome-platform, linux-kernel,
	Andrei Kuchynski

Extract the EC feature-reading logic from cros_ec_check_features() into
cros_ec_read_features() helper function.

Currently, cros_ec_check_features() swallows command transfer errors. By
isolating the transaction logic into an explicit helper that returns the
actual transfer error code, subsequent callers (such as the cros_ec_dev
driver during device probing) can catch a read error.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
Acked-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/cros_ec_proto.c     | 30 +++++++++++++++------
 include/linux/platform_data/cros_ec_proto.h |  2 ++
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 1d8d9168ec1aa..724d1313f6b21 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -946,6 +946,27 @@ u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev)
 }
 EXPORT_SYMBOL(cros_ec_get_host_event);
 
+/**
+ * cros_ec_read_features() - Read EC features
+ *
+ * @ec: EC device.
+ *
+ * Return: >= 0 on success, negative error number on failure.
+ */
+int cros_ec_read_features(struct cros_ec_dev *ec)
+{
+	int ret = cros_ec_cmd(ec->ec_dev, 0, EC_CMD_GET_FEATURES + ec->cmd_offset,
+			      NULL, 0, &ec->features, sizeof(ec->features));
+
+	if (ret < 0) {
+		dev_warn(ec->dev, "cannot get EC features: %d\n", ret);
+		memset(&ec->features, 0, sizeof(ec->features));
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cros_ec_read_features);
+
 /**
  * cros_ec_check_features() - Test for the presence of EC features
  *
@@ -960,17 +981,10 @@ EXPORT_SYMBOL(cros_ec_get_host_event);
 bool cros_ec_check_features(struct cros_ec_dev *ec, int feature)
 {
 	struct ec_response_get_features *features = &ec->features;
-	int ret;
 
 	if (features->flags[0] == -1U && features->flags[1] == -1U) {
 		/* features bitmap not read yet */
-		ret = cros_ec_cmd(ec->ec_dev, 0, EC_CMD_GET_FEATURES + ec->cmd_offset,
-				  NULL, 0, features, sizeof(*features));
-		if (ret < 0) {
-			dev_warn(ec->dev, "cannot get EC features: %d\n", ret);
-			memset(features, 0, sizeof(*features));
-		}
-
+		cros_ec_read_features(ec);
 		dev_dbg(ec->dev, "EC features %08x %08x\n",
 			features->flags[0], features->flags[1]);
 	}
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index 6ed1c4c5ce2ef..a1ccecf5e1f83 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -271,6 +271,8 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev,
 
 u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
 
+int cros_ec_read_features(struct cros_ec_dev *ec);
+
 bool cros_ec_check_features(struct cros_ec_dev *ec, int feature);
 
 int cros_ec_get_sensor_count(struct cros_ec_dev *ec);
-- 
2.54.0.1032.g2f8565e1d1-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] mfd: cros_ec: Read EC features during probe to catch transfer error
  2026-06-08 21:15 [PATCH v2 0/2] mfd: platform/chrome: Catch EC feature read errors during probe Andrei Kuchynski
  2026-06-08 21:15 ` [PATCH v2 1/2] platform/chrome: cros_ec_proto: Introduce cros_ec_read_features helper Andrei Kuchynski
@ 2026-06-08 21:15 ` Andrei Kuchynski
  2026-06-18  8:40   ` Lee Jones
  1 sibling, 1 reply; 6+ messages in thread
From: Andrei Kuchynski @ 2026-06-08 21:15 UTC (permalink / raw)
  To: Lee Jones, Benson Leung, Tzung-Bi Shih
  Cc: Guenter Roeck, Gwendal Grignou, chrome-platform, linux-kernel,
	Andrei Kuchynski

cros_ec_check_features() does not return an error if the underlying
EC_CMD_GET_FEATURES command fails. Consequently, when the Fingerprint
device fails to respond, the probe function ignores the failure and falls
back to installing it as 'cros_ec' device instead of 'cros_fp'.
This leads to a sysfs duplicate filename collision later when the real
'cros_ec' device attempts to register:

  cros-ec-spi spi5.0: EC failed to respond in time
  cros-ec-dev.19.auto: cannot get EC features: -110
  sysfs : cannot create duplicate filename '/class/chromeos/cros_ec'
        : sysfs_do_create_link_sd+0x94/0xdc
        : ec_device_probe+0x150/0x4f0

Fix this by explicitly calling the newly introduced cros_ec_read_features()
function. If the transfer fails, abort the broken device initialization.
Move the initialization of class_dev before this call to prevent a missing
release() callback warning on the error path.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 drivers/mfd/cros_ec_dev.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 39430dd44e30c..a8c6300248d59 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -203,6 +203,17 @@ static int ec_device_probe(struct platform_device *pdev)
 	ec->features.flags[1] = -1U; /* Not cached yet */
 	device_initialize(&ec->class_dev);
 
+	/*
+	 * Add the class device
+	 */
+	ec->class_dev.class = &cros_class;
+	ec->class_dev.parent = dev;
+	ec->class_dev.release = cros_ec_class_release;
+
+	retval = cros_ec_read_features(ec);
+	if (retval < 0)
+		goto failed;
+
 	for (i = 0; i < ARRAY_SIZE(cros_mcu_devices); i++) {
 		/*
 		 * Check whether this is actually a dedicated MCU rather
@@ -220,13 +231,6 @@ static int ec_device_probe(struct platform_device *pdev)
 		}
 	}
 
-	/*
-	 * Add the class device
-	 */
-	ec->class_dev.class = &cros_class;
-	ec->class_dev.parent = dev;
-	ec->class_dev.release = cros_ec_class_release;
-
 	retval = dev_set_name(&ec->class_dev, "%s", ec_platform->ec_name);
 	if (retval) {
 		dev_err(dev, "dev_set_name failed => %d\n", retval);
-- 
2.54.0.1032.g2f8565e1d1-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] mfd: cros_ec: Read EC features during probe to catch transfer error
  2026-06-08 21:15 ` [PATCH v2 2/2] mfd: cros_ec: Read EC features during probe to catch transfer error Andrei Kuchynski
@ 2026-06-18  8:40   ` Lee Jones
  2026-06-18  9:31     ` Tzung-Bi Shih
  0 siblings, 1 reply; 6+ messages in thread
From: Lee Jones @ 2026-06-18  8:40 UTC (permalink / raw)
  To: Andrei Kuchynski
  Cc: Benson Leung, Tzung-Bi Shih, Guenter Roeck, Gwendal Grignou,
	chrome-platform, linux-kernel

On Mon, 08 Jun 2026, Andrei Kuchynski wrote:

> cros_ec_check_features() does not return an error if the underlying
> EC_CMD_GET_FEATURES command fails. Consequently, when the Fingerprint
> device fails to respond, the probe function ignores the failure and falls
> back to installing it as 'cros_ec' device instead of 'cros_fp'.
> This leads to a sysfs duplicate filename collision later when the real
> 'cros_ec' device attempts to register:
> 
>   cros-ec-spi spi5.0: EC failed to respond in time
>   cros-ec-dev.19.auto: cannot get EC features: -110
>   sysfs : cannot create duplicate filename '/class/chromeos/cros_ec'
>         : sysfs_do_create_link_sd+0x94/0xdc
>         : ec_device_probe+0x150/0x4f0
> 
> Fix this by explicitly calling the newly introduced cros_ec_read_features()
> function. If the transfer fails, abort the broken device initialization.
> Move the initialization of class_dev before this call to prevent a missing
> release() callback warning on the error path.
> 
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> ---
>  drivers/mfd/cros_ec_dev.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index 39430dd44e30c..a8c6300248d59 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -203,6 +203,17 @@ static int ec_device_probe(struct platform_device *pdev)
>  	ec->features.flags[1] = -1U; /* Not cached yet */
>  	device_initialize(&ec->class_dev);
>  
> +	/*
> +	 * Add the class device
> +	 */
> +	ec->class_dev.class = &cros_class;
> +	ec->class_dev.parent = dev;
> +	ec->class_dev.release = cros_ec_class_release;
> +
> +	retval = cros_ec_read_features(ec);

This depends on the other patch.

Once it's been Reviewed / Acked, I'd be happy to take both and submit an IB.

> +	if (retval < 0)
> +		goto failed;
> +
>  	for (i = 0; i < ARRAY_SIZE(cros_mcu_devices); i++) {
>  		/*
>  		 * Check whether this is actually a dedicated MCU rather
> @@ -220,13 +231,6 @@ static int ec_device_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	/*
> -	 * Add the class device
> -	 */
> -	ec->class_dev.class = &cros_class;
> -	ec->class_dev.parent = dev;
> -	ec->class_dev.release = cros_ec_class_release;
> -
>  	retval = dev_set_name(&ec->class_dev, "%s", ec_platform->ec_name);
>  	if (retval) {
>  		dev_err(dev, "dev_set_name failed => %d\n", retval);
> -- 
> 2.54.0.1032.g2f8565e1d1-goog
> 

-- 
Lee Jones

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] mfd: cros_ec: Read EC features during probe to catch transfer error
  2026-06-18  8:40   ` Lee Jones
@ 2026-06-18  9:31     ` Tzung-Bi Shih
  2026-06-18 11:21       ` Lee Jones
  0 siblings, 1 reply; 6+ messages in thread
From: Tzung-Bi Shih @ 2026-06-18  9:31 UTC (permalink / raw)
  To: Lee Jones
  Cc: Andrei Kuchynski, Benson Leung, Guenter Roeck, Gwendal Grignou,
	chrome-platform, linux-kernel

On Thu, Jun 18, 2026 at 09:40:37AM +0100, Lee Jones wrote:
> On Mon, 08 Jun 2026, Andrei Kuchynski wrote:
> 
> > cros_ec_check_features() does not return an error if the underlying
> > EC_CMD_GET_FEATURES command fails. Consequently, when the Fingerprint
> > device fails to respond, the probe function ignores the failure and falls
> > back to installing it as 'cros_ec' device instead of 'cros_fp'.
> > This leads to a sysfs duplicate filename collision later when the real
> > 'cros_ec' device attempts to register:
> > 
> >   cros-ec-spi spi5.0: EC failed to respond in time
> >   cros-ec-dev.19.auto: cannot get EC features: -110
> >   sysfs : cannot create duplicate filename '/class/chromeos/cros_ec'
> >         : sysfs_do_create_link_sd+0x94/0xdc
> >         : ec_device_probe+0x150/0x4f0
> > 
> > Fix this by explicitly calling the newly introduced cros_ec_read_features()
> > function. If the transfer fails, abort the broken device initialization.
> > Move the initialization of class_dev before this call to prevent a missing
> > release() callback warning on the error path.
> > 
> > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > ---
> >  drivers/mfd/cros_ec_dev.c | 18 +++++++++++-------
> >  1 file changed, 11 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> > index 39430dd44e30c..a8c6300248d59 100644
> > --- a/drivers/mfd/cros_ec_dev.c
> > +++ b/drivers/mfd/cros_ec_dev.c
> > @@ -203,6 +203,17 @@ static int ec_device_probe(struct platform_device *pdev)
> >  	ec->features.flags[1] = -1U; /* Not cached yet */
> >  	device_initialize(&ec->class_dev);
> >  
> > +	/*
> > +	 * Add the class device
> > +	 */
> > +	ec->class_dev.class = &cros_class;
> > +	ec->class_dev.parent = dev;
> > +	ec->class_dev.release = cros_ec_class_release;
> > +
> > +	retval = cros_ec_read_features(ec);
> 
> This depends on the other patch.
> 
> Once it's been Reviewed / Acked, I'd be happy to take both and submit an IB.

FWIW: I gave my A-b tag in v1 and it has been carried to [PATCH v2 1/2]
platform/chrome: cros_ec_proto: Introduce cros_ec_read_features helper.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] mfd: cros_ec: Read EC features during probe to catch transfer error
  2026-06-18  9:31     ` Tzung-Bi Shih
@ 2026-06-18 11:21       ` Lee Jones
  0 siblings, 0 replies; 6+ messages in thread
From: Lee Jones @ 2026-06-18 11:21 UTC (permalink / raw)
  To: Tzung-Bi Shih
  Cc: Andrei Kuchynski, Benson Leung, Guenter Roeck, Gwendal Grignou,
	chrome-platform, linux-kernel

On Thu, 18 Jun 2026, Tzung-Bi Shih wrote:

> On Thu, Jun 18, 2026 at 09:40:37AM +0100, Lee Jones wrote:
> > On Mon, 08 Jun 2026, Andrei Kuchynski wrote:
> > 
> > > cros_ec_check_features() does not return an error if the underlying
> > > EC_CMD_GET_FEATURES command fails. Consequently, when the Fingerprint
> > > device fails to respond, the probe function ignores the failure and falls
> > > back to installing it as 'cros_ec' device instead of 'cros_fp'.
> > > This leads to a sysfs duplicate filename collision later when the real
> > > 'cros_ec' device attempts to register:
> > > 
> > >   cros-ec-spi spi5.0: EC failed to respond in time
> > >   cros-ec-dev.19.auto: cannot get EC features: -110
> > >   sysfs : cannot create duplicate filename '/class/chromeos/cros_ec'
> > >         : sysfs_do_create_link_sd+0x94/0xdc
> > >         : ec_device_probe+0x150/0x4f0
> > > 
> > > Fix this by explicitly calling the newly introduced cros_ec_read_features()
> > > function. If the transfer fails, abort the broken device initialization.
> > > Move the initialization of class_dev before this call to prevent a missing
> > > release() callback warning on the error path.
> > > 
> > > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > > ---
> > >  drivers/mfd/cros_ec_dev.c | 18 +++++++++++-------
> > >  1 file changed, 11 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> > > index 39430dd44e30c..a8c6300248d59 100644
> > > --- a/drivers/mfd/cros_ec_dev.c
> > > +++ b/drivers/mfd/cros_ec_dev.c
> > > @@ -203,6 +203,17 @@ static int ec_device_probe(struct platform_device *pdev)
> > >  	ec->features.flags[1] = -1U; /* Not cached yet */
> > >  	device_initialize(&ec->class_dev);
> > >  
> > > +	/*
> > > +	 * Add the class device
> > > +	 */
> > > +	ec->class_dev.class = &cros_class;
> > > +	ec->class_dev.parent = dev;
> > > +	ec->class_dev.release = cros_ec_class_release;
> > > +
> > > +	retval = cros_ec_read_features(ec);
> > 
> > This depends on the other patch.
> > 
> > Once it's been Reviewed / Acked, I'd be happy to take both and submit an IB.
> 
> FWIW: I gave my A-b tag in v1 and it has been carried to [PATCH v2 1/2]
> platform/chrome: cros_ec_proto: Introduce cros_ec_read_features helper.

I see it.  Thank you.

Note to self: Take the set after -rc1

-- 
Lee Jones

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-18 11:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-08 21:15 [PATCH v2 0/2] mfd: platform/chrome: Catch EC feature read errors during probe Andrei Kuchynski
2026-06-08 21:15 ` [PATCH v2 1/2] platform/chrome: cros_ec_proto: Introduce cros_ec_read_features helper Andrei Kuchynski
2026-06-08 21:15 ` [PATCH v2 2/2] mfd: cros_ec: Read EC features during probe to catch transfer error Andrei Kuchynski
2026-06-18  8:40   ` Lee Jones
2026-06-18  9:31     ` Tzung-Bi Shih
2026-06-18 11:21       ` Lee Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox