On Tue, Jan 18, 2011 at 05:35:13PM -0500, Jean Delvare wrote:
> On Tue, 18 Jan 2011 17:32:46 +0100, Jean Delvare wrote:
> > On Tue, 18 Jan 2011 10:03:23 -0600, Nick Hall wrote:
> > > And regarding the sensors-detect script, I understand now. If anyone is
> > > interested, my Sandy Bridge processor would be model "0x2A" according to how
> > > that script does things.
> >
> > This is an option, yes, but it would be better if we could implement
> > the same detection logic as the coretemp driver has. For one thing, this
> > would guarantee that the two are always in sync. For another, it would
> > lower the maintenance effort from our side, as the new detection logic
> > is universal and doesn't need to be updated with every new CPU model.
> > I'm looking into it but this is non-trivial.
>
> I have come up with the following patch:
>
> Index: prog/detect/sensors-detect
> ===================================================================
> --- prog/detect/sensors-detect (révision 5904)
> +++ prog/detect/sensors-detect (copie de travail)
> @@ -23,7 +23,7 @@
> require 5.004;
>
> use strict;
> -use Fcntl;
> +use Fcntl qw(:DEFAULT :seek);
> use File::Basename;
>
> # We will call modprobe, which typically lives in either /sbin,
> @@ -2046,14 +2046,10 @@
> driver => "k10temp",
> detect => \&fam11h_pci_detect,
> }, {
> - name => "Intel Core family thermal sensor",
> + name => "Intel digital thermal sensor",
> driver => "coretemp",
> - detect => sub { coretemp_detect(0); },
> + detect => \&coretemp_detect,
> }, {
> - name => "Intel Atom thermal sensor",
> - driver => "coretemp",
> - detect => sub { coretemp_detect(1); },
> - }, {
> name => "Intel AMB FB-DIMM thermal sensor",
> driver => "i5k_amb",
> detect => \&intel_amb_detect,
> @@ -2314,10 +2310,10 @@
> while (<INPUTFILE>) {
> if (m/^processor\s*:\s*(\d+)/) {
> push @cpu, $entry if scalar keys(%{$entry}); # Previous entry
> - $entry = {}; # New entry
> + $entry = { nr => $1 }; # New entry
> next;
> }
> - if (m/^(vendor_id|cpu family|model|model name|stepping)\s*:\s*(.+)$/) {
> + if (m/^(vendor_id|cpu family|model|model name|stepping|cpuid level)\s*:\s*(.+)$/) {
> my $k = $1;
> my $v = $2;
> $v =~ s/\s+/ /g; # Merge multiple spaces
> @@ -2486,6 +2482,15 @@
> $modules_list{$normalized} = 1;
> }
>
> +# udev may take some time to create device nodes when loading modules
> +sub udev_settle
> +{
> + if (!(-x "/sbin/udevadm" && system("/sbin/udevadm settle") == 0)
> + && !(-x "/sbin/udevsettle" && system("/sbin/udevsettle") == 0)) {
> + sleep(1);
> + }
> +}
> +
> sub initialize_modules_supported
> {
> foreach my $chip (@chip_ids) {
> @@ -5833,23 +5838,33 @@
> return;
> }
>
> +sub cpuid
> +{
> + my ($cpu_nr, $eax) = @_;
> +
> + sysopen(CPUID, "/dev/cpu/$cpu_nr/cpuid", O_RDONLY) or return;
> + binmode CPUID;
> + sysseek(CPUID, $eax, SEEK_SET)
> + or die "Cannot seek /dev/cpu/$cpu_nr/cpuid";
> + sysread(CPUID, my $data, 16)
> + or die "Cannot read /dev/cpu/$cpu_nr/cpuid";
> + close CPUID;
> +
> + return unpack("L4", $data);
> +}
> +
> sub coretemp_detect
> {
> - my $chip = shift;
> my $probecpu;
>
> foreach $probecpu (@cpu) {
> next unless $probecpu->{vendor_id} eq 'GenuineIntel' &&
> - $probecpu->{'cpu family'} == 6;
> - return 9 if $chip == 0 &&
> - ($probecpu->{model} == 14 || # Pentium M DC
> - $probecpu->{model} == 15 || # Core 2 DC 65nm
> - $probecpu->{model} == 0x16 || # Core 2 SC 65nm
> - $probecpu->{model} == 0x17 || # Penryn 45nm
> - $probecpu->{model} == 0x1a || # Nehalem
> - $probecpu->{model} == 0x1e); # Lynnfield
> - return 9 if $chip == 1 &&
> - ($probecpu->{model} == 0x1c); # Atom
> + $probecpu->{'cpuid level'} >= 6;
> +
> + # Now we check for the DTS flag
> + my @regs = cpuid($probecpu->{nr}, 0x06);
> + return unless @regs == 4;
> + return 9 if ($regs[0] & (1 << 0)); # eax, bit 0
> }
> return;
> }
> @@ -6203,6 +6218,12 @@
> print "Some south bridges, CPUs or memory controllers contain embedded sensors.\n".
> "Do you want to scan for them? This is totally safe. (YES/no): ";
> unless (<STDIN> =~ /^\s*n/i) {
> + # Load the cpuid driver if needed
> + if (@cpu >= 1 && ! -e "/dev/cpu/$cpu[0]->{nr}/cpuid") {
> + load_module("cpuid");
> + udev_settle();
> + }
> +
> $| = 1;
> foreach my $entry (@cpu_ids) {
> scan_cpu($entry);
> @@ -6278,12 +6299,7 @@
> $by_default = 1 if dmi_match('board_vendor', 'asustek', 'tyan',
> 'supermicro');
>
> - # udev may take some time to create the device node
> - if (!(-x "/sbin/udevadm" && system("/sbin/udevadm settle") == 0)
> - && !(-x "/sbin/udevsettle" && system("/sbin/udevsettle") == 0)) {
> - sleep(1);
> - }
> -
> + udev_settle();
> for (my $dev_nr = 0; $dev_nr < @i2c_adapters; $dev_nr++) {
> next unless exists $i2c_adapters[$dev_nr];
> scan_i2c_adapter($dev_nr, $by_default);
>
> This seems to do the trick for me. Obviously it assumes that the cpuid
> kernel driver is available. All my systems have it available as a
> module, but I don't know if we can reasonably assume that it will be
> available on all x86 systems where sensors-detect is used.
>
> I would like this change to get as wide a test coverage as possible.
> To make testing easier, I've made the full script available for
> download at:
> http://khali.linux-fr.org/devel/misc/sensors-detect
> (It's exactly equivalent to sensors-detect SVN + the patch above.) In
> particular, this needs testing on Intel CPUs with cpuid level >= 6 but
> without DTS support... if such systems exist (I don't have any here,
> for sure.)
>
> If this appears to work for everyone and nobody comes up with a major
> objection, then we have our fix.
>
Works for me as expected on three systems. One has AMD CPUs and doesn't detect
anything, the other two report as expected that CPU sensors exist.
CPUs: Intel(R) Core(TM)2 Duo CPU T9300 @ 2.50GHz
Intel(R) Xeon(R) CPU C5528 @ 2.13GHz
Kernel versions 2.6.32-26 (Ubuntu) and 2.6.35.10 (generic)
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
