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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2008-03-17 at 20:37 +0800, 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.
Interesting.

> 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.
Yes, you're right.

> The
> labelling issue could be solved by adding temp*_label files, but this
> still prevents the user from overriding a label.
why user needs to override the 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.
Well. I disagree.
First, the generic thermal driver only checks the callbacks and builds
the sys I/F for each thermal zone registered. So there is no difference
between different thermal zones. Checking the type of different thermal
zone and register a hwmon device for each _type of thermal zones doesn't
make sense to me. :(
Second, take ACPI thermal zone for example, the number/order of the ACPI
thermal zones can not be predicted either. All of this information is
gotten during system boot time.
 
>  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.
If we really need to do this, I suggest we do this in the native driver.
Say acpi thermal driver is responsible for registering a hwmon device
for ACPI thermal zones, just like the other hwmon native sensor drivers.
what do you think?

thanks,
rui
> 
> --
> Jean Delvare
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi"
> in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux