Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1705987 > unrolled thread

[RFC v1 0/4] ipmi_bmc: framework for IPMI on BMCs

Back to article view | Back to linux.kernel

Contents

  [RFC v1 0/4] ipmi_bmc: framework for IPMI on BMCs Brendan Higgins <brendanhiggins@google.com> - 2017-08-08 06:00 +0200
    [RFC v1 3/4] ipmi_bmc: bt-i2c: port driver to IPMI BMC framework Brendan Higgins <brendanhiggins@google.com> - 2017-08-08 06:00 +0200

#1705987 — [RFC v1 0/4] ipmi_bmc: framework for IPMI on BMCs

This introduces a framework for implementing the BMC side of the IPMI protocol,
roughly mirroring the host side OpenIPMI framework; it attempts to abstract away
hardware interfaces, such as Block Transfer interface hardware implementations
from IPMI command handlers.

It does this by implementing the traditional driver model of a bus with devices;
however, in this case a struct ipmi_bmc_bus represents a hardware interface,
where a struct ipmi_bmc_device represents a handler. A handler filters messages
by registering a function which returns whether a given message matches the
handler; it also has the concept of a default handler which is forwarded all
messages which are not matched by some other interface.

In this patchset, we introduce an example of a default handler: a misc device
file interface which implements the same interface as the the device file
interface used by the Aspeed BT driver.

Currently, OpenBMC handles all IPMI message routing and handling in userland;
the existing drivers simply provide a file interface for the hardware on the
device. In this patchset, we propose a common file interface to be shared by all
IPMI hardware interfaces, but also a framework for implementing handlers at the
kernel level, similar to how the existing OpenIPMI framework supports both
kernel users, as well as misc device file interface.

This patchset depends on the "ipmi: bt-i2c: added IPMI Block Transfer over I2C"
patchset, which can be found here:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1461960.html
However, I can fix this if desired.

Tested on the AST2500 EVB.

[toc] | [next] | [standalone]

#1705990 — [RFC v1 3/4] ipmi_bmc: bt-i2c: port driver to IPMI BMC framework

From: Benjamin Fair <benjaminfair@google.com>

Instead of handling interaction with userspace and providing a file
interface, rely on the IPMI BMC framework to do this. This simplifies
the logic and eliminates duplicate code.

Signed-off-by: Benjamin Fair <benjaminfair@google.com>
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
 drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c | 202 +++++---------------------------
 1 file changed, 28 insertions(+), 174 deletions(-)

diff --git a/drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c b/drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c
index 686b83fa42a4..6665aa9d4300 100644
--- a/drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c
+++ b/drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c
@@ -14,102 +14,51 @@
 #include <linux/errno.h>
 #include <linux/i2c.h>
 #include <linux/ipmi_bmc.h>
-#include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
-#include <linux/poll.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
-#include <linux/wait.h>
 
 #define PFX "IPMI BMC BT-I2C: "
 
-/*
- * TODO: This is "bt-host" to match the bt-host driver; however, I think this is
- * unclear in the context of a CPU side driver. Should probably name this
- * and the DEVICE_NAME in bt-host to something like "bt-bmc" or "bt-slave".
- */
-#define DEVICE_NAME	"ipmi-bt-host"
-
-static const unsigned long request_queue_max_len = 256;
-
-struct bt_request_elem {
-	struct list_head	list;
-	struct bt_msg		request;
-};
-
 struct bt_i2c_slave {
+	struct ipmi_bmc_bus	bus;
 	struct i2c_client	*client;
-	struct miscdevice	miscdev;
+	struct ipmi_bmc_ctx	*bmc_ctx;
 	struct bt_msg		request;
-	struct list_head	request_queue;
-	atomic_t		request_queue_len;
 	struct bt_msg		response;
 	bool			response_in_progress;
 	size_t			msg_idx;
 	spinlock_t		lock;
-	wait_queue_head_t	wait_queue;
-	struct mutex		file_mutex;
 };
 
-static int receive_bt_request(struct bt_i2c_slave *bt_slave, bool non_blocking,
-			      struct bt_msg *bt_request)
+static bool bt_i2c_is_response_open(struct ipmi_bmc_bus *bus)
 {
-	int res;
+	struct bt_i2c_slave *bt_slave;
+	bool response_in_progress;
 	unsigned long flags;
-	struct bt_request_elem *queue_elem;
-
-	if (!non_blocking) {
-try_again:
-		res = wait_event_interruptible(
-				bt_slave->wait_queue,
-				atomic_read(&bt_slave->request_queue_len));
-		if (res)
-			return res;
-	}
 
-	spin_lock_irqsave(&bt_slave->lock, flags);
-	if (!atomic_read(&bt_slave->request_queue_len)) {
-		spin_unlock_irqrestore(&bt_slave->lock, flags);
-		if (non_blocking)
-			return -EAGAIN;
-		goto try_again;
-	}
+	bt_slave = container_of(bus, struct bt_i2c_slave, bus);
 
-	if (list_empty(&bt_slave->request_queue)) {
-		pr_err(PFX "request_queue was empty despite nonzero request_queue_len\n");
-		return -EIO;
-	}
-	queue_elem = list_first_entry(&bt_slave->request_queue,
-				      struct bt_request_elem, list);
-	memcpy(bt_request, &queue_elem->request, sizeof(*bt_request));
-	list_del(&queue_elem->list);
-	kfree(queue_elem);
-	atomic_dec(&bt_slave->request_queue_len);
+	spin_lock_irqsave(&bt_slave->lock, flags);
+	response_in_progress = bt_slave->response_in_progress;
 	spin_unlock_irqrestore(&bt_slave->lock, flags);
-	return 0;
+
+	return !response_in_progress;
 }
 
-static int send_bt_response(struct bt_i2c_slave *bt_slave, bool non_blocking,
-			    struct bt_msg *bt_response)
+static int bt_i2c_send_response(struct ipmi_bmc_bus *bus,
+				struct bt_msg *bt_response)
 {
-	int res;
+	struct bt_i2c_slave *bt_slave;
 	unsigned long flags;
 
-	if (!non_blocking) {
-try_again:
-		res = wait_event_interruptible(bt_slave->wait_queue,
-					       !bt_slave->response_in_progress);
-		if (res)
-			return res;
-	}
+	bt_slave = container_of(bus, struct bt_i2c_slave, bus);
 
 	spin_lock_irqsave(&bt_slave->lock, flags);
 	if (bt_slave->response_in_progress) {
 		spin_unlock_irqrestore(&bt_slave->lock, flags);
-		if (non_blocking)
-			return -EAGAIN;
-		goto try_again;
+		return -EAGAIN;
 	}
 
 	memcpy(&bt_slave->response, bt_response, sizeof(*bt_response));
@@ -118,106 +67,13 @@ static int send_bt_response(struct bt_i2c_slave *bt_slave, bool non_blocking,
 	return 0;
 }
 
-static inline struct bt_i2c_slave *to_bt_i2c_slave(struct file *file)
-{
-	return container_of(file->private_data, struct bt_i2c_slave, miscdev);
-}
-
-static ssize_t bt_read(struct file *file, char __user *buf, size_t count,
-		       loff_t *ppos)
-{
-	struct bt_i2c_slave *bt_slave = to_bt_i2c_slave(file);
-	struct bt_msg msg;
-	ssize_t ret;
-
-	mutex_lock(&bt_slave->file_mutex);
-	ret = receive_bt_request(bt_slave, file->f_flags & O_NONBLOCK, &msg);
-	if (ret < 0)
-		goto out;
-	count = min_t(size_t, count, bt_msg_len(&msg));
-	if (copy_to_user(buf, &msg, count)) {
-		ret = -EFAULT;
-		goto out;
-	}
-
-out:
-	mutex_unlock(&bt_slave->file_mutex);
-	if (ret < 0)
-		return ret;
-	else
-		return count;
-}
-
-static ssize_t bt_write(struct file *file, const char __user *buf, size_t count,
-			loff_t *ppos)
-{
-	struct bt_i2c_slave *bt_slave = to_bt_i2c_slave(file);
-	struct bt_msg msg;
-	ssize_t ret;
-
-	if (count > sizeof(msg))
-		return -EINVAL;
-
-	if (copy_from_user(&msg, buf, count) || count < bt_msg_len(&msg))
-		return -EINVAL;
-
-	mutex_lock(&bt_slave->file_mutex);
-	ret = send_bt_response(bt_slave, file->f_flags & O_NONBLOCK, &msg);
-	mutex_unlock(&bt_slave->file_mutex);
-
-	if (ret < 0)
-		return ret;
-	else
-		return count;
-}
-
-static unsigned int bt_poll(struct file *file, poll_table *wait)
-{
-	struct bt_i2c_slave *bt_slave = to_bt_i2c_slave(file);
-	unsigned int mask = 0;
-
-	mutex_lock(&bt_slave->file_mutex);
-	poll_wait(file, &bt_slave->wait_queue, wait);
-
-	if (atomic_read(&bt_slave->request_queue_len))
-		mask |= POLLIN;
-	if (!bt_slave->response_in_progress)
-		mask |= POLLOUT;
-	mutex_unlock(&bt_slave->file_mutex);
-	return mask;
-}
-
-static const struct file_operations bt_fops = {
-	.owner		= THIS_MODULE,
-	.read		= bt_read,
-	.write		= bt_write,
-	.poll		= bt_poll,
-};
-
-/* Called with bt_slave->lock held. */
-static int handle_request(struct bt_i2c_slave *bt_slave)
-{
-	struct bt_request_elem *queue_elem;
-
-	if (atomic_read(&bt_slave->request_queue_len) >= request_queue_max_len)
-		return -EFAULT;
-	queue_elem = kmalloc(sizeof(*queue_elem), GFP_KERNEL);
-	if (!queue_elem)
-		return -ENOMEM;
-	memcpy(&queue_elem->request, &bt_slave->request, sizeof(struct bt_msg));
-	list_add(&queue_elem->list, &bt_slave->request_queue);
-	atomic_inc(&bt_slave->request_queue_len);
-	wake_up_all(&bt_slave->wait_queue);
-	return 0;
-}
-
 /* Called with bt_slave->lock held. */
 static int complete_response(struct bt_i2c_slave *bt_slave)
 {
 	/* Invalidate response in buffer to denote it having been sent. */
 	bt_slave->response.len = 0;
 	bt_slave->response_in_progress = false;
-	wake_up_all(&bt_slave->wait_queue);
+	ipmi_bmc_signal_response_open(bt_slave->bmc_ctx);
 	return 0;
 }
 
@@ -240,7 +96,8 @@ static int bt_i2c_slave_cb(struct i2c_client *client,
 
 		buf[bt_slave->msg_idx++] = *val;
 		if (bt_slave->msg_idx >= bt_msg_len(&bt_slave->request))
-			handle_request(bt_slave);
+			ipmi_bmc_handle_request(bt_slave->bmc_ctx,
+						&bt_slave->request);
 		break;
 
 	case I2C_SLAVE_READ_REQUESTED:
@@ -290,26 +147,24 @@ static int bt_i2c_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	spin_lock_init(&bt_slave->lock);
-	init_waitqueue_head(&bt_slave->wait_queue);
-	atomic_set(&bt_slave->request_queue_len, 0);
 	bt_slave->response_in_progress = false;
-	INIT_LIST_HEAD(&bt_slave->request_queue);
+	bt_slave->bus.send_response = bt_i2c_send_response;
+	bt_slave->bus.is_response_open = bt_i2c_is_response_open;
 
-	mutex_init(&bt_slave->file_mutex);
+	bt_slave->bmc_ctx = ipmi_bmc_get_global_ctx();
 
-	bt_slave->miscdev.minor = MISC_DYNAMIC_MINOR;
-	bt_slave->miscdev.name = DEVICE_NAME;
-	bt_slave->miscdev.fops = &bt_fops;
-	bt_slave->miscdev.parent = &client->dev;
-	ret = misc_register(&bt_slave->miscdev);
-	if (ret)
+	ret = ipmi_bmc_register_bus(bt_slave->bmc_ctx, &bt_slave->bus);
+	if (ret) {
+		pr_err(PFX "Failed to register IPMI BMC bus\n");
 		return ret;
+	}
 
 	bt_slave->client = client;
 	i2c_set_clientdata(client, bt_slave);
 	ret = i2c_slave_register(client, bt_i2c_slave_cb);
+
 	if (ret) {
-		misc_deregister(&bt_slave->miscdev);
+		ipmi_bmc_unregister_bus(bt_slave->bmc_ctx, &bt_slave->bus);
 		return ret;
 	}
 
@@ -321,8 +176,7 @@ static int bt_i2c_remove(struct i2c_client *client)
 	struct bt_i2c_slave *bt_slave = i2c_get_clientdata(client);
 
 	i2c_slave_unregister(client);
-	misc_deregister(&bt_slave->miscdev);
-	return 0;
+	return ipmi_bmc_unregister_bus(bt_slave->bmc_ctx, &bt_slave->bus);
 }
 
 static const struct i2c_device_id bt_i2c_id[] = {
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web