Hi everyone, Attached is a driver to export sensor readings from power meters that are found in several IBM x86 systems. At the moment, the hwmon sysfs documentation doesn't mention any naming conventions for sensors that measure Watts, so I am proposing that they be called "powerX_input" in a fashion similar to temperature/rpm/current sensors. If that is agreeable to everyone, I'll post a follow-up patch to amend the documentation. The patch should apply against 2.6.23-rc3 and has been tested on the x3550, x3650, x3655, x3755 and HS20 blades that support it. As far as I know, those are the only systems in existence that have this interface. --- ibm_pex: Driver to export IBM PowerExecutive power meter sensors. Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> --- drivers/hwmon/Kconfig | 12 + drivers/hwmon/Makefile | 1=20 drivers/hwmon/ibmpex.c | 615 ++++++++++++++++++++++++++++++++++++++++++++= ++++ 3 files changed, 628 insertions(+), 0 deletions(-) diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 555f470..41ffa2e 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -275,6 +275,18 @@ config SENSORS_CORETEMP sensor inside your CPU. Supported all are all known variants of Intel Core family. =20 +config SENSORS_IBMPEX + tristate "IBM PowerExecutive temperature/power sensors" + depends on IPMI_SI + help + If you say yes here you get support for the temperature and + power sensors in various IBM System X servers that support + PowerExecutive. So far this includes the x3550, x3650, x3655, + x3755, and certain HS20 blades. + + This driver can also be built as a module. If so, the module + will be called ibmpex. + config SENSORS_IT87 tristate "ITE IT87xx and compatibles" select HWMON_VID diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index a133981..31da6fe 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -35,6 +35,7 @@ obj-$(CONFIG_SENSORS_FSCPOS) +=3D fsc...
What unit should we use? Watts are way, way too big as there is no floating/fixed point in sysfs. 10^-6 W is probably what is called for, since we already need 10^-3 V and 10^-3 A. Small portable devices can easily draw less than 10^-3 W nowadays. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh -
Hi Darrick, hi Henrique, Good thing that manufacturers start including wattmeters in their hardware. Hopefully this will help users better control their power consumption in the long run. Good point. The driver currently exports non-integer values, which is not acceptable, so indeed it needs to be changed. We want at least a resolution of 10^-3 W. Not sure about 10^-6 W. I am surprised that portable devices can really draw less than 1 mW, and be it the case, I doubt that manufacturers will embed a wattmetter: it would probably draw more current than the rest of the device ;) so it may not be relevant for our decision. So I think I'd go with 10^-3 W, but I welcome diverging opinions. Out of curiosity, what is the physical resolution of IBM's device? I see that the driver relies on IPMI. Can't it be merged with the out-of-tree impisensors driver then? This would give that driver some momentum so that it can finally be merged, and I would like to avoid having two drivers if one is enough. Note though that I don't know anything about IPMI so I might as well be totally wrong ;) -- Jean Delvare -
ibm_pex: Driver to export IBM PowerExecutive power meter sensor readings in mW. Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> --- drivers/hwmon/Kconfig | 12 + drivers/hwmon/Makefile | 1=20 drivers/hwmon/ibmpex.c | 600 ++++++++++++++++++++++++++++++++++++++++++++= ++++ 3 files changed, 613 insertions(+), 0 deletions(-) diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 555f470..41ffa2e 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -275,6 +275,18 @@ config SENSORS_CORETEMP sensor inside your CPU. Supported all are all known variants of Intel Core family. =20 +config SENSORS_IBMPEX + tristate "IBM PowerExecutive temperature/power sensors" + depends on IPMI_SI + help + If you say yes here you get support for the temperature and + power sensors in various IBM System X servers that support + PowerExecutive. So far this includes the x3550, x3650, x3655, + x3755, and certain HS20 blades. + + This driver can also be built as a module. If so, the module + will be called ibmpex. + config SENSORS_IT87 tristate "ITE IT87xx and compatibles" select HWMON_VID diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index a133981..31da6fe 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -35,6 +35,7 @@ obj-$(CONFIG_SENSORS_FSCPOS) +=3D fscpos.o obj-$(CONFIG_SENSORS_GL518SM) +=3D gl518sm.o obj-$(CONFIG_SENSORS_GL520SM) +=3D gl520sm.o obj-$(CONFIG_SENSORS_HDAPS) +=3D hdaps.o +obj-$(CONFIG_SENSORS_IBMPEX) +=3D ibmpex.o obj-$(CONFIG_SENSORS_IT87) +=3D it87.o obj-$(CONFIG_SENSORS_K8TEMP) +=3D k8temp.o obj-$(CONFIG_SENSORS_LM63) +=3D lm63.o diff --git a/drivers/hwmon/ibmpex.c b/drivers/hwmon/ibmpex.c new file mode 100644 index 0000000..10f1205 --- /dev/null +++ b/drivers/hwmon/ibmpex.c @@ -0,0 +1,600 @@ +/* + * A hwmon driver for the IBM PowerExecutive temperature/power sensors + * Copyright (C) 2007 IBM + * + * Author: Darrick J. Wong <djwong@us.ibm.com> + * ...
Dave Hansen complained about the magic numbers, repetitive code, and various other minor problems with the driver code, so here's a v3 with the magic numbers migrated to the top of the file and #define'd, helper macros taking place of the bit shifting/masking activities, and the compression of the value/min/max sysfs code into parameterized functions. -- ibm_pex: Driver to export IBM PowerExecutive power meter sensors. Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> --- drivers/hwmon/Kconfig | 12 + drivers/hwmon/Makefile | 1=20 drivers/hwmon/ibmpex.c | 564 ++++++++++++++++++++++++++++++++++++++++++++= ++++ 3 files changed, 577 insertions(+), 0 deletions(-) diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 555f470..41ffa2e 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -275,6 +275,18 @@ config SENSORS_CORETEMP sensor inside your CPU. Supported all are all known variants of Intel Core family. =20 +config SENSORS_IBMPEX + tristate "IBM PowerExecutive temperature/power sensors" + depends on IPMI_SI + help + If you say yes here you get support for the temperature and + power sensors in various IBM System X servers that support + PowerExecutive. So far this includes the x3550, x3650, x3655, + x3755, and certain HS20 blades. + + This driver can also be built as a module. If so, the module + will be called ibmpex. + config SENSORS_IT87 tristate "ITE IT87xx and compatibles" select HWMON_VID diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index a133981..31da6fe 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -35,6 +35,7 @@ obj-$(CONFIG_SENSORS_FSCPOS) +=3D fscpos.o obj-$(CONFIG_SENSORS_GL518SM) +=3D gl518sm.o obj-$(CONFIG_SENSORS_GL520SM) +=3D gl520sm.o obj-$(CONFIG_SENSORS_HDAPS) +=3D hdaps.o +obj-$(CONFIG_SENSORS_IBMPEX) +=3D ibmpex.o obj-$(CONFIG_SENSORS_IT87) +=3D it87.o obj-$(CONFIG_SENSORS_K8TEMP) +=3D k8temp.o obj-$(CONFIG_SENSORS_LM63) +=3D lm...
Hi Darrick: I am not an IPMI expert, so I would appreciate getting an Acked-by from someone who knows more about that subsystem. Open question: can we use "select" here? As written, it took some hunting to My current stack of patches includes one which requires that this be changed to 'struct device *hwmon_dev', as 'struct class_device' is going away soon. You may rebase on my testing tree[1], or else I will just follow up with a patch to fix this up after I eventually merge yours. ... especially given how many times you're going to call it. Is there any reason you can't use the driver_data field of struct device *dev for that? E.g. i2c based hwmon drivers do this at some point during the probe: i2c_set_clientdata(new_client, data); (which becomes) dev_set_drvdata(&new_client->dev, data); If you could do that, then you no longer need 'iface' at all in the function above... *that* may allow you to use the SENSOR_ATTR_2 mechanism from hwmon-sysfs.h - much easier to read than the manual number packing for 'sensor' Regards, -- Mark M. Hoffman mhoffman@lightlink.com -
Here's a fourth revision where I've tried to clean up the things that people complained about, as well as shifted the sysfs file names to match the spec that we've been drifting towards. --- ibm_pex: Driver to export IBM PowerExecutive power meter sensors. Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> --- drivers/hwmon/Kconfig | 13 + drivers/hwmon/Makefile | 1=20 drivers/hwmon/ibmpex.c | 608 ++++++++++++++++++++++++++++++++++++++++++++= ++++ 3 files changed, 622 insertions(+), 0 deletions(-) diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 5ca5d95..591b666 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -315,6 +315,19 @@ config SENSORS_CORETEMP sensor inside your CPU. Supported all are all known variants of Intel Core family. =20 +config SENSORS_IBMPEX + tristate "IBM PowerExecutive temperature/power sensors" + select IPMI_SI + depends on IPMI_HANDLER + help + If you say yes here you get support for the temperature and + power sensors in various IBM System X servers that support + PowerExecutive. So far this includes the x3550, x3650, x3655, + x3755, and certain HS20 blades. + + This driver can also be built as a module. If so, the module + will be called ibmpex. + config SENSORS_IT87 tristate "ITE IT87xx and compatibles" select HWMON_VID diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 5070cf7..0fcbcb4 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -38,6 +38,7 @@ obj-$(CONFIG_SENSORS_GL518SM) +=3D gl518sm.o obj-$(CONFIG_SENSORS_GL520SM) +=3D gl520sm.o obj-$(CONFIG_SENSORS_HDAPS) +=3D hdaps.o obj-$(CONFIG_SENSORS_I5K_AMB) +=3D i5k_amb.o +obj-$(CONFIG_SENSORS_IBMPEX) +=3D ibmpex.o obj-$(CONFIG_SENSORS_IPMI) +=3D ipmisensors.o obj-$(CONFIG_SENSORS_IT87) +=3D it87.o obj-$(CONFIG_SENSORS_K8TEMP) +=3D k8temp.o diff --git a/drivers/hwmon/ibmpex.c b/drivers/hwmon/ibmpex.c new file mode 100644 index 0000000..fe2c261 --- /dev/null +++ b/dr...
Hi Darrick: Sorry it took me so long to get back to this. I've applied this to my hwmon testing branch, but I would still like to get a review from an IPMI expert. OBTW: please try to send patches which apply cleanly to some tree to which I have access. E.g. from the Makefile - preceding and trailing context are not Regards, -- Mark M. Hoffman mhoffman@lightlink.com -
Oops, sorry about that. The i5k_amb driver referenced in the Makefile diff is the same one that I posted to lm-sensors a few weeks ago, though there hasn't been much discussion after it was pointed out that someone had already written a driver to achieve the same purpose. Unfortunately, nothing's happened to either driver; any thoughts? I'm thinking that we ought to get one of them into shape for upstream, though I don't particularly care which one it is... --D
-
Roel Kluin <12o3l@tiscali.nl> found a minor defect in the init code if hwmon device registration fails. Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> --- drivers/hwmon/ibmpex.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/ibmpex.c b/drivers/hwmon/ibmpex.c index fe2c261..c462824 100644 --- a/drivers/hwmon/ibmpex.c +++ b/drivers/hwmon/ibmpex.c @@ -498,8 +498,7 @@ static void ibmpex_register_bmc(int iface, struct device *dev) printk(KERN_ERR DRVNAME ": Error, unable to register hwmon " "class device for interface %d\n", data->interface); - kfree(data); - return; + goto out_user; } /* finally add the new bmc data to the bmc data list */ -
Hi: Applied to hwmon-2.6.git/testing, thanks. -- Mark M. Hoffman mhoffman@lightlink.com -
Err, yes, thank you for catching that. --D -
Update the hwmon sysfs interface documentation to include a specification for power meters. Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> --- Documentation/hwmon/sysfs-interface | 30 ++++++++++++++++++++++++++++++ 1 files changed, 30 insertions(+), 0 deletions(-) diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysf= s-interface index db7bb4a..5c98bee 100644 --- a/Documentation/hwmon/sysfs-interface +++ b/Documentation/hwmon/sysfs-interface @@ -324,6 +324,36 @@ curr[1-*]_input Current input value Unit: milliampere RO =20 +********* +* Power * +********* + +power[1-*]_average Average power use + Unit: microWatt + RO + +power[1-*]_average_highest Historical average maximum power use + Unit: microWatt + RO + +power[1-*]_average_lowest Historical average minimum power use + Unit: microWatt + RO + +power[1-*]_input Instantaneous power use + Unit: microWatt + RO + +power[1-*]_input_highest Historical maximum power use + Unit: microWatt + RO + +power[1-*]_input_lowest Historical minimum power use + Unit: microWatt + RO + +power[1-*]_high_low_reset Reset input_highest/input_lowest. + WO =20 ********** * Alarms *
Hi Darrick, Thanks for the update. I have some more comments. And sorry if it looks like nitpicking, but this is a standard interface we're defining so we better make sure that we get it right. First of all, please think of a better subject line for this patch. "Update Documentation/hwmon/sysfs-interface" is too vague when your I don't much like this name. It sounds like a name crafted for the specific feature of a given chip, while we try to use generic names for the standard interface. I would rather go for power[1-*]_reset_history if we have a single file for resetting all the extremes of a given channel, or power[1-*]_input_lowest_reset and power[1-*]_input_highest_reset if we go for a per-value reset. Alternatively, we could simply make the power[1-*]_input_lowest and power[1-*]_input_highest files writable, and "cat power1_input > Thanks, -- Jean Delvare -
Will change to: I don't know if anybody ever really requires this data, but the device Guilty as charged. Since ibmpex is the only on-board power meter hardware to which I have access, the interface proposal includes the various odd pieces that the hardware provides in addition to the meters. I could take the historical extreme and reset bits out of the proposal and put them in something like Documentation/hwmon/ibmpex.txt as extra I'd prefer to leave it explicitly called out as a separate sysfs knob, unless there are established precedents for resetting a sensor by writing something to its sysfs file. --D
Hi Darrick, I'm fine having them in the spec as long as the interface is made This doesn't mean we do not want separate files. The same problem exists for many other hardware monitoring features. For example most thermal sensor chips have one high temperature limit per temperature channel. However some of them have a single limit register for several channels! The approach we have taken so far was to have individual files, and it's up to the driver to keep them in sync. Another example is alarm files. Some chips have a single alarm for out-of-range voltage measurements. Others have separate alarms for low voltage and high voltage limit crossing. In that case, we decided to have either a single file (in0_alarm) or separate files (in0_min_alarm and in0_max_alarm) depending on what the chip supports. Admittedly this is not very consistent with what we did for the shared temperature No, there's no such established precedent. This was only a suggestion from me as it seems to make some sense. If you don't like the idea, just ignore it. -- Jean Delvare -
yet. Thank you for the review! Comments interspersed below, though for Changed, since it seems reasonable that someone looking for PEx support ortlog;h=3Dtesting I can (and did) update the code to use dev_get/set_drvdata for the accessors. However, the "iface" field exists as a mechanism to map interface numbers to struct ibmpex_bmc_data/struct device data because the callback that IPMI uses to notify clients that BMCs are going away only passes the interface number, not the struct device itself. Unfortunately, this means that get_bmc_data() must remain, but now it is Rough draft syndrome? 'tis fixed. :) --D
Update the hwmon sysfs interface documentation to include a specification for power meters. Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> --- Documentation/hwmon/sysfs-interface | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysf= s-interface index b3a9e1b..da546ce 100644 --- a/Documentation/hwmon/sysfs-interface +++ b/Documentation/hwmon/sysfs-interface @@ -304,6 +304,21 @@ curr[1-*]_input Current input value Unit: milliampere RO =20 +********* +* Power * +********* + +power[1-*]_input Current power use + Unit: milliWatt + RO + +power[1-*]_max_input Historical maximum power use + Unit: milliWatt + RO + +power[1-*]_min_input Historical minimum power use + Unit: milliWatt + RO =20 ********** * Alarms *
Should we add power?_10sec_avg_input? IIRC thinkpads export that, too. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -
Indeed, ThinkPads report both instantaneous power/current and a rolling average (exponentially decaying, I think) over the last ~10 seconds. ACPI provides only the rolling average, and the out-of-tree tp_smapi driver provides both. More generally, linux/power_supply.h defines these attributes: POWER_SUPPLY_PROP_VOLTAGE_NOW, POWER_SUPPLY_PROP_VOLTAGE_AVG, POWER_SUPPLY_PROP_CURRENT_NOW, POWER_SUPPLY_PROP_CURRENT_AVG, It would be nice if the power meter interface uses the same conventions as the power supply interface, since the former is essentially a subset of the latter. Userspace app that read power meters via sysfs should work for power supplies too. Shem -
I'd say it is enough to know it is an average, you don't need to specify HOW it is averaged. But there is a real need to export both instantaneous and averaged values in power supplies/power sources, so we should have a standard way to do so in hwmon, and power_supply class should use it (or the opposite, it doesn't Agreed. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh -
I guess power[1-*]_average would be OK? -- Jean Delvare -
AFAIK, yes. It is probably not 100% in sync with the power supply class, though. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh -
Is the power supply class creating sysfs files? I see a number of attributes listed in Documentation/power_supply_class.txt, but they are all uppercase, which doesn't seem suitable for sysfs file names. That document also doesn't list the numbering convention when multiple channels are present. My main worry is that we will have to add support for power measurement in libsensors, and I would like it to be as easy as possible. Thus sticking to the same naming convention hwmon have been using for years appears to be the best solution. I see that the power supply class units are 10^-6 A and 10^-6 W, so if we are supposed to be compatible, I guess we'll have to use this too. -- Jean Delvare -
There was a good reason for that, and people who deal with small portables said that they needed 10^-4 A or somesuch, at which point it makes more sense to just go to 10^-6 already. I don't recall why 10^-6 W, though. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh -
Well, if we need 10^-4 A, and we use voltages in the 1-2 V range (that's what CPU, AGP etc. use these days), then we obviously need 10^-4 W as well. -- Jean Delvare -
I see, and in that case it is safer to just go to 10^-6 for both. It is more future-proof. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh -
Hi Darrick, I'm not sure if we want these "historical" files. We don't have them for the other input types, and I believe that it's not the driver's job to compute and export these values. If anyone cares about the history of sensed values, that's something for a user-space application to implement. This will also be much more flexible in user-space, as it becomes possible to decide the exact time range to consider, to remember at which time the peak occurred, etc. -- Jean Delvare -
In the case of ibmpex, it is the _hardware_ that computes the historical data; the driver merely exports what it sees. --D
Hi Darrick, OK, that's a bit different then, but I'm still not sure that there is much value in exporting these values in sysfs, in particular if there is no way to reset them. I am also not happy with the names you proposed: power1_max_input and power1_min_input are somewhat confusing IMHO, I'd suggest power1_input_highest and power1_input_lowest to make them clearly different from the min and max limits we have for other sensor types. If we have them at all, of course. -- Jean Delvare -
It turns out there _is_ a way to reset them; the next iteration of the Agreed and changed. --D
I had a feeling you might say that. :) I'll post a follow-up patch that Are there meters that can measure that small a quantity of power? If I don't know for sure, but my observation is that they're no more accurate than a Watt. Some of those meters appear to be averages, which The IBM PEx meters are accessible via custom IPMI commands, not the standard IPMI sensor interface. I considered modifying the ipmisensors driver, but reached the opinion that it would clutter that driver unnecessarily, particularly since there are a lot of systems with IPMI BMCs, but most of them will not have this interface. That said, what is standing in the way of ipmisensors being merged? I think I can shake out some free time to apply TLC. --D
Hi Darrick, I don't know exactly, check out the website for details: http://bmcsensors-26.sourceforge.net/ It needs at least review and testing, but maybe there are a few more steps before merging can be considered. Adding Yani (the author) to Cc, please work with him if you're interested. -- Jean Delvare -
As a user, I can tell you that it completely fills my dmesg with this sort ipmisensors: sensor 50 (type 1) reading 60 ipmisensors: Send 0x2d 0x30 0x0 ipmisensors: received message ipmisensors: sensor 48 (type 1) reading 28 ipmisensors: Send 0x2d 0x1b 0x0 ipmisensors: received message ipmisensors: sensor 27 (type 2) reading 34 ipmisensors: Send 0x2d 0x1a 0x0 ipmisensors: received message ipmisensors: sensor 26 (type 2) reading 187 ipmisensors: Send 0x2d 0x18 0x0 That's the only problem I have. It would be great to see it merged. Frank -
| Linus Torvalds | Linux 2.6.26-rc4 |
| Satyam Sharma | Re: 2.6.23-rc6-mm1 |
| Chuck Ebbert | Why do so many machines need "noapic"? |
| Jan Engelhardt | Re: LSM conversion to static interface |
| Theo de Raadt | That whole "Linux stealing our code" thing |
| Marco Peereboom | Re: Real men don't attack straw men |
| Marius ROMAN | 1440x900 resolution problem |
| GVG GVG | ssh_exchange_identification: Connection closed by remote host |
git: | |
| Martin Langhoff | Re: git versus CVS (versus bk) |
| Linus Torvalds | People unaware of the importance of "git gc"? |
| Martin Langhoff | Handling large files with GIT |
| Jeff King | Re: Git vs Monotone |
| David Miller | [GIT]: Networking |
| Matheos Worku | 2.6.24 BUG: soft lockup - CPU#X |
| Auke Kok | [PATCH] e1000e: test MSI interrupts |
| Wang Jian | drivers/net/phy/marvell.c: 88e1111 can't get out sleep mode |
