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
