[PATCH] Add sysfs support to the IPMI driver

[PATCH] Add sysfs support to the IPMI driver

[Corey Minyard]

[-- Attachment #1: Type: text/plain, Size: 245 bytes --]

The IPMI driver has long needed to tie into the device model (and I've 
long been hoping someone else would do it).  I finally gave up and spent 
the time to learn how to do it.  I think this is right, it seems to work 
on on my system.

-Corey

[-- Attachment #2: ipmi-sysfs.diff --]
[-- Type: text/x-patch, Size: 2111 bytes --]

Add support for sysfs to the IPMI device interface.

Signed-off-by: Corey Minyard <minyard@acm.org>

Index: linux-2.6.11-mm1/drivers/char/ipmi/ipmi_devintf.c
===================================================================
--- linux-2.6.11-mm1.orig/drivers/char/ipmi/ipmi_devintf.c
+++ linux-2.6.11-mm1/drivers/char/ipmi/ipmi_devintf.c
@@ -44,6 +44,7 @@
 #include <linux/ipmi.h>
 #include <asm/semaphore.h>
 #include <linux/init.h>
+#include <linux/device.h>
 
 #define IPMI_DEVINTF_VERSION "v33"
 
@@ -519,15 +520,24 @@
 		 " interface.  Other values will set the major device number"
 		 " to that value.");
 
+static struct class_simple *ipmi_class;
+
 static void ipmi_new_smi(int if_num)
 {
+	char                  name[10];
+	dev_t                 dev = MKDEV(ipmi_major, if_num);
+
 	devfs_mk_cdev(MKDEV(ipmi_major, if_num),
 		      S_IFCHR | S_IRUSR | S_IWUSR,
 		      "ipmidev/%d", if_num);
+
+	snprintf(name, sizeof(name), "ipmi%d", if_num);
+	class_simple_device_add(ipmi_class, dev, NULL, name);
 }
 
 static void ipmi_smi_gone(int if_num)
 {
+	class_simple_device_remove(MKDEV(ipmi_major, if_num));
 	devfs_remove("ipmidev/%d", if_num);
 }
 
@@ -548,8 +558,15 @@
 	printk(KERN_INFO "ipmi device interface version "
 	       IPMI_DEVINTF_VERSION "\n");
 
+	ipmi_class = class_simple_create(THIS_MODULE, "ipmi");
+	if (IS_ERR(ipmi_class)) {
+		printk(KERN_ERR "ipmi: can't register device class\n");
+		return PTR_ERR(ipmi_class);
+	}
+
 	rv = register_chrdev(ipmi_major, DEVICE_NAME, &ipmi_fops);
 	if (rv < 0) {
+		class_simple_destroy(ipmi_class);
 		printk(KERN_ERR "ipmi: can't get major %d\n", ipmi_major);
 		return rv;
 	}
@@ -563,6 +580,7 @@
 	rv = ipmi_smi_watcher_register(&smi_watcher);
 	if (rv) {
 		unregister_chrdev(ipmi_major, DEVICE_NAME);
+		class_simple_destroy(ipmi_class);
 		printk(KERN_WARNING "ipmi: can't register smi watcher\n");
 		return rv;
 	}
@@ -573,6 +591,7 @@
 
 static __exit void cleanup_ipmi(void)
 {
+	class_simple_destroy(ipmi_class);
 	ipmi_smi_watcher_unregister(&smi_watcher);
 	devfs_remove(DEVICE_NAME);
 	unregister_chrdev(ipmi_major, DEVICE_NAME);

Re: [PATCH] Add sysfs support to the IPMI driver

[Greg KH]

On Sat, Mar 12, 2005 at 10:57:24PM -0600, Corey Minyard wrote:
> The IPMI driver has long needed to tie into the device model (and I've 
> long been hoping someone else would do it).  I finally gave up and spent 
> the time to learn how to do it.  I think this is right, it seems to work 
> on on my system.

Looks good.  One minor question:

> +
> +	snprintf(name, sizeof(name), "ipmi%d", if_num);
> +	class_simple_device_add(ipmi_class, dev, NULL, name);

What do ipmi class devices live on?  pci devices?  i2c devices?
platform devices?  Or are they purely virtual things?

thanks,

greg k-h

Re: [PATCH] Add sysfs support to the IPMI driver

[Corey Minyard]

Greg KH wrote:

>On Sat, Mar 12, 2005 at 10:57:24PM -0600, Corey Minyard wrote:
>  
>
>>The IPMI driver has long needed to tie into the device model (and I've 
>>long been hoping someone else would do it).  I finally gave up and spent 
>>the time to learn how to do it.  I think this is right, it seems to work 
>>on on my system.
>>    
>>
>
>Looks good.  One minor question:
>
>  
>
>>+
>>+	snprintf(name, sizeof(name), "ipmi%d", if_num);
>>+	class_simple_device_add(ipmi_class, dev, NULL, name);
>>    
>>
>
>What do ipmi class devices live on?  pci devices?  i2c devices?
>platform devices?  Or are they purely virtual things?
>  
>
Good question.  I struggled with this for a little while and decided the 
class interface was important to have in first and I'd figure out the 
rest later.  They live in different places depending on the particular 
low-level interface.  Some live on the I2C bus (and will show up there 
in sysfs with the I2C driver).  Some live on the ISA bus, some are 
memory-mapped, some are on the PCI bus (though there is not a driver for 
PCI support yet), and some sit on the end of a serial port (driver is in 
the works).  I know, it's a mess, but there's not much I can do about 
these crazy hardware manufacturers.

I wasn't sure where to handle all this.  The I2C and PCI bus side of 
things should be handled.  However, the others probably need to sit 
someplace on a bus, right?  That should probably be handled in the 
low-level code that actually knows where the hardware sits.

Thanks,

-Corey

Re: [PATCH] Add sysfs support to the IPMI driver

[Greg KH]

On Sun, Mar 13, 2005 at 04:59:14PM -0600, Corey Minyard wrote:
> Greg KH wrote:
> >On Sat, Mar 12, 2005 at 10:57:24PM -0600, Corey Minyard wrote:
> >>The IPMI driver has long needed to tie into the device model (and I've 
> >>long been hoping someone else would do it).  I finally gave up and spent 
> >>the time to learn how to do it.  I think this is right, it seems to work 
> >>on on my system.
> >
> >Looks good.  One minor question:
> >
> >>+
> >>+	snprintf(name, sizeof(name), "ipmi%d", if_num);
> >>+	class_simple_device_add(ipmi_class, dev, NULL, name);
> >
> >What do ipmi class devices live on?  pci devices?  i2c devices?
> >platform devices?  Or are they purely virtual things?
> >
> Good question.  I struggled with this for a little while and decided the 
> class interface was important to have in first and I'd figure out the 
> rest later.  They live in different places depending on the particular 
> low-level interface.  Some live on the I2C bus (and will show up there 
> in sysfs with the I2C driver).  Some live on the ISA bus, some are 
> memory-mapped, some are on the PCI bus (though there is not a driver for 
> PCI support yet), and some sit on the end of a serial port (driver is in 
> the works).  I know, it's a mess, but there's not much I can do about 
> these crazy hardware manufacturers.
> 
> I wasn't sure where to handle all this.  The I2C and PCI bus side of 
> things should be handled.  However, the others probably need to sit 
> someplace on a bus, right?  That should probably be handled in the 
> low-level code that actually knows where the hardware sits.

Well, how about handling the devices that already have a struct device
today (like the i2c and pci devices)?  Pass the pointer to that device
into your class_simple_device_add() call.  Then, work on figuring out
where your other devices live on some new bus later.

thanks,

greg k-h