On Sun, Jul 18, 2010 at 07:30:27AM -0400, Henrique de Moraes Holschuh wrote: > On Thu, 15 Jul 2010, Chen Gong wrote: > > >>+pkg_temp[1-*]_input > > >>+ Package temperature input value. > > >>+ Unit: millidegree Celsius > > >>+ RO > > >>+ > > > > > >Unless I am missing something, this is just another temperature sensor. > > >You should just name it temp2_input; there should be no need to change the API. > > Indeed. pkg_temp* violates the hwmon sysfs ABI. > > > I'm not very sure what you mean. Yes, it does be another sensor, but > > it only can be accessed by MSR_IA32_PACKAGE_THERM_STATUS, not > > original register MSR_IA32_THERM_STATUS. IMO, temp2_input should > > mean another same MSR based on per CPU, but apparently, here it's > > not. Please fix me. > > You can use temp# for the packages without causing trouble. You just > need to provide labels so that userspace can know what it is. Please do > think *hard* on how you will name the sensors, because it will become a > stable ABI you cannot change later. > > Obviously, you will have to document the labels, and it is probably a > good idea to make it clear on the documentation that userspace needs to > look at the labels to know what kind of sensor it is. > All correct. I have a second concern after looking through the IA-32 documentation. The package sensor is not supported for all chips, and there is only one sensor per package (not per core). Yet, the code as written adds the new sensor unconditionally. I think this needs to change to only create the sysfs file if the sensor is supported by the CPU, and only once per core. Guenter _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
