On Fri, Feb 22, 2019 at 05:11:49PM +0800, Yang Yingliang wrote: > Tested-by: Yang Yingliang <[email protected]>
Thanks for testing. This is queued for 5.1. I'd like to let the soak a bit in linux-next. It is tagged for a backport to stable releases. -corey > > [ 128.069528] ipmi_si: IPMI System Interface driver > [ 128.069556] ipmi_si dmi-ipmi-si.0: ipmi_platform: probing via SMBIOS > [ 128.069559] ipmi_platform: ipmi_si: SMBIOS: io 0xe4 regsize 1 spacing 1 > irq 0 > [ 128.069561] ipmi_si: Adding SMBIOS-specified bt state machine > [ 128.069657] ipmi_si hisi-lpc-ipmi.1.auto: ipmi_platform: probing via ACPI > [ 128.069694] ipmi_si hisi-lpc-ipmi.1.auto: ipmi_platform: [io > 0xffc0e3-0xffc0e7] regsize 1 spacing 1 irq 0 > [ 128.069696] ipmi_si hisi-lpc-ipmi.1.auto: Hard-coded device at this > address already exists > [ 128.069815] ipmi_si hardcode-ipmi-si.0: ipmi_platform: probing via > hardcoded > [ 128.069817] ipmi_platform: ipmi_si: hardcoded: io 0xffc0e3 regsize 1 > spacing 1 irq 0 > [ 128.069819] ipmi_si: Adding hardcoded-specified bt state machine > [ 128.069892] ipmi_si: Trying SMBIOS-specified bt state machine at i/o > address 0xe4, slave address 0x0, irq 0 > [ 128.069896] ipmi_si dmi-ipmi-si.0: Could not set up I/O space > [ 128.069900] ipmi_si: Trying hardcoded-specified bt state machine at i/o > address 0xffc0e3, slave address 0x0, irq 0 > [ 128.081782] ipmi_si hardcode-ipmi-si.0: bt cap response too short: 3 > [ 128.081784] ipmi_si hardcode-ipmi-si.0: using default values > [ 128.081786] ipmi_si hardcode-ipmi-si.0: req2rsp=5 secs retries=2 > [ 128.157855] ipmi_si hardcode-ipmi-si.0: IPMI message handler: Found new > BMC (man_id: 0x0007db, prod_id: 0x0001, dev_id: 0x01) > [ 128.441860] ipmi_si hardcode-ipmi-si.0: IPMI bt interface initialized > > On 2019/2/22 2:33, [email protected] wrote: > > From: Corey Minyard <[email protected]> > > > > When excuting a command like: > > modprobe ipmi_si ports=0xffc0e3 type=bt > > The system would get an oops. > > > > The trouble here is that ipmi_si_hardcode_find_bmc() is called before > > ipmi_si_platform_init(), but initialization of the hard-coded device > > creates an IPMI platform device, which won't be initialized yet. > > > > The real trouble is that hard-coded devices aren't created with > > any device, and the fixup is done later. So do it right, create the > > hard-coded devices as normal platform devices. > > > > This required adding some new resource types to the IPMI platform > > code for passing information required by the hard-coded device > > and adding some code to remove the hard-coded platform devices > > on module removal. > > > > To enforce the "hard-coded devices passed by the user take priority > > over firmware devices" rule, some special code was added to check > > and see if a hard-coded device already exists. > > > > Reported-by: Yang Yingliang <[email protected]> > > Signed-off-by: Corey Minyard <[email protected]> > > --- > > > > I believe this is the right fix. Can you test/review and send the > > results? > > > > Thanks, > > > > -corey > > > > drivers/char/ipmi/ipmi_si.h | 4 +- > > drivers/char/ipmi/ipmi_si_hardcode.c | 236 ++++++++++++++++++++------- > > drivers/char/ipmi/ipmi_si_intf.c | 23 ++- > > drivers/char/ipmi/ipmi_si_platform.c | 25 ++- > > 4 files changed, 214 insertions(+), 74 deletions(-) > > > > diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h > > index 52f6152d1fcb..7ae52c17618e 100644 > > --- a/drivers/char/ipmi/ipmi_si.h > > +++ b/drivers/char/ipmi/ipmi_si.h > > @@ -25,7 +25,9 @@ void ipmi_irq_finish_setup(struct si_sm_io *io); > > int ipmi_si_remove_by_dev(struct device *dev); > > void ipmi_si_remove_by_data(int addr_space, enum si_type si_type, > > unsigned long addr); > > -int ipmi_si_hardcode_find_bmc(void); > > +void ipmi_hardcode_init(void); > > +void ipmi_si_hardcode_exit(void); > > +int ipmi_si_hardcode_match(int addr_type, unsigned long addr); > > void ipmi_si_platform_init(void); > > void ipmi_si_platform_shutdown(void); > > diff --git a/drivers/char/ipmi/ipmi_si_hardcode.c > > b/drivers/char/ipmi/ipmi_si_hardcode.c > > index 487642809c58..1e5783961b0d 100644 > > --- a/drivers/char/ipmi/ipmi_si_hardcode.c > > +++ b/drivers/char/ipmi/ipmi_si_hardcode.c > > @@ -3,6 +3,7 @@ > > #define pr_fmt(fmt) "ipmi_hardcode: " fmt > > #include <linux/moduleparam.h> > > +#include <linux/platform_device.h> > > #include "ipmi_si.h" > > /* > > @@ -12,23 +13,22 @@ > > #define SI_MAX_PARMS 4 > > -static char *si_type[SI_MAX_PARMS]; > > #define MAX_SI_TYPE_STR 30 > > -static char si_type_str[MAX_SI_TYPE_STR]; > > +static char si_type_str[MAX_SI_TYPE_STR] __initdata; > > static unsigned long addrs[SI_MAX_PARMS]; > > static unsigned int num_addrs; > > static unsigned int ports[SI_MAX_PARMS]; > > static unsigned int num_ports; > > -static int irqs[SI_MAX_PARMS]; > > -static unsigned int num_irqs; > > -static int regspacings[SI_MAX_PARMS]; > > -static unsigned int num_regspacings; > > -static int regsizes[SI_MAX_PARMS]; > > -static unsigned int num_regsizes; > > -static int regshifts[SI_MAX_PARMS]; > > -static unsigned int num_regshifts; > > -static int slave_addrs[SI_MAX_PARMS]; /* Leaving 0 chooses the default > > value */ > > -static unsigned int num_slave_addrs; > > +static int irqs[SI_MAX_PARMS] __initdata; > > +static unsigned int num_irqs __initdata; > > +static int regspacings[SI_MAX_PARMS] __initdata; > > +static unsigned int num_regspacings __initdata; > > +static int regsizes[SI_MAX_PARMS] __initdata; > > +static unsigned int num_regsizes __initdata; > > +static int regshifts[SI_MAX_PARMS] __initdata; > > +static unsigned int num_regshifts __initdata; > > +static int slave_addrs[SI_MAX_PARMS] __initdata; > > +static unsigned int num_slave_addrs __initdata; > > module_param_string(type, si_type_str, MAX_SI_TYPE_STR, 0); > > MODULE_PARM_DESC(type, "Defines the type of each interface, each" > > @@ -73,12 +73,133 @@ MODULE_PARM_DESC(slave_addrs, "Set the default IPMB > > slave address for" > > " overridden by this parm. This is an array indexed" > > " by interface number."); > > -int ipmi_si_hardcode_find_bmc(void) > > +static struct platform_device *ipmi_hc_pdevs[SI_MAX_PARMS]; > > + > > +static void __init ipmi_hardcode_init_one(const char *si_type_str, > > + unsigned int i, > > + unsigned long addr, > > + unsigned int flags) > > { > > - int ret = -ENODEV; > > - int i; > > - struct si_sm_io io; > > + struct platform_device *pdev; > > + unsigned int num_r = 1, size; > > + struct resource r[4]; > > + struct property_entry p[6]; > > + enum si_type si_type; > > + unsigned int regspacing, regsize; > > + int rv; > > + > > + memset(p, 0, sizeof(p)); > > + memset(r, 0, sizeof(r)); > > + > > + if (!si_type_str || !*si_type_str || strcmp(si_type_str, "kcs") == 0) { > > + size = 2; > > + si_type = SI_KCS; > > + } else if (strcmp(si_type_str, "smic") == 0) { > > + size = 2; > > + si_type = SI_SMIC; > > + } else if (strcmp(si_type_str, "bt") == 0) { > > + size = 3; > > + si_type = SI_BT; > > + } else if (strcmp(si_type_str, "invalid") == 0) { > > + /* > > + * Allow a firmware-specified interface to be > > + * disabled. > > + */ > > + size = 1; > > + si_type = SI_TYPE_INVALID; > > + } else { > > + pr_warn("Interface type specified for interface %d, was > > invalid: %s\n", > > + i, si_type_str); > > + return; > > + } > > + > > + regsize = regsizes[i]; > > + if (regsize == 0) > > + regsize = DEFAULT_REGSIZE; > > + > > + p[0] = PROPERTY_ENTRY_U8("ipmi-type", si_type); > > + p[1] = PROPERTY_ENTRY_U8("slave-addr", slave_addrs[i]); > > + p[2] = PROPERTY_ENTRY_U8("addr-source", SI_HARDCODED); > > + p[3] = PROPERTY_ENTRY_U8("reg-shift", regshifts[i]); > > + p[4] = PROPERTY_ENTRY_U8("reg-size", regsize); > > + /* Last entry must be left NULL to terminate it. */ > > + > > + /* > > + * Register spacing is derived from the resources in > > + * the IPMI platform code. > > + */ > > + regspacing = regspacings[i]; > > + if (regspacing == 0) > > + regspacing = regsize; > > + > > + r[0].start = addr; > > + r[0].end = r[0].start + regsize - 1; > > + r[0].name = "IPMI Address 1"; > > + r[0].flags = flags; > > + > > + if (size > 1) { > > + r[1].start = r[0].start + regspacing; > > + r[1].end = r[1].start + regsize - 1; > > + r[1].name = "IPMI Address 2"; > > + r[1].flags = flags; > > + num_r++; > > + } > > + > > + if (size > 2) { > > + r[2].start = r[1].start + regspacing; > > + r[2].end = r[2].start + regsize - 1; > > + r[2].name = "IPMI Address 3"; > > + r[2].flags = flags; > > + num_r++; > > + } > > + > > + if (irqs[i]) { > > + r[num_r].start = irqs[i]; > > + r[num_r].end = irqs[i]; > > + r[num_r].name = "IPMI IRQ"; > > + r[num_r].flags = IORESOURCE_IRQ; > > + num_r++; > > + } > > + > > + pdev = platform_device_alloc("hardcode-ipmi-si", i); > > + if (!pdev) { > > + pr_err("Error allocating IPMI platform device %d\n", i); > > + return; > > + } > > + > > + rv = platform_device_add_resources(pdev, r, num_r); > > + if (rv) { > > + dev_err(&pdev->dev, > > + "Unable to add hard-code resources: %d\n", rv); > > + goto err; > > + } > > + > > + rv = platform_device_add_properties(pdev, p); > > + if (rv) { > > + dev_err(&pdev->dev, > > + "Unable to add hard-code properties: %d\n", rv); > > + goto err; > > + } > > + > > + rv = platform_device_add(pdev); > > + if (rv) { > > + dev_err(&pdev->dev, > > + "Unable to add hard-code device: %d\n", rv); > > + goto err; > > + } > > + > > + ipmi_hc_pdevs[i] = pdev; > > + return; > > + > > +err: > > + platform_device_put(pdev); > > +} > > + > > +void __init ipmi_hardcode_init(void) > > +{ > > + unsigned int i; > > char *str; > > + char *si_type[SI_MAX_PARMS]; > > /* Parse out the si_type string into its components. */ > > str = si_type_str; > > @@ -95,54 +216,45 @@ int ipmi_si_hardcode_find_bmc(void) > > } > > } > > - memset(&io, 0, sizeof(io)); > > for (i = 0; i < SI_MAX_PARMS; i++) { > > - if (!ports[i] && !addrs[i]) > > - continue; > > - > > - io.addr_source = SI_HARDCODED; > > - pr_info("probing via hardcoded address\n"); > > - > > - if (!si_type[i] || strcmp(si_type[i], "kcs") == 0) { > > - io.si_type = SI_KCS; > > - } else if (strcmp(si_type[i], "smic") == 0) { > > - io.si_type = SI_SMIC; > > - } else if (strcmp(si_type[i], "bt") == 0) { > > - io.si_type = SI_BT; > > - } else { > > - pr_warn("Interface type specified for interface %d, was > > invalid: %s\n", > > - i, si_type[i]); > > - continue; > > - } > > + if (i < num_ports && ports[i]) > > + ipmi_hardcode_init_one(si_type[i], i, ports[i], > > + IORESOURCE_IO); > > + if (i < num_addrs && addrs[i]) > > + ipmi_hardcode_init_one(si_type[i], i, addrs[i], > > + IORESOURCE_MEM); > > + } > > +} > > - if (ports[i]) { > > - /* An I/O port */ > > - io.addr_data = ports[i]; > > - io.addr_type = IPMI_IO_ADDR_SPACE; > > - } else if (addrs[i]) { > > - /* A memory port */ > > - io.addr_data = addrs[i]; > > - io.addr_type = IPMI_MEM_ADDR_SPACE; > > - } else { > > - pr_warn("Interface type specified for interface %d, but > > port and address were not set or set to zero\n", > > - i); > > - continue; > > - } > > +void ipmi_si_hardcode_exit(void) > > +{ > > + unsigned int i; > > - io.addr = NULL; > > - io.regspacing = regspacings[i]; > > - if (!io.regspacing) > > - io.regspacing = DEFAULT_REGSPACING; > > - io.regsize = regsizes[i]; > > - if (!io.regsize) > > - io.regsize = DEFAULT_REGSIZE; > > - io.regshift = regshifts[i]; > > - io.irq = irqs[i]; > > - if (io.irq) > > - io.irq_setup = ipmi_std_irq_setup; > > - io.slave_addr = slave_addrs[i]; > > - > > - ret = ipmi_si_add_smi(&io); > > + for (i = 0; i < SI_MAX_PARMS; i++) { > > + if (ipmi_hc_pdevs[i]) > > + platform_device_unregister(ipmi_hc_pdevs[i]); > > } > > - return ret; > > +} > > + > > +/* > > + * Returns true of the given address exists as a hardcoded address, > > + * false if not. > > + */ > > +int ipmi_si_hardcode_match(int addr_type, unsigned long addr) > > +{ > > + unsigned int i; > > + > > + if (addr_type == IPMI_IO_ADDR_SPACE) { > > + for (i = 0; i < num_ports; i++) { > > + if (ports[i] == addr) > > + return 1; > > + } > > + } else { > > + for (i = 0; i < num_addrs; i++) { > > + if (addrs[i] == addr) > > + return 1; > > + } > > + } > > + > > + return 0; > > } > > diff --git a/drivers/char/ipmi/ipmi_si_intf.c > > b/drivers/char/ipmi/ipmi_si_intf.c > > index ae99d6a14789..abbd526626d5 100644 > > --- a/drivers/char/ipmi/ipmi_si_intf.c > > +++ b/drivers/char/ipmi/ipmi_si_intf.c > > @@ -1865,6 +1865,18 @@ int ipmi_si_add_smi(struct si_sm_io *io) > > int rv = 0; > > struct smi_info *new_smi, *dup; > > + /* > > + * If the user gave us a hard-coded device at the same > > + * address, they presumably want us to use it and not what is > > + * in the firmware. > > + */ > > + if (io->addr_source != SI_HARDCODED && > > + ipmi_si_hardcode_match(io->addr_type, io->addr_data)) { > > + dev_info(io->dev, > > + "Hard-coded device at this address already exists"); > > + return -ENODEV; > > + } > > + > > if (!io->io_setup) { > > if (io->addr_type == IPMI_IO_ADDR_SPACE) { > > io->io_setup = ipmi_si_port_setup; > > @@ -2097,7 +2109,7 @@ static int try_smi_init(struct smi_info *new_smi) > > return rv; > > } > > -static int init_ipmi_si(void) > > +static int __init init_ipmi_si(void) > > { > > struct smi_info *e; > > enum ipmi_addr_src type = SI_INVALID; > > @@ -2105,11 +2117,9 @@ static int init_ipmi_si(void) > > if (initialized) > > return 0; > > - pr_info("IPMI System Interface driver\n"); > > + ipmi_hardcode_init(); > > - /* If the user gave us a device, they presumably want us to use it */ > > - if (!ipmi_si_hardcode_find_bmc()) > > - goto do_scan; > > + pr_info("IPMI System Interface driver\n"); > > ipmi_si_platform_init(); > > @@ -2121,7 +2131,6 @@ static int init_ipmi_si(void) > > with multiple BMCs we assume that there will be several instances > > of a given type so if we succeed in registering a type then also > > try to register everything else of the same type */ > > -do_scan: > > mutex_lock(&smi_infos_lock); > > list_for_each_entry(e, &smi_infos, link) { > > /* Try to register a device if it has an IRQ and we either > > @@ -2307,6 +2316,8 @@ static void cleanup_ipmi_si(void) > > list_for_each_entry_safe(e, tmp_e, &smi_infos, link) > > cleanup_one_si(e); > > mutex_unlock(&smi_infos_lock); > > + > > + ipmi_si_hardcode_exit(); > > } > > module_exit(cleanup_ipmi_si); > > diff --git a/drivers/char/ipmi/ipmi_si_platform.c > > b/drivers/char/ipmi/ipmi_si_platform.c > > index 15cf819f884f..f8f85416d47a 100644 > > --- a/drivers/char/ipmi/ipmi_si_platform.c > > +++ b/drivers/char/ipmi/ipmi_si_platform.c > > @@ -128,8 +128,6 @@ ipmi_get_info_from_resources(struct platform_device > > *pdev, > > if (res_second->start > io->addr_data) > > io->regspacing = res_second->start - io->addr_data; > > } > > - io->regsize = DEFAULT_REGSIZE; > > - io->regshift = 0; > > return res; > > } > > @@ -137,7 +135,7 @@ ipmi_get_info_from_resources(struct platform_device > > *pdev, > > static int platform_ipmi_probe(struct platform_device *pdev) > > { > > struct si_sm_io io; > > - u8 type, slave_addr, addr_source; > > + u8 type, slave_addr, addr_source, regsize, regshift; > > int rv; > > rv = device_property_read_u8(&pdev->dev, "addr-source", &addr_source); > > @@ -149,7 +147,7 @@ static int platform_ipmi_probe(struct platform_device > > *pdev) > > if (addr_source == SI_SMBIOS) { > > if (!si_trydmi) > > return -ENODEV; > > - } else { > > + } else if (addr_source != SI_HARDCODED) { > > if (!si_tryplatform) > > return -ENODEV; > > } > > @@ -169,11 +167,23 @@ static int platform_ipmi_probe(struct platform_device > > *pdev) > > case SI_BT: > > io.si_type = type; > > break; > > + case SI_TYPE_INVALID: /* User disabled this in hardcode. */ > > + return -ENODEV; > > default: > > dev_err(&pdev->dev, "ipmi-type property is invalid\n"); > > return -EINVAL; > > } > > + io.regsize = DEFAULT_REGSIZE; > > + rv = device_property_read_u8(&pdev->dev, "reg-size", ®size); > > + if (!rv) > > + io.regsize = regsize; > > + > > + io.regshift = 0; > > + rv = device_property_read_u8(&pdev->dev, "reg-shift", ®shift); > > + if (!rv) > > + io.regshift = regshift; > > + > > if (!ipmi_get_info_from_resources(pdev, &io)) > > return -EINVAL; > > @@ -193,7 +203,8 @@ static int platform_ipmi_probe(struct platform_device > > *pdev) > > io.dev = &pdev->dev; > > - pr_info("ipmi_si: SMBIOS: %s %#lx regsize %d spacing %d irq %d\n", > > + pr_info("ipmi_si: %s: %s %#lx regsize %d spacing %d irq %d\n", > > + ipmi_addr_src_to_str(addr_source), > > (io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem", > > io.addr_data, io.regsize, io.regspacing, io.irq); > > @@ -358,6 +369,9 @@ static int acpi_ipmi_probe(struct platform_device *pdev) > > goto err_free; > > } > > + io.regsize = DEFAULT_REGSIZE; > > + io.regshift = 0; > > + > > res = ipmi_get_info_from_resources(pdev, &io); > > if (!res) { > > rv = -EINVAL; > > @@ -421,6 +435,7 @@ static int ipmi_remove(struct platform_device *pdev) > > static const struct platform_device_id si_plat_ids[] = { > > { "dmi-ipmi-si", 0 }, > > + { "hardcode-ipmi-si", 0 }, > > { } > > }; > > _______________________________________________ Openipmi-developer mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openipmi-developer
