i2c: new sysfs class "hwmon"

This patch adds the sysfs class "hwmon" for use by hardware monitoring (sensors) chip drivers. It (the Kconfig text) presumes that sensors chip drivers will be moved to drivers/hwmon (although that is not done by this patch).

Index: linux-2.6.12-rc3-mm2/drivers/Kconfig
=================================--- linux-2.6.12-rc3-mm2.orig/drivers/Kconfig 2005-05-01 08:48:35.000000000 -0400
+++ linux-2.6.12-rc3-mm2/drivers/Kconfig 2005-05-01 08:48:42.000000000 -0400
@@ -40,6 +40,8 @@ source "drivers/input/Kconfig"

source "drivers/char/Kconfig"

+source "drivers/hwmon/Kconfig"
+
source "drivers/i2c/Kconfig"

source "drivers/w1/Kconfig"
Index: linux-2.6.12-rc3-mm2/drivers/Makefile
=================================--- linux-2.6.12-rc3-mm2.orig/drivers/Makefile 2005-05-01 08:48:35.000000000 -0400
+++ linux-2.6.12-rc3-mm2/drivers/Makefile 2005-05-01 08:48:42.000000000 -0400
@@ -51,6 +51,7 @@ obj-$(CONFIG_GAMEPORT) += input/gamepor
obj-$(CONFIG_INPUT) += input/
obj-$(CONFIG_I2O) += message/
obj-$(CONFIG_I2C) += i2c/
+obj-$(CONFIG_HWMON) += hwmon/
obj-$(CONFIG_W1) += w1/
obj-$(CONFIG_PHONE) += telephony/
obj-$(CONFIG_MD) += md/
Index: linux-2.6.12-rc3-mm2/drivers/hwmon/Kconfig
=================================--- linux-2.6.12-rc3-mm2.orig/drivers/hwmon/Kconfig 2005-04-22 03:12:06.429003480 -0400
+++ linux-2.6.12-rc3-mm2/drivers/hwmon/Kconfig 2005-05-01 08:48:42.000000000 -0400
@@ -0,0 +1,15 @@
+
+menu "Hardware Monitoring (Sensors) support"
+
+config HWMON
+ tristate "Hardware Monitoring Core support"
+ help

+	  If you want hardware monitoring (sensors) support, you should
+	  say Y here and also to the specific driver(s) for your sensors
+	  chip(s) below.

+
+ This support can also be built as a module. If so, the module
+ will be called hwmon.
+
+endmenu
+
Index: linux-2.6.12-rc3-mm2/drivers/hwmon/Makefile
=================================--- linux-2.6.12-rc3-mm2.orig/drivers/hwmon/Makefile 2005-04-22 03:12:06.429003480 -0400
+++ linux-2.6.12-rc3-mm2/drivers/hwmon/Makefile 2005-05-01 08:48:42.000000000 -0400
@@ -0,0 +1,5 @@
+#
+# Makefile for hardware monitoring (sensors) +#
+obj-$(CONFIG_HWMON) += hwmon.o
+
Index: linux-2.6.12-rc3-mm2/drivers/hwmon/hwmon.c
=================================--- linux-2.6.12-rc3-mm2.orig/drivers/hwmon/hwmon.c 2005-04-22 03:12:06.429003480 -0400
+++ linux-2.6.12-rc3-mm2/drivers/hwmon/hwmon.c 2005-05-02 22:34:51.000000000 -0400
@@ -0,0 +1,64 @@
+/*
+ hwmon.c - part of lm_sensors, Linux kernel modules for hardware monitoring
+
+ This file defines the sysfs class "hwmon", for use by sensors drivers.
+
+ Copyright (C) 2005 Mark M. Hoffman <[email protected]>
+

+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.

+*/
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kdev_t.h>
+
+static struct class *hwmon_class;
+
+struct class_device *hwmon_device_register(struct device *dev) +{
+ return class_device_create(hwmon_class, MKDEV(0,0), dev, dev->bus_id);
+}
+
+void hwmon_device_unregister(struct class_device *cdev) +{
+ class_device_unregister(cdev);
+}
+
+static int __init hwmon_init(void)
+{
+ hwmon_class = class_create(THIS_MODULE, "hwmon");
+ if (IS_ERR(hwmon_class)) {
+ printk(KERN_ERR "hwmon.c: couldn't create sysfs class\n");
+ return PTR_ERR(hwmon_class);
+ }
+ return 0;
+}
+
+static void __exit hwmon_exit(void)
+{
+ class_destroy(hwmon_class);
+}
+
+module_init(hwmon_init);
+module_exit(hwmon_exit);
+
+EXPORT_SYMBOL(hwmon_device_register); +EXPORT_SYMBOL(hwmon_device_unregister);
+
+MODULE_AUTHOR("Mark M. Hoffman <[email protected]>"); +MODULE_DESCRIPTION("hardware monitoring sysfs/class support"); +MODULE_LICENSE("GPL");
+
Index: linux-2.6.12-rc3-mm2/include/linux/hwmon.h
=================================--- linux-2.6.12-rc3-mm2.orig/include/linux/hwmon.h 2005-04-22 03:12:06.429003480 -0400
+++ linux-2.6.12-rc3-mm2/include/linux/hwmon.h 2005-05-02 22:22:55.000000000 -0400
@@ -0,0 +1,46 @@
+/*
+ hwmon.h - part of lm_sensors, Linux kernel modules for hardware monitoring
+
+ This file declares helper functions for the sysfs class "hwmon",
+ for use by sensors drivers.
+
+ Copyright (C) 2005 Mark M. Hoffman <[email protected]>
+

+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.

+*/
+
+#ifndef _HWMON_H_
+#define _HWMON_H_
+
+#include <linux/device.h>
+
+/**
+ * hwmon_device_register - register w/ hwmon sysfs class
+ * @dev: the device to register
+ *
+ * hwmon_device_unregister() must be called when the class device is no longer needed.
+ *
+ * Returns the pointer to the new struct class device.
+ */
+struct class_device *hwmon_device_register(struct device *dev);
+
+/**
+ * hwmon_device_unregister - removes the class device created with hwmon_device_register
+ * @cdev: the class device to destroy
+ */
+void hwmon_device_unregister(struct class_device *cdev);
+
+#endif
+
Index: linux-2.6.12-rc3-mm2/include/linux/i2c.h
=================================--- linux-2.6.12-rc3-mm2.orig/include/linux/i2c.h 2005-05-01 08:48:35.000000000 -0400
+++ linux-2.6.12-rc3-mm2/include/linux/i2c.h 2005-05-02 22:02:34.000000000 -0400
@@ -154,6 +154,7 @@ struct i2c_client {
int usage_count; /* How many accesses currently */
/* to the client */
struct device dev; /* the device structure */
+ struct class_device *class_dev;
struct list_head list;
char name[I2C_NAME_SIZE];
struct completion released;

Mark M. Hoffman
[email protected]

Looks good, minor comments:

Hi Mark,

What's the status on the hwmon patch? I'm looking at updating bmcsensors now and Jean suggested bmcsensors should be modified to get rid of any i2c references before submitting it (since it has absolutely nothing to do with i2c), but from what I can tell this was the last message about it. I'd really like to see something like this get accepted soon.

Thanks,
Yani

Hello Yani:

I thought I would finish the userspace/libsensors modifications first, then I could shake out any interface problems. As I wrote earlier:

I was thinking of passing the class device ID string as a parameter to hwmon_device_register() instead of pulling it out of dev->bus_id. If I were to re-submit the patch for inclusion (w/ the changes suggested by Greg)... what would you use for the class device ID string for bmcsensors?

If we can have such a patch included on the condition that it may still change (due to interface / naming conventions) then I will rework it and submit it on Wed or Thu. Maybe we can keep it in -mm for an extended period until I finish the libsensors stuff. Would that work for you?

Regards,

Mark (hacking on his house instead of his computer) Hoffman
[email protected]

Hi Mark,

...

err... I don't know...not bus id :-). bmcsensors works using the kernel IPMI messaging interface, and that really isn't a bus at all...its more like a messaging protocol, hence the need in my mind to remove all the i2c references.

Jean was explaining to me today that this patch just adds the sysfs clsss and drivers would still be i2c based, I was under the mistaken impression it was standalone, but as your e-mail stated that is yet to be implemented.

I fear that bmcsensors really needs the full separation from i2c, how realistic would it be to go ahead and try something like that at this stage (I can help out)? It would seem to me that basically everything under drivers/i2c/chips should be moved into drivers/hwmon and perhaps drivers/i2c/bus to drivers/i2c (although the latter isn't necessary, it just makes sense to me).

Yes, I need the dynamic sysfs stuff that will be in -mm anyway, I so that's perfectly fine, I expect all of this to be in testing for a while.

Breaking things takes on all new meaning with that type of hacking, be careful ;-)

Thanks,
Yani

Hi Yani,

No, not all i2c chip drivers are hardware monitoring monitors. At least the following should stay in drivers/i2c/chips:
* ds1337
* eeprom
* isp1301_omap (what is this BTW?)
* m41t00
* pcf8574
* pcf8591
* rtc8564

And then there's atxp1, not too sure what to do with this one (it's meant to overclock systems, not monitor them, but it shares code and interface with hwmon).

I don't this it is a good idea. The current scheme makes it clear which drivers create i2c busses, which are i2c chip drivers, which are algorithms and which are core drivers. I wouldn't change that.

Thanks,
--
Jean Delvare

Ah, yes you are right of course. I have no idea what isp1301_omap is :), but I'd have to say that about the majority of stuff in i2c.

My thought was more a removal of a redundancy, but since there will still be some i2c/chips as you point out it doesn't make much sense.

I guess I'll have a little look into things myself, see how I'd go about making bmcsensors independent from i2c on its own for now.

Thanks,
Yani

Hi Mark,

Sorry to bother you again, but would it be possible to see/explain what modifications you plan to make to libsensors to read/write to sensors for hwmon class devices, I'm in particular what specific sysfs attributes you are reading from in /sys/class/hwmon/blah/.

At first I assumed you would probably be using the device_attributes under /sys/class/hwmon/blah/device/ (assuming the particular hwmon driver has an associated device - its optional), but on further though that really doesn't make much sense because we would be assuming all the hwmon class driver devices would share the same driver type (or common sysfs device attribute interface)..which would sort of seem to defy the whole point of having a hwmon class in the first place. From what I see really we should be using class_device_attributes instead of device_attributes for a hwmon driver, in which case I'm going to have to submit a patch to Greg adding dynamic sysfs callbacks to class_device_attribute sooner than I expected..

Using class_device_attributes would also lend the ability to, for example, adding a hwmon interface to a device that's primary function isn't to interface sensors, e.g. instead of having a bmcsensors driver we would add a hwmon interface to the actual kernel IPMI bmc driver, this would make much more sense to me.

Thanks,
Yani

Sure, I realize that non I2C/SMBus sensors will need to somehow be uniquely identified in the hwmon class. That's why I was looking for feedback on just how to do that. For now, we can just let the hwmon registrants use an ID of their own choosing.

I'm not sure what you mean here. Of course, sensor chips that live on SMBus will still be i2c based. As for the hwmon class, they will all register to that *in addition to* the rest of the existing sysfs interface.

Assuming that I modify my previous patch to allow hwmon class registrants to pass their own class ID, your bmcsensors driver would just add a bunch of sysfs device attributes just like existing drivers. You would not need to register with the I2C subsystem for anything. Point the hwmon class at your device by registering, and then userspace (will) know about it.

Of course, libsensors is not yet ready for that.

OK, I'll try to submit a patch tomorrow night.

Regards,

Mark M. Hoffman
[email protected]

Hi Yani:

That is just it, nothing more.

We could use class device attributes, sure. We could move all the existing device attributes, even. It's not like the sensors kernel interface has been stable in 2.6.x since... well... ever.

I suggested the current path (use hwmon class just for the link to device attributes) to Khali, rather than something more elaborate, mostly so that I could hope to actually complete it while leaving libsensors looking less ugly than it does now. When libsensors has a more flexible and maintainable way of parsing sysfs structure (like using libsysfs, for one) then the rest may follow.

That would be neat, true.

Regards,

Mark M. Hoffman
[email protected]

lol..ok, you have a point. I'm thinking about things without regard to the size/pain of the change, I let people with more sense think about that and shoot me down ;-). Using class_device_attributes is definitely something to keep in the back of our minds though.

OK, fair enough. I'm a fair way along re-writing bmcsensors (probably to be called ipmisensors now) to take advantage of the hwmon patch and dynamic device attributes, and its all looking very promising and much cleaner so far, so thanks for writing the patch. When it's at a stage that it works I'll CC a copy to you guys and the list for comment. I definitely think hwmon is the way forward, but I have a much different perspective than most other lm_sensors authors due to the nature of bmc/ipmisensors.

Oh, is there a userspace patch somewhere that I can use to test things out when I get to that stage?

Thanks,
Yani