From 5ac7b2fccd0cc2e1451d5d5388dad69e858fa0d4 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 27 Oct 2016 10:12:18 -0500 Subject: [PATCH 1/8] ipmi: Periodically check for events, not messages Commit d9b7e4f717a1 ("ipmi: Periodically check to see if irqs and messages are set right") to verify the contents of global events. However, the wrong function was being called in some cases, checking for messages, not events. Signed-off-by: Corey Minyard Tested-by: Jason DiPietro --- drivers/char/ipmi/ipmi_si_intf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index a112c0146012e..cb451088a4afc 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -789,7 +789,7 @@ static void handle_transaction_done(struct smi_info *smi_info) smi_info->si_state = SI_NORMAL; break; } - start_getting_msg_queue(smi_info); + start_getting_events(smi_info); } else { smi_info->si_state = SI_NORMAL; } @@ -812,7 +812,7 @@ static void handle_transaction_done(struct smi_info *smi_info) smi_info->si_state = SI_NORMAL; break; } - start_getting_msg_queue(smi_info); + start_getting_events(smi_info); } else { smi_info->si_state = SI_NORMAL; } From 56189b5f2f0f386838f278e93654d2e2cbb51c9a Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Tue, 25 Oct 2016 20:31:23 -0500 Subject: [PATCH 2/8] ipmi_ssif: Remove an unused module parameter Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_ssif.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 5673ffff00be7..04232431eaf7a 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1154,10 +1154,6 @@ static bool ssif_dbg_probe; module_param_named(dbg_probe, ssif_dbg_probe, bool, 0); MODULE_PARM_DESC(dbg_probe, "Enable debugging of probing of adapters."); -static int use_thread; -module_param(use_thread, int, 0); -MODULE_PARM_DESC(use_thread, "Use the thread interface."); - static bool ssif_tryacpi = true; module_param_named(tryacpi, ssif_tryacpi, bool, 0); MODULE_PARM_DESC(tryacpi, "Setting this to zero will disable the default scan of the interfaces identified via ACPI"); From c11daf6a8fcc42d540ad6d7bb06a6a203de908b3 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Tue, 25 Oct 2016 20:31:53 -0500 Subject: [PATCH 3/8] ipmi: Update documentation The documentation has some information that was old and needed some things added that are new. Signed-off-by: Corey Minyard --- Documentation/IPMI.txt | 57 ++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/Documentation/IPMI.txt b/Documentation/IPMI.txt index c0d8788e75d3f..72292308d0f5b 100644 --- a/Documentation/IPMI.txt +++ b/Documentation/IPMI.txt @@ -111,6 +111,8 @@ ipmi_ssif - A driver for accessing BMCs on the SMBus. It uses the I2C kernel driver's SMBus interfaces to send and receive IPMI messages over the SMBus. +ipmi_powernv - A driver for access BMCs on POWERNV systems. + ipmi_watchdog - IPMI requires systems to have a very capable watchdog timer. This driver implements the standard Linux watchdog timer interface on top of the IPMI message handler. @@ -118,17 +120,15 @@ interface on top of the IPMI message handler. ipmi_poweroff - Some systems support the ability to be turned off via IPMI commands. -These are all individually selectable via configuration options. +bt-bmc - This is not part of the main driver, but instead a driver for +accessing a BMC-side interface of a BT interface. It is used on BMCs +running Linux to provide an interface to the host. -Note that the KCS-only interface has been removed. The af_ipmi driver -is no longer supported and has been removed because it was impossible -to do 32 bit emulation on 64-bit kernels with it. +These are all individually selectable via configuration options. Much documentation for the interface is in the include files. The IPMI include files are: -net/af_ipmi.h - Contains the socket interface. - linux/ipmi.h - Contains the user interface and IOCTL interface for IPMI. linux/ipmi_smi.h - Contains the interface for system management interfaces @@ -245,6 +245,16 @@ addressed (because some boards actually have multiple BMCs on them) and the user should not have to care what type of SMI is below them. +Watching For Interfaces + +When your code comes up, the IPMI driver may or may not have detected +if IPMI devices exist. So you might have to defer your setup until +the device is detected, or you might be able to do it immediately. +To handle this, and to allow for discovery, you register an SMI +watcher with ipmi_smi_watcher_register() to iterate over interfaces +and tell you when they come and go. + + Creating the User To user the message handler, you must first create a user using @@ -263,7 +273,7 @@ closing the device automatically destroys the user. Messaging -To send a message from kernel-land, the ipmi_request() call does +To send a message from kernel-land, the ipmi_request_settime() call does pretty much all message handling. Most of the parameter are self-explanatory. However, it takes a "msgid" parameter. This is NOT the sequence number of messages. It is simply a long value that is @@ -352,11 +362,12 @@ that for more details. The SI Driver ------------- -The SI driver allows up to 4 KCS or SMIC interfaces to be configured -in the system. By default, scan the ACPI tables for interfaces, and -if it doesn't find any the driver will attempt to register one KCS -interface at the spec-specified I/O port 0xca2 without interrupts. -You can change this at module load time (for a module) with: +The SI driver allows KCS, BT, and SMIC interfaces to be configured +in the system. It discovers interfaces through a host of different +methods, depending on the system. + +You can specify up to four interfaces on the module load line and +control some module parameters: modprobe ipmi_si.o type=,.... ports=,... addrs=,... @@ -367,7 +378,7 @@ You can change this at module load time (for a module) with: force_kipmid=,,... kipmid_max_busy_us=,,... unload_when_empty=[0|1] - trydefaults=[0|1] trydmi=[0|1] tryacpi=[0|1] + trydmi=[0|1] tryacpi=[0|1] tryplatform=[0|1] trypci=[0|1] Each of these except try... items is a list, the first item for the @@ -386,10 +397,6 @@ use the I/O port given as the device address. If you specify irqs as non-zero for an interface, the driver will attempt to use the given interrupt for the device. -trydefaults sets whether the standard IPMI interface at 0xca2 and -any interfaces specified by ACPE are tried. By default, the driver -tries it, set this value to zero to turn this off. - The other try... items disable discovery by their corresponding names. These are all enabled by default, set them to zero to disable them. The tryplatform disables openfirmware. @@ -434,7 +441,7 @@ kernel command line as: ipmi_si.type=,... ipmi_si.ports=,... ipmi_si.addrs=,... - ipmi_si.irqs=,... ipmi_si.trydefaults=[0|1] + ipmi_si.irqs=,... ipmi_si.regspacings=,,... ipmi_si.regsizes=,,... ipmi_si.regshifts=,,... @@ -444,11 +451,6 @@ kernel command line as: It works the same as the module parameters of the same names. -By default, the driver will attempt to detect any device specified by -ACPI, and if none of those then a KCS device at the spec-specified -0xca2. If you want to turn this off, set the "trydefaults" option to -false. - If your IPMI interface does not support interrupts and is a KCS or SMIC interface, the IPMI driver will start a kernel thread for the interface to help speed things up. This is a low-priority kernel @@ -500,7 +502,8 @@ at module load time (for a module) with: addr=[,[,...]] adapter=[,[...]] dbg=,... - slave_addrs=,,... + slave_addrs=,,... + tryacpi=[0|1] trydmi=[0|1] [dbg_probe=1] The addresses are normal I2C addresses. The adapter is the string @@ -513,6 +516,9 @@ spaces in kernel parameters. The debug flags are bit flags for each BMC found, they are: IPMI messages: 1, driver state: 2, timing: 4, I2C probe: 8 +The tryxxx parameters can be used to disable detecting interfaces +from various sources. + Setting dbg_probe to 1 will enable debugging of the probing and detection process for BMCs on the SMBusses. @@ -535,7 +541,8 @@ kernel command line as: ipmi_ssif.adapter=[,[...]] ipmi_ssif.dbg=[,[...]] ipmi_ssif.dbg_probe=1 - ipmi_ssif.slave_addrs=[,[...]] + ipmi_ssif.slave_addrs=[,[...]] + ipmi_ssif.tryacpi=[0|1] ipmi_ssif.trydmi=[0|1] These are the same options as on the module command line. From 1abf71eef32c7f242fe30ea66a635e297b2c1c8d Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Mon, 7 Nov 2016 07:37:06 -0700 Subject: [PATCH 4/8] Move platform device creation earlier in the initialization Some logs are printed out early using smi->dev, but on a platform device that is not created until later. So move the creation of that device structure earlier in the sequence so it can be used for printing. Signed-off-by: Corey Minyard Tested-by: Corentin Labbe --- drivers/char/ipmi/ipmi_si_intf.c | 46 +++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index cb451088a4afc..751c2815fc2a1 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -3502,6 +3502,7 @@ static int try_smi_init(struct smi_info *new_smi) { int rv = 0; int i; + char *init_name = NULL; printk(KERN_INFO PFX "Trying %s-specified %s state" " machine at %s address 0x%lx, slave address 0x%x," @@ -3531,6 +3532,26 @@ static int try_smi_init(struct smi_info *new_smi) goto out_err; } + /* Do this early so it's available for logs. */ + if (!new_smi->dev) { + init_name = kasprintf(GFP_KERNEL, "ipmi_si.%d", 0); + + /* + * If we don't already have a device from something + * else (like PCI), then register a new one. + */ + new_smi->pdev = platform_device_alloc("ipmi_si", + new_smi->intf_num); + if (!new_smi->pdev) { + pr_err(PFX "Unable to allocate platform device\n"); + goto out_err; + } + new_smi->dev = &new_smi->pdev->dev; + new_smi->dev->driver = &ipmi_driver.driver; + /* Nulled by device_add() */ + new_smi->dev->init_name = init_name; + } + /* Allocate the state machine's data and initialize it. */ new_smi->si_sm = kmalloc(new_smi->handlers->size(), GFP_KERNEL); if (!new_smi->si_sm) { @@ -3604,21 +3625,7 @@ static int try_smi_init(struct smi_info *new_smi) atomic_set(&new_smi->req_events, 1); } - if (!new_smi->dev) { - /* - * If we don't already have a device from something - * else (like PCI), then register a new one. - */ - new_smi->pdev = platform_device_alloc("ipmi_si", - new_smi->intf_num); - if (!new_smi->pdev) { - printk(KERN_ERR PFX - "Unable to allocate platform device\n"); - goto out_err; - } - new_smi->dev = &new_smi->pdev->dev; - new_smi->dev->driver = &ipmi_driver.driver; - + if (new_smi->pdev) { rv = platform_device_add(new_smi->pdev); if (rv) { printk(KERN_ERR PFX @@ -3668,6 +3675,9 @@ static int try_smi_init(struct smi_info *new_smi) dev_info(new_smi->dev, "IPMI %s interface initialized\n", si_to_str[new_smi->si_type]); + WARN_ON(new_smi->dev->init_name != NULL); + kfree(init_name); + return 0; out_err_stop_timer: @@ -3712,8 +3722,14 @@ static int try_smi_init(struct smi_info *new_smi) if (new_smi->dev_registered) { platform_device_unregister(new_smi->pdev); new_smi->dev_registered = false; + new_smi->pdev = NULL; + } else if (new_smi->pdev) { + platform_device_put(new_smi->pdev); + new_smi->pdev = NULL; } + kfree(init_name); + return rv; } From bb2a08c01a66e77917bd2fdaf5cffd917ec03573 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Mon, 7 Nov 2016 11:27:18 -0600 Subject: [PATCH 5/8] ipmi_si: Clean up printks Convert them to pr_xxx or dev_xxx. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_intf.c | 130 +++++++++++++------------------ 1 file changed, 54 insertions(+), 76 deletions(-) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 751c2815fc2a1..017994900ad71 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1764,7 +1764,7 @@ static int parse_str(const struct hotmod_vals *v, int *val, char *name, s = strchr(*curr, ','); if (!s) { - printk(KERN_WARNING PFX "No hotmod %s given.\n", name); + pr_warn(PFX "No hotmod %s given.\n", name); return -EINVAL; } *s = '\0'; @@ -1777,7 +1777,7 @@ static int parse_str(const struct hotmod_vals *v, int *val, char *name, } } - printk(KERN_WARNING PFX "Invalid hotmod %s '%s'\n", name, *curr); + pr_warn(PFX "Invalid hotmod %s '%s'\n", name, *curr); return -EINVAL; } @@ -1788,16 +1788,12 @@ static int check_hotmod_int_op(const char *curr, const char *option, if (strcmp(curr, name) == 0) { if (!option) { - printk(KERN_WARNING PFX - "No option given for '%s'\n", - curr); + pr_warn(PFX "No option given for '%s'\n", curr); return -EINVAL; } *val = simple_strtoul(option, &n, 0); if ((*n != '\0') || (*option == '\0')) { - printk(KERN_WARNING PFX - "Bad option given for '%s'\n", - curr); + pr_warn(PFX "Bad option given for '%s'\n", curr); return -EINVAL; } return 1; @@ -1877,8 +1873,7 @@ static int hotmod_handler(const char *val, struct kernel_param *kp) } addr = simple_strtoul(curr, &n, 0); if ((*n != '\0') || (*curr == '\0')) { - printk(KERN_WARNING PFX "Invalid hotmod address" - " '%s'\n", curr); + pr_warn(PFX "Invalid hotmod address '%s'\n", curr); break; } @@ -1921,9 +1916,7 @@ static int hotmod_handler(const char *val, struct kernel_param *kp) continue; rv = -EINVAL; - printk(KERN_WARNING PFX - "Invalid hotmod option '%s'\n", - curr); + pr_warn(PFX "Invalid hotmod option '%s'\n", curr); goto out; } @@ -2003,7 +1996,7 @@ static int hardcode_find_bmc(void) return -ENOMEM; info->addr_source = SI_HARDCODED; - printk(KERN_INFO PFX "probing via hardcoded address\n"); + pr_info(PFX "probing via hardcoded address\n"); if (!si_type[i] || strcmp(si_type[i], "kcs") == 0) { info->si_type = SI_KCS; @@ -2012,9 +2005,8 @@ static int hardcode_find_bmc(void) } else if (strcmp(si_type[i], "bt") == 0) { info->si_type = SI_BT; } else { - printk(KERN_WARNING PFX "Interface type specified " - "for interface %d, was invalid: %s\n", - i, si_type[i]); + pr_warn(PFX "Interface type specified for interface %d, was invalid: %s\n", + i, si_type[i]); kfree(info); continue; } @@ -2030,9 +2022,8 @@ static int hardcode_find_bmc(void) info->io.addr_data = addrs[i]; info->io.addr_type = IPMI_MEM_ADDR_SPACE; } else { - printk(KERN_WARNING PFX "Interface type specified " - "for interface %d, but port and address were " - "not set or set to zero.\n", i); + pr_warn(PFX "Interface type specified for interface %d, but port and address were not set or set to zero.\n", + i); kfree(info); continue; } @@ -2173,18 +2164,18 @@ static int try_init_spmi(struct SPMITable *spmi) int rv; if (spmi->IPMIlegacy != 1) { - printk(KERN_INFO PFX "Bad SPMI legacy %d\n", spmi->IPMIlegacy); + pr_info(PFX "Bad SPMI legacy %d\n", spmi->IPMIlegacy); return -ENODEV; } info = smi_info_alloc(); if (!info) { - printk(KERN_ERR PFX "Could not allocate SI data (3)\n"); + pr_err(PFX "Could not allocate SI data (3)\n"); return -ENOMEM; } info->addr_source = SI_SPMI; - printk(KERN_INFO PFX "probing via SPMI\n"); + pr_info(PFX "probing via SPMI\n"); /* Figure out the interface type. */ switch (spmi->InterfaceType) { @@ -2201,8 +2192,8 @@ static int try_init_spmi(struct SPMITable *spmi) kfree(info); return -EIO; default: - printk(KERN_INFO PFX "Unknown ACPI/SPMI SI type %d\n", - spmi->InterfaceType); + pr_info(PFX "Unknown ACPI/SPMI SI type %d\n", + spmi->InterfaceType); kfree(info); return -EIO; } @@ -2238,15 +2229,15 @@ static int try_init_spmi(struct SPMITable *spmi) info->io.addr_type = IPMI_IO_ADDR_SPACE; } else { kfree(info); - printk(KERN_WARNING PFX "Unknown ACPI I/O Address type\n"); + pr_warn(PFX "Unknown ACPI I/O Address type\n"); return -EIO; } info->io.addr_data = spmi->addr.address; pr_info("ipmi_si: SPMI: %s %#lx regsize %d spacing %d irq %d\n", - (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem", - info->io.addr_data, info->io.regsize, info->io.regspacing, - info->irq); + (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem", + info->io.addr_data, info->io.regsize, info->io.regspacing, + info->irq); rv = add_smi(info); if (rv) @@ -2356,12 +2347,12 @@ static void try_init_dmi(struct dmi_ipmi_data *ipmi_data) info = smi_info_alloc(); if (!info) { - printk(KERN_ERR PFX "Could not allocate SI data\n"); + pr_err(PFX "Could not allocate SI data\n"); return; } info->addr_source = SI_SMBIOS; - printk(KERN_INFO PFX "probing via SMBIOS\n"); + pr_info(PFX "probing via SMBIOS\n"); switch (ipmi_data->type) { case 0x01: /* KCS */ @@ -2391,8 +2382,8 @@ static void try_init_dmi(struct dmi_ipmi_data *ipmi_data) default: kfree(info); - printk(KERN_WARNING PFX "Unknown SMBIOS I/O Address type: %d\n", - ipmi_data->addr_space); + pr_warn(PFX "Unknown SMBIOS I/O Address type: %d\n", + ipmi_data->addr_space); return; } info->io.addr_data = ipmi_data->base_addr; @@ -2410,9 +2401,9 @@ static void try_init_dmi(struct dmi_ipmi_data *ipmi_data) info->irq_setup = std_irq_setup; pr_info("ipmi_si: SMBIOS: %s %#lx regsize %d spacing %d irq %d\n", - (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem", - info->io.addr_data, info->io.regsize, info->io.regspacing, - info->irq); + (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem", + info->io.addr_data, info->io.regsize, info->io.regspacing, + info->irq); if (add_smi(info)) kfree(info); @@ -3141,9 +3132,7 @@ static int try_enable_event_buffer(struct smi_info *smi_info) rv = wait_for_msg_done(smi_info); if (rv) { - printk(KERN_WARNING PFX "Error getting response from get" - " global enables command, the event buffer is not" - " enabled.\n"); + pr_warn(PFX "Error getting response from get global enables command, the event buffer is not enabled.\n"); goto out; } @@ -3154,8 +3143,7 @@ static int try_enable_event_buffer(struct smi_info *smi_info) resp[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2 || resp[1] != IPMI_GET_BMC_GLOBAL_ENABLES_CMD || resp[2] != 0) { - printk(KERN_WARNING PFX "Invalid return from get global" - " enables command, cannot enable the event buffer.\n"); + pr_warn(PFX "Invalid return from get global enables command, cannot enable the event buffer.\n"); rv = -EINVAL; goto out; } @@ -3173,9 +3161,7 @@ static int try_enable_event_buffer(struct smi_info *smi_info) rv = wait_for_msg_done(smi_info); if (rv) { - printk(KERN_WARNING PFX "Error getting response from set" - " global, enables command, the event buffer is not" - " enabled.\n"); + pr_warn(PFX "Error getting response from set global, enables command, the event buffer is not enabled.\n"); goto out; } @@ -3185,8 +3171,7 @@ static int try_enable_event_buffer(struct smi_info *smi_info) if (resp_len < 3 || resp[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2 || resp[1] != IPMI_SET_BMC_GLOBAL_ENABLES_CMD) { - printk(KERN_WARNING PFX "Invalid return from get global," - "enables command, not enable the event buffer.\n"); + pr_warn(PFX "Invalid return from get global, enables command, not enable the event buffer.\n"); rv = -EINVAL; goto out; } @@ -3474,17 +3459,18 @@ static int add_smi(struct smi_info *new_smi) { int rv = 0; - printk(KERN_INFO PFX "Adding %s-specified %s state machine", - ipmi_addr_src_to_str(new_smi->addr_source), - si_to_str[new_smi->si_type]); mutex_lock(&smi_infos_lock); if (!is_new_interface(new_smi)) { - printk(KERN_CONT " duplicate interface\n"); + pr_info(PFX "%s-specified %s state machine: duplicate\n", + ipmi_addr_src_to_str(new_smi->addr_source), + si_to_str[new_smi->si_type]); rv = -EBUSY; goto out_err; } - printk(KERN_CONT "\n"); + pr_info(PFX "Adding %s-specified %s state machine\n", + ipmi_addr_src_to_str(new_smi->addr_source), + si_to_str[new_smi->si_type]); /* So we know not to free it unless we have allocated one. */ new_smi->intf = NULL; @@ -3504,14 +3490,12 @@ static int try_smi_init(struct smi_info *new_smi) int i; char *init_name = NULL; - printk(KERN_INFO PFX "Trying %s-specified %s state" - " machine at %s address 0x%lx, slave address 0x%x," - " irq %d\n", - ipmi_addr_src_to_str(new_smi->addr_source), - si_to_str[new_smi->si_type], - addr_space_to_str[new_smi->io.addr_type], - new_smi->io.addr_data, - new_smi->slave_addr, new_smi->irq); + pr_info(PFX "Trying %s-specified %s state machine at %s address 0x%lx, slave address 0x%x, irq %d\n", + ipmi_addr_src_to_str(new_smi->addr_source), + si_to_str[new_smi->si_type], + addr_space_to_str[new_smi->io.addr_type], + new_smi->io.addr_data, + new_smi->slave_addr, new_smi->irq); switch (new_smi->si_type) { case SI_KCS: @@ -3555,8 +3539,7 @@ static int try_smi_init(struct smi_info *new_smi) /* Allocate the state machine's data and initialize it. */ new_smi->si_sm = kmalloc(new_smi->handlers->size(), GFP_KERNEL); if (!new_smi->si_sm) { - printk(KERN_ERR PFX - "Could not allocate state machine memory\n"); + pr_err(PFX "Could not allocate state machine memory\n"); rv = -ENOMEM; goto out_err; } @@ -3566,14 +3549,14 @@ static int try_smi_init(struct smi_info *new_smi) /* Now that we know the I/O size, we can set up the I/O. */ rv = new_smi->io_setup(new_smi); if (rv) { - printk(KERN_ERR PFX "Could not set up I/O space\n"); + dev_err(new_smi->dev, "Could not set up I/O space\n"); goto out_err; } /* Do low-level detection first. */ if (new_smi->handlers->detect(new_smi->si_sm)) { if (new_smi->addr_source) - printk(KERN_INFO PFX "Interface detection failed\n"); + dev_err(new_smi->dev, "Interface detection failed\n"); rv = -ENODEV; goto out_err; } @@ -3585,8 +3568,7 @@ static int try_smi_init(struct smi_info *new_smi) rv = try_get_dev_id(new_smi); if (rv) { if (new_smi->addr_source) - printk(KERN_INFO PFX "There appears to be no BMC" - " at this location\n"); + dev_err(new_smi->dev, "There appears to be no BMC at this location\n"); goto out_err; } @@ -3628,10 +3610,9 @@ static int try_smi_init(struct smi_info *new_smi) if (new_smi->pdev) { rv = platform_device_add(new_smi->pdev); if (rv) { - printk(KERN_ERR PFX - "Unable to register system interface device:" - " %d\n", - rv); + dev_err(new_smi->dev, + "Unable to register system interface device: %d\n", + rv); goto out_err; } new_smi->dev_registered = true; @@ -3748,8 +3729,7 @@ static int init_ipmi_si(void) if (si_tryplatform) { rv = platform_driver_register(&ipmi_driver); if (rv) { - printk(KERN_ERR PFX "Unable to register " - "driver: %d\n", rv); + pr_err(PFX "Unable to register driver: %d\n", rv); return rv; } } @@ -3769,7 +3749,7 @@ static int init_ipmi_si(void) } } - printk(KERN_INFO "IPMI System Interface driver.\n"); + pr_info("IPMI System Interface driver.\n"); /* If the user gave us a device, they presumably want us to use it */ if (!hardcode_find_bmc()) @@ -3779,8 +3759,7 @@ static int init_ipmi_si(void) if (si_trypci) { rv = pci_register_driver(&ipmi_pci_driver); if (rv) - printk(KERN_ERR PFX "Unable to register " - "PCI driver: %d\n", rv); + pr_err(PFX "Unable to register PCI driver: %d\n", rv); else pci_registered = true; } @@ -3842,8 +3821,7 @@ static int init_ipmi_si(void) if (unload_when_empty && list_empty(&smi_infos)) { mutex_unlock(&smi_infos_lock); cleanup_ipmi_si(); - printk(KERN_WARNING PFX - "Unable to find any System Interface(s)\n"); + pr_warn(PFX "Unable to find any System Interface(s)\n"); return -ENODEV; } else { mutex_unlock(&smi_infos_lock); From 94671710183c3f5eed7cf33f73600c836e4777fd Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Mon, 7 Nov 2016 12:07:09 -0600 Subject: [PATCH 6/8] ipmi: Pick up slave address from SMBIOS on an ACPI device When added by ACPI, the information does not contain the slave address of the BMC. However, that information is available from SMBIOS. So if we add a device that doesn't have a slave address, look at the other devices that are duplicate interfaces and see if they have a slave address. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_intf.c | 10 +++++++++- drivers/char/ipmi/ipmi_ssif.c | 33 ++++++++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 017994900ad71..2a7c425ddfa73 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -3448,8 +3448,16 @@ static int is_new_interface(struct smi_info *info) list_for_each_entry(e, &smi_infos, link) { if (e->io.addr_type != info->io.addr_type) continue; - if (e->io.addr_data == info->io.addr_data) + if (e->io.addr_data == info->io.addr_data) { + /* + * This is a cheap hack, ACPI doesn't have a defined + * slave address but SMBIOS does. Pick it up from + * any source that has it available. + */ + if (info->slave_addr && !e->slave_addr) + e->slave_addr = info->slave_addr; return 0; + } } return 1; diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 04232431eaf7a..cca6e5bc1cea3 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -174,7 +174,6 @@ enum ssif_stat_indexes { }; struct ssif_addr_info { - unsigned short addr; struct i2c_board_info binfo; char *adapter_name; int debug; @@ -1401,6 +1400,34 @@ static bool check_acpi(struct ssif_info *ssif_info, struct device *dev) return false; } +static int find_slave_address(struct i2c_client *client, int slave_addr) +{ + struct ssif_addr_info *info; + + if (slave_addr) + return slave_addr; + + /* + * Came in without a slave address, search around to see if + * the other sources have a slave address. This lets us pick + * up an SMBIOS slave address when using ACPI. + */ + list_for_each_entry(info, &ssif_infos, link) { + if (info->binfo.addr != client->addr) + continue; + if (info->adapter_name && client->adapter->name && + strcmp_nospace(info->adapter_name, + client->adapter->name)) + continue; + if (info->slave_addr) { + slave_addr = info->slave_addr; + break; + } + } + + return slave_addr; +} + /* * Global enables we care about. */ @@ -1443,6 +1470,8 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) } } + slave_addr = find_slave_address(client, slave_addr); + pr_info(PFX "Trying %s-specified SSIF interface at i2c address 0x%x, adapter %s, slave address 0x%x\n", ipmi_addr_src_to_str(ssif_info->addr_source), client->addr, client->adapter->name, slave_addr); @@ -1931,7 +1960,7 @@ static int decode_dmi(const struct dmi_device *dmi_dev) slave_addr = data[6]; } - return new_ssif_client(myaddr, NULL, 0, 0, SI_SMBIOS); + return new_ssif_client(myaddr, NULL, 0, slave_addr, SI_SMBIOS); } static void dmi_iterator(void) From a24b5dd5eda73b956da27031cefbe4374b6af2bc Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Mon, 14 Nov 2016 10:11:30 -0600 Subject: [PATCH 7/8] ipmi: Fix sequence number handling The IPMI message handler uses a message id that the lower-layer preserved to track the sequence number of the message. The macros that handled these sequence numbers were somewhat broken as they could result in sequence number truncation and they were not doing an "and" of the proper number of bits. I think this actually is not a problem, because the truncation should be harmless and the improper "and" didn't hurt anything because sequence number generation used the same improper "and" and wouldn't generate a sequence number that would get truncated wrong. However, it should be fixed. Reported-by: Dan Carpenter Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index fcdd886819f5c..be982d10d305c 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -158,15 +158,16 @@ struct seq_table { * Store the information in a msgid (long) to allow us to find a * sequence table entry from the msgid. */ -#define STORE_SEQ_IN_MSGID(seq, seqid) (((seq&0xff)<<26) | (seqid&0x3ffffff)) +#define STORE_SEQ_IN_MSGID(seq, seqid) \ + ((((seq) & 0x3f) << 26) | ((seqid) & 0x3ffffff)) #define GET_SEQ_FROM_MSGID(msgid, seq, seqid) \ do { \ - seq = ((msgid >> 26) & 0x3f); \ - seqid = (msgid & 0x3fffff); \ + seq = (((msgid) >> 26) & 0x3f); \ + seqid = ((msgid) & 0x3ffffff); \ } while (0) -#define NEXT_SEQID(seqid) (((seqid) + 1) & 0x3fffff) +#define NEXT_SEQID(seqid) (((seqid) + 1) & 0x3ffffff) struct ipmi_channel { unsigned char medium; From 070cbd1d42aa0e359c9957cd73c2a529dec62047 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 25 Nov 2016 10:55:36 +0100 Subject: [PATCH 8/8] ipmi: create hardware-independent softdep for ipmi_devintf When a computer has an IPMI system interface, the device interface is most probably also desired. Autoloading of ipmi_devintf currently works only if ipmi_si has allocated a platform device. That doesn't happen if the SI interface was detected e.g. via ACPI. But ACPI detection is preferred these days, see e.g. kernel.org bug 46741. This patch introduces a softdep in place of the existing modalias for ipmi_devintf. Signed-off-by: Martin Wilck Suggested-by: Takashi Iwai I moved this to ipmi_msghandler.c, so it works for all IPMI interfaces. Retested by Martin. Tested-by: Martin Wilck Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_devintf.c | 1 - drivers/char/ipmi/ipmi_msghandler.c | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/ipmi/ipmi_devintf.c b/drivers/char/ipmi/ipmi_devintf.c index 1786574536b21..a21407de46aeb 100644 --- a/drivers/char/ipmi/ipmi_devintf.c +++ b/drivers/char/ipmi/ipmi_devintf.c @@ -989,4 +989,3 @@ module_exit(cleanup_ipmi); MODULE_LICENSE("GPL"); MODULE_AUTHOR("Corey Minyard "); MODULE_DESCRIPTION("Linux device interface for the IPMI message handler."); -MODULE_ALIAS("platform:ipmi_si"); diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index be982d10d305c..92e53acf2cd20 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -4646,3 +4646,4 @@ MODULE_AUTHOR("Corey Minyard "); MODULE_DESCRIPTION("Incoming and outgoing message routing for an IPMI" " interface."); MODULE_VERSION(IPMI_DRIVER_VERSION); +MODULE_SOFTDEP("post: ipmi_devintf");