On Sun, 18 Jul 2010, Guenter Roeck wrote: > On Sun, Jul 18, 2010 at 10:22:14PM -0400, Chen Gong wrote: > > 7/18/2010 10:10 PM, Henrique de Moraes Holschuh > > > On Sun, 18 Jul 2010, Guenter Roeck wrote: > > >> 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. > > > > > > It is a really good idea to never expose through sysfs something that > > > isn't there. Alternatively, you can return errors like ENXIO, but you > > > should only do that when you have no other choice (because, e.g. you > > > cannot know beforehand whether a sensor will be available or not). > > > > > > If one can detect when processor packages and cores come and go, and one > > > can ask the package and cores about the sensors that are really > > > available, one can expose to sysfs just the sensors that are > > > operational, and remove or add sensors when needed. > > > > > > > Grrr, It looks like a separate driver for package thermal sensor is a > > good idea ? > > Why ? Just make the sysfs entry conditional. All you need to do is to detect > in the probe function if there is a package sensor, and only create the sysfs > entry if there is. Plus, make sure that it is only created once. And make sure to recompute it when CPUs are hotadded or hotremoved, I suppose. Maybe it doesn't need to do it right away (I don't even know if coretemp already does it), but it will have to sooner or later... -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
