linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] drivers/fsi: Add SBEFIFO client driver
@ 2017-11-17 19:34 Eddie James
  2017-11-17 19:34 ` [PATCH v4 1/4] dt-bindings: fsi: Add SBEFIFO documentation Eddie James
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Eddie James @ 2017-11-17 19:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, devicetree, robh+dt, mark.rutland, bradleyb, cbostic,
	joel, eajames, Edward A. James

From: "Edward A. James" <eajames@us.ibm.com>

This series adds an FSI-based device driver for the SBEFIFO.

IBM POWER9 processors contain some embedded hardware and software bits
collectively referred to as the self boot engine (SBE).  One role of
the SBE is to act as a proxy that provides access to the registers of
the POWER chip from other (embedded) systems.

The POWER9 chip contains a hardware frontend for communicating with
the SBE from remote systems called the SBEFIFO.  The SBEFIFO logic
is contained within an FSI CFAM  and as such the driver implements an
FSI bus device.

The SBE expects to communicate using a defined wire protocol; however,
the driver knows nothing of the protocol and only provides raw access
to the fifo device to userspace applications wishing to communicate with
the SBE using the wire protocol.

The SBEFIFO consists of two hardware fifos. The upstream fifo is used
by the driver to transfer data to the SBE on the POWER chip, from the
system hosting the driver.  The downstream fifo is used by the driver to
transfer data from the SBE on the power chip to the system hosting the
driver.

Changes since v3:
 - Add reset procedure and use it if there is data in the FIFO at probe time.
 - Add timeout for waiting for data to appear in the FIFO; if the SBE isn't
   running, then previously we would wait forever.
 - Fix remove() order of operations.
 - Fix xfr memory leak.
 - Formatting fixes.

Edward A. James (4):
  dt-bindings: fsi: Add SBEFIFO documentation
  drivers/fsi: Add SBEFIFO FSI client device driver
  drivers/fsi: sbefifo: Add miscdevice
  drivers/fsi: sbefifo: Add in-kernel API

 .../devicetree/bindings/fsi/ibm,p9-sbefifo.txt     |   35 +
 drivers/fsi/Kconfig                                |    7 +
 drivers/fsi/Makefile                               |    1 +
 drivers/fsi/fsi-sbefifo.c                          | 1024 ++++++++++++++++++++
 include/linux/fsi-sbefifo.h                        |   30 +
 5 files changed, 1097 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/ibm,p9-sbefifo.txt
 create mode 100644 drivers/fsi/fsi-sbefifo.c
 create mode 100644 include/linux/fsi-sbefifo.h

-- 
1.8.3.1

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

* [PATCH v4 1/4] dt-bindings: fsi: Add SBEFIFO documentation
  2017-11-17 19:34 [PATCH v4 0/4] drivers/fsi: Add SBEFIFO client driver Eddie James
@ 2017-11-17 19:34 ` Eddie James
  2017-11-17 19:34 ` [PATCH v4 2/4] drivers/fsi: Add SBEFIFO FSI client device driver Eddie James
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Eddie James @ 2017-11-17 19:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, devicetree, robh+dt, mark.rutland, bradleyb, cbostic,
	joel, eajames, Edward A. James

From: "Edward A. James" <eajames@us.ibm.com>

Document the bindings for the SBE CFAM device. SBE devices are
located on a CFAM off an FSI bus.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
 .../devicetree/bindings/fsi/ibm,p9-sbefifo.txt     | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/ibm,p9-sbefifo.txt

diff --git a/Documentation/devicetree/bindings/fsi/ibm,p9-sbefifo.txt b/Documentation/devicetree/bindings/fsi/ibm,p9-sbefifo.txt
new file mode 100644
index 0000000..e69b0c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/fsi/ibm,p9-sbefifo.txt
@@ -0,0 +1,35 @@
+Device-tree bindings for P9 SBEFIFO CFAM device
+-----------------------------------------------
+
+Required properties:
+ - compatible = "ibm,p9-sbefifo";
+ - reg = < address size >;		: FSI CFAM address for the SBE engine
+					  and address space size.
+ - #address-cells = <1>;		: Number of address cells in child
+					  nodes.
+ - #size-cells = <0>;			: Number of size cells in child nodes.
+
+Optional properties:
+ - <child nodes>			: Devices that are accessible through
+					  the SBEFIFO. Child nodes are "opaque"
+					  to the SBEFIFO; the SBEFIFO needs no
+					  information about a child node other
+					  than if it exists or not.
+
+Child node optional properties:
+ - compatible = "dts-name"
+ - reg = < address >;			: Meaningful only for the child node
+
+Examples:
+
+    sbefifo@2400 {
+        compatible = "ibm,p9-sbefifo";
+        reg = < 0x2400 0x400 >;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        occ@1 {
+            compatible = "ibm,p9-occ";
+            reg = < 1 >;
+        };
+    };
-- 
1.8.3.1

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

* [PATCH v4 2/4] drivers/fsi: Add SBEFIFO FSI client device driver
  2017-11-17 19:34 [PATCH v4 0/4] drivers/fsi: Add SBEFIFO client driver Eddie James
  2017-11-17 19:34 ` [PATCH v4 1/4] dt-bindings: fsi: Add SBEFIFO documentation Eddie James
@ 2017-11-17 19:34 ` Eddie James
  2017-11-17 19:34 ` [PATCH v4 3/4] drivers/fsi: sbefifo: Add miscdevice Eddie James
  2017-11-17 19:34 ` [PATCH v4 4/4] drivers/fsi: sbefifo: Add in-kernel API Eddie James
  3 siblings, 0 replies; 10+ messages in thread
From: Eddie James @ 2017-11-17 19:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, devicetree, robh+dt, mark.rutland, bradleyb, cbostic,
	joel, eajames, Edward A. James

From: "Edward A. James" <eajames@us.ibm.com>

IBM POWER9 processors contain some embedded hardware and software bits
collectively referred to as the self boot engine (SBE).  One role of
the SBE is to act as a proxy that provides access to the registers of
the POWER chip from other (embedded) systems.

The POWER9 chip contains a hardware frontend for communicating with
the SBE from remote systems called the SBEFIFO.  The SBEFIFO logic
is contained within an FSI CFAM  and as such the driver implements an
FSI bus device.

The SBE expects to communicate using a defined wire protocol; however,
the driver knows nothing of the protocol and only provides raw access
to the fifo device to userspace applications wishing to communicate with
the SBE using the wire protocol.

The SBEFIFO consists of two hardware fifos.  The upstream fifo is used
by the driver to transfer data to the SBE on the POWER chip, from the
system hosting the driver.  The downstream fifo is used by the driver to
transfer data from the SBE on the power chip to the system hosting the
driver.

Contributions from Brad Bishop <bradleyb@fuzziesquirrel.com>

Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
 drivers/fsi/Kconfig       |   7 +
 drivers/fsi/Makefile      |   1 +
 drivers/fsi/fsi-sbefifo.c | 575 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 583 insertions(+)
 create mode 100644 drivers/fsi/fsi-sbefifo.c

diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
index 6821ed0..8c4d903 100644
--- a/drivers/fsi/Kconfig
+++ b/drivers/fsi/Kconfig
@@ -33,6 +33,13 @@ config FSI_SCOM
 	---help---
 	This option enables an FSI based SCOM device driver.
 
+config FSI_SBEFIFO
+	tristate "SBEFIFO FSI client device driver"
+	---help---
+	This option enables an FSI based SBEFIFO device driver. The SBEFIFO is
+	a pipe-like FSI device for communicating with the self boot engine
+	(SBE) on POWER processors.
+
 endif
 
 endmenu
diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
index 65eb99d..851182e 100644
--- a/drivers/fsi/Makefile
+++ b/drivers/fsi/Makefile
@@ -3,3 +3,4 @@ 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
diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
new file mode 100644
index 0000000..2439958
--- /dev/null
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -0,0 +1,575 @@
+/*
+ * 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/idr.h>
+#include <linux/kernel.h>
+#include <linux/kref.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/poll.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/timer.h>
+#include <linux/uaccess.h>
+#include <linux/wait.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 timer_list poll_timer;
+	struct fsi_device *fsi_dev;
+	wait_queue_head_t wait;
+	struct list_head xfrs;
+	struct kref kref;
+	spinlock_t lock;
+	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;
+};
+
+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_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_poll_timer(unsigned long data)
+{
+	static const unsigned long EOT_MASK = 0x000000ff;
+	struct sbefifo *sbefifo = (void *)data;
+	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;
+
+	spin_lock(&sbefifo->lock);
+	xfr = list_first_entry_or_null(&sbefifo->xfrs, struct sbefifo_xfr,
+				       xfrs);
+	if (!xfr)
+		goto out_unlock;
+
+	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. */
+			sbefifo->poll_timer.expires = jiffies +
+				SBEFIFO_RESCHEDULE;
+			add_timer(&sbefifo->poll_timer);
+			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. */
+			sbefifo->poll_timer.expires = jiffies +
+				SBEFIFO_RESCHEDULE;
+			add_timer(&sbefifo->poll_timer);
+			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);
+			list_del(&xfr->xfrs);
+			if (unlikely(test_bit(SBEFIFO_XFR_CANCEL,
+					      &xfr->flags)))
+				kfree(xfr);
+			break;
+		}
+	}
+
+out:
+	if (unlikely(ret)) {
+		sbefifo->rc = ret;
+		dev_err(&sbefifo->fsi_dev->dev,
+			"Fatal bus access failure: %d\n", ret);
+		list_for_each_entry_safe(xfr, tmp, &sbefifo->xfrs, xfrs) {
+			list_del(&xfr->xfrs);
+			kfree(xfr);
+		}
+		INIT_LIST_HEAD(&sbefifo->xfrs);
+
+	} else if (eot && sbefifo_next_xfr(sbefifo)) {
+		sbefifo_get(sbefifo);
+		sbefifo->poll_timer.expires = jiffies;
+		add_timer(&sbefifo->poll_timer);
+	}
+
+	sbefifo_put(sbefifo);
+	wake_up_interruptible(&sbefifo->wait);
+
+out_unlock:
+	spin_unlock(&sbefifo->lock);
+}
+
+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;
+	u32 up, down;
+	int ret;
+
+	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;
+		}
+	}
+
+	spin_lock_init(&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... */
+	setup_timer(&sbefifo->poll_timer, sbefifo_poll_timer,
+		    (unsigned long)sbefifo);
+
+	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;
+
+	spin_lock(&sbefifo->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);
+
+	spin_unlock(&sbefifo->lock);
+
+	wake_up_all(&sbefifo->wait);
+
+	ida_simple_remove(&sbefifo_ida, sbefifo->idx);
+
+	if (del_timer_sync(&sbefifo->poll_timer))
+		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)
+{
+	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_DESCRIPTION("Linux device interface to the POWER Self Boot Engine");
-- 
1.8.3.1

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

* [PATCH v4 3/4] drivers/fsi: sbefifo: Add miscdevice
  2017-11-17 19:34 [PATCH v4 0/4] drivers/fsi: Add SBEFIFO client driver Eddie James
  2017-11-17 19:34 ` [PATCH v4 1/4] dt-bindings: fsi: Add SBEFIFO documentation Eddie James
  2017-11-17 19:34 ` [PATCH v4 2/4] drivers/fsi: Add SBEFIFO FSI client device driver Eddie James
@ 2017-11-17 19:34 ` Eddie James
  2017-11-17 19:34 ` [PATCH v4 4/4] drivers/fsi: sbefifo: Add in-kernel API Eddie James
  3 siblings, 0 replies; 10+ messages in thread
From: Eddie James @ 2017-11-17 19:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, devicetree, robh+dt, mark.rutland, bradleyb, cbostic,
	joel, eajames, Edward A. James

From: "Edward A. James" <eajames@us.ibm.com>

Add a miscdevice for the sbefifo to allow userspace access through
standard file operations.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
 drivers/fsi/fsi-sbefifo.c | 354 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 354 insertions(+)

diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
index 2439958..9970e2d 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -19,6 +19,7 @@
 #include <linux/kernel.h>
 #include <linux/kref.h>
 #include <linux/list.h>
+#include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/poll.h>
 #include <linux/sched.h>
@@ -54,6 +55,7 @@
 struct sbefifo {
 	struct timer_list poll_timer;
 	struct fsi_device *fsi_dev;
+	struct miscdevice mdev;
 	wait_queue_head_t wait;
 	struct list_head xfrs;
 	struct kref kref;
@@ -256,6 +258,99 @@ 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);
+
+	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;
+
+	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_client_release(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 (mod_timer(&client->dev->poll_timer, jiffies))
+				sbefifo_put(sbefifo);
+		}
+	}
+
+	sbefifo_put(sbefifo);
+	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_client_release);
+}
+
 static struct sbefifo_xfr *sbefifo_next_xfr(struct sbefifo *sbefifo)
 {
 	struct sbefifo_xfr *xfr, *tmp;
@@ -429,6 +524,251 @@ static void sbefifo_poll_timer(unsigned long data)
 	spin_unlock(&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;
+
+	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(struct file *file, char __user *buf, size_t len,
+			    loff_t *offset)
+{
+	struct sbefifo_client *client = file->private_data;
+	struct sbefifo *sbefifo = client->dev;
+	struct sbefifo_xfr *xfr;
+	size_t n;
+	ssize_t ret = 0;
+
+	if ((len >> 2) << 2 != len)
+		return -EINVAL;
+
+	if ((file->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;
+	}
+
+	n = min_t(size_t, n, len);
+
+	if (copy_to_user(buf, READ_ONCE(client->rbuf.rpos), n)) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	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 (mod_timer(&client->dev->poll_timer, jiffies))
+				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 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(struct file *file, const char __user *buf,
+			     size_t len, loff_t *offset)
+{
+	struct sbefifo_client *client = file->private_data;
+	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);
+
+	spin_lock_irq(&sbefifo->lock);
+	xfr = sbefifo_next_xfr(sbefifo);	/* next xfr to be executed */
+
+	if ((file->f_flags & O_NONBLOCK) && xfr && n < len) {
+		spin_unlock_irq(&sbefifo->lock);
+		ret = -EAGAIN;
+		goto out;
+	}
+
+	xfr = sbefifo_enq_xfr(client);		/* this xfr queued up */
+	if (IS_ERR(xfr)) {
+		spin_unlock_irq(&sbefifo->lock);
+		ret = PTR_ERR(xfr);
+		goto out;
+	}
+
+	spin_unlock_irq(&sbefifo->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 (mod_timer(&sbefifo->poll_timer, jiffies))
+				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 (copy_from_user(READ_ONCE(client->wbuf.wpos), buf, n)) {
+			set_bit(SBEFIFO_XFR_CANCEL, &xfr->flags);
+			sbefifo_get(sbefifo);
+			if (mod_timer(&sbefifo->poll_timer, jiffies))
+				sbefifo_put(sbefifo);
+			ret = -EFAULT;
+			goto out;
+		}
+
+		buf += 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 (mod_timer(&client->dev->poll_timer, jiffies))
+			sbefifo_put(sbefifo);
+	}
+
+out:
+	sbefifo_put_client(client);
+	return ret;
+}
+
+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,
+};
+
 static int sbefifo_request_reset(struct sbefifo *sbefifo)
 {
 	int ret;
@@ -503,6 +843,18 @@ static int sbefifo_probe(struct device *dev)
 	setup_timer(&sbefifo->poll_timer, sbefifo_poll_timer,
 		    (unsigned long)sbefifo);
 
+	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;
+	}
+
 	dev_set_drvdata(dev, sbefifo);
 
 	return 0;
@@ -527,6 +879,8 @@ static int sbefifo_remove(struct device *dev)
 
 	wake_up_all(&sbefifo->wait);
 
+	misc_deregister(&sbefifo->mdev);
+
 	ida_simple_remove(&sbefifo_ida, sbefifo->idx);
 
 	if (del_timer_sync(&sbefifo->poll_timer))
-- 
1.8.3.1

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

* [PATCH v4 4/4] drivers/fsi: sbefifo: Add in-kernel API
  2017-11-17 19:34 [PATCH v4 0/4] drivers/fsi: Add SBEFIFO client driver Eddie James
                   ` (2 preceding siblings ...)
  2017-11-17 19:34 ` [PATCH v4 3/4] drivers/fsi: sbefifo: Add miscdevice Eddie James
@ 2017-11-17 19:34 ` Eddie James
  2017-11-21  8:07   ` kbuild test robot
  2017-12-01 13:07   ` [PATCH] fsi: Add Self Boot Engine FIFO FSI client Andrew Jeffery
  3 siblings, 2 replies; 10+ messages in thread
From: Eddie James @ 2017-11-17 19:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, devicetree, robh+dt, mark.rutland, bradleyb, cbostic,
	joel, eajames, Edward A. James

From: "Edward A. James" <eajames@us.ibm.com>

Refactor the user interface of the SBEFIFO driver to allow for an
in-kernel read/write API. Add exported functions for other drivers to
call, and add an include file with those functions. Also parse the
device tree for child nodes and create child platform devices
accordingly.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
 drivers/fsi/fsi-sbefifo.c   | 137 +++++++++++++++++++++++++++++++++++++-------
 include/linux/fsi-sbefifo.h |  30 ++++++++++
 2 files changed, 146 insertions(+), 21 deletions(-)
 create mode 100644 include/linux/fsi-sbefifo.h

diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
index 9970e2d..92947b0 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -15,12 +15,16 @@
 #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/of.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
 #include <linux/poll.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
@@ -92,6 +96,7 @@ struct sbefifo_client {
 	struct list_head xfrs;
 	struct sbefifo *dev;
 	struct kref kref;
+	unsigned long f_flags;
 };
 
 static DEFINE_IDA(sbefifo_ida);
@@ -540,6 +545,7 @@ static int sbefifo_open(struct inode *inode, struct file *file)
 		return -ENOMEM;
 
 	file->private_data = client;
+	client->f_flags = file->f_flags;
 
 	return 0;
 }
@@ -579,10 +585,9 @@ static bool sbefifo_read_ready(struct sbefifo *sbefifo,
 		(xfr && test_bit(SBEFIFO_XFR_COMPLETE, &xfr->flags));
 }
 
-static ssize_t sbefifo_read(struct file *file, char __user *buf, size_t len,
-			    loff_t *offset)
+static ssize_t sbefifo_read_common(struct sbefifo_client *client,
+				   char __user *ubuf, char *kbuf, size_t len)
 {
-	struct sbefifo_client *client = file->private_data;
 	struct sbefifo *sbefifo = client->dev;
 	struct sbefifo_xfr *xfr;
 	size_t n;
@@ -591,7 +596,7 @@ static ssize_t sbefifo_read(struct file *file, char __user *buf, size_t len,
 	if ((len >> 2) << 2 != len)
 		return -EINVAL;
 
-	if ((file->f_flags & O_NONBLOCK) && !sbefifo_xfr_rsp_pending(client))
+	if ((client->f_flags & O_NONBLOCK) && !sbefifo_xfr_rsp_pending(client))
 		return -EAGAIN;
 
 	sbefifo_get_client(client);
@@ -609,9 +614,13 @@ static ssize_t sbefifo_read(struct file *file, char __user *buf, size_t len,
 
 	n = min_t(size_t, n, len);
 
-	if (copy_to_user(buf, READ_ONCE(client->rbuf.rpos), n)) {
-		ret = -EFAULT;
-		goto out;
+	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)) {
@@ -643,6 +652,14 @@ static ssize_t sbefifo_read(struct file *file, char __user *buf, size_t len,
 	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)
@@ -655,10 +672,10 @@ static bool sbefifo_write_ready(struct sbefifo *sbefifo,
 	return READ_ONCE(sbefifo->rc) || (next == xfr && *n);
 }
 
-static ssize_t sbefifo_write(struct file *file, const char __user *buf,
-			     size_t len, loff_t *offset)
+static ssize_t sbefifo_write_common(struct sbefifo_client *client,
+				    const char __user *ubuf, const char *kbuf,
+				    size_t len)
 {
-	struct sbefifo_client *client = file->private_data;
 	struct sbefifo *sbefifo = client->dev;
 	struct sbefifo_xfr *xfr;
 	ssize_t ret = 0;
@@ -676,7 +693,7 @@ static ssize_t sbefifo_write(struct file *file, const char __user *buf,
 	spin_lock_irq(&sbefifo->lock);
 	xfr = sbefifo_next_xfr(sbefifo);	/* next xfr to be executed */
 
-	if ((file->f_flags & O_NONBLOCK) && xfr && n < len) {
+	if ((client->f_flags & O_NONBLOCK) && xfr && n < len) {
 		spin_unlock_irq(&sbefifo->lock);
 		ret = -EAGAIN;
 		goto out;
@@ -717,16 +734,22 @@ static ssize_t sbefifo_write(struct file *file, const char __user *buf,
 
 		n = min_t(size_t, n, len);
 
-		if (copy_from_user(READ_ONCE(client->wbuf.wpos), buf, n)) {
-			set_bit(SBEFIFO_XFR_CANCEL, &xfr->flags);
-			sbefifo_get(sbefifo);
-			if (mod_timer(&sbefifo->poll_timer, jiffies))
-				sbefifo_put(sbefifo);
-			ret = -EFAULT;
-			goto out;
-		}
+		if (ubuf) {
+			if (copy_from_user(READ_ONCE(client->wbuf.wpos), ubuf,
+			    n)) {
+				set_bit(SBEFIFO_XFR_CANCEL, &xfr->flags);
+				sbefifo_get(sbefifo);
+				if (mod_timer(&sbefifo->poll_timer, jiffies))
+					sbefifo_put(sbefifo);
+				ret = -EFAULT;
+				goto out;
+			}
 
-		buf += n;
+			ubuf += n;
+		} else {
+			memcpy(READ_ONCE(client->wbuf.wpos), kbuf, n);
+			kbuf += n;
+		}
 
 		sbefifo_buf_wrotenb(&client->wbuf, n);
 		len -= n;
@@ -750,6 +773,14 @@ static ssize_t sbefifo_write(struct file *file, const char __user *buf,
 	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;
@@ -769,6 +800,56 @@ static int sbefifo_release(struct inode *inode, struct file *file)
 	.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;
@@ -803,8 +884,11 @@ 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;
+	int ret, child_idx = 0;
 
 	dev_dbg(dev, "Found sbefifo device\n");
 	sbefifo = kzalloc(sizeof(*sbefifo), GFP_KERNEL);
@@ -855,6 +939,16 @@ static int sbefifo_probe(struct device *dev)
 		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;
@@ -880,6 +974,7 @@ static int sbefifo_remove(struct device *dev)
 	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);
 
diff --git a/include/linux/fsi-sbefifo.h b/include/linux/fsi-sbefifo.h
new file mode 100644
index 0000000..8e55891
--- /dev/null
+++ b/include/linux/fsi-sbefifo.h
@@ -0,0 +1,30 @@
+/*
+ * 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 */
-- 
1.8.3.1

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

* Re: [PATCH v4 4/4] drivers/fsi: sbefifo: Add in-kernel API
  2017-11-17 19:34 ` [PATCH v4 4/4] drivers/fsi: sbefifo: Add in-kernel API Eddie James
@ 2017-11-21  8:07   ` kbuild test robot
  2017-12-01 13:07   ` [PATCH] fsi: Add Self Boot Engine FIFO FSI client Andrew Jeffery
  1 sibling, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2017-11-21  8:07 UTC (permalink / raw)
  To: Eddie James
  Cc: kbuild-all, linux-kernel, gregkh, devicetree, robh+dt,
	mark.rutland, bradleyb, cbostic, joel, eajames, Edward A. James

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

Hi Edward,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.14 next-20171120]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Eddie-James/drivers-fsi-Add-SBEFIFO-client-driver/20171121-024602
config: um-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=um 

All errors (new ones prefixed by >>):

   WARNING: "memcpy" [drivers/net/ppp/ppp_generic.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/ppp/ppp_async.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/phy/xilinx_gmii2rgmii.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/phy/micrel.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/phy/mdio-gpio.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/phy/marvell.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/phy/bcm-phy-lib.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/netconsole.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/macvlan.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/macsec.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/ipvlan/ipvlan.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/hamradio/yam.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/hamradio/mkiss.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/hamradio/hdlcdrv.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/hamradio/bpqether.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/hamradio/6pack.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/geneve.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/ethernet/qualcomm/rmnet/rmnet.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/ethernet/qualcomm/qcauart.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/ethernet/mellanox/mlxsw/mlxsw_i2c.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/ethernet/mellanox/mlxsw/mlxsw_core.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/dummy.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/dsa/mv88e6xxx/mv88e6xxx.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/dsa/microchip/ksz_common.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/dsa/dsa_loop.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/dsa/b53/b53_common.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/can/slcan.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/can/can-dev.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/caif/caif_serial.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/caif/caif_hsi.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/bonding/bonding.ko] has no CRC!
   WARNING: "memcpy" [drivers/net/appletalk/ipddp.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/ubi/ubi.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/tests/mtd_pagetest.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/tests/mtd_nandecctest.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/tests/mtd_nandbiterrs.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/spi-nor/spi-nor.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/sm_ftl.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/nftl.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/nand/nandsim.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/nand/nand.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/mtdswap.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/mtdblock.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/mtd.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/lpddr/qinfo_probe.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/lpddr/lpddr_cmds.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/inftl.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/chips/map_rom.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/chips/map_ram.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/chips/jedec_probe.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/chips/gen_probe.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/chips/cfi_util.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/chips/cfi_probe.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/chips/cfi_cmdset_0020.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/chips/cfi_cmdset_0002.ko] has no CRC!
   WARNING: "memcpy" [drivers/mtd/chips/cfi_cmdset_0001.ko] has no CRC!
   WARNING: "memcpy" [drivers/misc/ti-st/st_drv.ko] has no CRC!
   WARNING: "memcpy" [drivers/misc/lkdtm.ko] has no CRC!
   WARNING: "memcpy" [drivers/misc/eeprom/max6875.ko] has no CRC!
   WARNING: "memcpy" [drivers/misc/eeprom/idt_89hpesx.ko] has no CRC!
   WARNING: "memcpy" [drivers/misc/eeprom/eeprom.ko] has no CRC!
   WARNING: "memcpy" [drivers/misc/eeprom/at24.ko] has no CRC!
   WARNING: "memcpy" [drivers/misc/echo/echo.ko] has no CRC!
   WARNING: "memcpy" [drivers/memstick/core/mspro_block.ko] has no CRC!
   WARNING: "memcpy" [drivers/memstick/core/memstick.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/raid456.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/raid10.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/raid1.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/persistent-data/dm-persistent-data.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/md-mod.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/md-cluster.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-zoned.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-verity.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-thin-pool.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-snapshot.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-mod.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-log.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-log-writes.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-log-userspace.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-integrity.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-era.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-crypt.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-cache.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/dm-bio-prison.ko] has no CRC!
   WARNING: "memcpy" [drivers/md/bcache/bcache.ko] has no CRC!
   WARNING: "memcpy" [drivers/leds/leds-tca6507.ko] has no CRC!
   WARNING: "memcpy" [drivers/iio/light/tsl2583.ko] has no CRC!
   WARNING: "memcpy" [drivers/iio/industrialio.ko] has no CRC!
   WARNING: "memcpy" [drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.ko] has no CRC!
   WARNING: "memcpy" [drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.ko] has no CRC!
   WARNING: "memcpy" [drivers/iio/imu/inv_mpu6050/inv-mpu6050.ko] has no CRC!
   WARNING: "memcpy" [drivers/iio/humidity/hts221_i2c.ko] has no CRC!
   WARNING: "memcpy" [drivers/iio/health/max30102.ko] has no CRC!
   WARNING: "memcpy" [drivers/iio/adc/ti-ads1015.ko] has no CRC!
   WARNING: "memcpy" [drivers/iio/accel/mma9551_core.ko] has no CRC!
   WARNING: "memcpy" [drivers/iio/accel/bmc150-accel-core.ko] has no CRC!
   WARNING: "memcpy" [drivers/i2c/muxes/i2c-mux-gpio.ko] has no CRC!
   WARNING: "memcpy" [drivers/i2c/i2c-slave-eeprom.ko] has no CRC!
   WARNING: "memcpy" [drivers/i2c/i2c-core.ko] has no CRC!
   WARNING: "memcpy" [drivers/gpio/gpio-pca953x.ko] has no CRC!
>> ERROR: "of_platform_device_create" [drivers/fsi/fsi-sbefifo.ko] undefined!
   WARNING: "memcpy" [drivers/fsi/fsi-sbefifo.ko] has no CRC!
   WARNING: "memcpy" [drivers/fsi/fsi-core.ko] has no CRC!
   WARNING: "memcpy" [drivers/fmc/fmc.ko] has no CRC!
   WARNING: "memcpy" [drivers/fmc/fmc-fakedev.ko] has no CRC!
   WARNING: "memcpy" [drivers/connector/cn.ko] has no CRC!
   WARNING: "memcpy" [drivers/cdrom/cdrom.ko] has no CRC!
   WARNING: "memcpy" [drivers/bluetooth/hci_vhci.ko] has no CRC!
   WARNING: "memcpy" [drivers/bluetooth/hci_uart.ko] has no CRC!
   WARNING: "memcpy" [drivers/bluetooth/btwilink.ko] has no CRC!
   WARNING: "memcpy" [drivers/bluetooth/btqca.ko] has no CRC!
   WARNING: "memcpy" [drivers/bluetooth/btmrvl.ko] has no CRC!
   WARNING: "memcpy" [drivers/bluetooth/btintel.ko] has no CRC!
   WARNING: "memcpy" [drivers/block/rbd.ko] has no CRC!
   WARNING: "memcpy" [drivers/block/null_blk.ko] has no CRC!
   WARNING: "memcpy" [drivers/block/nbd.ko] has no CRC!
   WARNING: "memcpy" [drivers/block/loop.ko] has no CRC!
   WARNING: "memcpy" [drivers/block/drbd/drbd.ko] has no CRC!
   WARNING: "memcpy" [drivers/block/cryptoloop.ko] has no CRC!
   WARNING: "memcpy" [drivers/block/brd.ko] has no CRC!
   WARNING: "memcpy" [drivers/block/aoe/aoe.ko] has no CRC!
   WARNING: "memcpy" [drivers/base/firmware_class.ko] has no CRC!
   ERROR: "devm_ioremap_resource" [drivers/auxdisplay/img-ascii-lcd.ko] undefined!
   WARNING: "memcpy" [drivers/auxdisplay/img-ascii-lcd.ko] has no CRC!
   WARNING: "memcpy" [drivers/atm/atmtcp.ko] has no CRC!
   WARNING: "memcpy" [crypto/xcbc.ko] has no CRC!
   WARNING: "memcpy" [crypto/wp512.ko] has no CRC!
   WARNING: "memcpy" [crypto/vmac.ko] has no CRC!
   WARNING: "memcpy" [crypto/tgr192.ko] has no CRC!
   WARNING: "memcpy" [crypto/sm3_generic.ko] has no CRC!
   WARNING: "memcpy" [crypto/sha512_generic.ko] has no CRC!
   WARNING: "memcpy" [crypto/sha3_generic.ko] has no CRC!
   WARNING: "memcpy" [crypto/salsa20_generic.ko] has no CRC!
   WARNING: "memcpy" [crypto/rmd320.ko] has no CRC!
   WARNING: "memcpy" [crypto/rmd256.ko] has no CRC!
   WARNING: "memcpy" [crypto/rmd160.ko] has no CRC!
   WARNING: "memcpy" [crypto/rmd128.ko] has no CRC!
   WARNING: "memcpy" [crypto/poly1305_generic.ko] has no CRC!
   WARNING: "memcpy" [crypto/pcbc.ko] has no CRC!
   WARNING: "memcpy" [crypto/michael_mic.ko] has no CRC!
   WARNING: "memcpy" [crypto/md4.ko] has no CRC!
   WARNING: "memcpy" [crypto/mcryptd.ko] has no CRC!
   WARNING: "memcpy" [crypto/lrw.ko] has no CRC!
   WARNING: "memcpy" [crypto/keywrap.ko] has no CRC!
   WARNING: "memcpy" [crypto/fcrypt.ko] has no CRC!
   WARNING: "memcpy" [crypto/echainiv.ko] has no CRC!
   WARNING: "memcpy" [crypto/ecdh_generic.ko] has no CRC!
   WARNING: "memcpy" [crypto/des_generic.ko] has no CRC!
   WARNING: "memcpy" [crypto/cts.ko] has no CRC!
   WARNING: "memcpy" [crypto/cryptd.ko] has no CRC!
   WARNING: "memcpy" [crypto/cmac.ko] has no CRC!
   WARNING: "memcpy" [crypto/chacha20poly1305.ko] has no CRC!
   WARNING: "memcpy" [crypto/chacha20_generic.ko] has no CRC!
   WARNING: "memcpy" [crypto/ccm.ko] has no CRC!
   WARNING: "memcpy" [crypto/cast6_generic.ko] has no CRC!
   WARNING: "memcpy" [crypto/cast5_generic.ko] has no CRC!
   WARNING: "memcpy" [crypto/camellia_generic.ko] has no CRC!
   WARNING: "memcpy" [crypto/async_tx/async_memcpy.ko] has no CRC!
   WARNING: "memcpy" [crypto/ansi_cprng.ko] has no CRC!
   WARNING: "memcpy" [crypto/af_alg.ko] has no CRC!

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

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

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

* [PATCH] fsi: Add Self Boot Engine FIFO FSI client
  2017-11-17 19:34 ` [PATCH v4 4/4] drivers/fsi: sbefifo: Add in-kernel API Eddie James
  2017-11-21  8:07   ` kbuild test robot
@ 2017-12-01 13:07   ` Andrew Jeffery
  2017-12-03 20:25     ` kbuild test robot
  2017-12-04  4:50     ` kbuild test robot
  1 sibling, 2 replies; 10+ messages in thread
From: Andrew Jeffery @ 2017-12-01 13:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Jeffery, gregkh, devicetree, robh+dt, mark.rutland,
	bradleyb, cbostic, joel, eajames, Edward A. James

The POWER Self Boot Engine (SBE) is an on-chip micro-controller that cranks
POWER processors into action and provides support for some runtime services.
One such runtime service is mediation of communication between the service
processor (that initiated the boot process) and the POWER chip at large. This
is handled using message passing over a pair of FIFOs on top of an FSI bus
between the service processor and the POWER chip.

This driver implements support for management of the SBE FIFO interface to the
POWER chip.

The FIFO semantics are fairly straight forward:

1. A command message is enqueued from the service processor to the upstream
   FIFO, followed by queuing an End-of-Transfer (EOT) marker
2. The message on the upstream FIFO is consumed by the SBE and acted on as
   required
3. The SBE responds on the downstream FIFO, terminating its response with an
   EOT marker

Additionally, the following rules apply:

1. Only one command is accepted on the upstream FIFO until the EOT marker on
   the downstream FIFO is dequeued.
2. A response will not be enqueued on the downstream FIFO until a complete
   command with EOT has been dequeued from the upstream FIFO

A significant pain point in the design is an interrupt is not available to the
service processor to know when data is available for dequeuing from the
downstream FIFO. As a consequence the driver sets up a timer as required to
poll the hardware state and drive consumption of a response when enqueued.

If the FIFOs become corrupt via either parity or logic faults, they can be
reset. With respect to resets the SBE will always act as a slave and will never
initiate a FIFO reset, rather the service processor must request the reset and
receive an acknowledgement from the SBE that the reset has been performed.

Both FIFOs are implemented in hardware as a ring of eight 32-bit data slots,
where each slot has associated EOT and validity bits.

The implementation provides mutual exclusion over access to the FIFO to enforce
integrity whilst supporting multiple open clients. An in-kernel API is exposed
that maps nicely onto character device semantics, making way for a
straight-forward patch to expose the FIFO to userspace e.g. via a misc-device.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
Hello,

I'm reviewing the patch by way of offering an alternative implementation. My
concerns with Eddie and Brad's implementation are:

1. Transfers are entirely handled by the timer callback, driving complexity
   with enqueuing and dequeuing jobs from the "xfers" list (which are consumed
   by the callback). This enqueuing/dequeuing is necessary even if the transfer
   is started immediately, as this is done by tripping the timer using a zero
   offset from jiffies. In the past we ran into a number of instances of memory
   corruption with mishandling of the list.
2. Delegation of transfers to the timer callback leads to complexity of
   reference counting for the FIFO and clients as everything is done
   asynchronously, increasing the cleanup complexity
3. Complexity of the transfer state machine, represented by properties that are
   not mutually exclusive (i.e. a state is built from a set of independent bit
   states).
4. Unergonomic interfaces for handling in-kernel vs userspace consumers: For
   instance sbefifo_read_common() takes both userspace and kernel buffer
   pointers as arguments, one of which is expected to be NULL, and the
   userspace pointer takes preference. I think a better design would be to
   separate the userspace interface from the driver, and make the userspace
   implementation call through the in-kernel APIs.
5. The sbefifo_buf struct mirrors the hardware ringbuffers with in-memory ring
   buffers, adding a layer of indirection that feels odd
6. Unclear locking semantics combined with {READ,WRITE}_ONCE() makes it more
   difficult to verify correct behaviour

So, I thought I'd have a crack at implementing something more direct. It may
still have disadvantages - for instance I haven't thought about how to
implement non-blocking IO with my approach, which is something that Eddie's
patch has. It may prove to be an Achilles heal.

The two approaches roughly break even in size when the chardev interface is
implemented (not included here).

Something that I hoped would turn out simpler than it did was state management
of the FIFO, clients and the polling thread. But I think there are still
benefits to having explicit states.

So the approach I've taken is:

1. Reduce the timer callback to something approximating an interrupt: When it
   detects the FIFO is ready it simply issues a wake_up() to the waiter on the
   poll waitqueue.
2. The consequence of this is we no-longer need the xfer lists and intermediate
   buffers, as the FIFO is directly accessed the calling process
3. We wind up with a system of three waitqueues, reflecting the three concepts
   required for use of the FIFO: One to mediate access to the FIFO, woken on
   FIFO state changes; one for each client, woken when it's the client's turn
   to access the FIFO, and one for the timer, woken when the hardware becomes
   ready for reading or writing.
4. With the waitqueues we use two locks: The FIFO waitqueue lock covers FIFO
   and client state changes, whilst the timer waitqueue lock covers changes to
   the polling state.
5. Between the waitqueues and state machines we mostly only have to hold the
   locks on state transitions. The exception is on enqueuing and dequeuing data
   from the FIFO, where we need to first check if the client has been
   release()ed or the driver remove()ed

I've tested the patch by reworking the OCC/hwmon patches on top of the
in-kernel interface implemented here. It's survived light testing with probing
and reading values via the hwmon sysfs interface. It survives reboots of the
host, then killing the host to exercise the cleanup on remove. The final test
was booting with the host "half-on" (power applied but nothing further), where
we can talk to the FIFO but the SBE is not active. This exercises the timeout
paths in the driver on probe. It survived these tests, but this still falls far
short of the testing that Eddie's driver has seen internally, which is another
feather in its cap.

I'm interested in everyone's thoughts on the pros and cons of each approach.

Cheers,

Andrew

 drivers/fsi/Kconfig               |   2 +
 drivers/fsi/Makefile              |   9 +-
 drivers/fsi/clients/Kconfig       |   5 +
 drivers/fsi/clients/Makefile      |   1 +
 drivers/fsi/clients/fsi-sbefifo.c | 892 ++++++++++++++++++++++++++++++++++++++
 drivers/fsi/clients/fsi-sbefifo.h |  82 ++++
 6 files changed, 987 insertions(+), 4 deletions(-)
 create mode 100644 drivers/fsi/clients/Kconfig
 create mode 100644 drivers/fsi/clients/Makefile
 create mode 100644 drivers/fsi/clients/fsi-sbefifo.c
 create mode 100644 drivers/fsi/clients/fsi-sbefifo.h

diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
index 6821ed0cd5e8..1a6548e3be92 100644
--- a/drivers/fsi/Kconfig
+++ b/drivers/fsi/Kconfig
@@ -33,6 +33,8 @@ config FSI_SCOM
 	---help---
 	This option enables an FSI based SCOM device driver.
 
+source "drivers/fsi/clients/Kconfig"
+
 endif
 
 endmenu
diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
index 65eb99dfafdb..8e158a299701 100644
--- a/drivers/fsi/Makefile
+++ b/drivers/fsi/Makefile
@@ -1,5 +1,6 @@
+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) += 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)		+= clients/
diff --git a/drivers/fsi/clients/Kconfig b/drivers/fsi/clients/Kconfig
new file mode 100644
index 000000000000..3cb3feba84bb
--- /dev/null
+++ b/drivers/fsi/clients/Kconfig
@@ -0,0 +1,5 @@
+config FSI_SBEFIFO
+	tristate "Self Boot Engine FIFO"
+	---help---
+	The Self Boot Engine FIFO manages communication between a service
+	processor and the Self Boot Engine on POWER processors.
diff --git a/drivers/fsi/clients/Makefile b/drivers/fsi/clients/Makefile
new file mode 100644
index 000000000000..d71a4de7aa0f
--- /dev/null
+++ b/drivers/fsi/clients/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_FSI_SBEFIFO)	+= fsi-sbefifo.o
diff --git a/drivers/fsi/clients/fsi-sbefifo.c b/drivers/fsi/clients/fsi-sbefifo.c
new file mode 100644
index 000000000000..c93c2d577f6d
--- /dev/null
+++ b/drivers/fsi/clients/fsi-sbefifo.c
@@ -0,0 +1,892 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/* Copyright (C) IBM Corporation 2017 */
+
+#include <linux/bitops.h>
+#include <linux/fsi.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+
+#include "fsi-sbefifo.h"
+
+#define FSI_ENGINE_ID_SBE		0x22
+
+#define SBEFIFO_FIFO_DEPTH		8
+
+#define SBEFIFO_UP_FIFO			0x0
+#define SBEFIFO_UP_STS			0x4
+#define   SBEFIFO_STS_PARITY		BIT(29)
+#define   SBEFIFO_STS_RESET		BIT(25)
+#define   SBEFIFO_STS_FULL		BIT(21)
+#define   SBEFIFO_STS_EMPTY		BIT(20)
+#define   SBEFIFO_STS_ENTRIES_SHIFT	16
+#define   SBEFIFO_STS_ENTRIES_MASK	GENMASK(19, 16)
+#define   SBEFIFO_STS_VALID_SHIFT	8
+#define   SBEFIFO_STS_VALID_MASK	GENMASK(15, 8)
+#define   SBEFIFO_STS_EOT_MASK		GENMASK(7, 0)
+#define SBEFIFO_UP_EOT			0x8
+#define SBEFIFO_UP_RESET_REQ		0xC
+
+#define SBEFIFO_DOWN_FIFO		0x40
+#define SBEFIFO_DOWN_STS		0x44
+#define SBEFIFO_DOWN_RESET		0x48
+#define SBEFIFO_DOWN_EOT_ACK		0x54
+
+#define SBEFIFO_POLL_INTERVAL		msecs_to_jiffies(50)
+#define SBEFIFO_LONG_TIMEOUT		msecs_to_jiffies(30 * 1000)
+
+LIST_HEAD(sbefifos);
+
+static DEFINE_IDA(sbefifo_ida);
+
+static int sbefifo_readl(struct sbefifo *fifo, u32 addr, u32 *word)
+{
+	__be32 raw;
+	int rv;
+
+	rv = fsi_device_read(fifo->fsi, addr, &raw, sizeof(raw));
+	if (rv < 0)
+		return rv;
+
+	*word = be32_to_cpu(raw);
+
+	return 0;
+}
+
+static int sbefifo_writel(struct sbefifo *fifo, u32 addr, u32 word)
+{
+	__be32 cooked = cpu_to_be32(word);
+
+	return fsi_device_write(fifo->fsi, addr, &cooked, sizeof(cooked));
+}
+
+#define sbefifo_up_sts(f, dp)	sbefifo_readl(f, SBEFIFO_UP_STS, dp)
+#define sbefifo_down_sts(f, dp)	sbefifo_readl(f, SBEFIFO_DOWN_STS, dp);
+
+#define sbefifo_parity(sts)	((sts) & SBEFIFO_STS_PARITY)
+#define sbefifo_populated(sts)	\
+	(((sts) & SBEFIFO_STS_ENTRIES_MASK) >> SBEFIFO_STS_ENTRIES_SHIFT)
+#define sbefifo_vacant(sts)	(SBEFIFO_FIFO_DEPTH - sbefifo_populated(sts))
+#define sbefifo_empty(sts)	((sts) & SBEFIFO_STS_EMPTY)
+#define sbefifo_full(sts)	((sts) & SBEFIFO_STS_FULL)
+#define sbefifo_eot_set(sts)	((sts) & SBEFIFO_STS_EOT_MASK)
+#define sbefifo_valid_set(sts)	\
+	(((sts) & SBEFIFO_STS_VALID_MASK) >> SBEFIFO_STS_VALID_SHIFT)
+
+#define sbefifo_reset_req(sts)	(!!((sts) & SBEFIFO_STS_RESET))
+#define sbefifo_do_reset(f)	sbefifo_writel(f, SBEFIFO_DOWN_RESET, 0)
+#define sbefifo_req_reset(f)	sbefifo_writel(f, SBEFIFO_UP_RESET_REQ, 0)
+
+static int sbefifo_wait_reset(struct sbefifo *fifo, unsigned long expire)
+{
+	u32 sts;
+	int rv;
+
+	do {
+		rv = sbefifo_up_sts(fifo, &sts);
+		if (rv < 0)
+			return rv;
+	} while (sbefifo_reset_req(sts) && time_before(jiffies, expire));
+
+	if (sbefifo_reset_req(sts)) {
+		dev_warn(fifo->dev, "FIFO reset request timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	dev_info(fifo->dev, "SBE acknowleged reset request, FIFO is reset\n");
+
+	return 0;
+}
+
+static int sbefifo_reset(struct sbefifo *fifo)
+{
+	int rv;
+
+	rv = sbefifo_req_reset(fifo);
+	if (!rv)
+		rv = sbefifo_wait_reset(fifo, jiffies + SBEFIFO_POLL_INTERVAL);
+
+	if (rv < 0)
+		dev_err(fifo->dev, "FIFO reset failed: %d\n", rv);
+
+	return rv;
+}
+
+static int sbefifo_wait(struct sbefifo *fifo, enum sbefifo_direction dir,
+			unsigned long period)
+{
+	struct sbefifo_poll *poll = &fifo->poll;
+	unsigned long flags;
+	bool ready;
+	u32 addr;
+	bool up;
+	u32 sts;
+	int rv;
+
+	up = (dir == sbefifo_up);
+	addr = up ? SBEFIFO_UP_STS : SBEFIFO_DOWN_STS;
+	rv = sbefifo_readl(fifo, addr, &sts);
+	if (rv < 0)
+		return rv;
+
+	ready = !(up ? sbefifo_full(sts) : sbefifo_empty(sts));
+	if (ready)
+		return 0;
+
+	dev_info(fifo->dev, "Polling for FIFO response every %ld jiffies (%s)",
+		SBEFIFO_POLL_INTERVAL, period ? "bounded" : "unbounded");
+
+	spin_lock_irqsave(&poll->wait.lock, flags);
+	poll->interval = SBEFIFO_POLL_INTERVAL;
+	poll->expire = period;
+	poll->expire_at = period ? jiffies + period : 0;
+	poll->state = sbefifo_poll_wait;
+	poll->dir = dir;
+	poll->rv = 0;
+
+	mod_timer(&poll->timer, jiffies + poll->interval);
+
+	if (period) {
+		rv = wait_event_interruptible_locked_irq(poll->wait,
+			(poll->state != sbefifo_poll_wait || poll->rv ||
+			 time_after(jiffies, poll->expire_at)));
+	} else {
+		rv = wait_event_interruptible_locked_irq(poll->wait,
+			(poll->state != sbefifo_poll_wait || poll->rv));
+	}
+
+	if (rv < 0) {
+		spin_unlock_irqrestore(&poll->wait.lock, flags);
+		return rv;
+	}
+
+	if (poll->state == sbefifo_poll_wait && !poll->rv) {
+		spin_unlock_irqrestore(&poll->wait.lock, flags);
+		return -ETIMEDOUT;
+	}
+
+	if (poll->state == sbefifo_poll_ready || poll->rv) {
+		rv = poll->rv;
+		spin_unlock_irqrestore(&poll->wait.lock, flags);
+		return rv;
+	}
+
+	WARN_ON(poll->state != sbefifo_poll_reset);
+	spin_unlock_irqrestore(&poll->wait.lock, flags);
+
+	return -EIO;
+}
+
+#define sbefifo_wait_vacant(f, p)	sbefifo_wait(f, sbefifo_up, p);
+#define sbefifo_wait_primed(f, p)	sbefifo_wait(f, sbefifo_down, p);
+
+static void sbefifo_poll_device(unsigned long context)
+{
+	struct sbefifo *fifo = (struct sbefifo *) context;
+	struct sbefifo_poll *poll = &fifo->poll;
+	unsigned long flags;
+	u32 addr;
+	bool up;
+	u32 sts;
+	int rv;
+
+	/* Sanity check poll settings */
+	spin_lock_irqsave(&poll->wait.lock, flags);
+	up = (poll->dir == sbefifo_up);
+	spin_unlock_irqrestore(&poll->wait.lock, flags);
+
+	/* Read status */
+	addr = up ? SBEFIFO_UP_STS : SBEFIFO_DOWN_STS;
+	rv = sbefifo_readl(fifo, addr, &sts);
+	if (rv < 0) {
+		poll->rv = rv;
+		wake_up(&poll->wait);
+		return;
+	}
+
+	/* Update poll state */
+	spin_lock_irqsave(&poll->wait.lock, flags);
+	if (sbefifo_parity(sts))
+		poll->state = sbefifo_poll_reset;
+	else if (!(up ? sbefifo_full(sts) : sbefifo_empty(sts)))
+		poll->state = sbefifo_poll_ready;
+
+	if (poll->state != sbefifo_poll_wait) {
+		wake_up_locked(&poll->wait);
+	} else if (poll->expire && time_after(jiffies, poll->expire_at)) {
+		wake_up_locked(&poll->wait);
+	} else {
+		dev_dbg(fifo->dev, "Not ready, waiting another %lu jiffies\n",
+				poll->interval);
+		mod_timer(&fifo->poll.timer, jiffies + poll->interval);
+	}
+	spin_unlock_irqrestore(&poll->wait.lock, flags);
+}
+
+/* Precondition: Upstream FIFO is not full */
+static int sbefifo_enqueue(struct sbefifo *fifo, u32 data)
+{
+	unsigned long flags;
+	int rv;
+
+	/* Detect if we need to bail due to release() or remove() */
+	spin_lock_irqsave(&fifo->wait.lock, flags);
+	if (likely(fifo->state == sbefifo_tx))
+		rv = fsi_device_write(fifo->fsi, SBEFIFO_UP_FIFO, &data,
+				      sizeof(data));
+	else
+		rv = -EIO;
+	spin_unlock_irqrestore(&fifo->wait.lock, flags);
+
+	return rv;
+}
+
+/* Precondition: Downstream FIFO is not empty */
+static int sbefifo_dequeue(struct sbefifo *fifo, u32 *data)
+{
+	unsigned long flags;
+	int rv;
+
+	/* Detect if we need to bail due to release() or remove() */
+	spin_lock_irqsave(&fifo->wait.lock, flags);
+	if (likely(fifo->state == sbefifo_rx))
+		rv = fsi_device_read(fifo->fsi, SBEFIFO_DOWN_FIFO, data,
+				     sizeof(*data));
+	else
+		rv = -EIO;
+	spin_unlock_irqrestore(&fifo->wait.lock, flags);
+
+	return rv;
+}
+
+static int sbefifo_fill(struct sbefifo *fifo, const u32 *buf, ssize_t len)
+{
+	ssize_t vacant, remaining;
+	u32 sts;
+	int rv;
+
+	rv = sbefifo_up_sts(fifo, &sts);
+	if (rv < 0)
+		return rv;
+
+	vacant = sbefifo_vacant(sts);
+
+	vacant = min(vacant, len);
+	remaining = vacant;
+	while (remaining--) {
+		rv = sbefifo_enqueue(fifo, *buf++);
+		if (rv < 0)
+			return rv;
+	}
+
+	return vacant;
+}
+
+static int sbefifo_signal_eot(struct sbefifo *fifo)
+{
+	int rv;
+
+	rv = sbefifo_wait_vacant(fifo, SBEFIFO_LONG_TIMEOUT);
+	if (rv < 0)
+		return rv;
+
+	rv = sbefifo_writel(fifo, SBEFIFO_UP_EOT, 0);
+	return rv;
+}
+
+static ssize_t sbefifo_up_write(struct sbefifo *fifo, const u32 *buf,
+				ssize_t len)
+{
+	ssize_t remaining = len;
+	int wrote;
+	int rv;
+
+	while (remaining) {
+		rv = sbefifo_wait_vacant(fifo, SBEFIFO_LONG_TIMEOUT);
+		if (rv < 0)
+			return rv;
+
+		wrote = sbefifo_fill(fifo, buf, len);
+		if (wrote < 0)
+			return wrote;
+
+		buf += wrote;
+		remaining -= wrote;
+	}
+
+	rv = sbefifo_signal_eot(fifo);
+	if (rv < 0)
+		return rv;
+
+	return len;
+}
+
+#define TEST_SET(s)	((s) & BIT(7))
+#define IS_EOT(s)	TEST_SET(s)
+#define IS_VALID(s)	TEST_SET(s)
+
+static int sbefifo_drain(struct sbefifo *fifo, u32 *buf, ssize_t len)
+{
+	ssize_t nr_valid;
+	u8 valid_set;
+	int nr_xfer;
+	ssize_t rem;
+	u8 eot_set;
+	u32 sts;
+	u32 val;
+	int rv;
+
+	rv = sbefifo_down_sts(fifo, &sts);
+	if (rv < 0)
+		return rv;
+
+	/* Determine tranfer characteristics */
+	nr_xfer = sbefifo_populated(sts);
+	valid_set = sbefifo_valid_set(sts);
+	eot_set = sbefifo_eot_set(sts);
+
+	if (hweight8(eot_set) > 1) {
+		dev_err(fifo->dev, "More than one EOT in the pipe!\n");
+		return -EIO;
+	}
+
+	/* Number of data words in the transfer */
+	nr_valid = hweight8(valid_set);
+	len = min(len, nr_valid);
+	rem = len;
+
+	dev_dbg(fifo->dev, "%s: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
+		__func__, valid_set, eot_set, nr_valid, nr_xfer, rem);
+
+	/* Dequeue data */
+	while (nr_xfer && rem && !IS_EOT(eot_set)) {
+		rv = sbefifo_dequeue(fifo, &val);
+		if (rv < 0)
+			return rv;
+
+		if (IS_VALID(valid_set)) {
+			*buf++ = val;
+			rem--;
+		}
+
+		valid_set <<= 1;
+		eot_set <<= 1;
+		nr_xfer--;
+	}
+
+	dev_dbg(fifo->dev, "%s: Data phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
+		__func__, valid_set, eot_set, nr_valid, nr_xfer, rem);
+
+	/*
+	 * To allow the upper layers to manage state transitions, don't dequeue
+	 * EOT yet. Leave that for the subsequent, terminating read.
+	 */
+	if (nr_valid > 0)
+		return len;
+
+	/* Dequeue and ACK EOT word */
+	while (nr_xfer && IS_EOT(eot_set) && !IS_VALID(valid_set)) {
+		rv = sbefifo_dequeue(fifo, &val);
+		if (rv < 0)
+			return rv;
+
+		rv = sbefifo_writel(fifo, SBEFIFO_DOWN_EOT_ACK, val);
+		if (rv < 0)
+			return rv;
+
+		valid_set <<= 1;
+		eot_set <<= 1;
+		nr_xfer--;
+	}
+
+	dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
+		__func__, valid_set, eot_set, nr_valid, nr_xfer, rem);
+
+	/* Dequeue any remaining dummy values */
+	while (nr_xfer && !IS_EOT(eot_set) && !IS_VALID(valid_set)) {
+		rv = sbefifo_dequeue(fifo, &val);
+		if (rv < 0)
+			return rv;
+
+		valid_set <<= 1;
+		eot_set <<= 1;
+		nr_xfer--;
+	}
+
+	dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
+		__func__, valid_set, eot_set, nr_valid, nr_xfer, rem);
+
+	/* Test for parity failures */
+	rv = sbefifo_down_sts(fifo, &sts);
+	if (rv < 0)
+		return rv;
+
+	if (sbefifo_parity(sts)) {
+		dev_warn(fifo->dev, "Downstream FIFO parity failure\n");
+		return -EIO;
+	}
+
+	return len;
+}
+
+static ssize_t sbefifo_down_read(struct sbefifo *fifo, u32 *buf, ssize_t len)
+{
+	ssize_t rem = len;
+	int read;
+	int rv;
+
+	if (!rem)
+		return 0;
+
+	do {
+		rv = sbefifo_wait_primed(fifo, SBEFIFO_LONG_TIMEOUT);
+		if (rv < 0)
+			return rv;
+
+		read = sbefifo_drain(fifo, buf, rem);
+		if (read < 0)
+			return read;
+
+		buf += read;
+		rem -= read;
+	} while (rem && read && read == min((rem + read), SBEFIFO_FIFO_DEPTH));
+
+	return len - rem;
+}
+
+/* In-kernel API */
+
+/**
+ * sbefifo_open()
+ *
+ * @client	The client context for the SBEFIFO
+ * @flags	Flags controlling how to open the client.
+ *
+ * Returns 0 on success or negative values on failure.
+ */
+int sbefifo_open(struct sbefifo *fifo, struct sbefifo_client *client,
+		 unsigned long oflags)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&fifo->wait.lock, flags);
+	if (fifo->state == sbefifo_dead) {
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return -ENODEV;
+	}
+	if (WARN(client->state != sbefifo_client_closed, "Already open\n")) {
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return -EINVAL;
+	}
+
+	/* No flags at the moment, probably O_NONBLOCK in the future */
+	if (oflags) {
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return -EINVAL;
+	}
+
+	init_waitqueue_head(&client->wait);
+	client->fifo = fifo;
+	client->flags = oflags;
+	client->state = sbefifo_client_idle;
+	spin_unlock_irqrestore(&fifo->wait.lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(sbefifo_open);
+
+/**
+ * sbefifo_write()
+ *
+ * @client	The client context for the SBEFIFO
+ * @buf		The buffer of data to write, at least @len elements
+ * @len		The number elements in @buffer
+ *
+ * The buffer must represent a complete chip-op: EOT is signalled after the
+ * last element is written to the upstream FIFO.
+ *
+ * Returns the number of elements written on success and negative values on
+ * failure. If the call is successful a subsequent call to sbefifo_read() MUST
+ * be made.
+ */
+ssize_t sbefifo_write(struct sbefifo_client *client, const u32 *buf,
+		      ssize_t len)
+{
+	struct sbefifo *fifo = client->fifo;
+	unsigned long flags;
+	ssize_t rv;
+
+	spin_lock_irqsave(&fifo->wait.lock, flags);
+
+	if (client->state == sbefifo_client_active) {
+		dev_warn(fifo->dev, "Transfer already in progress\n");
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return -EBUSY;
+	}
+
+	rv = wait_event_interruptible_locked_irq(fifo->wait,
+						fifo->state == sbefifo_ready ||
+						fifo->state == sbefifo_dead);
+	if (rv < 0) {
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return rv;
+	}
+
+	if (fifo->state == sbefifo_dead) {
+		client->state = sbefifo_client_closed;
+		wake_up(&client->wait);
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return -ENODEV;
+	}
+
+	WARN_ON(fifo->state != sbefifo_ready);
+
+	fifo->curr = client;
+	fifo->state = sbefifo_tx;
+
+	/* Move a threaded read() onto waiting for FIFO read readiness */
+	client->state = sbefifo_client_active;
+	wake_up(&client->wait);
+
+	spin_unlock_irqrestore(&fifo->wait.lock, flags);
+
+	/* FIFO Tx, reset the FIFO on error */
+	rv = sbefifo_up_write(fifo, buf, len);
+	if (rv < len) {
+		dev_err(fifo->dev, "FIFO write failed: %d\n", rv);
+		rv = sbefifo_reset(fifo);
+		if (rv < 0)
+			return rv;
+
+		spin_lock_irqsave(&fifo->wait.lock, flags);
+		fifo->state = sbefifo_ready;
+		client->state = sbefifo_client_idle;
+		wake_up(&client->wait);
+		wake_up_locked(&fifo->wait);
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+
+		return -EIO;
+	}
+
+	WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
+
+	/* Write completed successfully */
+	spin_lock_irqsave(&fifo->wait.lock, flags);
+	fifo->state = sbefifo_interval;
+	wake_up(&client->wait);
+	spin_unlock_irqrestore(&fifo->wait.lock, flags);
+
+	return rv;
+}
+EXPORT_SYMBOL_GPL(sbefifo_write);
+
+/**
+ * sbefifo_read()
+ *
+ * @client	The client context for the SBEFIFO
+ * @data	The buffer of data to write, at least @len elements
+ * @len		The number elements in @buffer
+ *
+ * Returns the number of elements read on success and negative values on
+ * failure. A return value of 0 indicates EOT.
+ */
+ssize_t sbefifo_read(struct sbefifo_client *client, u32 *buf, ssize_t len)
+{
+	struct sbefifo *fifo = client->fifo;
+	unsigned long flags;
+	ssize_t rv;
+
+	rv = wait_event_interruptible(client->wait,
+				      (client->state == sbefifo_client_active ||
+				       client->state == sbefifo_client_closed));
+	if (rv < 0)
+		return rv;
+
+	spin_lock_irqsave(&fifo->wait.lock, flags);
+	if (client->state == sbefifo_client_closed) {
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return -EBADFD;
+	}
+
+	if (client->state == sbefifo_client_idle) {
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return -EIO;
+	}
+
+	rv = wait_event_interruptible_locked_irq(fifo->wait,
+					fifo->state == sbefifo_interval ||
+					fifo->state == sbefifo_rx ||
+					fifo->state == sbefifo_ready ||
+					fifo->state == sbefifo_dead);
+	if (rv < 0) {
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return rv;
+	}
+
+	if (fifo->state == sbefifo_ready) {
+		/* We've reset FIFO, whatever we were waiting for has gone */
+		client->state = sbefifo_client_idle;
+		/* We're done, wake another task up as the FIFO is ready */
+		wake_up_locked(&fifo->wait);
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return -EIO;
+	}
+
+	if (fifo->state == sbefifo_dead) {
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return -ENODEV;
+	}
+
+	fifo->state = sbefifo_rx;
+	spin_unlock_irqrestore(&fifo->wait.lock, flags);
+
+	rv = sbefifo_down_read(fifo, buf, len);
+	if (rv > 0)
+		return rv;
+
+	/* Reset the FIFO on error */
+	if (rv < 0) {
+		dev_err(fifo->dev, "FIFO read failed: %d\n", rv);
+		rv = sbefifo_reset(fifo);
+		if (rv < 0)
+			return rv;
+
+		rv = -EIO;
+	}
+
+	/* Read is complete one way or the other (0 length read or error) */
+	spin_lock_irqsave(&fifo->wait.lock, flags);
+	client->state = sbefifo_client_idle;
+
+	/* Queue next FIFO transfer */
+	fifo->curr = NULL;
+	fifo->state = sbefifo_ready;
+	wake_up_locked(&fifo->wait);
+
+	spin_unlock_irqrestore(&fifo->wait.lock, flags);
+
+	return rv;
+}
+EXPORT_SYMBOL_GPL(sbefifo_read);
+
+/**
+ * sbefifo_release()
+ *
+ * @client	The client context for the SBEFIFO
+ *
+ */
+int sbefifo_release(struct sbefifo_client *client)
+{
+	struct sbefifo *fifo = client->fifo;
+	enum sbefifo_client_state old;
+	unsigned long flags;
+	int rv;
+
+	/* Determine if we need to clean up */
+	spin_lock_irqsave(&client->fifo->wait.lock, flags);
+	old = client->state;
+	client->state = sbefifo_client_closed;
+
+	if (old == sbefifo_client_closed) {
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return -EBADFD;
+	}
+
+	if (old == sbefifo_client_idle) {
+		spin_unlock_irqrestore(&fifo->wait.lock, flags);
+		return 0;
+	}
+
+	/* We need to clean up, get noisy about inconsistencies */
+	dev_warn(fifo->dev, "Releasing client with transfer in progress!\n");
+	WARN_ON(old != sbefifo_client_active);
+	WARN_ON(fifo->state == sbefifo_ready);
+
+	/* Mark ourselves as broken for cleanup */
+	fifo->state = sbefifo_broken;
+	fifo->curr = NULL;
+
+	wake_up(&client->wait);
+	spin_unlock_irqrestore(&client->fifo->wait.lock, flags);
+
+	/* Clean up poll waiter */
+	spin_lock_irqsave(&fifo->poll.wait.lock, flags);
+	del_timer_sync(&fifo->poll.timer);
+	fifo->poll.rv = -EBADFD;
+	wake_up_all_locked(&fifo->poll.wait);
+	spin_unlock_irqrestore(&fifo->poll.wait.lock, flags);
+
+	/* Reset the FIFO */
+	rv = sbefifo_reset(fifo);
+	if (rv < 0)
+		return rv;
+
+	/* Mark the FIFO as ready and wake pending transfer */
+	spin_lock_irqsave(&client->fifo->wait.lock, flags);
+	fifo->state = sbefifo_ready;
+	wake_up_locked(&fifo->wait);
+	spin_unlock_irqrestore(&client->fifo->wait.lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(sbefifo_release);
+
+static int sbefifo_unregister_child(struct device *dev, void *data)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+
+	of_device_unregister(pdev);
+	if (dev->of_node)
+		of_node_clear_flag(dev->of_node, OF_POPULATED);
+
+	return 0;
+}
+
+static int sbefifo_probe(struct device *dev)
+{
+	struct device_node *np;
+	struct sbefifo *fifo;
+	int child_idx;
+	u32 up, down;
+	int rv;
+
+	fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
+	if (!fifo)
+		return -ENOMEM;
+
+	fifo->dev = dev;
+	fifo->state = sbefifo_ready;
+	fifo->fsi = to_fsi_dev(dev);
+
+	fifo->id = ida_simple_get(&sbefifo_ida, 0, 0, GFP_KERNEL);
+	if (fifo->id < 0)
+		return fifo->id;
+
+	init_waitqueue_head(&fifo->wait);
+
+	/* No interrupts, need to poll the controller */
+	setup_timer(&fifo->poll.timer, sbefifo_poll_device,
+		    (unsigned long)fifo);
+	init_waitqueue_head(&fifo->poll.wait);
+
+	rv = sbefifo_up_sts(fifo, &up);
+	if (rv < 0)
+		return rv;
+
+	rv = sbefifo_down_sts(fifo, &down);
+	if (rv < 0)
+		return rv;
+
+	if (!(sbefifo_empty(up) && sbefifo_empty(down)))  {
+		dev_warn(fifo->dev, "FIFOs were not empty, requesting reset from SBE\n");
+		/* Request the SBE reset the FIFOs */
+		rv = sbefifo_reset(fifo);
+		if (rv == -ETIMEDOUT) {
+			dev_warn(fifo->dev, "SBE unresponsive, probing FIFO clients may fail. Performing hard FIFO reset\n");
+			rv = sbefifo_do_reset(fifo);
+			if (rv < 0)
+				return rv;
+		} else if (rv < 0) {
+			return rv;
+		}
+	}
+
+	dev_set_drvdata(dev, fifo);
+	list_add(&fifo->entry, &sbefifos);
+
+	child_idx = 0;
+	for_each_available_child_of_node(dev->of_node, np) {
+		struct platform_device *child;
+		char name[32];
+
+		snprintf(name, sizeof(name), "sbefifo%d-dev%d", fifo->id,
+			 child_idx++);
+		child = of_platform_device_create(np, name, dev);
+		if (!child)
+			dev_warn(dev, "Failed to create platform device %s\n",
+				 name);
+	}
+
+	return 0;
+}
+
+static int sbefifo_remove(struct device *dev)
+{
+	struct sbefifo *fifo = dev_get_drvdata(dev);
+	unsigned long flags;
+
+	/*
+	 * Don't wait to reach sbefifo_ready, we may deadlock through power
+	 * being removed to the host without the FIFO driver being unbound,
+	 * which can stall the in-progress transfers. We don't really care as
+	 * the driver is now going away, and the reset in the probe() path
+	 * should recover it.
+	 */
+
+	device_for_each_child(dev, NULL, sbefifo_unregister_child);
+
+	list_del(&fifo->entry);
+
+	/* Kick out the waiting clients */
+	spin_lock_irqsave(&fifo->wait.lock, flags);
+	fifo->state = sbefifo_dead;
+
+	if (fifo->curr) {
+		fifo->curr->state = sbefifo_client_closed;
+		wake_up_all(&fifo->curr->wait);
+	}
+
+	wake_up_all_locked(&fifo->wait);
+	spin_unlock_irqrestore(&fifo->wait.lock, flags);
+
+	/* Kick out any in-progress job */
+	spin_lock_irqsave(&fifo->poll.wait.lock, flags);
+	del_timer_sync(&fifo->poll.timer);
+	fifo->poll.rv = -ENODEV;
+	wake_up_all_locked(&fifo->poll.wait);
+	spin_unlock_irqrestore(&fifo->poll.wait.lock, flags);
+
+	while (wq_has_sleeper(&fifo->wait) || wq_has_sleeper(&fifo->poll.wait))
+		schedule();
+
+	ida_simple_remove(&sbefifo_ida, fifo->id);
+
+	return 0;
+}
+
+static const struct fsi_device_id sbefifo_ids[] = {
+	{ .engine_type = FSI_ENGINE_ID_SBE, .version = FSI_VERSION_ANY, },
+	{ 0 },
+};
+
+static struct fsi_driver sbefifo_drv = {
+	.id_table = sbefifo_ids,
+	.drv = {
+		.name = "sbefifo",
+		.bus = &fsi_bus_type,
+		.probe = sbefifo_probe,
+		.remove = sbefifo_remove,
+	},
+};
+
+static __init int sbefifo_init(void)
+{
+	ida_init(&sbefifo_ida);
+	fsi_driver_register(&sbefifo_drv);
+
+	return 0;
+}
+
+static __exit void sbefifo_exit(void)
+{
+	fsi_driver_unregister(&sbefifo_drv);
+	ida_destroy(&sbefifo_ida);
+}
+
+module_init(sbefifo_init);
+module_exit(sbefifo_exit);
+
+MODULE_AUTHOR("Andrew Jeffery <andrew@aj.id.au>");
+MODULE_DESCRIPTION("POWER9 Self Boot Engine FIFO driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/fsi/clients/fsi-sbefifo.h b/drivers/fsi/clients/fsi-sbefifo.h
new file mode 100644
index 000000000000..323fabfa7fce
--- /dev/null
+++ b/drivers/fsi/clients/fsi-sbefifo.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef DRIVERS_FSI_SBEFIFO_H
+
+#include <linux/timer.h>
+#include <linux/types.h>
+#include <linux/wait.h>
+
+enum sbefifo_direction {
+	sbefifo_up = 0,
+	sbefifo_down,
+};
+
+enum sbefifo_poll_state {
+	sbefifo_poll_wait,
+	sbefifo_poll_ready,
+	sbefifo_poll_reset,
+};
+
+/* Readiness Polling */
+struct sbefifo_poll {
+	struct timer_list timer;
+	wait_queue_head_t wait;
+	enum sbefifo_direction dir;
+	unsigned long interval;
+	bool expire;
+	unsigned long expire_at;
+	enum sbefifo_poll_state state;
+	int rv;
+};
+
+struct sbefifo_client;
+
+enum sbefifo_state {
+	sbefifo_ready = 0,
+	sbefifo_tx,
+	sbefifo_interval,
+	sbefifo_rx,
+	sbefifo_broken,
+	sbefifo_dead,
+};
+
+/**
+ * @eot True when read() dequeues and ACKs an EOT. Set false in the write() path
+ */
+struct sbefifo {
+	struct device *dev;
+	struct fsi_device *fsi;
+	int id;
+	enum sbefifo_state state;
+	struct sbefifo_poll poll;
+	struct sbefifo_client *curr;
+	wait_queue_head_t wait;
+
+	struct list_head entry;
+};
+
+enum sbefifo_client_state {
+	sbefifo_client_closed = 0,
+	sbefifo_client_idle,
+	sbefifo_client_active,
+};
+
+struct sbefifo_client {
+	struct sbefifo *fifo;
+
+	wait_queue_head_t wait;
+	enum sbefifo_client_state state;
+	unsigned int flags;
+};
+
+int sbefifo_open(struct sbefifo *fifo, struct sbefifo_client *client,
+		 unsigned long flags);
+ssize_t sbefifo_write(struct sbefifo_client *client, const u32 *buf, ssize_t len);
+ssize_t sbefifo_read(struct sbefifo_client *client, u32 *buf, ssize_t len);
+int sbefifo_release(struct sbefifo_client *client);
+
+extern struct list_head sbefifos;
+
+#define sbefifo_for_each_dev(pos) \
+	list_for_each_entry(pos, &sbefifos, entry)
+
+#endif /* DRIVERS_FSI_SBEFIFO_H */
-- 
2.14.1

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

* Re: [PATCH] fsi: Add Self Boot Engine FIFO FSI client
  2017-12-01 13:07   ` [PATCH] fsi: Add Self Boot Engine FIFO FSI client Andrew Jeffery
@ 2017-12-03 20:25     ` kbuild test robot
  2017-12-04  0:05       ` Andrew Jeffery
  2017-12-04  4:50     ` kbuild test robot
  1 sibling, 1 reply; 10+ messages in thread
From: kbuild test robot @ 2017-12-03 20:25 UTC (permalink / raw)
  To: Andrew Jeffery
  Cc: kbuild-all, linux-kernel, Andrew Jeffery, gregkh, devicetree,
	robh+dt, mark.rutland, bradleyb, cbostic, joel, eajames,
	Edward A. James

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

Hi Andrew,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.15-rc2 next-20171201]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Andrew-Jeffery/fsi-Add-Self-Boot-Engine-FIFO-FSI-client/20171204-031454
config: blackfin-allmodconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=blackfin 

All error/warnings (new ones prefixed by >>):

>> drivers/fsi//clients/fsi-sbefifo.c:325:0: warning: "TEST_SET" redefined
    #define TEST_SET(s) ((s) & BIT(7))
    
   In file included from arch/blackfin/mach-bf533/include/mach/blackfin.h:15:0,
                    from arch/blackfin/include/asm/irqflags.h:11,
                    from include/linux/irqflags.h:16,
                    from arch/blackfin/include/asm/bitops.h:33,
                    from include/linux/bitops.h:38,
                    from drivers/fsi//clients/fsi-sbefifo.c:5:
   arch/blackfin/include/asm/def_LPBlackfin.h:687:0: note: this is the location of the previous definition
    #define TEST_SET(x)    ((x << 5) & 0x03E0) /* Set Index 0->31 */
    
   In file included from include/linux/printk.h:329:0,
                    from include/linux/kernel.h:14,
                    from include/linux/list.h:9,
                    from include/linux/kobject.h:20,
                    from include/linux/device.h:17,
                    from include/linux/fsi.h:18,
                    from drivers/fsi//clients/fsi-sbefifo.c:6:
   drivers/fsi//clients/fsi-sbefifo.c: In function 'sbefifo_drain':
>> drivers/fsi//clients/fsi-sbefifo.c:359:21: warning: format '%d' expects argument of type 'int', but argument 7 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
>> drivers/fsi//clients/fsi-sbefifo.c:359:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   drivers/fsi//clients/fsi-sbefifo.c:359:21: warning: format '%d' expects argument of type 'int', but argument 9 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
>> drivers/fsi//clients/fsi-sbefifo.c:359:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   drivers/fsi//clients/fsi-sbefifo.c:378:21: warning: format '%d' expects argument of type 'int', but argument 7 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: Data phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fsi//clients/fsi-sbefifo.c:378:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: Data phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   drivers/fsi//clients/fsi-sbefifo.c:378:21: warning: format '%d' expects argument of type 'int', but argument 9 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: Data phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fsi//clients/fsi-sbefifo.c:378:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: Data phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   drivers/fsi//clients/fsi-sbefifo.c:403:21: warning: format '%d' expects argument of type 'int', but argument 7 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fsi//clients/fsi-sbefifo.c:403:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   drivers/fsi//clients/fsi-sbefifo.c:403:21: warning: format '%d' expects argument of type 'int', but argument 9 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fsi//clients/fsi-sbefifo.c:403:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   drivers/fsi//clients/fsi-sbefifo.c:417:21: warning: format '%d' expects argument of type 'int', but argument 7 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fsi//clients/fsi-sbefifo.c:417:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   drivers/fsi//clients/fsi-sbefifo.c:417:21: warning: format '%d' expects argument of type 'int', but argument 9 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fsi//clients/fsi-sbefifo.c:417:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   In file included from include/linux/list.h:9:0,
                    from include/linux/kobject.h:20,
                    from include/linux/device.h:17,
                    from include/linux/fsi.h:18,
                    from drivers/fsi//clients/fsi-sbefifo.c:6:
   drivers/fsi//clients/fsi-sbefifo.c: In function 'sbefifo_down_read':
   include/linux/kernel.h:792:16: warning: comparison of distinct pointer types lacks a cast
     (void) (&min1 == &min2);   \
                   ^
   include/linux/kernel.h:801:2: note: in expansion of macro '__min'
     __min(typeof(x), typeof(y),   \
     ^~~~~
>> drivers/fsi//clients/fsi-sbefifo.c:453:34: note: in expansion of macro 'min'
     } while (rem && read && read == min((rem + read), SBEFIFO_FIFO_DEPTH));
                                     ^~~
   drivers/fsi//clients/fsi-sbefifo.c: In function 'sbefifo_write':
   drivers/fsi//clients/fsi-sbefifo.c:557:43: warning: format '%d' expects argument of type 'int', but argument 3 has type 'ssize_t {aka long int}' [-Wformat=]
      dev_err(fifo->dev, "FIFO write failed: %d\n", rv);
                                              ^
   In file included from arch/blackfin/include/asm/bug.h:71:0,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:12,
                    from include/asm-generic/current.h:5,
                    from ./arch/blackfin/include/generated/asm/current.h:1,
                    from include/linux/mutex.h:14,
                    from include/linux/kernfs.h:13,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:21,
                    from include/linux/device.h:17,
                    from include/linux/fsi.h:18,
                    from drivers/fsi//clients/fsi-sbefifo.c:6:
   drivers/fsi//clients/fsi-sbefifo.c:572:17: warning: format '%d' expects argument of type 'int', but argument 4 has type 'ssize_t {aka long int}' [-Wformat=]
     WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
                    ^
   include/asm-generic/bug.h:91:69: note: in definition of macro '__WARN_printf'
    #define __WARN_printf(arg...) warn_slowpath_fmt(__FILE__, __LINE__, arg)
                                                                        ^~~
>> drivers/fsi//clients/fsi-sbefifo.c:572:2: note: in expansion of macro 'WARN'
     WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
     ^~~~
   drivers/fsi//clients/fsi-sbefifo.c:572:17: warning: format '%d' expects argument of type 'int', but argument 5 has type 'ssize_t {aka long int}' [-Wformat=]
     WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
                    ^
   include/asm-generic/bug.h:91:69: note: in definition of macro '__WARN_printf'
    #define __WARN_printf(arg...) warn_slowpath_fmt(__FILE__, __LINE__, arg)
                                                                        ^~~
>> drivers/fsi//clients/fsi-sbefifo.c:572:2: note: in expansion of macro 'WARN'
     WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
     ^~~~
   drivers/fsi//clients/fsi-sbefifo.c: In function 'sbefifo_read':
   drivers/fsi//clients/fsi-sbefifo.c:650:42: warning: format '%d' expects argument of type 'int', but argument 3 has type 'ssize_t {aka long int}' [-Wformat=]
      dev_err(fifo->dev, "FIFO read failed: %d\n", rv);
                                             ^
   drivers/fsi//clients/fsi-sbefifo.c: In function 'sbefifo_probe':
>> drivers/fsi//clients/fsi-sbefifo.c:769:2: error: implicit declaration of function 'setup_timer' [-Werror=implicit-function-declaration]
     setup_timer(&fifo->poll.timer, sbefifo_poll_device,
     ^~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/setup_timer +769 drivers/fsi//clients/fsi-sbefifo.c

   498	
   499	/**
   500	 * sbefifo_write()
   501	 *
   502	 * @client	The client context for the SBEFIFO
   503	 * @buf		The buffer of data to write, at least @len elements
   504	 * @len		The number elements in @buffer
   505	 *
   506	 * The buffer must represent a complete chip-op: EOT is signalled after the
   507	 * last element is written to the upstream FIFO.
   508	 *
   509	 * Returns the number of elements written on success and negative values on
   510	 * failure. If the call is successful a subsequent call to sbefifo_read() MUST
   511	 * be made.
   512	 */
   513	ssize_t sbefifo_write(struct sbefifo_client *client, const u32 *buf,
   514			      ssize_t len)
   515	{
   516		struct sbefifo *fifo = client->fifo;
   517		unsigned long flags;
   518		ssize_t rv;
   519	
   520		spin_lock_irqsave(&fifo->wait.lock, flags);
   521	
   522		if (client->state == sbefifo_client_active) {
   523			dev_warn(fifo->dev, "Transfer already in progress\n");
   524			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   525			return -EBUSY;
   526		}
   527	
   528		rv = wait_event_interruptible_locked_irq(fifo->wait,
   529							fifo->state == sbefifo_ready ||
   530							fifo->state == sbefifo_dead);
   531		if (rv < 0) {
   532			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   533			return rv;
   534		}
   535	
   536		if (fifo->state == sbefifo_dead) {
   537			client->state = sbefifo_client_closed;
   538			wake_up(&client->wait);
   539			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   540			return -ENODEV;
   541		}
   542	
   543		WARN_ON(fifo->state != sbefifo_ready);
   544	
   545		fifo->curr = client;
   546		fifo->state = sbefifo_tx;
   547	
   548		/* Move a threaded read() onto waiting for FIFO read readiness */
   549		client->state = sbefifo_client_active;
   550		wake_up(&client->wait);
   551	
   552		spin_unlock_irqrestore(&fifo->wait.lock, flags);
   553	
   554		/* FIFO Tx, reset the FIFO on error */
   555		rv = sbefifo_up_write(fifo, buf, len);
   556		if (rv < len) {
   557			dev_err(fifo->dev, "FIFO write failed: %d\n", rv);
   558			rv = sbefifo_reset(fifo);
   559			if (rv < 0)
   560				return rv;
   561	
   562			spin_lock_irqsave(&fifo->wait.lock, flags);
   563			fifo->state = sbefifo_ready;
   564			client->state = sbefifo_client_idle;
   565			wake_up(&client->wait);
   566			wake_up_locked(&fifo->wait);
   567			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   568	
   569			return -EIO;
   570		}
   571	
 > 572		WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
   573	
   574		/* Write completed successfully */
   575		spin_lock_irqsave(&fifo->wait.lock, flags);
   576		fifo->state = sbefifo_interval;
   577		wake_up(&client->wait);
   578		spin_unlock_irqrestore(&fifo->wait.lock, flags);
   579	
   580		return rv;
   581	}
   582	EXPORT_SYMBOL_GPL(sbefifo_write);
   583	
   584	/**
   585	 * sbefifo_read()
   586	 *
   587	 * @client	The client context for the SBEFIFO
   588	 * @data	The buffer of data to write, at least @len elements
   589	 * @len		The number elements in @buffer
   590	 *
   591	 * Returns the number of elements read on success and negative values on
   592	 * failure. A return value of 0 indicates EOT.
   593	 */
   594	ssize_t sbefifo_read(struct sbefifo_client *client, u32 *buf, ssize_t len)
   595	{
   596		struct sbefifo *fifo = client->fifo;
   597		unsigned long flags;
   598		ssize_t rv;
   599	
   600		rv = wait_event_interruptible(client->wait,
   601					      (client->state == sbefifo_client_active ||
   602					       client->state == sbefifo_client_closed));
   603		if (rv < 0)
   604			return rv;
   605	
   606		spin_lock_irqsave(&fifo->wait.lock, flags);
   607		if (client->state == sbefifo_client_closed) {
   608			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   609			return -EBADFD;
   610		}
   611	
   612		if (client->state == sbefifo_client_idle) {
   613			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   614			return -EIO;
   615		}
   616	
   617		rv = wait_event_interruptible_locked_irq(fifo->wait,
   618						fifo->state == sbefifo_interval ||
   619						fifo->state == sbefifo_rx ||
   620						fifo->state == sbefifo_ready ||
   621						fifo->state == sbefifo_dead);
   622		if (rv < 0) {
   623			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   624			return rv;
   625		}
   626	
   627		if (fifo->state == sbefifo_ready) {
   628			/* We've reset FIFO, whatever we were waiting for has gone */
   629			client->state = sbefifo_client_idle;
   630			/* We're done, wake another task up as the FIFO is ready */
   631			wake_up_locked(&fifo->wait);
   632			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   633			return -EIO;
   634		}
   635	
   636		if (fifo->state == sbefifo_dead) {
   637			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   638			return -ENODEV;
   639		}
   640	
   641		fifo->state = sbefifo_rx;
   642		spin_unlock_irqrestore(&fifo->wait.lock, flags);
   643	
   644		rv = sbefifo_down_read(fifo, buf, len);
   645		if (rv > 0)
   646			return rv;
   647	
   648		/* Reset the FIFO on error */
   649		if (rv < 0) {
   650			dev_err(fifo->dev, "FIFO read failed: %d\n", rv);
   651			rv = sbefifo_reset(fifo);
   652			if (rv < 0)
   653				return rv;
   654	
   655			rv = -EIO;
   656		}
   657	
   658		/* Read is complete one way or the other (0 length read or error) */
   659		spin_lock_irqsave(&fifo->wait.lock, flags);
   660		client->state = sbefifo_client_idle;
   661	
   662		/* Queue next FIFO transfer */
   663		fifo->curr = NULL;
   664		fifo->state = sbefifo_ready;
   665		wake_up_locked(&fifo->wait);
   666	
   667		spin_unlock_irqrestore(&fifo->wait.lock, flags);
   668	
   669		return rv;
   670	}
   671	EXPORT_SYMBOL_GPL(sbefifo_read);
   672	
   673	/**
   674	 * sbefifo_release()
   675	 *
   676	 * @client	The client context for the SBEFIFO
   677	 *
   678	 */
   679	int sbefifo_release(struct sbefifo_client *client)
   680	{
   681		struct sbefifo *fifo = client->fifo;
   682		enum sbefifo_client_state old;
   683		unsigned long flags;
   684		int rv;
   685	
   686		/* Determine if we need to clean up */
   687		spin_lock_irqsave(&client->fifo->wait.lock, flags);
   688		old = client->state;
   689		client->state = sbefifo_client_closed;
   690	
   691		if (old == sbefifo_client_closed) {
   692			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   693			return -EBADFD;
   694		}
   695	
   696		if (old == sbefifo_client_idle) {
   697			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   698			return 0;
   699		}
   700	
   701		/* We need to clean up, get noisy about inconsistencies */
   702		dev_warn(fifo->dev, "Releasing client with transfer in progress!\n");
   703		WARN_ON(old != sbefifo_client_active);
   704		WARN_ON(fifo->state == sbefifo_ready);
   705	
   706		/* Mark ourselves as broken for cleanup */
   707		fifo->state = sbefifo_broken;
   708		fifo->curr = NULL;
   709	
   710		wake_up(&client->wait);
   711		spin_unlock_irqrestore(&client->fifo->wait.lock, flags);
   712	
   713		/* Clean up poll waiter */
   714		spin_lock_irqsave(&fifo->poll.wait.lock, flags);
   715		del_timer_sync(&fifo->poll.timer);
   716		fifo->poll.rv = -EBADFD;
   717		wake_up_all_locked(&fifo->poll.wait);
   718		spin_unlock_irqrestore(&fifo->poll.wait.lock, flags);
   719	
   720		/* Reset the FIFO */
   721		rv = sbefifo_reset(fifo);
   722		if (rv < 0)
   723			return rv;
   724	
   725		/* Mark the FIFO as ready and wake pending transfer */
   726		spin_lock_irqsave(&client->fifo->wait.lock, flags);
   727		fifo->state = sbefifo_ready;
   728		wake_up_locked(&fifo->wait);
   729		spin_unlock_irqrestore(&client->fifo->wait.lock, flags);
   730	
   731		return 0;
   732	}
   733	EXPORT_SYMBOL_GPL(sbefifo_release);
   734	
   735	static int sbefifo_unregister_child(struct device *dev, void *data)
   736	{
   737		struct platform_device *pdev = to_platform_device(dev);
   738	
   739		of_device_unregister(pdev);
   740		if (dev->of_node)
   741			of_node_clear_flag(dev->of_node, OF_POPULATED);
   742	
   743		return 0;
   744	}
   745	
   746	static int sbefifo_probe(struct device *dev)
   747	{
   748		struct device_node *np;
   749		struct sbefifo *fifo;
   750		int child_idx;
   751		u32 up, down;
   752		int rv;
   753	
   754		fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
   755		if (!fifo)
   756			return -ENOMEM;
   757	
   758		fifo->dev = dev;
   759		fifo->state = sbefifo_ready;
   760		fifo->fsi = to_fsi_dev(dev);
   761	
   762		fifo->id = ida_simple_get(&sbefifo_ida, 0, 0, GFP_KERNEL);
   763		if (fifo->id < 0)
   764			return fifo->id;
   765	
   766		init_waitqueue_head(&fifo->wait);
   767	
   768		/* No interrupts, need to poll the controller */
 > 769		setup_timer(&fifo->poll.timer, sbefifo_poll_device,
   770			    (unsigned long)fifo);
   771		init_waitqueue_head(&fifo->poll.wait);
   772	
   773		rv = sbefifo_up_sts(fifo, &up);
   774		if (rv < 0)
   775			return rv;
   776	
   777		rv = sbefifo_down_sts(fifo, &down);
   778		if (rv < 0)
   779			return rv;
   780	
   781		if (!(sbefifo_empty(up) && sbefifo_empty(down)))  {
   782			dev_warn(fifo->dev, "FIFOs were not empty, requesting reset from SBE\n");
   783			/* Request the SBE reset the FIFOs */
   784			rv = sbefifo_reset(fifo);
   785			if (rv == -ETIMEDOUT) {
   786				dev_warn(fifo->dev, "SBE unresponsive, probing FIFO clients may fail. Performing hard FIFO reset\n");
   787				rv = sbefifo_do_reset(fifo);
   788				if (rv < 0)
   789					return rv;
   790			} else if (rv < 0) {
   791				return rv;
   792			}
   793		}
   794	
   795		dev_set_drvdata(dev, fifo);
   796		list_add(&fifo->entry, &sbefifos);
   797	
   798		child_idx = 0;
   799		for_each_available_child_of_node(dev->of_node, np) {
   800			struct platform_device *child;
   801			char name[32];
   802	
   803			snprintf(name, sizeof(name), "sbefifo%d-dev%d", fifo->id,
   804				 child_idx++);
   805			child = of_platform_device_create(np, name, dev);
   806			if (!child)
   807				dev_warn(dev, "Failed to create platform device %s\n",
   808					 name);
   809		}
   810	
   811		return 0;
   812	}
   813	

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

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

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

* Re: [PATCH] fsi: Add Self Boot Engine FIFO FSI client
  2017-12-03 20:25     ` kbuild test robot
@ 2017-12-04  0:05       ` Andrew Jeffery
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Jeffery @ 2017-12-04  0:05 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, linux-kernel, gregkh, devicetree, robh+dt,
	mark.rutland, bradleyb, cbostic, joel, eajames, Edward A. James

If there's enthusiasm for what I've proposed over Eddy and Brad's patch,
I'll clean up the kbuild issues and resend. It was more meant to trigger
discussion than sent as ready for submission.

Cheers,

Andrew

On Mon, 4 Dec 2017, at 06:55, kbuild test robot wrote:
> Hi Andrew,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.15-rc2 next-20171201]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
> 
> url:   
> https://github.com/0day-ci/linux/commits/Andrew-Jeffery/fsi-Add-Self-Boot-Engine-FIFO-FSI-client/20171204-031454
> config: blackfin-allmodconfig (attached as .config)
> compiler: bfin-uclinux-gcc (GCC) 6.2.0
> reproduce:
>         wget
>         https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
>         -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=blackfin 
> 
> All error/warnings (new ones prefixed by >>):
> 
> >> drivers/fsi//clients/fsi-sbefifo.c:325:0: warning: "TEST_SET" redefined
>     #define TEST_SET(s) ((s) & BIT(7))
>     
>    In file included from
>    arch/blackfin/mach-bf533/include/mach/blackfin.h:15:0,
>                     from arch/blackfin/include/asm/irqflags.h:11,
>                     from include/linux/irqflags.h:16,
>                     from arch/blackfin/include/asm/bitops.h:33,
>                     from include/linux/bitops.h:38,
>                     from drivers/fsi//clients/fsi-sbefifo.c:5:
>    arch/blackfin/include/asm/def_LPBlackfin.h:687:0: note: this is the
>    location of the previous definition
>     #define TEST_SET(x)    ((x << 5) & 0x03E0) /* Set Index 0->31 */
>     
>    In file included from include/linux/printk.h:329:0,
>                     from include/linux/kernel.h:14,
>                     from include/linux/list.h:9,
>                     from include/linux/kobject.h:20,
>                     from include/linux/device.h:17,
>                     from include/linux/fsi.h:18,
>                     from drivers/fsi//clients/fsi-sbefifo.c:6:
>    drivers/fsi//clients/fsi-sbefifo.c: In function 'sbefifo_drain':
> >> drivers/fsi//clients/fsi-sbefifo.c:359:21: warning: format '%d' expects argument of type 'int', but argument 7 has type 'ssize_t {aka long int}' [-Wformat=]
>      dev_dbg(fifo->dev, "%s: valid_set: 0x%x, eot_set: 0x%x, nr_valid:
>      %d, nr_xfer: %d, rem: %d\n",
>                         ^
>    include/linux/dynamic_debug.h:135:39: note: in definition of macro
>    'dynamic_dev_dbg'
>       __dynamic_dev_dbg(&descriptor, dev, fmt, \
>                                           ^~~
> >> drivers/fsi//clients/fsi-sbefifo.c:359:2: note: in expansion of macro 'dev_dbg'
>      dev_dbg(fifo->dev, "%s: valid_set: 0x%x, eot_set: 0x%x, nr_valid:
>      %d, nr_xfer: %d, rem: %d\n",
>      ^~~~~~~
>    drivers/fsi//clients/fsi-sbefifo.c:359:21: warning: format '%d'
>    expects argument of type 'int', but argument 9 has type 'ssize_t {aka
>    long int}' [-Wformat=]
>      dev_dbg(fifo->dev, "%s: valid_set: 0x%x, eot_set: 0x%x, nr_valid:
>      %d, nr_xfer: %d, rem: %d\n",
>                         ^
>    include/linux/dynamic_debug.h:135:39: note: in definition of macro
>    'dynamic_dev_dbg'
>       __dynamic_dev_dbg(&descriptor, dev, fmt, \
>                                           ^~~
> >> drivers/fsi//clients/fsi-sbefifo.c:359:2: note: in expansion of macro 'dev_dbg'
>      dev_dbg(fifo->dev, "%s: valid_set: 0x%x, eot_set: 0x%x, nr_valid:
>      %d, nr_xfer: %d, rem: %d\n",
>      ^~~~~~~
>    drivers/fsi//clients/fsi-sbefifo.c:378:21: warning: format '%d'
>    expects argument of type 'int', but argument 7 has type 'ssize_t {aka
>    long int}' [-Wformat=]
>      dev_dbg(fifo->dev, "%s: Data phase complete: valid_set: 0x%x,
>      eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
>                         ^
>    include/linux/dynamic_debug.h:135:39: note: in definition of macro
>    'dynamic_dev_dbg'
>       __dynamic_dev_dbg(&descriptor, dev, fmt, \
>                                           ^~~
>    drivers/fsi//clients/fsi-sbefifo.c:378:2: note: in expansion of macro
>    'dev_dbg'
>      dev_dbg(fifo->dev, "%s: Data phase complete: valid_set: 0x%x,
>      eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
>      ^~~~~~~
>    drivers/fsi//clients/fsi-sbefifo.c:378:21: warning: format '%d'
>    expects argument of type 'int', but argument 9 has type 'ssize_t {aka
>    long int}' [-Wformat=]
>      dev_dbg(fifo->dev, "%s: Data phase complete: valid_set: 0x%x,
>      eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
>                         ^
>    include/linux/dynamic_debug.h:135:39: note: in definition of macro
>    'dynamic_dev_dbg'
>       __dynamic_dev_dbg(&descriptor, dev, fmt, \
>                                           ^~~
>    drivers/fsi//clients/fsi-sbefifo.c:378:2: note: in expansion of macro
>    'dev_dbg'
>      dev_dbg(fifo->dev, "%s: Data phase complete: valid_set: 0x%x,
>      eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
>      ^~~~~~~
>    drivers/fsi//clients/fsi-sbefifo.c:403:21: warning: format '%d'
>    expects argument of type 'int', but argument 7 has type 'ssize_t {aka
>    long int}' [-Wformat=]
>      dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x,
>      eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
>                         ^
>    include/linux/dynamic_debug.h:135:39: note: in definition of macro
>    'dynamic_dev_dbg'
>       __dynamic_dev_dbg(&descriptor, dev, fmt, \
>                                           ^~~
>    drivers/fsi//clients/fsi-sbefifo.c:403:2: note: in expansion of macro
>    'dev_dbg'
>      dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x,
>      eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
>      ^~~~~~~
>    drivers/fsi//clients/fsi-sbefifo.c:403:21: warning: format '%d'
>    expects argument of type 'int', but argument 9 has type 'ssize_t {aka
>    long int}' [-Wformat=]
>      dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x,
>      eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
>                         ^
>    include/linux/dynamic_debug.h:135:39: note: in definition of macro
>    'dynamic_dev_dbg'
>       __dynamic_dev_dbg(&descriptor, dev, fmt, \
>                                           ^~~
>    drivers/fsi//clients/fsi-sbefifo.c:403:2: note: in expansion of macro
>    'dev_dbg'
>      dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x,
>      eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
>      ^~~~~~~
>    drivers/fsi//clients/fsi-sbefifo.c:417:21: warning: format '%d'
>    expects argument of type 'int', but argument 7 has type 'ssize_t {aka
>    long int}' [-Wformat=]
>      dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x,
>      eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
>                         ^
>    include/linux/dynamic_debug.h:135:39: note: in definition of macro
>    'dynamic_dev_dbg'
>       __dynamic_dev_dbg(&descriptor, dev, fmt, \
>                                           ^~~
>    drivers/fsi//clients/fsi-sbefifo.c:417:2: note: in expansion of macro
>    'dev_dbg'
>      dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x,
>      eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
>      ^~~~~~~
>    drivers/fsi//clients/fsi-sbefifo.c:417:21: warning: format '%d'
>    expects argument of type 'int', but argument 9 has type 'ssize_t {aka
>    long int}' [-Wformat=]
>      dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x,
>      eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
>                         ^
>    include/linux/dynamic_debug.h:135:39: note: in definition of macro
>    'dynamic_dev_dbg'
>       __dynamic_dev_dbg(&descriptor, dev, fmt, \
>                                           ^~~
>    drivers/fsi//clients/fsi-sbefifo.c:417:2: note: in expansion of macro
>    'dev_dbg'
>      dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x,
>      eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
>      ^~~~~~~
>    In file included from include/linux/list.h:9:0,
>                     from include/linux/kobject.h:20,
>                     from include/linux/device.h:17,
>                     from include/linux/fsi.h:18,
>                     from drivers/fsi//clients/fsi-sbefifo.c:6:
>    drivers/fsi//clients/fsi-sbefifo.c: In function 'sbefifo_down_read':
>    include/linux/kernel.h:792:16: warning: comparison of distinct pointer
>    types lacks a cast
>      (void) (&min1 == &min2);   \
>                    ^
>    include/linux/kernel.h:801:2: note: in expansion of macro '__min'
>      __min(typeof(x), typeof(y),   \
>      ^~~~~
> >> drivers/fsi//clients/fsi-sbefifo.c:453:34: note: in expansion of macro 'min'
>      } while (rem && read && read == min((rem + read),
>      SBEFIFO_FIFO_DEPTH));
>                                      ^~~
>    drivers/fsi//clients/fsi-sbefifo.c: In function 'sbefifo_write':
>    drivers/fsi//clients/fsi-sbefifo.c:557:43: warning: format '%d'
>    expects argument of type 'int', but argument 3 has type 'ssize_t {aka
>    long int}' [-Wformat=]
>       dev_err(fifo->dev, "FIFO write failed: %d\n", rv);
>                                               ^
>    In file included from arch/blackfin/include/asm/bug.h:71:0,
>                     from include/linux/bug.h:5,
>                     from include/linux/thread_info.h:12,
>                     from include/asm-generic/current.h:5,
>                     from
>                     ./arch/blackfin/include/generated/asm/current.h:1,
>                     from include/linux/mutex.h:14,
>                     from include/linux/kernfs.h:13,
>                     from include/linux/sysfs.h:16,
>                     from include/linux/kobject.h:21,
>                     from include/linux/device.h:17,
>                     from include/linux/fsi.h:18,
>                     from drivers/fsi//clients/fsi-sbefifo.c:6:
>    drivers/fsi//clients/fsi-sbefifo.c:572:17: warning: format '%d'
>    expects argument of type 'int', but argument 4 has type 'ssize_t {aka
>    long int}' [-Wformat=]
>      WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
>                     ^
>    include/asm-generic/bug.h:91:69: note: in definition of macro
>    '__WARN_printf'
>     #define __WARN_printf(arg...) warn_slowpath_fmt(__FILE__, __LINE__,
>     arg)
>                                                                         ^~~
> >> drivers/fsi//clients/fsi-sbefifo.c:572:2: note: in expansion of macro 'WARN'
>      WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
>      ^~~~
>    drivers/fsi//clients/fsi-sbefifo.c:572:17: warning: format '%d'
>    expects argument of type 'int', but argument 5 has type 'ssize_t {aka
>    long int}' [-Wformat=]
>      WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
>                     ^
>    include/asm-generic/bug.h:91:69: note: in definition of macro
>    '__WARN_printf'
>     #define __WARN_printf(arg...) warn_slowpath_fmt(__FILE__, __LINE__,
>     arg)
>                                                                         ^~~
> >> drivers/fsi//clients/fsi-sbefifo.c:572:2: note: in expansion of macro 'WARN'
>      WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
>      ^~~~
>    drivers/fsi//clients/fsi-sbefifo.c: In function 'sbefifo_read':
>    drivers/fsi//clients/fsi-sbefifo.c:650:42: warning: format '%d'
>    expects argument of type 'int', but argument 3 has type 'ssize_t {aka
>    long int}' [-Wformat=]
>       dev_err(fifo->dev, "FIFO read failed: %d\n", rv);
>                                              ^
>    drivers/fsi//clients/fsi-sbefifo.c: In function 'sbefifo_probe':
> >> drivers/fsi//clients/fsi-sbefifo.c:769:2: error: implicit declaration of function 'setup_timer' [-Werror=implicit-function-declaration]
>      setup_timer(&fifo->poll.timer, sbefifo_poll_device,
>      ^~~~~~~~~~~
>    cc1: some warnings being treated as errors
> 
> vim +/setup_timer +769 drivers/fsi//clients/fsi-sbefifo.c
> 
>    498     
>    499     /**
>    500      * sbefifo_write()
>    501      *
>    502      * @client      The client context for the SBEFIFO
>    503      * @buf         The buffer of data to write, at least @len
>    elements
>    504      * @len         The number elements in @buffer
>    505      *
>    506      * The buffer must represent a complete chip-op: EOT is
>    signalled after the
>    507      * last element is written to the upstream FIFO.
>    508      *
>    509      * Returns the number of elements written on success and
>    negative values on
>    510      * failure. If the call is successful a subsequent call to
>    sbefifo_read() MUST
>    511      * be made.
>    512      */
>    513     ssize_t sbefifo_write(struct sbefifo_client *client, const u32
>    *buf,
>    514                           ssize_t len)
>    515     {
>    516             struct sbefifo *fifo = client->fifo;
>    517             unsigned long flags;
>    518             ssize_t rv;
>    519     
>    520             spin_lock_irqsave(&fifo->wait.lock, flags);
>    521     
>    522             if (client->state == sbefifo_client_active) {
>    523                     dev_warn(fifo->dev, "Transfer already in
>    progress\n");
>    524                     spin_unlock_irqrestore(&fifo->wait.lock,
>    flags);
>    525                     return -EBUSY;
>    526             }
>    527     
>    528             rv = wait_event_interruptible_locked_irq(fifo->wait,
>    529                                                     fifo->state ==
>    sbefifo_ready ||
>    530                                                     fifo->state ==
>    sbefifo_dead);
>    531             if (rv < 0) {
>    532                     spin_unlock_irqrestore(&fifo->wait.lock,
>    flags);
>    533                     return rv;
>    534             }
>    535     
>    536             if (fifo->state == sbefifo_dead) {
>    537                     client->state = sbefifo_client_closed;
>    538                     wake_up(&client->wait);
>    539                     spin_unlock_irqrestore(&fifo->wait.lock,
>    flags);
>    540                     return -ENODEV;
>    541             }
>    542     
>    543             WARN_ON(fifo->state != sbefifo_ready);
>    544     
>    545             fifo->curr = client;
>    546             fifo->state = sbefifo_tx;
>    547     
>    548             /* Move a threaded read() onto waiting for FIFO read
>    readiness */
>    549             client->state = sbefifo_client_active;
>    550             wake_up(&client->wait);
>    551     
>    552             spin_unlock_irqrestore(&fifo->wait.lock, flags);
>    553     
>    554             /* FIFO Tx, reset the FIFO on error */
>    555             rv = sbefifo_up_write(fifo, buf, len);
>    556             if (rv < len) {
>    557                     dev_err(fifo->dev, "FIFO write failed: %d\n",
>    rv);
>    558                     rv = sbefifo_reset(fifo);
>    559                     if (rv < 0)
>    560                             return rv;
>    561     
>    562                     spin_lock_irqsave(&fifo->wait.lock, flags);
>    563                     fifo->state = sbefifo_ready;
>    564                     client->state = sbefifo_client_idle;
>    565                     wake_up(&client->wait);
>    566                     wake_up_locked(&fifo->wait);
>    567                     spin_unlock_irqrestore(&fifo->wait.lock,
>    flags);
>    568     
>    569                     return -EIO;
>    570             }
>    571     
>  > 572		WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
>    573     
>    574             /* Write completed successfully */
>    575             spin_lock_irqsave(&fifo->wait.lock, flags);
>    576             fifo->state = sbefifo_interval;
>    577             wake_up(&client->wait);
>    578             spin_unlock_irqrestore(&fifo->wait.lock, flags);
>    579     
>    580             return rv;
>    581     }
>    582     EXPORT_SYMBOL_GPL(sbefifo_write);
>    583     
>    584     /**
>    585      * sbefifo_read()
>    586      *
>    587      * @client      The client context for the SBEFIFO
>    588      * @data        The buffer of data to write, at least @len
>    elements
>    589      * @len         The number elements in @buffer
>    590      *
>    591      * Returns the number of elements read on success and negative
>    values on
>    592      * failure. A return value of 0 indicates EOT.
>    593      */
>    594     ssize_t sbefifo_read(struct sbefifo_client *client, u32 *buf,
>    ssize_t len)
>    595     {
>    596             struct sbefifo *fifo = client->fifo;
>    597             unsigned long flags;
>    598             ssize_t rv;
>    599     
>    600             rv = wait_event_interruptible(client->wait,
>    601                                           (client->state ==
>    sbefifo_client_active ||
>    602                                            client->state ==
>    sbefifo_client_closed));
>    603             if (rv < 0)
>    604                     return rv;
>    605     
>    606             spin_lock_irqsave(&fifo->wait.lock, flags);
>    607             if (client->state == sbefifo_client_closed) {
>    608                     spin_unlock_irqrestore(&fifo->wait.lock,
>    flags);
>    609                     return -EBADFD;
>    610             }
>    611     
>    612             if (client->state == sbefifo_client_idle) {
>    613                     spin_unlock_irqrestore(&fifo->wait.lock,
>    flags);
>    614                     return -EIO;
>    615             }
>    616     
>    617             rv = wait_event_interruptible_locked_irq(fifo->wait,
>    618                                             fifo->state ==
>    sbefifo_interval ||
>    619                                             fifo->state ==
>    sbefifo_rx ||
>    620                                             fifo->state ==
>    sbefifo_ready ||
>    621                                             fifo->state ==
>    sbefifo_dead);
>    622             if (rv < 0) {
>    623                     spin_unlock_irqrestore(&fifo->wait.lock,
>    flags);
>    624                     return rv;
>    625             }
>    626     
>    627             if (fifo->state == sbefifo_ready) {
>    628                     /* We've reset FIFO, whatever we were waiting
>    for has gone */
>    629                     client->state = sbefifo_client_idle;
>    630                     /* We're done, wake another task up as the
>    FIFO is ready */
>    631                     wake_up_locked(&fifo->wait);
>    632                     spin_unlock_irqrestore(&fifo->wait.lock,
>    flags);
>    633                     return -EIO;
>    634             }
>    635     
>    636             if (fifo->state == sbefifo_dead) {
>    637                     spin_unlock_irqrestore(&fifo->wait.lock,
>    flags);
>    638                     return -ENODEV;
>    639             }
>    640     
>    641             fifo->state = sbefifo_rx;
>    642             spin_unlock_irqrestore(&fifo->wait.lock, flags);
>    643     
>    644             rv = sbefifo_down_read(fifo, buf, len);
>    645             if (rv > 0)
>    646                     return rv;
>    647     
>    648             /* Reset the FIFO on error */
>    649             if (rv < 0) {
>    650                     dev_err(fifo->dev, "FIFO read failed: %d\n",
>    rv);
>    651                     rv = sbefifo_reset(fifo);
>    652                     if (rv < 0)
>    653                             return rv;
>    654     
>    655                     rv = -EIO;
>    656             }
>    657     
>    658             /* Read is complete one way or the other (0 length
>    read or error) */
>    659             spin_lock_irqsave(&fifo->wait.lock, flags);
>    660             client->state = sbefifo_client_idle;
>    661     
>    662             /* Queue next FIFO transfer */
>    663             fifo->curr = NULL;
>    664             fifo->state = sbefifo_ready;
>    665             wake_up_locked(&fifo->wait);
>    666     
>    667             spin_unlock_irqrestore(&fifo->wait.lock, flags);
>    668     
>    669             return rv;
>    670     }
>    671     EXPORT_SYMBOL_GPL(sbefifo_read);
>    672     
>    673     /**
>    674      * sbefifo_release()
>    675      *
>    676      * @client      The client context for the SBEFIFO
>    677      *
>    678      */
>    679     int sbefifo_release(struct sbefifo_client *client)
>    680     {
>    681             struct sbefifo *fifo = client->fifo;
>    682             enum sbefifo_client_state old;
>    683             unsigned long flags;
>    684             int rv;
>    685     
>    686             /* Determine if we need to clean up */
>    687             spin_lock_irqsave(&client->fifo->wait.lock, flags);
>    688             old = client->state;
>    689             client->state = sbefifo_client_closed;
>    690     
>    691             if (old == sbefifo_client_closed) {
>    692                     spin_unlock_irqrestore(&fifo->wait.lock,
>    flags);
>    693                     return -EBADFD;
>    694             }
>    695     
>    696             if (old == sbefifo_client_idle) {
>    697                     spin_unlock_irqrestore(&fifo->wait.lock,
>    flags);
>    698                     return 0;
>    699             }
>    700     
>    701             /* We need to clean up, get noisy about
>    inconsistencies */
>    702             dev_warn(fifo->dev, "Releasing client with transfer in
>    progress!\n");
>    703             WARN_ON(old != sbefifo_client_active);
>    704             WARN_ON(fifo->state == sbefifo_ready);
>    705     
>    706             /* Mark ourselves as broken for cleanup */
>    707             fifo->state = sbefifo_broken;
>    708             fifo->curr = NULL;
>    709     
>    710             wake_up(&client->wait);
>    711             spin_unlock_irqrestore(&client->fifo->wait.lock,
>    flags);
>    712     
>    713             /* Clean up poll waiter */
>    714             spin_lock_irqsave(&fifo->poll.wait.lock, flags);
>    715             del_timer_sync(&fifo->poll.timer);
>    716             fifo->poll.rv = -EBADFD;
>    717             wake_up_all_locked(&fifo->poll.wait);
>    718             spin_unlock_irqrestore(&fifo->poll.wait.lock, flags);
>    719     
>    720             /* Reset the FIFO */
>    721             rv = sbefifo_reset(fifo);
>    722             if (rv < 0)
>    723                     return rv;
>    724     
>    725             /* Mark the FIFO as ready and wake pending transfer */
>    726             spin_lock_irqsave(&client->fifo->wait.lock, flags);
>    727             fifo->state = sbefifo_ready;
>    728             wake_up_locked(&fifo->wait);
>    729             spin_unlock_irqrestore(&client->fifo->wait.lock,
>    flags);
>    730     
>    731             return 0;
>    732     }
>    733     EXPORT_SYMBOL_GPL(sbefifo_release);
>    734     
>    735     static int sbefifo_unregister_child(struct device *dev, void
>    *data)
>    736     {
>    737             struct platform_device *pdev =
>    to_platform_device(dev);
>    738     
>    739             of_device_unregister(pdev);
>    740             if (dev->of_node)
>    741                     of_node_clear_flag(dev->of_node,
>    OF_POPULATED);
>    742     
>    743             return 0;
>    744     }
>    745     
>    746     static int sbefifo_probe(struct device *dev)
>    747     {
>    748             struct device_node *np;
>    749             struct sbefifo *fifo;
>    750             int child_idx;
>    751             u32 up, down;
>    752             int rv;
>    753     
>    754             fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
>    755             if (!fifo)
>    756                     return -ENOMEM;
>    757     
>    758             fifo->dev = dev;
>    759             fifo->state = sbefifo_ready;
>    760             fifo->fsi = to_fsi_dev(dev);
>    761     
>    762             fifo->id = ida_simple_get(&sbefifo_ida, 0, 0,
>    GFP_KERNEL);
>    763             if (fifo->id < 0)
>    764                     return fifo->id;
>    765     
>    766             init_waitqueue_head(&fifo->wait);
>    767     
>    768             /* No interrupts, need to poll the controller */
>  > 769		setup_timer(&fifo->poll.timer, sbefifo_poll_device,
>    770                         (unsigned long)fifo);
>    771             init_waitqueue_head(&fifo->poll.wait);
>    772     
>    773             rv = sbefifo_up_sts(fifo, &up);
>    774             if (rv < 0)
>    775                     return rv;
>    776     
>    777             rv = sbefifo_down_sts(fifo, &down);
>    778             if (rv < 0)
>    779                     return rv;
>    780     
>    781             if (!(sbefifo_empty(up) && sbefifo_empty(down)))  {
>    782                     dev_warn(fifo->dev, "FIFOs were not empty,
>    requesting reset from SBE\n");
>    783                     /* Request the SBE reset the FIFOs */
>    784                     rv = sbefifo_reset(fifo);
>    785                     if (rv == -ETIMEDOUT) {
>    786                             dev_warn(fifo->dev, "SBE unresponsive,
>    probing FIFO clients may fail. Performing hard FIFO reset\n");
>    787                             rv = sbefifo_do_reset(fifo);
>    788                             if (rv < 0)
>    789                                     return rv;
>    790                     } else if (rv < 0) {
>    791                             return rv;
>    792                     }
>    793             }
>    794     
>    795             dev_set_drvdata(dev, fifo);
>    796             list_add(&fifo->entry, &sbefifos);
>    797     
>    798             child_idx = 0;
>    799             for_each_available_child_of_node(dev->of_node, np) {
>    800                     struct platform_device *child;
>    801                     char name[32];
>    802     
>    803                     snprintf(name, sizeof(name),
>    "sbefifo%d-dev%d", fifo->id,
>    804                              child_idx++);
>    805                     child = of_platform_device_create(np, name,
>    dev);
>    806                     if (!child)
>    807                             dev_warn(dev, "Failed to create
>    platform device %s\n",
>    808                                      name);
>    809             }
>    810     
>    811             return 0;
>    812     }
>    813     
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology
> Center
> https://lists.01.org/pipermail/kbuild-all                   Intel
> Corporation
> Email had 1 attachment:
> + .config.gz
>   64k (application/gzip)

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

* Re: [PATCH] fsi: Add Self Boot Engine FIFO FSI client
  2017-12-01 13:07   ` [PATCH] fsi: Add Self Boot Engine FIFO FSI client Andrew Jeffery
  2017-12-03 20:25     ` kbuild test robot
@ 2017-12-04  4:50     ` kbuild test robot
  1 sibling, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2017-12-04  4:50 UTC (permalink / raw)
  To: Andrew Jeffery
  Cc: kbuild-all, linux-kernel, Andrew Jeffery, gregkh, devicetree,
	robh+dt, mark.rutland, bradleyb, cbostic, joel, eajames,
	Edward A. James

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

Hi Andrew,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.15-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Andrew-Jeffery/fsi-Add-Self-Boot-Engine-FIFO-FSI-client/20171204-031454
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   drivers/fsi/clients/fsi-sbefifo.c:378:21: warning: format '%d' expects argument of type 'int', but argument 9 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: Data phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fsi/clients/fsi-sbefifo.c:378:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: Data phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   drivers/fsi/clients/fsi-sbefifo.c:403:21: warning: format '%d' expects argument of type 'int', but argument 7 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fsi/clients/fsi-sbefifo.c:403:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   drivers/fsi/clients/fsi-sbefifo.c:403:21: warning: format '%d' expects argument of type 'int', but argument 9 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fsi/clients/fsi-sbefifo.c:403:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: EOT phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d\n, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   drivers/fsi/clients/fsi-sbefifo.c:417:21: warning: format '%d' expects argument of type 'int', but argument 7 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fsi/clients/fsi-sbefifo.c:417:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   drivers/fsi/clients/fsi-sbefifo.c:417:21: warning: format '%d' expects argument of type 'int', but argument 9 has type 'ssize_t {aka long int}' [-Wformat=]
     dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
                        ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fsi/clients/fsi-sbefifo.c:417:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(fifo->dev, "%s: Drain phase complete: valid_set: 0x%x, eot_set: 0x%x, nr_valid: %d, nr_xfer: %d, rem: %d\n",
     ^~~~~~~
   In file included from include/linux/list.h:9:0,
                    from include/linux/kobject.h:20,
                    from include/linux/device.h:17,
                    from include/linux/fsi.h:18,
                    from drivers/fsi/clients/fsi-sbefifo.c:6:
   drivers/fsi/clients/fsi-sbefifo.c: In function 'sbefifo_down_read':
   include/linux/kernel.h:792:16: warning: comparison of distinct pointer types lacks a cast
     (void) (&min1 == &min2);   \
                   ^
   include/linux/kernel.h:801:2: note: in expansion of macro '__min'
     __min(typeof(x), typeof(y),   \
     ^~~~~
   drivers/fsi/clients/fsi-sbefifo.c:453:34: note: in expansion of macro 'min'
     } while (rem && read && read == min((rem + read), SBEFIFO_FIFO_DEPTH));
                                     ^~~
   drivers/fsi/clients/fsi-sbefifo.c: In function 'sbefifo_write':
   drivers/fsi/clients/fsi-sbefifo.c:557:43: warning: format '%d' expects argument of type 'int', but argument 3 has type 'ssize_t {aka long int}' [-Wformat=]
      dev_err(fifo->dev, "FIFO write failed: %d\n", rv);
                                             ~^
                                             %ld
   In file included from arch/sparc/include/asm/bug.h:21:0,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:12,
                    from arch/sparc/include/asm/current.h:15,
                    from include/linux/mutex.h:14,
                    from include/linux/kernfs.h:13,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:21,
                    from include/linux/device.h:17,
                    from include/linux/fsi.h:18,
                    from drivers/fsi/clients/fsi-sbefifo.c:6:
   drivers/fsi/clients/fsi-sbefifo.c:572:17: warning: format '%d' expects argument of type 'int', but argument 4 has type 'ssize_t {aka long int}' [-Wformat=]
     WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
                    ^
   include/asm-generic/bug.h:91:69: note: in definition of macro '__WARN_printf'
    #define __WARN_printf(arg...) warn_slowpath_fmt(__FILE__, __LINE__, arg)
                                                                        ^~~
   drivers/fsi/clients/fsi-sbefifo.c:572:2: note: in expansion of macro 'WARN'
     WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
     ^~~~
   drivers/fsi/clients/fsi-sbefifo.c:572:17: warning: format '%d' expects argument of type 'int', but argument 5 has type 'ssize_t {aka long int}' [-Wformat=]
     WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
                    ^
   include/asm-generic/bug.h:91:69: note: in definition of macro '__WARN_printf'
    #define __WARN_printf(arg...) warn_slowpath_fmt(__FILE__, __LINE__, arg)
                                                                        ^~~
   drivers/fsi/clients/fsi-sbefifo.c:572:2: note: in expansion of macro 'WARN'
     WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
     ^~~~
   drivers/fsi/clients/fsi-sbefifo.c: In function 'sbefifo_read':
   drivers/fsi/clients/fsi-sbefifo.c:650:42: warning: format '%d' expects argument of type 'int', but argument 3 has type 'ssize_t {aka long int}' [-Wformat=]
      dev_err(fifo->dev, "FIFO read failed: %d\n", rv);
                                            ~^
                                            %ld
   drivers/fsi/clients/fsi-sbefifo.c: In function 'sbefifo_probe':
>> drivers/fsi/clients/fsi-sbefifo.c:769:2: error: implicit declaration of function 'setup_timer'; did you mean 'setup_tba'? [-Werror=implicit-function-declaration]
     setup_timer(&fifo->poll.timer, sbefifo_poll_device,
     ^~~~~~~~~~~
     setup_tba
   cc1: some warnings being treated as errors

vim +769 drivers/fsi/clients/fsi-sbefifo.c

   498	
   499	/**
   500	 * sbefifo_write()
   501	 *
   502	 * @client	The client context for the SBEFIFO
   503	 * @buf		The buffer of data to write, at least @len elements
   504	 * @len		The number elements in @buffer
   505	 *
   506	 * The buffer must represent a complete chip-op: EOT is signalled after the
   507	 * last element is written to the upstream FIFO.
   508	 *
   509	 * Returns the number of elements written on success and negative values on
   510	 * failure. If the call is successful a subsequent call to sbefifo_read() MUST
   511	 * be made.
   512	 */
   513	ssize_t sbefifo_write(struct sbefifo_client *client, const u32 *buf,
   514			      ssize_t len)
   515	{
   516		struct sbefifo *fifo = client->fifo;
   517		unsigned long flags;
   518		ssize_t rv;
   519	
   520		spin_lock_irqsave(&fifo->wait.lock, flags);
   521	
   522		if (client->state == sbefifo_client_active) {
   523			dev_warn(fifo->dev, "Transfer already in progress\n");
   524			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   525			return -EBUSY;
   526		}
   527	
   528		rv = wait_event_interruptible_locked_irq(fifo->wait,
   529							fifo->state == sbefifo_ready ||
   530							fifo->state == sbefifo_dead);
   531		if (rv < 0) {
   532			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   533			return rv;
   534		}
   535	
   536		if (fifo->state == sbefifo_dead) {
   537			client->state = sbefifo_client_closed;
   538			wake_up(&client->wait);
   539			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   540			return -ENODEV;
   541		}
   542	
   543		WARN_ON(fifo->state != sbefifo_ready);
   544	
   545		fifo->curr = client;
   546		fifo->state = sbefifo_tx;
   547	
   548		/* Move a threaded read() onto waiting for FIFO read readiness */
   549		client->state = sbefifo_client_active;
   550		wake_up(&client->wait);
   551	
   552		spin_unlock_irqrestore(&fifo->wait.lock, flags);
   553	
   554		/* FIFO Tx, reset the FIFO on error */
   555		rv = sbefifo_up_write(fifo, buf, len);
   556		if (rv < len) {
   557			dev_err(fifo->dev, "FIFO write failed: %d\n", rv);
   558			rv = sbefifo_reset(fifo);
   559			if (rv < 0)
   560				return rv;
   561	
   562			spin_lock_irqsave(&fifo->wait.lock, flags);
   563			fifo->state = sbefifo_ready;
   564			client->state = sbefifo_client_idle;
   565			wake_up(&client->wait);
   566			wake_up_locked(&fifo->wait);
   567			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   568	
   569			return -EIO;
   570		}
   571	
 > 572		WARN(rv > len, "Unreachable state: len: %d, rv: %d\n", len, rv);
   573	
   574		/* Write completed successfully */
   575		spin_lock_irqsave(&fifo->wait.lock, flags);
   576		fifo->state = sbefifo_interval;
   577		wake_up(&client->wait);
   578		spin_unlock_irqrestore(&fifo->wait.lock, flags);
   579	
   580		return rv;
   581	}
   582	EXPORT_SYMBOL_GPL(sbefifo_write);
   583	
   584	/**
   585	 * sbefifo_read()
   586	 *
   587	 * @client	The client context for the SBEFIFO
   588	 * @data	The buffer of data to write, at least @len elements
   589	 * @len		The number elements in @buffer
   590	 *
   591	 * Returns the number of elements read on success and negative values on
   592	 * failure. A return value of 0 indicates EOT.
   593	 */
   594	ssize_t sbefifo_read(struct sbefifo_client *client, u32 *buf, ssize_t len)
   595	{
   596		struct sbefifo *fifo = client->fifo;
   597		unsigned long flags;
   598		ssize_t rv;
   599	
   600		rv = wait_event_interruptible(client->wait,
   601					      (client->state == sbefifo_client_active ||
   602					       client->state == sbefifo_client_closed));
   603		if (rv < 0)
   604			return rv;
   605	
   606		spin_lock_irqsave(&fifo->wait.lock, flags);
   607		if (client->state == sbefifo_client_closed) {
   608			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   609			return -EBADFD;
   610		}
   611	
   612		if (client->state == sbefifo_client_idle) {
   613			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   614			return -EIO;
   615		}
   616	
   617		rv = wait_event_interruptible_locked_irq(fifo->wait,
   618						fifo->state == sbefifo_interval ||
   619						fifo->state == sbefifo_rx ||
   620						fifo->state == sbefifo_ready ||
   621						fifo->state == sbefifo_dead);
   622		if (rv < 0) {
   623			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   624			return rv;
   625		}
   626	
   627		if (fifo->state == sbefifo_ready) {
   628			/* We've reset FIFO, whatever we were waiting for has gone */
   629			client->state = sbefifo_client_idle;
   630			/* We're done, wake another task up as the FIFO is ready */
   631			wake_up_locked(&fifo->wait);
   632			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   633			return -EIO;
   634		}
   635	
   636		if (fifo->state == sbefifo_dead) {
   637			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   638			return -ENODEV;
   639		}
   640	
   641		fifo->state = sbefifo_rx;
   642		spin_unlock_irqrestore(&fifo->wait.lock, flags);
   643	
   644		rv = sbefifo_down_read(fifo, buf, len);
   645		if (rv > 0)
   646			return rv;
   647	
   648		/* Reset the FIFO on error */
   649		if (rv < 0) {
   650			dev_err(fifo->dev, "FIFO read failed: %d\n", rv);
   651			rv = sbefifo_reset(fifo);
   652			if (rv < 0)
   653				return rv;
   654	
   655			rv = -EIO;
   656		}
   657	
   658		/* Read is complete one way or the other (0 length read or error) */
   659		spin_lock_irqsave(&fifo->wait.lock, flags);
   660		client->state = sbefifo_client_idle;
   661	
   662		/* Queue next FIFO transfer */
   663		fifo->curr = NULL;
   664		fifo->state = sbefifo_ready;
   665		wake_up_locked(&fifo->wait);
   666	
   667		spin_unlock_irqrestore(&fifo->wait.lock, flags);
   668	
   669		return rv;
   670	}
   671	EXPORT_SYMBOL_GPL(sbefifo_read);
   672	
   673	/**
   674	 * sbefifo_release()
   675	 *
   676	 * @client	The client context for the SBEFIFO
   677	 *
   678	 */
   679	int sbefifo_release(struct sbefifo_client *client)
   680	{
   681		struct sbefifo *fifo = client->fifo;
   682		enum sbefifo_client_state old;
   683		unsigned long flags;
   684		int rv;
   685	
   686		/* Determine if we need to clean up */
   687		spin_lock_irqsave(&client->fifo->wait.lock, flags);
   688		old = client->state;
   689		client->state = sbefifo_client_closed;
   690	
   691		if (old == sbefifo_client_closed) {
   692			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   693			return -EBADFD;
   694		}
   695	
   696		if (old == sbefifo_client_idle) {
   697			spin_unlock_irqrestore(&fifo->wait.lock, flags);
   698			return 0;
   699		}
   700	
   701		/* We need to clean up, get noisy about inconsistencies */
   702		dev_warn(fifo->dev, "Releasing client with transfer in progress!\n");
   703		WARN_ON(old != sbefifo_client_active);
   704		WARN_ON(fifo->state == sbefifo_ready);
   705	
   706		/* Mark ourselves as broken for cleanup */
   707		fifo->state = sbefifo_broken;
   708		fifo->curr = NULL;
   709	
   710		wake_up(&client->wait);
   711		spin_unlock_irqrestore(&client->fifo->wait.lock, flags);
   712	
   713		/* Clean up poll waiter */
   714		spin_lock_irqsave(&fifo->poll.wait.lock, flags);
   715		del_timer_sync(&fifo->poll.timer);
   716		fifo->poll.rv = -EBADFD;
   717		wake_up_all_locked(&fifo->poll.wait);
   718		spin_unlock_irqrestore(&fifo->poll.wait.lock, flags);
   719	
   720		/* Reset the FIFO */
   721		rv = sbefifo_reset(fifo);
   722		if (rv < 0)
   723			return rv;
   724	
   725		/* Mark the FIFO as ready and wake pending transfer */
   726		spin_lock_irqsave(&client->fifo->wait.lock, flags);
   727		fifo->state = sbefifo_ready;
   728		wake_up_locked(&fifo->wait);
   729		spin_unlock_irqrestore(&client->fifo->wait.lock, flags);
   730	
   731		return 0;
   732	}
   733	EXPORT_SYMBOL_GPL(sbefifo_release);
   734	
   735	static int sbefifo_unregister_child(struct device *dev, void *data)
   736	{
   737		struct platform_device *pdev = to_platform_device(dev);
   738	
   739		of_device_unregister(pdev);
   740		if (dev->of_node)
   741			of_node_clear_flag(dev->of_node, OF_POPULATED);
   742	
   743		return 0;
   744	}
   745	
   746	static int sbefifo_probe(struct device *dev)
   747	{
   748		struct device_node *np;
   749		struct sbefifo *fifo;
   750		int child_idx;
   751		u32 up, down;
   752		int rv;
   753	
   754		fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
   755		if (!fifo)
   756			return -ENOMEM;
   757	
   758		fifo->dev = dev;
   759		fifo->state = sbefifo_ready;
   760		fifo->fsi = to_fsi_dev(dev);
   761	
   762		fifo->id = ida_simple_get(&sbefifo_ida, 0, 0, GFP_KERNEL);
   763		if (fifo->id < 0)
   764			return fifo->id;
   765	
   766		init_waitqueue_head(&fifo->wait);
   767	
   768		/* No interrupts, need to poll the controller */
 > 769		setup_timer(&fifo->poll.timer, sbefifo_poll_device,
   770			    (unsigned long)fifo);
   771		init_waitqueue_head(&fifo->poll.wait);
   772	
   773		rv = sbefifo_up_sts(fifo, &up);
   774		if (rv < 0)
   775			return rv;
   776	
   777		rv = sbefifo_down_sts(fifo, &down);
   778		if (rv < 0)
   779			return rv;
   780	
   781		if (!(sbefifo_empty(up) && sbefifo_empty(down)))  {
   782			dev_warn(fifo->dev, "FIFOs were not empty, requesting reset from SBE\n");
   783			/* Request the SBE reset the FIFOs */
   784			rv = sbefifo_reset(fifo);
   785			if (rv == -ETIMEDOUT) {
   786				dev_warn(fifo->dev, "SBE unresponsive, probing FIFO clients may fail. Performing hard FIFO reset\n");
   787				rv = sbefifo_do_reset(fifo);
   788				if (rv < 0)
   789					return rv;
   790			} else if (rv < 0) {
   791				return rv;
   792			}
   793		}
   794	
   795		dev_set_drvdata(dev, fifo);
   796		list_add(&fifo->entry, &sbefifos);
   797	
   798		child_idx = 0;
   799		for_each_available_child_of_node(dev->of_node, np) {
   800			struct platform_device *child;
   801			char name[32];
   802	
   803			snprintf(name, sizeof(name), "sbefifo%d-dev%d", fifo->id,
   804				 child_idx++);
   805			child = of_platform_device_create(np, name, dev);
   806			if (!child)
   807				dev_warn(dev, "Failed to create platform device %s\n",
   808					 name);
   809		}
   810	
   811		return 0;
   812	}
   813	

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

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

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

end of thread, other threads:[~2017-12-04  4:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-17 19:34 [PATCH v4 0/4] drivers/fsi: Add SBEFIFO client driver Eddie James
2017-11-17 19:34 ` [PATCH v4 1/4] dt-bindings: fsi: Add SBEFIFO documentation Eddie James
2017-11-17 19:34 ` [PATCH v4 2/4] drivers/fsi: Add SBEFIFO FSI client device driver Eddie James
2017-11-17 19:34 ` [PATCH v4 3/4] drivers/fsi: sbefifo: Add miscdevice Eddie James
2017-11-17 19:34 ` [PATCH v4 4/4] drivers/fsi: sbefifo: Add in-kernel API Eddie James
2017-11-21  8:07   ` kbuild test robot
2017-12-01 13:07   ` [PATCH] fsi: Add Self Boot Engine FIFO FSI client Andrew Jeffery
2017-12-03 20:25     ` kbuild test robot
2017-12-04  0:05       ` Andrew Jeffery
2017-12-04  4:50     ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).