Re: [PATCH 1/2] thermal: add hwmon sys I/F for thermal device

[Len Brown <lenb@kernel.org>]

On Monday 17 March 2008, Jean Delvare wrote:
> On Mon, 17 Mar 2008 13:55:00 +0100, Hans de Goede wrote:
> > Jean Delvare wrote:
> > > Hi Hans,
> > > 
> > > On Tue, 26 Feb 2008 09:39:42 +0100, Hans de Goede wrote:
> > >> Zhang, Rui wrote:
> > >>> Add hwmon sys I/F for the generic thermal device.
> > >>>
> > >> Great!
> > >>
> > >> But I have several remarks:
> > >> 1) Looking at the new code, you only add temp1_input, so I'm guessing that you
> > >> are registering a seperate hwmon class entry per zone? Please don't do that, 
> > >> please register one hwmon class entry, and add multiple temp#_input attr to it 
> > >> (and the same for crit).
> > > 
> > > I am sorry that I did not notice when you suggested this. I disagree,
> > > but now Rui's code is upstream so I guess it's too late to complain.
> > > Still here are my reasons:
> > > 
> > > One of the great things about libsensors is that it gives unique names
> > > to hardware monitoring devices, and for each device, each feature has a
> > > unique name as well. This makes it possible to ignore or label a
> > > specific feature in /etc/sensors.conf in a way that is stable over
> > > reboot and addition of new hardware.
> > > 
> > > By going with a single virtual device for all thermal zones, you break
> > > this model. Depending on which thermal zone drivers are loaded and in
> > > which order they are loaded, there will be more or less temp* files in
> > > the hwmon directory and you also can't predict their order. The
> > > labelling issue could be solved by adding temp*_label files, but this
> > > still prevents the user from overriding a label. And there's no way to
> > > reliably ignore a specific thermal zone or to ask for information about
> > > a specific thermal zone with the current model.
> > > 
> > > For this reason, I think it would be much better to have one hwmon
> > > class device for each _type_ of thermal zone. For example, all ACPI
> > > thermal zones would be listed as one hwmon class device. If we later
> > > add support for another type of thermal zones, all thermal zones of
> > > this type would be listed as one (different) hwmon class device. This
> > > makes each specific thermal zone driver responsible for the stability
> > > of the numbering of the various thermal zones of a given type. This
> > > would also let us give names to the different thermal zone types (e.g.
> > > "acpitz" for ACPI thermal zones) so that the users and supporters have
> > > an idea who is providing these temperature values and limits.
> > > 
> > 
> > I fully agree, I didn't know there was a generic thermal zone model, and that 
> > there could be multiple drivers implementing it (let alone multiple thermal 
> > zone drivers active for one system ??) I thought this was all ACPI only,
> 
> My understanding is that the new thermal zone code is meant to be
> generic and ACPI is only one possible underlying implementations, which
> sounds very good to me. ACPI just happens to be the only implementation
> at the time being.

Your understanding is correct.
While ACPI is currently the only customer of the generic thermal I/F,
there is a future platform in the pipeline that has no ACPI
and it too will use the generic thermal I/F.

> > If this (multiple thermal zone drivers active for one system) can really happen 
> > then we really should fix it so that there is one hwmon class entry per thermal 
> > zone driver. This can be done without changing the ABI, as things would still 
> > follow the standard hwmon ABI.
> 
> I have to admit that I'm not sure what sense that would make (and how
> safe it would be) to have more than one type of thermal zones active at
> the same time. If the idea if that only one type of thermal zones can
> exist for a given system (i.e. the selection happens at build time),
> then my objection can safely be ignored, except for the fact that I
> still would like the "name" attribute to reflect the type of the
> thermal zones.
> 
> Rui, Len, how did you originally envision the coexistence (or not) of
> different types of thermal zones?

Right now we seem to be ACPI and then non-ACPI on different systems.
However, I don't see any reason that both can't be on the same system.

-Len