All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section
@ 2018-05-24  5:14 Benjamin Herrenschmidt
  2018-05-24  5:14 ` [PATCH linux dev-4.13 02/10] fsi/gpio: Use relative-addressing commands Benjamin Herrenschmidt
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-24  5:14 UTC (permalink / raw)
  To: openbmc

From: Jeremy Kerr <jk@ozlabs.org>

For implementing relative addressing mode, we'll need to build a command
that is coherent with CFAM state. To do that, include the
build_command_* functions in the locked section of read/write/term.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/fsi/fsi-master-gpio.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index e1bde9e3f855..d50ea4849377 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -463,14 +463,10 @@ static int fsi_master_gpio_xfer(struct fsi_master_gpio *master, uint8_t slave,
 {
 	int rc;
 
-	mutex_lock(&master->cmd_lock);
-
 	rc = send_request(master, cmd);
 	if (!rc)
 		rc = poll_for_response(master, slave, resp_len, resp);
 
-	mutex_unlock(&master->cmd_lock);
-
 	return rc;
 }
 
@@ -479,12 +475,17 @@ static int fsi_master_gpio_read(struct fsi_master *_master, int link,
 {
 	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
 	struct fsi_gpio_msg cmd;
+	int rc;
 
 	if (link != 0)
 		return -ENODEV;
 
+	mutex_lock(&master->cmd_lock);
 	build_abs_ar_command(&cmd, id, addr, size, NULL);
-	return fsi_master_gpio_xfer(master, id, &cmd, size, val);
+	rc = fsi_master_gpio_xfer(master, id, &cmd, size, val);
+	mutex_unlock(&master->cmd_lock);
+
+	return rc;
 }
 
 static int fsi_master_gpio_write(struct fsi_master *_master, int link,
@@ -492,12 +493,17 @@ static int fsi_master_gpio_write(struct fsi_master *_master, int link,
 {
 	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
 	struct fsi_gpio_msg cmd;
+	int rc;
 
 	if (link != 0)
 		return -ENODEV;
 
+	mutex_lock(&master->cmd_lock);
 	build_abs_ar_command(&cmd, id, addr, size, val);
-	return fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
+	rc = fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
+	mutex_unlock(&master->cmd_lock);
+
+	return rc;
 }
 
 static int fsi_master_gpio_term(struct fsi_master *_master,
@@ -505,12 +511,17 @@ static int fsi_master_gpio_term(struct fsi_master *_master,
 {
 	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
 	struct fsi_gpio_msg cmd;
+	int rc;
 
 	if (link != 0)
 		return -ENODEV;
 
+	mutex_lock(&master->cmd_lock);
 	build_term_command(&cmd, id);
-	return fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
+	rc = fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
+	mutex_unlock(&master->cmd_lock);
+
+	return rc;
 }
 
 static int fsi_master_gpio_break(struct fsi_master *_master, int link)
-- 
2.17.0

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

* [PATCH linux dev-4.13 02/10] fsi/gpio: Use relative-addressing commands
  2018-05-24  5:14 [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Benjamin Herrenschmidt
@ 2018-05-24  5:14 ` Benjamin Herrenschmidt
  2018-05-24 14:34   ` Christopher Bostic
  2018-05-24  5:14 ` [PATCH linux dev-4.13 03/10] fsi/fsi-master-gpio: Implement CRC error recovery Benjamin Herrenschmidt
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-24  5:14 UTC (permalink / raw)
  To: openbmc

From: Jeremy Kerr <jk@ozlabs.org>

FSI CFAMs support shorter commands that use a relative (or same) address
as the last. This change introduces a last_addr to the master state, and
uses it for subsequent reads/writes, and performs relative addressing
when a subsequent read/write is in range.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/fsi/fsi-master-gpio.c | 102 ++++++++++++++++++++++++++++++----
 1 file changed, 91 insertions(+), 11 deletions(-)

diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index d50ea4849377..0a6799bda294 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -28,6 +28,8 @@
 #define	FSI_GPIO_CMD_DPOLL      0x2
 #define	FSI_GPIO_CMD_TERM	0x3f
 #define FSI_GPIO_CMD_ABS_AR	0x4
+#define FSI_GPIO_CMD_REL_AR	0x5
+#define FSI_GPIO_CMD_SAME_AR	0x3	/* but only a 2-bit opcode... */
 
 
 #define	FSI_GPIO_DPOLL_CLOCKS	50      /* < 21 will cause slave to hang */
@@ -52,6 +54,8 @@
 #define	FSI_GPIO_MSG_RESPID_SIZE	2
 #define	FSI_GPIO_PRIME_SLAVE_CLOCKS	20
 
+#define LAST_ADDR_INVALID		0x1
+
 struct fsi_master_gpio {
 	struct fsi_master	master;
 	struct device		*dev;
@@ -64,6 +68,7 @@ struct fsi_master_gpio {
 	struct gpio_desc	*gpio_mux;	/* Mux control */
 	bool			external_mode;
 	bool			no_delays;
+	uint32_t		last_addr;
 };
 
 #define CREATE_TRACE_POINTS
@@ -205,22 +210,89 @@ static void msg_push_crc(struct fsi_gpio_msg *msg)
 	msg_push_bits(msg, crc, 4);
 }
 
+static bool check_same_address(struct fsi_master_gpio *master, int id,
+		uint32_t addr)
+{
+	/* this will also handle LAST_ADDR_INVALID */
+	return master->last_addr == (((id & 0x3) << 21) | (addr & ~0x3));
+}
+
+static bool check_relative_address(struct fsi_master_gpio *master, int id,
+		uint32_t addr, uint32_t *rel_addrp)
+{
+	uint32_t last_addr = master->last_addr;
+	int32_t rel_addr;
+
+	if (last_addr == LAST_ADDR_INVALID)
+		return false;
+
+	/* We may be in 23-bit addressing mode, which uses the id as the
+	 * top two address bits. So, if we're referencing a different ID,
+	 * use absolute addresses.
+	 */
+	if (((last_addr >> 21) & 0x3) != id)
+		return false;
+
+	/* remove the top two bits from any 23-bit addressing */
+	last_addr &= (1 << 21) - 1;
+
+	/* We know that the addresses are limited to 21 bits, so this won't
+	 * overflow the signed rel_addr */
+	rel_addr = addr - last_addr;
+	if (rel_addr > 255 || rel_addr < -256)
+		return false;
+
+	*rel_addrp = (uint32_t)rel_addr;
+
+	return true;
+}
+
+static void last_address_update(struct fsi_master_gpio *master,
+		int id, bool valid, uint32_t addr)
+{
+	if (!valid)
+		master->last_addr = LAST_ADDR_INVALID;
+	else
+		master->last_addr = ((id & 0x3) << 21) | (addr & ~0x3);
+}
+
 /*
- * Encode an Absolute Address command
+ * Encode an Absolute/Relative/Same Address command
  */
-static void build_abs_ar_command(struct fsi_gpio_msg *cmd,
-		uint8_t id, uint32_t addr, size_t size, const void *data)
+static void build_ar_command(struct fsi_master_gpio *master,
+		struct fsi_gpio_msg *cmd, uint8_t id,
+		uint32_t addr, size_t size, const void *data)
 {
+	int i, addr_bits, opcode_bits;
 	bool write = !!data;
-	uint8_t ds;
-	int i;
+	uint8_t ds, opcode;
+	uint32_t rel_addr;
 
 	cmd->bits = 0;
 	cmd->msg = 0;
 
-	msg_push_bits(cmd, id, 2);
-	msg_push_bits(cmd, FSI_GPIO_CMD_ABS_AR, 3);
-	msg_push_bits(cmd, write ? 0 : 1, 1);
+	/* we have 21 bits of address max */
+	addr &= ((1 << 21) - 1);
+
+	/* cmd opcodes are variable length - SAME_AR is only two bits */
+	opcode_bits = 3;
+
+	if (check_same_address(master, id, addr)) {
+		/* we still address the byte offset within the word */
+		addr_bits = 2;
+		opcode_bits = 2;
+		opcode = FSI_GPIO_CMD_SAME_AR;
+
+	} else if (check_relative_address(master, id, addr, &rel_addr)) {
+		/* 8 bits plus sign */
+		addr_bits = 9;
+		addr = rel_addr;
+		opcode = FSI_GPIO_CMD_REL_AR;
+
+	} else {
+		addr_bits = 21;
+		opcode = FSI_GPIO_CMD_ABS_AR;
+	}
 
 	/*
 	 * The read/write size is encoded in the lower bits of the address
@@ -237,7 +309,10 @@ static void build_abs_ar_command(struct fsi_gpio_msg *cmd,
 	if (size == 4)
 		addr |= 1;
 
-	msg_push_bits(cmd, addr & ((1 << 21) - 1), 21);
+	msg_push_bits(cmd, id, 2);
+	msg_push_bits(cmd, opcode, opcode_bits);
+	msg_push_bits(cmd, write ? 0 : 1, 1);
+	msg_push_bits(cmd, addr, addr_bits);
 	msg_push_bits(cmd, ds, 1);
 	for (i = 0; write && i < size; i++)
 		msg_push_bits(cmd, ((uint8_t *)data)[i], 8);
@@ -481,8 +556,9 @@ static int fsi_master_gpio_read(struct fsi_master *_master, int link,
 		return -ENODEV;
 
 	mutex_lock(&master->cmd_lock);
-	build_abs_ar_command(&cmd, id, addr, size, NULL);
+	build_ar_command(master, &cmd, id, addr, size, NULL);
 	rc = fsi_master_gpio_xfer(master, id, &cmd, size, val);
+	last_address_update(master, id, rc == 0, addr);
 	mutex_unlock(&master->cmd_lock);
 
 	return rc;
@@ -499,8 +575,9 @@ static int fsi_master_gpio_write(struct fsi_master *_master, int link,
 		return -ENODEV;
 
 	mutex_lock(&master->cmd_lock);
-	build_abs_ar_command(&cmd, id, addr, size, val);
+	build_ar_command(master, &cmd, id, addr, size, val);
 	rc = fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
+	last_address_update(master, id, rc == 0, addr);
 	mutex_unlock(&master->cmd_lock);
 
 	return rc;
@@ -519,6 +596,7 @@ static int fsi_master_gpio_term(struct fsi_master *_master,
 	mutex_lock(&master->cmd_lock);
 	build_term_command(&cmd, id);
 	rc = fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
+	last_address_update(master, id, false, 0);
 	mutex_unlock(&master->cmd_lock);
 
 	return rc;
@@ -552,6 +630,7 @@ static int fsi_master_gpio_break(struct fsi_master *_master, int link)
 	clock_toggle(master, FSI_POST_BREAK_CLOCKS);
 
 	spin_unlock_irqrestore(&master->bit_lock, flags);
+	last_address_update(master, 0, false, 0);
 	mutex_unlock(&master->cmd_lock);
 
 	/* Wait for logic reset to take effect */
@@ -662,6 +741,7 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
 	master->dev = &pdev->dev;
 	master->master.dev.parent = master->dev;
 	master->master.dev.of_node = of_node_get(dev_of_node(master->dev));
+	master->last_addr = LAST_ADDR_INVALID;
 
 	gpio = devm_gpiod_get(&pdev->dev, "clock", 0);
 	if (IS_ERR(gpio)) {
-- 
2.17.0

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

* [PATCH linux dev-4.13 03/10] fsi/fsi-master-gpio: Implement CRC error recovery
  2018-05-24  5:14 [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Benjamin Herrenschmidt
  2018-05-24  5:14 ` [PATCH linux dev-4.13 02/10] fsi/gpio: Use relative-addressing commands Benjamin Herrenschmidt
@ 2018-05-24  5:14 ` Benjamin Herrenschmidt
  2018-05-24 15:05   ` Christopher Bostic
  2018-05-24  5:14 ` [PATCH linux dev-4.13 04/10] fsi/fsi-master-gpio: More error handling cleanup Benjamin Herrenschmidt
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-24  5:14 UTC (permalink / raw)
  To: openbmc

The FSI protocol defines two modes of recovery from CRC errors,
this implements both:

 - If the device returns an ECRC (it detected a CRC error in the
   command), then we simply issue the command again.

 - If the master detects a CRC error in the response, we send
   an E_POLL command which requests a resend of the response
   without actually re-executing the command (which could otherwise
   have unwanted side effects such as dequeuing a FIFO twice).

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Note: This was actually tested by removing some of my fixes, thus
causing us to hit occasional CRC errors during high LPC activity.
---
 drivers/fsi/fsi-master-gpio.c          | 90 ++++++++++++++++++++------
 include/trace/events/fsi_master_gpio.h | 27 ++++++++
 2 files changed, 99 insertions(+), 18 deletions(-)

diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index 0a6799bda294..351c12f2ac55 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -22,20 +22,23 @@
 #define	FSI_BREAK_CLOCKS	256	/* Number of clocks to issue break */
 #define	FSI_POST_BREAK_CLOCKS	16000	/* Number clocks to set up cfam */
 #define	FSI_INIT_CLOCKS		5000	/* Clock out any old data */
+#define	FSI_GPIO_DPOLL_CLOCKS	50      /* < 21 will cause slave to hang */
+#define	FSI_GPIO_EPOLL_CLOCKS	50      /* Number of clocks for E_POLL retry */
 #define	FSI_GPIO_STD_DELAY	10	/* Standard GPIO delay in nS */
 					/* todo: adjust down as low as */
 					/* possible or eliminate */
+#define FSI_CRC_ERR_RETRIES	10
+
 #define	FSI_GPIO_CMD_DPOLL      0x2
+#define	FSI_GPIO_CMD_EPOLL      0x3
 #define	FSI_GPIO_CMD_TERM	0x3f
 #define FSI_GPIO_CMD_ABS_AR	0x4
 #define FSI_GPIO_CMD_REL_AR	0x5
 #define FSI_GPIO_CMD_SAME_AR	0x3	/* but only a 2-bit opcode... */
 
-
-#define	FSI_GPIO_DPOLL_CLOCKS	50      /* < 21 will cause slave to hang */
-
-/* Bus errors */
-#define	FSI_GPIO_ERR_BUSY	1	/* Slave stuck in busy state */
+/* Slave responses */
+#define	FSI_GPIO_RESP_ACK	0	/* Success */
+#define	FSI_GPIO_RESP_BUSY	1	/* Slave busy */
 #define	FSI_GPIO_RESP_ERRA	2	/* Any (misc) Error */
 #define	FSI_GPIO_RESP_ERRC	3	/* Slave reports master CRC error */
 #define	FSI_GPIO_MTOE		4	/* Master time out error */
@@ -330,6 +333,16 @@ static void build_dpoll_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
 	msg_push_crc(cmd);
 }
 
+static void build_epoll_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
+{
+	cmd->bits = 0;
+	cmd->msg = 0;
+
+	msg_push_bits(cmd, slave_id, 2);
+	msg_push_bits(cmd, FSI_GPIO_CMD_EPOLL, 3);
+	msg_push_crc(cmd);
+}
+
 static void echo_delay(struct fsi_master_gpio *master)
 {
 	set_sda_output(master, 1);
@@ -355,6 +368,12 @@ static void fsi_master_gpio_error(struct fsi_master_gpio *master, int error)
 
 }
 
+/*
+ * Note: callers rely specifically on this returning -EAGAIN for
+ * a CRC error detected in the response. Use other error code
+ * for other situations. It will be converted to something else
+ * higher up the stack before it reaches userspace.
+ */
 static int read_one_response(struct fsi_master_gpio *master,
 		uint8_t data_size, struct fsi_gpio_msg *msgp, uint8_t *tagp)
 {
@@ -379,7 +398,7 @@ static int read_one_response(struct fsi_master_gpio *master,
 			"Master time out waiting for response\n");
 		fsi_master_gpio_error(master, FSI_GPIO_MTOE);
 		spin_unlock_irqrestore(&master->bit_lock, flags);
-		return -EIO;
+		return -ETIMEDOUT;
 	}
 
 	msg.bits = 0;
@@ -405,7 +424,7 @@ static int read_one_response(struct fsi_master_gpio *master,
 	if (crc) {
 		dev_dbg(master->dev, "ERR response CRC\n");
 		fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
-		return -EIO;
+		return -EAGAIN;
 	}
 
 	if (msgp)
@@ -451,11 +470,33 @@ static int poll_for_response(struct fsi_master_gpio *master,
 	unsigned long flags;
 	uint8_t tag;
 	uint8_t *data_byte = data;
-
+	int crc_err_retries = 0;
 retry:
 	rc = read_one_response(master, size, &response, &tag);
-	if (rc)
-		return rc;
+
+	/* Handle retries on CRC errors */
+	if (rc == -EAGAIN) {
+		/* Too many retries ? */
+		if (crc_err_retries++ > FSI_CRC_ERR_RETRIES) {
+			/*
+			 * Pass it up as a -EIO otherwise upper level will retry
+			 * the whole command which isn't what we want here.
+			 */
+			rc = -EIO;
+			goto fail;
+		}
+		dev_dbg(master->dev,
+			 "CRC error retry %d\n", crc_err_retries);
+		trace_fsi_master_gpio_crc_rsp_error(master);
+		build_epoll_command(&cmd, slave);
+		spin_lock_irqsave(&master->bit_lock, flags);
+		clock_zeros(master, FSI_GPIO_EPOLL_CLOCKS);
+		serial_out(master, &cmd);
+		echo_delay(master);
+		spin_unlock_irqrestore(&master->bit_lock, flags);
+		goto retry;
+	} else if (rc)
+		goto fail;
 
 	switch (tag) {
 	case FSI_GPIO_RESP_ACK:
@@ -496,18 +537,21 @@ static int poll_for_response(struct fsi_master_gpio *master,
 		break;
 
 	case FSI_GPIO_RESP_ERRA:
-	case FSI_GPIO_RESP_ERRC:
-		dev_dbg(master->dev, "ERR%c received: 0x%x\n",
-			tag == FSI_GPIO_RESP_ERRA ? 'A' : 'C',
-			(int)response.msg);
+		dev_dbg(master->dev, "ERRA received: 0x%x\n", (int)response.msg);
 		fsi_master_gpio_error(master, response.msg);
 		rc = -EIO;
 		break;
+	case FSI_GPIO_RESP_ERRC:
+		dev_dbg(master->dev, "ERRC received: 0x%x\n", (int)response.msg);
+		fsi_master_gpio_error(master, response.msg);
+		trace_fsi_master_gpio_crc_cmd_error(master);
+		rc = -EAGAIN;
+		break;
 	}
 
 	if (busy_count > 0)
 		trace_fsi_master_gpio_poll_response_busy(master, busy_count);
-
+ fail:
 	/* Clock the slave enough to be ready for next operation */
 	spin_lock_irqsave(&master->bit_lock, flags);
 	clock_zeros(master, FSI_GPIO_PRIME_SLAVE_CLOCKS);
@@ -536,11 +580,21 @@ static int send_request(struct fsi_master_gpio *master,
 static int fsi_master_gpio_xfer(struct fsi_master_gpio *master, uint8_t slave,
 		struct fsi_gpio_msg *cmd, size_t resp_len, void *resp)
 {
-	int rc;
+	int rc = -EAGAIN, retries = 0;
 
-	rc = send_request(master, cmd);
-	if (!rc)
+	while ((retries++) < FSI_CRC_ERR_RETRIES) {
+		rc = send_request(master, cmd);
+		if (rc)
+			break;
 		rc = poll_for_response(master, slave, resp_len, resp);
+		if (rc != -EAGAIN)
+			break;
+		rc = -EIO;
+		dev_warn(master->dev, "ECRC retry %d\n", retries);
+
+		/* Pace it a bit before retry */
+		msleep(1);
+	}
 
 	return rc;
 }
diff --git a/include/trace/events/fsi_master_gpio.h b/include/trace/events/fsi_master_gpio.h
index 48e83e5755f4..e18b94ce85b2 100644
--- a/include/trace/events/fsi_master_gpio.h
+++ b/include/trace/events/fsi_master_gpio.h
@@ -63,6 +63,33 @@ TRACE_EVENT(fsi_master_gpio_break,
 	)
 );
 
+TRACE_EVENT(fsi_master_gpio_crc_cmd_error,
+	TP_PROTO(const struct fsi_master_gpio *master),
+	TP_ARGS(master),
+	TP_STRUCT__entry(
+		__field(int,	master_idx)
+	),
+	TP_fast_assign(
+		__entry->master_idx = master->master.idx;
+	),
+	TP_printk("fsi-gpio%d ----CRC command retry---",
+		__entry->master_idx
+	)
+);
+
+TRACE_EVENT(fsi_master_gpio_crc_rsp_error,
+	TP_PROTO(const struct fsi_master_gpio *master),
+	TP_ARGS(master),
+	TP_STRUCT__entry(
+		__field(int,	master_idx)
+	),
+	TP_fast_assign(
+		__entry->master_idx = master->master.idx;
+	),
+	TP_printk("fsi-gpio%d ----CRC response---",
+		__entry->master_idx
+	)
+);
 
 TRACE_EVENT(fsi_master_gpio_poll_response_busy,
 	TP_PROTO(const struct fsi_master_gpio *master, int busy),
-- 
2.17.0

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

* [PATCH linux dev-4.13 04/10] fsi/fsi-master-gpio: More error handling cleanup
  2018-05-24  5:14 [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Benjamin Herrenschmidt
  2018-05-24  5:14 ` [PATCH linux dev-4.13 02/10] fsi/gpio: Use relative-addressing commands Benjamin Herrenschmidt
  2018-05-24  5:14 ` [PATCH linux dev-4.13 03/10] fsi/fsi-master-gpio: Implement CRC error recovery Benjamin Herrenschmidt
@ 2018-05-24  5:14 ` Benjamin Herrenschmidt
  2018-05-24 18:50   ` Christopher Bostic
  2018-05-24  5:14 ` [PATCH linux dev-4.13 05/10] fsi/master-gpio: Replace bit_bit lock with IRQ disable/enable Benjamin Herrenschmidt
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-24  5:14 UTC (permalink / raw)
  To: openbmc

Remove calls to the empty and useless fsi_master_gpio_error()
function, and report CRC errors as "FSI_ERR_NO_SLAVE" when
reading an all 1's response.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/fsi/fsi-master-gpio.c | 26 +++++---------------------
 1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index 351c12f2ac55..da556da62846 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -41,13 +41,6 @@
 #define	FSI_GPIO_RESP_BUSY	1	/* Slave busy */
 #define	FSI_GPIO_RESP_ERRA	2	/* Any (misc) Error */
 #define	FSI_GPIO_RESP_ERRC	3	/* Slave reports master CRC error */
-#define	FSI_GPIO_MTOE		4	/* Master time out error */
-#define	FSI_GPIO_CRC_INVAL	5	/* Master reports slave CRC error */
-
-/* Normal slave responses */
-#define	FSI_GPIO_RESP_BUSY	1
-#define	FSI_GPIO_RESP_ACK	0
-#define	FSI_GPIO_RESP_ACKD	4
 
 #define	FSI_GPIO_MAX_BUSY	200
 #define	FSI_GPIO_MTOE_COUNT	1000
@@ -359,15 +352,6 @@ static void build_term_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
 	msg_push_crc(cmd);
 }
 
-/*
- * Store information on master errors so handler can detect and clean
- * up the bus
- */
-static void fsi_master_gpio_error(struct fsi_master_gpio *master, int error)
-{
-
-}
-
 /*
  * Note: callers rely specifically on this returning -EAGAIN for
  * a CRC error detected in the response. Use other error code
@@ -396,7 +380,6 @@ static int read_one_response(struct fsi_master_gpio *master,
 	if (i == FSI_GPIO_MTOE_COUNT) {
 		dev_dbg(master->dev,
 			"Master time out waiting for response\n");
-		fsi_master_gpio_error(master, FSI_GPIO_MTOE);
 		spin_unlock_irqrestore(&master->bit_lock, flags);
 		return -ETIMEDOUT;
 	}
@@ -422,8 +405,11 @@ static int read_one_response(struct fsi_master_gpio *master,
 	crc = crc4(0, 1, 1);
 	crc = crc4(crc, msg.msg, msg.bits);
 	if (crc) {
-		dev_dbg(master->dev, "ERR response CRC\n");
-		fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
+		/* Check if it's all 1's, that probably means the host is off */
+		if (((~msg.msg) & ((1ull << msg.bits) - 1)) == 0)
+			return -ENODEV;
+		dev_dbg(master->dev, "ERR response CRC msg: 0x%016llx (%d bits)\n",
+			msg.msg, msg.bits);
 		return -EAGAIN;
 	}
 
@@ -538,12 +524,10 @@ static int poll_for_response(struct fsi_master_gpio *master,
 
 	case FSI_GPIO_RESP_ERRA:
 		dev_dbg(master->dev, "ERRA received: 0x%x\n", (int)response.msg);
-		fsi_master_gpio_error(master, response.msg);
 		rc = -EIO;
 		break;
 	case FSI_GPIO_RESP_ERRC:
 		dev_dbg(master->dev, "ERRC received: 0x%x\n", (int)response.msg);
-		fsi_master_gpio_error(master, response.msg);
 		trace_fsi_master_gpio_crc_cmd_error(master);
 		rc = -EAGAIN;
 		break;
-- 
2.17.0

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

* [PATCH linux dev-4.13 05/10] fsi/master-gpio: Replace bit_bit lock with IRQ disable/enable
  2018-05-24  5:14 [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Benjamin Herrenschmidt
                   ` (2 preceding siblings ...)
  2018-05-24  5:14 ` [PATCH linux dev-4.13 04/10] fsi/fsi-master-gpio: More error handling cleanup Benjamin Herrenschmidt
@ 2018-05-24  5:14 ` Benjamin Herrenschmidt
  2018-05-24  5:14 ` [PATCH linux dev-4.13 06/10] fsi: Remove old sbefifo driver Benjamin Herrenschmidt
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-24  5:14 UTC (permalink / raw)
  To: openbmc

From: Jeremy Kerr <jk@ozlabs.org>

We currently use a spinlock (bit_lock) around operations that clock bits
out of the FSI bus, and a mutex to protect against simultaneous access
to the master.

This means that bit_lock isn't needed for mutual exlusion, only to
prevent timing issues when clocking bits out.

To reflect this, this change converts bit_lock to just the
local_irq_save/restore operation.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/fsi/fsi-master-gpio.c | 48 +++++++++++++++++------------------
 1 file changed, 23 insertions(+), 25 deletions(-)

diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index da556da62846..084e9da8d151 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -8,11 +8,11 @@
 #include <linux/fsi.h>
 #include <linux/gpio/consumer.h>
 #include <linux/io.h>
+#include <linux/irqflags.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
-#include <linux/spinlock.h>
 
 #include "fsi-master.h"
 
@@ -56,7 +56,6 @@ struct fsi_master_gpio {
 	struct fsi_master	master;
 	struct device		*dev;
 	struct mutex		cmd_lock;	/* mutex for command ordering */
-	spinlock_t		bit_lock;	/* lock for clocking bits out */
 	struct gpio_desc	*gpio_clk;
 	struct gpio_desc	*gpio_data;
 	struct gpio_desc	*gpio_trans;	/* Voltage translator */
@@ -367,7 +366,7 @@ static int read_one_response(struct fsi_master_gpio *master,
 	uint8_t tag;
 	int i;
 
-	spin_lock_irqsave(&master->bit_lock, flags);
+	local_irq_save(flags);
 
 	/* wait for the start bit */
 	for (i = 0; i < FSI_GPIO_MTOE_COUNT; i++) {
@@ -380,7 +379,7 @@ static int read_one_response(struct fsi_master_gpio *master,
 	if (i == FSI_GPIO_MTOE_COUNT) {
 		dev_dbg(master->dev,
 			"Master time out waiting for response\n");
-		spin_unlock_irqrestore(&master->bit_lock, flags);
+		local_irq_restore(flags);
 		return -ETIMEDOUT;
 	}
 
@@ -399,7 +398,7 @@ static int read_one_response(struct fsi_master_gpio *master,
 	/* read CRC */
 	serial_in(master, &msg, FSI_GPIO_CRC_SIZE);
 
-	spin_unlock_irqrestore(&master->bit_lock, flags);
+	local_irq_restore(flags);
 
 	/* we have a whole message now; check CRC */
 	crc = crc4(0, 1, 1);
@@ -430,10 +429,10 @@ static int issue_term(struct fsi_master_gpio *master, uint8_t slave)
 
 	build_term_command(&cmd, slave);
 
-	spin_lock_irqsave(&master->bit_lock, flags);
+	local_irq_save(flags);
 	serial_out(master, &cmd);
 	echo_delay(master);
-	spin_unlock_irqrestore(&master->bit_lock, flags);
+	local_irq_restore(flags);
 
 	rc = read_one_response(master, 0, NULL, &tag);
 	if (rc < 0) {
@@ -475,11 +474,11 @@ static int poll_for_response(struct fsi_master_gpio *master,
 			 "CRC error retry %d\n", crc_err_retries);
 		trace_fsi_master_gpio_crc_rsp_error(master);
 		build_epoll_command(&cmd, slave);
-		spin_lock_irqsave(&master->bit_lock, flags);
+		local_irq_save(flags);
 		clock_zeros(master, FSI_GPIO_EPOLL_CLOCKS);
 		serial_out(master, &cmd);
 		echo_delay(master);
-		spin_unlock_irqrestore(&master->bit_lock, flags);
+		local_irq_restore(flags);
 		goto retry;
 	} else if (rc)
 		goto fail;
@@ -506,18 +505,18 @@ static int poll_for_response(struct fsi_master_gpio *master,
 		 */
 		if (busy_count++ < FSI_GPIO_MAX_BUSY) {
 			build_dpoll_command(&cmd, slave);
-			spin_lock_irqsave(&master->bit_lock, flags);
+			local_irq_save(flags);
 			clock_zeros(master, FSI_GPIO_DPOLL_CLOCKS);
 			serial_out(master, &cmd);
 			echo_delay(master);
-			spin_unlock_irqrestore(&master->bit_lock, flags);
+			local_irq_restore(flags);
 			goto retry;
 		}
 		dev_warn(master->dev,
 			"ERR slave is stuck in busy state, issuing TERM\n");
-		spin_lock_irqsave(&master->bit_lock, flags);
+		local_irq_save(flags);
 		clock_zeros(master, FSI_GPIO_DPOLL_CLOCKS);
-		spin_unlock_irqrestore(&master->bit_lock, flags);
+		local_irq_restore(flags);
 		issue_term(master, slave);
 		rc = -EIO;
 		break;
@@ -537,9 +536,10 @@ static int poll_for_response(struct fsi_master_gpio *master,
 		trace_fsi_master_gpio_poll_response_busy(master, busy_count);
  fail:
 	/* Clock the slave enough to be ready for next operation */
-	spin_lock_irqsave(&master->bit_lock, flags);
+	local_irq_save(flags);
 	clock_zeros(master, FSI_GPIO_PRIME_SLAVE_CLOCKS);
-	spin_unlock_irqrestore(&master->bit_lock, flags);
+	local_irq_restore(flags);
+
 	return rc;
 }
 
@@ -548,15 +548,13 @@ static int send_request(struct fsi_master_gpio *master,
 {
 	unsigned long flags;
 
-	spin_lock_irqsave(&master->bit_lock, flags);
-	if (master->external_mode) {
-		spin_unlock_irqrestore(&master->bit_lock, flags);
+	if (master->external_mode)
 		return -EBUSY;
-	}
 
+	local_irq_save(flags);
 	serial_out(master, cmd);
 	echo_delay(master);
-	spin_unlock_irqrestore(&master->bit_lock, flags);
+	local_irq_restore(flags);
 
 	return 0;
 }
@@ -656,7 +654,7 @@ static int fsi_master_gpio_break(struct fsi_master *_master, int link)
 		return -EBUSY;
 	}
 
-	spin_lock_irqsave(&master->bit_lock, flags);
+	local_irq_save(flags);
 
 	set_sda_output(master, 1);
 	sda_out(master, 1);
@@ -667,7 +665,8 @@ static int fsi_master_gpio_break(struct fsi_master *_master, int link)
 	sda_out(master, 1);
 	clock_toggle(master, FSI_POST_BREAK_CLOCKS);
 
-	spin_unlock_irqrestore(&master->bit_lock, flags);
+	local_irq_restore(flags);
+
 	last_address_update(master, 0, false, 0);
 	mutex_unlock(&master->cmd_lock);
 
@@ -688,9 +687,9 @@ static void fsi_master_gpio_init(struct fsi_master_gpio *master)
 	gpiod_direction_output(master->gpio_data, 1);
 
 	/* todo: evaluate if clocks can be reduced */
-	spin_lock_irqsave(&master->bit_lock, flags);
+	local_irq_save(flags);
 	clock_zeros(master, FSI_INIT_CLOCKS);
-	spin_unlock_irqrestore(&master->bit_lock, flags);
+	local_irq_restore(flags);
 }
 
 static void fsi_master_gpio_init_external(struct fsi_master_gpio *master)
@@ -832,7 +831,6 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
 	master->master.send_break = fsi_master_gpio_break;
 	master->master.link_enable = fsi_master_gpio_link_enable;
 	platform_set_drvdata(pdev, master);
-	spin_lock_init(&master->bit_lock);
 	mutex_init(&master->cmd_lock);
 
 	fsi_master_gpio_init(master);
-- 
2.17.0

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

* [PATCH linux dev-4.13 06/10] fsi: Remove old sbefifo driver
  2018-05-24  5:14 [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Benjamin Herrenschmidt
                   ` (3 preceding siblings ...)
  2018-05-24  5:14 ` [PATCH linux dev-4.13 05/10] fsi/master-gpio: Replace bit_bit lock with IRQ disable/enable Benjamin Herrenschmidt
@ 2018-05-24  5:14 ` Benjamin Herrenschmidt
  2018-05-24  5:14 ` [PATCH linux dev-4.13 07/10] fsi/sbefifo: Add driver for the SBE FIFO Benjamin Herrenschmidt
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-24  5:14 UTC (permalink / raw)
  To: openbmc

And remporarily disable build of fsi-occ
---
 drivers/fsi/Makefile        |    4 +-
 drivers/fsi/fsi-sbefifo.c   | 1071 -----------------------------------
 drivers/hwmon/occ/Makefile  |    2 +-
 include/linux/fsi-sbefifo.h |   30 -
 4 files changed, 3 insertions(+), 1104 deletions(-)
 delete mode 100644 drivers/fsi/fsi-sbefifo.c
 delete mode 100644 include/linux/fsi-sbefifo.h

diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
index 75fdc6d8cfc4..61e8e420c7ed 100644
--- a/drivers/fsi/Makefile
+++ b/drivers/fsi/Makefile
@@ -3,5 +3,5 @@ obj-$(CONFIG_FSI) += fsi-core.o
 obj-$(CONFIG_FSI_MASTER_HUB) += fsi-master-hub.o
 obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
 obj-$(CONFIG_FSI_SCOM) += fsi-scom.o
-obj-$(CONFIG_FSI_SBEFIFO) += fsi-sbefifo.o
-obj-$(CONFIG_FSI_OCC) += fsi-occ.o
+#obj-$(CONFIG_FSI_SBEFIFO) += fsi-sbefifo.o
+#obj-$(CONFIG_FSI_OCC) += fsi-occ.o
diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
deleted file mode 100644
index cc9b9e36ac72..000000000000
--- a/drivers/fsi/fsi-sbefifo.c
+++ /dev/null
@@ -1,1071 +0,0 @@
-/*
- * Copyright (C) IBM Corporation 2017
- *
- * 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
- * MERGCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#include <linux/device.h>
-#include <linux/errno.h>
-#include <linux/fs.h>
-#include <linux/fsi.h>
-#include <linux/fsi-sbefifo.h>
-#include <linux/idr.h>
-#include <linux/kernel.h>
-#include <linux/kref.h>
-#include <linux/list.h>
-#include <linux/miscdevice.h>
-#include <linux/module.h>
-#include <linux/mutex.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
-#include <linux/of_platform.h>
-#include <linux/poll.h>
-#include <linux/sched.h>
-#include <linux/slab.h>
-#include <linux/spinlock.h>
-#include <linux/uaccess.h>
-#include <linux/wait.h>
-#include <linux/workqueue.h>
-
-#define CREATE_TRACE_POINTS
-#include <trace/events/sbefifo.h>
-
-/*
- * The SBEFIFO is a pipe-like FSI device for communicating with
- * the self boot engine on POWER processors.
- */
-
-#define DEVICE_NAME		"sbefifo"
-#define FSI_ENGID_SBE		0x22
-#define SBEFIFO_BUF_CNT		32
-
-#define SBEFIFO_UP		0x00	/* Up register offset */
-#define SBEFIFO_DWN		0x40	/* Down register offset */
-
-#define SBEFIFO_STS		0x04
-#define   SBEFIFO_EMPTY			BIT(20)
-#define   SBEFIFO_STS_RESET_REQ		BIT(25)
-#define SBEFIFO_EOT_RAISE	0x08
-#define   SBEFIFO_EOT_MAGIC		0xffffffff
-#define SBEFIFO_REQ_RESET	0x0C
-#define SBEFIFO_EOT_ACK		0x14
-
-#define SBEFIFO_RESCHEDULE	msecs_to_jiffies(500)
-#define SBEFIFO_MAX_RESCHDULE	msecs_to_jiffies(5000)
-
-struct sbefifo {
-	struct delayed_work work;
-	struct fsi_device *fsi_dev;
-	struct miscdevice mdev;
-	wait_queue_head_t wait;
-	struct list_head xfrs;
-	struct kref kref;
-	struct mutex list_lock;		/* lock access to the xfrs list */
-	struct mutex sbefifo_lock;	/* lock access to the hardware */
-	char name[32];
-	int idx;
-	int rc;
-};
-
-struct sbefifo_buf {
-	u32 buf[SBEFIFO_BUF_CNT];
-	unsigned long flags;
-#define SBEFIFO_BUF_FULL		1
-	u32 *rpos;
-	u32 *wpos;
-};
-
-struct sbefifo_xfr {
-	unsigned long wait_data_timeout;
-	struct sbefifo_buf *rbuf;
-	struct sbefifo_buf *wbuf;
-	struct list_head client;
-	struct list_head xfrs;
-	unsigned long flags;
-#define SBEFIFO_XFR_WRITE_DONE		1
-#define SBEFIFO_XFR_RESP_PENDING	2
-#define SBEFIFO_XFR_COMPLETE		3
-#define SBEFIFO_XFR_CANCEL		4
-};
-
-struct sbefifo_client {
-	struct sbefifo_buf rbuf;
-	struct sbefifo_buf wbuf;
-	struct list_head xfrs;
-	struct sbefifo *dev;
-	struct kref kref;
-	unsigned long f_flags;
-};
-
-static struct workqueue_struct *sbefifo_wq;
-
-static DEFINE_IDA(sbefifo_ida);
-
-static int sbefifo_inw(struct sbefifo *sbefifo, int reg, u32 *word)
-{
-	int rc;
-	__be32 raw_word;
-
-	rc = fsi_device_read(sbefifo->fsi_dev, reg, &raw_word,
-			     sizeof(raw_word));
-	if (rc)
-		return rc;
-
-	*word = be32_to_cpu(raw_word);
-
-	return 0;
-}
-
-static int sbefifo_outw(struct sbefifo *sbefifo, int reg, u32 word)
-{
-	__be32 raw_word = cpu_to_be32(word);
-
-	return fsi_device_write(sbefifo->fsi_dev, reg, &raw_word,
-				sizeof(raw_word));
-}
-
-/* Don't flip endianness of data to/from FIFO, just pass through. */
-static int sbefifo_readw(struct sbefifo *sbefifo, u32 *word)
-{
-	return fsi_device_read(sbefifo->fsi_dev, SBEFIFO_DWN, word,
-			       sizeof(*word));
-}
-
-static int sbefifo_writew(struct sbefifo *sbefifo, u32 word)
-{
-	return fsi_device_write(sbefifo->fsi_dev, SBEFIFO_UP, &word,
-				sizeof(word));
-}
-
-static int sbefifo_ack_eot(struct sbefifo *sbefifo)
-{
-	u32 discard;
-	int ret;
-
-	 /* Discard the EOT word. */
-	ret = sbefifo_readw(sbefifo, &discard);
-	if (ret)
-		return ret;
-
-	return sbefifo_outw(sbefifo, SBEFIFO_DWN | SBEFIFO_EOT_ACK,
-			    SBEFIFO_EOT_MAGIC);
-}
-
-static size_t sbefifo_dev_nwreadable(u32 sts)
-{
-	static const u32 FIFO_NTRY_CNT_MSK = 0x000f0000;
-	static const unsigned int FIFO_NTRY_CNT_SHIFT = 16;
-
-	return (sts & FIFO_NTRY_CNT_MSK) >> FIFO_NTRY_CNT_SHIFT;
-}
-
-static size_t sbefifo_dev_nwwriteable(u32 sts)
-{
-	static const size_t FIFO_DEPTH = 8;
-
-	return FIFO_DEPTH - sbefifo_dev_nwreadable(sts);
-}
-
-static void sbefifo_buf_init(struct sbefifo_buf *buf)
-{
-	WRITE_ONCE(buf->flags, 0);
-	WRITE_ONCE(buf->rpos, buf->buf);
-	WRITE_ONCE(buf->wpos, buf->buf);
-}
-
-static size_t sbefifo_buf_nbreadable(struct sbefifo_buf *buf)
-{
-	size_t n;
-	u32 *rpos = READ_ONCE(buf->rpos);
-	u32 *wpos = READ_ONCE(buf->wpos);
-
-	if (test_bit(SBEFIFO_BUF_FULL, &buf->flags))
-		n = SBEFIFO_BUF_CNT;
-	else if (rpos <= wpos)
-		n = wpos - rpos;
-	else
-		n = (buf->buf + SBEFIFO_BUF_CNT) - rpos;
-
-	return n << 2;
-}
-
-static size_t sbefifo_buf_nbwriteable(struct sbefifo_buf *buf)
-{
-	size_t n;
-	u32 *rpos = READ_ONCE(buf->rpos);
-	u32 *wpos = READ_ONCE(buf->wpos);
-
-	if (test_bit(SBEFIFO_BUF_FULL, &buf->flags))
-		n = 0;
-	else if (wpos < rpos)
-		n = rpos - wpos;
-	else
-		n = (buf->buf + SBEFIFO_BUF_CNT) - wpos;
-
-	return n << 2;
-}
-
-/*
- * Update pointers and flags after doing a buffer read.  Return true if the
- * buffer is now empty;
- */
-static bool sbefifo_buf_readnb(struct sbefifo_buf *buf, size_t n)
-{
-	u32 *rpos = READ_ONCE(buf->rpos);
-	u32 *wpos = READ_ONCE(buf->wpos);
-
-	if (n)
-		clear_bit(SBEFIFO_BUF_FULL, &buf->flags);
-
-	rpos += (n >> 2);
-	if (rpos == buf->buf + SBEFIFO_BUF_CNT)
-		rpos = buf->buf;
-
-	WRITE_ONCE(buf->rpos, rpos);
-
-	return rpos == wpos;
-}
-
-/*
- * Update pointers and flags after doing a buffer write.  Return true if the
- * buffer is now full.
- */
-static bool sbefifo_buf_wrotenb(struct sbefifo_buf *buf, size_t n)
-{
-	u32 *rpos = READ_ONCE(buf->rpos);
-	u32 *wpos = READ_ONCE(buf->wpos);
-
-	wpos += (n >> 2);
-	if (wpos == buf->buf + SBEFIFO_BUF_CNT)
-		wpos = buf->buf;
-	if (wpos == rpos)
-		set_bit(SBEFIFO_BUF_FULL, &buf->flags);
-
-	WRITE_ONCE(buf->wpos, wpos);
-
-	return rpos == wpos;
-}
-
-static void sbefifo_free(struct kref *kref)
-{
-	struct sbefifo *sbefifo = container_of(kref, struct sbefifo, kref);
-
-	kfree(sbefifo);
-}
-
-static void sbefifo_get(struct sbefifo *sbefifo)
-{
-	kref_get(&sbefifo->kref);
-}
-
-static void sbefifo_put(struct sbefifo *sbefifo)
-{
-	kref_put(&sbefifo->kref, sbefifo_free);
-}
-
-static struct sbefifo_xfr *sbefifo_enq_xfr(struct sbefifo_client *client)
-{
-	struct sbefifo *sbefifo = client->dev;
-	struct sbefifo_xfr *xfr;
-
-	if (READ_ONCE(sbefifo->rc))
-		return ERR_PTR(sbefifo->rc);
-
-	xfr = kzalloc(sizeof(*xfr), GFP_KERNEL);
-	if (!xfr)
-		return ERR_PTR(-ENOMEM);
-
-	trace_sbefifo_enq_xfer(client, xfr);
-
-	xfr->rbuf = &client->rbuf;
-	xfr->wbuf = &client->wbuf;
-	list_add_tail(&xfr->xfrs, &sbefifo->xfrs);
-	list_add_tail(&xfr->client, &client->xfrs);
-
-	return xfr;
-}
-
-static bool sbefifo_xfr_rsp_pending(struct sbefifo_client *client)
-{
-	struct sbefifo_xfr *xfr = list_first_entry_or_null(&client->xfrs,
-							   struct sbefifo_xfr,
-							   client);
-
-	if (xfr && test_bit(SBEFIFO_XFR_RESP_PENDING, &xfr->flags))
-		return true;
-
-	return false;
-}
-
-static struct sbefifo_client *sbefifo_new_client(struct sbefifo *sbefifo)
-{
-	struct sbefifo_client *client;
-
-	client = kzalloc(sizeof(*client), GFP_KERNEL);
-	if (!client)
-		return NULL;
-
-	trace_sbefifo_new_client(client);
-
-	kref_init(&client->kref);
-	client->dev = sbefifo;
-	sbefifo_buf_init(&client->rbuf);
-	sbefifo_buf_init(&client->wbuf);
-	INIT_LIST_HEAD(&client->xfrs);
-
-	sbefifo_get(sbefifo);
-
-	return client;
-}
-
-static void sbefifo_release_client(struct kref *kref)
-{
-	struct sbefifo *sbefifo;
-	struct sbefifo_client *client;
-	struct sbefifo_xfr *xfr, *tmp;
-
-	client = container_of(kref, struct sbefifo_client, kref);
-	sbefifo = client->dev;
-
-	if (!READ_ONCE(sbefifo->rc)) {
-		list_for_each_entry_safe(xfr, tmp, &client->xfrs, client) {
-			if (test_bit(SBEFIFO_XFR_COMPLETE, &xfr->flags)) {
-				list_del(&xfr->client);
-				kfree(xfr);
-				continue;
-			}
-
-			/*
-			 * The client left with pending or running xfrs.
-			 * Cancel them.
-			 */
-			set_bit(SBEFIFO_XFR_CANCEL, &xfr->flags);
-			sbefifo_get(sbefifo);
-			if (!queue_work(sbefifo_wq, &sbefifo->work.work))
-				sbefifo_put(sbefifo);
-		}
-	}
-
-	sbefifo_put(sbefifo);
-	trace_sbefifo_release_client(client);
-	kfree(client);
-}
-
-static void sbefifo_get_client(struct sbefifo_client *client)
-{
-	kref_get(&client->kref);
-}
-
-static void sbefifo_put_client(struct sbefifo_client *client)
-{
-	kref_put(&client->kref, sbefifo_release_client);
-}
-
-static struct sbefifo_xfr *sbefifo_next_xfr(struct sbefifo *sbefifo)
-{
-	struct sbefifo_xfr *xfr, *tmp;
-
-	list_for_each_entry_safe(xfr, tmp, &sbefifo->xfrs, xfrs) {
-		if (unlikely(test_bit(SBEFIFO_XFR_CANCEL, &xfr->flags))) {
-			/* Discard cancelled transfers. */
-			list_del(&xfr->xfrs);
-			kfree(xfr);
-			continue;
-		}
-
-		return xfr;
-	}
-
-	return NULL;
-}
-
-static void sbefifo_worker(struct work_struct *work)
-{
-	static const unsigned long EOT_MASK = 0x000000ff;
-	struct delayed_work *dwork = to_delayed_work(work);
-	struct sbefifo *sbefifo = container_of(dwork, struct sbefifo, work);
-	struct sbefifo_buf *rbuf, *wbuf;
-	struct sbefifo_xfr *xfr, *tmp;
-	struct sbefifo_buf drain;
-	size_t devn, bufn;
-	int eot = 0;
-	int ret = 0;
-	u32 sts;
-	int i;
-
-	mutex_lock(&sbefifo->list_lock);
-	xfr = list_first_entry_or_null(&sbefifo->xfrs, struct sbefifo_xfr,
-				       xfrs);
-	mutex_unlock(&sbefifo->list_lock);
-	if (!xfr)
-		return;
-
-	mutex_lock(&sbefifo->sbefifo_lock);
-
-	trace_sbefifo_begin_xfer(xfr);
-
-again:
-	rbuf = xfr->rbuf;
-	wbuf = xfr->wbuf;
-
-	if (unlikely(test_bit(SBEFIFO_XFR_CANCEL, &xfr->flags))) {
-		/* The client left. */
-		rbuf = &drain;
-		wbuf = &drain;
-		sbefifo_buf_init(&drain);
-		if (!test_bit(SBEFIFO_XFR_RESP_PENDING, &xfr->flags))
-			set_bit(SBEFIFO_XFR_WRITE_DONE, &xfr->flags);
-	}
-
-	 /* Drain the write buffer. */
-	while ((bufn = sbefifo_buf_nbreadable(wbuf))) {
-		ret = sbefifo_inw(sbefifo, SBEFIFO_UP | SBEFIFO_STS, &sts);
-		if (ret)
-			goto out;
-
-		devn = sbefifo_dev_nwwriteable(sts);
-		if (devn == 0) {
-			/* No open slot for write.  Reschedule. */
-			queue_delayed_work(sbefifo_wq, &sbefifo->work,
-					   SBEFIFO_RESCHEDULE);
-			goto out_unlock;
-		}
-
-		devn = min_t(size_t, devn, bufn >> 2);
-		for (i = 0; i < devn; i++) {
-			ret = sbefifo_writew(sbefifo, *wbuf->rpos);
-			if (ret)
-				goto out;
-
-			sbefifo_buf_readnb(wbuf, 1 << 2);
-		}
-	}
-
-	 /* Send EOT if the writer is finished. */
-	if (test_and_clear_bit(SBEFIFO_XFR_WRITE_DONE, &xfr->flags)) {
-		ret = sbefifo_outw(sbefifo, SBEFIFO_UP | SBEFIFO_EOT_RAISE,
-				   SBEFIFO_EOT_MAGIC);
-		if (ret)
-			goto out;
-
-		/* Inform reschedules that the writer is finished. */
-		set_bit(SBEFIFO_XFR_RESP_PENDING, &xfr->flags);
-	}
-
-	/* Nothing left to do if the writer is not finished. */
-	if (!test_bit(SBEFIFO_XFR_RESP_PENDING, &xfr->flags))
-		goto out;
-
-	 /* Fill the read buffer. */
-	while ((bufn = sbefifo_buf_nbwriteable(rbuf))) {
-		ret = sbefifo_inw(sbefifo, SBEFIFO_DWN | SBEFIFO_STS, &sts);
-		if (ret)
-			goto out;
-
-		devn = sbefifo_dev_nwreadable(sts);
-		if (devn == 0) {
-			/*
-			 * Limit the maximum waiting period for data in the
-			 * FIFO. If the SBE isn't running, we will wait
-			 * forever.
-			 */
-			if (!xfr->wait_data_timeout) {
-				xfr->wait_data_timeout =
-					jiffies + SBEFIFO_MAX_RESCHDULE;
-			} else if (time_after(jiffies,
-					      xfr->wait_data_timeout)) {
-				ret = -ETIME;
-				goto out;
-			}
-
-			/* No data yet.  Reschedule. */
-			queue_delayed_work(sbefifo_wq, &sbefifo->work,
-					   SBEFIFO_RESCHEDULE);
-			goto out_unlock;
-		} else {
-			xfr->wait_data_timeout = 0;
-		}
-
-		/* Fill.  The EOT word is discarded.  */
-		devn = min_t(size_t, devn, bufn >> 2);
-		eot = (sts & EOT_MASK) != 0;
-		if (eot)
-			devn--;
-
-		for (i = 0; i < devn; i++) {
-			ret = sbefifo_readw(sbefifo, rbuf->wpos);
-			if (ret)
-				goto out;
-
-			if (likely(!test_bit(SBEFIFO_XFR_CANCEL, &xfr->flags)))
-				sbefifo_buf_wrotenb(rbuf, 1 << 2);
-		}
-
-		if (eot) {
-			ret = sbefifo_ack_eot(sbefifo);
-			if (ret)
-				goto out;
-
-			set_bit(SBEFIFO_XFR_COMPLETE, &xfr->flags);
-
-			mutex_lock(&sbefifo->list_lock);
-			list_del(&xfr->xfrs);
-			mutex_unlock(&sbefifo->list_lock);
-
-			if (unlikely(test_bit(SBEFIFO_XFR_CANCEL,
-					      &xfr->flags)))
-				kfree(xfr);
-			break;
-		}
-	}
-
-out:
-	trace_sbefifo_end_xfer(xfr, ret);
-
-	if (unlikely(ret)) {
-		sbefifo->rc = ret;
-		dev_err(&sbefifo->fsi_dev->dev,
-			"Fatal bus access failure: %d\n", ret);
-
-		mutex_lock(&sbefifo->list_lock);
-		list_for_each_entry_safe(xfr, tmp, &sbefifo->xfrs, xfrs) {
-			list_del(&xfr->xfrs);
-			kfree(xfr);
-		}
-		INIT_LIST_HEAD(&sbefifo->xfrs);
-		mutex_unlock(&sbefifo->list_lock);
-	} else if (eot) {
-		mutex_lock(&sbefifo->list_lock);
-		xfr = sbefifo_next_xfr(sbefifo);
-		mutex_unlock(&sbefifo->list_lock);
-
-		if (xfr) {
-			wake_up_interruptible(&sbefifo->wait);
-			goto again;
-		}
-	}
-
-	sbefifo_put(sbefifo);
-	wake_up_interruptible(&sbefifo->wait);
-
-out_unlock:
-	mutex_unlock(&sbefifo->sbefifo_lock);
-}
-
-static int sbefifo_open(struct inode *inode, struct file *file)
-{
-	struct sbefifo *sbefifo = container_of(file->private_data,
-					       struct sbefifo, mdev);
-	struct sbefifo_client *client;
-	int ret;
-
-	ret = READ_ONCE(sbefifo->rc);
-	if (ret)
-		return ret;
-
-	client = sbefifo_new_client(sbefifo);
-	if (!client)
-		return -ENOMEM;
-
-	file->private_data = client;
-	client->f_flags = file->f_flags;
-
-	return 0;
-}
-
-static unsigned int sbefifo_poll(struct file *file, poll_table *wait)
-{
-	struct sbefifo_client *client = file->private_data;
-	struct sbefifo *sbefifo = client->dev;
-	unsigned int mask = 0;
-
-	poll_wait(file, &sbefifo->wait, wait);
-
-	if (READ_ONCE(sbefifo->rc))
-		mask |= POLLERR;
-
-	if (sbefifo_buf_nbreadable(&client->rbuf))
-		mask |= POLLIN;
-
-	if (sbefifo_buf_nbwriteable(&client->wbuf))
-		mask |= POLLOUT;
-
-	return mask;
-}
-
-static bool sbefifo_read_ready(struct sbefifo *sbefifo,
-			       struct sbefifo_client *client, size_t *n,
-			       size_t *ret)
-{
-	struct sbefifo_xfr *xfr = list_first_entry_or_null(&client->xfrs,
-							   struct sbefifo_xfr,
-							   client);
-
-	*n = sbefifo_buf_nbreadable(&client->rbuf);
-	*ret = READ_ONCE(sbefifo->rc);
-
-	return *ret || *n ||
-		(xfr && test_bit(SBEFIFO_XFR_COMPLETE, &xfr->flags));
-}
-
-static ssize_t sbefifo_read_common(struct sbefifo_client *client,
-				   char __user *ubuf, char *kbuf, size_t len)
-{
-	struct sbefifo *sbefifo = client->dev;
-	struct sbefifo_xfr *xfr;
-	size_t n;
-	ssize_t ret = 0;
-
-	if ((len >> 2) << 2 != len)
-		return -EINVAL;
-
-	if ((client->f_flags & O_NONBLOCK) && !sbefifo_xfr_rsp_pending(client))
-		return -EAGAIN;
-
-	sbefifo_get_client(client);
-	if (wait_event_interruptible(sbefifo->wait,
-				     sbefifo_read_ready(sbefifo, client, &n,
-							&ret))) {
-		ret = -ERESTARTSYS;
-		goto out;
-	}
-
-	if (ret) {
-		INIT_LIST_HEAD(&client->xfrs);
-		goto out;
-	}
-
-	trace_sbefifo_deq_xfer(client, list_first_entry_or_null(&client->xfrs,
-							   struct sbefifo_xfr,
-							   client));
-
-	n = min_t(size_t, n, len);
-
-	if (ubuf) {
-		if (copy_to_user(ubuf, READ_ONCE(client->rbuf.rpos), n)) {
-			ret = -EFAULT;
-			goto out;
-		}
-	} else {
-		memcpy(kbuf, READ_ONCE(client->rbuf.rpos), n);
-	}
-
-	if (sbefifo_buf_readnb(&client->rbuf, n)) {
-		xfr = list_first_entry_or_null(&client->xfrs,
-					       struct sbefifo_xfr, client);
-		if (!xfr) {
-			/* should be impossible to not have an xfr here */
-			WARN_ONCE(1, "no xfr in queue");
-			ret = -EPROTO;
-			goto out;
-		}
-
-		if (!test_bit(SBEFIFO_XFR_COMPLETE, &xfr->flags)) {
-			/* Fill the read buffer back up. */
-			sbefifo_get(sbefifo);
-			if (!queue_work(sbefifo_wq, &sbefifo->work.work))
-				sbefifo_put(sbefifo);
-		} else {
-			list_del(&xfr->client);
-			kfree(xfr);
-			wake_up_interruptible(&sbefifo->wait);
-		}
-	}
-
-	ret = n;
-
-out:
-	sbefifo_put_client(client);
-	return ret;
-}
-
-static ssize_t sbefifo_read(struct file *file, char __user *buf, size_t len,
-			    loff_t *offset)
-{
-	struct sbefifo_client *client = file->private_data;
-
-	return sbefifo_read_common(client, buf, NULL, len);
-}
-
-static bool sbefifo_write_ready(struct sbefifo *sbefifo,
-				struct sbefifo_xfr *xfr,
-				struct sbefifo_client *client, size_t *n)
-{
-	struct sbefifo_xfr *next = list_first_entry_or_null(&client->xfrs,
-							    struct sbefifo_xfr,
-							    client);
-
-	*n = sbefifo_buf_nbwriteable(&client->wbuf);
-	return READ_ONCE(sbefifo->rc) || (next == xfr && *n);
-}
-
-static ssize_t sbefifo_write_common(struct sbefifo_client *client,
-				    const char __user *ubuf, const char *kbuf,
-				    size_t len)
-{
-	struct sbefifo *sbefifo = client->dev;
-	struct sbefifo_xfr *xfr;
-	ssize_t ret = 0;
-	size_t n;
-
-	if ((len >> 2) << 2 != len)
-		return -EINVAL;
-
-	if (!len)
-		return 0;
-
-	sbefifo_get_client(client);
-	n = sbefifo_buf_nbwriteable(&client->wbuf);
-
-	mutex_lock(&sbefifo->list_lock);
- 
-	/* next xfr to be executed */
-	xfr = list_first_entry_or_null(&sbefifo->xfrs, struct sbefifo_xfr,
-				       xfrs);
-
-	if ((client->f_flags & O_NONBLOCK) && xfr && n < len) {
-		mutex_unlock(&sbefifo->list_lock);
-		ret = -EAGAIN;
-		goto out;
-	}
-
-	xfr = sbefifo_enq_xfr(client);		/* this xfr queued up */
-	if (IS_ERR(xfr)) {
-		mutex_unlock(&sbefifo->list_lock);
-		ret = PTR_ERR(xfr);
-		goto out;
-	}
-
-	mutex_unlock(&sbefifo->list_lock);
-
-	/*
-	 * Partial writes are not really allowed in that EOT is sent exactly
-	 * once per write.
-	 */
-	while (len) {
-		if (wait_event_interruptible(sbefifo->wait,
-					     sbefifo_write_ready(sbefifo, xfr,
-								 client,
-								 &n))) {
-			set_bit(SBEFIFO_XFR_CANCEL, &xfr->flags);
-			sbefifo_get(sbefifo);
-			if (!queue_work(sbefifo_wq, &sbefifo->work.work))
-				sbefifo_put(sbefifo);
-
-			ret = -ERESTARTSYS;
-			goto out;
-		}
-
-		if (sbefifo->rc) {
-			INIT_LIST_HEAD(&client->xfrs);
-			ret = sbefifo->rc;
-			goto out;
-		}
-
-		n = min_t(size_t, n, len);
-
-		if (ubuf) {
-			if (copy_from_user(READ_ONCE(client->wbuf.wpos), ubuf,
-			    n)) {
-				set_bit(SBEFIFO_XFR_CANCEL, &xfr->flags);
-				sbefifo_get(sbefifo);
-				if (!queue_work(sbefifo_wq,
-						&sbefifo->work.work))
-					sbefifo_put(sbefifo);
-				ret = -EFAULT;
-				goto out;
-			}
-
-			ubuf += n;
-		} else {
-			memcpy(READ_ONCE(client->wbuf.wpos), kbuf, n);
-			kbuf += n;
-		}
-
-		sbefifo_buf_wrotenb(&client->wbuf, n);
-		len -= n;
-		ret += n;
-
-		/*
-		 * Set this before starting timer to avoid race condition on
-		 * this flag with the timer function writer.
-		 */
-		if (!len)
-			set_bit(SBEFIFO_XFR_WRITE_DONE, &xfr->flags);
-
-		/* Drain the write buffer. */
-		sbefifo_get(sbefifo);
-		if (!queue_work(sbefifo_wq, &sbefifo->work.work))
-			sbefifo_put(sbefifo);
-	}
-
-out:
-	sbefifo_put_client(client);
-	return ret;
-}
-
-static ssize_t sbefifo_write(struct file *file, const char __user *buf,
-			     size_t len, loff_t *offset)
-{
-	struct sbefifo_client *client = file->private_data;
-
-	return sbefifo_write_common(client, buf, NULL, len);
-}
-
-static int sbefifo_release(struct inode *inode, struct file *file)
-{
-	struct sbefifo_client *client = file->private_data;
-	struct sbefifo *sbefifo = client->dev;
-
-	sbefifo_put_client(client);
-
-	return READ_ONCE(sbefifo->rc);
-}
-
-static const struct file_operations sbefifo_fops = {
-	.owner		= THIS_MODULE,
-	.open		= sbefifo_open,
-	.read		= sbefifo_read,
-	.write		= sbefifo_write,
-	.poll		= sbefifo_poll,
-	.release	= sbefifo_release,
-};
-
-struct sbefifo_client *sbefifo_drv_open(struct device *dev,
-					unsigned long flags)
-{
-	struct sbefifo_client *client;
-	struct sbefifo *sbefifo = dev_get_drvdata(dev);
-
-	if (!sbefifo)
-		return NULL;
-
-	client = sbefifo_new_client(sbefifo);
-	if (client)
-		client->f_flags = flags;
-
-	return client;
-}
-EXPORT_SYMBOL_GPL(sbefifo_drv_open);
-
-int sbefifo_drv_read(struct sbefifo_client *client, char *buf, size_t len)
-{
-	return sbefifo_read_common(client, NULL, buf, len);
-}
-EXPORT_SYMBOL_GPL(sbefifo_drv_read);
-
-int sbefifo_drv_write(struct sbefifo_client *client, const char *buf,
-		      size_t len)
-{
-	return sbefifo_write_common(client, NULL, buf, len);
-}
-EXPORT_SYMBOL_GPL(sbefifo_drv_write);
-
-void sbefifo_drv_release(struct sbefifo_client *client)
-{
-	if (!client)
-		return;
-
-	sbefifo_put_client(client);
-}
-EXPORT_SYMBOL_GPL(sbefifo_drv_release);
-
-static int sbefifo_unregister_child(struct device *dev, void *data)
-{
-	struct platform_device *child = to_platform_device(dev);
-
-	of_device_unregister(child);
-	if (dev->of_node)
-		of_node_clear_flag(dev->of_node, OF_POPULATED);
-
-	return 0;
-}
-
-static int sbefifo_request_reset(struct sbefifo *sbefifo)
-{
-	int ret;
-	u32 status;
-	unsigned long start;
-	const unsigned int wait_time = 5;	/* jiffies */
-	const unsigned long timeout = msecs_to_jiffies(250);
-
-	ret = sbefifo_outw(sbefifo, SBEFIFO_UP | SBEFIFO_REQ_RESET, 1);
-	if (ret)
-		return ret;
-
-	start = jiffies;
-
-	do {
-		ret = sbefifo_inw(sbefifo, SBEFIFO_UP | SBEFIFO_STS, &status);
-		if (ret)
-			return ret;
-
-		if (!(status & SBEFIFO_STS_RESET_REQ))
-			return 0;
-
-		set_current_state(TASK_INTERRUPTIBLE);
-		if (schedule_timeout(wait_time) > 0)
-			return -EINTR;
-	} while (time_after(start + timeout, jiffies));
-
-	return -ETIME;
-}
-
-static int sbefifo_probe(struct device *dev)
-{
-	struct fsi_device *fsi_dev = to_fsi_dev(dev);
-	struct sbefifo *sbefifo;
-	struct device_node *np;
-	struct platform_device *child;
-	char child_name[32];
-	u32 up, down;
-	int ret, child_idx = 0;
-
-	dev_dbg(dev, "Found sbefifo device\n");
-	sbefifo = kzalloc(sizeof(*sbefifo), GFP_KERNEL);
-	if (!sbefifo)
-		return -ENOMEM;
-
-	sbefifo->fsi_dev = fsi_dev;
-
-	ret = sbefifo_inw(sbefifo, SBEFIFO_UP | SBEFIFO_STS, &up);
-	if (ret)
-		return ret;
-
-	ret = sbefifo_inw(sbefifo, SBEFIFO_DWN | SBEFIFO_STS, &down);
-	if (ret)
-		return ret;
-
-	if (!(up & SBEFIFO_EMPTY) || !(down & SBEFIFO_EMPTY)) {
-		ret = sbefifo_request_reset(sbefifo);
-		if (ret) {
-			dev_err(dev,
-				"fifos weren't empty and failed the reset\n");
-			return ret;
-		}
-	}
-
-	mutex_init(&sbefifo->list_lock);
-	mutex_init(&sbefifo->sbefifo_lock);
-	kref_init(&sbefifo->kref);
-	init_waitqueue_head(&sbefifo->wait);
-	INIT_LIST_HEAD(&sbefifo->xfrs);
-
-	sbefifo->idx = ida_simple_get(&sbefifo_ida, 1, INT_MAX, GFP_KERNEL);
-	snprintf(sbefifo->name, sizeof(sbefifo->name), "sbefifo%d",
-		 sbefifo->idx);
-
-	/* This bit of silicon doesn't offer any interrupts... */
-	INIT_DELAYED_WORK(&sbefifo->work, sbefifo_worker);
-
-	sbefifo->mdev.minor = MISC_DYNAMIC_MINOR;
-	sbefifo->mdev.fops = &sbefifo_fops;
-	sbefifo->mdev.name = sbefifo->name;
-	sbefifo->mdev.parent = dev;
-	ret = misc_register(&sbefifo->mdev);
-	if (ret) {
-		dev_err(dev, "failed to register miscdevice: %d\n", ret);
-		ida_simple_remove(&sbefifo_ida, sbefifo->idx);
-		sbefifo_put(sbefifo);
-		return ret;
-	}
-
-	/* create platform devs for dts child nodes (occ, etc) */
-	for_each_available_child_of_node(dev->of_node, np) {
-		snprintf(child_name, sizeof(child_name), "%s-dev%d",
-			 sbefifo->name, child_idx++);
-		child = of_platform_device_create(np, child_name, dev);
-		if (!child)
-			dev_warn(dev, "failed to create child %s dev\n",
-				 child_name);
-	}
-
-	dev_set_drvdata(dev, sbefifo);
-
-	return 0;
-}
-
-static int sbefifo_remove(struct device *dev)
-{
-	struct sbefifo *sbefifo = dev_get_drvdata(dev);
-	struct sbefifo_xfr *xfr, *tmp;
-
-	/* lock the sbefifo so to prevent deleting an ongoing xfr */
-	mutex_lock(&sbefifo->sbefifo_lock);
-	mutex_lock(&sbefifo->list_lock);
-
-	WRITE_ONCE(sbefifo->rc, -ENODEV);
-	list_for_each_entry_safe(xfr, tmp, &sbefifo->xfrs, xfrs) {
-		list_del(&xfr->xfrs);
-		kfree(xfr);
-	}
-
-	INIT_LIST_HEAD(&sbefifo->xfrs);
-
-	mutex_unlock(&sbefifo->list_lock);
-	mutex_unlock(&sbefifo->sbefifo_lock);
-
-	wake_up_all(&sbefifo->wait);
-
-	misc_deregister(&sbefifo->mdev);
-	device_for_each_child(dev, NULL, sbefifo_unregister_child);
-
-	ida_simple_remove(&sbefifo_ida, sbefifo->idx);
-
-	if (cancel_delayed_work_sync(&sbefifo->work))
-		sbefifo_put(sbefifo);
-
-	sbefifo_put(sbefifo);
-
-	return 0;
-}
-
-static struct fsi_device_id sbefifo_ids[] = {
-	{
-		.engine_type = FSI_ENGID_SBE,
-		.version = FSI_VERSION_ANY,
-	},
-	{ 0 }
-};
-
-static struct fsi_driver sbefifo_drv = {
-	.id_table = sbefifo_ids,
-	.drv = {
-		.name = DEVICE_NAME,
-		.bus = &fsi_bus_type,
-		.probe = sbefifo_probe,
-		.remove = sbefifo_remove,
-	}
-};
-
-static int sbefifo_init(void)
-{
-	sbefifo_wq = create_singlethread_workqueue("sbefifo");
-	if (!sbefifo_wq)
-		return -ENOMEM;
-
-	return fsi_driver_register(&sbefifo_drv);
-}
-
-static void sbefifo_exit(void)
-{
-	destroy_workqueue(sbefifo_wq);
-
-	fsi_driver_unregister(&sbefifo_drv);
-
-	ida_destroy(&sbefifo_ida);
-}
-
-module_init(sbefifo_init);
-module_exit(sbefifo_exit);
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Brad Bishop <bradleyb@fuzziesquirrel.com>");
-MODULE_AUTHOR("Eddie James <eajames@linux.vnet.ibm.com>");
-MODULE_DESCRIPTION("Linux device interface to the POWER Self Boot Engine");
diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
index ab5c3e965ccb..ca6d25ae9da8 100644
--- a/drivers/hwmon/occ/Makefile
+++ b/drivers/hwmon/occ/Makefile
@@ -1,7 +1,7 @@
 occ-hwmon-objs := common.o
 
 ifeq ($(CONFIG_SENSORS_OCC_P9_SBE), y)
-occ-hwmon-objs += p9_sbe.o
+#occ-hwmon-objs += p9_sbe.o
 endif
 
 ifeq ($(CONFIG_SENSORS_OCC_P8_I2C), y)
diff --git a/include/linux/fsi-sbefifo.h b/include/linux/fsi-sbefifo.h
deleted file mode 100644
index 8e55891963a5..000000000000
--- a/include/linux/fsi-sbefifo.h
+++ /dev/null
@@ -1,30 +0,0 @@
-/*
- * SBEFIFO FSI Client device driver
- *
- * Copyright (C) IBM Corporation 2017
- *
- * 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
- * MERGCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#ifndef LINUX_FSI_SBEFIFO_H
-#define LINUX_FSI_SBEFIFO_H
-
-struct device;
-struct sbefifo_client;
-
-extern struct sbefifo_client *sbefifo_drv_open(struct device *dev,
-					       unsigned long flags);
-extern int sbefifo_drv_read(struct sbefifo_client *client, char *buf,
-			    size_t len);
-extern int sbefifo_drv_write(struct sbefifo_client *client, const char *buf,
-			     size_t len);
-extern void sbefifo_drv_release(struct sbefifo_client *client);
-
-#endif /* LINUX_FSI_SBEFIFO_H */
-- 
2.17.0

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

* [PATCH linux dev-4.13 07/10] fsi/sbefifo: Add driver for the SBE FIFO
  2018-05-24  5:14 [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Benjamin Herrenschmidt
                   ` (4 preceding siblings ...)
  2018-05-24  5:14 ` [PATCH linux dev-4.13 06/10] fsi: Remove old sbefifo driver Benjamin Herrenschmidt
@ 2018-05-24  5:14 ` Benjamin Herrenschmidt
  2018-05-24  5:14 ` [PATCH linux dev-4.13 08/10] fsi/fsi-occ: Simple conversion to new sbefifo driver Benjamin Herrenschmidt
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-24  5:14 UTC (permalink / raw)
  To: openbmc

This driver provides an in-kernel and a user API for accessing
the command FIFO of the SBE (Self Boot Engine) of the POWER9
processor, via the FSI bus.

It provides an in-kernel interface to submit command and receive
responses, along with a helper to locate and analyse the response
status block. It's a simple synchronous submit() type API.

The user interface uses the write/read interface that an earlier
version of this driver already provided, however it has some
specific limitations in order to keep the driver simple and
avoid using up a lot of kernel memory:

 - The user should perform a single write() with the command and
   a single read() to get the response (with a buffer big enough
   to hold the entire response).

 - On a write() the command is simply "stored" into a kernel buffer,
   it is submitted as one operation on the subsequent read(). This
   allows to have the code write directly from the FIFO into the user
   buffer and avoid hogging the SBE between the write() and read()
   syscall as it's critical that the SBE be freed asap to respond
   to the host. An extra write() will simply replace the previously
   written command.

 - A write of a single 4 bytes containing the value 0x52534554
   in big endian will trigger a reset request. No read is necessary,
   the write() call will return when the reset has been acknowledged
   or times out.

 - The command is limited to 4K bytes.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

v2. Return -EAGAIN on read() when there's no pending message as
    to be compatible with existing userspace (cronus) expectations

v3. Don't print "Initial HW cleanup failed" when the error is
    -ESHUTDOWN (is the host is not booted).

v4. Print FFDC packages on command errors and collect & print
    async FFDC when available.
    Use symbolic constants for commands
    Use an iovec iterator instead of set_fs/put_user
---
 drivers/fsi/Makefile        |    2 +-
 drivers/fsi/fsi-sbefifo.c   | 1007 +++++++++++++++++++++++++++++++++++
 include/linux/fsi-sbefifo.h |   33 ++
 3 files changed, 1041 insertions(+), 1 deletion(-)
 create mode 100644 drivers/fsi/fsi-sbefifo.c
 create mode 100644 include/linux/fsi-sbefifo.h

diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
index 61e8e420c7ed..f331e929423c 100644
--- a/drivers/fsi/Makefile
+++ b/drivers/fsi/Makefile
@@ -3,5 +3,5 @@ obj-$(CONFIG_FSI) += fsi-core.o
 obj-$(CONFIG_FSI_MASTER_HUB) += fsi-master-hub.o
 obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
 obj-$(CONFIG_FSI_SCOM) += fsi-scom.o
-#obj-$(CONFIG_FSI_SBEFIFO) += fsi-sbefifo.o
+obj-$(CONFIG_FSI_SBEFIFO) += fsi-sbefifo.o
 #obj-$(CONFIG_FSI_OCC) += fsi-occ.o
diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
new file mode 100644
index 000000000000..0cb22d0342b1
--- /dev/null
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -0,0 +1,1007 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) IBM Corporation 2017
+ *
+ * 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
+ * MERGCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/fsi.h>
+#include <linux/fsi-sbefifo.h>
+#include <linux/idr.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/delay.h>
+#include <linux/uio.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/sbefifo.h>
+
+/*
+ * The SBEFIFO is a pipe-like FSI device for communicating with
+ * the self boot engine on POWER processors.
+ */
+
+#define DEVICE_NAME		"sbefifo"
+#define FSI_ENGID_SBE		0x22
+
+/*
+ * Register layout
+ */
+
+/* Register banks */
+#define SBEFIFO_UP		0x00		/* FSI -> Host */
+#define SBEFIFO_DOWN		0x40		/* Host -> FSI */
+
+/* Per-bank registers */
+#define SBEFIFO_FIFO		0x00		/* The FIFO itself */
+#define SBEFIFO_STS		0x04		/* Status register */
+#define   SBEFIFO_STS_PARITY_ERR	0x20000000
+#define   SBEFIFO_STS_RESET_REQ		0x02000000
+#define   SBEFIFO_STS_GOT_EOT		0x00800000
+#define   SBEFIFO_STS_MAX_XFER_LIMIT	0x00400000
+#define   SBEFIFO_STS_FULL		0x00200000
+#define   SBEFIFO_STS_EMPTY		0x00100000
+#define   SBEFIFO_STS_ECNT_MASK		0x000f0000
+#define   SBEFIFO_STS_ECNT_SHIFT	16
+#define   SBEFIFO_STS_VALID_MASK	0x0000ff00
+#define   SBEFIFO_STS_VALID_SHIFT	8
+#define   SBEFIFO_STS_EOT_MASK		0x000000ff
+#define   SBEFIFO_STS_EOT_SHIFT		0
+#define SBEFIFO_EOT_RAISE	0x08		/* (Up only) Set End Of Transfer */
+#define SBEFIFO_REQ_RESET	0x0C		/* (Up only) Reset Request */
+#define SBEFIFO_PERFORM_RESET	0x10		/* (Down only) Perform Reset */
+#define SBEFIFO_EOT_ACK		0x14		/* (Down only) Acknowledge EOT */
+#define SBEFIFO_DOWN_MAX	0x18		/* (Down only) Max transfer */
+
+/* CFAM GP Mailbox SelfBoot Message register */
+#define CFAM_GP_MBOX_SBM_ADDR	0x2824	/* Converted 0x2809 */
+
+#define CFAM_SBM_SBE_BOOTED		0x80000000
+#define CFAM_SBM_SBE_ASYNC_FFDC		0x40000000
+#define CFAM_SBM_SBE_STATE_MASK		0x00f00000
+#define CFAM_SBM_SBE_STATE_SHIFT	20
+
+enum sbe_state
+{
+	SBE_STATE_UNKNOWN = 0x0, // Unkown, initial state
+	SBE_STATE_IPLING  = 0x1, // IPL'ing - autonomous mode (transient)
+	SBE_STATE_ISTEP   = 0x2, // ISTEP - Running IPL by steps (transient)
+	SBE_STATE_MPIPL   = 0x3, // MPIPL
+	SBE_STATE_RUNTIME = 0x4, // SBE Runtime
+	SBE_STATE_DMT     = 0x5, // Dead Man Timer State (transient)
+	SBE_STATE_DUMP    = 0x6, // Dumping
+	SBE_STATE_FAILURE = 0x7, // Internal SBE failure
+	SBE_STATE_QUIESCE = 0x8, // Final state - needs SBE reset to get out
+};
+
+/* FIFO depth */
+#define SBEFIFO_FIFO_DEPTH		8
+
+/* Helpers */
+#define sbefifo_empty(sts)	((sts) & SBEFIFO_STS_EMPTY)
+#define sbefifo_full(sts)	((sts) & SBEFIFO_STS_FULL)
+#define sbefifo_parity_err(sts)	((sts) & SBEFIFO_STS_PARITY_ERR)
+#define sbefifo_populated(sts)	(((sts) & SBEFIFO_STS_ECNT_MASK) >> SBEFIFO_STS_ECNT_SHIFT)
+#define sbefifo_vacant(sts)	(SBEFIFO_FIFO_DEPTH - sbefifo_populated(sts))
+#define sbefifo_eot_set(sts)	(((sts) & SBEFIFO_STS_EOT_MASK) >> SBEFIFO_STS_EOT_SHIFT)
+
+/* Reset request timeout in ms */
+#define SBEFIFO_RESET_TIMEOUT		10000
+
+/* Timeouts for commands in ms */
+#define SBEFIFO_TIMEOUT_START_CMD	10000
+#define SBEFIFO_TIMEOUT_IN_CMD		1000
+#define SBEFIFO_TIMEOUT_START_RSP	10000
+#define SBEFIFO_TIMEOUT_IN_RSP		1000
+
+/* Other constants */
+#define SBEFIFO_MAX_CMD_LEN		PAGE_SIZE
+#define SBEFIFO_RESET_MAGIC		0x52534554 /* "RSET" */
+
+struct sbefifo {
+	uint32_t		magic;
+#define SBEFIFO_MAGIC		0x53424546 /* "SBEF" */
+	struct fsi_device	*fsi_dev;
+	struct miscdevice	mdev;
+	struct mutex		lock;
+	char			name[32];
+	int			idx;
+	bool			broken;
+	bool			async_ffdc;
+};
+
+struct sbefifo_user {
+	struct sbefifo		*sbefifo;
+	struct mutex		file_lock;
+	void			*pending_cmd;
+	size_t			pending_len;
+};
+
+static DEFINE_IDA(sbefifo_ida);
+static DEFINE_MUTEX(sbefifo_ffdc_mutex);
+
+
+static void sbefifo_dump_ffdc(struct device *dev, const __be32 *ffdc,
+			      size_t ffdc_sz, bool internal)
+{
+	int pack = 0;
+#define FFDC_LSIZE	60
+	static char ffdc_line[FFDC_LSIZE];
+	char *p = ffdc_line;
+
+	mutex_lock(&sbefifo_ffdc_mutex);
+	while (ffdc_sz) {
+		u32 w0, w1, w2, i;
+		if (ffdc_sz < 3) {
+			dev_err(dev, "SBE invalid FFDC package size %zd\n", ffdc_sz);
+			return;
+		}
+		w0 = be32_to_cpu(*(ffdc++));
+		w1 = be32_to_cpu(*(ffdc++));
+		w2 = be32_to_cpu(*(ffdc++));
+		ffdc_sz -= 3;
+		if ((w0 >> 16) != 0xFFDC) {
+			dev_err(dev, "SBE invalid FFDC package signature %08x %08x %08x\n",
+				w0, w1, w2);
+			break;
+		}
+		w0 &= 0xffff;
+		if (w0 > ffdc_sz) {
+			dev_err(dev, "SBE FFDC package len %d words but only %zd remaining\n",
+				w0, ffdc_sz);
+			w0 = ffdc_sz;
+			break;
+		}
+		if (internal) {
+			dev_warn(dev, "+---- SBE FFDC package %d for async err -----+\n",
+				 pack++);
+		} else {
+			dev_warn(dev, "+---- SBE FFDC package %d for cmd %02x:%02x -----+\n",
+				 pack++, (w1 >> 8) & 0xff, w1 & 0xff);
+		}
+		dev_warn(dev, "| Response code: %08x                   |\n", w2);
+		dev_warn(dev, "|-------------------------------------------|\n");
+		for (i = 0; i < w0; i++) {
+			if ((i & 3) == 0) {
+				p = ffdc_line;
+				p += sprintf(p, "| %04x:", i << 4);
+			}
+			p += sprintf(p, " %08x", be32_to_cpu(*(ffdc++)));
+			ffdc_sz--;
+			if ((i & 3) == 3 || i == (w0 - 1)) {
+				while ((i & 3) < 3) {
+					p += sprintf(p, "         ");
+					i++;
+				}
+				dev_warn(dev, "%s |\n", ffdc_line);
+			}
+		}
+		dev_warn(dev, "+-------------------------------------------+\n");
+	}
+}
+
+int sbefifo_parse_status(struct device *dev, u16 cmd, __be32 *response,
+			 size_t resp_len, size_t *data_len)
+{
+	u32 dh, s0, s1;
+	size_t ffdc_sz;
+
+	if (resp_len < 3) {
+		pr_debug("sbefifo: cmd %04x, response too small: %zd\n",
+			 cmd, resp_len);
+		return -ENXIO;
+	}
+	dh = be32_to_cpu(response[resp_len - 1]);
+	if (dh > resp_len || dh < 3) {
+		dev_err(dev, "SBE cmd %02x:%02x status offset out of range: %d/%zd\n",
+			cmd >> 8, cmd & 0xff, dh, resp_len);
+		return -ENXIO;
+	}
+	s0 = be32_to_cpu(response[resp_len - dh]);
+	s1 = be32_to_cpu(response[resp_len - dh + 1]);
+	if (((s0 >> 16) != 0xC0DE) || ((s0 & 0xffff) != cmd)) {
+		dev_err(dev, "SBE cmd %02x:%02x, status signature invalid: 0x%08x 0x%08x\n",
+			cmd >> 8, cmd & 0xff, s0, s1);
+		return -ENXIO;
+	}
+	if (s1 != 0) {
+		ffdc_sz = dh - 3;
+		dev_warn(dev, "SBE error cmd %02x:%02x status=%04x:%04x\n",
+			 cmd >> 8, cmd & 0xff, s1 >> 16, s1 & 0xffff);
+		if (ffdc_sz)
+			sbefifo_dump_ffdc(dev, &response[resp_len - dh + 2],
+					  ffdc_sz, false);
+	}
+	if (data_len)
+		*data_len = resp_len - dh;
+
+	/*
+	 * Primary status don't have the top bit set, so can't be confused with
+	 * Linux negative error codes, so return the status word whole.
+	 */
+	return s1;
+}
+EXPORT_SYMBOL_GPL(sbefifo_parse_status);
+
+static int sbefifo_regr(struct sbefifo *sbefifo, int reg, u32 *word)
+{
+	__be32 raw_word;
+	int rc;
+
+	rc = fsi_device_read(sbefifo->fsi_dev, reg, &raw_word,
+			     sizeof(raw_word));
+	if (rc)
+		return rc;
+
+	*word = be32_to_cpu(raw_word);
+
+	return 0;
+}
+
+static int sbefifo_regw(struct sbefifo *sbefifo, int reg, u32 word)
+{
+	__be32 raw_word = cpu_to_be32(word);
+
+	return fsi_device_write(sbefifo->fsi_dev, reg, &raw_word,
+				sizeof(raw_word));
+}
+
+static int sbefifo_check_sbe_state(struct sbefifo *sbefifo)
+{
+	__be32 raw_word;
+	u32 sbm;
+	int rc;
+
+	rc = fsi_slave_read(sbefifo->fsi_dev->slave, CFAM_GP_MBOX_SBM_ADDR,
+			    &raw_word, sizeof(raw_word));
+	if (rc)
+		return rc;
+	sbm = be32_to_cpu(raw_word);
+
+	/* SBE booted at all ? */
+	if (!(sbm & CFAM_SBM_SBE_BOOTED))
+		return -ESHUTDOWN;
+
+	/* Check its state */
+	switch ((sbm & CFAM_SBM_SBE_STATE_MASK) >> CFAM_SBM_SBE_STATE_SHIFT) {
+	case SBE_STATE_UNKNOWN:
+		return -ESHUTDOWN;
+	case SBE_STATE_IPLING:
+	case SBE_STATE_ISTEP:
+	case SBE_STATE_MPIPL:
+	case SBE_STATE_DMT:
+		return -EBUSY;
+	case SBE_STATE_RUNTIME:
+	case SBE_STATE_DUMP: /* Not sure about that one */
+		break;
+	case SBE_STATE_FAILURE:
+	case SBE_STATE_QUIESCE:
+		return -ESHUTDOWN;
+	}
+
+	/* Is there async FFDC available ? Remember it */
+	if (sbm & CFAM_SBM_SBE_ASYNC_FFDC)
+		sbefifo->async_ffdc = true;
+
+	return 0;
+}
+
+/* Don't flip endianness of data to/from FIFO, just pass through. */
+static int sbefifo_down_read(struct sbefifo *sbefifo, __be32 *word)
+{
+	return fsi_device_read(sbefifo->fsi_dev, SBEFIFO_DOWN, word,
+			       sizeof(*word));
+}
+
+static int sbefifo_up_write(struct sbefifo *sbefifo, __be32 word)
+{
+	return fsi_device_write(sbefifo->fsi_dev, SBEFIFO_UP, &word,
+				sizeof(word));
+}
+
+static int sbefifo_request_reset(struct sbefifo *sbefifo)
+{
+	struct device *dev = &sbefifo->fsi_dev->dev;
+	u32 status, timeout;
+	int rc;
+
+	dev_dbg(dev, "Requesting FIFO reset\n");
+
+	/* Mark broken first, will be cleared if reset succeeds */
+	sbefifo->broken = true;
+
+	/* Send reset request */
+	rc = sbefifo_regw(sbefifo, SBEFIFO_UP | SBEFIFO_REQ_RESET, 1);
+	if (rc) {
+		dev_err(dev, "Sending reset request failed, rc=%d\n", rc);
+		return rc;
+	}
+
+	/* Wait for it to complete */
+	for (timeout = 0; timeout < SBEFIFO_RESET_TIMEOUT; timeout++) {
+		rc = sbefifo_regr(sbefifo, SBEFIFO_UP | SBEFIFO_STS, &status);
+		if (rc) {
+			dev_err(dev, "Failed to read UP fifo status during reset"
+				" , rc=%d\n", rc);
+			return rc;
+		}
+
+		if (!(status & SBEFIFO_STS_RESET_REQ)) {
+			dev_dbg(dev, "FIFO reset done\n");
+			sbefifo->broken = false;
+			return 0;
+		}
+
+		msleep(1);
+	}
+	dev_err(dev, "FIFO reset timed out\n");
+
+	return -ETIMEDOUT;
+}
+
+static int sbefifo_cleanup_hw(struct sbefifo *sbefifo)
+{
+	struct device *dev = &sbefifo->fsi_dev->dev;
+	u32 up_status, down_status;
+	bool need_reset = false;
+	int rc;
+
+	rc = sbefifo_check_sbe_state(sbefifo);
+	if (rc) {
+		dev_dbg(dev, "SBE state=%d\n", rc);
+		return rc;
+	}
+
+	/* If broken, we don't need to look at status, go straight to reset */
+	if (sbefifo->broken)
+		goto do_reset;
+
+	rc = sbefifo_regr(sbefifo, SBEFIFO_UP | SBEFIFO_STS, &up_status);
+	if (rc) {
+		dev_err(dev, "Cleanup: Reading UP status failed, rc=%d\n", rc);
+
+		/* Will try reset again on next attempt at using it */
+		sbefifo->broken = true;
+		return rc;
+	}
+
+	rc = sbefifo_regr(sbefifo, SBEFIFO_DOWN | SBEFIFO_STS, &down_status);
+	if (rc) {
+		dev_err(dev, "Cleanup: Reading DOWN status failed, rc=%d\n", rc);
+
+		/* Will try reset again on next attempt at using it */
+		sbefifo->broken = true;
+		return rc;
+	}
+
+	/* The FIFO already contains a reset request from the SBE ? */
+	if (down_status & SBEFIFO_STS_RESET_REQ) {
+		dev_info(dev, "Cleanup: FIFO reset request set, resetting\n");
+		rc = sbefifo_regw(sbefifo, SBEFIFO_UP, SBEFIFO_PERFORM_RESET);
+		if (rc) {
+			sbefifo->broken = true;
+			dev_err(dev, "Cleanup: Reset reg write failed, rc=%d\n", rc);
+			return rc;
+		}
+		sbefifo->broken = false;
+		return 0;
+	}
+
+	/* Parity error on either FIFO ? */
+	if ((up_status | down_status) & SBEFIFO_STS_PARITY_ERR)
+		need_reset = true;
+
+	/* Either FIFO not empty ? */
+	if (!((up_status & down_status) & SBEFIFO_STS_EMPTY))
+		need_reset = true;
+
+	if (!need_reset)
+		return 0;
+
+	dev_info(dev, "Cleanup: FIFO not clean (up=0x%08x down=0x%08x)\n",
+		 up_status, down_status);
+
+ do_reset:
+
+	/* Mark broken, will be cleared if/when reset succeeds */
+	return sbefifo_request_reset(sbefifo);
+}
+
+static int sbefifo_wait(struct sbefifo *sbefifo, bool up,
+			u32 *status, unsigned long timeout)
+{
+	struct device *dev = &sbefifo->fsi_dev->dev;
+	unsigned long end_time;
+	bool ready = false;
+	u32 addr, sts = 0;
+	int rc;
+
+	dev_vdbg(dev, "Wait on %s fifo...\n", up ? "up" : "down");
+
+	addr = (up ? SBEFIFO_UP : SBEFIFO_DOWN) | SBEFIFO_STS;
+
+	end_time = jiffies + timeout;
+	while (!time_after(jiffies, end_time)) {
+		cond_resched();
+		rc = sbefifo_regr(sbefifo, addr, &sts);
+		if (rc < 0) {
+			dev_err(dev, "FSI error %d reading status register\n", rc);
+			return rc;
+		}
+		if (!up && sbefifo_parity_err(sts)) {
+			dev_err(dev, "Parity error in DOWN FIFO\n");
+			return -ENXIO;
+		}
+		ready = !(up ? sbefifo_full(sts) : sbefifo_empty(sts));
+		if (ready)
+			break;
+	}
+	if (!ready) {
+		dev_err(dev, "%s FIFO Timeout ! status=%08x\n", up ? "UP" : "DOWN", sts);
+		return -ETIMEDOUT;
+	}
+	dev_vdbg(dev, "End of wait status: %08x\n", sts);
+
+	*status = sts;
+
+	return 0;
+}
+
+static int sbefifo_send_command(struct sbefifo *sbefifo,
+				const __be32 *command, size_t cmd_len)
+{
+	struct device *dev = &sbefifo->fsi_dev->dev;
+	size_t len, chunk, vacant = 0, remaining = cmd_len;
+	unsigned long timeout;
+	u32 status;
+	int rc;
+
+	dev_vdbg(dev, "sending command (%zd words, cmd=%04x)\n",
+		 cmd_len, be32_to_cpu(command[1]));
+
+	/* As long as there's something to send */
+	timeout = msecs_to_jiffies(SBEFIFO_TIMEOUT_START_CMD);
+	while (remaining) {
+		/* Wait for room in the FIFO */
+		rc = sbefifo_wait(sbefifo, true, &status, timeout);
+		if (rc < 0)
+			return rc;
+		timeout = msecs_to_jiffies(SBEFIFO_TIMEOUT_IN_CMD);
+
+		vacant = sbefifo_vacant(status);
+		len = chunk = min(vacant, remaining);
+
+		dev_vdbg(dev, "  status=%08x vacant=%zd chunk=%zd\n",
+			 status, vacant, chunk);
+
+		/* Write as much as we can */
+		while (len--) {
+			rc = sbefifo_up_write(sbefifo, *(command++));
+			if (rc) {
+				dev_err(dev, "FSI error %d writing UP FIFO\n", rc);
+				return rc;
+			}
+		}
+		remaining -= chunk;
+		vacant -= chunk;
+	}
+
+	/* If there's no room left, wait for some to write EOT */
+	if (!vacant) {
+		rc = sbefifo_wait(sbefifo, true, &status, timeout);
+		if (rc)
+			return rc;
+	}
+
+	/* Send an EOT */
+	rc = sbefifo_regw(sbefifo, SBEFIFO_UP | SBEFIFO_EOT_RAISE, 0);
+	if (rc)
+		dev_err(dev, "FSI error %d writing EOT\n", rc);
+	return rc;
+}
+
+static int sbefifo_read_response(struct sbefifo *sbefifo, struct iov_iter *response)
+{
+	struct device *dev = &sbefifo->fsi_dev->dev;
+	u32 data, status, eot_set;
+	unsigned long timeout;
+	bool overflow = false;
+	size_t len;
+	int rc;
+
+	dev_vdbg(dev, "reading response, buflen = %zd\n", iov_iter_count(response));
+
+	timeout = msecs_to_jiffies(SBEFIFO_TIMEOUT_START_RSP);
+	for (;;) {
+		/* Grab FIFO status (this will handle parity errors) */
+		rc = sbefifo_wait(sbefifo, false, &status, timeout);
+		if (rc < 0)
+			return rc;
+		timeout = msecs_to_jiffies(SBEFIFO_TIMEOUT_IN_RSP);
+
+		/* Decode status */
+		len = sbefifo_populated(status);
+		eot_set = sbefifo_eot_set(status);
+
+		dev_vdbg(dev, "  chunk size %zd eot_set=0x%x\n", len, eot_set);
+
+		/* Go through the chunk */
+		while(len--) {
+			/* Read the data */
+			rc = sbefifo_down_read(sbefifo, &data);
+			if (rc < 0)
+				return rc;
+
+			/* Was it an EOT ? */
+			if (eot_set & 0x80) {
+				/*
+				 * There should be nothing else in the FIFO,
+				 * if there is, mark broken, this will force
+				 * a reset on next use, but don't fail the
+				 * command.
+				 */
+				if (len) {
+					dev_warn(dev, "FIFO read hit"
+						 " EOT with still %zd data\n",
+						 len);
+					sbefifo->broken = true;
+				}
+
+				/* We are done */
+				rc = sbefifo_regw(sbefifo,
+						  SBEFIFO_DOWN | SBEFIFO_EOT_ACK, 0);
+
+				/*
+				 * If that write fail, still complete the request but mark
+				 * the fifo as broken for subsequent reset (not much else
+				 * we can do here).
+				 */
+				if (rc) {
+					dev_err(dev, "FSI error %d ack'ing EOT\n", rc);
+					sbefifo->broken = true;
+				}
+
+				/* Tell whether we overflowed */
+				return overflow ? -EOVERFLOW : 0;
+			}
+
+			/* Store it if there is room */
+			if (iov_iter_count(response) >= sizeof(__be32)) {
+				if (copy_to_iter(&data, sizeof(__be32), response) < sizeof(__be32))
+					return -EFAULT;
+			} else {
+				dev_vdbg(dev, "Response overflowed !\n");
+
+				overflow = true;
+			}
+
+			/* Next EOT bit */
+			eot_set <<= 1;
+		}
+	}
+	/* Shouldn't happen */
+	return -EIO;
+}
+
+static int sbefifo_do_command(struct sbefifo *sbefifo,
+			      const __be32 *command, size_t cmd_len,
+			      struct iov_iter *response)
+{
+	/* Try sending the command */
+	int rc = sbefifo_send_command(sbefifo, command, cmd_len);
+	if (rc)
+		return rc;
+
+	/* Now, get the response */
+	return sbefifo_read_response(sbefifo, response);
+}
+
+static void sbefifo_collect_async_ffdc(struct sbefifo *sbefifo)
+{
+	struct device *dev = &sbefifo->fsi_dev->dev;
+        struct iov_iter ffdc_iter;
+        struct kvec ffdc_iov;
+	__be32 *ffdc;
+	size_t ffdc_sz;
+	u32 cmd[2];
+	int rc;
+
+	sbefifo->async_ffdc = false;
+	ffdc = vmalloc(SBEFIFO_MAX_FFDC_SIZE);
+	if (!ffdc) {
+		dev_err(dev, "Failed to allocate SBE FFDC buffer\n");
+		return;
+	}
+        ffdc_iov.iov_base = ffdc;
+	ffdc_iov.iov_len = SBEFIFO_MAX_FFDC_SIZE;;
+        iov_iter_kvec(&ffdc_iter, WRITE | ITER_KVEC, &ffdc_iov, 1, SBEFIFO_MAX_FFDC_SIZE);
+	cmd[0] = cpu_to_be32(2);
+	cmd[1] = cpu_to_be32(SBEFIFO_CMD_GET_SBE_FFDC);
+	rc = sbefifo_do_command(sbefifo, cmd, 2, &ffdc_iter);
+	if (rc != 0) {
+		dev_err(dev, "Error %d retrieving SBE FFDC\n", rc);
+		goto bail;
+	}
+	ffdc_sz = SBEFIFO_MAX_FFDC_SIZE - iov_iter_count(&ffdc_iter);
+	ffdc_sz /= sizeof(__be32);
+	rc = sbefifo_parse_status(dev, SBEFIFO_CMD_GET_SBE_FFDC, ffdc,
+				  ffdc_sz, &ffdc_sz);
+	if (rc != 0) {
+		dev_err(dev, "Error %d decoding SBE FFDC\n", rc);
+		goto bail;
+	}
+	if (ffdc_sz > 0)
+		sbefifo_dump_ffdc(dev, ffdc, ffdc_sz, true);
+ bail:
+	vfree(ffdc);
+
+}
+
+static int __sbefifo_submit(struct sbefifo *sbefifo,
+			    const __be32 *command, size_t cmd_len,
+			    struct iov_iter *response)
+{
+	struct device *dev = &sbefifo->fsi_dev->dev;
+	int rc;
+
+	if (cmd_len < 2 || be32_to_cpu(command[0]) != cmd_len) {
+		dev_vdbg(dev, "Invalid command len %zd (header: %d)\n",
+			 cmd_len, be32_to_cpu(command[0]));
+		return -EINVAL;
+	}
+
+	/* First ensure the HW is in a clean state */
+	rc = sbefifo_cleanup_hw(sbefifo);
+	if (rc)
+		return rc;
+
+	/* Look for async FFDC first if any */
+	if (sbefifo->async_ffdc)
+		sbefifo_collect_async_ffdc(sbefifo);
+
+	rc = sbefifo_do_command(sbefifo, command, cmd_len, response);
+	if (rc != 0 && rc != -EOVERFLOW)
+		goto fail;
+	return rc;
+ fail:
+	/*
+	 * On failure, attempt a reset. Ignore the result, it will mark
+	 * the fifo broken if the reset fails
+	 */
+        sbefifo_request_reset(sbefifo);
+
+	/* Return original error */
+	return rc;
+}
+
+/**
+ * sbefifo_submit() - Submit and SBE fifo command and receive response
+ * @dev: The sbefifo device
+ * @command: The raw command data
+ * @cmd_len: The command size (in 32-bit words)
+ * @response: The output response buffer
+ * @resp_len: In: Response buffer size, Out: Response size
+ *
+ * This will perform the entire operation. If the reponse buffer
+ * overflows, returns -EOVERFLOW
+ */
+int sbefifo_submit(struct device *dev, const __be32 *command, size_t cmd_len,
+		   __be32 *response, size_t *resp_len)
+{
+	struct sbefifo *sbefifo = dev_get_drvdata(dev);
+        struct iov_iter resp_iter;
+        struct kvec resp_iov;
+	size_t rbytes;
+	int rc;
+
+	if (!dev || !sbefifo)
+		return -ENODEV;
+	if (WARN_ON_ONCE(sbefifo->magic != SBEFIFO_MAGIC))
+		return -ENODEV;
+	if (!resp_len || !command || !response || cmd_len > SBEFIFO_MAX_CMD_LEN)
+		return -EINVAL;
+
+	/* Prepare iov iterator */
+	rbytes = (*resp_len) * sizeof(__be32);
+	resp_iov.iov_base = response;
+	resp_iov.iov_len = rbytes;
+        iov_iter_kvec(&resp_iter, WRITE | ITER_KVEC, &resp_iov, 1, rbytes);
+
+	/* Perform the command */
+	mutex_lock(&sbefifo->lock);
+	rc = __sbefifo_submit(sbefifo, command, cmd_len, &resp_iter);
+	mutex_unlock(&sbefifo->lock);
+
+	/* Extract the response length */
+	rbytes -= iov_iter_count(&resp_iter);
+	*resp_len = rbytes / sizeof(__be32);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(sbefifo_submit);
+
+/*
+ * Char device interface
+ */
+static int sbefifo_user_open(struct inode *inode, struct file *file)
+{
+	struct sbefifo *sbefifo = container_of(file->private_data,
+					       struct sbefifo, mdev);
+	struct sbefifo_user *user;
+
+	user = kzalloc(sizeof(struct sbefifo_user), GFP_KERNEL);
+	if (!user)
+		return -ENOMEM;
+
+	file->private_data = user;
+	user->sbefifo = sbefifo;
+	user->pending_cmd = (void *)__get_free_page(GFP_KERNEL);
+	if (!user->pending_cmd) {
+		kfree(user);
+		return -ENOMEM;
+	}
+	mutex_init(&user->file_lock);
+
+	return 0;
+}
+
+static ssize_t sbefifo_user_read(struct file *file, char __user *buf,
+				 size_t len, loff_t *offset)
+{
+	struct sbefifo_user *user = file->private_data;
+	struct sbefifo *sbefifo;
+	struct iov_iter resp_iter;
+        struct iovec resp_iov;
+	size_t cmd_len;
+	int rc;
+
+	if (!user)
+		return -EINVAL;
+	sbefifo = user->sbefifo;
+	if (len & 3)
+		return -EINVAL;
+
+	mutex_lock(&user->file_lock);
+
+	/* Cronus relies on -EAGAIN after a short read */
+	if (user->pending_len == 0) {
+		rc = -EAGAIN;
+		goto bail;
+	}
+	if (user->pending_len < 8) {
+		rc = -EINVAL;
+		goto bail;
+	}
+	cmd_len = user->pending_len >> 2;
+
+	/* Prepare iov iterator */
+	resp_iov.iov_base = buf;
+	resp_iov.iov_len = len;
+	iov_iter_init(&resp_iter, WRITE, &resp_iov, 1, len);
+
+	/* Perform the command */
+	mutex_lock(&sbefifo->lock);
+	rc = __sbefifo_submit(sbefifo, user->pending_cmd, cmd_len, &resp_iter);
+	mutex_unlock(&sbefifo->lock);
+	if (rc < 0)
+		goto bail;
+
+	/* Extract the response length */
+	rc = len - iov_iter_count(&resp_iter);
+ bail:
+	user->pending_len = 0;
+	mutex_unlock(&user->file_lock);
+	return rc;
+}
+
+static ssize_t sbefifo_user_write(struct file *file, const char __user *buf,
+				  size_t len, loff_t *offset)
+{
+	struct sbefifo_user *user = file->private_data;
+	struct sbefifo *sbefifo;
+	int rc = len;
+
+	if (!user)
+		return -EINVAL;
+	sbefifo = user->sbefifo;
+	if (len > SBEFIFO_MAX_CMD_LEN)
+		return -EINVAL;
+	if (len & 3)
+		return -EINVAL;
+
+	mutex_lock(&user->file_lock);
+
+	/* Copy the command into the staging buffer */
+	if (copy_from_user(user->pending_cmd, buf, len)) {
+		rc = -EFAULT;
+		goto bail;
+	}
+
+	/* Check for the magic reset command */
+	if (len == 4 && be32_to_cpu(*(__be32 *)user->pending_cmd) ==
+	    SBEFIFO_RESET_MAGIC)  {
+
+		/* Clear out any pending command */
+		user->pending_len = 0;
+
+		/* Trigger reset request */
+		mutex_lock(&sbefifo->lock);
+		rc = sbefifo_request_reset(user->sbefifo);
+		mutex_unlock(&sbefifo->lock);
+		if (rc == 0)
+			rc = 4;
+		goto bail;
+	}
+
+	/* Update the staging buffer size */
+	user->pending_len = len;
+ bail:
+	mutex_unlock(&user->file_lock);
+
+	/* And that's it, we'll issue the command on a read */
+	return rc;
+}
+
+static int sbefifo_user_release(struct inode *inode, struct file *file)
+{
+	struct sbefifo_user *user = file->private_data;
+
+	if (!user)
+		return -EINVAL;
+
+	free_page((unsigned long)user->pending_cmd);
+	kfree(user);
+
+	return 0;
+}
+
+static const struct file_operations sbefifo_fops = {
+	.owner		= THIS_MODULE,
+	.open		= sbefifo_user_open,
+	.read		= sbefifo_user_read,
+	.write		= sbefifo_user_write,
+	.release	= sbefifo_user_release,
+};
+
+/*
+ * Probe/remove
+ */
+
+static int sbefifo_probe(struct device *dev)
+{
+	struct fsi_device *fsi_dev = to_fsi_dev(dev);
+	struct sbefifo *sbefifo;
+	struct device_node *np;
+	struct platform_device *child;
+	char child_name[32];
+	int rc, child_idx = 0;
+
+	dev_dbg(dev, "Found sbefifo device\n");
+
+	sbefifo = devm_kzalloc(dev, sizeof(*sbefifo), GFP_KERNEL);
+	if (!sbefifo)
+		return -ENOMEM;
+	sbefifo->magic = SBEFIFO_MAGIC;
+	sbefifo->fsi_dev = fsi_dev;
+	mutex_init(&sbefifo->lock);
+
+	/*
+	 * Try cleaning up the FIFO. If this fails, we still register the
+	 * driver and will try cleaning things up again on the next access.
+	 */
+	rc = sbefifo_cleanup_hw(sbefifo);
+	if (rc && rc != -ESHUTDOWN)
+		dev_err(dev, "Initial HW cleanup failed, will retry later\n");
+
+	sbefifo->idx = ida_simple_get(&sbefifo_ida, 1, INT_MAX, GFP_KERNEL);
+	snprintf(sbefifo->name, sizeof(sbefifo->name), "sbefifo%d",
+		 sbefifo->idx);
+
+	dev_set_drvdata(dev, sbefifo);
+
+	/* Create misc chardev for userspace access */
+	sbefifo->mdev.minor = MISC_DYNAMIC_MINOR;
+	sbefifo->mdev.fops = &sbefifo_fops;
+	sbefifo->mdev.name = sbefifo->name;
+	sbefifo->mdev.parent = dev;
+	rc = misc_register(&sbefifo->mdev);
+	if (rc) {
+		dev_err(dev, "Failed to register miscdevice: %d\n", rc);
+		ida_simple_remove(&sbefifo_ida, sbefifo->idx);
+		return rc;
+	}
+
+	/* Create platform devs for dts child nodes (occ, etc) */
+	for_each_available_child_of_node(dev->of_node, np) {
+		snprintf(child_name, sizeof(child_name), "%s-dev%d",
+			 sbefifo->name, child_idx++);
+		child = of_platform_device_create(np, child_name, dev);
+		if (!child)
+			dev_warn(dev, "failed to create child %s dev\n",
+				 child_name);
+	}
+
+	return 0;
+}
+
+static int sbefifo_unregister_child(struct device *dev, void *data)
+{
+	struct platform_device *child = to_platform_device(dev);
+
+	of_device_unregister(child);
+	if (dev->of_node)
+		of_node_clear_flag(dev->of_node, OF_POPULATED);
+
+	return 0;
+}
+
+static int sbefifo_remove(struct device *dev)
+{
+	struct sbefifo *sbefifo = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "Removing sbefifo device...\n");
+
+	misc_deregister(&sbefifo->mdev);
+	device_for_each_child(dev, NULL, sbefifo_unregister_child);
+
+	ida_simple_remove(&sbefifo_ida, sbefifo->idx);
+
+	return 0;
+}
+
+static struct fsi_device_id sbefifo_ids[] = {
+	{
+		.engine_type = FSI_ENGID_SBE,
+		.version = FSI_VERSION_ANY,
+	},
+	{ 0 }
+};
+
+static struct fsi_driver sbefifo_drv = {
+	.id_table = sbefifo_ids,
+	.drv = {
+		.name = DEVICE_NAME,
+		.bus = &fsi_bus_type,
+		.probe = sbefifo_probe,
+		.remove = sbefifo_remove,
+	}
+};
+
+static int sbefifo_init(void)
+{
+	return fsi_driver_register(&sbefifo_drv);
+}
+
+static void sbefifo_exit(void)
+{
+	fsi_driver_unregister(&sbefifo_drv);
+
+	ida_destroy(&sbefifo_ida);
+}
+
+module_init(sbefifo_init);
+module_exit(sbefifo_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Brad Bishop <bradleyb@fuzziesquirrel.com>");
+MODULE_AUTHOR("Eddie James <eajames@linux.vnet.ibm.com>");
+MODULE_AUTHOR("Andrew Jeffery <andrew@aj.id.au>");
+MODULE_AUTHOR("Benjamin Herrenschmidt <benh@kernel.crashing.org>");
+MODULE_DESCRIPTION("Linux device interface to the POWER Self Boot Engine");
diff --git a/include/linux/fsi-sbefifo.h b/include/linux/fsi-sbefifo.h
new file mode 100644
index 000000000000..13f9ebeaa25e
--- /dev/null
+++ b/include/linux/fsi-sbefifo.h
@@ -0,0 +1,33 @@
+/*
+ * SBEFIFO FSI Client device driver
+ *
+ * Copyright (C) IBM Corporation 2017
+ *
+ * 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
+ * MERGCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef LINUX_FSI_SBEFIFO_H
+#define LINUX_FSI_SBEFIFO_H
+
+#define SBEFIFO_CMD_PUT_OCC_SRAM	0xa404
+#define SBEFIFO_CMD_GET_OCC_SRAM	0xa403
+#define SBEFIFO_CMD_GET_SBE_FFDC	0xa801
+
+#define SBEFIFO_MAX_FFDC_SIZE		0x2000
+
+struct device;
+
+int sbefifo_submit(struct device *dev, const __be32 *command, size_t cmd_len,
+		   __be32 *response, size_t *resp_len);
+
+int sbefifo_parse_status(struct device *dev, u16 cmd, __be32 *response,
+			 size_t resp_len, size_t *data_len);
+
+#endif /* LINUX_FSI_SBEFIFO_H */
-- 
2.17.0

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

* [PATCH linux dev-4.13 08/10] fsi/fsi-occ: Simple conversion to new sbefifo driver
  2018-05-24  5:14 [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Benjamin Herrenschmidt
                   ` (5 preceding siblings ...)
  2018-05-24  5:14 ` [PATCH linux dev-4.13 07/10] fsi/sbefifo: Add driver for the SBE FIFO Benjamin Herrenschmidt
@ 2018-05-24  5:14 ` Benjamin Herrenschmidt
  2018-05-24  5:14 ` [PATCH linux dev-4.13 09/10] fsi/occ: Don't set driver data late Benjamin Herrenschmidt
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-24  5:14 UTC (permalink / raw)
  To: openbmc

Replace open/close/write/read API with the simple submit()
API and the helper to parse status.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/fsi/Makefile       |   2 +-
 drivers/fsi/fsi-occ.c      | 209 +++++++++++++++++--------------------
 drivers/hwmon/occ/Makefile |   2 +-
 3 files changed, 99 insertions(+), 114 deletions(-)

diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
index f331e929423c..75fdc6d8cfc4 100644
--- a/drivers/fsi/Makefile
+++ b/drivers/fsi/Makefile
@@ -4,4 +4,4 @@ obj-$(CONFIG_FSI_MASTER_HUB) += fsi-master-hub.o
 obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
 obj-$(CONFIG_FSI_SCOM) += fsi-scom.o
 obj-$(CONFIG_FSI_SBEFIFO) += fsi-sbefifo.o
-#obj-$(CONFIG_FSI_OCC) += fsi-occ.o
+obj-$(CONFIG_FSI_OCC) += fsi-occ.o
diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
index d368b62e5471..3f96d7ddea91 100644
--- a/drivers/fsi/fsi-occ.c
+++ b/drivers/fsi/fsi-occ.c
@@ -36,6 +36,13 @@
 #define OCC_CMD_DATA_BYTES	4090
 #define OCC_RESP_DATA_BYTES	4089
 
+/*
+ * Assume we don't have FFDC, if we do we'll overflow and
+ * fail the command. This needs to be big enough for simple
+ * commands as well.
+ */
+#define OCC_SBE_STATUS_WORDS	16
+
 #define OCC_TIMEOUT_MS		1000
 #define OCC_CMD_IN_PRG_WAIT_MS	50
 
@@ -447,110 +454,72 @@ static int occ_verify_checksum(struct occ_response *resp, u16 data_length)
 	return 0;
 }
 
-static int occ_write_sbefifo(struct sbefifo_client *client, const char *buf,
-			     ssize_t len)
-{
-	int rc;
-	ssize_t total = 0;
-
-	do {
-		rc = sbefifo_drv_write(client, &buf[total], len - total);
-		if (rc < 0)
-			return rc;
-		else if (!rc)
-			break;
-
-		total += rc;
-	} while (total < len);
-
-	return (total == len) ? 0 : -ENOSPC;
-}
-
-static int occ_read_sbefifo(struct sbefifo_client *client, char *buf,
-			    ssize_t len)
-{
-	int rc;
-	ssize_t total = 0;
-
-	do {
-		rc = sbefifo_drv_read(client, &buf[total], len - total);
-		if (rc < 0)
-			return rc;
-		else if (!rc)
-			break;
-
-		total += rc;
-	} while (total < len);
-
-	return (total == len) ? 0 : -ENODATA;
-}
-
 static int occ_getsram(struct device *sbefifo, u32 address, u8 *data,
 		       ssize_t len)
 {
-	int rc;
-	u8 *resp;
-	__be32 buf[5];
 	u32 data_len = ((len + 7) / 8) * 8;	/* must be multiples of 8 B */
-	struct sbefifo_client *client;
+	size_t resp_len, resp_data_len;
+	__be32 *resp, cmd[5];
+	int rc;
 
 	/*
 	 * Magic sequence to do SBE getsram command. SBE will fetch data from
 	 * specified SRAM address.
 	 */
-	buf[0] = cpu_to_be32(0x5);
-	buf[1] = cpu_to_be32(0xa403);
-	buf[2] = cpu_to_be32(1);
-	buf[3] = cpu_to_be32(address);
-	buf[4] = cpu_to_be32(data_len);
-
-	client = sbefifo_drv_open(sbefifo, 0);
-	if (!client)
-		return -ENODEV;
-
-	rc = occ_write_sbefifo(client, (const char *)buf, sizeof(buf));
-	if (rc)
-		goto done;
-
-	resp = kzalloc(data_len, GFP_KERNEL);
-	if (!resp) {
-		rc = -ENOMEM;
-		goto done;
-	}
+	cmd[0] = cpu_to_be32(0x5);
+	cmd[1] = cpu_to_be32(SBEFIFO_CMD_GET_OCC_SRAM);
+	cmd[2] = cpu_to_be32(1);
+	cmd[3] = cpu_to_be32(address);
+	cmd[4] = cpu_to_be32(data_len);
+
+	resp_len = (data_len >> 2) + OCC_SBE_STATUS_WORDS;
+	resp = kzalloc(resp_len << 2 , GFP_KERNEL);
+	if (!resp)
+		return -ENOMEM;
 
-	rc = occ_read_sbefifo(client, (char *)resp, data_len);
+	rc = sbefifo_submit(sbefifo, cmd, 5, resp, &resp_len);
 	if (rc)
 		goto free;
-
-	/* check for good response */
-	rc = occ_read_sbefifo(client, (char *)buf, 8);
+	rc = sbefifo_parse_status(sbefifo, SBEFIFO_CMD_GET_OCC_SRAM,
+				  resp, resp_len, &resp_len);
 	if (rc)
 		goto free;
 
-	if ((be32_to_cpu(buf[0]) == data_len) &&
-	    (be32_to_cpu(buf[1]) == 0xC0DEA403))
-		memcpy(data, resp, len);
-	else
+	resp_data_len = be32_to_cpu(resp[resp_len - 1]);
+	if (resp_data_len != data_len) {
+		pr_err("occ: SRAM read expected %d bytes got %zd\n",
+		       data_len, resp_data_len);
 		rc = -EBADMSG;
+	} else {
+		memcpy(data, resp, len);
+	}
 
 free:
+	/* Convert positive SBEI status */
+	if (rc > 0) {
+		pr_err("occ: SRAM read returned failure status: %08x\n", rc);
+		rc = -EBADMSG;
+	}
 	kfree(resp);
-
-done:
-	sbefifo_drv_release(client);
 	return rc;
 }
 
 static int occ_putsram(struct device *sbefifo, u32 address, u8 *data,
 		       ssize_t len)
 {
-	int rc;
-	__be32 *buf;
+	size_t cmd_len, buf_len, resp_len, resp_data_len;
 	u32 data_len = ((len + 7) / 8) * 8;	/* must be multiples of 8 B */
-	size_t cmd_len = data_len + 20;
-	struct sbefifo_client *client;
+	__be32 *buf;
+	int rc;
 
-	buf = kzalloc(cmd_len, GFP_KERNEL);
+	/*
+	 * We use the same buffer for command and response, make
+	 * sure it's big enough
+	 */
+	resp_len = OCC_SBE_STATUS_WORDS;
+	cmd_len = (data_len >> 2) + 5;
+	buf_len = max(cmd_len, resp_len);
+	buf = kzalloc(buf_len << 2, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
@@ -558,72 +527,88 @@ static int occ_putsram(struct device *sbefifo, u32 address, u8 *data,
 	 * Magic sequence to do SBE putsram command. SBE will transfer
 	 * data to specified SRAM address.
 	 */
-	buf[0] = cpu_to_be32(0x5 + (data_len / 4));
-	buf[1] = cpu_to_be32(0xa404);
+	buf[0] = cpu_to_be32(cmd_len);
+	buf[1] = cpu_to_be32(SBEFIFO_CMD_PUT_OCC_SRAM);
 	buf[2] = cpu_to_be32(1);
 	buf[3] = cpu_to_be32(address);
 	buf[4] = cpu_to_be32(data_len);
 
 	memcpy(&buf[5], data, len);
 
-	client = sbefifo_drv_open(sbefifo, 0);
-	if (!client) {
-		rc = -ENODEV;
-		goto free;
-	}
-
-	rc = occ_write_sbefifo(client, (const char *)buf, cmd_len);
+	rc = sbefifo_submit(sbefifo, buf, cmd_len, buf, &resp_len);
 	if (rc)
-		goto done;
-
-	rc = occ_read_sbefifo(client, (char *)buf, 8);
+		goto free;
+	rc = sbefifo_parse_status(sbefifo, SBEFIFO_CMD_PUT_OCC_SRAM,
+				  buf, resp_len, &resp_len);
 	if (rc)
-		goto done;
+		goto free;
 
-	/* check for good response */
-	if ((be32_to_cpu(buf[0]) != data_len) ||
-	    (be32_to_cpu(buf[1]) != 0xC0DEA404))
+	if (resp_len != 1) {
+		pr_err("occ: SRAM write response lenght invalid: %zd\n",
+		       resp_len);
 		rc = -EBADMSG;
-
-done:
-	sbefifo_drv_release(client);
+	} else {
+		resp_data_len = be32_to_cpu(buf[0]);
+		if (resp_data_len != data_len) {
+			pr_err("occ: SRAM write expected %d bytes got %zd\n",
+			       data_len, resp_data_len);
+			rc = -EBADMSG;
+		}
+	}
 free:
+	/* Convert positive SBEI status */
+	if (rc > 0) {
+		pr_err("occ: SRAM write returned failure status: %08x\n", rc);
+		rc = -EBADMSG;
+	}
 	kfree(buf);
 	return rc;
 }
 
 static int occ_trigger_attn(struct device *sbefifo)
 {
+	__be32 buf[OCC_SBE_STATUS_WORDS];
+	size_t resp_len, resp_data_len;
 	int rc;
-	__be32 buf[7];
-	struct sbefifo_client *client;
+
+	BUILD_BUG_ON(OCC_SBE_STATUS_WORDS < 7);
+	resp_len = OCC_SBE_STATUS_WORDS;
 
 	buf[0] = cpu_to_be32(0x5 + 0x2);        /* Chip-op length in words */
-	buf[1] = cpu_to_be32(0xa404);           /* PutOCCSRAM */
+	buf[1] = cpu_to_be32(SBEFIFO_CMD_PUT_OCC_SRAM);
 	buf[2] = cpu_to_be32(0x3);              /* Mode: Circular */
 	buf[3] = cpu_to_be32(0x0);              /* Address: ignored in mode 3 */
 	buf[4] = cpu_to_be32(0x8);              /* Data length in bytes */
 	buf[5] = cpu_to_be32(0x20010000);       /* Trigger OCC attention */
 	buf[6] = 0;
 
-	client = sbefifo_drv_open(sbefifo, 0);
-	if (!client)
-		return -ENODEV;
-
-	rc = occ_write_sbefifo(client, (const char *)buf, sizeof(buf));
+	rc = sbefifo_submit(sbefifo, buf, 7, buf, &resp_len);
 	if (rc)
-		goto done;
-
-	rc = occ_read_sbefifo(client, (char *)buf, 8);
+		goto error;
+	rc = sbefifo_parse_status(sbefifo, SBEFIFO_CMD_PUT_OCC_SRAM,
+				  buf, resp_len, &resp_len);
 	if (rc)
-		goto done;
+		goto error;
 
-	/* check for good response */
-	if ((be32_to_cpu(buf[0]) != 8) || (be32_to_cpu(buf[1]) != 0xC0DEA404))
+	if (resp_len != 1) {
+		pr_err("occ: SRAM attn response lenght invalid: %zd\n",
+		       resp_len);
 		rc = -EBADMSG;
+	} else {
+		resp_data_len = be32_to_cpu(buf[0]);
+		if (resp_data_len != 8) {
+			pr_err("occ: SRAM attn expected 8 bytes got %zd\n",
+			       resp_data_len);
+			rc = -EBADMSG;
+		}
+	}
+ error:
+	/* Convert positive SBEI status */
+	if (rc > 0) {
+		pr_err("occ: SRAM attn returned failure status: %08x\n", rc);
+		rc = -EBADMSG;
+	}
 
-done:
-	sbefifo_drv_release(client);
 
 	return rc;
 }
diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
index ca6d25ae9da8..ab5c3e965ccb 100644
--- a/drivers/hwmon/occ/Makefile
+++ b/drivers/hwmon/occ/Makefile
@@ -1,7 +1,7 @@
 occ-hwmon-objs := common.o
 
 ifeq ($(CONFIG_SENSORS_OCC_P9_SBE), y)
-#occ-hwmon-objs += p9_sbe.o
+occ-hwmon-objs += p9_sbe.o
 endif
 
 ifeq ($(CONFIG_SENSORS_OCC_P8_I2C), y)
-- 
2.17.0

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

* [PATCH linux dev-4.13 09/10] fsi/occ: Don't set driver data late
  2018-05-24  5:14 [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Benjamin Herrenschmidt
                   ` (6 preceding siblings ...)
  2018-05-24  5:14 ` [PATCH linux dev-4.13 08/10] fsi/fsi-occ: Simple conversion to new sbefifo driver Benjamin Herrenschmidt
@ 2018-05-24  5:14 ` Benjamin Herrenschmidt
  2018-05-24  5:14 ` [PATCH linux dev-4.13 10/10] hwmon/occ: Silence probe error message when host is shutdown Benjamin Herrenschmidt
  2018-05-24 13:40 ` [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Christopher Bostic
  9 siblings, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-24  5:14 UTC (permalink / raw)
  To: openbmc

Until now, the OCC driver was setting the driver data after
registering the character device and the hwmon device.

This might have been intentional, as doing so makes the initial
probe of the OCC by the hwmon device fail while the data is NULL
(provided you are lucky and the hwmon driver doesn't get bound
asynchronously). That failure used to be necessary, otherwise
the driver would try to access the SBE fifo at a time when it's
not ready, causing all sort of problems.

The new SBE fifo driver is much more robust and will return an
appropriate error code, so that (fragile) tweak is no longer
necessary.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/fsi/fsi-occ.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
index 3f96d7ddea91..15f41f45fb31 100644
--- a/drivers/fsi/fsi-occ.c
+++ b/drivers/fsi/fsi-occ.c
@@ -794,6 +794,8 @@ static int occ_probe(struct platform_device *pdev)
 		occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX, GFP_KERNEL);
 	}
 
+	platform_set_drvdata(pdev, occ);
+
 	snprintf(occ->name, sizeof(occ->name), "occ%d", occ->idx);
 	occ->mdev.fops = &occ_fops;
 	occ->mdev.minor = MISC_DYNAMIC_MINOR;
@@ -812,8 +814,6 @@ static int occ_probe(struct platform_device *pdev)
 	if (!hwmon_dev)
 		dev_warn(dev, "failed to create hwmon device\n");
 
-	platform_set_drvdata(pdev, occ);
-
 	return 0;
 }
 
-- 
2.17.0

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

* [PATCH linux dev-4.13 10/10] hwmon/occ: Silence probe error message when host is shutdown
  2018-05-24  5:14 [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Benjamin Herrenschmidt
                   ` (7 preceding siblings ...)
  2018-05-24  5:14 ` [PATCH linux dev-4.13 09/10] fsi/occ: Don't set driver data late Benjamin Herrenschmidt
@ 2018-05-24  5:14 ` Benjamin Herrenschmidt
  2018-05-24 13:40 ` [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Christopher Bostic
  9 siblings, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-24  5:14 UTC (permalink / raw)
  To: openbmc

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/hwmon/occ/p9_sbe.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c
index 27886897a3d6..9a1d3ae56d69 100644
--- a/drivers/hwmon/occ/p9_sbe.c
+++ b/drivers/hwmon/occ/p9_sbe.c
@@ -110,6 +110,7 @@ static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd)
 static int p9_sbe_occ_probe(struct platform_device *pdev)
 {
 	struct occ *occ;
+	int rc;
 	struct p9_sbe_occ *ctx = devm_kzalloc(&pdev->dev,
 						     sizeof(*ctx),
 						     GFP_KERNEL);
@@ -126,7 +127,12 @@ static int p9_sbe_occ_probe(struct platform_device *pdev)
 	occ->poll_cmd_data = 0x20;		/* P9 OCC poll data */
 	occ->send_cmd = p9_sbe_occ_send_cmd;
 
-	return occ_setup(occ, "p9_occ");
+	rc = occ_setup(occ, "p9_occ");
+
+	/* Host is shutdown, don't spew errors */
+	if (rc == -ESHUTDOWN)
+		rc = -ENODEV;
+	return rc;
 }
 
 static int p9_sbe_occ_remove(struct platform_device *pdev)
-- 
2.17.0

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

* Re: [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section
  2018-05-24  5:14 [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Benjamin Herrenschmidt
                   ` (8 preceding siblings ...)
  2018-05-24  5:14 ` [PATCH linux dev-4.13 10/10] hwmon/occ: Silence probe error message when host is shutdown Benjamin Herrenschmidt
@ 2018-05-24 13:40 ` Christopher Bostic
  9 siblings, 0 replies; 14+ messages in thread
From: Christopher Bostic @ 2018-05-24 13:40 UTC (permalink / raw)
  To: openbmc

Reviewed-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>


On 5/24/18 12:14 AM, Benjamin Herrenschmidt wrote:
> From: Jeremy Kerr <jk@ozlabs.org>
>
> For implementing relative addressing mode, we'll need to build a command
> that is coherent with CFAM state. To do that, include the
> build_command_* functions in the locked section of read/write/term.
>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>   drivers/fsi/fsi-master-gpio.c | 25 ++++++++++++++++++-------
>   1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index e1bde9e3f855..d50ea4849377 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -463,14 +463,10 @@ static int fsi_master_gpio_xfer(struct fsi_master_gpio *master, uint8_t slave,
>   {
>   	int rc;
>
> -	mutex_lock(&master->cmd_lock);
> -
>   	rc = send_request(master, cmd);
>   	if (!rc)
>   		rc = poll_for_response(master, slave, resp_len, resp);
>
> -	mutex_unlock(&master->cmd_lock);
> -
>   	return rc;
>   }
>
> @@ -479,12 +475,17 @@ static int fsi_master_gpio_read(struct fsi_master *_master, int link,
>   {
>   	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
>   	struct fsi_gpio_msg cmd;
> +	int rc;
>
>   	if (link != 0)
>   		return -ENODEV;
>
> +	mutex_lock(&master->cmd_lock);
>   	build_abs_ar_command(&cmd, id, addr, size, NULL);
> -	return fsi_master_gpio_xfer(master, id, &cmd, size, val);
> +	rc = fsi_master_gpio_xfer(master, id, &cmd, size, val);
> +	mutex_unlock(&master->cmd_lock);
> +
> +	return rc;
>   }
>
>   static int fsi_master_gpio_write(struct fsi_master *_master, int link,
> @@ -492,12 +493,17 @@ static int fsi_master_gpio_write(struct fsi_master *_master, int link,
>   {
>   	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
>   	struct fsi_gpio_msg cmd;
> +	int rc;
>
>   	if (link != 0)
>   		return -ENODEV;
>
> +	mutex_lock(&master->cmd_lock);
>   	build_abs_ar_command(&cmd, id, addr, size, val);
> -	return fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
> +	rc = fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
> +	mutex_unlock(&master->cmd_lock);
> +
> +	return rc;
>   }
>
>   static int fsi_master_gpio_term(struct fsi_master *_master,
> @@ -505,12 +511,17 @@ static int fsi_master_gpio_term(struct fsi_master *_master,
>   {
>   	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
>   	struct fsi_gpio_msg cmd;
> +	int rc;
>
>   	if (link != 0)
>   		return -ENODEV;
>
> +	mutex_lock(&master->cmd_lock);
>   	build_term_command(&cmd, id);
> -	return fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
> +	rc = fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
> +	mutex_unlock(&master->cmd_lock);
> +
> +	return rc;
>   }
>
>   static int fsi_master_gpio_break(struct fsi_master *_master, int link)

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

* Re: [PATCH linux dev-4.13 02/10] fsi/gpio: Use relative-addressing commands
  2018-05-24  5:14 ` [PATCH linux dev-4.13 02/10] fsi/gpio: Use relative-addressing commands Benjamin Herrenschmidt
@ 2018-05-24 14:34   ` Christopher Bostic
  0 siblings, 0 replies; 14+ messages in thread
From: Christopher Bostic @ 2018-05-24 14:34 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, openbmc

Reviewed-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>


On 5/24/18 12:14 AM, Benjamin Herrenschmidt wrote:
> From: Jeremy Kerr <jk@ozlabs.org>
>
> FSI CFAMs support shorter commands that use a relative (or same) address
> as the last. This change introduces a last_addr to the master state, and
> uses it for subsequent reads/writes, and performs relative addressing
> when a subsequent read/write is in range.
>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>   drivers/fsi/fsi-master-gpio.c | 102 ++++++++++++++++++++++++++++++----
>   1 file changed, 91 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index d50ea4849377..0a6799bda294 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -28,6 +28,8 @@
>   #define	FSI_GPIO_CMD_DPOLL      0x2
>   #define	FSI_GPIO_CMD_TERM	0x3f
>   #define FSI_GPIO_CMD_ABS_AR	0x4
> +#define FSI_GPIO_CMD_REL_AR	0x5
> +#define FSI_GPIO_CMD_SAME_AR	0x3	/* but only a 2-bit opcode... */
>
>
>   #define	FSI_GPIO_DPOLL_CLOCKS	50      /* < 21 will cause slave to hang */
> @@ -52,6 +54,8 @@
>   #define	FSI_GPIO_MSG_RESPID_SIZE	2
>   #define	FSI_GPIO_PRIME_SLAVE_CLOCKS	20
>
> +#define LAST_ADDR_INVALID		0x1
> +
>   struct fsi_master_gpio {
>   	struct fsi_master	master;
>   	struct device		*dev;
> @@ -64,6 +68,7 @@ struct fsi_master_gpio {
>   	struct gpio_desc	*gpio_mux;	/* Mux control */
>   	bool			external_mode;
>   	bool			no_delays;
> +	uint32_t		last_addr;
>   };
>
>   #define CREATE_TRACE_POINTS
> @@ -205,22 +210,89 @@ static void msg_push_crc(struct fsi_gpio_msg *msg)
>   	msg_push_bits(msg, crc, 4);
>   }
>
> +static bool check_same_address(struct fsi_master_gpio *master, int id,
> +		uint32_t addr)
> +{
> +	/* this will also handle LAST_ADDR_INVALID */
> +	return master->last_addr == (((id & 0x3) << 21) | (addr & ~0x3));
> +}
> +
> +static bool check_relative_address(struct fsi_master_gpio *master, int id,
> +		uint32_t addr, uint32_t *rel_addrp)
> +{
> +	uint32_t last_addr = master->last_addr;
> +	int32_t rel_addr;
> +
> +	if (last_addr == LAST_ADDR_INVALID)
> +		return false;
> +
> +	/* We may be in 23-bit addressing mode, which uses the id as the
> +	 * top two address bits. So, if we're referencing a different ID,
> +	 * use absolute addresses.
> +	 */
> +	if (((last_addr >> 21) & 0x3) != id)
> +		return false;
> +
> +	/* remove the top two bits from any 23-bit addressing */
> +	last_addr &= (1 << 21) - 1;
> +
> +	/* We know that the addresses are limited to 21 bits, so this won't
> +	 * overflow the signed rel_addr */
> +	rel_addr = addr - last_addr;
> +	if (rel_addr > 255 || rel_addr < -256)
> +		return false;
> +
> +	*rel_addrp = (uint32_t)rel_addr;
> +
> +	return true;
> +}
> +
> +static void last_address_update(struct fsi_master_gpio *master,
> +		int id, bool valid, uint32_t addr)
> +{
> +	if (!valid)
> +		master->last_addr = LAST_ADDR_INVALID;
> +	else
> +		master->last_addr = ((id & 0x3) << 21) | (addr & ~0x3);
> +}
> +
>   /*
> - * Encode an Absolute Address command
> + * Encode an Absolute/Relative/Same Address command
>    */
> -static void build_abs_ar_command(struct fsi_gpio_msg *cmd,
> -		uint8_t id, uint32_t addr, size_t size, const void *data)
> +static void build_ar_command(struct fsi_master_gpio *master,
> +		struct fsi_gpio_msg *cmd, uint8_t id,
> +		uint32_t addr, size_t size, const void *data)
>   {
> +	int i, addr_bits, opcode_bits;
>   	bool write = !!data;
> -	uint8_t ds;
> -	int i;
> +	uint8_t ds, opcode;
> +	uint32_t rel_addr;
>
>   	cmd->bits = 0;
>   	cmd->msg = 0;
>
> -	msg_push_bits(cmd, id, 2);
> -	msg_push_bits(cmd, FSI_GPIO_CMD_ABS_AR, 3);
> -	msg_push_bits(cmd, write ? 0 : 1, 1);
> +	/* we have 21 bits of address max */
> +	addr &= ((1 << 21) - 1);
> +
> +	/* cmd opcodes are variable length - SAME_AR is only two bits */
> +	opcode_bits = 3;
> +
> +	if (check_same_address(master, id, addr)) {
> +		/* we still address the byte offset within the word */
> +		addr_bits = 2;
> +		opcode_bits = 2;
> +		opcode = FSI_GPIO_CMD_SAME_AR;
> +
> +	} else if (check_relative_address(master, id, addr, &rel_addr)) {
> +		/* 8 bits plus sign */
> +		addr_bits = 9;
> +		addr = rel_addr;
> +		opcode = FSI_GPIO_CMD_REL_AR;
> +
> +	} else {
> +		addr_bits = 21;
> +		opcode = FSI_GPIO_CMD_ABS_AR;
> +	}
>
>   	/*
>   	 * The read/write size is encoded in the lower bits of the address
> @@ -237,7 +309,10 @@ static void build_abs_ar_command(struct fsi_gpio_msg *cmd,
>   	if (size == 4)
>   		addr |= 1;
>
> -	msg_push_bits(cmd, addr & ((1 << 21) - 1), 21);
> +	msg_push_bits(cmd, id, 2);
> +	msg_push_bits(cmd, opcode, opcode_bits);
> +	msg_push_bits(cmd, write ? 0 : 1, 1);
> +	msg_push_bits(cmd, addr, addr_bits);
>   	msg_push_bits(cmd, ds, 1);
>   	for (i = 0; write && i < size; i++)
>   		msg_push_bits(cmd, ((uint8_t *)data)[i], 8);
> @@ -481,8 +556,9 @@ static int fsi_master_gpio_read(struct fsi_master *_master, int link,
>   		return -ENODEV;
>
>   	mutex_lock(&master->cmd_lock);
> -	build_abs_ar_command(&cmd, id, addr, size, NULL);
> +	build_ar_command(master, &cmd, id, addr, size, NULL);
>   	rc = fsi_master_gpio_xfer(master, id, &cmd, size, val);
> +	last_address_update(master, id, rc == 0, addr);
>   	mutex_unlock(&master->cmd_lock);
>
>   	return rc;
> @@ -499,8 +575,9 @@ static int fsi_master_gpio_write(struct fsi_master *_master, int link,
>   		return -ENODEV;
>
>   	mutex_lock(&master->cmd_lock);
> -	build_abs_ar_command(&cmd, id, addr, size, val);
> +	build_ar_command(master, &cmd, id, addr, size, val);
>   	rc = fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
> +	last_address_update(master, id, rc == 0, addr);
>   	mutex_unlock(&master->cmd_lock);
>
>   	return rc;
> @@ -519,6 +596,7 @@ static int fsi_master_gpio_term(struct fsi_master *_master,
>   	mutex_lock(&master->cmd_lock);
>   	build_term_command(&cmd, id);
>   	rc = fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
> +	last_address_update(master, id, false, 0);
>   	mutex_unlock(&master->cmd_lock);
>
>   	return rc;
> @@ -552,6 +630,7 @@ static int fsi_master_gpio_break(struct fsi_master *_master, int link)
>   	clock_toggle(master, FSI_POST_BREAK_CLOCKS);
>
>   	spin_unlock_irqrestore(&master->bit_lock, flags);
> +	last_address_update(master, 0, false, 0);
>   	mutex_unlock(&master->cmd_lock);
>
>   	/* Wait for logic reset to take effect */
> @@ -662,6 +741,7 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
>   	master->dev = &pdev->dev;
>   	master->master.dev.parent = master->dev;
>   	master->master.dev.of_node = of_node_get(dev_of_node(master->dev));
> +	master->last_addr = LAST_ADDR_INVALID;
>
>   	gpio = devm_gpiod_get(&pdev->dev, "clock", 0);
>   	if (IS_ERR(gpio)) {

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

* Re: [PATCH linux dev-4.13 03/10] fsi/fsi-master-gpio: Implement CRC error recovery
  2018-05-24  5:14 ` [PATCH linux dev-4.13 03/10] fsi/fsi-master-gpio: Implement CRC error recovery Benjamin Herrenschmidt
@ 2018-05-24 15:05   ` Christopher Bostic
  0 siblings, 0 replies; 14+ messages in thread
From: Christopher Bostic @ 2018-05-24 15:05 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, openbmc

Reviewed-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>


On 5/24/18 12:14 AM, Benjamin Herrenschmidt wrote:
> The FSI protocol defines two modes of recovery from CRC errors,
> this implements both:
>
>   - If the device returns an ECRC (it detected a CRC error in the
>     command), then we simply issue the command again.
>
>   - If the master detects a CRC error in the response, we send
>     an E_POLL command which requests a resend of the response
>     without actually re-executing the command (which could otherwise
>     have unwanted side effects such as dequeuing a FIFO twice).
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>
> Note: This was actually tested by removing some of my fixes, thus
> causing us to hit occasional CRC errors during high LPC activity.
> ---
>   drivers/fsi/fsi-master-gpio.c          | 90 ++++++++++++++++++++------
>   include/trace/events/fsi_master_gpio.h | 27 ++++++++
>   2 files changed, 99 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index 0a6799bda294..351c12f2ac55 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -22,20 +22,23 @@
>   #define	FSI_BREAK_CLOCKS	256	/* Number of clocks to issue break */
>   #define	FSI_POST_BREAK_CLOCKS	16000	/* Number clocks to set up cfam */
>   #define	FSI_INIT_CLOCKS		5000	/* Clock out any old data */
> +#define	FSI_GPIO_DPOLL_CLOCKS	50      /* < 21 will cause slave to hang */
> +#define	FSI_GPIO_EPOLL_CLOCKS	50      /* Number of clocks for E_POLL retry */
>   #define	FSI_GPIO_STD_DELAY	10	/* Standard GPIO delay in nS */
>   					/* todo: adjust down as low as */
>   					/* possible or eliminate */
> +#define FSI_CRC_ERR_RETRIES	10
> +
>   #define	FSI_GPIO_CMD_DPOLL      0x2
> +#define	FSI_GPIO_CMD_EPOLL      0x3
>   #define	FSI_GPIO_CMD_TERM	0x3f
>   #define FSI_GPIO_CMD_ABS_AR	0x4
>   #define FSI_GPIO_CMD_REL_AR	0x5
>   #define FSI_GPIO_CMD_SAME_AR	0x3	/* but only a 2-bit opcode... */
>
> -
> -#define	FSI_GPIO_DPOLL_CLOCKS	50      /* < 21 will cause slave to hang */
> -
> -/* Bus errors */
> -#define	FSI_GPIO_ERR_BUSY	1	/* Slave stuck in busy state */
> +/* Slave responses */
> +#define	FSI_GPIO_RESP_ACK	0	/* Success */
> +#define	FSI_GPIO_RESP_BUSY	1	/* Slave busy */
>   #define	FSI_GPIO_RESP_ERRA	2	/* Any (misc) Error */
>   #define	FSI_GPIO_RESP_ERRC	3	/* Slave reports master CRC error */
>   #define	FSI_GPIO_MTOE		4	/* Master time out error */
> @@ -330,6 +333,16 @@ static void build_dpoll_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
>   	msg_push_crc(cmd);
>   }
>
> +static void build_epoll_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
> +{
> +	cmd->bits = 0;
> +	cmd->msg = 0;
> +
> +	msg_push_bits(cmd, slave_id, 2);
> +	msg_push_bits(cmd, FSI_GPIO_CMD_EPOLL, 3);
> +	msg_push_crc(cmd);
> +}
> +
>   static void echo_delay(struct fsi_master_gpio *master)
>   {
>   	set_sda_output(master, 1);
> @@ -355,6 +368,12 @@ static void fsi_master_gpio_error(struct fsi_master_gpio *master, int error)
>
>   }
>
> +/*
> + * Note: callers rely specifically on this returning -EAGAIN for
> + * a CRC error detected in the response. Use other error code
> + * for other situations. It will be converted to something else
> + * higher up the stack before it reaches userspace.
> + */
>   static int read_one_response(struct fsi_master_gpio *master,
>   		uint8_t data_size, struct fsi_gpio_msg *msgp, uint8_t *tagp)
>   {
> @@ -379,7 +398,7 @@ static int read_one_response(struct fsi_master_gpio *master,
>   			"Master time out waiting for response\n");
>   		fsi_master_gpio_error(master, FSI_GPIO_MTOE);
>   		spin_unlock_irqrestore(&master->bit_lock, flags);
> -		return -EIO;
> +		return -ETIMEDOUT;
>   	}
>
>   	msg.bits = 0;
> @@ -405,7 +424,7 @@ static int read_one_response(struct fsi_master_gpio *master,
>   	if (crc) {
>   		dev_dbg(master->dev, "ERR response CRC\n");
>   		fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
> -		return -EIO;
> +		return -EAGAIN;
>   	}
>
>   	if (msgp)
> @@ -451,11 +470,33 @@ static int poll_for_response(struct fsi_master_gpio *master,
>   	unsigned long flags;
>   	uint8_t tag;
>   	uint8_t *data_byte = data;
> -
> +	int crc_err_retries = 0;
>   retry:
>   	rc = read_one_response(master, size, &response, &tag);
> -	if (rc)
> -		return rc;
> +
> +	/* Handle retries on CRC errors */
> +	if (rc == -EAGAIN) {
> +		/* Too many retries ? */
> +		if (crc_err_retries++ > FSI_CRC_ERR_RETRIES) {
> +			/*
> +			 * Pass it up as a -EIO otherwise upper level will retry
> +			 * the whole command which isn't what we want here.
> +			 */
> +			rc = -EIO;
> +			goto fail;
> +		}
> +		dev_dbg(master->dev,
> +			 "CRC error retry %d\n", crc_err_retries);
> +		trace_fsi_master_gpio_crc_rsp_error(master);
> +		build_epoll_command(&cmd, slave);
> +		spin_lock_irqsave(&master->bit_lock, flags);
> +		clock_zeros(master, FSI_GPIO_EPOLL_CLOCKS);
> +		serial_out(master, &cmd);
> +		echo_delay(master);
> +		spin_unlock_irqrestore(&master->bit_lock, flags);
> +		goto retry;
> +	} else if (rc)
> +		goto fail;
>
>   	switch (tag) {
>   	case FSI_GPIO_RESP_ACK:
> @@ -496,18 +537,21 @@ static int poll_for_response(struct fsi_master_gpio *master,
>   		break;
>
>   	case FSI_GPIO_RESP_ERRA:
> -	case FSI_GPIO_RESP_ERRC:
> -		dev_dbg(master->dev, "ERR%c received: 0x%x\n",
> -			tag == FSI_GPIO_RESP_ERRA ? 'A' : 'C',
> -			(int)response.msg);
> +		dev_dbg(master->dev, "ERRA received: 0x%x\n", (int)response.msg);
>   		fsi_master_gpio_error(master, response.msg);
>   		rc = -EIO;
>   		break;
> +	case FSI_GPIO_RESP_ERRC:
> +		dev_dbg(master->dev, "ERRC received: 0x%x\n", (int)response.msg);
> +		fsi_master_gpio_error(master, response.msg);
> +		trace_fsi_master_gpio_crc_cmd_error(master);
> +		rc = -EAGAIN;
> +		break;
>   	}
>
>   	if (busy_count > 0)
>   		trace_fsi_master_gpio_poll_response_busy(master, busy_count);
> -
> + fail:
>   	/* Clock the slave enough to be ready for next operation */
>   	spin_lock_irqsave(&master->bit_lock, flags);
>   	clock_zeros(master, FSI_GPIO_PRIME_SLAVE_CLOCKS);
> @@ -536,11 +580,21 @@ static int send_request(struct fsi_master_gpio *master,
>   static int fsi_master_gpio_xfer(struct fsi_master_gpio *master, uint8_t slave,
>   		struct fsi_gpio_msg *cmd, size_t resp_len, void *resp)
>   {
> -	int rc;
> +	int rc = -EAGAIN, retries = 0;
>
> -	rc = send_request(master, cmd);
> -	if (!rc)
> +	while ((retries++) < FSI_CRC_ERR_RETRIES) {
> +		rc = send_request(master, cmd);
> +		if (rc)
> +			break;
>   		rc = poll_for_response(master, slave, resp_len, resp);
> +		if (rc != -EAGAIN)
> +			break;
> +		rc = -EIO;
> +		dev_warn(master->dev, "ECRC retry %d\n", retries);
> +
> +		/* Pace it a bit before retry */
> +		msleep(1);
> +	}
>
>   	return rc;
>   }
> diff --git a/include/trace/events/fsi_master_gpio.h b/include/trace/events/fsi_master_gpio.h
> index 48e83e5755f4..e18b94ce85b2 100644
> --- a/include/trace/events/fsi_master_gpio.h
> +++ b/include/trace/events/fsi_master_gpio.h
> @@ -63,6 +63,33 @@ TRACE_EVENT(fsi_master_gpio_break,
>   	)
>   );
>
> +TRACE_EVENT(fsi_master_gpio_crc_cmd_error,
> +	TP_PROTO(const struct fsi_master_gpio *master),
> +	TP_ARGS(master),
> +	TP_STRUCT__entry(
> +		__field(int,	master_idx)
> +	),
> +	TP_fast_assign(
> +		__entry->master_idx = master->master.idx;
> +	),
> +	TP_printk("fsi-gpio%d ----CRC command retry---",
> +		__entry->master_idx
> +	)
> +);
> +
> +TRACE_EVENT(fsi_master_gpio_crc_rsp_error,
> +	TP_PROTO(const struct fsi_master_gpio *master),
> +	TP_ARGS(master),
> +	TP_STRUCT__entry(
> +		__field(int,	master_idx)
> +	),
> +	TP_fast_assign(
> +		__entry->master_idx = master->master.idx;
> +	),
> +	TP_printk("fsi-gpio%d ----CRC response---",
> +		__entry->master_idx
> +	)
> +);
>
>   TRACE_EVENT(fsi_master_gpio_poll_response_busy,
>   	TP_PROTO(const struct fsi_master_gpio *master, int busy),

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

* Re: [PATCH linux dev-4.13 04/10] fsi/fsi-master-gpio: More error handling cleanup
  2018-05-24  5:14 ` [PATCH linux dev-4.13 04/10] fsi/fsi-master-gpio: More error handling cleanup Benjamin Herrenschmidt
@ 2018-05-24 18:50   ` Christopher Bostic
  0 siblings, 0 replies; 14+ messages in thread
From: Christopher Bostic @ 2018-05-24 18:50 UTC (permalink / raw)
  To: openbmc

Reviewed-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>


On 5/24/18 12:14 AM, Benjamin Herrenschmidt wrote:
> Remove calls to the empty and useless fsi_master_gpio_error()
> function, and report CRC errors as "FSI_ERR_NO_SLAVE" when
> reading an all 1's response.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>   drivers/fsi/fsi-master-gpio.c | 26 +++++---------------------
>   1 file changed, 5 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index 351c12f2ac55..da556da62846 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -41,13 +41,6 @@
>   #define	FSI_GPIO_RESP_BUSY	1	/* Slave busy */
>   #define	FSI_GPIO_RESP_ERRA	2	/* Any (misc) Error */
>   #define	FSI_GPIO_RESP_ERRC	3	/* Slave reports master CRC error */
> -#define	FSI_GPIO_MTOE		4	/* Master time out error */
> -#define	FSI_GPIO_CRC_INVAL	5	/* Master reports slave CRC error */
> -
> -/* Normal slave responses */
> -#define	FSI_GPIO_RESP_BUSY	1
> -#define	FSI_GPIO_RESP_ACK	0
> -#define	FSI_GPIO_RESP_ACKD	4
>
>   #define	FSI_GPIO_MAX_BUSY	200
>   #define	FSI_GPIO_MTOE_COUNT	1000
> @@ -359,15 +352,6 @@ static void build_term_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
>   	msg_push_crc(cmd);
>   }
>
> -/*
> - * Store information on master errors so handler can detect and clean
> - * up the bus
> - */
> -static void fsi_master_gpio_error(struct fsi_master_gpio *master, int error)
> -{
> -
> -}
> -
>   /*
>    * Note: callers rely specifically on this returning -EAGAIN for
>    * a CRC error detected in the response. Use other error code
> @@ -396,7 +380,6 @@ static int read_one_response(struct fsi_master_gpio *master,
>   	if (i == FSI_GPIO_MTOE_COUNT) {
>   		dev_dbg(master->dev,
>   			"Master time out waiting for response\n");
> -		fsi_master_gpio_error(master, FSI_GPIO_MTOE);
>   		spin_unlock_irqrestore(&master->bit_lock, flags);
>   		return -ETIMEDOUT;
>   	}
> @@ -422,8 +405,11 @@ static int read_one_response(struct fsi_master_gpio *master,
>   	crc = crc4(0, 1, 1);
>   	crc = crc4(crc, msg.msg, msg.bits);
>   	if (crc) {
> -		dev_dbg(master->dev, "ERR response CRC\n");
> -		fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
> +		/* Check if it's all 1's, that probably means the host is off */
> +		if (((~msg.msg) & ((1ull << msg.bits) - 1)) == 0)
> +			return -ENODEV;
> +		dev_dbg(master->dev, "ERR response CRC msg: 0x%016llx (%d bits)\n",
> +			msg.msg, msg.bits);
>   		return -EAGAIN;
>   	}
>
> @@ -538,12 +524,10 @@ static int poll_for_response(struct fsi_master_gpio *master,
>
>   	case FSI_GPIO_RESP_ERRA:
>   		dev_dbg(master->dev, "ERRA received: 0x%x\n", (int)response.msg);
> -		fsi_master_gpio_error(master, response.msg);
>   		rc = -EIO;
>   		break;
>   	case FSI_GPIO_RESP_ERRC:
>   		dev_dbg(master->dev, "ERRC received: 0x%x\n", (int)response.msg);
> -		fsi_master_gpio_error(master, response.msg);
>   		trace_fsi_master_gpio_crc_cmd_error(master);
>   		rc = -EAGAIN;
>   		break;

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

end of thread, other threads:[~2018-05-24 18:50 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-24  5:14 [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Benjamin Herrenschmidt
2018-05-24  5:14 ` [PATCH linux dev-4.13 02/10] fsi/gpio: Use relative-addressing commands Benjamin Herrenschmidt
2018-05-24 14:34   ` Christopher Bostic
2018-05-24  5:14 ` [PATCH linux dev-4.13 03/10] fsi/fsi-master-gpio: Implement CRC error recovery Benjamin Herrenschmidt
2018-05-24 15:05   ` Christopher Bostic
2018-05-24  5:14 ` [PATCH linux dev-4.13 04/10] fsi/fsi-master-gpio: More error handling cleanup Benjamin Herrenschmidt
2018-05-24 18:50   ` Christopher Bostic
2018-05-24  5:14 ` [PATCH linux dev-4.13 05/10] fsi/master-gpio: Replace bit_bit lock with IRQ disable/enable Benjamin Herrenschmidt
2018-05-24  5:14 ` [PATCH linux dev-4.13 06/10] fsi: Remove old sbefifo driver Benjamin Herrenschmidt
2018-05-24  5:14 ` [PATCH linux dev-4.13 07/10] fsi/sbefifo: Add driver for the SBE FIFO Benjamin Herrenschmidt
2018-05-24  5:14 ` [PATCH linux dev-4.13 08/10] fsi/fsi-occ: Simple conversion to new sbefifo driver Benjamin Herrenschmidt
2018-05-24  5:14 ` [PATCH linux dev-4.13 09/10] fsi/occ: Don't set driver data late Benjamin Herrenschmidt
2018-05-24  5:14 ` [PATCH linux dev-4.13 10/10] hwmon/occ: Silence probe error message when host is shutdown Benjamin Herrenschmidt
2018-05-24 13:40 ` [PATCH linux dev-4.13 01/10] fsi/gpio: Include command build in locked section Christopher Bostic

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.