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 22:19:10 -0700 Message-ID: <5264B94E.9090805@roeck-us.net> References: <87ppqzolsu.fsf@natisbad.org> <52642DAF.8080301@roeck-us.net> <1382322502.2410.12.camel@rzhang1-mobl4> 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]:58994 "EHLO mail.active-venture.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752490Ab3JUFTS (ORCPT ); Mon, 21 Oct 2013 01:19:18 -0400 In-Reply-To: <1382322502.2410.12.camel@rzhang1-mobl4> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Zhang Rui Cc: Arnaud Ebalard , Eduardo Valentin , Jean Delvare , 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 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 hw= mon >>> 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 arm= ada >>> 370 platforms (not tested on others). Additionally, the changes bre= ak >>> 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 ch= anges >>> 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 > 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 > Cc: linux-pm@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Eduardo Valentin > >> >> The 'name' attribute should not be the problem, since there is a 'na= me' >> 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 tuto= rial, 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 happe= ns 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 devic= e, >> libsensors tries to scan the 'subsystem' entry which in turn must be= well >> defined and known. My suspicion is that the reported subsystem may n= ot 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 m= aintainers >> for an answer. > >> I am also concerned about the 'hwmon1' subdirectory underneath hwmon= 1/device; >> that suggests that hwmon1 may be declared to be a child of itself, w= hich 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 >> > > > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guenter Roeck Date: Mon, 21 Oct 2013 05:19:10 +0000 Subject: Re: [lm-sensors] [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series Message-Id: <5264B94E.9090805@roeck-us.net> List-Id: References: <87ppqzolsu.fsf@natisbad.org> <52642DAF.8080301@roeck-us.net> <1382322502.2410.12.camel@rzhang1-mobl4> In-Reply-To: <1382322502.2410.12.camel@rzhang1-mobl4> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Zhang Rui Cc: Arnaud Ebalard , Eduardo Valentin , Jean Delvare , linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org, linux-arm-kernel@lists.infradead.org, Andrew Lunn , Gregory Clement , Sebastian Hesselbarth , Jason Cooper T24gMTAvMjAvMjAxMyAwNzoyOCBQTSwgWmhhbmcgUnVpIHdyb3RlOgo+IEhpLAo+Cj4gT24gU3Vu LCAyMDEzLTEwLTIwIGF0IDEyOjIzIC0wNzAwLCBHdWVudGVyIFJvZWNrIHdyb3RlOgo+PiBPbiAx MC8yMC8yMDEzIDExOjEwIEFNLCBBcm5hdWQgRWJhbGFyZCB3cm90ZToKPj4+IEhpLAo+Pj4KPj4+ IFdpdGggMy4xMi1yYyBzZXJpZXMsIHN5c2ZzIHN1cHBvcnQgZm9yIHRoZXJtYWwgc3VzYnN5dGVt IChhbmQvb3IgaHdtb24KPj4+IG9uZSkgd2FzIG1vZGlmaWVkIGluIHN1Y2ggYSB3YXkgdGhhdCBz ZW5zb3JzIHV0aWxpdHkgKGN1cnJlbnQgMy4zLjQKPj4+IHZlcnNpb24gd2l0aCAzLjMuNCB2ZXJz aW9uIG9mIGxpYnNlbnNvcnMgZnJvbSBsbS1zZW5zb3JzIHBhY2thZ2Ugb24KPj4+IERlYmlhbiB1 bnN0YWJsZSkgZG9lcyBub3Qgc2VlIHRoZSB0ZW1wZXJhdHVyZSBzZW5zb3IgYW55bW9yZSBvbiBh cm1hZGEKPj4+IDM3MCBwbGF0Zm9ybXMgKG5vdCB0ZXN0ZWQgb24gb3RoZXJzKS4gQWRkaXRpb25h bGx5LCB0aGUgY2hhbmdlcyBicmVhawo+Pj4gZXhpc3RpbmcgY29uZmlndXJhdGlvbnMgb2YgZmFu Y29udHJvbCB1dGlsaXR5LCB3aGljaCBwcmV2ZW50cyB0aGUKPj4+IGZhbiB0byBiZSByZWd1bGF0 ZWQgY29ycmVjdGx5IHcvbyByZWNyZWF0aW5nIGFuIC9ldGMvZmFuY29udHJvbCB3Lwo+Pj4gcHdt Y29uZmlnLgo+Pj4KPj4+IEhlcmUgaXMgd2hhdCBJIGhhdmUgb24gbXkgQXJtYWRhIDM3MC1iYXNl ZCBzeXN0ZW0gb24gYSAzLjExLjU6Cj4+Pgo+Pj4gIyBzZW5zb3JzCj4+PiBnNzYyLWkyYy0wLTNl Cj4+PiBBZGFwdGVyOiBtdjY0eHh4X2kyYyBhZGFwdGVyCj4+PiBmYW4xOiAgICAgICAgMjQ1NyBS UE0gIChkaXYgPSAxKQo+Pj4KPj4+IGFybWFkYV90aGVybWFsLXZpcnR1YWwtMAo+Pj4gQWRhcHRl cjogVmlydHVhbCBkZXZpY2UKPj4+IHRlbXAxOiAgICAgICAgKzQ1LjfCsEMKPj4+Cj4+PiBBbmQg d2hhdCBJIGdldCBvbiAzLjEyLXJjNjoKPj4+Cj4+PiAjIHNlbnNvcnMKPj4+IGc3NjItaTJjLTAt M2UKPj4+IEFkYXB0ZXI6IG12NjR4eHhfaTJjIGFkYXB0ZXIKPj4+IGZhbjE6ICAgICAgICAxMzUw IFJQTSAgKGRpdiA9IDEpCj4+Pgo+Pj4KPj4+IE1vbml0b3Jpbmcgd2hhdCBzZW5zb3JzIGRvZXMg dy8gc3RyYWNlLCBJIHN0YXJ0ZWQgbG9va2luZyBhdCB0aGUgY2hhbmdlcwo+Pj4gdG8gL3N5cy9j bGFzcy9od21vbi9od21vbjEvOgo+Pj4KPj4+IE9uIDMuMTEuNToKPj4+Cj4+PiAjIGZpbmQgL3N5 cy9jbGFzcy9od21vbi9od21vbjEvCj4+PiAvc3lzL2NsYXNzL2h3bW9uL2h3bW9uMS8KPj4+IC9z eXMvY2xhc3MvaHdtb24vaHdtb24xL25hbWUKPj4+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24xL3N1 YnN5c3RlbQo+Pj4gL3N5cy9jbGFzcy9od21vbi9od21vbjEvdWV2ZW50Cj4+PiAvc3lzL2NsYXNz L2h3bW9uL2h3bW9uMS90ZW1wMV9pbnB1dAo+Pj4KPj4+IE9uIDMuMTItcmM2Ogo+Pj4KPj4+ICMg ZmluZCAvc3lzL2NsYXNzL2h3bW9uL2h3bW9uMS8KPj4+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24x Lwo+Pj4gL3N5cy9jbGFzcy9od21vbi9od21vbjEvbmFtZQo+Pj4gL3N5cy9jbGFzcy9od21vbi9o d21vbjEvZGV2aWNlCj4+PiAvc3lzL2NsYXNzL2h3bW9uL2h3bW9uMS9zdWJzeXN0ZW0KPj4+IC9z eXMvY2xhc3MvaHdtb24vaHdtb24xL3VldmVudAo+Pj4gL3N5cy9jbGFzcy9od21vbi9od21vbjEv dGVtcDFfaW5wdXQKPj4+Cj4+PiAjIGZpbmQgL3N5cy9jbGFzcy9od21vbi9od21vbjEvZGV2aWNl Lwo+Pj4gL3N5cy9jbGFzcy9od21vbi9od21vbjEvZGV2aWNlLwo+Pj4gL3N5cy9jbGFzcy9od21v bi9od21vbjEvZGV2aWNlL3RlbXAKPj4+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24xL2RldmljZS90 eXBlCj4+PiAvc3lzL2NsYXNzL2h3bW9uL2h3bW9uMS9kZXZpY2UvaHdtb24xCj4+PiAvc3lzL2Ns YXNzL2h3bW9uL2h3bW9uMS9kZXZpY2UvaHdtb24xL25hbWUKPj4+IC9zeXMvY2xhc3MvaHdtb24v aHdtb24xL2RldmljZS9od21vbjEvZGV2aWNlCj4+PiAvc3lzL2NsYXNzL2h3bW9uL2h3bW9uMS9k ZXZpY2UvaHdtb24xL3N1YnN5c3RlbQo+Pj4gL3N5cy9jbGFzcy9od21vbi9od21vbjEvZGV2aWNl L2h3bW9uMS91ZXZlbnQKPj4+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24xL2RldmljZS9od21vbjEv dGVtcDFfaW5wdXQKPj4+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24xL2RldmljZS9zdWJzeXN0ZW0K Pj4+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24xL2RldmljZS9wb2xpY3kKPj4+IC9zeXMvY2xhc3Mv aHdtb24vaHdtb24xL2RldmljZS91ZXZlbnQKPj4+IC9zeXMvY2xhc3MvaHdtb24vaHdtb24xL2Rl dmljZS9wYXNzaXZlCj4+Pgo+Pj4gSXMgdGhhdCBleHBlY3RlZD8gQXMgZm9yIHNlbnNvcnMsIGl0 ICpzZWVtcyogdG8gYmUgYm90aGVyZWQgdG8gZmluZCBhCj4+PiBkZXZpY2UvIGZvbGRlciBpbiAv c3lzL2NsYXNzL2h3bW9uL2h3bW9uMS8gdy9vIG5vIG5hbWUgZW50cnkgaW4gaXQuCj4+Pgo+Cj4g SSBhZ3JlZS4gQW5kIGl0IHNob3VsZCBiZSBjYXVzZWQgYnkgdGhpcyBjb21taXQuCj4KPiBjb21t aXQgYjgyNzE1ZmRkNGE1NDA3ZjU2ODUzYjI0ZDM4N2Q0ODRkZDljM2I1Ygo+IEF1dGhvcjogRWR1 YXJkbyBWYWxlbnRpbiA8ZWR1YXJkby52YWxlbnRpbkB0aS5jb20+Cj4gRGF0ZTogICBGcmkgQXVn IDIzIDE3OjA3OjU4IDIwMTMgLTA0MDAKPgo+ICAgICAgZHJpdmVyczogdGhlcm1hbDogcGFyZW50 IHZpcnR1YWwgaHdtb24gd2l0aCB0aGVybWFsIHpvbmUKPgo+ICAgICAgV2hlbiAgY3JlYXRpbmcg dmlydHVhbCBod21vbiBkZXZpY2VzIGJhc2VkIG91dCBvZiB0aGVybWFsCj4gICAgICB6b25lIGRl dmljZXMsIHRoZSB2aXJ0dWFsIGRldmljZXMgd29uJ3QgaGF2ZSBwYXJlbnRzLgo+Cj4gICAgICBU aGlzIHBhdGNoIGNoYW5nZXMgdGhlIGNvZGUgc28gdGhhdCB0aGUgcGFyZW50IG9mIHZpcnR1YWwK PiAgICAgIGh3bW9uIGRldmljZXMgaXMgdGhlIHRoZXJtYWwgem9uZSBkZXZpY2UgdGhhdCB0aGV5 IGFyZQo+ICAgICAgYmFzZWQgb2YuCj4KPiAgICAgIENjOiBaaGFuZyBSdWkgPHJ1aS56aGFuZ0Bp bnRlbC5jb20+Cj4gICAgICBDYzogbGludXgtcG1Admdlci5rZXJuZWwub3JnCj4gICAgICBDYzog bGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZwo+ICAgICAgU2lnbmVkLW9mZi1ieTogRWR1YXJk byBWYWxlbnRpbiA8ZWR1YXJkby52YWxlbnRpbkB0aS5jb20+Cj4KPj4KPj4gVGhlICduYW1lJyBh dHRyaWJ1dGUgc2hvdWxkIG5vdCBiZSB0aGUgcHJvYmxlbSwgc2luY2UgdGhlcmUgaXMgYSAnbmFt ZScKPj4gYXR0cmlidXRlIGluIHRoZSAvc3lzL2NsYXNzL2h3bW9uL2h3bW9uMS8gZGlyZWN0b3J5 Lgo+Pgo+PiBLZXkgZGlmZmVyZW5jZSBpcyB0aGF0IHRoZXJlIGlzIG5vdyBhICdkZXZpY2UnIHN1 YmRpcmVjdG9yeSwKPgo+IFJpZ2h0Lgo+Cj4+ICAgd2hpY2ggcmVzdWx0cwo+PiBpbiBkaWZmZXJl bnQgaGFuZGxpbmcgYnkgbGlic2Vuc29yczsKPiBPaCwgSSdtIG5vdCBhd2FyZSBvZiB0aGlzIGJl Zm9yZS4KPiBUaGVyZSBpcyBubyBzdWNoIHN0YXRlbWVudCBpbiB0aGUgY29tbWVudHMgb2YgaHdt b25fZGV2aWNlX3JlZ2lzdGVyKCksCj4gb3IgYW55d2hlcmUgZWxzZS4KPiBDb3VsZCB5b3Ugc2hv dyBtZSBzb21lIHR1dG9yaWFscyBhYm91dCBob3cgdGhlIHN5c2ZzIEkvRiBtaXNsZWFkcwo+IGxp YnNlbnNvcnM/Cj4KCkkgbG9va2VkIGludG8gdGhlIGxpYnNlbnNvcnMgc291cmNlIGNvZGUuIEkg ZG9uJ3QgdGhpbmsgdGhlcmUgaXMgYSB0dXRvcmlhbCwKb3IgYXQgbGVhc3QgSSBhbSBub3QgYXdh cmUgb2Ygb25lLgoKU3RpbGwsIEkgZG9uJ3QgZW50aXJlbHkgdW5kZXJzdGFuZCBob3cgdGhlIGFi b3ZlIGNvbW1pdCByZXN1bHRzIGluIHdoYXQgc2VlbXMKdG8gYmUgcmVjdXJzaXZlIHBhcmVudHMg KG9yLCBmcm9tIGxvb2tpbmcgaW50byB0aGUgY29kZSwgaG93IHRoYXQgaGFwcGVucyBpbgp0aGUg Zmlyc3QgcGxhY2UpLgoKR3VlbnRlcgoKPiBCdXQgYW55d2F5LCBJIHdpbGwgcmV2ZXJ0IHRoaXMg cGF0Y2ggZmlyc3QuCj4gVGhhbmtzIGZvciByZXBvcnRpbmcgdGhlIHByb2JsZW0sIEFybmF1ZCEK Pgo+IHRoYW5rcywKPiBydWkKPj4gICB0aGUgZW50cnkgaXMgbm8gbG9uZ2VyIGEgdmlydHVhbCBl bnRyeQo+PiBidXQgaXMgZXhwZWN0ZWQgdG8gaGF2ZSBhIHJlYWwgZGV2aWNlIGF0dGFjaGVkIHRv IGl0LiBGb3IgdGhpcyBkZXZpY2UsCj4+IGxpYnNlbnNvcnMgdHJpZXMgdG8gc2NhbiB0aGUgJ3N1 YnN5c3RlbScgZW50cnkgd2hpY2ggaW4gdHVybiBtdXN0IGJlIHdlbGwKPj4gZGVmaW5lZCBhbmQg a25vd24uIE15IHN1c3BpY2lvbiBpcyB0aGF0IHRoZSByZXBvcnRlZCBzdWJzeXN0ZW0gbWF5IG5v dCBiZQo+PiByZWNvZ25pemVkIGJ5IGxpYnNlbnNvcnMuCj4KPj4KPj4gT25lIHF1ZXN0aW9uIGlz IHdoeSB0aGVyZSBpcyBub3cgYSBkZXZpY2UgZW50cnksIGV2ZW4gdGhvdWdoIHRoaXMgaXMgc3Rp bGwgYXMKPj4gdmlydHVhbCBhcyBpdCB3YXMgYmVmb3JlLiBZb3UnbGwgaGF2ZSB0byBhc2sgdGhl IHRoZXJtYWwgc3Vic3lzdGVtIG1haW50YWluZXJzCj4+IGZvciBhbiBhbnN3ZXIuCj4KPj4gSSBh bSBhbHNvIGNvbmNlcm5lZCBhYm91dCB0aGUgJ2h3bW9uMScgc3ViZGlyZWN0b3J5IHVuZGVybmVh dGggaHdtb24xL2RldmljZTsKPj4gdGhhdCBzdWdnZXN0cyB0aGF0IGh3bW9uMSBtYXkgYmUgZGVj bGFyZWQgdG8gYmUgYSBjaGlsZCBvZiBpdHNlbGYsIHdoaWNoIHdvdWxkCj4+IG9idmlvdXNseSBu b3QgYmUgYSBnb29kIGlkZWEuCj4+Cj4+IEFsc28sIG5vdGUgdGhhdCB0aGUgdGhlcm1hbCBzdWJz eXN0ZW0gY3JlYXRlcyAob3IgbWF5IGNyZWF0ZSkgc2Vuc29yIGF0dHJpYnV0ZXMKPj4gYWZ0ZXIg cmVnaXN0ZXJpbmcgdGhlIGh3bW9uIGRldmljZSwgd2hpY2ggbWVhbnMgeW91IGNhbiBub3QgcmVs eSBvbiB0aGUgdWRldgo+PiBldmVudCB0aGF0IGNvbWVzIHdpdGggdGhlIGh3bW9uIGRldmljZSBj cmVhdGlvbiBhbmQgYXNzdW1lIHRoYXQgYWxsIHNlbnNvcgo+PiBhdHRyaWJ1dGVzIGV4aXN0IGF0 IHRoYXQgdGltZS4gSSBkb24ndCBjdXJyZW50bHkga25vdyBob3cgdG8gaGFuZGxlIHRoaXMgc2l0 dWF0aW9uLgo+PiBUaGlzIGlzIG5vdCB1bmlxdWUsIHRob3VnaDsgdGhlIGNvcmV0ZW1wIGRyaXZl ciBkb2VzIHRoZSBzYW1lLiBKdXN0IHNvbWV0aGluZwo+PiB0byBrZWVwIGluIG1pbmQuCj4+Cj4+ IEd1ZW50ZXIKPj4KPgo+Cj4KPgoKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fCmxtLXNlbnNvcnMgbWFpbGluZyBsaXN0CmxtLXNlbnNvcnNAbG0tc2Vuc29y cy5vcmcKaHR0cDovL2xpc3RzLmxtLXNlbnNvcnMub3JnL21haWxtYW4vbGlzdGluZm8vbG0tc2Vu c29ycw= From mboxrd@z Thu Jan 1 00:00:00 1970 From: linux@roeck-us.net (Guenter Roeck) Date: Sun, 20 Oct 2013 22:19:10 -0700 Subject: [REGRESSION?] sensors and fancontrol not seeing armada_thermal on 3.12-rc series In-Reply-To: <1382322502.2410.12.camel@rzhang1-mobl4> References: <87ppqzolsu.fsf@natisbad.org> <52642DAF.8080301@roeck-us.net> <1382322502.2410.12.camel@rzhang1-mobl4> Message-ID: <5264B94E.9090805@roeck-us.net> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 > 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 > Cc: linux-pm at vger.kernel.org > Cc: linux-kernel at vger.kernel.org > Signed-off-by: Eduardo Valentin > >> >> 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 >> > > > >