linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series
@ 2013-10-20 18:10 Arnaud Ebalard
  2013-10-20 19:23 ` Guenter Roeck
  2013-10-21  7:17 ` Jean Delvare
  0 siblings, 2 replies; 9+ messages in thread
From: Arnaud Ebalard @ 2013-10-20 18:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

With 3.12-rc series, sysfs support for thermal susbsytem (and/or hwmon
one) was modified in such a way that sensors utility (current 3.3.4
version with 3.3.4 version of libsensors from lm-sensors package on
Debian unstable) does not see the temperature sensor anymore on armada
370 platforms (not tested on others). Additionally, the changes break
existing configurations of fancontrol utility, which prevents the 
fan to be regulated correctly w/o recreating an /etc/fancontrol w/
pwmconfig.

Here is what I have on my Armada 370-based system on a 3.11.5:

# sensors
g762-i2c-0-3e
Adapter: mv64xxx_i2c adapter
fan1:        2457 RPM  (div = 1)

armada_thermal-virtual-0
Adapter: Virtual device
temp1:        +45.7?C  

And what I get on 3.12-rc6:

# sensors
g762-i2c-0-3e
Adapter: mv64xxx_i2c adapter
fan1:        1350 RPM  (div = 1)


Monitoring what sensors does w/ strace, I started looking at the changes
to /sys/class/hwmon/hwmon1/:

On 3.11.5:

# find /sys/class/hwmon/hwmon1/
/sys/class/hwmon/hwmon1/
/sys/class/hwmon/hwmon1/name
/sys/class/hwmon/hwmon1/subsystem
/sys/class/hwmon/hwmon1/uevent
/sys/class/hwmon/hwmon1/temp1_input

On 3.12-rc6:

# find /sys/class/hwmon/hwmon1/
/sys/class/hwmon/hwmon1/
/sys/class/hwmon/hwmon1/name
/sys/class/hwmon/hwmon1/device
/sys/class/hwmon/hwmon1/subsystem
/sys/class/hwmon/hwmon1/uevent
/sys/class/hwmon/hwmon1/temp1_input

# find /sys/class/hwmon/hwmon1/device/
/sys/class/hwmon/hwmon1/device/
/sys/class/hwmon/hwmon1/device/temp
/sys/class/hwmon/hwmon1/device/type
/sys/class/hwmon/hwmon1/device/hwmon1
/sys/class/hwmon/hwmon1/device/hwmon1/name
/sys/class/hwmon/hwmon1/device/hwmon1/device
/sys/class/hwmon/hwmon1/device/hwmon1/subsystem
/sys/class/hwmon/hwmon1/device/hwmon1/uevent
/sys/class/hwmon/hwmon1/device/hwmon1/temp1_input
/sys/class/hwmon/hwmon1/device/subsystem
/sys/class/hwmon/hwmon1/device/policy
/sys/class/hwmon/hwmon1/device/uevent
/sys/class/hwmon/hwmon1/device/passive

Is that expected? As for sensors, it *seems* to be bothered to find a
device/ folder in /sys/class/hwmon/hwmon1/ w/o no name entry in it.

Cheers,

a+

ps: I can test if this is the same on kirkwood if there is a need.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series
  2013-10-20 18:10 [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series Arnaud Ebalard
@ 2013-10-20 19:23 ` Guenter Roeck
  2013-10-21  2:28   ` Zhang Rui
  2013-10-21  7:17 ` Jean Delvare
  1 sibling, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2013-10-20 19:23 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/20/2013 11:10 AM, Arnaud Ebalard wrote:
> Hi,
>
> With 3.12-rc series, sysfs support for thermal susbsytem (and/or hwmon
> one) was modified in such a way that sensors utility (current 3.3.4
> version with 3.3.4 version of libsensors from lm-sensors package on
> Debian unstable) does not see the temperature sensor anymore on armada
> 370 platforms (not tested on others). Additionally, the changes break
> existing configurations of fancontrol utility, which prevents the
> fan to be regulated correctly w/o recreating an /etc/fancontrol w/
> pwmconfig.
>
> Here is what I have on my Armada 370-based system on a 3.11.5:
>
> # sensors
> g762-i2c-0-3e
> Adapter: mv64xxx_i2c adapter
> fan1:        2457 RPM  (div = 1)
>
> armada_thermal-virtual-0
> Adapter: Virtual device
> temp1:        +45.7?C
>
> And what I get on 3.12-rc6:
>
> # sensors
> g762-i2c-0-3e
> Adapter: mv64xxx_i2c adapter
> fan1:        1350 RPM  (div = 1)
>
>
> Monitoring what sensors does w/ strace, I started looking at the changes
> to /sys/class/hwmon/hwmon1/:
>
> On 3.11.5:
>
> # find /sys/class/hwmon/hwmon1/
> /sys/class/hwmon/hwmon1/
> /sys/class/hwmon/hwmon1/name
> /sys/class/hwmon/hwmon1/subsystem
> /sys/class/hwmon/hwmon1/uevent
> /sys/class/hwmon/hwmon1/temp1_input
>
> On 3.12-rc6:
>
> # find /sys/class/hwmon/hwmon1/
> /sys/class/hwmon/hwmon1/
> /sys/class/hwmon/hwmon1/name
> /sys/class/hwmon/hwmon1/device
> /sys/class/hwmon/hwmon1/subsystem
> /sys/class/hwmon/hwmon1/uevent
> /sys/class/hwmon/hwmon1/temp1_input
>
> # find /sys/class/hwmon/hwmon1/device/
> /sys/class/hwmon/hwmon1/device/
> /sys/class/hwmon/hwmon1/device/temp
> /sys/class/hwmon/hwmon1/device/type
> /sys/class/hwmon/hwmon1/device/hwmon1
> /sys/class/hwmon/hwmon1/device/hwmon1/name
> /sys/class/hwmon/hwmon1/device/hwmon1/device
> /sys/class/hwmon/hwmon1/device/hwmon1/subsystem
> /sys/class/hwmon/hwmon1/device/hwmon1/uevent
> /sys/class/hwmon/hwmon1/device/hwmon1/temp1_input
> /sys/class/hwmon/hwmon1/device/subsystem
> /sys/class/hwmon/hwmon1/device/policy
> /sys/class/hwmon/hwmon1/device/uevent
> /sys/class/hwmon/hwmon1/device/passive
>
> Is that expected? As for sensors, it *seems* to be bothered to find a
> device/ folder in /sys/class/hwmon/hwmon1/ w/o no name entry in it.
>

The 'name' attribute should not be the problem, since there is a 'name'
attribute in the /sys/class/hwmon/hwmon1/ directory.

Key difference is that there is now a 'device' subdirectory, which results
in different handling by libsensors; the entry is no longer a virtual entry
but is expected to have a real device attached to it. For this device,
libsensors tries to scan the 'subsystem' entry which in turn must be well
defined and known. My suspicion is that the reported subsystem may not be
recognized by libsensors.

One question is why there is now a device entry, even though this is still as
virtual as it was before. You'll have to ask the thermal subsystem maintainers
for an answer.

I am also concerned about the 'hwmon1' subdirectory underneath hwmon1/device;
that suggests that hwmon1 may be declared to be a child of itself, which would
obviously not be a good idea.

Also, note that the thermal subsystem creates (or may create) sensor attributes
after registering the hwmon device, which means you can not rely on the udev
event that comes with the hwmon device creation and assume that all sensor
attributes exist at that time. I don't currently know how to handle this situation.
This is not unique, though; the coretemp driver does the same. Just something
to keep in mind.

Guenter

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series
  2013-10-20 19:23 ` Guenter Roeck
@ 2013-10-21  2:28   ` Zhang Rui
  2013-10-21  5:19     ` Guenter Roeck
  2013-10-21 18:49     ` Arnaud Ebalard
  0 siblings, 2 replies; 9+ messages in thread
From: Zhang Rui @ 2013-10-21  2:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Sun, 2013-10-20 at 12:23 -0700, Guenter Roeck wrote:
> On 10/20/2013 11:10 AM, Arnaud Ebalard wrote:
> > Hi,
> >
> > With 3.12-rc series, sysfs support for thermal susbsytem (and/or hwmon
> > one) was modified in such a way that sensors utility (current 3.3.4
> > version with 3.3.4 version of libsensors from lm-sensors package on
> > Debian unstable) does not see the temperature sensor anymore on armada
> > 370 platforms (not tested on others). Additionally, the changes break
> > existing configurations of fancontrol utility, which prevents the
> > fan to be regulated correctly w/o recreating an /etc/fancontrol w/
> > pwmconfig.
> >
> > Here is what I have on my Armada 370-based system on a 3.11.5:
> >
> > # sensors
> > g762-i2c-0-3e
> > Adapter: mv64xxx_i2c adapter
> > fan1:        2457 RPM  (div = 1)
> >
> > armada_thermal-virtual-0
> > Adapter: Virtual device
> > temp1:        +45.7?C
> >
> > And what I get on 3.12-rc6:
> >
> > # sensors
> > g762-i2c-0-3e
> > Adapter: mv64xxx_i2c adapter
> > fan1:        1350 RPM  (div = 1)
> >
> >
> > Monitoring what sensors does w/ strace, I started looking at the changes
> > to /sys/class/hwmon/hwmon1/:
> >
> > On 3.11.5:
> >
> > # find /sys/class/hwmon/hwmon1/
> > /sys/class/hwmon/hwmon1/
> > /sys/class/hwmon/hwmon1/name
> > /sys/class/hwmon/hwmon1/subsystem
> > /sys/class/hwmon/hwmon1/uevent
> > /sys/class/hwmon/hwmon1/temp1_input
> >
> > On 3.12-rc6:
> >
> > # find /sys/class/hwmon/hwmon1/
> > /sys/class/hwmon/hwmon1/
> > /sys/class/hwmon/hwmon1/name
> > /sys/class/hwmon/hwmon1/device
> > /sys/class/hwmon/hwmon1/subsystem
> > /sys/class/hwmon/hwmon1/uevent
> > /sys/class/hwmon/hwmon1/temp1_input
> >
> > # find /sys/class/hwmon/hwmon1/device/
> > /sys/class/hwmon/hwmon1/device/
> > /sys/class/hwmon/hwmon1/device/temp
> > /sys/class/hwmon/hwmon1/device/type
> > /sys/class/hwmon/hwmon1/device/hwmon1
> > /sys/class/hwmon/hwmon1/device/hwmon1/name
> > /sys/class/hwmon/hwmon1/device/hwmon1/device
> > /sys/class/hwmon/hwmon1/device/hwmon1/subsystem
> > /sys/class/hwmon/hwmon1/device/hwmon1/uevent
> > /sys/class/hwmon/hwmon1/device/hwmon1/temp1_input
> > /sys/class/hwmon/hwmon1/device/subsystem
> > /sys/class/hwmon/hwmon1/device/policy
> > /sys/class/hwmon/hwmon1/device/uevent
> > /sys/class/hwmon/hwmon1/device/passive
> >
> > Is that expected? As for sensors, it *seems* to be bothered to find a
> > device/ folder in /sys/class/hwmon/hwmon1/ w/o no name entry in it.
> >

I agree. And it should be caused by this commit.

commit b82715fdd4a5407f56853b24d387d484dd9c3b5b
Author: Eduardo Valentin <eduardo.valentin@ti.com>
Date:   Fri Aug 23 17:07:58 2013 -0400

    drivers: thermal: parent virtual hwmon with thermal zone
    
    When  creating virtual hwmon devices based out of thermal
    zone devices, the virtual devices won't have parents.
    
    This patch changes the code so that the parent of virtual
    hwmon devices is the thermal zone device that they are
    based of.
    
    Cc: Zhang Rui <rui.zhang@intel.com>
    Cc: linux-pm at vger.kernel.org
    Cc: linux-kernel at vger.kernel.org
    Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>

> 
> The 'name' attribute should not be the problem, since there is a 'name'
> attribute in the /sys/class/hwmon/hwmon1/ directory.
> 
> Key difference is that there is now a 'device' subdirectory,

Right.

>  which results
> in different handling by libsensors;
Oh, I'm not aware of this before.
There is no such statement in the comments of hwmon_device_register(),
or anywhere else.
Could you show me some tutorials about how the sysfs I/F misleads
libsensors?

But anyway, I will revert this patch first.
Thanks for reporting the problem, Arnaud!

thanks,
rui
>  the entry is no longer a virtual entry
> but is expected to have a real device attached to it. For this device,
> libsensors tries to scan the 'subsystem' entry which in turn must be well
> defined and known. My suspicion is that the reported subsystem may not be
> recognized by libsensors.

> 
> One question is why there is now a device entry, even though this is still as
> virtual as it was before. You'll have to ask the thermal subsystem maintainers
> for an answer.

> I am also concerned about the 'hwmon1' subdirectory underneath hwmon1/device;
> that suggests that hwmon1 may be declared to be a child of itself, which would
> obviously not be a good idea.
> 
> Also, note that the thermal subsystem creates (or may create) sensor attributes
> after registering the hwmon device, which means you can not rely on the udev
> event that comes with the hwmon device creation and assume that all sensor
> attributes exist at that time. I don't currently know how to handle this situation.
> This is not unique, though; the coretemp driver does the same. Just something
> to keep in mind.
> 
> Guenter
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series
  2013-10-21  2:28   ` Zhang Rui
@ 2013-10-21  5:19     ` Guenter Roeck
  2013-10-21 18:49     ` Arnaud Ebalard
  1 sibling, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2013-10-21  5:19 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/20/2013 07:28 PM, Zhang Rui wrote:
> Hi,
>
> On Sun, 2013-10-20 at 12:23 -0700, Guenter Roeck wrote:
>> On 10/20/2013 11:10 AM, Arnaud Ebalard wrote:
>>> Hi,
>>>
>>> With 3.12-rc series, sysfs support for thermal susbsytem (and/or hwmon
>>> one) was modified in such a way that sensors utility (current 3.3.4
>>> version with 3.3.4 version of libsensors from lm-sensors package on
>>> Debian unstable) does not see the temperature sensor anymore on armada
>>> 370 platforms (not tested on others). Additionally, the changes break
>>> existing configurations of fancontrol utility, which prevents the
>>> fan to be regulated correctly w/o recreating an /etc/fancontrol w/
>>> pwmconfig.
>>>
>>> Here is what I have on my Armada 370-based system on a 3.11.5:
>>>
>>> # sensors
>>> g762-i2c-0-3e
>>> Adapter: mv64xxx_i2c adapter
>>> fan1:        2457 RPM  (div = 1)
>>>
>>> armada_thermal-virtual-0
>>> Adapter: Virtual device
>>> temp1:        +45.7?C
>>>
>>> And what I get on 3.12-rc6:
>>>
>>> # sensors
>>> g762-i2c-0-3e
>>> Adapter: mv64xxx_i2c adapter
>>> fan1:        1350 RPM  (div = 1)
>>>
>>>
>>> Monitoring what sensors does w/ strace, I started looking at the changes
>>> to /sys/class/hwmon/hwmon1/:
>>>
>>> On 3.11.5:
>>>
>>> # find /sys/class/hwmon/hwmon1/
>>> /sys/class/hwmon/hwmon1/
>>> /sys/class/hwmon/hwmon1/name
>>> /sys/class/hwmon/hwmon1/subsystem
>>> /sys/class/hwmon/hwmon1/uevent
>>> /sys/class/hwmon/hwmon1/temp1_input
>>>
>>> On 3.12-rc6:
>>>
>>> # find /sys/class/hwmon/hwmon1/
>>> /sys/class/hwmon/hwmon1/
>>> /sys/class/hwmon/hwmon1/name
>>> /sys/class/hwmon/hwmon1/device
>>> /sys/class/hwmon/hwmon1/subsystem
>>> /sys/class/hwmon/hwmon1/uevent
>>> /sys/class/hwmon/hwmon1/temp1_input
>>>
>>> # find /sys/class/hwmon/hwmon1/device/
>>> /sys/class/hwmon/hwmon1/device/
>>> /sys/class/hwmon/hwmon1/device/temp
>>> /sys/class/hwmon/hwmon1/device/type
>>> /sys/class/hwmon/hwmon1/device/hwmon1
>>> /sys/class/hwmon/hwmon1/device/hwmon1/name
>>> /sys/class/hwmon/hwmon1/device/hwmon1/device
>>> /sys/class/hwmon/hwmon1/device/hwmon1/subsystem
>>> /sys/class/hwmon/hwmon1/device/hwmon1/uevent
>>> /sys/class/hwmon/hwmon1/device/hwmon1/temp1_input
>>> /sys/class/hwmon/hwmon1/device/subsystem
>>> /sys/class/hwmon/hwmon1/device/policy
>>> /sys/class/hwmon/hwmon1/device/uevent
>>> /sys/class/hwmon/hwmon1/device/passive
>>>
>>> Is that expected? As for sensors, it *seems* to be bothered to find a
>>> device/ folder in /sys/class/hwmon/hwmon1/ w/o no name entry in it.
>>>
>
> I agree. And it should be caused by this commit.
>
> commit b82715fdd4a5407f56853b24d387d484dd9c3b5b
> Author: Eduardo Valentin <eduardo.valentin@ti.com>
> Date:   Fri Aug 23 17:07:58 2013 -0400
>
>      drivers: thermal: parent virtual hwmon with thermal zone
>
>      When  creating virtual hwmon devices based out of thermal
>      zone devices, the virtual devices won't have parents.
>
>      This patch changes the code so that the parent of virtual
>      hwmon devices is the thermal zone device that they are
>      based of.
>
>      Cc: Zhang Rui <rui.zhang@intel.com>
>      Cc: linux-pm at vger.kernel.org
>      Cc: linux-kernel at vger.kernel.org
>      Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>
>>
>> The 'name' attribute should not be the problem, since there is a 'name'
>> attribute in the /sys/class/hwmon/hwmon1/ directory.
>>
>> Key difference is that there is now a 'device' subdirectory,
>
> Right.
>
>>   which results
>> in different handling by libsensors;
> Oh, I'm not aware of this before.
> There is no such statement in the comments of hwmon_device_register(),
> or anywhere else.
> Could you show me some tutorials about how the sysfs I/F misleads
> libsensors?
>

I looked into the libsensors source code. I don't think there is a tutorial,
or at least I am not aware of one.

Still, I don't entirely understand how the above commit results in what seems
to be recursive parents (or, from looking into the code, how that happens in
the first place).

Guenter

> But anyway, I will revert this patch first.
> Thanks for reporting the problem, Arnaud!
>
> thanks,
> rui
>>   the entry is no longer a virtual entry
>> but is expected to have a real device attached to it. For this device,
>> libsensors tries to scan the 'subsystem' entry which in turn must be well
>> defined and known. My suspicion is that the reported subsystem may not be
>> recognized by libsensors.
>
>>
>> One question is why there is now a device entry, even though this is still as
>> virtual as it was before. You'll have to ask the thermal subsystem maintainers
>> for an answer.
>
>> I am also concerned about the 'hwmon1' subdirectory underneath hwmon1/device;
>> that suggests that hwmon1 may be declared to be a child of itself, which would
>> obviously not be a good idea.
>>
>> Also, note that the thermal subsystem creates (or may create) sensor attributes
>> after registering the hwmon device, which means you can not rely on the udev
>> event that comes with the hwmon device creation and assume that all sensor
>> attributes exist at that time. I don't currently know how to handle this situation.
>> This is not unique, though; the coretemp driver does the same. Just something
>> to keep in mind.
>>
>> Guenter
>>
>
>
>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series
  2013-10-20 18:10 [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series Arnaud Ebalard
  2013-10-20 19:23 ` Guenter Roeck
@ 2013-10-21  7:17 ` Jean Delvare
  2013-10-21 15:16   ` Guenter Roeck
  2013-10-21 18:14   ` Arnaud Ebalard
  1 sibling, 2 replies; 9+ messages in thread
From: Jean Delvare @ 2013-10-21  7:17 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnaud,

On Sun, 20 Oct 2013 20:10:41 +0200, Arnaud Ebalard wrote:
> With 3.12-rc series, sysfs support for thermal susbsytem (and/or hwmon
> one) was modified in such a way that sensors utility (current 3.3.4
> version with 3.3.4 version of libsensors from lm-sensors package on
> Debian unstable) does not see the temperature sensor anymore on armada
> 370 platforms (not tested on others). Additionally, the changes break
> existing configurations of fancontrol utility, which prevents the 
> fan to be regulated correctly w/o recreating an /etc/fancontrol w/
> pwmconfig.
> 
> Here is what I have on my Armada 370-based system on a 3.11.5:
> 
> # sensors
> g762-i2c-0-3e
> Adapter: mv64xxx_i2c adapter
> fan1:        2457 RPM  (div = 1)
> 
> armada_thermal-virtual-0
> Adapter: Virtual device
> temp1:        +45.7?C  
> 
> And what I get on 3.12-rc6:
> 
> # sensors
> g762-i2c-0-3e
> Adapter: mv64xxx_i2c adapter
> fan1:        1350 RPM  (div = 1)
> 
> 
> Monitoring what sensors does w/ strace, I started looking at the changes
> to /sys/class/hwmon/hwmon1/:
> 
> On 3.11.5:
> 
> # find /sys/class/hwmon/hwmon1/
> /sys/class/hwmon/hwmon1/
> /sys/class/hwmon/hwmon1/name
> /sys/class/hwmon/hwmon1/subsystem
> /sys/class/hwmon/hwmon1/uevent
> /sys/class/hwmon/hwmon1/temp1_input
> 
> On 3.12-rc6:
> 
> # find /sys/class/hwmon/hwmon1/
> /sys/class/hwmon/hwmon1/
> /sys/class/hwmon/hwmon1/name
> /sys/class/hwmon/hwmon1/device
> /sys/class/hwmon/hwmon1/subsystem
> /sys/class/hwmon/hwmon1/uevent
> /sys/class/hwmon/hwmon1/temp1_input
> 
> # find /sys/class/hwmon/hwmon1/device/
> /sys/class/hwmon/hwmon1/device/
> /sys/class/hwmon/hwmon1/device/temp
> /sys/class/hwmon/hwmon1/device/type
> /sys/class/hwmon/hwmon1/device/hwmon1
> /sys/class/hwmon/hwmon1/device/hwmon1/name
> /sys/class/hwmon/hwmon1/device/hwmon1/device
> /sys/class/hwmon/hwmon1/device/hwmon1/subsystem
> /sys/class/hwmon/hwmon1/device/hwmon1/uevent
> /sys/class/hwmon/hwmon1/device/hwmon1/temp1_input
> /sys/class/hwmon/hwmon1/device/subsystem
> /sys/class/hwmon/hwmon1/device/policy
> /sys/class/hwmon/hwmon1/device/uevent
> /sys/class/hwmon/hwmon1/device/passive

Can you please share the full output of "strace sensors"? This will
help me understand which exact code paths are taken in libsensors.

> Is that expected? As for sensors, it *seems* to be bothered to find a
> device/ folder in /sys/class/hwmon/hwmon1/ w/o no name entry in it.

It should be able to cope with this just fine. The radeon driver does
exactly this and libsensors has no problem with it. 

-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series
  2013-10-21  7:17 ` Jean Delvare
@ 2013-10-21 15:16   ` Guenter Roeck
  2013-10-22 12:04     ` Jean Delvare
  2013-10-21 18:14   ` Arnaud Ebalard
  1 sibling, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2013-10-21 15:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 21, 2013 at 09:17:39AM +0200, Jean Delvare wrote:
> Hi Arnaud,
> 
> On Sun, 20 Oct 2013 20:10:41 +0200, Arnaud Ebalard wrote:
> > With 3.12-rc series, sysfs support for thermal susbsytem (and/or hwmon
> > one) was modified in such a way that sensors utility (current 3.3.4
> > version with 3.3.4 version of libsensors from lm-sensors package on
> > Debian unstable) does not see the temperature sensor anymore on armada
> > 370 platforms (not tested on others). Additionally, the changes break
> > existing configurations of fancontrol utility, which prevents the 
> > fan to be regulated correctly w/o recreating an /etc/fancontrol w/
> > pwmconfig.
> > 
> > Here is what I have on my Armada 370-based system on a 3.11.5:
> > 
> > # sensors
> > g762-i2c-0-3e
> > Adapter: mv64xxx_i2c adapter
> > fan1:        2457 RPM  (div = 1)
> > 
> > armada_thermal-virtual-0
> > Adapter: Virtual device
> > temp1:        +45.7?C  
> > 
> > And what I get on 3.12-rc6:
> > 
> > # sensors
> > g762-i2c-0-3e
> > Adapter: mv64xxx_i2c adapter
> > fan1:        1350 RPM  (div = 1)
> > 
> > 
> > Monitoring what sensors does w/ strace, I started looking at the changes
> > to /sys/class/hwmon/hwmon1/:
> > 
> > On 3.11.5:
> > 
> > # find /sys/class/hwmon/hwmon1/
> > /sys/class/hwmon/hwmon1/
> > /sys/class/hwmon/hwmon1/name
> > /sys/class/hwmon/hwmon1/subsystem
> > /sys/class/hwmon/hwmon1/uevent
> > /sys/class/hwmon/hwmon1/temp1_input
> > 
> > On 3.12-rc6:
> > 
> > # find /sys/class/hwmon/hwmon1/
> > /sys/class/hwmon/hwmon1/
> > /sys/class/hwmon/hwmon1/name
> > /sys/class/hwmon/hwmon1/device
> > /sys/class/hwmon/hwmon1/subsystem
> > /sys/class/hwmon/hwmon1/uevent
> > /sys/class/hwmon/hwmon1/temp1_input
> > 
> > # find /sys/class/hwmon/hwmon1/device/
> > /sys/class/hwmon/hwmon1/device/
> > /sys/class/hwmon/hwmon1/device/temp
> > /sys/class/hwmon/hwmon1/device/type
> > /sys/class/hwmon/hwmon1/device/hwmon1
> > /sys/class/hwmon/hwmon1/device/hwmon1/name
> > /sys/class/hwmon/hwmon1/device/hwmon1/device
> > /sys/class/hwmon/hwmon1/device/hwmon1/subsystem
> > /sys/class/hwmon/hwmon1/device/hwmon1/uevent
> > /sys/class/hwmon/hwmon1/device/hwmon1/temp1_input
> > /sys/class/hwmon/hwmon1/device/subsystem
> > /sys/class/hwmon/hwmon1/device/policy
> > /sys/class/hwmon/hwmon1/device/uevent
> > /sys/class/hwmon/hwmon1/device/passive
> 
> Can you please share the full output of "strace sensors"? This will
> help me understand which exact code paths are taken in libsensors.
> 
> > Is that expected? As for sensors, it *seems* to be bothered to find a
> > device/ folder in /sys/class/hwmon/hwmon1/ w/o no name entry in it.
> 
> It should be able to cope with this just fine. The radeon driver does
> exactly this and libsensors has no problem with it. 
> 
Hi Jean,

I think it is more likely that the problem is related to parsing the 'subsystem'
link. If I understand the code in lib/sysfs.c correctly, it doesn't recognize
'thermal' (which I think is the subsystem name, but I may be wrong) and ignores
the device as unknown.

Question is if we can come up with some more generic code to handle that case.
Define SENSORS_BUS_TYPE_OTHER and treat it similar to virtual, maybe ?
But then there can be more than one of those, so you would need some means
for enumerating bus and chip numbers, so I don't know if that is feasible.

Guenter

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series
  2013-10-21  7:17 ` Jean Delvare
  2013-10-21 15:16   ` Guenter Roeck
@ 2013-10-21 18:14   ` Arnaud Ebalard
  1 sibling, 0 replies; 9+ messages in thread
From: Arnaud Ebalard @ 2013-10-21 18:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Jean,

Jean Delvare <khali@linux-fr.org> writes:

> Can you please share the full output of "strace sensors"? This will
> help me understand which exact code paths are taken in libsensors.

The 'strace sensors' output on a 3.12-rc6 is below, followed by the
output of the same command on a 3.11.6.

-3.12-rc6--8<--------------------------------------------------------

execve("/usr/bin/sensors", ["sensors"], [/* 13 vars */]) = 0
brk(0)                                  = 0xf63000
uname({sys="Linux", node="mood", ...})  = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6f15000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=24842, ...}) = 0
mmap2(NULL, 24842, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb6ee9000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/usr/lib/arm-linux-gnueabi/libsensors.so.4", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0(\0\1\0\0\0\210\27\0\0004\0\0\0"..., 512) = 512
lseek(3, 53908, SEEK_SET)               = 53908
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 1040) = 1040
lseek(3, 53644, SEEK_SET)               = 53644
read(3, "A)\0\0\0aeabi\0\1\37\0\0\0\0054T\0\6\2\10\1\t\1\22\4\24\1\25\1"..., 42) = 42
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6f14000
fstat64(3, {st_mode=S_IFREG|0644, st_size=54948, ...}) = 0
mmap2(NULL, 86952, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xb6ed3000
mprotect(0xb6ee0000, 28672, PROT_NONE)  = 0
mmap2(0xb6ee7000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0xc) = 0xb6ee7000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/arm-linux-gnueabi/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0(\0\1\0\0\0h\202\1\0004\0\0\0"..., 512) = 512
lseek(3, 1240084, SEEK_SET)             = 1240084
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 2840) = 2840
lseek(3, 1236484, SEEK_SET)             = 1236484
read(3, "A)\0\0\0aeabi\0\1\37\0\0\0\0054T\0\6\2\10\1\t\1\22\4\23\1\24\1"..., 42) = 42
fstat64(3, {st_mode=S_IFREG|0755, st_size=1242924, ...}) = 0
mmap2(NULL, 1279368, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xb6d9a000
mprotect(0xb6ec5000, 32768, PROT_NONE)  = 0
mmap2(0xb6ecd000, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x12b) = 0xb6ecd000
mmap2(0xb6ed0000, 9608, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb6ed0000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/arm-linux-gnueabi/libm.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0(\0\1\0\0\0000<\0\0004\0\0\0"..., 512) = 512
lseek(3, 659912, SEEK_SET)              = 659912
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 1160) = 1160
lseek(3, 659588, SEEK_SET)              = 659588
read(3, "A)\0\0\0aeabi\0\1\37\0\0\0\0054T\0\6\2\10\1\t\1\22\4\23\1\24\1"..., 42) = 42
fstat64(3, {st_mode=S_IFREG|0644, st_size=661072, ...}) = 0
mmap2(NULL, 692364, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xb6cf0000
mprotect(0xb6d91000, 28672, PROT_NONE)  = 0
mmap2(0xb6d98000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0xa0) = 0xb6d98000
close(3)                                = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6f13000
set_tls(0xb6f134c0, 0xb6f13b98, 0xb6f18050, 0xb6f134c0, 0xb6f18050) = 0
mprotect(0xb6ecd000, 8192, PROT_READ)   = 0
mprotect(0xb6d98000, 4096, PROT_READ)   = 0
mprotect(0xb6ee7000, 4096, PROT_READ)   = 0
mprotect(0x14000, 4096, PROT_READ)      = 0
mprotect(0xb6f17000, 4096, PROT_READ)   = 0
munmap(0xb6ee9000, 24842)               = 0
brk(0)                                  = 0xf63000
brk(0xf84000)                           = 0xf84000
open("/usr/lib/locale/locale-archive", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=1607632, ...}) = 0
mmap2(NULL, 1607632, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb6b67000
close(3)                                = 0
statfs("/sys", {f_type="SYSFS_MAGIC", f_bsize=4096, f_blocks=0, f_bfree=0, f_bavail=0, f_files=0, f_ffree=0, f_fsid={0, 0}, f_namelen=255, f_frsize=4096}) = 0
openat(AT_FDCWD, "/sys/class/i2c-adapter", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 3
fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
getdents(3, /* 3 entries */, 32768)     = 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) = 0xb6f12000
read(4, "mv64xxx_i2c adapter\n", 4096)  = 20
close(4)                                = 0
munmap(0xb6f12000, 4096)                = 0
getdents(3, /* 0 entries */, 32768)     = 0
close(3)                                = 0
openat(AT_FDCWD, "/sys/class/hwmon", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 3
getdents(3, /* 4 entries */, 32768)     = 72
readlink("/sys/class/hwmon/hwmon0/device", "../../../0-003e"..., 254) = 15
open("/sys/class/hwmon/hwmon0/name", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/sys/class/hwmon/hwmon0/device/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) = 0xb6f12000
read(4, "g762\n", 4096)                 = 5
close(4)                                = 0
munmap(0xb6f12000, 4096)                = 0
readlink("/sys/class/hwmon/hwmon0/device/subsystem", "../../../../../../bus/i2c", 254) = 25
open("/sys/class/i2c-adapter/i2c-0/device/name", O_RDONLY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/sys/class/hwmon/hwmon0/device", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 4
brk(0xfab000)                           = 0xfab000
getdents(4, /* 17 entries */, 32768)    = 356
stat64("/sys/class/hwmon/hwmon0/device/fan1_pulses", {st_mode=S_IFREG|0644, st_size=4096, ...}) = 0
stat64("/sys/class/hwmon/hwmon0/device/fan1_div", {st_mode=S_IFREG|0644, st_size=4096, ...}) = 0
stat64("/sys/class/hwmon/hwmon0/device/fan1_alarm", {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
stat64("/sys/class/hwmon/hwmon0/device/fan1_fault", {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
stat64("/sys/class/hwmon/hwmon0/device/fan1_input", {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
getdents(4, /* 0 entries */, 32768)     = 0
close(4)                                = 0
readlink("/sys/class/hwmon/hwmon1/device", "../../thermal_zone0"..., 254) = 19
open("/sys/class/hwmon/hwmon1/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) = 0xb6f12000
read(4, "armada_thermal\n", 4096)       = 15
close(4)                                = 0
munmap(0xb6f12000, 4096)                = 0
readlink("/sys/class/hwmon/hwmon1/device/subsystem", "../../../../class/thermal", 254) = 25
open("/sys/class/hwmon/hwmon1/device/name", O_RDONLY) = -1 ENOENT (No such file or directory)
getdents(3, /* 0 entries */, 32768)     = 0
close(3)                                = 0
open("/etc/sensors3.conf", O_RDONLY)    = 3
ioctl(3, SNDCTL_TMR_TIMEBASE or TCGETS, 0xbebc4af4) = -1 ENOTTY (Inappropriate ioctl for device)
fstat64(3, {st_mode=S_IFREG|0644, st_size=10344, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6f12000
read(3, "# libsensors configuration file\n"..., 8192) = 8192
read(3, " label in4 \"+12V\"\n    label in5 "..., 8192) = 2152
read(3, "", 4096)                       = 0
read(3, "", 8192)                       = 0
ioctl(3, SNDCTL_TMR_TIMEBASE or TCGETS, 0xbebc3b04) = -1 ENOTTY (Inappropriate ioctl for device)
close(3)                                = 0
munmap(0xb6f12000, 4096)                = 0
openat(AT_FDCWD, "/etc/sensors.d", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 3
getdents(3, /* 3 entries */, 32768)     = 56
getdents(3, /* 0 entries */, 32768)     = 0
close(3)                                = 0
open("/usr/lib/arm-linux-gnueabi/gconv/gconv-modules.cache", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=26260, ...}) = 0
mmap2(NULL, 26260, PROT_READ, MAP_SHARED, 3, 0) = 0xb6ee9000
close(3)                                = 0
open("/usr/lib/arm-linux-gnueabi/gconv/ISO8859-1.so", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0(\0\1\0\0\0\354\3\0\0004\0\0\0"..., 512) = 512
lseek(3, 8548, SEEK_SET)                = 8548
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 1080) = 1080
lseek(3, 8248, SEEK_SET)                = 8248
read(3, "A)\0\0\0aeabi\0\1\37\0\0\0\0054T\0\6\2\10\1\t\1\22\4\23\1\24\1"..., 42) = 42
fstat64(3, {st_mode=S_IFREG|0644, st_size=9628, ...}) = 0
mmap2(NULL, 41020, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xb6b5c000
mprotect(0xb6b5e000, 28672, PROT_NONE)  = 0
mmap2(0xb6b65000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1) = 0xb6b65000
close(3)                                = 0
mprotect(0xb6b65000, 4096, PROT_READ)   = 0
fstat64(1, {st_mode=S_IFREG|0644, st_size=9216, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6f12000
open("/sys/class/hwmon/hwmon0/device/fan1_label", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/sys/class/hwmon/hwmon0/device/fan1_label", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/sys/class/hwmon/hwmon0/device/fan1_fault", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6f11000
read(3, "0\n", 4096)                    = 2
close(3)                                = 0
munmap(0xb6f11000, 4096)                = 0
open("/sys/class/hwmon/hwmon0/device/fan1_input", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6f11000
read(3, "1536\n", 4096)                 = 5
close(3)                                = 0
munmap(0xb6f11000, 4096)                = 0
open("/sys/class/hwmon/hwmon0/device/fan1_div", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6f11000
read(3, "1\n", 4096)                    = 2
close(3)                                = 0
munmap(0xb6f11000, 4096)                = 0
open("/sys/class/hwmon/hwmon0/device/fan1_alarm", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6f11000
read(3, "0\n", 4096)                    = 2
close(3)                                = 0
munmap(0xb6f11000, 4096)                = 0
write(1, "g762-i2c-0-3e\nAdapter: mv64xxx_i"..., 77g762-i2c-0-3e
Adapter: mv64xxx_i2c adapter
fan1:        1536 RPM  (div = 1)

) = 77
exit_group(0)                           = ?





-3.11.6--8<--------------------------------------------------------

execve("/usr/bin/sensors", ["sensors"], [/* 13 vars */]) = 0
brk(0)                                  = 0x1ae9000
uname({sys="Linux", node="mood", ...})  = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe7000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=24842, ...}) = 0
mmap2(NULL, 24842, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb6fbb000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/usr/lib/arm-linux-gnueabi/libsensors.so.4", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0(\0\1\0\0\0\210\27\0\0004\0\0\0"..., 512) = 512
lseek(3, 53908, SEEK_SET)               = 53908
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 1040) = 1040
lseek(3, 53644, SEEK_SET)               = 53644
read(3, "A)\0\0\0aeabi\0\1\37\0\0\0\0054T\0\6\2\10\1\t\1\22\4\24\1\25\1"..., 42) = 42
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe6000
fstat64(3, {st_mode=S_IFREG|0644, st_size=54948, ...}) = 0
mmap2(NULL, 86952, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xb6fa5000
mprotect(0xb6fb2000, 28672, PROT_NONE)  = 0
mmap2(0xb6fb9000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0xc) = 0xb6fb9000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/arm-linux-gnueabi/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0(\0\1\0\0\0h\202\1\0004\0\0\0"..., 512) = 512
lseek(3, 1240084, SEEK_SET)             = 1240084
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 2840) = 2840
lseek(3, 1236484, SEEK_SET)             = 1236484
read(3, "A)\0\0\0aeabi\0\1\37\0\0\0\0054T\0\6\2\10\1\t\1\22\4\23\1\24\1"..., 42) = 42
fstat64(3, {st_mode=S_IFREG|0755, st_size=1242924, ...}) = 0
mmap2(NULL, 1279368, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xb6e6c000
mprotect(0xb6f97000, 32768, PROT_NONE)  = 0
mmap2(0xb6f9f000, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x12b) = 0xb6f9f000
mmap2(0xb6fa2000, 9608, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb6fa2000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/arm-linux-gnueabi/libm.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0(\0\1\0\0\0000<\0\0004\0\0\0"..., 512) = 512
lseek(3, 659912, SEEK_SET)              = 659912
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 1160) = 1160
lseek(3, 659588, SEEK_SET)              = 659588
read(3, "A)\0\0\0aeabi\0\1\37\0\0\0\0054T\0\6\2\10\1\t\1\22\4\23\1\24\1"..., 42) = 42
fstat64(3, {st_mode=S_IFREG|0644, st_size=661072, ...}) = 0
mmap2(NULL, 692364, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xb6dc2000
mprotect(0xb6e63000, 28672, PROT_NONE)  = 0
mmap2(0xb6e6a000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0xa0) = 0xb6e6a000
close(3)                                = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe5000
set_tls(0xb6fe54c0, 0xb6fe5b98, 0xb6fea050, 0xb6fe54c0, 0xb6fea050) = 0
mprotect(0xb6f9f000, 8192, PROT_READ)   = 0
mprotect(0xb6e6a000, 4096, PROT_READ)   = 0
mprotect(0xb6fb9000, 4096, PROT_READ)   = 0
mprotect(0x14000, 4096, PROT_READ)      = 0
mprotect(0xb6fe9000, 4096, PROT_READ)   = 0
munmap(0xb6fbb000, 24842)               = 0
brk(0)                                  = 0x1ae9000
brk(0x1b0a000)                          = 0x1b0a000
open("/usr/lib/locale/locale-archive", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=1607632, ...}) = 0
mmap2(NULL, 1607632, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb6c39000
close(3)                                = 0
statfs("/sys", {f_type="SYSFS_MAGIC", f_bsize=4096, f_blocks=0, f_bfree=0, f_bavail=0, f_files=0, f_ffree=0, f_fsid={0, 0}, f_namelen=255, f_frsize=4096}) = 0
openat(AT_FDCWD, "/sys/class/i2c-adapter", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 3
fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
getdents(3, /* 3 entries */, 32768)     = 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) = 0xb6fe4000
read(4, "mv64xxx_i2c adapter\n", 4096)  = 20
close(4)                                = 0
munmap(0xb6fe4000, 4096)                = 0
getdents(3, /* 0 entries */, 32768)     = 0
close(3)                                = 0
openat(AT_FDCWD, "/sys/class/hwmon", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 3
getdents(3, /* 4 entries */, 32768)     = 72
readlink("/sys/class/hwmon/hwmon0/device", "../../../0-003e"..., 254) = 15
open("/sys/class/hwmon/hwmon0/name", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/sys/class/hwmon/hwmon0/device/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) = 0xb6fe4000
read(4, "g762\n", 4096)                 = 5
close(4)                                = 0
munmap(0xb6fe4000, 4096)                = 0
readlink("/sys/class/hwmon/hwmon0/device/subsystem", "../../../../../../bus/i2c", 254) = 25
open("/sys/class/i2c-adapter/i2c-0/device/name", O_RDONLY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/sys/class/hwmon/hwmon0/device", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 4
brk(0x1b31000)                          = 0x1b31000
getdents(4, /* 17 entries */, 32768)    = 356
stat64("/sys/class/hwmon/hwmon0/device/fan1_pulses", {st_mode=S_IFREG|0644, st_size=4096, ...}) = 0
stat64("/sys/class/hwmon/hwmon0/device/fan1_div", {st_mode=S_IFREG|0644, st_size=4096, ...}) = 0
stat64("/sys/class/hwmon/hwmon0/device/fan1_alarm", {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
stat64("/sys/class/hwmon/hwmon0/device/fan1_fault", {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
stat64("/sys/class/hwmon/hwmon0/device/fan1_input", {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
getdents(4, /* 0 entries */, 32768)     = 0
close(4)                                = 0
readlink("/sys/class/hwmon/hwmon1/device", 0xbeb3596c, 254) = -1 ENOENT (No such file or directory)
open("/sys/class/hwmon/hwmon1/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) = 0xb6fe4000
read(4, "armada_thermal\n", 4096)       = 15
close(4)                                = 0
munmap(0xb6fe4000, 4096)                = 0
openat(AT_FDCWD, "/sys/class/hwmon/hwmon1", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 4
getdents(4, /* 6 entries */, 32768)     = 116
stat64("/sys/class/hwmon/hwmon1/temp1_input", {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
getdents(4, /* 0 entries */, 32768)     = 0
close(4)                                = 0
getdents(3, /* 0 entries */, 32768)     = 0
close(3)                                = 0
open("/etc/sensors3.conf", O_RDONLY)    = 3
ioctl(3, SNDCTL_TMR_TIMEBASE or TCGETS, 0xbeb35af4) = -1 ENOTTY (Inappropriate ioctl for device)
fstat64(3, {st_mode=S_IFREG|0644, st_size=10344, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe4000
read(3, "# libsensors configuration file\n"..., 8192) = 8192
read(3, " label in4 \"+12V\"\n    label in5 "..., 8192) = 2152
read(3, "", 4096)                       = 0
read(3, "", 8192)                       = 0
ioctl(3, SNDCTL_TMR_TIMEBASE or TCGETS, 0xbeb34b04) = -1 ENOTTY (Inappropriate ioctl for device)
close(3)                                = 0
munmap(0xb6fe4000, 4096)                = 0
openat(AT_FDCWD, "/etc/sensors.d", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 3
getdents(3, /* 3 entries */, 32768)     = 56
getdents(3, /* 0 entries */, 32768)     = 0
close(3)                                = 0
open("/usr/lib/arm-linux-gnueabi/gconv/gconv-modules.cache", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=26260, ...}) = 0
mmap2(NULL, 26260, PROT_READ, MAP_SHARED, 3, 0) = 0xb6fbb000
close(3)                                = 0
open("/usr/lib/arm-linux-gnueabi/gconv/ISO8859-1.so", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0(\0\1\0\0\0\354\3\0\0004\0\0\0"..., 512) = 512
lseek(3, 8548, SEEK_SET)                = 8548
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 1080) = 1080
lseek(3, 8248, SEEK_SET)                = 8248
read(3, "A)\0\0\0aeabi\0\1\37\0\0\0\0054T\0\6\2\10\1\t\1\22\4\23\1\24\1"..., 42) = 42
fstat64(3, {st_mode=S_IFREG|0644, st_size=9628, ...}) = 0
mmap2(NULL, 41020, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xb6c2e000
mprotect(0xb6c30000, 28672, PROT_NONE)  = 0
mmap2(0xb6c37000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1) = 0xb6c37000
close(3)                                = 0
mprotect(0xb6c37000, 4096, PROT_READ)   = 0
fstat64(1, {st_mode=S_IFREG|0644, st_size=9385, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe4000
open("/sys/class/hwmon/hwmon0/device/fan1_label", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/sys/class/hwmon/hwmon0/device/fan1_label", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/sys/class/hwmon/hwmon0/device/fan1_fault", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe3000
read(3, "0\n", 4096)                    = 2
close(3)                                = 0
munmap(0xb6fe3000, 4096)                = 0
open("/sys/class/hwmon/hwmon0/device/fan1_input", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe3000
read(3, "3510\n", 4096)                 = 5
close(3)                                = 0
munmap(0xb6fe3000, 4096)                = 0
open("/sys/class/hwmon/hwmon0/device/fan1_div", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe3000
read(3, "1\n", 4096)                    = 2
close(3)                                = 0
munmap(0xb6fe3000, 4096)                = 0
open("/sys/class/hwmon/hwmon0/device/fan1_alarm", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe3000
read(3, "0\n", 4096)                    = 2
close(3)                                = 0
munmap(0xb6fe3000, 4096)                = 0
open("/sys/class/hwmon/hwmon1/temp1_label", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/sys/class/hwmon/hwmon1/temp1_label", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/sys/class/hwmon/hwmon1/temp1_input", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe3000
read(3, "42893\n", 4096)                = 6
close(3)                                = 0
munmap(0xb6fe3000, 4096)                = 0
open("/sys/class/hwmon/hwmon1/temp1_input", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe3000
read(3, "42893\n", 4096)                = 6
close(3)                                = 0
munmap(0xb6fe3000, 4096)                = 0
brk(0x1b13000)                          = 0x1b13000
write(1, "g762-i2c-0-3e\nAdapter: mv64xxx_i"..., 152g762-i2c-0-3e
Adapter: mv64xxx_i2c adapter
fan1:        3510 RPM  (div = 1)

armada_thermal-virtual-0
Adapter: Virtual device
temp1:        +42.9?C  

) = 152
exit_group(0)                           = ?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series
  2013-10-21  2:28   ` Zhang Rui
  2013-10-21  5:19     ` Guenter Roeck
@ 2013-10-21 18:49     ` Arnaud Ebalard
  1 sibling, 0 replies; 9+ messages in thread
From: Arnaud Ebalard @ 2013-10-21 18:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

Zhang Rui <rui.zhang@intel.com> writes:

>> > Is that expected? As for sensors, it *seems* to be bothered to find a
>> > device/ folder in /sys/class/hwmon/hwmon1/ w/o no name entry in it.
>> >
>
> I agree. And it should be caused by this commit.
>
> commit b82715fdd4a5407f56853b24d387d484dd9c3b5b
> Author: Eduardo Valentin <eduardo.valentin@ti.com>
> Date:   Fri Aug 23 17:07:58 2013 -0400
>
>     drivers: thermal: parent virtual hwmon with thermal zone

FWIW, the one-liner reverting this commit in your tree (6ddcb7e635
Revert "drivers: thermal: parent virtual hwmon with thermal zone")
does indeed makes sensors and fancontrol happy again.

Cheers,

a+

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series
  2013-10-21 15:16   ` Guenter Roeck
@ 2013-10-22 12:04     ` Jean Delvare
  0 siblings, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2013-10-22 12:04 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Guenter,

On Mon, 21 Oct 2013 08:16:16 -0700, Guenter Roeck wrote:
> I think it is more likely that the problem is related to parsing the 'subsystem'
> link. If I understand the code in lib/sysfs.c correctly, it doesn't recognize
> 'thermal' (which I think is the subsystem name, but I may be wrong) and ignores
> the device as unknown.

Yes, you are right, that's exactly what is happening here.

> Question is if we can come up with some more generic code to handle that case.
> Define SENSORS_BUS_TYPE_OTHER and treat it similar to virtual, maybe ?
> But then there can be more than one of those, so you would need some means
> for enumerating bus and chip numbers, so I don't know if that is feasible.

The whole point of identifying the subsystem is because we want to
assign a unique, stable identifier to each chip, and the naming scheme
depends on the subsystem. If we start treating all unsupported
subsystems as "other", we simply won't be able to give these devices
unique names. Developers and users are unlikely to care and report the
problem to us if libsensors shows them their "foo-other-0" device. This
is why libsensors ignores these devices: developers and users will
report about the missing device and we can add proper support to
libsensors. That way we can get the naming scheme right immediately
instead of later when this will break running configurations.

Commit b82715fdd4a5407f56853b24d387d484dd9c3b5b is simply wrong, so it
should be reverted. It assumes that there's a 1:1 mapping between
thermal zones and hwmon devices, which is not the case. See this
comment near the top of thermal_hwmon.c:

/* thermal zone devices with the same type share one hwmon device */

So associating the hwmon device with the first registered thermal zone
of its kind is arbitrary and asymmetric and makes no sense. I don't
know what the goal of that change was, but that will have to be
achieved in a different way.

Note that merging all thermal zones of one kind into a single hwmon
device is exactly what makes it possible to treat it as a virtual
device and still guarantee uniqueness of the identifier (e.g.
acpitz-virtual-0.) It has the drawback of leaving the input ordering
stability up to the kernel, but back then it was not considered an
issue.

If we now want to have one hwmon device per thermal zone, then the
kernel must provide a way for libsensors to give each of them a unique
and stable identifier. Naming scheme is relatively free.

Note that I am not necessarily in favor of such a change, as 1* it
could lead to a huge number of hwmon devices, making the output of
"sensors" harder to read, and 2* the change could break existing
configurations. So if anyone wants to change this, a really really good
reason must be provided, i.e. you will have to demonstrate that the
benefits outweigh the pain.

Hope it helps,
-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-10-22 12:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-20 18:10 [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series Arnaud Ebalard
2013-10-20 19:23 ` Guenter Roeck
2013-10-21  2:28   ` Zhang Rui
2013-10-21  5:19     ` Guenter Roeck
2013-10-21 18:49     ` Arnaud Ebalard
2013-10-21  7:17 ` Jean Delvare
2013-10-21 15:16   ` Guenter Roeck
2013-10-22 12:04     ` Jean Delvare
2013-10-21 18:14   ` Arnaud Ebalard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).