From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guenter Roeck Subject: Re: [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series Date: Sun, 20 Oct 2013 12:23:27 -0700 Message-ID: <52642DAF.8080301@roeck-us.net> References: <87ppqzolsu.fsf@natisbad.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mail.active-venture.com ([67.228.131.205]:53878 "EHLO mail.active-venture.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751450Ab3JTTXf (ORCPT ); Sun, 20 Oct 2013 15:23:35 -0400 In-Reply-To: <87ppqzolsu.fsf@natisbad.org> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Arnaud Ebalard , Eduardo Valentin , Zhang Rui , Jean Delvare Cc: linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org, linux-arm-kernel@lists.infradead.org, Andrew Lunn , Gregory Clement , Sebastian Hesselbarth , Jason Cooper On 10/20/2013 11:10 AM, Arnaud Ebalard wrote: > Hi, > > With 3.12-rc series, sysfs support for thermal susbsytem (and/or hwmo= n > 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 armad= a > 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 =3D 1) > > armada_thermal-virtual-0 > Adapter: Virtual device > temp1: +45.7=C2=B0C > > And what I get on 3.12-rc6: > > # sensors > g762-i2c-0-3e > Adapter: mv64xxx_i2c adapter > fan1: 1350 RPM (div =3D 1) > > > Monitoring what sensors does w/ strace, I started looking at the chan= ges > 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 resu= lts in different handling by libsensors; the entry is no longer a virtual e= ntry 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 we= ll 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 st= ill as virtual as it was before. You'll have to ask the thermal subsystem main= tainers for an answer. I am also concerned about the 'hwmon1' subdirectory underneath hwmon1/d= evice; that suggests that hwmon1 may be declared to be a child of itself, whic= h would obviously not be a good idea. Also, note that the thermal subsystem creates (or may create) sensor at= tributes 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 sen= sor attributes exist at that time. I don't currently know how to handle thi= s situation. This is not unique, though; the coretemp driver does the same. Just som= ething to keep in mind. Guenter From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guenter Roeck Date: Sun, 20 Oct 2013 19:23:27 +0000 Subject: Re: [lm-sensors] [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series Message-Id: <52642DAF.8080301@roeck-us.net> List-Id: References: <87ppqzolsu.fsf@natisbad.org> In-Reply-To: <87ppqzolsu.fsf@natisbad.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Arnaud Ebalard , Eduardo Valentin , Zhang Rui , Jean Delvare Cc: linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org, linux-arm-kernel@lists.infradead.org, Andrew Lunn , Gregory Clement , Sebastian Hesselbarth , Jason Cooper T24gMTAvMjAvMjAxMyAxMToxMCBBTSwgQXJuYXVkIEViYWxhcmQgd3JvdGU6Cj4gSGksCj4KPiBX aXRoIDMuMTItcmMgc2VyaWVzLCBzeXNmcyBzdXBwb3J0IGZvciB0aGVybWFsIHN1c2JzeXRlbSAo YW5kL29yIGh3bW9uCj4gb25lKSB3YXMgbW9kaWZpZWQgaW4gc3VjaCBhIHdheSB0aGF0IHNlbnNv cnMgdXRpbGl0eSAoY3VycmVudCAzLjMuNAo+IHZlcnNpb24gd2l0aCAzLjMuNCB2ZXJzaW9uIG9m IGxpYnNlbnNvcnMgZnJvbSBsbS1zZW5zb3JzIHBhY2thZ2Ugb24KPiBEZWJpYW4gdW5zdGFibGUp IGRvZXMgbm90IHNlZSB0aGUgdGVtcGVyYXR1cmUgc2Vuc29yIGFueW1vcmUgb24gYXJtYWRhCj4g MzcwIHBsYXRmb3JtcyAobm90IHRlc3RlZCBvbiBvdGhlcnMpLiBBZGRpdGlvbmFsbHksIHRoZSBj aGFuZ2VzIGJyZWFrCj4gZXhpc3RpbmcgY29uZmlndXJhdGlvbnMgb2YgZmFuY29udHJvbCB1dGls aXR5LCB3aGljaCBwcmV2ZW50cyB0aGUKPiBmYW4gdG8gYmUgcmVndWxhdGVkIGNvcnJlY3RseSB3 L28gcmVjcmVhdGluZyBhbiAvZXRjL2ZhbmNvbnRyb2wgdy8KPiBwd21jb25maWcuCj4KPiBIZXJl IGlzIHdoYXQgSSBoYXZlIG9uIG15IEFybWFkYSAzNzAtYmFzZWQgc3lzdGVtIG9uIGEgMy4xMS41 Ogo+Cj4gIyBzZW5zb3JzCj4gZzc2Mi1pMmMtMC0zZQo+IEFkYXB0ZXI6IG12NjR4eHhfaTJjIGFk YXB0ZXIKPiBmYW4xOiAgICAgICAgMjQ1NyBSUE0gIChkaXYgPSAxKQo+Cj4gYXJtYWRhX3RoZXJt YWwtdmlydHVhbC0wCj4gQWRhcHRlcjogVmlydHVhbCBkZXZpY2UKPiB0ZW1wMTogICAgICAgICs0 NS43wrBDCj4KPiBBbmQgd2hhdCBJIGdldCBvbiAzLjEyLXJjNjoKPgo+ICMgc2Vuc29ycwo+IGc3 NjItaTJjLTAtM2UKPiBBZGFwdGVyOiBtdjY0eHh4X2kyYyBhZGFwdGVyCj4gZmFuMTogICAgICAg IDEzNTAgUlBNICAoZGl2ID0gMSkKPgo+Cj4gTW9uaXRvcmluZyB3aGF0IHNlbnNvcnMgZG9lcyB3 LyBzdHJhY2UsIEkgc3RhcnRlZCBsb29raW5nIGF0IHRoZSBjaGFuZ2VzCj4gdG8gL3N5cy9jbGFz cy9od21vbi9od21vbjEvOgo+Cj4gT24gMy4xMS41Ogo+Cj4gIyBmaW5kIC9zeXMvY2xhc3MvaHdt b24vaHdtb24xLwo+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24xLwo+IC9zeXMvY2xhc3MvaHdtb24v aHdtb24xL25hbWUKPiAvc3lzL2NsYXNzL2h3bW9uL2h3bW9uMS9zdWJzeXN0ZW0KPiAvc3lzL2Ns YXNzL2h3bW9uL2h3bW9uMS91ZXZlbnQKPiAvc3lzL2NsYXNzL2h3bW9uL2h3bW9uMS90ZW1wMV9p bnB1dAo+Cj4gT24gMy4xMi1yYzY6Cj4KPiAjIGZpbmQgL3N5cy9jbGFzcy9od21vbi9od21vbjEv Cj4gL3N5cy9jbGFzcy9od21vbi9od21vbjEvCj4gL3N5cy9jbGFzcy9od21vbi9od21vbjEvbmFt ZQo+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24xL2RldmljZQo+IC9zeXMvY2xhc3MvaHdtb24vaHdt b24xL3N1YnN5c3RlbQo+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24xL3VldmVudAo+IC9zeXMvY2xh c3MvaHdtb24vaHdtb24xL3RlbXAxX2lucHV0Cj4KPiAjIGZpbmQgL3N5cy9jbGFzcy9od21vbi9o d21vbjEvZGV2aWNlLwo+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24xL2RldmljZS8KPiAvc3lzL2Ns YXNzL2h3bW9uL2h3bW9uMS9kZXZpY2UvdGVtcAo+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24xL2Rl dmljZS90eXBlCj4gL3N5cy9jbGFzcy9od21vbi9od21vbjEvZGV2aWNlL2h3bW9uMQo+IC9zeXMv Y2xhc3MvaHdtb24vaHdtb24xL2RldmljZS9od21vbjEvbmFtZQo+IC9zeXMvY2xhc3MvaHdtb24v aHdtb24xL2RldmljZS9od21vbjEvZGV2aWNlCj4gL3N5cy9jbGFzcy9od21vbi9od21vbjEvZGV2 aWNlL2h3bW9uMS9zdWJzeXN0ZW0KPiAvc3lzL2NsYXNzL2h3bW9uL2h3bW9uMS9kZXZpY2UvaHdt b24xL3VldmVudAo+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24xL2RldmljZS9od21vbjEvdGVtcDFf aW5wdXQKPiAvc3lzL2NsYXNzL2h3bW9uL2h3bW9uMS9kZXZpY2Uvc3Vic3lzdGVtCj4gL3N5cy9j bGFzcy9od21vbi9od21vbjEvZGV2aWNlL3BvbGljeQo+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24x L2RldmljZS91ZXZlbnQKPiAvc3lzL2NsYXNzL2h3bW9uL2h3bW9uMS9kZXZpY2UvcGFzc2l2ZQo+ Cj4gSXMgdGhhdCBleHBlY3RlZD8gQXMgZm9yIHNlbnNvcnMsIGl0ICpzZWVtcyogdG8gYmUgYm90 aGVyZWQgdG8gZmluZCBhCj4gZGV2aWNlLyBmb2xkZXIgaW4gL3N5cy9jbGFzcy9od21vbi9od21v bjEvIHcvbyBubyBuYW1lIGVudHJ5IGluIGl0Lgo+CgpUaGUgJ25hbWUnIGF0dHJpYnV0ZSBzaG91 bGQgbm90IGJlIHRoZSBwcm9ibGVtLCBzaW5jZSB0aGVyZSBpcyBhICduYW1lJwphdHRyaWJ1dGUg aW4gdGhlIC9zeXMvY2xhc3MvaHdtb24vaHdtb24xLyBkaXJlY3RvcnkuCgpLZXkgZGlmZmVyZW5j ZSBpcyB0aGF0IHRoZXJlIGlzIG5vdyBhICdkZXZpY2UnIHN1YmRpcmVjdG9yeSwgd2hpY2ggcmVz dWx0cwppbiBkaWZmZXJlbnQgaGFuZGxpbmcgYnkgbGlic2Vuc29yczsgdGhlIGVudHJ5IGlzIG5v IGxvbmdlciBhIHZpcnR1YWwgZW50cnkKYnV0IGlzIGV4cGVjdGVkIHRvIGhhdmUgYSByZWFsIGRl dmljZSBhdHRhY2hlZCB0byBpdC4gRm9yIHRoaXMgZGV2aWNlLApsaWJzZW5zb3JzIHRyaWVzIHRv IHNjYW4gdGhlICdzdWJzeXN0ZW0nIGVudHJ5IHdoaWNoIGluIHR1cm4gbXVzdCBiZSB3ZWxsCmRl ZmluZWQgYW5kIGtub3duLiBNeSBzdXNwaWNpb24gaXMgdGhhdCB0aGUgcmVwb3J0ZWQgc3Vic3lz dGVtIG1heSBub3QgYmUKcmVjb2duaXplZCBieSBsaWJzZW5zb3JzLgoKT25lIHF1ZXN0aW9uIGlz IHdoeSB0aGVyZSBpcyBub3cgYSBkZXZpY2UgZW50cnksIGV2ZW4gdGhvdWdoIHRoaXMgaXMgc3Rp bGwgYXMKdmlydHVhbCBhcyBpdCB3YXMgYmVmb3JlLiBZb3UnbGwgaGF2ZSB0byBhc2sgdGhlIHRo ZXJtYWwgc3Vic3lzdGVtIG1haW50YWluZXJzCmZvciBhbiBhbnN3ZXIuCgpJIGFtIGFsc28gY29u Y2VybmVkIGFib3V0IHRoZSAnaHdtb24xJyBzdWJkaXJlY3RvcnkgdW5kZXJuZWF0aCBod21vbjEv ZGV2aWNlOwp0aGF0IHN1Z2dlc3RzIHRoYXQgaHdtb24xIG1heSBiZSBkZWNsYXJlZCB0byBiZSBh IGNoaWxkIG9mIGl0c2VsZiwgd2hpY2ggd291bGQKb2J2aW91c2x5IG5vdCBiZSBhIGdvb2QgaWRl YS4KCkFsc28sIG5vdGUgdGhhdCB0aGUgdGhlcm1hbCBzdWJzeXN0ZW0gY3JlYXRlcyAob3IgbWF5 IGNyZWF0ZSkgc2Vuc29yIGF0dHJpYnV0ZXMKYWZ0ZXIgcmVnaXN0ZXJpbmcgdGhlIGh3bW9uIGRl dmljZSwgd2hpY2ggbWVhbnMgeW91IGNhbiBub3QgcmVseSBvbiB0aGUgdWRldgpldmVudCB0aGF0 IGNvbWVzIHdpdGggdGhlIGh3bW9uIGRldmljZSBjcmVhdGlvbiBhbmQgYXNzdW1lIHRoYXQgYWxs IHNlbnNvcgphdHRyaWJ1dGVzIGV4aXN0IGF0IHRoYXQgdGltZS4gSSBkb24ndCBjdXJyZW50bHkg a25vdyBob3cgdG8gaGFuZGxlIHRoaXMgc2l0dWF0aW9uLgpUaGlzIGlzIG5vdCB1bmlxdWUsIHRo b3VnaDsgdGhlIGNvcmV0ZW1wIGRyaXZlciBkb2VzIHRoZSBzYW1lLiBKdXN0IHNvbWV0aGluZwp0 byBrZWVwIGluIG1pbmQuCgpHdWVudGVyCgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KbG0tc2Vuc29ycyBtYWlsaW5nIGxpc3QKbG0tc2Vuc29yc0BsbS1z ZW5zb3JzLm9yZwpodHRwOi8vbGlzdHMubG0tc2Vuc29ycy5vcmcvbWFpbG1hbi9saXN0aW5mby9s bS1zZW5zb3Jz From mboxrd@z Thu Jan 1 00:00:00 1970 From: linux@roeck-us.net (Guenter Roeck) Date: Sun, 20 Oct 2013 12:23:27 -0700 Subject: [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series In-Reply-To: <87ppqzolsu.fsf@natisbad.org> References: <87ppqzolsu.fsf@natisbad.org> Message-ID: <52642DAF.8080301@roeck-us.net> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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