Bug 437637

Summary: sensors_init: Kernel interface error
Product: [Fedora] Fedora Reporter: Tom London <selinux>
Component: lm_sensorsAssignee: Hans de Goede <hdegoede>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: low Docs Contact:
Priority: low    
Version: rawhideCC: bruno, hdegoede, jdelvare, jfrieben, pknirsch, rui.zhang, swsnyder
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-04-10 14:52:28 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
PATCH fixing libsensors failing on hwmon class entries without a device link
none
PATCH: check if a device is a PCI device before assuming its a platform device.
none
PATCH: Recognize and properly handle virtual hwmon devices none

Description Tom London 2008-03-15 16:29:40 UTC
Description of problem:
Running kernel-2.6.25-0.121.rc5.git4.fc9.i686, lm_sensors won't start.

[root@localhost Downloads]# sensors
sensors_init: Kernel interface error
[root@localhost Downloads]# 

Tried updating to latest koji build, but still fails.

Version-Release number of selected component (if applicable):
lm_sensors-3.0.1-4.fc9.i386
lm_sensors-3.0.1-3.fc9.i386

How reproducible:
every time

Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:

Comment 1 Hans de Goede 2008-03-15 23:39:06 UTC
You don't happen to be running into this on a thinkpad are you?


Comment 2 Tom London 2008-03-16 00:19:54 UTC
Yup.

Thinkpad X60.



Comment 3 Tom London 2008-03-16 00:24:02 UTC
Here is output from 'strace sensors':

[root@localhost ~]# strace sensors
execve("/usr/bin/sensors", ["sensors"], [/* 24 vars */]) = 0
brk(0)                                  = 0x8622000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY)      = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=98064, ...}) = 0
mmap2(NULL, 98064, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f7d000
close(3)                                = 0
open("/usr/lib/libsensors.so.4", O_RDONLY) = 3
read(3,
"\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\2600\266\0004\0\0\0"..., 512) = 512
fstat64(3, {st_mode=S_IFREG|0755, st_size=53244, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xb7f7c000
mmap2(0xb61000, 50892, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) =
0xb61000
mmap2(0xb6d000, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE,
3, 0xc) = 0xb6d000
close(3)                                = 0
open("/lib/libc.so.6", O_RDONLY)        = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0p7\230\0004\0\0\0"...,
512) = 512
fstat64(3, {st_mode=S_IFREG|0755, st_size=1796316, ...}) = 0
mmap2(0x96d000, 1504848, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) =
0x96d000
mmap2(0xad7000, 12288, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x16a) = 0xad7000
mmap2(0xada000, 9808, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS,
-1, 0) = 0xada000
close(3)                                = 0
open("/lib/libm.so.6", O_RDONLY)        = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0`$\256\0004\0\0\0"...,
512) = 512
fstat64(3, {st_mode=S_IFREG|0755, st_size=211128, ...}) = 0
mmap2(0xadf000, 163968, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) =
0xadf000
mmap2(0xb06000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE,
3, 0x26) = 0xb06000
close(3)                                = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xb7f7b000
set_thread_area({entry_number:-1 -> 6, base_addr:0xb7f7b6c0, limit:1048575,
seg_32bit:1, contents:0, read_exec_only:0, limit_in_pages:1, seg_not_present:0,
useable:1}) = 0
mprotect(0xad7000, 8192, PROT_READ)     = 0
mprotect(0xb06000, 4096, PROT_READ)     = 0
mprotect(0x969000, 4096, PROT_READ)     = 0
munmap(0xb7f7d000, 98064)               = 0
brk(0)                                  = 0x8622000
brk(0x8643000)                          = 0x8643000
open("/usr/lib/locale/locale-archive", O_RDONLY|O_LARGEFILE) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=77970672, ...}) = 0
mmap2(NULL, 2097152, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7d7b000
close(3)                                = 0
stat64("/sys", {st_mode=S_IFDIR|0755, st_size=0, ...}) = 0
open("/sys/class/i2c-adapter",
O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|0x80000) = 3
fstat64(3, {st_mode=S_IFDIR|0755, st_size=0, ...}) = 0
fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
getdents(3, /* 3 entries */, 4096)      = 52
open("/sys/class/i2c-adapter/i2c-0/name", O_RDONLY) = 4
fstat64(4, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xb7f94000
read(4, "SMBus I801 adapter at 18e0\n", 4096) = 27
close(4)                                = 0
munmap(0xb7f94000, 4096)                = 0
getdents(3, /* 0 entries */, 4096)      = 0
close(3)                                = 0
open("/sys/class/hwmon", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|0x80000) = 3
fstat64(3, {st_mode=S_IFDIR|0755, st_size=0, ...}) = 0
getdents(3, /* 6 entries */, 4096)      = 112
readlink("/sys/class/hwmon/hwmon0/device", 0xbf892a1a, 254) = -1 ENOENT (No such
file or directory)
close(3)                                = 0
write(2, "sensors_init: Kernel interface e"..., 37sensors_init: Kernel interface
error
) = 37
exit_group(1)                           = ?
[root@localhost ~]# 

Here is 'ls /sys/class/hwmon/hwmon0':
[root@localhost hwmon0]# ls
name   subsystem   temp1_input  temp2_input
power  temp1_crit  temp2_crit   uevent
[root@localhost hwmon0]# 

and

[root@localhost hwmon]# ls -l *
lrwxrwxrwx 1 root root 0 2008-03-15 15:07 hwmon0 ->
../../devices/virtual/hwmon/hwmon0
lrwxrwxrwx 1 root root 0 2008-03-15 15:07 hwmon1 ->
../../devices/platform/thinkpad_hwmon/hwmon/hwmon1
lrwxrwxrwx 1 root root 0 2008-03-15 15:07 hwmon2 ->
../../devices/platform/coretemp.0/hwmon/hwmon2
lrwxrwxrwx 1 root root 0 2008-03-15 15:07 hwmon3 ->
../../devices/platform/coretemp.1/hwmon/hwmon3
[root@localhost hwmon]# 

Seems to be a 'device' in hwmon1:

[root@localhost hwmon]# ls -l hwmon1/*
lrwxrwxrwx 1 root root    0 2008-03-15 17:22 hwmon1/device ->
../../../thinkpad_hwmon
lrwxrwxrwx 1 root root    0 2008-03-15 15:07 hwmon1/subsystem ->
../../../../../class/hwmon
-rw-r--r-- 1 root root 4096 2008-03-15 15:07 hwmon1/uevent

hwmon1/power:
total 0
-rw-r--r-- 1 root root 4096 2008-03-15 17:23 wakeup
[root@localhost hwmon]# 


Comment 4 Hans de Goede 2008-03-16 09:39:49 UTC
I thought this is the same bug as 436767, but looking at the strace its different.

Jean (added to the CC),

I believe this is triggered by the hwmon interface of the new acpi thermal zone
code, which should work with 3.0.1 because of your fix for:
http://lm-sensors.org/ticket/2260

Which is this commit:
http://lm-sensors.org/changeset/5093

However the readlink in this function:

static int sensors_add_hwmon_device(const char *path, const char *classdev)
{
        char linkpath[NAME_MAX];
        char device[NAME_MAX], *device_p;
        int dev_len, err;
        (void)classdev; /* hide warning */

        snprintf(linkpath, NAME_MAX, "%s/device", path);
        dev_len = readlink(linkpath, device, NAME_MAX - 1);
        if (dev_len < 0)
                return -SENSORS_ERR_KERNEL;
        device[dev_len] = '\0';
        device_p = strrchr(device, '/') + 1;
 
        /* The attributes we want might be those of the hwmon class device,
           or those of the device itself. */
        err = sensors_read_one_sysfs_chip(linkpath, device_p, path);
        if (err == 0)
                err = sensors_read_one_sysfs_chip(linkpath, device_p, linkpath)
        if (err < 0)
                return err;
        return 0;
}

Is failing as there is no device pointer for the hwmon class entry made by the
acpi thermal zone driver, note that the hwmon0 entry in /sys/class/hwmon points to:
/sys/devices/virtual/hwmon/hwmon0

And appearantly hwmon class entries of a virtual device, do not have a device
pointer. Notice that the results of the readlink are only used to pass as second
argument to sensors_read_one_sysfs_chip(), the second argument of
sensors_read_one_sysfs_chip(), only gets used by sensors_read_one_sysfs_chip()
to determine the bus and address of the sensor, passing in something like
"virtual" in the scenario when there is no device link should work.

Another problem here is that linkpath, which now does not exists, gets passed as
first param to sensors_read_one_sysfs_chip(), linkpath can simply be replaced in
this case by path, as the first param to sensors_read_one_sysfs_chip() is used
to look for a "subsystem" link and then fallback to a "sub" link in that path,
and sensors_read_one_sysfs_chip() will still succeed if no subsystem nor sub
links are found, in which case when it does not recognize its second param as
representing a certain subsys type, it will assume platform/ISA as type for the
sensor, which sounds reasonable.

I'll attach a patch implementing this, and do a scratch build of a new
lm_sensors with this fix in.


Comment 5 Hans de Goede 2008-03-16 09:45:08 UTC
Created attachment 298182 [details]
PATCH fixing libsensors failing on hwmon class entries without a device link

Comment 6 Hans de Goede 2008-03-16 10:11:52 UTC
Created attachment 298183 [details]
PATCH: check if a device is a PCI device before assuming its a platform device.

Jean,

While looking into this I also noticed that sensors_read_one_sysfs_chip(),
currently, if there is no subsytem or sub link in device_path, will recognize a
PCI device as being an ISA device, even if dev_name does match a PCI device,
because if there is no subsys link found it assumes a platform device, before
checking for a PCI device.

This simple patch fixes this (theoretical, all pci drivers should have a subsys
link, given when they were introduced) problem.

Let me know if its ok to commit this fix to svn.

Comment 7 Hans de Goede 2008-03-16 10:16:56 UTC
Tom,

Here is a sratch build of a new version of lm_sensors which hopefully fixes this:
http://koji.fedoraproject.org/koji/taskinfo?taskID=518228

Can you give this a try and see if it fixes things for you please?


Comment 8 Jean Delvare 2008-03-16 10:27:07 UTC
(In reply to comment #6)
> Created an attachment (id=298183) [edit]
> PATCH: check if a device is a PCI device before assuming its a platform
> device.
> 
> Jean,
> 
> While looking into this I also noticed that sensors_read_one_sysfs_chip(),
> currently, if there is no subsytem or sub link in device_path, will recognize
> a PCI device as being an ISA device, even if dev_name does match a PCI device,
> because if there is no subsys link found it assumes a platform device, before
> checking for a PCI device.
> 
> This simple patch fixes this (theoretical, all pci drivers should have a
> subsys link, given when they were introduced) problem.
> 
> Let me know if its ok to commit this fix to svn.

Yes, your patch is correct and should be committed to SVN. The platform case
acts as a fallback solution so it must be last in the list. Thanks for spotting
this bug. That being said, it probably won't change anything in practice.


Comment 9 Jean Delvare 2008-03-16 12:42:42 UTC
Problem might go beyond ACPI thermal zone and virtual devices... See:
http://bugzilla.kernel.org/show_bug.cgi?id=10259

It seems that something changed in sysfs between 2.6.25-rc5-git3 and -git4. I'll
investigate later this afternoon.


Comment 10 Jean Delvare 2008-03-16 15:14:57 UTC
(In reply to comment #9)
> Problem might go beyond ACPI thermal zone and virtual devices... See:
> http://bugzilla.kernel.org/show_bug.cgi?id=10259

In fact this is the exact same bug. The reporter simply got confused by the
renumbering of the hwmon class devices caused by the new generic thermal driver.


Comment 11 Hans de Goede 2008-03-16 15:50:14 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > Problem might go beyond ACPI thermal zone and virtual devices... See:
> > http://bugzilla.kernel.org/show_bug.cgi?id=10259
> 
> In fact this is the exact same bug. The reporter simply got confused by the
> renumbering of the hwmon class devices caused by the new generic thermal driver.
> 

Then my patch should fix things, Tom any chance you can provide us with some
feedback on this?

IOW can you install:
http://koji.fedoraproject.org/koji/taskinfo?taskID=518228

And let us know if it fixes things?



Comment 12 Tom London 2008-03-16 16:38:57 UTC
Thanks for the quick fix.  [Sorry for the slow response.... ]

I installed lm_sensors-3.0.1-5.fc9.i386.rpm and ran "sensors":

[root@localhost Downloads]# sensors
thinkpad-isa-0000
Adapter: ISA adapter
fan1:       3008 RPM
temp1:       +42.0°C                                    
temp2:       +32.0°C                                    
ERROR: Can't get value of subfeature temp3_input: Can't read
temp3:        +0.0°C                                    
temp4:       +42.0°C                                    
temp5:       +16.0°C                                    
ERROR: Can't get value of subfeature temp6_input: Can't read
temp6:        +0.0°C                                    
temp7:       +16.0°C                                    
ERROR: Can't get value of subfeature temp8_input: Can't read
temp8:        +0.0°C                                    
temp9:       +34.0°C                                    
temp10:      +36.0°C                                    
ERROR: Can't get value of subfeature temp11_input: Can't read
temp11:       +0.0°C                                    
ERROR: Can't get value of subfeature temp12_input: Can't read
temp12:       +0.0°C                                    
ERROR: Can't get value of subfeature temp13_input: Can't read
temp13:       +0.0°C                                    
ERROR: Can't get value of subfeature temp14_input: Can't read
temp14:       +0.0°C                                    
ERROR: Can't get value of subfeature temp15_input: Can't read
temp15:       +0.0°C                                    
ERROR: Can't get value of subfeature temp16_input: Can't read
temp16:       +0.0°C                                    

coretemp-isa-0000
Adapter: ISA adapter
Core 0:      +45.0°C  (crit = +100.0°C)                  

coretemp-isa-0001
Adapter: ISA adapter
Core 1:      +44.0°C  (crit = +100.0°C)                  

[root@localhost Downloads]# 

and

[root@localhost Downloads]# service lm_sensors restart
Stopping lm_sensors:                                       [  OK  ]
Starting lm_sensors: loading module coretemp               [  OK  ]
[root@localhost Downloads]# 

so looks like this fixes it.




Comment 13 Jean Delvare 2008-03-16 16:50:35 UTC
(In reply to comment #5)
> Created an attachment (id=298182) [edit]
> PATCH fixing libsensors failing on hwmon class entries without a device link

Hans, your patch doesn't work. It lets libsensors go past the missing "device"
link, however in sensors_read_one_sysfs_chip(), when reading the "subsystem"
link, it gets "hwmon" for the new thermal zone device, and this isn't supported,
so the device gets ignored. While this is an improvement compared to the current
situation (at least other hwmon devices are still displayed) that's probably not
what you hoped for.

There are a number of design decisions around the new thermal zone device which
I do not like. I'll start a discussion thread about this. Depending on the
outcast, libsensors may or may not need additional tweaks. Until this is sorted
out, I think that we should simply let libsensors ignore hwmon entries with no
device link. Ignoring unsupported stuff is the right thing to do anyway,
otherwise we end up fixing libsensors in a rush each time something changes on
the kernel side.

Comment 14 Hans de Goede 2008-03-16 19:04:31 UTC
Ah, the hwmon class dir does have a subsystem link, I thought it didn't have any
and thus we would end up calling the thermalzone hwmon entry an ISA device at
address 0.

Comment 15 Zhang Rui 2008-03-17 02:07:54 UTC
Hi, as Hans suggested that all hwmon attributes for generic thermal driver go 
to one hwmonX entry, the hwmonX registered by the Generic Thermal driver is a 
virtual devices, thus no device link available.
And hwmonX contains the sys I/F for several thermal zones, while each of them 
has a device node.
I'm not quite clear about why a device link is mandatory.
But I don't see the benefit of a device link for hwmonX registered by the 
generic thermal driver. IMO, it can't be used to do anything. So we should 
either ignore the check of a device link, or ingore the hwmonX with the 
name "thermal_sys_class". :)


Comment 16 Hans de Goede 2008-03-17 09:31:33 UTC
I still believe that having one hwmon entry with all the attributes of the
thermal zone driver there is the best interface, having one hwmon class per zone
doesn't make sense.

If this means that there won't be a device link and libsensors needs to be
updated to handle this, so be it.


Comment 17 Jean Delvare 2008-03-17 12:10:11 UTC
I do not share your views that one hwmon entry with all the attributes of the
thermal zone driver is the best choice. I'm sorry that I did not notice your
proposal in this direction earlier. I think that it would make more sense to
have one hwmon entry for each "user" of the generic thermal zone driver, i.e.
one hwmon entry for all ACPI thermal zones, and one more hwmon entry for each
other type of thermal zones.

That being said, doing this would probably not help with the problem at hand, as
each ACPI thermal zone has it's own device. They have a common parent, which
could be used as the hwmon's device link... Not sure if this could be
implemented easily though. Anyway, let's discuss this on the mailing lists,
bugzilla isn't the place.


Comment 18 Hans de Goede 2008-03-17 13:01:01 UTC
(In reply to comment #17)
> I do not share your views that one hwmon entry with all the attributes of the
> thermal zone driver is the best choice. I'm sorry that I did not notice your
> proposal in this direction earlier. I think that it would make more sense to
> have one hwmon entry for each "user" of the generic thermal zone driver, i.e.
> one hwmon entry for all ACPI thermal zones, and one more hwmon entry for each
> other type of thermal zones.
> 

+1,

> I didn't know there could be multiple drivers exporting thermal zones.
> Anyway, let's discuss this on the mailing lists,
> bugzilla isn't the place.

+1, See you on the lists.



Comment 19 Hans de Goede 2008-03-17 13:01:52 UTC
Crap,

Notice that "I didn't know there could be multiple drivers exporting thermal
zones" in comment #18 is not text being replied to but me motivating my +1.


Comment 20 Jean Delvare 2008-03-17 14:59:01 UTC
Created attachment 298270 [details]
PATCH: Recognize and properly handle virtual hwmon devices

This is my attempt to add support for virtual devices to lm-sensors 3.0.1.
Tested successfully on two different systems. Hans, can you please review and
comment? Note that I am not yet sure if this can be the final fix, depends on
the conclusion of our discussion on the mailing lists, and others things I want
to investigate.

Comment 21 Joachim Frieben 2008-03-18 06:11:54 UTC
Same issue for a Tyan Tomcat K8E running the x86_64 branch of "rawhide".

Comment 22 Hans de Goede 2008-03-18 10:03:07 UTC
(In reply to comment #20)
> Created an attachment (id=298270) [edit]
> PATCH: Recognize and properly handle virtual hwmon devices
> 
> This is my attempt to add support for virtual devices to lm-sensors 3.0.1.
> Tested successfully on two different systems. Hans, can you please review and
> comment? Note that I am not yet sure if this can be the final fix, depends on
> the conclusion of our discussion on the mailing lists, and others things I want
> to investigate.
> 

Ok, here is my review:

1) I'm worried that we may one day get multiple instances of a virtual device,
then we need todo something smarter with chip->addr, as the prefix will be the
same and if we keep addr at 0 for all virtual devices 2 instances of a virtual
device will get the same result from sensors_snprintf_chip_name(), and many
libsensors using apps use sensors_snprintf_chip_name() /
sensors_parse_chip_name() to store / retrieve chip name structures. But it might
be best to leave fixing this until we actually run into this problem.

2) You've accidentally included a local Makefile change in the patch

3) Since you (correctly) bump the API define, you really need to document both
the bump and what changed in CHANGES


Comment 23 Hans de Goede 2008-03-18 10:06:42 UTC
New scratch-build with Jean's version of the patch:
http://koji.fedoraproject.org/koji/taskinfo?taskID=520591

Tom and/or Joachim, can you give this a test spin?

Tom, this should give you one more additional device listed in your sensors
command output (the acpi thermal zone's temperature readings).


Comment 24 Tom London 2008-03-18 13:32:55 UTC
OK.  Installed (had to use '--force' since the version # didn't change).

Here is the output from 'sensors'.  Believe the
'thermal_sys_class-virtual/Virtual device' is new:

[root@localhost Downloads]# sensors
thermal_sys_class-virtual-0
Adapter: Virtual device
temp1:       +28.0°C  (crit = +127.0°C)                  
temp2:       +30.0°C  (crit = +97.0°C)                  

thinkpad-isa-0000
Adapter: ISA adapter
fan1:       3009 RPM
temp1:       +47.0°C                                    
temp2:       +33.0°C                                    
ERROR: Can't get value of subfeature temp3_input: Can't read
temp3:        +0.0°C                                    
temp4:       +44.0°C                                    
temp5:       +18.0°C                                    
ERROR: Can't get value of subfeature temp6_input: Can't read
temp6:        +0.0°C                                    
temp7:       +18.0°C                                    
ERROR: Can't get value of subfeature temp8_input: Can't read
temp8:        +0.0°C                                    
temp9:       +34.0°C                                    
temp10:      +37.0°C                                    
ERROR: Can't get value of subfeature temp11_input: Can't read
temp11:       +0.0°C                                    
ERROR: Can't get value of subfeature temp12_input: Can't read
temp12:       +0.0°C                                    
ERROR: Can't get value of subfeature temp13_input: Can't read
temp13:       +0.0°C                                    
ERROR: Can't get value of subfeature temp14_input: Can't read
temp14:       +0.0°C                                    
ERROR: Can't get value of subfeature temp15_input: Can't read
temp15:       +0.0°C                                    
ERROR: Can't get value of subfeature temp16_input: Can't read
temp16:       +0.0°C                                    

coretemp-isa-0000
Adapter: ISA adapter
Core 0:      +48.0°C  (crit = +100.0°C)                  

coretemp-isa-0001
Adapter: ISA adapter
Core 1:      +48.0°C  (crit = +100.0°C)                  

[root@localhost Downloads]# 


Comment 25 Jean Delvare 2008-03-18 13:39:04 UTC
(In reply to comment #22)
> 1) I'm worried that we may one day get multiple instances of a virtual device,
> then we need todo something smarter with chip->addr, as the prefix will be the
> same and if we keep addr at 0 for all virtual devices 2 instances of a virtual
> device will get the same result from sensors_snprintf_chip_name(), and many
> libsensors using apps use sensors_snprintf_chip_name() /
> sensors_parse_chip_name() to store / retrieve chip name structures. But it
> might be best to leave fixing this until we actually run into this problem.

I agree, it's really only a quick hack at the moment. We need to give it some
more thought, but I am waiting to see what will happen on the kernel side first.
I do not want libsensors to attribute arbitrary sequence numbers (it wouldn't be
stable across reboots etc.) so the "address" must come from the kernel somehow.

> 2) You've accidentally included a local Makefile change in the patch

Damn. It happens to me all the time :( Sorry about this.

> 3) Since you (correctly) bump the API define, you really need to document both
> the bump and what changed in CHANGES

I agree. It might even deserve a separate file under doc/developers/.

Comment 26 Joachim Frieben 2008-03-18 14:02:02 UTC
lm_sensors-3.0.1-5.fc9 works for me:

thermal_sys_class-virtual-0
Adapter: Virtual device
temp1:       +40.0°C  (crit = +100.0°C)                  

k8temp-pci-00c3
Adapter: PCI adapter
Core0 Temp:  +51.0°C

[I have neither tailored a custom /etc/sensors3.conf nor have I run
sensors-detect.]

Comment 27 Bruno Wolff III 2008-03-19 07:54:35 UTC
3.0.1-5.fc9 fixed a problem I was having.
Previously I was getting:
Starting lm_sensors: loading module i2c-amd756 lm90 w83627hf sensors_init:
Kernel interface error
After upgrading I got:
Starting lm_sensors: loading module i2c-amd756 lm90 w83627h[  OK  ]

Comment 28 Steve Snyder 2008-03-27 05:25:27 UTC
Another data point:

I had the same problem as reported in comment #27, on installed F9 Beta.  
Updating to lm_sensors-3.0.1-5.fc9.x86_64.rpm fixed it for me as well.  Updated 
only this package, not a general update.

---------------------

[root@localhost tmp]# service lm_sensors start
Starting lm_sensors: loading module i2c-i801 w83793 coretem[  OK  ]
[root@localhost tmp]# sensors
w83793-i2c-0-2f
Adapter: SMBus I801 adapter at 1100
VCoreA:      +1.20 V  (min =  +0.92 V, max =  +1.49 V)
VCoreB:      +1.20 V  (min =  +0.92 V, max =  +1.49 V)
Vtt:         +1.08 V  (min =  +1.08 V, max =  +1.33 V)   ALARM
in3:         +0.51 V  (min =  +0.38 V, max =  +0.69 V)
in4:         +1.46 V  (min =  +1.34 V, max =  +1.65 V)
+3.3V:       +3.30 V  (min =  +2.96 V, max =  +3.63 V)
+12V:       +12.00 V  (min = +10.75 V, max = +13.25 V)
+5V:         +5.05 V  (min =  +4.64 V, max =  +5.65 V)
5VSB:        +5.07 V  (min =  +4.64 V, max =  +5.65 V)
VBAT:        +3.22 V  (min =  +2.99 V, max =  +3.66 V)
fan1:       3678 RPM  (min =  712 RPM)
fan2:       3658 RPM  (min =  712 RPM)
fan3:       3562 RPM  (min =  712 RPM)
fan4:       3802 RPM  (min =  712 RPM)
fan5:       2890 RPM  (min =  712 RPM)
fan6:          0 RPM  (min =  712 RPM)  ALARM
fan7:          0 RPM  (min =  712 RPM)  ALARM
fan8:          0 RPM  (min =  712 RPM)  ALARM
fan9:          0 RPM  (min =  712 RPM)  ALARM
fan10:         0 RPM  (min =  712 RPM)  ALARM
CPU1 Temp:   +23.0°C  (high = +70.0°C, hyst = +65.0°C)  sensor = Intel PECI
CPU2 Temp:   +19.0°C  (high = +70.0°C, hyst = +65.0°C)  sensor = Intel PECI
temp3:      -128.0°C  (high = +70.0°C, hyst = +65.0°C)  sensor = Intel PECI
temp4:      -128.0°C  (high = +70.0°C, hyst = +65.0°C)  sensor = Intel PECI
temp5:       +26.0°C  (high = +50.0°C, hyst = +45.0°C)  sensor = thermistor
beep_enable:disabled

coretemp-isa-0000
Adapter: ISA adapter
Core 0:      +31.0°C  (high = +76.0°C, crit = +100.0°C)

coretemp-isa-0001
Adapter: ISA adapter
Core 1:      +16.0°C  (high = +76.0°C, crit = +100.0°C)

coretemp-isa-0002
Adapter: ISA adapter
Core 2:      +34.0°C  (high = +76.0°C, crit = +100.0°C)

coretemp-isa-0003
Adapter: ISA adapter
Core 3:      +16.0°C  (high = +76.0°C, crit = +100.0°C)

coretemp-isa-0004
Adapter: ISA adapter
Core 4:      +30.0°C  (high = +76.0°C, crit = +100.0°C)

coretemp-isa-0005
Adapter: ISA adapter
Core 5:      +29.0°C  (high = +76.0°C, crit = +100.0°C)

coretemp-isa-0006
Adapter: ISA adapter
Core 6:      +27.0°C  (high = +76.0°C, crit = +100.0°C)

coretemp-isa-0007
Adapter: ISA adapter
Core 7:      +29.0°C  (high = +76.0°C, crit = +100.0°C)


Comment 29 Tom London 2008-04-10 13:56:13 UTC
This is "working for me".

Close?

Comment 30 Hans de Goede 2008-04-10 14:52:28 UTC
(In reply to comment #29)
> This is "working for me".
> 
> Close?

Erm, yes its working with the scratch build from koji, but I didn't do a real
build yet (waiting for possible another solution from upstream). I've just
started a real build, and will request for F-9 inclusion of this.


Comment 31 Jean Delvare 2008-04-14 15:34:56 UTC
Patch has been applied upstream:
http://www.lm-sensors.org/changeset/5176


Comment 32 Steve Snyder 2008-04-14 16:44:38 UTC
(In reply to comment #31)
> Patch has been applied upstream:
> http://www.lm-sensors.org/changeset/5176

Will this (working) version of lm_sensors make it into the Fedora 9 release?




Comment 33 Hans de Goede 2008-04-14 17:20:02 UTC
(In reply to comment #32)
> (In reply to comment #31)
> > Patch has been applied upstream:
> > http://www.lm-sensors.org/changeset/5176
> 
> Will this (working) version of lm_sensors make it into the Fedora 9 release?
> 

I've requested rel-eng to include it and they haven't objected, so yes.