Message-ID: <20130317133920.465e8a8c@endymion.delvare>
Date:	Sun, 17 Mar 2013 13:39:20 +0100
From:	Jean Delvare <khali@...ux-fr.org>
To:	Guenter Roeck <linux@...ck-us.net>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, lm-sensors@...sensors.org
Subject: Re: [lm-sensors] [RFC PATCH 0/2] fs: sysfs: Add devres support

On Sat, 16 Mar 2013 14:25:40 -0700, Guenter Roeck wrote:
> On Sat, Mar 16, 2013 at 12:50:02PM -0700, Greg Kroah-Hartman wrote:
> > On Sat, Mar 16, 2013 at 11:12:53AM -0700, Guenter Roeck wrote:
> > > My use case is primarily for hwmon drivers.
> > > 
> > > hwmon has a separate API call to register a driver with the hwmon subsystem,
> > > which creates a separate hwmon device and provides the user space notification.
> > > As the created attribute files are often conditional on device variant and device
> > > configuration, I don't see how this could be done through a default attribute
> > > list (even though it might be worthwhile exploring if it can be used for some
> > > of the simpler drivers).
> > 
> > The default attribute list functionality offers you the ability to have
> > callbacks to your driver to validate if you really want this sysfs file
> > to be created or not.  Just use that like other subsystems do, then you
> > will never have to be making these create and remove calls at all.
> 
> I thought about it, but right now I have no idea how to make it work.
> Initialization sequence in hwmon drivers is
>     probe():
>         allocate and initialize local driver data structures
>     	detect configuration and initialize hardware if necessary
> 	create attribute files
> 	register with hwmon subsystem
> 	sometimes do additional work, such as enable interrupts
> 
> If I use attribute_group of the device_driver structure to create the attribute
> files, my understanding is that those would be created prior to calling
> the probe function.
> 
> This would be too early, since local data structures do not yet exist, and
> the chip configuration is unknown and uninitialized.
> 
> On the other side, attribute files must exist before hwmon_device_register()
> is called, since otherwise userspace would get confused.

I'd like to add something at this point.

We have historically created the hwmon attributes in the hardware (i2c,
platform...) device, and then created an empty hwmon class device on
top of it so that libsensors etc. can locate all hardware monitoring
chips on the system. This is probably wrong and this may explain the
difference of views between Greg and Guenter.

I suspect that ideally all hwmon-related attributes should belong to the
hwmon-class device and not the physical device. Would doing so solve
the problem of is_visible() needing chip-specific information that can
only be gathered during probe()? Sure this is an interface change, but
a few hwmon drivers already do it that way (the ones without an actual
hardware device, e.g. ACPI thermal zones) and libsensors supports this
since version 3.0.3, which was released in September 2008 - 4.5 years
ago.

This would require creating the attributes after calling
hwmon_device_register() rather than before, but from the ongoing
discussion I seem to understand that the driver core supports creating
the attributes for us, possibly at the same time as the class device
will be created. Would this solve the userspace timing issue?

Also note that libsensors is really old fashioned when it comes to
device discovery. It doesn't support hot-plug nor hot-remove. So some
work would be needed in this area anyway if we want libsensors-based
applications to properly cope with device addition and removal.

Just my 2 cents.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists