Re: [PATCH v3 4/8] platform/chrome: Disallow sending commands through unregistered ec_dev

[Tzung-Bi Shih <tzungbi@kernel.org>]

On Mon, Jul 21, 2025 at 07:47:47AM +0200, Greg KH wrote:
> On Mon, Jul 21, 2025 at 04:44:52AM +0000, Tzung-Bi Shih wrote:
> > Return earlier if attempting to send commands through an unregistered
> > struct cros_ec_device.
> 
> Why would it ever be "uregistered"?

If the EC rebooted while a userland program opened a file of the
chardev in cros_ec_chardev, the struct cros_ec_device [1][2] may be
unregistered due to some drivers re-probe the device.

The issue mainly comes from the discussion of cros_ec_usb [3]: using a
`registered_list` vs. calling cros_ec_{un,}register() everytime in
usb_driver's .probe()/.disconnect().

[1] https://elixir.bootlin.com/linux/v6.15/source/drivers/platform/chrome/cros_ec_chardev.c#L241
[2] https://elixir.bootlin.com/linux/v6.15/source/drivers/platform/chrome/cros_ec_chardev.c#L71
[3] https://lore.kernel.org/chrome-platform/20250624110028.409318-1-dawidn@google.com/T/#u

> > +/**
> > + * cros_ec_device_registered - Check if the ec_dev is registered.
> > + *
> > + * @ec_dev: EC device
> > + */
> > +bool cros_ec_device_registered(struct cros_ec_device *ec_dev)
> > +{
> > +	return atomic_read(&ec_dev->registered) == 1;
> > +}
> > +EXPORT_SYMBOL_GPL(cros_ec_device_registered);
> 
> This isn't going to do what you think it does :(
> 
> Hint, the state can change right after you call this, making it
> pointless.

In current use cases, once it turns from "registered" to "unregistered", it
should never back to "registered".  The flag is more or less to let the
kernel know the struct cros_ec_device is stale.  The object is still valid
for accessing because an opening file instance is referencing.