On Tue, 2010-07-27 at 00:52 +0800, Bjorn Helgaas wrote: > On Monday, July 26, 2010 08:46:04 am [email protected] wrote: > > From: Zhao yakui <[email protected]> > > > > This needs a changelog.
OK. I will try to add more changelog about this patch. > > This seems like a complicated solution to a simple problem. Do you have any simple idea to fix the issue? Yes. It will be very simple to merge the IPMI opregion into the drivers/char/ipmi/ipmi_si_intf.c But in fact the a lot of IPMI opregion code has nothing related with the IPMI detection. Maybe it is appropriate to separate the IPMI opregion code into the separated file. BTW: This is also the Corey's viewpoint. So IMO the next discussion had better be based on the condition that the IPMI opregion code is put into the separated file. > > I don't understand why the ACPI IPMI opregion stuff can't be made an > optional feature of the ACPI IPMI driver. Trying to completely decouple > things is just going to add corner cases and weird dependencies. Can the following patch handle the "hot-plug" case of PNP IPI0001 as what you mentioned in last thread? > > Bjorn > > > Signed-off-by: Zhao yakui <[email protected]> > > cc: Bjorn Helgaas <[email protected]> > > --- > > drivers/char/ipmi/ipmi_si_intf.c | 63 > > ++++++++++++++++++++++++++++++++++++++ > > include/linux/ipmi.h | 8 +++++ > > 2 files changed, 71 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/char/ipmi/ipmi_si_intf.c > > b/drivers/char/ipmi/ipmi_si_intf.c > > index 094bdc3..91c5d37 100644 > > --- a/drivers/char/ipmi/ipmi_si_intf.c > > +++ b/drivers/char/ipmi/ipmi_si_intf.c > > @@ -65,6 +65,7 @@ > > #include <linux/string.h> > > #include <linux/ctype.h> > > #include <linux/pnp.h> > > +#include <linux/ipmi.h> > > > > #ifdef CONFIG_PPC_OF > > #include <linux/of_device.h> > > @@ -1907,6 +1908,13 @@ static __devinit void hardcode_find_bmc(void) > > */ > > static int acpi_failure; > > > > +static BLOCKING_NOTIFIER_HEAD(pnp_ipmi_notifier); > > +static LIST_HEAD(pnp_ipmi_list); > > +struct pnp_ipmi_device { > > + struct list_head head; > > + struct pnp_dev *pnp_dev; > > +}; > > + > > /* For GPE-type interrupts. */ > > static u32 ipmi_acpi_gpe(void *context) > > { > > @@ -2124,6 +2132,7 @@ static int __devinit ipmi_pnp_probe(struct pnp_dev > > *dev, > > acpi_handle handle; > > acpi_status status; > > unsigned long long tmp; > > + struct pnp_ipmi_device *pnp_ipmi; > > > > acpi_dev = pnp_acpi_device(dev); > > if (!acpi_dev) > > @@ -2133,6 +2142,11 @@ static int __devinit ipmi_pnp_probe(struct pnp_dev > > *dev, > > if (!info) > > return -ENOMEM; > > > > + pnp_ipmi = kzalloc(sizeof(*pnp_ipmi), GFP_KERNEL); > > + if (!pnp_ipmi) { > > + kfree(info); > > + return -ENOMEM; > > + } > > info->addr_source = SI_ACPI; > > printk(KERN_INFO PFX "probing via ACPI\n"); > > > > @@ -2196,20 +2210,69 @@ static int __devinit ipmi_pnp_probe(struct pnp_dev > > *dev, > > res, info->io.regsize, info->io.regspacing, > > info->irq); > > > > + pnp_ipmi->pnp_dev = dev; > > + list_add_tail(&pnp_ipmi->head, &pnp_ipmi_list); > > + blocking_notifier_call_chain(&pnp_ipmi_notifier, IPMI_PNP_ADD, > > + (void *)dev); > > + > > return add_smi(info); > > > > err_free: > > kfree(info); > > + kfree(pnp_ipmi); > > return -EINVAL; > > } > > > > static void __devexit ipmi_pnp_remove(struct pnp_dev *dev) > > { > > struct smi_info *info = pnp_get_drvdata(dev); > > + struct pnp_ipmi_device *pnp_ipmi; > > + > > + list_for_each_entry(pnp_ipmi, &pnp_ipmi_list, head) { > > + if (pnp_ipmi->pnp_dev == dev) { > > + list_del(&pnp_ipmi->head); > > + blocking_notifier_call_chain(&pnp_ipmi_notifier, > > + IPMI_PNP_REMOVE, (void *)dev); > > + break; > > + } > > + } > > > > cleanup_one_si(info); > > } > > > > +int acpi_ipmi_notifier_register(struct notifier_block *nb) > > +{ > > + int ret; > > + struct pnp_ipmi_device *pnp_ipmi; > > + > > + ret = blocking_notifier_chain_register(&pnp_ipmi_notifier, nb); > > + if (ret == 0) { > > + /* > > + * Maybe we already get the corresponding pnp_ipmi_list before > > + * registering the notifier chain. So call the notifer > > + * chain list for every pnp_ipmi device. > > + */ > > + list_for_each_entry(pnp_ipmi, &pnp_ipmi_list, head) { > > + blocking_notifier_call_chain(&pnp_ipmi_notifier, > > + IPMI_PNP_ADD, (void *)(pnp_ipmi->pnp_dev)); > > + } > > + } > > + return ret; > > +} > > +EXPORT_SYMBOL(acpi_ipmi_notifier_register); > > + > > +int acpi_ipmi_notifier_unregister(struct notifier_block *nb) > > +{ > > + struct pnp_ipmi_device *pnp_ipmi; > > + > > + list_for_each_entry(pnp_ipmi, &pnp_ipmi_list, head) { > > + blocking_notifier_call_chain(&pnp_ipmi_notifier, > > + IPMI_PNP_REMOVE, (void *)(pnp_ipmi->pnp_dev)); > > + } > > + return blocking_notifier_chain_unregister(&pnp_ipmi_notifier, nb); > > +} > > +EXPORT_SYMBOL(acpi_ipmi_notifier_unregister); > > + > > static const struct pnp_device_id pnp_dev_table[] = { > > {"IPI0001", 0}, > > {"", 0}, > > diff --git a/include/linux/ipmi.h b/include/linux/ipmi.h > > index 65aae34..4ea2a69 100644 > > --- a/include/linux/ipmi.h > > +++ b/include/linux/ipmi.h > > @@ -694,4 +694,12 @@ struct ipmi_timing_parms { > > #define IPMICTL_GET_MAINTENANCE_MODE_CMD _IOR(IPMI_IOC_MAGIC, 30, int) > > #define IPMICTL_SET_MAINTENANCE_MODE_CMD _IOW(IPMI_IOC_MAGIC, 31, int) > > > > +#ifdef CONFIG_ACPI > > +#define IPMI_PNP_ADD 1 > > +#define IPMI_PNP_REMOVE 2 > > +extern int acpi_ipmi_notifier_register(struct notifier_block *nb); > > +extern int acpi_ipmi_notifier_unregister(struct notifier_block *nb); > > + > > +#endif > > + > > #endif /* __LINUX_IPMI_H */ > > ------------------------------------------------------------------------------ The Palm PDK Hot Apps Program offers developers who use the Plug-In Development Kit to bring their C/C++ apps to Palm for a share of $1 Million in cash or HP Products. Visit us here for more details: http://ad.doubleclick.net/clk;226879339;13503038;l? http://clk.atdmt.com/CRS/go/247765532/direct/01/ _______________________________________________ Openipmi-developer mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openipmi-developer
