From 2f66353963043e1d8dfacfbdf509acc5d3be7698 Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Fri, 17 May 2019 18:12:44 +0800 Subject: [PATCH 1/9] ipmi_si: fix unexpected driver unregister warning If ipmi_si_platform_init()->platform_driver_register() fails, platform_driver_unregister() called unconditionally will trigger following warning, ipmi_platform: Unable to register driver: -12 ------------[ cut here ]------------ Unexpected driver unregister! WARNING: CPU: 1 PID: 7210 at drivers/base/driver.c:193 driver_unregister+0x60/0x70 drivers/base/driver.c:193 Fix it by adding platform_registered variable, only unregister platform driver when it is already successfully registered. Reported-by: Hulk Robot Signed-off-by: Kefeng Wang Message-Id: <20190517101245.4341-1-wangkefeng.wang@huawei.com> Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_platform.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c index f2a91c4d8cab..0cd849675d99 100644 --- a/drivers/char/ipmi/ipmi_si_platform.c +++ b/drivers/char/ipmi/ipmi_si_platform.c @@ -19,6 +19,7 @@ #include "ipmi_si.h" #include "ipmi_dmi.h" +static bool platform_registered; static bool si_tryplatform = true; #ifdef CONFIG_ACPI static bool si_tryacpi = true; @@ -469,9 +470,12 @@ void ipmi_si_platform_init(void) int rv = platform_driver_register(&ipmi_platform_driver); if (rv) pr_err("Unable to register driver: %d\n", rv); + else + platform_registered = true; } void ipmi_si_platform_shutdown(void) { - platform_driver_unregister(&ipmi_platform_driver); + if (platform_registered) + platform_driver_unregister(&ipmi_platform_driver); } From dd7450cab71174637329e5ef57d8474fc7ff2ea0 Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Fri, 17 May 2019 18:12:45 +0800 Subject: [PATCH 2/9] ipmi_si: use bool type for initialized variable Cover 'int' to 'bool' type for initialized variable. Signed-off-by: Kefeng Wang Message-Id: <20190517101245.4341-2-wangkefeng.wang@huawei.com> Signed-off-by: Corey Minyard --- 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 f124a2d2bb9f..da5b6723329a 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -71,7 +71,7 @@ enum si_intf_state { static const char * const si_to_str[] = { "invalid", "kcs", "smic", "bt" }; -static int initialized; +static bool initialized; /* * Indexes into stats[] in smi_info below. @@ -2124,7 +2124,7 @@ static int __init init_ipmi_si(void) } skip_fallback_noirq: - initialized = 1; + initialized = true; mutex_unlock(&smi_infos_lock); if (type) From 2cd0e54489e65b8e22124a8b053aff40815487f7 Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Fri, 24 May 2019 22:37:24 +0800 Subject: [PATCH 3/9] ipmi_ssif: fix unexpected driver unregister warning If platform_driver_register() fails from init_ipmi_ssif(), platform_driver_unregister() called unconditionally will trigger following warning, ipmi_ssif: Unable to register driver: -12 ------------[ cut here ]------------ Unexpected driver unregister! WARNING: CPU: 1 PID: 6305 at drivers/base/driver.c:193 driver_unregister+0x60/0x70 drivers/base/driver.c:193 Fix it by adding platform_registered variable, only unregister platform driver when it is already successfully registered. Reported-by: Hulk Robot Signed-off-by: Kefeng Wang Message-Id: <20190524143724.43218-1-wangkefeng.wang@huawei.com> Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_ssif.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index cf8156d6bc07..305fa5054274 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -303,6 +303,7 @@ struct ssif_info { ((unsigned int) atomic_read(&(ssif)->stats[SSIF_STAT_ ## stat])) static bool initialized; +static bool platform_registered; static void return_hosed_msg(struct ssif_info *ssif_info, struct ipmi_smi_msg *msg); @@ -2088,6 +2089,8 @@ static int init_ipmi_ssif(void) rv = platform_driver_register(&ipmi_driver); if (rv) pr_err("Unable to register driver: %d\n", rv); + else + platform_registered = true; } ssif_i2c_driver.address_list = ssif_address_list(); @@ -2111,7 +2114,7 @@ static void cleanup_ipmi_ssif(void) kfree(ssif_i2c_driver.address_list); - if (ssif_trydmi) + if (ssif_trydmi && platform_registered) platform_driver_unregister(&ipmi_driver); free_ssif_clients(); From 3559c3270aa3b8ac9afa3eac25f0c4e5bfb701b4 Mon Sep 17 00:00:00 2001 From: Suzuki K Poulose Date: Mon, 3 Jun 2019 16:49:28 +0100 Subject: [PATCH 4/9] drivers: ipmi: Drop device reference Drop the reference to a device found via bus_find_device() Cc: Corey Minyard Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Signed-off-by: Suzuki K Poulose Message-Id: <1559577023-558-3-git-send-email-suzuki.poulose@arm.com> [Moved the put_device() to after the platform_device_unregister(), for better style.] Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_platform.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c index 0cd849675d99..5c51f525b9fb 100644 --- a/drivers/char/ipmi/ipmi_si_platform.c +++ b/drivers/char/ipmi/ipmi_si_platform.c @@ -444,6 +444,7 @@ void ipmi_remove_platform_device_by_name(char *name) struct platform_device *pdev = to_platform_device(dev); platform_device_unregister(pdev); + put_device(dev); } } From 51bd6f291583684f495ea498984dfc22049d7fd2 Mon Sep 17 00:00:00 2001 From: Asmaa Mnebhi Date: Mon, 10 Jun 2019 14:57:02 -0400 Subject: [PATCH 5/9] Add support for IPMB driver Support receiving IPMB requests on a Satellite MC from the BMC. Once a response is ready, this driver will send back a response to the BMC via the IPMB channel. Signed-off-by: Asmaa Mnebhi Acked-by: vadimp@mellanox.com Message-Id: <319690553a0da2a1e80b400941341081b383e5f1.1560192707.git.Asmaa@mellanox.com> [Move the config option to outside the ipmi msghandler, as it's not dependent on that. Fixed one small whitespace issue.] Signed-off-by: Corey Minyard --- Documentation/IPMB.txt | 103 +++++++++ drivers/char/ipmi/Kconfig | 8 + drivers/char/ipmi/Makefile | 1 + drivers/char/ipmi/ipmb_dev_int.c | 364 +++++++++++++++++++++++++++++++ 4 files changed, 476 insertions(+) create mode 100644 Documentation/IPMB.txt create mode 100644 drivers/char/ipmi/ipmb_dev_int.c diff --git a/Documentation/IPMB.txt b/Documentation/IPMB.txt new file mode 100644 index 000000000000..a6ed8b68bd0f --- /dev/null +++ b/Documentation/IPMB.txt @@ -0,0 +1,103 @@ +============================== +IPMB Driver for a Satellite MC +============================== + +The Intelligent Platform Management Bus or IPMB, is an +I2C bus that provides a standardized interconnection between +different boards within a chassis. This interconnection is +between the baseboard management (BMC) and chassis electronics. +IPMB is also associated with the messaging protocol through the +IPMB bus. + +The devices using the IPMB are usually management +controllers that perform management functions such as servicing +the front panel interface, monitoring the baseboard, +hot-swapping disk drivers in the system chassis, etc... + +When an IPMB is implemented in the system, the BMC serves as +a controller to give system software access to the IPMB. The BMC +sends IPMI requests to a device (usually a Satellite Management +Controller or Satellite MC) via IPMB and the device +sends a response back to the BMC. + +For more information on IPMB and the format of an IPMB message, +refer to the IPMB and IPMI specifications. + +IPMB driver for Satellite MC +---------------------------- + +ipmb-dev-int - This is the driver needed on a Satellite MC to +receive IPMB messages from a BMC and send a response back. +This driver works with the I2C driver and a userspace +program such as OpenIPMI: + +1) It is an I2C slave backend driver. So, it defines a callback +function to set the Satellite MC as an I2C slave. +This callback function handles the received IPMI requests. + +2) It defines the read and write functions to enable a user +space program (such as OpenIPMI) to communicate with the kernel. + + +Load the IPMB driver +-------------------- + +The driver needs to be loaded at boot time or manually first. +First, make sure you have the following in your config file: +CONFIG_IPMB_DEVICE_INTERFACE=y + +1) If you want the driver to be loaded at boot time: + +a) Add this entry to your ACPI table, under the appropriate SMBus: + +Device (SMB0) // Example SMBus host controller +{ + Name (_HID, "") // Vendor-Specific HID + Name (_UID, 0) // Unique ID of particular host controller + : + : + Device (IPMB) + { + Name (_HID, "IPMB0001") // IPMB device interface + Name (_UID, 0) // Unique device identifier + } +} + +b) Example for device tree: + +&i2c2 { + status = "okay"; + + ipmb@10 { + compatible = "ipmb-dev"; + reg = <0x10>; + }; +}; + +2) Manually from Linux: +modprobe ipmb-dev-int + + +Instantiate the device +---------------------- + +After loading the driver, you can instantiate the device as +described in 'Documentation/i2c/instantiating-devices'. +If you have multiple BMCs, each connected to your Satellite MC via +a different I2C bus, you can instantiate a device for each of +those BMCs. +The name of the instantiated device contains the I2C bus number +associated with it as follows: + +BMC1 ------ IPMB/I2C bus 1 ---------| /dev/ipmb-1 + Satellite MC +BMC1 ------ IPMB/I2C bus 2 ---------| /dev/ipmb-2 + +For instance, you can instantiate the ipmb-dev-int device from +user space at the 7 bit address 0x10 on bus 2: + + # echo ipmb-dev 0x1010 > /sys/bus/i2c/devices/i2c-2/new_device + +This will create the device file /dev/ipmb-2, which can be accessed +by the user space program. The device needs to be instantiated +before running the user space program. diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig index 94719fc6ff9d..a9cfe4c05e64 100644 --- a/drivers/char/ipmi/Kconfig +++ b/drivers/char/ipmi/Kconfig @@ -131,3 +131,11 @@ config ASPEED_BT_IPMI_BMC Provides a driver for the BT (Block Transfer) IPMI interface found on Aspeed SOCs (AST2400 and AST2500). The driver implements the BMC side of the BT interface. + +config IPMB_DEVICE_INTERFACE + tristate 'IPMB Interface handler' + depends on I2C_SLAVE + help + Provides a driver for a device (Satellite MC) to + receive requests and send responses back to the BMC via + the IPMB interface. This module requires I2C support. diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile index 3f06b2062475..0822adc2ec41 100644 --- a/drivers/char/ipmi/Makefile +++ b/drivers/char/ipmi/Makefile @@ -26,3 +26,4 @@ obj-$(CONFIG_IPMI_KCS_BMC) += kcs_bmc.o obj-$(CONFIG_ASPEED_BT_IPMI_BMC) += bt-bmc.o obj-$(CONFIG_ASPEED_KCS_IPMI_BMC) += kcs_bmc_aspeed.o obj-$(CONFIG_NPCM7XX_KCS_IPMI_BMC) += kcs_bmc_npcm7xx.o +obj-$(CONFIG_IPMB_DEVICE_INTERFACE) += ipmb_dev_int.o diff --git a/drivers/char/ipmi/ipmb_dev_int.c b/drivers/char/ipmi/ipmb_dev_int.c new file mode 100644 index 000000000000..2895abf72e61 --- /dev/null +++ b/drivers/char/ipmi/ipmb_dev_int.c @@ -0,0 +1,364 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * IPMB driver to receive a request and send a response + * + * Copyright (C) 2019 Mellanox Techologies, Ltd. + * + * This was inspired by Brendan Higgins' ipmi-bmc-bt-i2c driver. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define MAX_MSG_LEN 128 +#define IPMB_REQUEST_LEN_MIN 7 +#define NETFN_RSP_BIT_MASK 0x4 +#define REQUEST_QUEUE_MAX_LEN 256 + +#define IPMB_MSG_LEN_IDX 0 +#define RQ_SA_8BIT_IDX 1 +#define NETFN_LUN_IDX 2 + +#define GET_7BIT_ADDR(addr_8bit) (addr_8bit >> 1) +#define GET_8BIT_ADDR(addr_7bit) ((addr_7bit << 1) & 0xff) + +#define IPMB_MSG_PAYLOAD_LEN_MAX (MAX_MSG_LEN - IPMB_REQUEST_LEN_MIN - 1) + +#define SMBUS_MSG_HEADER_LENGTH 2 +#define SMBUS_MSG_IDX_OFFSET (SMBUS_MSG_HEADER_LENGTH + 1) + +struct ipmb_msg { + u8 len; + u8 rs_sa; + u8 netfn_rs_lun; + u8 checksum1; + u8 rq_sa; + u8 rq_seq_rq_lun; + u8 cmd; + u8 payload[IPMB_MSG_PAYLOAD_LEN_MAX]; + /* checksum2 is included in payload */ +} __packed; + +struct ipmb_request_elem { + struct list_head list; + struct ipmb_msg request; +}; + +struct ipmb_dev { + struct i2c_client *client; + struct miscdevice miscdev; + struct ipmb_msg request; + struct list_head request_queue; + atomic_t request_queue_len; + size_t msg_idx; + spinlock_t lock; + wait_queue_head_t wait_queue; + struct mutex file_mutex; +}; + +static inline struct ipmb_dev *to_ipmb_dev(struct file *file) +{ + return container_of(file->private_data, struct ipmb_dev, miscdev); +} + +static ssize_t ipmb_read(struct file *file, char __user *buf, size_t count, + loff_t *ppos) +{ + struct ipmb_dev *ipmb_dev = to_ipmb_dev(file); + struct ipmb_request_elem *queue_elem; + struct ipmb_msg msg; + ssize_t ret; + + memset(&msg, 0, sizeof(msg)); + + spin_lock_irq(&ipmb_dev->lock); + + while (list_empty(&ipmb_dev->request_queue)) { + spin_unlock_irq(&ipmb_dev->lock); + + if (file->f_flags & O_NONBLOCK) + return -EAGAIN; + + ret = wait_event_interruptible(ipmb_dev->wait_queue, + !list_empty(&ipmb_dev->request_queue)); + if (ret) + return ret; + + spin_lock_irq(&ipmb_dev->lock); + } + + queue_elem = list_first_entry(&ipmb_dev->request_queue, + struct ipmb_request_elem, list); + memcpy(&msg, &queue_elem->request, sizeof(msg)); + list_del(&queue_elem->list); + kfree(queue_elem); + atomic_dec(&ipmb_dev->request_queue_len); + + spin_unlock_irq(&ipmb_dev->lock); + + count = min_t(size_t, count, msg.len + 1); + if (copy_to_user(buf, &msg, count)) + ret = -EFAULT; + + return ret < 0 ? ret : count; +} + +static ssize_t ipmb_write(struct file *file, const char __user *buf, + size_t count, loff_t *ppos) +{ + struct ipmb_dev *ipmb_dev = to_ipmb_dev(file); + u8 rq_sa, netf_rq_lun, msg_len; + struct i2c_client rq_client; + u8 msg[MAX_MSG_LEN]; + ssize_t ret; + + if (count > sizeof(msg)) + return -EINVAL; + + if (copy_from_user(&msg, buf, count)) + return -EFAULT; + + if (count < msg[0]) + return -EINVAL; + + rq_sa = GET_7BIT_ADDR(msg[RQ_SA_8BIT_IDX]); + netf_rq_lun = msg[NETFN_LUN_IDX]; + + if (!(netf_rq_lun & NETFN_RSP_BIT_MASK)) + return -EINVAL; + + /* + * subtract rq_sa and netf_rq_lun from the length of the msg passed to + * i2c_smbus_write_block_data_local + */ + msg_len = msg[IPMB_MSG_LEN_IDX] - SMBUS_MSG_HEADER_LENGTH; + + strcpy(rq_client.name, "ipmb_requester"); + rq_client.adapter = ipmb_dev->client->adapter; + rq_client.flags = ipmb_dev->client->flags; + rq_client.addr = rq_sa; + + ret = i2c_smbus_write_block_data(&rq_client, netf_rq_lun, msg_len, + msg + SMBUS_MSG_IDX_OFFSET); + + return ret ? : count; +} + +static unsigned int ipmb_poll(struct file *file, poll_table *wait) +{ + struct ipmb_dev *ipmb_dev = to_ipmb_dev(file); + unsigned int mask = POLLOUT; + + mutex_lock(&ipmb_dev->file_mutex); + poll_wait(file, &ipmb_dev->wait_queue, wait); + + if (atomic_read(&ipmb_dev->request_queue_len)) + mask |= POLLIN; + mutex_unlock(&ipmb_dev->file_mutex); + + return mask; +} + +static const struct file_operations ipmb_fops = { + .owner = THIS_MODULE, + .read = ipmb_read, + .write = ipmb_write, + .poll = ipmb_poll, +}; + +/* Called with ipmb_dev->lock held. */ +static void ipmb_handle_request(struct ipmb_dev *ipmb_dev) +{ + struct ipmb_request_elem *queue_elem; + + if (atomic_read(&ipmb_dev->request_queue_len) >= + REQUEST_QUEUE_MAX_LEN) + return; + + queue_elem = kmalloc(sizeof(*queue_elem), GFP_ATOMIC); + if (!queue_elem) + return; + + memcpy(&queue_elem->request, &ipmb_dev->request, + sizeof(struct ipmb_msg)); + list_add(&queue_elem->list, &ipmb_dev->request_queue); + atomic_inc(&ipmb_dev->request_queue_len); + wake_up_all(&ipmb_dev->wait_queue); +} + +static u8 ipmb_verify_checksum1(struct ipmb_dev *ipmb_dev, u8 rs_sa) +{ + /* The 8 lsb of the sum is 0 when the checksum is valid */ + return (rs_sa + ipmb_dev->request.netfn_rs_lun + + ipmb_dev->request.checksum1); +} + +static bool is_ipmb_request(struct ipmb_dev *ipmb_dev, u8 rs_sa) +{ + if (ipmb_dev->msg_idx >= IPMB_REQUEST_LEN_MIN) { + if (ipmb_verify_checksum1(ipmb_dev, rs_sa)) + return false; + + /* + * Check whether this is an IPMB request or + * response. + * The 6 MSB of netfn_rs_lun are dedicated to the netfn + * while the remaining bits are dedicated to the lun. + * If the LSB of the netfn is cleared, it is associated + * with an IPMB request. + * If the LSB of the netfn is set, it is associated with + * an IPMB response. + */ + if (!(ipmb_dev->request.netfn_rs_lun & NETFN_RSP_BIT_MASK)) + return true; + } + return false; +} + +/* + * The IPMB protocol only supports I2C Writes so there is no need + * to support I2C_SLAVE_READ* events. + * This i2c callback function only monitors IPMB request messages + * and adds them in a queue, so that they can be handled by + * receive_ipmb_request. + */ +static int ipmb_slave_cb(struct i2c_client *client, + enum i2c_slave_event event, u8 *val) +{ + struct ipmb_dev *ipmb_dev = i2c_get_clientdata(client); + u8 *buf = (u8 *)&ipmb_dev->request; + unsigned long flags; + + spin_lock_irqsave(&ipmb_dev->lock, flags); + switch (event) { + case I2C_SLAVE_WRITE_REQUESTED: + memset(&ipmb_dev->request, 0, sizeof(ipmb_dev->request)); + ipmb_dev->msg_idx = 0; + + /* + * At index 0, ipmb_msg stores the length of msg, + * skip it for now. + * The len will be populated once the whole + * buf is populated. + * + * The I2C bus driver's responsibility is to pass the + * data bytes to the backend driver; it does not + * forward the i2c slave address. + * Since the first byte in the IPMB message is the + * address of the responder, it is the responsibility + * of the IPMB driver to format the message properly. + * So this driver prepends the address of the responder + * to the received i2c data before the request message + * is handled in userland. + */ + buf[++ipmb_dev->msg_idx] = GET_8BIT_ADDR(client->addr); + break; + + case I2C_SLAVE_WRITE_RECEIVED: + if (ipmb_dev->msg_idx >= sizeof(struct ipmb_msg)) + break; + + buf[++ipmb_dev->msg_idx] = *val; + break; + + case I2C_SLAVE_STOP: + ipmb_dev->request.len = ipmb_dev->msg_idx; + + if (is_ipmb_request(ipmb_dev, GET_8BIT_ADDR(client->addr))) + ipmb_handle_request(ipmb_dev); + break; + + default: + break; + } + spin_unlock_irqrestore(&ipmb_dev->lock, flags); + + return 0; +} + +static int ipmb_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct ipmb_dev *ipmb_dev; + int ret; + + ipmb_dev = devm_kzalloc(&client->dev, sizeof(*ipmb_dev), + GFP_KERNEL); + if (!ipmb_dev) + return -ENOMEM; + + spin_lock_init(&ipmb_dev->lock); + init_waitqueue_head(&ipmb_dev->wait_queue); + atomic_set(&ipmb_dev->request_queue_len, 0); + INIT_LIST_HEAD(&ipmb_dev->request_queue); + + mutex_init(&ipmb_dev->file_mutex); + + ipmb_dev->miscdev.minor = MISC_DYNAMIC_MINOR; + + ipmb_dev->miscdev.name = devm_kasprintf(&client->dev, GFP_KERNEL, + "%s%d", "ipmb-", + client->adapter->nr); + ipmb_dev->miscdev.fops = &ipmb_fops; + ipmb_dev->miscdev.parent = &client->dev; + ret = misc_register(&ipmb_dev->miscdev); + if (ret) + return ret; + + ipmb_dev->client = client; + i2c_set_clientdata(client, ipmb_dev); + ret = i2c_slave_register(client, ipmb_slave_cb); + if (ret) { + misc_deregister(&ipmb_dev->miscdev); + return ret; + } + + return 0; +} + +static int ipmb_remove(struct i2c_client *client) +{ + struct ipmb_dev *ipmb_dev = i2c_get_clientdata(client); + + i2c_slave_unregister(client); + misc_deregister(&ipmb_dev->miscdev); + + return 0; +} + +static const struct i2c_device_id ipmb_id[] = { + { "ipmb-dev", 0 }, + {}, +}; +MODULE_DEVICE_TABLE(i2c, ipmb_id); + +static const struct acpi_device_id acpi_ipmb_id[] = { + { "IPMB0001", 0 }, + {}, +}; +MODULE_DEVICE_TABLE(acpi, acpi_ipmb_id); + +static struct i2c_driver ipmb_driver = { + .driver = { + .owner = THIS_MODULE, + .name = "ipmb-dev", + .acpi_match_table = ACPI_PTR(acpi_ipmb_id), + }, + .probe = ipmb_probe, + .remove = ipmb_remove, + .id_table = ipmb_id, +}; +module_i2c_driver(ipmb_driver); + +MODULE_AUTHOR("Mellanox Technologies"); +MODULE_DESCRIPTION("IPMB driver"); +MODULE_LICENSE("GPL v2"); From 102308f557bca8e539a800614b0e274945a81cdf Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Wed, 12 Jun 2019 11:18:25 +0800 Subject: [PATCH 6/9] ipmi: ipmb: Fix build error while CONFIG_I2C is set to m If CONFIG_I2C is m and CONFIG_I2C_SLAVE is y, building with CONFIG_IPMB_DEVICE_INTERFACE setting to y will fail: drivers/char/ipmi/ipmb_dev_int.o: In function `ipmb_remove': ipmb_dev_int.c: undefined reference to `i2c_slave_unregister' drivers/char/ipmi/ipmb_dev_int.o: In function `ipmb_write': ipmb_dev_int.c: undefined reference to `i2c_smbus_write_block_data' drivers/char/ipmi/ipmb_dev_int.o: In function `ipmb_probe': ipmb_dev_int.c: undefined reference to `i2c_slave_register' drivers/char/ipmi/ipmb_dev_int.o: In function `ipmb_driver_init': ipmb_dev_int.c: undefined reference to `i2c_register_driver' drivers/char/ipmi/ipmb_dev_int.o: In function `ipmb_driver_exit': ipmb_dev_int.c: undefined reference to `i2c_del_driver' Add I2C Kconfig dependency to fix this. Reported-by: Hulk Robot Fixes: 51bd6f291583 ("Add support for IPMB driver") Signed-off-by: YueHaibing Message-Id: <20190612031825.24732-1-yuehaibing@huawei.com> Signed-off-by: Corey Minyard --- drivers/char/ipmi/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig index a9cfe4c05e64..e36db1b085d8 100644 --- a/drivers/char/ipmi/Kconfig +++ b/drivers/char/ipmi/Kconfig @@ -134,6 +134,7 @@ config ASPEED_BT_IPMI_BMC config IPMB_DEVICE_INTERFACE tristate 'IPMB Interface handler' + depends on I2C depends on I2C_SLAVE help Provides a driver for a device (Satellite MC) to From 21feadd024e747123bc0aaba8d8cbef2b5eda058 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Wed, 19 Jun 2019 14:50:34 +0200 Subject: [PATCH 7/9] ipmi: ipmb: don't allocate i2c_client on stack The i2c_client structure can be fairly large, which leads to a warning about possible kernel stack overflow in some configurations: drivers/char/ipmi/ipmb_dev_int.c:115:16: error: stack frame size of 1032 bytes in function 'ipmb_write' [-Werror,-Wframe-larger-than=] There is no real reason to even declare an i2c_client, as we can simply call i2c_smbus_xfer() directly instead of the i2c_smbus_write_block_data() wrapper. Convert the ipmb_write() to use an open-coded i2c_smbus_write_block_data() here, without changing the behavior. It seems that there is another problem with this implementation; when user space passes a length of more than I2C_SMBUS_BLOCK_MAX bytes, all the rest is silently ignored. This should probably be addressed in a separate patch, but I don't know what the intended behavior is here. Fixes: 51bd6f291583 ("Add support for IPMB driver") Signed-off-by: Arnd Bergmann Message-Id: <20190619125045.918700-1-arnd@arndb.de> Acked-by: Asmaa Mnebhi [Broke up a line >80 characters on i2c_smbus_xfer().] Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmb_dev_int.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/char/ipmi/ipmb_dev_int.c b/drivers/char/ipmi/ipmb_dev_int.c index 2895abf72e61..4d608a67f9a3 100644 --- a/drivers/char/ipmi/ipmb_dev_int.c +++ b/drivers/char/ipmi/ipmb_dev_int.c @@ -117,7 +117,7 @@ static ssize_t ipmb_write(struct file *file, const char __user *buf, { struct ipmb_dev *ipmb_dev = to_ipmb_dev(file); u8 rq_sa, netf_rq_lun, msg_len; - struct i2c_client rq_client; + union i2c_smbus_data data; u8 msg[MAX_MSG_LEN]; ssize_t ret; @@ -138,17 +138,18 @@ static ssize_t ipmb_write(struct file *file, const char __user *buf, /* * subtract rq_sa and netf_rq_lun from the length of the msg passed to - * i2c_smbus_write_block_data_local + * i2c_smbus_xfer */ msg_len = msg[IPMB_MSG_LEN_IDX] - SMBUS_MSG_HEADER_LENGTH; + if (msg_len > I2C_SMBUS_BLOCK_MAX) + msg_len = I2C_SMBUS_BLOCK_MAX; - strcpy(rq_client.name, "ipmb_requester"); - rq_client.adapter = ipmb_dev->client->adapter; - rq_client.flags = ipmb_dev->client->flags; - rq_client.addr = rq_sa; - - ret = i2c_smbus_write_block_data(&rq_client, netf_rq_lun, msg_len, - msg + SMBUS_MSG_IDX_OFFSET); + data.block[0] = msg_len; + memcpy(&data.block[1], msg + SMBUS_MSG_IDX_OFFSET, msg_len); + ret = i2c_smbus_xfer(ipmb_dev->client->adapter, rq_sa, + ipmb_dev->client->flags, + I2C_SMBUS_WRITE, netf_rq_lun, + I2C_SMBUS_BLOCK_DATA, &data); return ret ? : count; } From a88b6d5668bef303d01b706a35946bfc8e67402c Mon Sep 17 00:00:00 2001 From: kbuild test robot Date: Mon, 24 Jun 2019 02:50:44 +0800 Subject: [PATCH 8/9] fix platform_no_drv_owner.cocci warnings drivers/char/ipmi/ipmb_dev_int.c:352:3-8: No need to set .owner here. The core will do it. Remove .owner field if calls are used which set it automatically Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci Fixes: 51bd6f291583 ("Add support for IPMB driver") CC: Asmaa Mnebhi Signed-off-by: kbuild test robot Message-Id: <20190623185044.GA94834@lkp-kbuild21> Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmb_dev_int.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/char/ipmi/ipmb_dev_int.c b/drivers/char/ipmi/ipmb_dev_int.c index 4d608a67f9a3..57204335c5f5 100644 --- a/drivers/char/ipmi/ipmb_dev_int.c +++ b/drivers/char/ipmi/ipmb_dev_int.c @@ -350,7 +350,6 @@ MODULE_DEVICE_TABLE(acpi, acpi_ipmb_id); static struct i2c_driver ipmb_driver = { .driver = { - .owner = THIS_MODULE, .name = "ipmb-dev", .acpi_match_table = ACPI_PTR(acpi_ipmb_id), }, From ac499fba98c3c65078fd84fa0a62cd6f6d5837ed Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Sat, 29 Jun 2019 07:36:46 -0300 Subject: [PATCH 9/9] docs: ipmb: place it at driver-api and convert to ReST No new doc should be added at the main Documentation/ directory. Instead, new docs should be added as ReST files, within the Kernel documentation body. Fixes: 51bd6f291583 ("Add support for IPMB driver") Signed-off-by: Mauro Carvalho Chehab Message-Id: Signed-off-by: Corey Minyard --- Documentation/driver-api/index.rst | 1 + .../{IPMB.txt => driver-api/ipmb.rst} | 64 ++++++++++--------- 2 files changed, 34 insertions(+), 31 deletions(-) rename Documentation/{IPMB.txt => driver-api/ipmb.rst} (71%) diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst index d26308af6036..b189bd3013ff 100644 --- a/Documentation/driver-api/index.rst +++ b/Documentation/driver-api/index.rst @@ -34,6 +34,7 @@ available subsections can be seen below. pci/index spi i2c + ipmb i3c/index hsi edac diff --git a/Documentation/IPMB.txt b/Documentation/driver-api/ipmb.rst similarity index 71% rename from Documentation/IPMB.txt rename to Documentation/driver-api/ipmb.rst index a6ed8b68bd0f..7e2265144157 100644 --- a/Documentation/IPMB.txt +++ b/Documentation/driver-api/ipmb.rst @@ -32,11 +32,11 @@ This driver works with the I2C driver and a userspace program such as OpenIPMI: 1) It is an I2C slave backend driver. So, it defines a callback -function to set the Satellite MC as an I2C slave. -This callback function handles the received IPMI requests. + function to set the Satellite MC as an I2C slave. + This callback function handles the received IPMI requests. 2) It defines the read and write functions to enable a user -space program (such as OpenIPMI) to communicate with the kernel. + space program (such as OpenIPMI) to communicate with the kernel. Load the IPMB driver @@ -48,34 +48,35 @@ CONFIG_IPMB_DEVICE_INTERFACE=y 1) If you want the driver to be loaded at boot time: -a) Add this entry to your ACPI table, under the appropriate SMBus: +a) Add this entry to your ACPI table, under the appropriate SMBus:: -Device (SMB0) // Example SMBus host controller -{ - Name (_HID, "") // Vendor-Specific HID - Name (_UID, 0) // Unique ID of particular host controller - : - : - Device (IPMB) - { - Name (_HID, "IPMB0001") // IPMB device interface - Name (_UID, 0) // Unique device identifier - } -} + Device (SMB0) // Example SMBus host controller + { + Name (_HID, "") // Vendor-Specific HID + Name (_UID, 0) // Unique ID of particular host controller + : + : + Device (IPMB) + { + Name (_HID, "IPMB0001") // IPMB device interface + Name (_UID, 0) // Unique device identifier + } + } -b) Example for device tree: +b) Example for device tree:: -&i2c2 { - status = "okay"; + &i2c2 { + status = "okay"; - ipmb@10 { - compatible = "ipmb-dev"; - reg = <0x10>; - }; -}; + ipmb@10 { + compatible = "ipmb-dev"; + reg = <0x10>; + }; + }; -2) Manually from Linux: -modprobe ipmb-dev-int +2) Manually from Linux:: + + modprobe ipmb-dev-int Instantiate the device @@ -86,15 +87,16 @@ described in 'Documentation/i2c/instantiating-devices'. If you have multiple BMCs, each connected to your Satellite MC via a different I2C bus, you can instantiate a device for each of those BMCs. -The name of the instantiated device contains the I2C bus number -associated with it as follows: -BMC1 ------ IPMB/I2C bus 1 ---------| /dev/ipmb-1 +The name of the instantiated device contains the I2C bus number +associated with it as follows:: + + BMC1 ------ IPMB/I2C bus 1 ---------| /dev/ipmb-1 Satellite MC -BMC1 ------ IPMB/I2C bus 2 ---------| /dev/ipmb-2 + BMC1 ------ IPMB/I2C bus 2 ---------| /dev/ipmb-2 For instance, you can instantiate the ipmb-dev-int device from -user space at the 7 bit address 0x10 on bus 2: +user space at the 7 bit address 0x10 on bus 2:: # echo ipmb-dev 0x1010 > /sys/bus/i2c/devices/i2c-2/new_device