All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C
@ 2017-08-04 23:18 Brendan Higgins
  2017-08-04 23:18 ` [PATCH v1 1/3] ipmi: bt-i2c: added documentation for bt-i2c drivers Brendan Higgins
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Brendan Higgins @ 2017-08-04 23:18 UTC (permalink / raw)
  To: corbet, robh+dt, mark.rutland, arnd, gregkh, minyard, joel, benh
  Cc: linux-doc, devicetree, openipmi-developer, openbmc, linux-kernel

This patchset introduces IPMI Block Transfer over I2C (BT-I2C), which has the
same semantics as IPMI Block Transfer except it done over I2C.

The documentation discusses the reason for this in greater detail, suffice it to
say SSIF cannot be correctly implemented on some naive I2C devices. There are
some additional reasons why we don't like SSIF, but those are again covered in
the documentation for all those who are interested.

In addition, since I am adding both host side and BMC side support, I figured
that now is a good time to resolve the problem of where to put BMC side IPMI
drivers; right now we have it (there is only one) in drivers/char/ipmi/ with the
rest of the host side IPMI drivers, but I think it makes sense to put all of the
host side IPMI drivers in one directory and all of the BMC side drivers in
another, preferably in a way that does not effect all of the current OpenIPMI
users. I have not created a MAINTAINERS entry for the new directory yet, as I
figured there might be some discussion to be had about it.

I have tested this patchset on the Aspeed 2500 EVB.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v1 1/3] ipmi: bt-i2c: added documentation for bt-i2c drivers
  2017-08-04 23:18 [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C Brendan Higgins
@ 2017-08-04 23:18 ` Brendan Higgins
  2017-08-04 23:18   ` Brendan Higgins
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Brendan Higgins @ 2017-08-04 23:18 UTC (permalink / raw)
  To: corbet, robh+dt, mark.rutland, arnd, gregkh, minyard, joel, benh
  Cc: linux-doc, devicetree, openipmi-developer, openbmc, linux-kernel,
	Brendan Higgins

Added device tree binding documentation for ipmi-bt-i2c (host) and
ipmi-bmc-bt-i2c (BMC) and documentation for the Block Transfer over I2C
(bt-i2c) protocol.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
 Documentation/bt-i2c.txt                           | 121 +++++++++++++++++++++
 .../devicetree/bindings/ipmi/ipmi-bt-i2c.txt       |  21 ++++
 .../bindings/ipmi_bmc/ipmi-bmc-bt-i2c.txt          |  21 ++++
 3 files changed, 163 insertions(+)
 create mode 100644 Documentation/bt-i2c.txt
 create mode 100644 Documentation/devicetree/bindings/ipmi/ipmi-bt-i2c.txt
 create mode 100644 Documentation/devicetree/bindings/ipmi_bmc/ipmi-bmc-bt-i2c.txt

diff --git a/Documentation/bt-i2c.txt b/Documentation/bt-i2c.txt
new file mode 100644
index 000000000000..1b375359c519
--- /dev/null
+++ b/Documentation/bt-i2c.txt
@@ -0,0 +1,121 @@
+Linux Block Transfer over I2C (bt-i2c) interface description
+============================================================
+
+by Brendan Higgins <brendanhiggins@google.com> in 2016
+
+Introduction
+------------
+
+IPMI defines an interface for communication between a CPU, a BMC (Baseboard
+Management Controller), and sensors and various other peripherals. For a more
+complete description of IPMI please see:
+http://www.intel.com/content/www/us/en/servers/ipmi/ipmi-second-gen-interface-spec-v2-rev1-1.html
+
+IPMI defines a *common* message format, as in a set of fields that are common
+across all IPMI messages; they could be viewed as part of the framing
+information for an IPMI message. They include:
+
+	- netfn
+	- lun
+	- cmd
+
+netfn and cmd together define the type of the message; netfn can be viewed as a
+message class and cmd is a subtype of sorts. lun (logical unit number) is used
+for routing between messages between different interfaces. After the last field
+there is usually a variable length payload. Despite these common fields, the
+remainder of the framing varies widely between the IPMI defined hardware
+interfaces; some specify a length as part of the framing which is never more
+than a byte in length; others use a special signal to denote the end of message.
+Some IPMI hardware interfaces, the Block Transfer interface in particular,
+support a sequence number that aids in support of multiple in-flight IPMI
+messages.
+
+IPMI defines SSIF (SMBus System Interface) as the IPMI hardware interface for
+SMBus/I2C. It supports a maximum total message length of 255 bytes that is
+broken up across several SMBus block operations. It does not define a sequence
+field in the IPMI framing making it very difficult to support multiple in flight
+messages (it is also intentionally left out of the specification). SSIF also
+requires the slave device to NACK until it is ready to accept data (technically
+it only specifies that it may NACK until it is ready, but must NACK on attempted
+reads if it does not support SMBus Alert; however, this is an effective
+requirements since a slave device is supposed to start with SMBus Alert
+disabled); this again makes SSIF very difficult to support for some slave
+devices which may not support NACKing arbitrary messages; indeed, at the time of
+writing, the Linux I2C slave driver framework did not have support for sending
+NACKs.
+
+Block Transfer over I2C defines a new IPMI compatible interface that uses Block
+Transfer messages and semantics on top of plain old I2C; it does not assume that
+the I2C slave is capable of NACKing arbitrary messages; however, it is designed
+such that it could take advantage of SMBus Alert so that the master does not
+have to poll (the Linux I2C core slave mode does not currently support SMBus
+Alert, but a patch adding this support is currently on the way).
+
+Protocol Definition
+-------------------
+
+Block Transfer over I2C uses the IPMI defined Block Transfer message format; it
+supports variable length messages with a maximum length of 255 bytes (limited by
+the IPMI Block Transfer length byte).
+
+A Block Transfer over I2C Request is structured as follows:
+
+------------------------------------------------------------------------------------------------------
+| I2C start | slave address / RW bit unset | Block Transfer message | ... (another message or stop ) |
+------------------------------------------------------------------------------------------------------
+
+Multiple requests can be sent before any responses are received. Sequence
+numbers are to be handled by the users of the drivers; thus, no semantics are
+prescribed to their usage; however, the slave driver is required to buffer at
+least 256 requests before dropping requests; this can be used in conjunction
+with sequence numbers to prevent messages from being dropped by the slave.
+
+A Block Transfer over I2C Response is structured as follows:
+
+----------------------------------------------------------------------------------------------------
+| I2C start | slave address / RW bit set | Block Transfer message | ... (another message or stop ) |
+----------------------------------------------------------------------------------------------------
+
+Until a response is ready to send, the slave will respond only with zero bytes.
+If the slave receives a start or a stop before it was done sending a response,
+it will resend the entire response the next time a read is requested; in this
+way, an I2C master may poll for responses by reading a single byte until it is
+non-zero and then perform the read transaction shown above.
+
+In the future, it is planned that the slave will support using SMBus Alert to
+notify the master of an available response, but will never be required.
+
+Driver Interface
+----------------
+
+The slave side of the device file interface is modeled after ipmi-bmc-bt-i2c, a
+driver for the Aspeed 24xx/255xx IPMI Block Transfer controller.
+
+A read (request) should be large enough to fit a Block Transfer message
+(including the length byte) of 256 bytes; if the provided buffer is smaller, the
+message will be truncated.
+
+A write (response) must be no greater than the maximum valid length of a Block
+Transfer message (including the length byte), 256 bytes; if the provided buffer
+is larger, the write will fail with EINVAL. The driver also enforces a valid
+length byte which must contain the total length of the message not including the
+length byte; thus, the write will fail with EINVAL if the buffer length is less
+than the length field plus one. The driver will also only send length field plus
+one bytes.
+
+The slave device file interface also supports the poll system call; it will
+report when a request is available to read or it is ready to accept a response.
+
+The master side conforms to the OpenIPMI kernel framework.
+
+A read (response) should be large enough to fit a Block Transfer message
+(including the length byte) of 256 bytes; if the provided buffer is smaller, the
+message will be truncated.
+
+A write (request) must be no greater than the maximum valid length of a Block
+Transfer message (including the length byte), 256 bytes; if the provided buffer
+is larger, the write will fail with EFAULT. The driver also enforces a valid
+length byte which must contain the total length of the message not including the
+length byte; thus, the write will fail with EFAULT if the buffer length is less
+than the length field plus one. The driver will also only send length field plus
+one bytes.
diff --git a/Documentation/devicetree/bindings/ipmi/ipmi-bt-i2c.txt b/Documentation/devicetree/bindings/ipmi/ipmi-bt-i2c.txt
new file mode 100644
index 000000000000..bd956f2805e4
--- /dev/null
+++ b/Documentation/devicetree/bindings/ipmi/ipmi-bt-i2c.txt
@@ -0,0 +1,21 @@
+Device tree configuration for the ipmi-bt-i2c driver.
+
+Required Properties:
+- compatible 	: should be "ipmi-bt-i2c"
+- reg		: the I2C slave address of the ipmi-bmc-bt-i2c
+
+i2c0: i2c-bus@40 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x40 0x40>;
+	compatible = "aspeed,ast2400-i2c-bus";
+	clock-frequency = <100000>;
+	status = "disabled";
+	interrupts = <0>;
+	interrupt-parent = <&i2c>;
+
+	bt-i2c-master@41 {
+		compatible = "ipmi-bt-i2c";
+		reg = <0x41>;
+	};
+};
diff --git a/Documentation/devicetree/bindings/ipmi_bmc/ipmi-bmc-bt-i2c.txt b/Documentation/devicetree/bindings/ipmi_bmc/ipmi-bmc-bt-i2c.txt
new file mode 100644
index 000000000000..6e82693ee50e
--- /dev/null
+++ b/Documentation/devicetree/bindings/ipmi_bmc/ipmi-bmc-bt-i2c.txt
@@ -0,0 +1,21 @@
+Device tree configuration for the ipmi-bmc-bt-i2c driver.
+
+Required Properties:
+- compatible 	: should be "ipmi-bmc-bt-i2c"
+- reg		: the I2C slave address of this driver.
+
+i2c0: i2c-bus@40 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x40 0x40>;
+	compatible = "aspeed,ast2400-i2c-bus";
+	clock-frequency = <100000>;
+	status = "disabled";
+	interrupts = <0>;
+	interrupt-parent = <&i2c>;
+
+	bt-i2c-slave@41 {
+		compatible = "ipmi-bmc-bt-i2c";
+		reg = <0x41>;
+	};
+};
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 2/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C host side
@ 2017-08-04 23:18   ` Brendan Higgins
  0 siblings, 0 replies; 15+ messages in thread
From: Brendan Higgins @ 2017-08-04 23:18 UTC (permalink / raw)
  To: corbet, robh+dt, mark.rutland, arnd, gregkh, minyard, joel, benh
  Cc: linux-doc, devicetree, openipmi-developer, openbmc, linux-kernel,
	Brendan Higgins

The IPMI definition of the Block Transfer protocol defines the hardware
registers and behavior in addition to the message format and messaging
semantics. This implements a new protocol that uses IPMI Block Transfer
messages and semantics on top of a standard I2C interface.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
 drivers/char/ipmi/Kconfig       |   4 +
 drivers/char/ipmi/Makefile      |   1 +
 drivers/char/ipmi/ipmi_bt_i2c.c | 452 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 457 insertions(+)
 create mode 100644 drivers/char/ipmi/ipmi_bt_i2c.c

diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index f6fa056a52fc..a8734a369cb0 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -79,6 +79,10 @@ config IPMI_POWEROFF
          This enables a function to power off the system with IPMI if
 	 the IPMI management controller is capable of this.
 
+config IPMI_BT_I2C
+	select I2C
+	tristate 'BT IPMI bmc driver over I2c'
+
 endif # IPMI_HANDLER
 
 config ASPEED_BT_IPMI_BMC
diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
index eefb0b301e83..323de0b0b8b5 100644
--- a/drivers/char/ipmi/Makefile
+++ b/drivers/char/ipmi/Makefile
@@ -12,4 +12,5 @@ obj-$(CONFIG_IPMI_SSIF) += ipmi_ssif.o
 obj-$(CONFIG_IPMI_POWERNV) += ipmi_powernv.o
 obj-$(CONFIG_IPMI_WATCHDOG) += ipmi_watchdog.o
 obj-$(CONFIG_IPMI_POWEROFF) += ipmi_poweroff.o
+obj-$(CONFIG_IPMI_BT_I2C) += ipmi_bt_i2c.o
 obj-$(CONFIG_ASPEED_BT_IPMI_BMC) += bt-bmc.o
diff --git a/drivers/char/ipmi/ipmi_bt_i2c.c b/drivers/char/ipmi/ipmi_bt_i2c.c
new file mode 100644
index 000000000000..94b5c11d23cd
--- /dev/null
+++ b/drivers/char/ipmi/ipmi_bt_i2c.c
@@ -0,0 +1,452 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt)        "ipmi-bt-i2c: " fmt
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/errno.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/ipmi_smi.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/timer.h>
+#include <linux/jiffies.h>
+#include <linux/types.h>
+
+#define IPMI_BT_I2C_TIMEOUT (msecs_to_jiffies(1000))
+
+/* If we don't have netfn_lun, seq, and cmd, we might as well have nothing. */
+#define IPMI_BT_I2C_LEN_MIN 3
+/* We need at least netfn_lun, seq, cmd, and completion. */
+#define IPMI_BT_I2C_RESPONSE_LEN_MIN 4
+#define IPMI_BT_I2C_MSG_MAX_PAYLOAD_SIZE 252
+
+struct ipmi_bt_i2c_msg {
+	u8 len;
+	u8 netfn_lun;
+	u8 seq;
+	u8 cmd;
+	u8 payload[IPMI_BT_I2C_MSG_MAX_PAYLOAD_SIZE];
+} __packed;
+
+#define IPMI_BT_I2C_MAX_SMI_SIZE 254 /* Need extra byte for seq. */
+#define IPMI_BT_I2C_SMI_MSG_HEADER_SIZE 2
+
+struct ipmi_bt_i2c_smi_msg {
+	u8 netfn_lun;
+	u8 cmd;
+	u8 payload[IPMI_MAX_MSG_LENGTH - 2];
+} __packed;
+
+static inline u32 bt_msg_len(struct ipmi_bt_i2c_msg *bt_request)
+{
+	return bt_request->len + 1;
+}
+
+#define IPMI_BT_I2C_SEQ_MAX 256
+
+struct ipmi_bt_i2c_seq_entry {
+	struct ipmi_smi_msg		*msg;
+	unsigned long			send_time;
+};
+
+struct ipmi_bt_i2c_master {
+	struct ipmi_device_id		ipmi_id;
+	struct i2c_client		*client;
+	ipmi_smi_t			intf;
+	spinlock_t			lock;
+	struct ipmi_bt_i2c_seq_entry	seq_msg_map[IPMI_BT_I2C_SEQ_MAX];
+	struct work_struct		ipmi_bt_i2c_recv_work;
+	struct work_struct		ipmi_bt_i2c_send_work;
+	struct ipmi_smi_msg		*msg_to_send;
+};
+
+static const unsigned long write_timeout = 25;
+
+static int ipmi_bt_i2c_send_request(struct ipmi_bt_i2c_master *master,
+				    struct ipmi_bt_i2c_msg *request)
+{
+	struct i2c_client *client = master->client;
+	unsigned long timeout, read_time;
+	u8 *buf = (u8 *) request;
+	int ret;
+
+	timeout = jiffies + msecs_to_jiffies(write_timeout);
+	do {
+		read_time = jiffies;
+		ret = i2c_master_send(client, buf, bt_msg_len(request));
+		if (ret >= 0)
+			return 0;
+		usleep_range(1000, 1500);
+	} while (time_before(read_time, timeout));
+	return ret;
+}
+
+static int ipmi_bt_i2c_receive_response(struct ipmi_bt_i2c_master *master,
+					struct ipmi_bt_i2c_msg *response)
+{
+	struct i2c_client *client = master->client;
+	unsigned long timeout, read_time;
+	u8 *buf = (u8 *) response;
+	u8 len = 0;
+	int ret;
+
+	/*
+	 * Slave may not NACK when not ready, so we peek at the first byte to
+	 * see if it is a valid length.
+	 */
+	ret = i2c_master_recv(client, &len, 1);
+	while (ret != 1 || len == 0) {
+		if (ret < 0)
+			return ret;
+
+		usleep_range(1000, 1500);
+
+		/* Signal received: quit syscall. */
+		if (signal_pending(current))
+			return -ERESTARTSYS;
+
+		ret = i2c_master_recv(client, &len, 1);
+	}
+
+	timeout = jiffies + msecs_to_jiffies(write_timeout);
+	do {
+		read_time = jiffies;
+		ret = i2c_master_recv(client, buf, len + 1);
+		if (ret >= 0)
+			return 0;
+		usleep_range(1000, 1500);
+	} while (time_before(read_time, timeout));
+	return ret;
+}
+
+static int ipmi_bt_i2c_start_processing(void *data, ipmi_smi_t intf)
+{
+	struct ipmi_bt_i2c_master *master = data;
+
+	master->intf = intf;
+
+	return 0;
+}
+
+static void __ipmi_bt_i2c_error_reply(struct ipmi_bt_i2c_master *master,
+				      struct ipmi_smi_msg *msg,
+				      u8 completion_code)
+{
+	struct ipmi_bt_i2c_smi_msg *response;
+	struct ipmi_bt_i2c_smi_msg *request;
+
+	response = (struct ipmi_bt_i2c_smi_msg *) msg->rsp;
+	request = (struct ipmi_bt_i2c_smi_msg *) msg->data;
+
+	response->netfn_lun = request->netfn_lun | 0x4;
+	response->cmd = request->cmd;
+	response->payload[0] = completion_code;
+	msg->rsp_size = 3;
+	ipmi_smi_msg_received(master->intf, msg);
+}
+
+static void ipmi_bt_i2c_error_reply(struct ipmi_bt_i2c_master *master,
+				    struct ipmi_smi_msg *msg,
+				    u8 completion_code)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&master->lock, flags);
+	__ipmi_bt_i2c_error_reply(master, msg, completion_code);
+	spin_unlock_irqrestore(&master->lock, flags);
+}
+
+/*
+ * ipmi_bt_i2c_smi_msg contains a payload and 2 header fields, each 1 byte:
+ * netfn_lun and cmd. They're passed to OpenIPMI within an ipmi_smi_msg struct
+ * along with their length.
+ *
+ * ipmi_bt_i2c_msg contains a payload and 4 header fields: the two above in
+ * addition to seq and len. However, len is not included in the length count so
+ * this message encapsulation is considered 1 byte longer than the other.
+ */
+static u8 ipmi_bt_i2c_smi_to_bt_len(u8 smi_msg_len)
+{
+	/* Only field that BT adds to the header is seq. */
+	return smi_msg_len + 1;
+}
+
+static u8 ipmi_bt_i2c_bt_to_smi_len(struct ipmi_bt_i2c_msg *bt_msg)
+{
+	/* Subtract one byte for seq (opposite of above) */
+	return bt_msg->len - 1;
+}
+
+static size_t ipmi_bt_i2c_payload_len(struct ipmi_bt_i2c_msg *bt_msg)
+{
+	/* Subtract one byte for each: netfn_lun, seq, cmd. */
+	return bt_msg->len - 3;
+}
+
+static bool ipmi_bt_i2c_assign_seq(struct ipmi_bt_i2c_master *master,
+				   struct ipmi_smi_msg *msg, u8 *ret_seq)
+{
+	struct ipmi_bt_i2c_seq_entry *entry;
+	bool did_cleanup = false;
+	unsigned long flags;
+	u8 seq;
+
+	spin_lock_irqsave(&master->lock, flags);
+retry:
+	for (seq = 0; seq < IPMI_BT_I2C_SEQ_MAX; seq++) {
+		if (!master->seq_msg_map[seq].msg) {
+			master->seq_msg_map[seq].msg = msg;
+			master->seq_msg_map[seq].send_time = jiffies;
+			spin_unlock_irqrestore(&master->lock, flags);
+			*ret_seq = seq;
+			return true;
+		}
+	}
+
+	if (did_cleanup) {
+		spin_unlock_irqrestore(&master->lock, flags);
+		return false;
+	}
+
+	/*
+	 * TODO: we should do cleanup at times other than only when we run out
+	 * of sequence numbers.
+	 */
+	for (seq = 0; seq < IPMI_BT_I2C_SEQ_MAX; seq++) {
+		entry = &master->seq_msg_map[seq];
+		if (entry->msg &&
+		    time_after(entry->send_time + IPMI_BT_I2C_TIMEOUT,
+			       jiffies)) {
+			__ipmi_bt_i2c_error_reply(master, entry->msg,
+						  IPMI_TIMEOUT_ERR);
+			entry->msg = NULL;
+		}
+	}
+	did_cleanup = true;
+	goto retry;
+}
+
+static struct ipmi_smi_msg *ipmi_bt_i2c_find_msg(
+		struct ipmi_bt_i2c_master *master, u8 seq)
+{
+	struct ipmi_smi_msg *msg;
+	unsigned long flags;
+
+	spin_lock_irqsave(&master->lock, flags);
+	msg = master->seq_msg_map[seq].msg;
+	spin_unlock_irqrestore(&master->lock, flags);
+	return msg;
+}
+
+static void ipmi_bt_i2c_free_seq(struct ipmi_bt_i2c_master *master, u8 seq)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&master->lock, flags);
+	master->seq_msg_map[seq].msg = NULL;
+	spin_unlock_irqrestore(&master->lock, flags);
+}
+
+static void ipmi_bt_i2c_send_workfn(struct work_struct *work)
+{
+	struct ipmi_bt_i2c_smi_msg *smi_msg;
+	struct ipmi_bt_i2c_master *master;
+	struct ipmi_bt_i2c_msg bt_msg;
+	struct ipmi_smi_msg *msg;
+	size_t smi_msg_size;
+	unsigned long flags;
+
+	master = container_of(work, struct ipmi_bt_i2c_master,
+			      ipmi_bt_i2c_send_work);
+
+	msg = master->msg_to_send;
+	smi_msg_size = msg->data_size;
+	smi_msg = (struct ipmi_bt_i2c_smi_msg *) msg->data;
+
+	if (smi_msg_size > IPMI_BT_I2C_MAX_SMI_SIZE) {
+		ipmi_bt_i2c_error_reply(master, msg, IPMI_REQ_LEN_EXCEEDED_ERR);
+		return;
+	}
+
+	if (smi_msg_size < IPMI_BT_I2C_SMI_MSG_HEADER_SIZE) {
+		ipmi_bt_i2c_error_reply(master, msg, IPMI_REQ_LEN_INVALID_ERR);
+		return;
+	}
+
+	if (!ipmi_bt_i2c_assign_seq(master, msg, &bt_msg.seq)) {
+		ipmi_bt_i2c_error_reply(master, msg, IPMI_NODE_BUSY_ERR);
+		return;
+	}
+
+	bt_msg.len = ipmi_bt_i2c_smi_to_bt_len(smi_msg_size);
+	bt_msg.netfn_lun = smi_msg->netfn_lun;
+	bt_msg.cmd = smi_msg->cmd;
+	memcpy(bt_msg.payload, smi_msg->payload,
+	       ipmi_bt_i2c_payload_len(&bt_msg));
+
+	if (ipmi_bt_i2c_send_request(master, &bt_msg) < 0) {
+		ipmi_bt_i2c_free_seq(master, bt_msg.seq);
+		ipmi_bt_i2c_error_reply(master, msg, IPMI_BUS_ERR);
+	}
+
+	spin_lock_irqsave(&master->lock, flags);
+	master->msg_to_send = NULL;
+	spin_unlock_irqrestore(&master->lock, flags);
+}
+
+void ipmi_bt_i2c_recv_workfn(struct work_struct *work)
+{
+	struct ipmi_bt_i2c_smi_msg *smi_msg;
+	struct ipmi_bt_i2c_master *master;
+	struct ipmi_bt_i2c_msg bt_msg;
+	struct ipmi_smi_msg *msg;
+
+	master = container_of(work, struct ipmi_bt_i2c_master,
+			      ipmi_bt_i2c_recv_work);
+
+	if (ipmi_bt_i2c_receive_response(master, &bt_msg) < 0)
+		return;
+
+	if (bt_msg.len < IPMI_BT_I2C_LEN_MIN)
+		return;
+
+	msg = ipmi_bt_i2c_find_msg(master, bt_msg.seq);
+	if (!msg)
+		return;
+
+	ipmi_bt_i2c_free_seq(master, bt_msg.seq);
+
+	if (bt_msg.len < IPMI_BT_I2C_RESPONSE_LEN_MIN)
+		ipmi_bt_i2c_error_reply(master, msg, IPMI_ERR_MSG_TRUNCATED);
+
+	msg->rsp_size = ipmi_bt_i2c_bt_to_smi_len(&bt_msg);
+	smi_msg = (struct ipmi_bt_i2c_smi_msg *) msg->rsp;
+	smi_msg->netfn_lun = bt_msg.netfn_lun;
+	smi_msg->cmd = bt_msg.cmd;
+	memcpy(smi_msg->payload, bt_msg.payload,
+	       ipmi_bt_i2c_payload_len(&bt_msg));
+	ipmi_smi_msg_received(master->intf, msg);
+}
+
+static void ipmi_bt_i2c_sender(void *data, struct ipmi_smi_msg *msg)
+{
+	struct ipmi_bt_i2c_master *master = data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&master->lock, flags);
+	if (master->msg_to_send) {
+		/*
+		 * TODO(benjaminfair): Queue messages to send instead of only
+		 * keeping one.
+		 */
+		__ipmi_bt_i2c_error_reply(master, msg, IPMI_NODE_BUSY_ERR);
+	} else {
+		master->msg_to_send = msg;
+		schedule_work(&master->ipmi_bt_i2c_send_work);
+	}
+	spin_unlock_irqrestore(&master->lock, flags);
+}
+
+static void ipmi_bt_i2c_request_events(void *data)
+{
+	struct ipmi_bt_i2c_master *master = data;
+
+	schedule_work(&master->ipmi_bt_i2c_recv_work);
+}
+
+static void ipmi_bt_i2c_set_run_to_completion(void *data,
+					      bool run_to_completion)
+{
+}
+
+static void ipmi_bt_i2c_poll(void *data)
+{
+	struct ipmi_bt_i2c_master *master = data;
+
+	schedule_work(&master->ipmi_bt_i2c_recv_work);
+}
+
+static struct ipmi_smi_handlers ipmi_bt_i2c_smi_handlers = {
+	.owner			= THIS_MODULE,
+	.start_processing	= ipmi_bt_i2c_start_processing,
+	.sender			= ipmi_bt_i2c_sender,
+	.request_events		= ipmi_bt_i2c_request_events,
+	.set_run_to_completion	= ipmi_bt_i2c_set_run_to_completion,
+	.poll			= ipmi_bt_i2c_poll,
+};
+
+static int ipmi_bt_i2c_probe(struct i2c_client *client,
+			     const struct i2c_device_id *id)
+{
+	struct ipmi_bt_i2c_master *master;
+	int ret;
+
+	master = devm_kzalloc(&client->dev, sizeof(struct ipmi_bt_i2c_master),
+			      GFP_KERNEL);
+	if (!master)
+		return -ENOMEM;
+
+	spin_lock_init(&master->lock);
+	INIT_WORK(&master->ipmi_bt_i2c_recv_work, ipmi_bt_i2c_recv_workfn);
+	INIT_WORK(&master->ipmi_bt_i2c_send_work, ipmi_bt_i2c_send_workfn);
+	master->client = client;
+	i2c_set_clientdata(client, master);
+
+	/*
+	 * TODO(benjaminfair): read ipmi_device_id from BMC to determine version
+	 * information and be able to tell multiple BMCs apart
+	 */
+	ret = ipmi_register_smi(&ipmi_bt_i2c_smi_handlers, master,
+				&master->ipmi_id, &client->dev, 0);
+
+	return ret;
+}
+
+static int ipmi_bt_i2c_remove(struct i2c_client *client)
+{
+	struct ipmi_bt_i2c_master *master;
+
+	master = i2c_get_clientdata(client);
+	return ipmi_unregister_smi(master->intf);
+}
+
+static const struct acpi_device_id ipmi_bt_i2c_acpi_id[] = {
+	{"BTMA0001", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, ipmi_bt_i2c_acpi_id);
+
+static const struct i2c_device_id ipmi_bt_i2c_i2c_id[] = {
+	{"ipmi-bt-i2c", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, ipmi_bt_i2c_i2c_id);
+
+static struct i2c_driver ipmi_bt_i2c_driver = {
+	.driver = {
+		.name = "ipmi-bt-i2c",
+		.acpi_match_table = ipmi_bt_i2c_acpi_id,
+	},
+	.id_table = ipmi_bt_i2c_i2c_id,
+	.probe = ipmi_bt_i2c_probe,
+	.remove = ipmi_bt_i2c_remove,
+};
+module_i2c_driver(ipmi_bt_i2c_driver);
+
+MODULE_AUTHOR("Brendan Higgins <brendanhiggins@google.com>");
+MODULE_DESCRIPTION("IPMI Block Transfer over I2C.");
+MODULE_LICENSE("GPL v2");
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 2/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C host side
@ 2017-08-04 23:18   ` Brendan Higgins
  0 siblings, 0 replies; 15+ messages in thread
From: Brendan Higgins @ 2017-08-04 23:18 UTC (permalink / raw)
  To: corbet-T1hC0tSOHrs, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, arnd-r2nGTMty4D4,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, minyard-HInyCGIudOg,
	joel-U3u1mxZcP9KHXe+LvDLADg,
	benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r
  Cc: linux-doc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	openipmi-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Brendan Higgins

The IPMI definition of the Block Transfer protocol defines the hardware
registers and behavior in addition to the message format and messaging
semantics. This implements a new protocol that uses IPMI Block Transfer
messages and semantics on top of a standard I2C interface.

Signed-off-by: Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 drivers/char/ipmi/Kconfig       |   4 +
 drivers/char/ipmi/Makefile      |   1 +
 drivers/char/ipmi/ipmi_bt_i2c.c | 452 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 457 insertions(+)
 create mode 100644 drivers/char/ipmi/ipmi_bt_i2c.c

diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index f6fa056a52fc..a8734a369cb0 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -79,6 +79,10 @@ config IPMI_POWEROFF
          This enables a function to power off the system with IPMI if
 	 the IPMI management controller is capable of this.
 
+config IPMI_BT_I2C
+	select I2C
+	tristate 'BT IPMI bmc driver over I2c'
+
 endif # IPMI_HANDLER
 
 config ASPEED_BT_IPMI_BMC
diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
index eefb0b301e83..323de0b0b8b5 100644
--- a/drivers/char/ipmi/Makefile
+++ b/drivers/char/ipmi/Makefile
@@ -12,4 +12,5 @@ obj-$(CONFIG_IPMI_SSIF) += ipmi_ssif.o
 obj-$(CONFIG_IPMI_POWERNV) += ipmi_powernv.o
 obj-$(CONFIG_IPMI_WATCHDOG) += ipmi_watchdog.o
 obj-$(CONFIG_IPMI_POWEROFF) += ipmi_poweroff.o
+obj-$(CONFIG_IPMI_BT_I2C) += ipmi_bt_i2c.o
 obj-$(CONFIG_ASPEED_BT_IPMI_BMC) += bt-bmc.o
diff --git a/drivers/char/ipmi/ipmi_bt_i2c.c b/drivers/char/ipmi/ipmi_bt_i2c.c
new file mode 100644
index 000000000000..94b5c11d23cd
--- /dev/null
+++ b/drivers/char/ipmi/ipmi_bt_i2c.c
@@ -0,0 +1,452 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt)        "ipmi-bt-i2c: " fmt
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/errno.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/ipmi_smi.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/timer.h>
+#include <linux/jiffies.h>
+#include <linux/types.h>
+
+#define IPMI_BT_I2C_TIMEOUT (msecs_to_jiffies(1000))
+
+/* If we don't have netfn_lun, seq, and cmd, we might as well have nothing. */
+#define IPMI_BT_I2C_LEN_MIN 3
+/* We need at least netfn_lun, seq, cmd, and completion. */
+#define IPMI_BT_I2C_RESPONSE_LEN_MIN 4
+#define IPMI_BT_I2C_MSG_MAX_PAYLOAD_SIZE 252
+
+struct ipmi_bt_i2c_msg {
+	u8 len;
+	u8 netfn_lun;
+	u8 seq;
+	u8 cmd;
+	u8 payload[IPMI_BT_I2C_MSG_MAX_PAYLOAD_SIZE];
+} __packed;
+
+#define IPMI_BT_I2C_MAX_SMI_SIZE 254 /* Need extra byte for seq. */
+#define IPMI_BT_I2C_SMI_MSG_HEADER_SIZE 2
+
+struct ipmi_bt_i2c_smi_msg {
+	u8 netfn_lun;
+	u8 cmd;
+	u8 payload[IPMI_MAX_MSG_LENGTH - 2];
+} __packed;
+
+static inline u32 bt_msg_len(struct ipmi_bt_i2c_msg *bt_request)
+{
+	return bt_request->len + 1;
+}
+
+#define IPMI_BT_I2C_SEQ_MAX 256
+
+struct ipmi_bt_i2c_seq_entry {
+	struct ipmi_smi_msg		*msg;
+	unsigned long			send_time;
+};
+
+struct ipmi_bt_i2c_master {
+	struct ipmi_device_id		ipmi_id;
+	struct i2c_client		*client;
+	ipmi_smi_t			intf;
+	spinlock_t			lock;
+	struct ipmi_bt_i2c_seq_entry	seq_msg_map[IPMI_BT_I2C_SEQ_MAX];
+	struct work_struct		ipmi_bt_i2c_recv_work;
+	struct work_struct		ipmi_bt_i2c_send_work;
+	struct ipmi_smi_msg		*msg_to_send;
+};
+
+static const unsigned long write_timeout = 25;
+
+static int ipmi_bt_i2c_send_request(struct ipmi_bt_i2c_master *master,
+				    struct ipmi_bt_i2c_msg *request)
+{
+	struct i2c_client *client = master->client;
+	unsigned long timeout, read_time;
+	u8 *buf = (u8 *) request;
+	int ret;
+
+	timeout = jiffies + msecs_to_jiffies(write_timeout);
+	do {
+		read_time = jiffies;
+		ret = i2c_master_send(client, buf, bt_msg_len(request));
+		if (ret >= 0)
+			return 0;
+		usleep_range(1000, 1500);
+	} while (time_before(read_time, timeout));
+	return ret;
+}
+
+static int ipmi_bt_i2c_receive_response(struct ipmi_bt_i2c_master *master,
+					struct ipmi_bt_i2c_msg *response)
+{
+	struct i2c_client *client = master->client;
+	unsigned long timeout, read_time;
+	u8 *buf = (u8 *) response;
+	u8 len = 0;
+	int ret;
+
+	/*
+	 * Slave may not NACK when not ready, so we peek at the first byte to
+	 * see if it is a valid length.
+	 */
+	ret = i2c_master_recv(client, &len, 1);
+	while (ret != 1 || len == 0) {
+		if (ret < 0)
+			return ret;
+
+		usleep_range(1000, 1500);
+
+		/* Signal received: quit syscall. */
+		if (signal_pending(current))
+			return -ERESTARTSYS;
+
+		ret = i2c_master_recv(client, &len, 1);
+	}
+
+	timeout = jiffies + msecs_to_jiffies(write_timeout);
+	do {
+		read_time = jiffies;
+		ret = i2c_master_recv(client, buf, len + 1);
+		if (ret >= 0)
+			return 0;
+		usleep_range(1000, 1500);
+	} while (time_before(read_time, timeout));
+	return ret;
+}
+
+static int ipmi_bt_i2c_start_processing(void *data, ipmi_smi_t intf)
+{
+	struct ipmi_bt_i2c_master *master = data;
+
+	master->intf = intf;
+
+	return 0;
+}
+
+static void __ipmi_bt_i2c_error_reply(struct ipmi_bt_i2c_master *master,
+				      struct ipmi_smi_msg *msg,
+				      u8 completion_code)
+{
+	struct ipmi_bt_i2c_smi_msg *response;
+	struct ipmi_bt_i2c_smi_msg *request;
+
+	response = (struct ipmi_bt_i2c_smi_msg *) msg->rsp;
+	request = (struct ipmi_bt_i2c_smi_msg *) msg->data;
+
+	response->netfn_lun = request->netfn_lun | 0x4;
+	response->cmd = request->cmd;
+	response->payload[0] = completion_code;
+	msg->rsp_size = 3;
+	ipmi_smi_msg_received(master->intf, msg);
+}
+
+static void ipmi_bt_i2c_error_reply(struct ipmi_bt_i2c_master *master,
+				    struct ipmi_smi_msg *msg,
+				    u8 completion_code)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&master->lock, flags);
+	__ipmi_bt_i2c_error_reply(master, msg, completion_code);
+	spin_unlock_irqrestore(&master->lock, flags);
+}
+
+/*
+ * ipmi_bt_i2c_smi_msg contains a payload and 2 header fields, each 1 byte:
+ * netfn_lun and cmd. They're passed to OpenIPMI within an ipmi_smi_msg struct
+ * along with their length.
+ *
+ * ipmi_bt_i2c_msg contains a payload and 4 header fields: the two above in
+ * addition to seq and len. However, len is not included in the length count so
+ * this message encapsulation is considered 1 byte longer than the other.
+ */
+static u8 ipmi_bt_i2c_smi_to_bt_len(u8 smi_msg_len)
+{
+	/* Only field that BT adds to the header is seq. */
+	return smi_msg_len + 1;
+}
+
+static u8 ipmi_bt_i2c_bt_to_smi_len(struct ipmi_bt_i2c_msg *bt_msg)
+{
+	/* Subtract one byte for seq (opposite of above) */
+	return bt_msg->len - 1;
+}
+
+static size_t ipmi_bt_i2c_payload_len(struct ipmi_bt_i2c_msg *bt_msg)
+{
+	/* Subtract one byte for each: netfn_lun, seq, cmd. */
+	return bt_msg->len - 3;
+}
+
+static bool ipmi_bt_i2c_assign_seq(struct ipmi_bt_i2c_master *master,
+				   struct ipmi_smi_msg *msg, u8 *ret_seq)
+{
+	struct ipmi_bt_i2c_seq_entry *entry;
+	bool did_cleanup = false;
+	unsigned long flags;
+	u8 seq;
+
+	spin_lock_irqsave(&master->lock, flags);
+retry:
+	for (seq = 0; seq < IPMI_BT_I2C_SEQ_MAX; seq++) {
+		if (!master->seq_msg_map[seq].msg) {
+			master->seq_msg_map[seq].msg = msg;
+			master->seq_msg_map[seq].send_time = jiffies;
+			spin_unlock_irqrestore(&master->lock, flags);
+			*ret_seq = seq;
+			return true;
+		}
+	}
+
+	if (did_cleanup) {
+		spin_unlock_irqrestore(&master->lock, flags);
+		return false;
+	}
+
+	/*
+	 * TODO: we should do cleanup at times other than only when we run out
+	 * of sequence numbers.
+	 */
+	for (seq = 0; seq < IPMI_BT_I2C_SEQ_MAX; seq++) {
+		entry = &master->seq_msg_map[seq];
+		if (entry->msg &&
+		    time_after(entry->send_time + IPMI_BT_I2C_TIMEOUT,
+			       jiffies)) {
+			__ipmi_bt_i2c_error_reply(master, entry->msg,
+						  IPMI_TIMEOUT_ERR);
+			entry->msg = NULL;
+		}
+	}
+	did_cleanup = true;
+	goto retry;
+}
+
+static struct ipmi_smi_msg *ipmi_bt_i2c_find_msg(
+		struct ipmi_bt_i2c_master *master, u8 seq)
+{
+	struct ipmi_smi_msg *msg;
+	unsigned long flags;
+
+	spin_lock_irqsave(&master->lock, flags);
+	msg = master->seq_msg_map[seq].msg;
+	spin_unlock_irqrestore(&master->lock, flags);
+	return msg;
+}
+
+static void ipmi_bt_i2c_free_seq(struct ipmi_bt_i2c_master *master, u8 seq)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&master->lock, flags);
+	master->seq_msg_map[seq].msg = NULL;
+	spin_unlock_irqrestore(&master->lock, flags);
+}
+
+static void ipmi_bt_i2c_send_workfn(struct work_struct *work)
+{
+	struct ipmi_bt_i2c_smi_msg *smi_msg;
+	struct ipmi_bt_i2c_master *master;
+	struct ipmi_bt_i2c_msg bt_msg;
+	struct ipmi_smi_msg *msg;
+	size_t smi_msg_size;
+	unsigned long flags;
+
+	master = container_of(work, struct ipmi_bt_i2c_master,
+			      ipmi_bt_i2c_send_work);
+
+	msg = master->msg_to_send;
+	smi_msg_size = msg->data_size;
+	smi_msg = (struct ipmi_bt_i2c_smi_msg *) msg->data;
+
+	if (smi_msg_size > IPMI_BT_I2C_MAX_SMI_SIZE) {
+		ipmi_bt_i2c_error_reply(master, msg, IPMI_REQ_LEN_EXCEEDED_ERR);
+		return;
+	}
+
+	if (smi_msg_size < IPMI_BT_I2C_SMI_MSG_HEADER_SIZE) {
+		ipmi_bt_i2c_error_reply(master, msg, IPMI_REQ_LEN_INVALID_ERR);
+		return;
+	}
+
+	if (!ipmi_bt_i2c_assign_seq(master, msg, &bt_msg.seq)) {
+		ipmi_bt_i2c_error_reply(master, msg, IPMI_NODE_BUSY_ERR);
+		return;
+	}
+
+	bt_msg.len = ipmi_bt_i2c_smi_to_bt_len(smi_msg_size);
+	bt_msg.netfn_lun = smi_msg->netfn_lun;
+	bt_msg.cmd = smi_msg->cmd;
+	memcpy(bt_msg.payload, smi_msg->payload,
+	       ipmi_bt_i2c_payload_len(&bt_msg));
+
+	if (ipmi_bt_i2c_send_request(master, &bt_msg) < 0) {
+		ipmi_bt_i2c_free_seq(master, bt_msg.seq);
+		ipmi_bt_i2c_error_reply(master, msg, IPMI_BUS_ERR);
+	}
+
+	spin_lock_irqsave(&master->lock, flags);
+	master->msg_to_send = NULL;
+	spin_unlock_irqrestore(&master->lock, flags);
+}
+
+void ipmi_bt_i2c_recv_workfn(struct work_struct *work)
+{
+	struct ipmi_bt_i2c_smi_msg *smi_msg;
+	struct ipmi_bt_i2c_master *master;
+	struct ipmi_bt_i2c_msg bt_msg;
+	struct ipmi_smi_msg *msg;
+
+	master = container_of(work, struct ipmi_bt_i2c_master,
+			      ipmi_bt_i2c_recv_work);
+
+	if (ipmi_bt_i2c_receive_response(master, &bt_msg) < 0)
+		return;
+
+	if (bt_msg.len < IPMI_BT_I2C_LEN_MIN)
+		return;
+
+	msg = ipmi_bt_i2c_find_msg(master, bt_msg.seq);
+	if (!msg)
+		return;
+
+	ipmi_bt_i2c_free_seq(master, bt_msg.seq);
+
+	if (bt_msg.len < IPMI_BT_I2C_RESPONSE_LEN_MIN)
+		ipmi_bt_i2c_error_reply(master, msg, IPMI_ERR_MSG_TRUNCATED);
+
+	msg->rsp_size = ipmi_bt_i2c_bt_to_smi_len(&bt_msg);
+	smi_msg = (struct ipmi_bt_i2c_smi_msg *) msg->rsp;
+	smi_msg->netfn_lun = bt_msg.netfn_lun;
+	smi_msg->cmd = bt_msg.cmd;
+	memcpy(smi_msg->payload, bt_msg.payload,
+	       ipmi_bt_i2c_payload_len(&bt_msg));
+	ipmi_smi_msg_received(master->intf, msg);
+}
+
+static void ipmi_bt_i2c_sender(void *data, struct ipmi_smi_msg *msg)
+{
+	struct ipmi_bt_i2c_master *master = data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&master->lock, flags);
+	if (master->msg_to_send) {
+		/*
+		 * TODO(benjaminfair): Queue messages to send instead of only
+		 * keeping one.
+		 */
+		__ipmi_bt_i2c_error_reply(master, msg, IPMI_NODE_BUSY_ERR);
+	} else {
+		master->msg_to_send = msg;
+		schedule_work(&master->ipmi_bt_i2c_send_work);
+	}
+	spin_unlock_irqrestore(&master->lock, flags);
+}
+
+static void ipmi_bt_i2c_request_events(void *data)
+{
+	struct ipmi_bt_i2c_master *master = data;
+
+	schedule_work(&master->ipmi_bt_i2c_recv_work);
+}
+
+static void ipmi_bt_i2c_set_run_to_completion(void *data,
+					      bool run_to_completion)
+{
+}
+
+static void ipmi_bt_i2c_poll(void *data)
+{
+	struct ipmi_bt_i2c_master *master = data;
+
+	schedule_work(&master->ipmi_bt_i2c_recv_work);
+}
+
+static struct ipmi_smi_handlers ipmi_bt_i2c_smi_handlers = {
+	.owner			= THIS_MODULE,
+	.start_processing	= ipmi_bt_i2c_start_processing,
+	.sender			= ipmi_bt_i2c_sender,
+	.request_events		= ipmi_bt_i2c_request_events,
+	.set_run_to_completion	= ipmi_bt_i2c_set_run_to_completion,
+	.poll			= ipmi_bt_i2c_poll,
+};
+
+static int ipmi_bt_i2c_probe(struct i2c_client *client,
+			     const struct i2c_device_id *id)
+{
+	struct ipmi_bt_i2c_master *master;
+	int ret;
+
+	master = devm_kzalloc(&client->dev, sizeof(struct ipmi_bt_i2c_master),
+			      GFP_KERNEL);
+	if (!master)
+		return -ENOMEM;
+
+	spin_lock_init(&master->lock);
+	INIT_WORK(&master->ipmi_bt_i2c_recv_work, ipmi_bt_i2c_recv_workfn);
+	INIT_WORK(&master->ipmi_bt_i2c_send_work, ipmi_bt_i2c_send_workfn);
+	master->client = client;
+	i2c_set_clientdata(client, master);
+
+	/*
+	 * TODO(benjaminfair): read ipmi_device_id from BMC to determine version
+	 * information and be able to tell multiple BMCs apart
+	 */
+	ret = ipmi_register_smi(&ipmi_bt_i2c_smi_handlers, master,
+				&master->ipmi_id, &client->dev, 0);
+
+	return ret;
+}
+
+static int ipmi_bt_i2c_remove(struct i2c_client *client)
+{
+	struct ipmi_bt_i2c_master *master;
+
+	master = i2c_get_clientdata(client);
+	return ipmi_unregister_smi(master->intf);
+}
+
+static const struct acpi_device_id ipmi_bt_i2c_acpi_id[] = {
+	{"BTMA0001", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, ipmi_bt_i2c_acpi_id);
+
+static const struct i2c_device_id ipmi_bt_i2c_i2c_id[] = {
+	{"ipmi-bt-i2c", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, ipmi_bt_i2c_i2c_id);
+
+static struct i2c_driver ipmi_bt_i2c_driver = {
+	.driver = {
+		.name = "ipmi-bt-i2c",
+		.acpi_match_table = ipmi_bt_i2c_acpi_id,
+	},
+	.id_table = ipmi_bt_i2c_i2c_id,
+	.probe = ipmi_bt_i2c_probe,
+	.remove = ipmi_bt_i2c_remove,
+};
+module_i2c_driver(ipmi_bt_i2c_driver);
+
+MODULE_AUTHOR("Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("IPMI Block Transfer over I2C.");
+MODULE_LICENSE("GPL v2");
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 3/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C BMC side
@ 2017-08-04 23:18   ` Brendan Higgins
  0 siblings, 0 replies; 15+ messages in thread
From: Brendan Higgins @ 2017-08-04 23:18 UTC (permalink / raw)
  To: corbet, robh+dt, mark.rutland, arnd, gregkh, minyard, joel, benh
  Cc: linux-doc, devicetree, openipmi-developer, openbmc, linux-kernel,
	Brendan Higgins

The IPMI definition of the Block Transfer protocol defines the hardware
registers and behavior in addition to the message format and messaging
semantics. This implements a new protocol that uses IPMI Block Transfer
messages and semantics on top of a standard I2C interface. This protocol
has the same BMC side file system interface as "ipmi-bt-host".

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
 drivers/char/Kconfig                    |   1 +
 drivers/char/Makefile                   |   1 +
 drivers/char/ipmi_bmc/Kconfig           |  22 ++
 drivers/char/ipmi_bmc/Makefile          |   5 +
 drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c | 346 ++++++++++++++++++++++++++++++++
 include/linux/ipmi_bmc.h                |  76 +++++++
 6 files changed, 451 insertions(+)
 create mode 100644 drivers/char/ipmi_bmc/Kconfig
 create mode 100644 drivers/char/ipmi_bmc/Makefile
 create mode 100644 drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c
 create mode 100644 include/linux/ipmi_bmc.h

diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index ccd239ab879f..2a6ca2325a45 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -195,6 +195,7 @@ config POWERNV_OP_PANEL
 	  If unsure, say M here to build it as a module called powernv-op-panel.
 
 source "drivers/char/ipmi/Kconfig"
+source "drivers/char/ipmi_bmc/Kconfig"
 
 config DS1620
 	tristate "NetWinder thermometer support"
diff --git a/drivers/char/Makefile b/drivers/char/Makefile
index 53e33720818c..9e143186fa30 100644
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -58,4 +58,5 @@ js-rtc-y = rtc.o
 
 obj-$(CONFIG_TILE_SROM)		+= tile-srom.o
 obj-$(CONFIG_XILLYBUS)		+= xillybus/
+obj-$(CONFIG_IPMI_BMC)		+= ipmi_bmc/
 obj-$(CONFIG_POWERNV_OP_PANEL)	+= powernv-op-panel.o
diff --git a/drivers/char/ipmi_bmc/Kconfig b/drivers/char/ipmi_bmc/Kconfig
new file mode 100644
index 000000000000..26c8e0cb765c
--- /dev/null
+++ b/drivers/char/ipmi_bmc/Kconfig
@@ -0,0 +1,22 @@
+#
+# IPMI BMC configuration
+#
+
+menuconfig IPMI_BMC
+	tristate 'IPMI BMC core'
+	help
+	  This enables the BMC-side IPMI drivers.
+
+	  If unsure, say N.
+
+if IPMI_BMC
+
+config IPMI_BMC_BT_I2C
+	depends on I2C
+	select I2C_SLAVE
+	tristate 'Generic I2C BT IPMI BMC driver'
+	help
+	  Provides a driver that uses IPMI Block Transfer messages and
+	  semantics on top of plain old I2C.
+
+endif # IPMI_BMC
diff --git a/drivers/char/ipmi_bmc/Makefile b/drivers/char/ipmi_bmc/Makefile
new file mode 100644
index 000000000000..dfe5128f8158
--- /dev/null
+++ b/drivers/char/ipmi_bmc/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the ipmi bmc drivers.
+#
+
+obj-$(CONFIG_IPMI_BMC_BT_I2C) += ipmi_bmc_bt_i2c.o
diff --git a/drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c b/drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c
new file mode 100644
index 000000000000..686b83fa42a4
--- /dev/null
+++ b/drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c
@@ -0,0 +1,346 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#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 i2c_client	*client;
+	struct miscdevice	miscdev;
+	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)
+{
+	int res;
+	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;
+	}
+
+	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_unlock_irqrestore(&bt_slave->lock, flags);
+	return 0;
+}
+
+static int send_bt_response(struct bt_i2c_slave *bt_slave, bool non_blocking,
+			    struct bt_msg *bt_response)
+{
+	int res;
+	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;
+	}
+
+	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;
+	}
+
+	memcpy(&bt_slave->response, bt_response, sizeof(*bt_response));
+	bt_slave->response_in_progress = true;
+	spin_unlock_irqrestore(&bt_slave->lock, flags);
+	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);
+	return 0;
+}
+
+static int bt_i2c_slave_cb(struct i2c_client *client,
+			   enum i2c_slave_event event, u8 *val)
+{
+	struct bt_i2c_slave *bt_slave = i2c_get_clientdata(client);
+	u8 *buf;
+
+	spin_lock(&bt_slave->lock);
+	switch (event) {
+	case I2C_SLAVE_WRITE_REQUESTED:
+		bt_slave->msg_idx = 0;
+		break;
+
+	case I2C_SLAVE_WRITE_RECEIVED:
+		buf = (u8 *) &bt_slave->request;
+		if (bt_slave->msg_idx >= sizeof(struct bt_msg))
+			break;
+
+		buf[bt_slave->msg_idx++] = *val;
+		if (bt_slave->msg_idx >= bt_msg_len(&bt_slave->request))
+			handle_request(bt_slave);
+		break;
+
+	case I2C_SLAVE_READ_REQUESTED:
+		buf = (u8 *) &bt_slave->response;
+		bt_slave->msg_idx = 0;
+		*val = buf[bt_slave->msg_idx];
+		/*
+		 * Do not increment buffer_idx here, because we don't know if
+		 * this byte will be actually used. Read Linux I2C slave docs
+		 * for details.
+		 */
+		break;
+
+	case I2C_SLAVE_READ_PROCESSED:
+		buf = (u8 *) &bt_slave->response;
+		if (bt_slave->response.len &&
+		    bt_slave->msg_idx < bt_msg_len(&bt_slave->response)) {
+			*val = buf[++bt_slave->msg_idx];
+		} else {
+			*val = 0;
+		}
+		if (bt_slave->msg_idx + 1 >= bt_msg_len(&bt_slave->response))
+			complete_response(bt_slave);
+		break;
+
+	case I2C_SLAVE_STOP:
+		bt_slave->msg_idx = 0;
+		break;
+
+	default:
+		break;
+	}
+	spin_unlock(&bt_slave->lock);
+
+	return 0;
+}
+
+static int bt_i2c_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct bt_i2c_slave *bt_slave;
+	int ret;
+
+	bt_slave = devm_kzalloc(&client->dev, sizeof(*bt_slave),
+				GFP_KERNEL);
+	if (!bt_slave)
+		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);
+
+	mutex_init(&bt_slave->file_mutex);
+
+	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)
+		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);
+		return ret;
+	}
+
+	return 0;
+}
+
+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;
+}
+
+static const struct i2c_device_id bt_i2c_id[] = {
+	{"ipmi-bmc-bt-i2c", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, bt_i2c_id);
+
+static struct i2c_driver bt_i2c_driver = {
+	.driver = {
+		.name		= "ipmi-bmc-bt-i2c",
+	},
+	.probe		= bt_i2c_probe,
+	.remove		= bt_i2c_remove,
+	.id_table	= bt_i2c_id,
+};
+module_i2c_driver(bt_i2c_driver);
+
+MODULE_AUTHOR("Brendan Higgins <brendanhiggins@google.com>");
+MODULE_DESCRIPTION("BMC-side IPMI Block Transfer over I2C.");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/ipmi_bmc.h b/include/linux/ipmi_bmc.h
new file mode 100644
index 000000000000..d0885c0bf190
--- /dev/null
+++ b/include/linux/ipmi_bmc.h
@@ -0,0 +1,76 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __LINUX_IPMI_BMC_H
+#define __LINUX_IPMI_BMC_H
+
+#include <linux/bug.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+
+#define BT_MSG_PAYLOAD_LEN_MAX 252
+
+/**
+ * struct bt_msg - Block Transfer IPMI message.
+ * @len: Length of the message, not including this field.
+ * @netfn_lun: 6-bit netfn field definining the category of message and 2-bit
+ *             lun field used for routing.
+ * @seq: Sequence number used to associate requests with responses.
+ * @cmd: Command within a netfn category.
+ * @payload: Variable length field. May have specific requirements based on
+ *           netfn/cmd pair.
+ *
+ * Use bt_msg_len() to determine the total length of a message (including
+ * the @len field) rather than reading it directly.
+ */
+struct bt_msg {
+	u8 len;
+	u8 netfn_lun;
+	u8 seq;
+	u8 cmd;
+	u8 payload[BT_MSG_PAYLOAD_LEN_MAX];
+} __packed;
+
+/**
+ * bt_msg_len() - Determine the total length of a Block Transfer message.
+ * @bt_msg: Pointer to the message.
+ *
+ * This function calculates the length of an IPMI Block Transfer message
+ * including the length field itself.
+ *
+ * Return: Length of @bt_msg.
+ */
+static inline u32 bt_msg_len(struct bt_msg *bt_msg)
+{
+	return bt_msg->len + 1;
+}
+
+/**
+ * bt_msg_payload_to_len() - Calculate the len field of a Block Transfer message
+ *                           given the length of the payload.
+ * @payload_len: Length of the payload.
+ *
+ * Return: len field of the Block Transfer message which contains this payload.
+ */
+static inline u8 bt_msg_payload_to_len(u8 payload_len)
+{
+	if (unlikely(payload_len > BT_MSG_PAYLOAD_LEN_MAX)) {
+		payload_len = BT_MSG_PAYLOAD_LEN_MAX;
+		WARN(1, "BT message payload is too large. Truncating to %u.\n",
+		     BT_MSG_PAYLOAD_LEN_MAX);
+	}
+	return payload_len + 3;
+}
+
+#endif /* __LINUX_IPMI_BMC_H */
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 3/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C BMC side
@ 2017-08-04 23:18   ` Brendan Higgins
  0 siblings, 0 replies; 15+ messages in thread
From: Brendan Higgins @ 2017-08-04 23:18 UTC (permalink / raw)
  To: corbet-T1hC0tSOHrs, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, arnd-r2nGTMty4D4,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, minyard-HInyCGIudOg,
	joel-U3u1mxZcP9KHXe+LvDLADg,
	benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r
  Cc: linux-doc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	openipmi-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Brendan Higgins

The IPMI definition of the Block Transfer protocol defines the hardware
registers and behavior in addition to the message format and messaging
semantics. This implements a new protocol that uses IPMI Block Transfer
messages and semantics on top of a standard I2C interface. This protocol
has the same BMC side file system interface as "ipmi-bt-host".

Signed-off-by: Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 drivers/char/Kconfig                    |   1 +
 drivers/char/Makefile                   |   1 +
 drivers/char/ipmi_bmc/Kconfig           |  22 ++
 drivers/char/ipmi_bmc/Makefile          |   5 +
 drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c | 346 ++++++++++++++++++++++++++++++++
 include/linux/ipmi_bmc.h                |  76 +++++++
 6 files changed, 451 insertions(+)
 create mode 100644 drivers/char/ipmi_bmc/Kconfig
 create mode 100644 drivers/char/ipmi_bmc/Makefile
 create mode 100644 drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c
 create mode 100644 include/linux/ipmi_bmc.h

diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index ccd239ab879f..2a6ca2325a45 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -195,6 +195,7 @@ config POWERNV_OP_PANEL
 	  If unsure, say M here to build it as a module called powernv-op-panel.
 
 source "drivers/char/ipmi/Kconfig"
+source "drivers/char/ipmi_bmc/Kconfig"
 
 config DS1620
 	tristate "NetWinder thermometer support"
diff --git a/drivers/char/Makefile b/drivers/char/Makefile
index 53e33720818c..9e143186fa30 100644
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -58,4 +58,5 @@ js-rtc-y = rtc.o
 
 obj-$(CONFIG_TILE_SROM)		+= tile-srom.o
 obj-$(CONFIG_XILLYBUS)		+= xillybus/
+obj-$(CONFIG_IPMI_BMC)		+= ipmi_bmc/
 obj-$(CONFIG_POWERNV_OP_PANEL)	+= powernv-op-panel.o
diff --git a/drivers/char/ipmi_bmc/Kconfig b/drivers/char/ipmi_bmc/Kconfig
new file mode 100644
index 000000000000..26c8e0cb765c
--- /dev/null
+++ b/drivers/char/ipmi_bmc/Kconfig
@@ -0,0 +1,22 @@
+#
+# IPMI BMC configuration
+#
+
+menuconfig IPMI_BMC
+	tristate 'IPMI BMC core'
+	help
+	  This enables the BMC-side IPMI drivers.
+
+	  If unsure, say N.
+
+if IPMI_BMC
+
+config IPMI_BMC_BT_I2C
+	depends on I2C
+	select I2C_SLAVE
+	tristate 'Generic I2C BT IPMI BMC driver'
+	help
+	  Provides a driver that uses IPMI Block Transfer messages and
+	  semantics on top of plain old I2C.
+
+endif # IPMI_BMC
diff --git a/drivers/char/ipmi_bmc/Makefile b/drivers/char/ipmi_bmc/Makefile
new file mode 100644
index 000000000000..dfe5128f8158
--- /dev/null
+++ b/drivers/char/ipmi_bmc/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the ipmi bmc drivers.
+#
+
+obj-$(CONFIG_IPMI_BMC_BT_I2C) += ipmi_bmc_bt_i2c.o
diff --git a/drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c b/drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c
new file mode 100644
index 000000000000..686b83fa42a4
--- /dev/null
+++ b/drivers/char/ipmi_bmc/ipmi_bmc_bt_i2c.c
@@ -0,0 +1,346 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#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 i2c_client	*client;
+	struct miscdevice	miscdev;
+	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)
+{
+	int res;
+	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;
+	}
+
+	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_unlock_irqrestore(&bt_slave->lock, flags);
+	return 0;
+}
+
+static int send_bt_response(struct bt_i2c_slave *bt_slave, bool non_blocking,
+			    struct bt_msg *bt_response)
+{
+	int res;
+	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;
+	}
+
+	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;
+	}
+
+	memcpy(&bt_slave->response, bt_response, sizeof(*bt_response));
+	bt_slave->response_in_progress = true;
+	spin_unlock_irqrestore(&bt_slave->lock, flags);
+	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);
+	return 0;
+}
+
+static int bt_i2c_slave_cb(struct i2c_client *client,
+			   enum i2c_slave_event event, u8 *val)
+{
+	struct bt_i2c_slave *bt_slave = i2c_get_clientdata(client);
+	u8 *buf;
+
+	spin_lock(&bt_slave->lock);
+	switch (event) {
+	case I2C_SLAVE_WRITE_REQUESTED:
+		bt_slave->msg_idx = 0;
+		break;
+
+	case I2C_SLAVE_WRITE_RECEIVED:
+		buf = (u8 *) &bt_slave->request;
+		if (bt_slave->msg_idx >= sizeof(struct bt_msg))
+			break;
+
+		buf[bt_slave->msg_idx++] = *val;
+		if (bt_slave->msg_idx >= bt_msg_len(&bt_slave->request))
+			handle_request(bt_slave);
+		break;
+
+	case I2C_SLAVE_READ_REQUESTED:
+		buf = (u8 *) &bt_slave->response;
+		bt_slave->msg_idx = 0;
+		*val = buf[bt_slave->msg_idx];
+		/*
+		 * Do not increment buffer_idx here, because we don't know if
+		 * this byte will be actually used. Read Linux I2C slave docs
+		 * for details.
+		 */
+		break;
+
+	case I2C_SLAVE_READ_PROCESSED:
+		buf = (u8 *) &bt_slave->response;
+		if (bt_slave->response.len &&
+		    bt_slave->msg_idx < bt_msg_len(&bt_slave->response)) {
+			*val = buf[++bt_slave->msg_idx];
+		} else {
+			*val = 0;
+		}
+		if (bt_slave->msg_idx + 1 >= bt_msg_len(&bt_slave->response))
+			complete_response(bt_slave);
+		break;
+
+	case I2C_SLAVE_STOP:
+		bt_slave->msg_idx = 0;
+		break;
+
+	default:
+		break;
+	}
+	spin_unlock(&bt_slave->lock);
+
+	return 0;
+}
+
+static int bt_i2c_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct bt_i2c_slave *bt_slave;
+	int ret;
+
+	bt_slave = devm_kzalloc(&client->dev, sizeof(*bt_slave),
+				GFP_KERNEL);
+	if (!bt_slave)
+		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);
+
+	mutex_init(&bt_slave->file_mutex);
+
+	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)
+		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);
+		return ret;
+	}
+
+	return 0;
+}
+
+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;
+}
+
+static const struct i2c_device_id bt_i2c_id[] = {
+	{"ipmi-bmc-bt-i2c", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, bt_i2c_id);
+
+static struct i2c_driver bt_i2c_driver = {
+	.driver = {
+		.name		= "ipmi-bmc-bt-i2c",
+	},
+	.probe		= bt_i2c_probe,
+	.remove		= bt_i2c_remove,
+	.id_table	= bt_i2c_id,
+};
+module_i2c_driver(bt_i2c_driver);
+
+MODULE_AUTHOR("Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("BMC-side IPMI Block Transfer over I2C.");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/ipmi_bmc.h b/include/linux/ipmi_bmc.h
new file mode 100644
index 000000000000..d0885c0bf190
--- /dev/null
+++ b/include/linux/ipmi_bmc.h
@@ -0,0 +1,76 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __LINUX_IPMI_BMC_H
+#define __LINUX_IPMI_BMC_H
+
+#include <linux/bug.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+
+#define BT_MSG_PAYLOAD_LEN_MAX 252
+
+/**
+ * struct bt_msg - Block Transfer IPMI message.
+ * @len: Length of the message, not including this field.
+ * @netfn_lun: 6-bit netfn field definining the category of message and 2-bit
+ *             lun field used for routing.
+ * @seq: Sequence number used to associate requests with responses.
+ * @cmd: Command within a netfn category.
+ * @payload: Variable length field. May have specific requirements based on
+ *           netfn/cmd pair.
+ *
+ * Use bt_msg_len() to determine the total length of a message (including
+ * the @len field) rather than reading it directly.
+ */
+struct bt_msg {
+	u8 len;
+	u8 netfn_lun;
+	u8 seq;
+	u8 cmd;
+	u8 payload[BT_MSG_PAYLOAD_LEN_MAX];
+} __packed;
+
+/**
+ * bt_msg_len() - Determine the total length of a Block Transfer message.
+ * @bt_msg: Pointer to the message.
+ *
+ * This function calculates the length of an IPMI Block Transfer message
+ * including the length field itself.
+ *
+ * Return: Length of @bt_msg.
+ */
+static inline u32 bt_msg_len(struct bt_msg *bt_msg)
+{
+	return bt_msg->len + 1;
+}
+
+/**
+ * bt_msg_payload_to_len() - Calculate the len field of a Block Transfer message
+ *                           given the length of the payload.
+ * @payload_len: Length of the payload.
+ *
+ * Return: len field of the Block Transfer message which contains this payload.
+ */
+static inline u8 bt_msg_payload_to_len(u8 payload_len)
+{
+	if (unlikely(payload_len > BT_MSG_PAYLOAD_LEN_MAX)) {
+		payload_len = BT_MSG_PAYLOAD_LEN_MAX;
+		WARN(1, "BT message payload is too large. Truncating to %u.\n",
+		     BT_MSG_PAYLOAD_LEN_MAX);
+	}
+	return payload_len + 3;
+}
+
+#endif /* __LINUX_IPMI_BMC_H */
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C
  2017-08-04 23:18 [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C Brendan Higgins
@ 2017-08-04 23:25   ` Brendan Higgins
  2017-08-04 23:18   ` Brendan Higgins
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Brendan Higgins @ 2017-08-04 23:25 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Arnd Bergmann,
	Greg KH, Corey Minyard, Joel Stanley, Benjamin Herrenschmidt
  Cc: linux-doc, devicetree, openipmi-developer, OpenBMC Maillist,
	Linux Kernel Mailing List

On Fri, Aug 4, 2017 at 4:18 PM, Brendan Higgins
<brendanhiggins@google.com> wrote:
> This patchset introduces IPMI Block Transfer over I2C (BT-I2C), which has the
> same semantics as IPMI Block Transfer except it done over I2C.
>
> The documentation discusses the reason for this in greater detail, suffice it to
> say SSIF cannot be correctly implemented on some naive I2C devices. There are
> some additional reasons why we don't like SSIF, but those are again covered in
> the documentation for all those who are interested.
>
> In addition, since I am adding both host side and BMC side support, I figured
> that now is a good time to resolve the problem of where to put BMC side IPMI
> drivers; right now we have it (there is only one) in drivers/char/ipmi/ with the
> rest of the host side IPMI drivers, but I think it makes sense to put all of the
> host side IPMI drivers in one directory and all of the BMC side drivers in
> another, preferably in a way that does not effect all of the current OpenIPMI
> users. I have not created a MAINTAINERS entry for the new directory yet, as I
> figured there might be some discussion to be had about it.
>
> I have tested this patchset on the Aspeed 2500 EVB.

I forgot to mention, for the OpenBMC people, this is based on an RFC:
https://lists.ozlabs.org/pipermail/openbmc/2016-September/004505.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C
@ 2017-08-04 23:25   ` Brendan Higgins
  0 siblings, 0 replies; 15+ messages in thread
From: Brendan Higgins @ 2017-08-04 23:25 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Arnd Bergmann,
	Greg KH, Corey Minyard, Joel Stanley, Benjamin Herrenschmidt
  Cc: linux-doc, devicetree, openipmi-developer, OpenBMC Maillist,
	Linux Kernel Mailing List

On Fri, Aug 4, 2017 at 4:18 PM, Brendan Higgins
<brendanhiggins@google.com> wrote:
> This patchset introduces IPMI Block Transfer over I2C (BT-I2C), which has the
> same semantics as IPMI Block Transfer except it done over I2C.
>
> The documentation discusses the reason for this in greater detail, suffice it to
> say SSIF cannot be correctly implemented on some naive I2C devices. There are
> some additional reasons why we don't like SSIF, but those are again covered in
> the documentation for all those who are interested.
>
> In addition, since I am adding both host side and BMC side support, I figured
> that now is a good time to resolve the problem of where to put BMC side IPMI
> drivers; right now we have it (there is only one) in drivers/char/ipmi/ with the
> rest of the host side IPMI drivers, but I think it makes sense to put all of the
> host side IPMI drivers in one directory and all of the BMC side drivers in
> another, preferably in a way that does not effect all of the current OpenIPMI
> users. I have not created a MAINTAINERS entry for the new directory yet, as I
> figured there might be some discussion to be had about it.
>
> I have tested this patchset on the Aspeed 2500 EVB.

I forgot to mention, for the OpenBMC people, this is based on an RFC:
https://lists.ozlabs.org/pipermail/openbmc/2016-September/004505.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 2/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C host side
  2017-08-04 23:18   ` Brendan Higgins
@ 2017-08-05 21:37     ` kbuild test robot
  -1 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2017-08-05 21:37 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: kbuild-all, corbet, robh+dt, mark.rutland, arnd, gregkh, minyard,
	joel, benh, linux-doc, devicetree, openipmi-developer, openbmc,
	linux-kernel, Brendan Higgins

[-- Attachment #1: Type: text/plain, Size: 2411 bytes --]

Hi Brendan,

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v4.13-rc3 next-20170804]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Brendan-Higgins/ipmi-bt-i2c-added-IPMI-Block-Transfer-over-I2C/20170806-033950
config: ia64-allyesconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

All errors (new ones prefixed by >>):

   drivers/char/ipmi/ipmi_bt_i2c.c: In function 'ipmi_bt_i2c_receive_response':
>> drivers/char/ipmi/ipmi_bt_i2c.c:119:7: error: implicit declaration of function 'signal_pending' [-Werror=implicit-function-declaration]
      if (signal_pending(current))
          ^~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/signal_pending +119 drivers/char/ipmi/ipmi_bt_i2c.c

    97	
    98	static int ipmi_bt_i2c_receive_response(struct ipmi_bt_i2c_master *master,
    99						struct ipmi_bt_i2c_msg *response)
   100	{
   101		struct i2c_client *client = master->client;
   102		unsigned long timeout, read_time;
   103		u8 *buf = (u8 *) response;
   104		u8 len = 0;
   105		int ret;
   106	
   107		/*
   108		 * Slave may not NACK when not ready, so we peek at the first byte to
   109		 * see if it is a valid length.
   110		 */
   111		ret = i2c_master_recv(client, &len, 1);
   112		while (ret != 1 || len == 0) {
   113			if (ret < 0)
   114				return ret;
   115	
   116			usleep_range(1000, 1500);
   117	
   118			/* Signal received: quit syscall. */
 > 119			if (signal_pending(current))
   120				return -ERESTARTSYS;
   121	
   122			ret = i2c_master_recv(client, &len, 1);
   123		}
   124	
   125		timeout = jiffies + msecs_to_jiffies(write_timeout);
   126		do {
   127			read_time = jiffies;
   128			ret = i2c_master_recv(client, buf, len + 1);
   129			if (ret >= 0)
   130				return 0;
   131			usleep_range(1000, 1500);
   132		} while (time_before(read_time, timeout));
   133		return ret;
   134	}
   135	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51472 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 2/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C host side
@ 2017-08-05 21:37     ` kbuild test robot
  0 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2017-08-05 21:37 UTC (permalink / raw)
  Cc: kbuild-all, corbet, robh+dt, mark.rutland, arnd, gregkh, minyard,
	joel, benh, linux-doc, devicetree, openipmi-developer, openbmc,
	linux-kernel, Brendan Higgins

[-- Attachment #1: Type: text/plain, Size: 2411 bytes --]

Hi Brendan,

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v4.13-rc3 next-20170804]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Brendan-Higgins/ipmi-bt-i2c-added-IPMI-Block-Transfer-over-I2C/20170806-033950
config: ia64-allyesconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

All errors (new ones prefixed by >>):

   drivers/char/ipmi/ipmi_bt_i2c.c: In function 'ipmi_bt_i2c_receive_response':
>> drivers/char/ipmi/ipmi_bt_i2c.c:119:7: error: implicit declaration of function 'signal_pending' [-Werror=implicit-function-declaration]
      if (signal_pending(current))
          ^~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/signal_pending +119 drivers/char/ipmi/ipmi_bt_i2c.c

    97	
    98	static int ipmi_bt_i2c_receive_response(struct ipmi_bt_i2c_master *master,
    99						struct ipmi_bt_i2c_msg *response)
   100	{
   101		struct i2c_client *client = master->client;
   102		unsigned long timeout, read_time;
   103		u8 *buf = (u8 *) response;
   104		u8 len = 0;
   105		int ret;
   106	
   107		/*
   108		 * Slave may not NACK when not ready, so we peek at the first byte to
   109		 * see if it is a valid length.
   110		 */
   111		ret = i2c_master_recv(client, &len, 1);
   112		while (ret != 1 || len == 0) {
   113			if (ret < 0)
   114				return ret;
   115	
   116			usleep_range(1000, 1500);
   117	
   118			/* Signal received: quit syscall. */
 > 119			if (signal_pending(current))
   120				return -ERESTARTSYS;
   121	
   122			ret = i2c_master_recv(client, &len, 1);
   123		}
   124	
   125		timeout = jiffies + msecs_to_jiffies(write_timeout);
   126		do {
   127			read_time = jiffies;
   128			ret = i2c_master_recv(client, buf, len + 1);
   129			if (ret >= 0)
   130				return 0;
   131			usleep_range(1000, 1500);
   132		} while (time_before(read_time, timeout));
   133		return ret;
   134	}
   135	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51472 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C
  2017-08-04 23:18 [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C Brendan Higgins
@ 2017-08-09 10:56   ` Anton D. Kachalov
  2017-08-04 23:18   ` Brendan Higgins
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Anton D. Kachalov @ 2017-08-09 10:56 UTC (permalink / raw)
  To: Brendan Higgins, corbet, robh+dt, mark.rutland, arnd, gregkh,
	minyard, joel, benh
  Cc: devicetree, openipmi-developer, openbmc, linux-kernel, linux-doc

[-- Attachment #1: Type: text/html, Size: 2387 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C
@ 2017-08-09 10:56   ` Anton D. Kachalov
  0 siblings, 0 replies; 15+ messages in thread
From: Anton D. Kachalov @ 2017-08-09 10:56 UTC (permalink / raw)
  To: Brendan Higgins, corbet, robh+dt, mark.rutland, arnd, gregkh,
	minyard, joel, benh
  Cc: devicetree, openipmi-developer, openbmc, linux-kernel, linux-doc

[-- Attachment #1: Type: text/html, Size: 2387 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C
@ 2017-08-10  1:14     ` Brendan Higgins
  0 siblings, 0 replies; 15+ messages in thread
From: Brendan Higgins @ 2017-08-10  1:14 UTC (permalink / raw)
  To: Anton D. Kachalov
  Cc: corbet, robh+dt, mark.rutland, arnd, gregkh, minyard, joel, benh,
	devicetree, openipmi-developer, openbmc, linux-kernel, linux-doc

On Wed, Aug 9, 2017 at 3:56 AM, Anton D. Kachalov <mouse@yandex-team.ru> wrote:
> Hello,
>
> I would like to mention one of the our related work for IPMI and I2C.
>
> We use OpenIPMI stack to connect to the computing nodes through the I2C
> using IPMB (BT is not supported by nodes):
>
> https://github.com/ya-mouse/meta-openbmc-yandex/blob/master/meta-yandex/meta-openrack/meta-shaosi/recipes-kernel/linux/linux-obmc/ipmi_i2c.c
>
> It lacks complete slave support (slave part is only for receiving known
> packets with query results due to OpenIPMI implementation in kernel) and use
> one local slave to communicate with a number of target systems on the same
> bus (currently supported only 1-to-1 schema).
>
> With this stuff we able to use ipmitool across different /dev/ipmiX devices
> to communicate with nodes.

Cool, I met someone else who had a similar use case which is part of why
I decided to share this (not sure if should say who).

So it sounds like we are probably not going to go with the approach I proposed;
if you indeed find this useful, I would suggest that we put this in our OpenBMC
repository and switch it out with the suggested method at some point.

Let me know what you think

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C
@ 2017-08-10  1:14     ` Brendan Higgins
  0 siblings, 0 replies; 15+ messages in thread
From: Brendan Higgins @ 2017-08-10  1:14 UTC (permalink / raw)
  To: Anton D. Kachalov
  Cc: corbet-T1hC0tSOHrs, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, arnd-r2nGTMty4D4,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, minyard-HInyCGIudOg,
	joel-U3u1mxZcP9KHXe+LvDLADg,
	benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	openipmi-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA

On Wed, Aug 9, 2017 at 3:56 AM, Anton D. Kachalov <mouse-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org> wrote:
> Hello,
>
> I would like to mention one of the our related work for IPMI and I2C.
>
> We use OpenIPMI stack to connect to the computing nodes through the I2C
> using IPMB (BT is not supported by nodes):
>
> https://github.com/ya-mouse/meta-openbmc-yandex/blob/master/meta-yandex/meta-openrack/meta-shaosi/recipes-kernel/linux/linux-obmc/ipmi_i2c.c
>
> It lacks complete slave support (slave part is only for receiving known
> packets with query results due to OpenIPMI implementation in kernel) and use
> one local slave to communicate with a number of target systems on the same
> bus (currently supported only 1-to-1 schema).
>
> With this stuff we able to use ipmitool across different /dev/ipmiX devices
> to communicate with nodes.

Cool, I met someone else who had a similar use case which is part of why
I decided to share this (not sure if should say who).

So it sounds like we are probably not going to go with the approach I proposed;
if you indeed find this useful, I would suggest that we put this in our OpenBMC
repository and switch it out with the suggested method at some point.

Let me know what you think
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C
@ 2017-08-10  1:14     ` Brendan Higgins
  0 siblings, 0 replies; 15+ messages in thread
From: Brendan Higgins @ 2017-08-10  1:14 UTC (permalink / raw)
  To: Anton D. Kachalov
  Cc: corbet, robh+dt, mark.rutland, arnd, gregkh, minyard, joel, benh,
	devicetree, openipmi-developer, openbmc, linux-kernel, linux-doc

On Wed, Aug 9, 2017 at 3:56 AM, Anton D. Kachalov <mouse@yandex-team.ru> wrote:
> Hello,
>
> I would like to mention one of the our related work for IPMI and I2C.
>
> We use OpenIPMI stack to connect to the computing nodes through the I2C
> using IPMB (BT is not supported by nodes):
>
> https://github.com/ya-mouse/meta-openbmc-yandex/blob/master/meta-yandex/meta-openrack/meta-shaosi/recipes-kernel/linux/linux-obmc/ipmi_i2c.c
>
> It lacks complete slave support (slave part is only for receiving known
> packets with query results due to OpenIPMI implementation in kernel) and use
> one local slave to communicate with a number of target systems on the same
> bus (currently supported only 1-to-1 schema).
>
> With this stuff we able to use ipmitool across different /dev/ipmiX devices
> to communicate with nodes.

Cool, I met someone else who had a similar use case which is part of why
I decided to share this (not sure if should say who).

So it sounds like we are probably not going to go with the approach I proposed;
if you indeed find this useful, I would suggest that we put this in our OpenBMC
repository and switch it out with the suggested method at some point.

Let me know what you think

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-08-10  1:14 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-04 23:18 [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C Brendan Higgins
2017-08-04 23:18 ` [PATCH v1 1/3] ipmi: bt-i2c: added documentation for bt-i2c drivers Brendan Higgins
2017-08-04 23:18 ` [PATCH v1 2/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C host side Brendan Higgins
2017-08-04 23:18   ` Brendan Higgins
2017-08-05 21:37   ` kbuild test robot
2017-08-05 21:37     ` kbuild test robot
2017-08-04 23:18 ` [PATCH v1 3/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C BMC side Brendan Higgins
2017-08-04 23:18   ` Brendan Higgins
2017-08-04 23:25 ` [PATCH v1 0/3] ipmi: bt-i2c: added IPMI Block Transfer over I2C Brendan Higgins
2017-08-04 23:25   ` Brendan Higgins
2017-08-09 10:56 ` Anton D. Kachalov
2017-08-09 10:56   ` Anton D. Kachalov
2017-08-10  1:14   ` Brendan Higgins
2017-08-10  1:14     ` Brendan Higgins
2017-08-10  1:14     ` Brendan Higgins

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.