Re: [PATCH] mfd: cros_ec: Read EC features during probe to catch transfer error

On Thu, Jun 4, 2026 at 5:57 PM Lee Jones <[email protected]> wrote:
>
> On Fri, 22 May 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 extracting the feature reading logic into a new helper function
> > cros_ec_read_features() and calling it during ec_device_probe().
> > If the transfer fails, abort the broken device initialization.
>
> You're doing 2 things here.  Move the function first, then add the new
> call into MFD in a subsequent patch.
>

I will split this into 2-patch series for v2:
 - Patch 1 introduces the helper function in platform/chrome.
 - Patch 2 consumes the helper in the MFD driver to resolve the bug.

> > Signed-off-by: Andrei Kuchynski <[email protected]>
> > ---
> >  drivers/mfd/cros_ec_dev.c                   |  4 +++
> >  drivers/platform/chrome/cros_ec_proto.c     | 30 +++++++++++++++------
> >  include/linux/platform_data/cros_ec_proto.h |  2 ++
> >  3 files changed, 28 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> > index 39430dd44e30c..7810b1c871849 100644
> > --- a/drivers/mfd/cros_ec_dev.c
> > +++ b/drivers/mfd/cros_ec_dev.c
> > @@ -203,6 +203,10 @@ static int ec_device_probe(struct platform_device *pdev)
> >       ec->features.flags[1] = -1U; /* Not cached yet */
> >       device_initialize(&ec->class_dev);
> >
> > +     retval = cros_ec_read_features(ec);
> > +     if (retval < 0)
> > +             return retval;
>
> You just leaked ec->class_dev.
>
>         goto failed;  ?
>

You are completely right.
I will also change the initialization sequence so that the `class_dev`
fields (class, parent, and release callback) are fully assigned before
this to prevent the warning:

  Device '(null)' does not have a release() function, it is broken and
      must be fixed. See Documentation/core-api/kobject.rst.
  WARNING: CPU: 6 PID: 132 at drivers/base/core.c:2569
      device_release+0x9c/0xb0

Thanks,
Andrei