Hi,
first:
Corey's email address in MAINTAINERS is: [email protected]
But the last commits where from: [email protected]
I added both now in this mail. Just a heads up if the
MAINTAINERS email address is or will be invalid...
I try to understand why or whether there can be improvment
in (auto-) loading ipmi drivers.
Some things I realized which look as they could be improved:
- ipmi_devintf not loaded
When impi_si is loaded, ipmi_msghandler gets loaded, but
the user interface is missing.
This might be wanted if only watchdog functionality is needed,
but nice would be if ipmi_si or ipmi_msghandler would load
this one automatically (at least via a y/n Kconfig statically
or even dynamically by e.g. making ipmi_devinitf part of
ipmi_msghandler, for the latter, find an example patch at
the end).
When trying on a system which has no IPMI controller:
- acpi_ipmi succeeds and stays loaded
- when trying to load ipmi_si, it fails, but ipmi_msghandler
got requested and stays loaded
I try to understand why this is and whether it's worth digging
deeper into this and try to improve things.
Typically I guess one wants to have the HW controlling driver
(e.g. ipmi_si, what others are there?) to autoload, requesting
all other useful drivers like ipmi_msghandler, ipmi_devintf and
if ACPI tables show an IPM adress space handler acpi_ipmi.
If there is no underlying HW driver all the others: ipmi_msghandler,
ipmi_devintf, acpi_ipmi should not load successfully?
Thanks for any hints. I am willing to look deeper into this (if
it makes sense and there hasn't be a consense like: Does not work
because...).
Find attached a dirty patch, which still needs cleanup.
This one should address one of above issues.
It is compile tested only (CONFIG_IPMI_HANDLER=m).
EXAMPLE PATCH DO NOT ADD!!!
ipmi: Setup ipmi_devintf automatically if ipmi_msghandler gets loaded
This removes the ipmi_devintf to be a module, but it will automatically
compiled in if ipmi_msghandler is set.
ipmi_devintf is really small and typically is always wanted if IPMI
functionality is used. If really needed, a boot param could be added
to not export userspace interface to the ipmi_msghandler driver, but
I cannot see why this would be needed...
Thanks,
Thomas
Signed-off-by: Thomas Renninger <[email protected]>
Index: kernel_ipmi/drivers/char/ipmi/Kconfig
===================================================================
--- kernel_ipmi.orig/drivers/char/ipmi/Kconfig
+++ kernel_ipmi/drivers/char/ipmi/Kconfig
@@ -37,12 +37,6 @@ config IPMI_PANIC_STRING
You can fetch these events and use the sequence numbers to piece the
string together.
-config IPMI_DEVICE_INTERFACE
- tristate 'Device interface for IPMI'
- help
- This provides an IOCTL interface to the IPMI message handler so
- userland processes may use IPMI. It supports poll() and select().
-
config IPMI_SI
tristate 'IPMI System Interface handler'
help
Index: kernel_ipmi/drivers/char/ipmi/Makefile
===================================================================
--- kernel_ipmi.orig/drivers/char/ipmi/Makefile
+++ kernel_ipmi/drivers/char/ipmi/Makefile
@@ -5,7 +5,7 @@
ipmi_si-y := ipmi_si_intf.o ipmi_kcs_sm.o ipmi_smic_sm.o ipmi_bt_sm.o
obj-$(CONFIG_IPMI_HANDLER) += ipmi_msghandler.o
-obj-$(CONFIG_IPMI_DEVICE_INTERFACE) += ipmi_devintf.o
+obj-$(CONFIG_IPMI_HANDLER) += ipmi_devintf.o
obj-$(CONFIG_IPMI_SI) += ipmi_si.o
obj-$(CONFIG_IPMI_WATCHDOG) += ipmi_watchdog.o
obj-$(CONFIG_IPMI_POWEROFF) += ipmi_poweroff.o
Index: kernel_ipmi/drivers/char/ipmi/ipmi_devintf.c
===================================================================
--- kernel_ipmi.orig/drivers/char/ipmi/ipmi_devintf.c
+++ kernel_ipmi/drivers/char/ipmi/ipmi_devintf.c
@@ -928,7 +928,7 @@ static struct ipmi_smi_watcher smi_watch
.smi_gone = ipmi_smi_gone,
};
-static int __init init_ipmi_devintf(void)
+int __init init_ipmi_devintf(void)
{
int rv;
@@ -964,9 +964,8 @@ static int __init init_ipmi_devintf(void
return 0;
}
-module_init(init_ipmi_devintf);
-static void __exit cleanup_ipmi(void)
+void cleanup_ipmi_dev(void)
{
struct ipmi_reg_list *entry, *entry2;
mutex_lock(®_list_mutex);
@@ -980,7 +979,6 @@ static void __exit cleanup_ipmi(void)
ipmi_smi_watcher_unregister(&smi_watcher);
unregister_chrdev(ipmi_major, DEVICE_NAME);
}
-module_exit(cleanup_ipmi);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Corey Minyard <[email protected]>");
Index: kernel_ipmi/drivers/char/ipmi/ipmi_msghandler.c
===================================================================
--- kernel_ipmi.orig/drivers/char/ipmi/ipmi_msghandler.c
+++ kernel_ipmi/drivers/char/ipmi/ipmi_msghandler.c
@@ -4545,6 +4545,7 @@ static int ipmi_init_msghandler(void)
proc_ipmi_root = proc_mkdir("ipmi", NULL);
if (!proc_ipmi_root) {
printk(KERN_ERR PFX "Unable to create IPMI proc dir");
+ driver_unregister(&ipmidriver.driver);
return -ENOMEM;
}
@@ -4560,13 +4561,27 @@ static int ipmi_init_msghandler(void)
return 0;
}
+void cleanup_ipmi_dev(void);
+static void cleanup_ipmi(void);
+int init_ipmi_devintf(void);
+
+
static int __init ipmi_init_msghandler_mod(void)
{
- ipmi_init_msghandler();
+ int ret;
+
+ ret = ipmi_init_msghandler();
+ if (ret)
+ return ret;
+ ret = init_ipmi_devintf();
+ if (ret) {
+ cleanup_ipmi();
+ return ret;
+ }
return 0;
}
-static void __exit cleanup_ipmi(void)
+static void cleanup_ipmi(void)
{
int count;
@@ -4605,6 +4620,7 @@ static void __exit cleanup_ipmi(void)
if (count != 0)
printk(KERN_WARNING PFX "recv message count %d at exit\n",
count);
+ cleanup_ipmi_dev();
}
module_exit(cleanup_ipmi);
------------------------------------------------------------------------------
Slashdot TV. Videos for Nerds. Stuff that Matters.
http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk
_______________________________________________
Openipmi-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
