Skip to contentSkip to conversation

drivers/fsi: Add SBEFIFO client driver

PATCH Superseded Eddie James 3 participants 10 messages Nov 17, 2017 – Dec 4, 2017

Subsystems (2)

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v4 cover drivers/fsi: Add SBEFIFO client driver

From: "Edward A. James" <eajames-r/[email protected]>

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
Signature · 6 lines
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#1 lore raw

PATCH v4 1/4 dt-bindings: fsi: Add SBEFIFO documentation

From: "Edward A. James" <eajames-r/[email protected]>

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

---
 .../devicetree/bindings/fsi/ibm,p9-sbefifo.txt     | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/ibm,p9-sbefifo.txt
Documentation/devicetree/bindings/fsi/ibm,p9-sbefifo.txt new file +35 -0
new file mode 100644
index 0000000..e69b0c0
@@ -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 >;
+        };
+    };
Signature · 6 lines
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#2 lore raw

PATCH v4 2/4 drivers/fsi: Add SBEFIFO FSI client device driver

In reply to Eddie James

From: "Edward A. James" <[email protected]>

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 <[email protected]>

---
 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
drivers/fsi/Kconfig +7 -0
index 6821ed0..8c4d903 100644
@@ -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
drivers/fsi/Makefile +1 -0
index 65eb99d..851182e 100644
@@ -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
drivers/fsi/fsi-sbefifo.c new file +575 -0

575 lines not on this page · raw message

Signature · 1 line
1.8.3.1
#3 lore raw

PATCH v4 3/4 drivers/fsi: sbefifo: Add miscdevice

In reply to Eddie James

From: "Edward A. James" <[email protected]>

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

---
 drivers/fsi/fsi-sbefifo.c | 354 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 354 insertions(+)
drivers/fsi/fsi-sbefifo.c +354 -0
index 2439958..9970e2d 100644
@@ -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))
Signature · 1 line
1.8.3.1
#4 lore raw

PATCH v4 4/4 drivers/fsi: sbefifo: Add in-kernel API

In reply to Eddie James

From: "Edward A. James" <[email protected]>

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.

---
 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
drivers/fsi/fsi-sbefifo.c +116 -21
index 9970e2d..92947b0 100644
@@ -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);
 
include/linux/fsi-sbefifo.h new file +30 -0
new file mode 100644
index 0000000..8e55891
@@ -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 */
Signature · 1 line
1.8.3.1
#5 lore raw

Re: [PATCH v4 4/4] drivers/fsi: sbefifo: Add in-kernel API

Thank you for the patch! Yet something to improve:

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

#6 lore raw

PATCH 1/1 fsi: Add Self Boot Engine FIFO FSI client

In reply to Eddie 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.

---
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
drivers/fsi/Kconfig +2 -0
index 6821ed0cd5e8..1a6548e3be92 100644
@@ -33,6 +33,8 @@ config FSI_SCOM
 	---help---
 	This option enables an FSI based SCOM device driver.
 
+source "drivers/fsi/clients/Kconfig"
+
 endif
 
 endmenu
drivers/fsi/Makefile +5 -4
index 65eb99dfafdb..8e158a299701 100644
@@ -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/
drivers/fsi/clients/Kconfig new file +5 -0
new file mode 100644
index 000000000000..3cb3feba84bb
@@ -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.
drivers/fsi/clients/Makefile new file +1 -0
new file mode 100644
index 000000000000..d71a4de7aa0f
@@ -0,0 +1 @@
+obj-$(CONFIG_FSI_SBEFIFO)	+= fsi-sbefifo.o
drivers/fsi/clients/fsi-sbefifo.c new file +892 -0

892 lines not on this page · raw message

drivers/fsi/clients/fsi-sbefifo.h new file +82 -0

82 lines not on this page · raw message

Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#7 lore raw

Re: [PATCH] fsi: Add Self Boot Engine FIFO FSI client

Thank you for the patch! Yet something to improve:

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

#8 lore raw

Re: [PATCH] fsi: Add Self Boot Engine FIFO FSI client

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

Quoted 601 lines · kbuild test robot

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 >>):

Quoted 1 line
Quoted 1 line

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':

Quoted 1 line
Quoted 1 line

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, \
                                          ^~~
Quoted 1 line
Quoted 1 line

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, \
                                          ^~~
Quoted 1 line
Quoted 1 line

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),   \
     ^~~~~
Quoted 1 line
Quoted 1 line

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)
                                                                        ^~~
Quoted 1 line
Quoted 1 line

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)
                                                                        ^~~
Quoted 1 line
Quoted 1 line

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':
Quoted 1 line
Quoted 1 line

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     
Quoted 1 line

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 */
Quoted 1 line

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     
Quoted 8 lines

---
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)

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#9 lore raw

Re: [PATCH] fsi: Add Self Boot Engine FIFO FSI client

In reply to Andrew Jeffery
Thank you for the patch! Yet something to improve:

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