From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cedric Le Goater Date: Wed, 08 Apr 2015 13:33:03 +0000 Subject: Re: [lm-sensors] [PATCH v3] hwmon: (ibmpowernv) pretty print labels Message-Id: <55252E0F.8080200@fr.ibm.com> List-Id: References: <1428476427-23311-1-git-send-email-clg@fr.ibm.com> <55252C9A.4050505@roeck-us.net> In-Reply-To: <55252C9A.4050505@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Guenter Roeck , lm-sensors@lm-sensors.org Cc: Stewart Smith , Neelesh Gupta , skiboot@lists.ozlabs.org, linuxppc-dev@lists.ozlabs.org, Jean Delvare T24gMDQvMDgvMjAxNSAwMzoyNiBQTSwgR3VlbnRlciBSb2VjayB3cm90ZToKPiBPbiAwNC8wOC8y MDE1IDEyOjAwIEFNLCBDw6lkcmljIExlIEdvYXRlciB3cm90ZToKPj4gVGhlIG5ldyBPUEFMIGRl dmljZSB0cmVlIGFkZHMgYSBmZXcgcHJvcGVydGllcyB3aGljaCBjYW4gYmUgdXNlZCB0byBhZGQK Pj4gZXh0cmEgaW5mb3JtYXRpb24gb24gdGhlIHNlbnNvciBsYWJlbC4KPj4KPj4gSW4gdGhlIGNh c2Ugb2YgYSBjcHUgY29yZSBzZW5zb3IsIHRoZSBmaXJtd2FyZSBleHBvc2VzIHRoZSBwaHlzaWNh bAo+PiBpZGVudGlmaWVyIG9mIHRoZSBjb3JlIGluIHRoZSAiaWJtLHBpciIgcHJvcGVydHkuIFRo ZSBkcml2ZXIKPj4gdHJhbnNsYXRlcyB0aGlzIGlkZW50aWZpZXIgaW4gYSBsaW51eCBjcHUgbnVt YmVyIGFuZCBwcmludHMgb3V0IGEKPj4gcmFuZ2UgY29ycmVzcG9uZGluZyB0byB0aGUgaGFyZHdh cmUgdGhyZWFkcyBvZiB0aGUgY29yZSAoYXMgdGhleQo+PiBzaGFyZSB0aGUgc2FtZSBzZW5zb3Ip Lgo+Pgo+PiBUaGUgbnVtYmVyaW5nIGdpdmVzIGEgaGludCBvbiB0aGUgbG9jYWxpemF0aW9uIG9m IHRoZSBjb3JlIGluIHRoZQo+PiBzeXN0ZW0gKHdoaWNoIHNvY2tldCwgd2hpY2ggY2hpcCkuCj4+ Cj4+IFNpZ25lZC1vZmYtYnk6IEPDqWRyaWMgTGUgR29hdGVyIDxjbGdAZnIuaWJtLmNvbT4KPj4g LS0tCj4+Cj4+ICAgQ2hhbmdlcyBzaW5jZSB2MjoKPj4KPj4gICAtIGZpeCBib2d1cyBsb2dpY2Fs IGNwdSByZXRyaWV2YWwKPj4gICAtIHVzZSAndGhyZWFkc19wZXJfY29yZScgdG8gcHJpbnQgb3V0 IGNwdSByYW5nZQo+Pgo+PiAgIENoYW5nZXMgc2luY2UgdjE6Cj4+Cj4+ICAgLSBjaGVjayBjcHUg dmFsaWRpdHkgYmVmb3JlIHByaW50aW5nIG91dCB0aGUgYXR0cmlidXRlIGxhYmVsLgo+PiAgICAg aWYgaW52YWxpZCwgdXNlIGEgInBoeSIgcHJlZml4IHRvIGRpc3Rpbmd1aXNoIGEgbGludXggY3B1 Cj4+ICAgICBudW1iZXIgZnJvbSBhIHBoeXNpY2FsIGNwdSBudW1iZXIuCj4+Cj4+ICAgZHJpdmVy cy9od21vbi9pYm1wb3dlcm52LmMgfCAgIDQzICsrKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKysrKysrKysKPj4gICAxIGZpbGUgY2hhbmdlZCwgNDMgaW5zZXJ0aW9ucygrKQo+Pgo+ PiBJbmRleDogbGludXguZ2l0L2RyaXZlcnMvaHdtb24vaWJtcG93ZXJudi5jCj4+ID09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KPj4gLS0tIGxpbnV4LmdpdC5vcmlnL2RyaXZlcnMvaHdtb24vaWJtcG93ZXJudi5jCj4+ICsr KyBsaW51eC5naXQvZHJpdmVycy9od21vbi9pYm1wb3dlcm52LmMKPj4gQEAgLTMwLDYgKzMwLDcg QEAKPj4gICAjaW5jbHVkZSA8bGludXgvcGxhdGZvcm1fZGV2aWNlLmg+Cj4+ICAgI2luY2x1ZGUg PGFzbS9vcGFsLmg+Cj4+ICAgI2luY2x1ZGUgPGxpbnV4L2Vyci5oPgo+PiArI2luY2x1ZGUgPGFz bS9jcHV0aHJlYWRzLmg+Cj4+Cj4+ICAgI2RlZmluZSBNQVhfQVRUUl9MRU4gICAgMzIKPj4gICAj ZGVmaW5lIE1BWF9MQUJFTF9MRU4gICAgNjQKPj4gQEAgLTExMCwxMiArMTExLDU0IEBAIHN0YXRp YyBzc2l6ZV90IHNob3dfbGFiZWwoc3RydWN0IGRldmljZQo+PiAgICAgICByZXR1cm4gc3ByaW50 ZihidWYsICIlc1xuIiwgc2RhdGEtPmxhYmVsKTsKPj4gICB9Cj4+Cj4+ICtzdGF0aWMgaW50IF9f aW5pdCBnZXRfbG9naWNhbF9jcHUodW5zaWduZWQgaW50IGh3Y3B1KQo+PiArewo+PiArICAgIGlu dCBjcHU7Cj4+ICsKPj4gKyAgICBmb3JfZWFjaF9wb3NzaWJsZV9jcHUoY3B1KQo+PiArICAgICAg ICBpZiAoZ2V0X2hhcmRfc21wX3Byb2Nlc3Nvcl9pZChjcHUpID09IGh3Y3B1KQo+PiArICAgICAg ICAgICAgcmV0dXJuIGNwdTsKPj4gKwo+PiArICAgIHByX2VycigiJXM6IGNvdWxkIG5vdCBmaW5k IGEgY3B1IHdpdGggcGh5c2ljYWwgaWQgMHgleFxuIiwKPj4gKyAgICAgICAgICAgX19mdW5jX18s IGh3Y3B1KTsKPiAKPiBJIGxpa2UgbW92aW5nIHRoaXMgaW50byBhIGZ1bmN0aW9uLCBidXQgSSBy ZWFsbHkgZGlzbGlrZSB0aGlzIGVycm9yCj4gbWVzc2FnZS4gSWYgdGhlIGRldmljZXRyZWUgZGF0 YSBpcyB3cm9uZy9iYWQsIHRoZSBsb2cgYW5kIHRoZSBjb25zb2xlCj4gd2lsbCBiZSBjbG9nZ2Vk IHdpdGggdGhhdCBtZXNzYWdlLiBBbmQgdGhlIHVzZXIgd2lsbCBub3QgYmUgYWJsZQo+IHRvIGRv IGFueXRoaW5nIGFib3V0IGl0LgoKeWVzLiBJdCBpcyBub3Qgc3VwZXIgdXNlZnVsIGFueXdheSBh bmQgaXQgaXMgcmVkdW5kYW50IHdpdGggdGhlICJwaHkiIApwcmVmaXggYmVsb3cuIERvIHlvdSB3 YW50IG1lIHRvIGtpbGwgaXQgaW4gYSB2NCA/IAoKVGhhbmtzLAoKQy4gCgo+IAo+IEd1ZW50ZXIK PiAKPj4gKyAgICByZXR1cm4gLUVOT0VOVDsKPj4gK30KPj4gKwo+PiAgIHN0YXRpYyB2b2lkIF9f aW5pdCBtYWtlX3NlbnNvcl9sYWJlbChzdHJ1Y3QgZGV2aWNlX25vZGUgKm5wLAo+PiAgICAgICAg ICAgICAgIHN0cnVjdCBzZW5zb3JfZGF0YSAqc2RhdGEsIGNvbnN0IGNoYXIgKmxhYmVsKQo+PiAg IHsKPj4gKyAgICB1MzIgaWQ7Cj4+ICAgICAgIHNpemVfdCBuOwo+Pgo+PiAgICAgICBuID0gc25w cmludGYoc2RhdGEtPmxhYmVsLCBzaXplb2Yoc2RhdGEtPmxhYmVsKSwgIiVzIiwgbGFiZWwpOwo+ PiArCj4+ICsgICAgLyoKPj4gKyAgICAgKiBDb3JlIHRlbXAgcHJldHR5IHByaW50Cj4+ICsgICAg ICovCj4+ICsgICAgaWYgKCFvZl9wcm9wZXJ0eV9yZWFkX3UzMihucCwgImlibSxwaXIiLCAmaWQp KSB7Cj4+ICsgICAgICAgIGludCBjcHVpZCA9IGdldF9sb2dpY2FsX2NwdShpZCk7Cj4+ICsKPj4g KyAgICAgICAgaWYgKGNwdWlkID49IDApCj4+ICsgICAgICAgICAgICAvKgo+PiArICAgICAgICAg ICAgICogVGhlIGRpZ2l0YWwgdGhlcm1hbCBzZW5zb3JzIGFyZSBhc3NvY2lhdGVkCj4+ICsgICAg ICAgICAgICAgKiB3aXRoIGEgY29yZS4gTGV0J3MgcHJpbnQgb3V0IHRoZSByYW5nZSBvZgo+PiAr ICAgICAgICAgICAgICogY3B1IGlkcyBjb3JyZXNwb25kaW5nIHRvIHRoZSBoYXJkd2FyZQo+PiAr ICAgICAgICAgICAgICogdGhyZWFkcyBvZiB0aGUgY29yZS4KPj4gKyAgICAgICAgICAgICAqLwo+ PiArICAgICAgICAgICAgbiArPSBzbnByaW50ZihzZGF0YS0+bGFiZWwgKyBuLAo+PiArICAgICAg ICAgICAgICAgICAgICAgIHNpemVvZihzZGF0YS0+bGFiZWwpIC0gbiwgIiAlZC0lZCIsCj4+ICsg ICAgICAgICAgICAgICAgICAgICAgY3B1aWQsIGNwdWlkICsgdGhyZWFkc19wZXJfY29yZSAtIDEp Owo+PiArICAgICAgICBlbHNlCj4+ICsgICAgICAgICAgICBuICs9IHNucHJpbnRmKHNkYXRhLT5s YWJlbCArIG4sCj4+ICsgICAgICAgICAgICAgICAgICAgICAgc2l6ZW9mKHNkYXRhLT5sYWJlbCkg LSBuLCAiIHBoeSVkIiwgaWQpOwo+PiArICAgIH0KPj4gKwo+PiArICAgIC8qCj4+ICsgICAgICog TWVtYnVmZmVyIHByZXR0eSBwcmludAo+PiArICAgICAqLwo+PiArICAgIGlmICghb2ZfcHJvcGVy dHlfcmVhZF91MzIobnAsICJpYm0sY2hpcC1pZCIsICZpZCkpCj4+ICsgICAgICAgIG4gKz0gc25w cmludGYoc2RhdGEtPmxhYmVsICsgbiwgc2l6ZW9mKHNkYXRhLT5sYWJlbCkgLSBuLAo+PiArICAg ICAgICAgICAgICAgICAgIiAlZCIsIGlkICYgMHhmZmZmKTsKPj4gICB9Cj4+Cj4+ICAgc3RhdGlj IGludCBnZXRfc2Vuc29yX2luZGV4X2F0dHIoY29uc3QgY2hhciAqbmFtZSwgdTMyICppbmRleCwK Pj4KPj4KPiAKCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f XwpsbS1zZW5zb3JzIG1haWxpbmcgbGlzdApsbS1zZW5zb3JzQGxtLXNlbnNvcnMub3JnCmh0dHA6 Ly9saXN0cy5sbS1zZW5zb3JzLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xtLXNlbnNvcnM From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from e06smtp10.uk.ibm.com (e06smtp10.uk.ibm.com [195.75.94.106]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id C10FA1A057D for ; Wed, 8 Apr 2015 23:33:15 +1000 (AEST) Received: from /spool/local by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 8 Apr 2015 14:33:10 +0100 Message-ID: <55252E0F.8080200@fr.ibm.com> Date: Wed, 08 Apr 2015 15:33:03 +0200 From: Cedric Le Goater MIME-Version: 1.0 To: Guenter Roeck , lm-sensors@lm-sensors.org Subject: Re: [PATCH v3] hwmon: (ibmpowernv) pretty print labels References: <1428476427-23311-1-git-send-email-clg@fr.ibm.com> <55252C9A.4050505@roeck-us.net> In-Reply-To: <55252C9A.4050505@roeck-us.net> Content-Type: text/plain; charset=utf-8 Cc: Stewart Smith , Neelesh Gupta , skiboot@lists.ozlabs.org, linuxppc-dev@lists.ozlabs.org, Jean Delvare List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 04/08/2015 03:26 PM, Guenter Roeck wrote: > On 04/08/2015 12:00 AM, Cédric Le Goater wrote: >> The new OPAL device tree adds a few properties which can be used to add >> extra information on the sensor label. >> >> In the case of a cpu core sensor, the firmware exposes the physical >> identifier of the core in the "ibm,pir" property. The driver >> translates this identifier in a linux cpu number and prints out a >> range corresponding to the hardware threads of the core (as they >> share the same sensor). >> >> The numbering gives a hint on the localization of the core in the >> system (which socket, which chip). >> >> Signed-off-by: Cédric Le Goater >> --- >> >> Changes since v2: >> >> - fix bogus logical cpu retrieval >> - use 'threads_per_core' to print out cpu range >> >> Changes since v1: >> >> - check cpu validity before printing out the attribute label. >> if invalid, use a "phy" prefix to distinguish a linux cpu >> number from a physical cpu number. >> >> drivers/hwmon/ibmpowernv.c | 43 +++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 43 insertions(+) >> >> Index: linux.git/drivers/hwmon/ibmpowernv.c >> =================================================================== >> --- linux.git.orig/drivers/hwmon/ibmpowernv.c >> +++ linux.git/drivers/hwmon/ibmpowernv.c >> @@ -30,6 +30,7 @@ >> #include >> #include >> #include >> +#include >> >> #define MAX_ATTR_LEN 32 >> #define MAX_LABEL_LEN 64 >> @@ -110,12 +111,54 @@ static ssize_t show_label(struct device >> return sprintf(buf, "%s\n", sdata->label); >> } >> >> +static int __init get_logical_cpu(unsigned int hwcpu) >> +{ >> + int cpu; >> + >> + for_each_possible_cpu(cpu) >> + if (get_hard_smp_processor_id(cpu) == hwcpu) >> + return cpu; >> + >> + pr_err("%s: could not find a cpu with physical id 0x%x\n", >> + __func__, hwcpu); > > I like moving this into a function, but I really dislike this error > message. If the devicetree data is wrong/bad, the log and the console > will be clogged with that message. And the user will not be able > to do anything about it. yes. It is not super useful anyway and it is redundant with the "phy" prefix below. Do you want me to kill it in a v4 ? Thanks, C. > > Guenter > >> + return -ENOENT; >> +} >> + >> static void __init make_sensor_label(struct device_node *np, >> struct sensor_data *sdata, const char *label) >> { >> + u32 id; >> size_t n; >> >> n = snprintf(sdata->label, sizeof(sdata->label), "%s", label); >> + >> + /* >> + * Core temp pretty print >> + */ >> + if (!of_property_read_u32(np, "ibm,pir", &id)) { >> + int cpuid = get_logical_cpu(id); >> + >> + if (cpuid >= 0) >> + /* >> + * The digital thermal sensors are associated >> + * with a core. Let's print out the range of >> + * cpu ids corresponding to the hardware >> + * threads of the core. >> + */ >> + n += snprintf(sdata->label + n, >> + sizeof(sdata->label) - n, " %d-%d", >> + cpuid, cpuid + threads_per_core - 1); >> + else >> + n += snprintf(sdata->label + n, >> + sizeof(sdata->label) - n, " phy%d", id); >> + } >> + >> + /* >> + * Membuffer pretty print >> + */ >> + if (!of_property_read_u32(np, "ibm,chip-id", &id)) >> + n += snprintf(sdata->label + n, sizeof(sdata->label) - n, >> + " %d", id & 0xffff); >> } >> >> static int get_sensor_index_attr(const char *name, u32 *index, >> >> >