Hi Guenter, On Mon, 19 Jul 2010 08:27:52 -0700, Guenter Roeck wrote: > On Mon, Jul 19, 2010 at 07:24:13AM -0400, Jean Delvare wrote: > > On Mon, 19 Jul 2010 00:44:02 -0300, Henrique de Moraes Holschuh wrote: > > > On Sun, 18 Jul 2010, Guenter Roeck wrote: > > > > 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, > > > > How? > > > One method would be to use rdmsr_safe_on_cpu() and try to read the register. > If the read fails, the register isn't there, and the sensor is not supported. If it is that easy, and reading a non-existent MSR can't cause any nasty side effect, then alright. > > > > and only create the sysfs > > > > entry if there is. Plus, make sure that it is only created once. > > > > As said above, this isn't trivial. If you just add the sysfs attribute > > to existing hwmon devices, you'll have duplicates. If you don't want > > duplicates, you'll get an asymmetry (an arbitrary hwmon entry will get > > the extra temperature, the user will get inevitably confused by this) > > and the hotplug code will be difficult to deal with. > > > > The most direct approach is probably to have a separate hwmon entry for > > the package temperature. But I start feeling dizzy when looking at > > these 8 coretemp hwmon entries I already have on my system, and adding > > a couple more doesn't exactly please me. So it may be the right time to > > start thinking of a different strategy - even though this comes with > > backwards compatibility pain. > > > Personally I would prefer to have one entry per CPU, with one sensor per core. > In other words, something like > > 1st CPU: > > coretemp-xxx-0000 > temp1_input core 0 > temp2_input core 1 > .. > tempN_input core N-1 > tempN+1_input package temperature, if the sensor exists > > 2nd CPU: > coretemp-xxx-0001 > > and so on. Yes, I think it would make a lot of sense. I was just not sure if we could know the value of N before seeing all the cores, but apparently there is a way to get that value, as /proc/cpuinfo has it (under "cpu cores"). So I would be perfectly fine with a patch implementing the above proposal. Preferably two patches, BTW: one moving core temperatures to the same hwmon entry, and a second one adding the package temperature. We will have to update libsensors 2 to prevent a regression here. What's the highest count of cores for these CPUs? 4? -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
