Re: [PATCH] sysfs: add devm_sysfs_create_group() and friends

Greg Kroah-Hartman
2017-06-18T00:25:10Z

On Wed, Jun 07, 2017 at 10:12:08AM -0700, Dmitry Torokhov wrote:
> On Fri, Nov 06, 2015 at 11:23:37PM -0800, Dmitry Torokhov wrote:
> > On Fri, Nov 06, 2015 at 04:24:07PM -0800, Greg Kroah-Hartman wrote:
> > > On Fri, Oct 30, 2015 at 08:13:11AM -0700, Dmitry Torokhov wrote:
> > > > On Fri, Oct 30, 2015 at 07:40:37AM -0700, Greg Kroah-Hartman wrote:
> > > > > On Fri, Oct 30, 2015 at 04:47:06AM -0700, Dmitry Torokhov wrote:
> > > > > > Many drivers create additional driver-specific device attributes when
> > > > > > binding to the device and providing managed version of sysfs_create_group()
> > > > > > will simplify unbinding and error handling in probe path for such drivers.
> > > > >
> > > > > But they really shouldn't, because if they do this, they have raced
> > > > > userspace and tools don't know that the files are present.
> > > > >
> > > > > I don't want to encourage drivers to do this at all, so I don't want to
> > > > > make it easier for them to do things incorrectly.
> > > >
> > > > The solution is not to forbid drivers from establishing attributes but
> > > > rather notify userspace when device is fully bound to the driver. Then
> > > > userspace that actually cares about these attributes will listen to
> > > > proper events.
> > > >
> > > > We can either do KOBJ_BOUND/KOBJ_UNBOUND or reuse
> > > > KOBJ_ONLINE/KOBJ_OFFLINE. I'd prefer the former (adding new events).
> > >
> > > bound/unbound makes a bit more sense, maybe that would work, haven't
> > > thought that much about it. Given that no one in the "real world" seem
> > > to notice the race condition, that means that people aren't really using
> > > tools like libudev to read sysfs attributes, so maybe no one even cares
> > > about the contents of them :)
> >
> > Could be, or they have workarounds for this scenario.
>
> So I have another concrete use-case for this feature: firmware updating.
>
> Several touch controllers may come up in either normal (application) or
> boot mode, depending on whether firmware/configuration is corrupted or
> not when they are powered on. In boot mode we do not create input
> device instance (because we do not necessarily know the characteristics
> of the input device in question).
>
> I would like to have firmware update scripts to be fired via udev. The
> scripts would check the current firmware version and state of the device
> and decide whether they need to update firmware or not. To do that they
> need to have driver bound to the physical device (i2c or spi), and
> therefore I can't rely on ADD events as those happen too early. Having
> the BOUND/UNBOUND events will allow me to do this cleanly.

Yeah, this is what convinced me we need this. The act of
binding/unbinding a driver to a device is a state change for the device,
and userspace should learn about it.

> I will look into using this mechanism on chrome OS.

If you resend the patches, I can work on adding support for it in
libudev to see how that works out as well.

thanks,

greg k-h