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


Groups > linux.kernel > #1537372 > unrolled thread

[PATCH 00/16] FSI device driver introduction

Started byChris Bostic <christopher.lee.bostic@gmail.com>
First post2016-12-07 01:20 +0100
Last post2016-12-07 03:00 +0100
Articles 7 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 00/16] FSI device driver introduction Chris Bostic <christopher.lee.bostic@gmail.com> - 2016-12-07 01:20 +0100
    [PATCH 03/16] drivers/fsi: add driver to device matches Chris Bostic <christopher.lee.bostic@gmail.com> - 2016-12-07 01:20 +0100
    [PATCH 15/16] drivers/fsi: Add documentation for GPIO bindings Chris Bostic <christopher.lee.bostic@gmail.com> - 2016-12-07 01:20 +0100
      Re: [PATCH 15/16] drivers/fsi: Add documentation for GPIO bindings Mark Rutland <mark.rutland@arm.com> - 2016-12-07 13:10 +0100
    [PATCH 06/16] drivers/fsi: Add slave definition Chris Bostic <christopher.lee.bostic@gmail.com> - 2016-12-07 01:20 +0100
    [PATCH 10/16] drivers/fsi: scan slaves & register devices Chris Bostic <christopher.lee.bostic@gmail.com> - 2016-12-07 01:20 +0100
    Re: [PATCH 00/16] FSI device driver introduction Sebastian Reichel <sre@kernel.org> - 2016-12-07 03:00 +0100

#1537372 — [PATCH 00/16] FSI device driver introduction

FromChris Bostic <christopher.lee.bostic@gmail.com>
Date2016-12-07 01:20 +0100
Subject[PATCH 00/16] FSI device driver introduction
Message-ID<sLy7L-GS-9@gated-at.bofh.it>
From: Chris Bostic <cbostic@us.ibm.com>

Introduction of the IBM 'Flexible Support Interface' (FSI) bus device
driver. FSI is a high fan out serial bus consisting of a clock and a serial
data line capable of running at speeds up to 166 MHz.

This set provides the basic framework to add FSI extensions to the
Linux bus and device models. Master specific implementations are
defined to utilize the core FSI function.

In Linux, we have a core FSI "bus type", along with drivers for FSI
masters and engines.

The FSI master drivers expose a read/write interface to the bus address
space. The master drivers are under drivers/fsi/fsi-master-*.c.

The core handles probing and discovery of slaves and slave
engines, using those read/write interfaces. It is responsible for
creating the endpoint Linux devices corresponding to the discovered
engines on each slave.

Slave engines are identified by an 'engine' type, and an optional
version. Engine, a.k.a. client, drivers are matched and bound to these
engines during discovery.

This patch set does not include extended FSI function such as:
    *  Hub master support
    *  Cascaded master support
    *  Application layer hot plug notification
    *  Application layer FSI bus status interface

Common FSI terminology:

* Master
    Controller of the FSI bus.  Only the master is allowed to control the
    clock line and is the initiator of all transactions on a bus.

* Slave
    The receiver or target of a master initiated transaction.  The slave
    cannot initiate communications on a bus and must respond to any
    master requests for data.

* CFAM
    Stands for Common Field replaceable unit Access Macro.  A CFAM is an
    ASIC residing in any device requiring FSI communications. CFAMs
    consist of an array of hardware 'engines' used for various purposes.
    I2C masters, UARTs, General Purpose IO hardware are common types of
    these engines.

* Configuration Space / Table
    A table contained at the beginning of each CFAM address space.
    This table lists information such as the CFAM's ID, which engine types
    and versions it has available, as well as its addressing range.

* FSI Engine driver
    A device driver that registers with the FSI core so that it can access
    devices it owns on an FSI bus.


Chris Bostic (5):
  drivers/fsi: Set up links for slave communication
  drivers/fsi: Set slave SMODE to init communication
  drivers/fsi: Add master unscan
  drivers/fsi: Add documentation for GPIO bindings
  drivers/fsi: Add GPIO based FSI master

Jeremy Kerr (11):
  drivers/fsi: Add empty fsi bus definitions
  drivers/fsi: Add device & driver definitions
  drivers/fsi: add driver to device matches
  drivers/fsi: Add fsi master definition
  drivers/fsi: Add fake master driver
  drivers/fsi: Add slave definition
  drivers/fsi: Add empty master scan
  drivers/fsi: Add crc4 helpers
  drivers/fsi: Implement slave initialisation
  drivers/fsi: scan slaves & register devices
  drivers/fsi: Add device read/write/peek functions

 .../devicetree/bindings/fsi/fsi-master-gpio.txt    |  21 +
 drivers/Kconfig                                    |   2 +
 drivers/Makefile                                   |   1 +
 drivers/fsi/Kconfig                                |  29 ++
 drivers/fsi/Makefile                               |   4 +
 drivers/fsi/fsi-core.c                             | 514 +++++++++++++++++++
 drivers/fsi/fsi-master-fake.c                      |  95 ++++
 drivers/fsi/fsi-master-gpio.c                      | 552 +++++++++++++++++++++
 drivers/fsi/fsi-master.h                           |  62 +++
 include/linux/fsi.h                                |  60 +++
 10 files changed, 1340 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
 create mode 100644 drivers/fsi/Kconfig
 create mode 100644 drivers/fsi/Makefile
 create mode 100644 drivers/fsi/fsi-core.c
 create mode 100644 drivers/fsi/fsi-master-fake.c
 create mode 100644 drivers/fsi/fsi-master-gpio.c
 create mode 100644 drivers/fsi/fsi-master.h
 create mode 100644 include/linux/fsi.h

-- 
1.8.2.2

[toc] | [next] | [standalone]


#1537373 — [PATCH 03/16] drivers/fsi: add driver to device matches

FromChris Bostic <christopher.lee.bostic@gmail.com>
Date2016-12-07 01:20 +0100
Subject[PATCH 03/16] drivers/fsi: add driver to device matches
Message-ID<sLy7M-GS-23@gated-at.bofh.it>
In reply to#1537372
From: Jeremy Kerr <jk@ozlabs.org>

Driver bind to devices based on the engine types & (optional) versions.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 drivers/fsi/fsi-core.c | 21 +++++++++++++++++++++
 include/linux/fsi.h    | 21 +++++++++++++++++++--
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 3e45306..3d55bd5 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -19,8 +19,29 @@
 
 /* FSI core & Linux bus type definitions */
 
+static int fsi_bus_match(struct device *dev, struct device_driver *drv)
+{
+	struct fsi_device *fsi_dev = to_fsi_dev(dev);
+	struct fsi_driver *fsi_drv = to_fsi_drv(drv);
+	const struct fsi_device_id *id;
+
+	if (!fsi_drv->id_table)
+		return 0;
+
+	for (id = fsi_drv->id_table; id->engine_type; id++) {
+		if (id->engine_type != fsi_dev->engine_type)
+			continue;
+		if (id->version == FSI_VERSION_ANY ||
+				id->version == fsi_dev->version)
+			return 1;
+	}
+
+	return 0;
+}
+
 struct bus_type fsi_bus_type = {
 	.name		= "fsi",
+	.match		= fsi_bus_match,
 };
 EXPORT_SYMBOL_GPL(fsi_bus_type);
 
diff --git a/include/linux/fsi.h b/include/linux/fsi.h
index f73886a..273cbf6 100644
--- a/include/linux/fsi.h
+++ b/include/linux/fsi.h
@@ -18,11 +18,28 @@
 #include <linux/device.h>
 
 struct fsi_device {
-	struct device dev;
+	struct device		dev;
+	u8			engine_type;
+	u8			version;
 };
 
+struct fsi_device_id {
+	u8	engine_type;
+	u8	version;
+};
+
+#define FSI_VERSION_ANY		0
+
+#define FSI_DEVICE(t) \
+	.engine_type = (t), .version = FSI_VERSION_ANY,
+
+#define FSI_DEVICE_VERSIONED(t, v) \
+	.engine_type = (t), .version = (v),
+
+
 struct fsi_driver {
-	struct device_driver drv;
+	struct device_driver		drv;
+	const struct fsi_device_id	*id_table;
 };
 
 #define to_fsi_dev(devp) container_of(devp, struct fsi_device, dev)
-- 
1.8.2.2

[toc] | [prev] | [next] | [standalone]


#1537374 — [PATCH 15/16] drivers/fsi: Add documentation for GPIO bindings

FromChris Bostic <christopher.lee.bostic@gmail.com>
Date2016-12-07 01:20 +0100
Subject[PATCH 15/16] drivers/fsi: Add documentation for GPIO bindings
Message-ID<sLy7M-GS-35@gated-at.bofh.it>
In reply to#1537372
From: Chris Bostic <cbostic@us.ibm.com>

Add fsi master gpio device tree binding documentation

Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 .../devicetree/bindings/fsi/fsi-master-gpio.txt     | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt

diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt b/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
new file mode 100644
index 0000000..ff3a62e
--- /dev/null
+++ b/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
@@ -0,0 +1,21 @@
+Device-tree bindings for gpio-based FSI master driver
+-----------------------------------------------------
+
+Required properties:
+	- compatible = "ibm,fsi-master-gpio";
+	- clk-gpios;
+	- data-gpios;
+
+Optional properties:
+	- enable-gpios;
+	- trans-gpios;
+	- mux-gpios;
+
+fsi-master {
+	compatible = "ibm,fsi-master", "ibm,fsi-master-gpio";
+	clk-gpios = <&gpio 0 &gpio 6>;
+	data-gpios = <&gpio 1 &gpio 7>;
+	enable-gpios = <&gpio 2 &gpio 8>;	/* Enable FSI data in/out */
+	trans-gpios = <&gpio 3 &gpio 9>;	/* Volts translator direction */
+	mux-gpios = <&gpio 4> &gpio 10>;	/* Multiplexer for FSI pins */
+}
-- 
1.8.2.2

[toc] | [prev] | [next] | [standalone]


#1537707 — Re: [PATCH 15/16] drivers/fsi: Add documentation for GPIO bindings

FromMark Rutland <mark.rutland@arm.com>
Date2016-12-07 13:10 +0100
SubjectRe: [PATCH 15/16] drivers/fsi: Add documentation for GPIO bindings
Message-ID<sLJcR-8nG-3@gated-at.bofh.it>
In reply to#1537374
On Tue, Dec 06, 2016 at 06:14:36PM -0600, Chris Bostic wrote:
> From: Chris Bostic <cbostic@us.ibm.com>
> 
> Add fsi master gpio device tree binding documentation

Please see Documentation/devicetree/bindings/submitting-patches.txt.

Specifically:

* Please put binding documents earlier in the series than code
  implementing the binding.

* Please document _all_ compatible strings used in the
  series.

Please also write the binding documents in terms of the hardware, rather
then the driver (e.g. introduce what the hardware is in the document,
don't mention the driver). The bindings are there to describe the former
to the latter, and the latter may change arbitrarily.

> Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
> ---
>  .../devicetree/bindings/fsi/fsi-master-gpio.txt     | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
> 
> diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt b/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt

AFAICT, this is the first use of this directory. We should have a
general FSI binding document in there, covering what FSI is, the
"ibm,fsi-master" binding, etc.

> new file mode 100644
> index 0000000..ff3a62e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
> @@ -0,0 +1,21 @@
> +Device-tree bindings for gpio-based FSI master driver

There's very little information here, so I'm not sure what to make of
this. Can you please elaborate on the above to make it clear what this
means?

IIUC, this is an FSI controller/master that we only communicate with via
GPIOs, right?

Or is this a *virtual* master? i.e. the GPIOs themselves form the master
and are directly connected to slaves?

> +-----------------------------------------------------
> +
> +Required properties:
> +	- compatible = "ibm,fsi-master-gpio";
> +	- clk-gpios;
> +	- data-gpios;

Please give a description of what each of these are used for, how many
are required, and what order elements must come in.

> +Optional properties:
> +	- enable-gpios;
> +	- trans-gpios;
> +	- mux-gpios;

Likewise.

> +
> +fsi-master {
> +	compatible = "ibm,fsi-master", "ibm,fsi-master-gpio";

This is backwards. The most specific string must come first.

> +	clk-gpios = <&gpio 0 &gpio 6>;
> +	data-gpios = <&gpio 1 &gpio 7>;
> +	enable-gpios = <&gpio 2 &gpio 8>;	/* Enable FSI data in/out */
> +	trans-gpios = <&gpio 3 &gpio 9>;	/* Volts translator direction */
> +	mux-gpios = <&gpio 4> &gpio 10>;	/* Multiplexer for FSI pins */

If this were described above, we don't need the comment here.

I note that in the patch, the mux-gpios property has an unmatched '>'
and won't compile.

As a general nit, please bracket elements of a list individually, e.g.

	trans-gpios = <&gpio 3>, <&gpio 9>;
	mux-gpios = <&gpio 4>, <&gpio 10>;

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]


#1537375 — [PATCH 06/16] drivers/fsi: Add slave definition

FromChris Bostic <christopher.lee.bostic@gmail.com>
Date2016-12-07 01:20 +0100
Subject[PATCH 06/16] drivers/fsi: Add slave definition
Message-ID<sLy7M-GS-25@gated-at.bofh.it>
In reply to#1537372
From: Jeremy Kerr <jk@ozlabs.org>

Add the initial fsi slave device, which is private to the core code.
This will be a child of the master, and parent to endpoint devices.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 drivers/fsi/fsi-core.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index ce9428d..60a6d91 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -21,6 +21,15 @@
 
 static atomic_t master_idx = ATOMIC_INIT(-1);
 
+struct fsi_slave {
+	struct device		dev;
+	struct fsi_master	*master;
+	int			link;
+	uint8_t			id;
+};
+
+#define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
+
 /* FSI master support */
 
 int fsi_master_register(struct fsi_master *master)
-- 
1.8.2.2

[toc] | [prev] | [next] | [standalone]


#1537376 — [PATCH 10/16] drivers/fsi: scan slaves & register devices

FromChris Bostic <christopher.lee.bostic@gmail.com>
Date2016-12-07 01:20 +0100
Subject[PATCH 10/16] drivers/fsi: scan slaves & register devices
Message-ID<sLy7M-GS-31@gated-at.bofh.it>
In reply to#1537372
From: Jeremy Kerr <jk@ozlabs.org>

Now that we have fsi_slave devices, scan each for endpoints, and
register them on the fsi bus.

Includes contributions from Chris Bostic <cbostic@us.ibm.com>

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 drivers/fsi/fsi-core.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/fsi.h    |   4 ++
 2 files changed, 136 insertions(+), 4 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index aa4330a..b51ea35 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -21,9 +21,19 @@
 #include "fsi-master.h"
 
 #define FSI_N_SLAVES	4
-#define FSI_SLAVE_CONF_CRC_SHIFT        4
-#define FSI_SLAVE_CONF_CRC_MASK         0x0000000f
-#define FSI_SLAVE_CONF_DATA_BITS        28
+
+#define FSI_SLAVE_CONF_NEXT_MASK	0x80000000
+#define FSI_SLAVE_CONF_SLOTS_MASK	0x00ff0000
+#define FSI_SLAVE_CONF_SLOTS_SHIFT	16
+#define FSI_SLAVE_CONF_VERSION_MASK	0x0000f000
+#define FSI_SLAVE_CONF_VERSION_SHIFT	12
+#define FSI_SLAVE_CONF_TYPE_MASK	0x00000ff0
+#define FSI_SLAVE_CONF_TYPE_SHIFT	4
+#define FSI_SLAVE_CONF_CRC_SHIFT	4
+#define FSI_SLAVE_CONF_CRC_MASK		0x0000000f
+#define FSI_SLAVE_CONF_DATA_BITS	28
+
+static const int engine_page_size = 0x400;
 
 static atomic_t master_idx = ATOMIC_INIT(-1);
 
@@ -36,6 +46,30 @@ struct fsi_slave {
 
 #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+/* FSI endpoint-device support */
+
+static void fsi_device_release(struct device *_device)
+{
+	struct fsi_device *device = to_fsi_dev(_device);
+
+	kfree(device);
+}
+
+static struct fsi_device *fsi_create_device(struct fsi_slave *slave)
+{
+	struct fsi_device *dev;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return NULL;
+
+	dev->dev.parent = &slave->dev;
+	dev->dev.bus = &fsi_bus_type;
+	dev->dev.release = fsi_device_release;
+
+	return dev;
+}
+
 /* crc helpers */
 static const uint8_t crc4_tab[] = {
 	0x0, 0x7, 0xe, 0x9, 0xb, 0xc, 0x5, 0x2,
@@ -59,6 +93,99 @@ uint8_t fsi_crc4(uint8_t c, uint64_t x, int bits)
 
 /* FSI slave support */
 
+static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
+			void *val, size_t size)
+{
+	return slave->master->read(slave->master, slave->link,
+			slave->id, addr, val, size);
+}
+
+static int fsi_slave_scan(struct fsi_slave *slave)
+{
+	uint32_t engine_addr;
+	uint32_t conf;
+	int rc, i;
+
+	/*
+	 * scan engines
+	 *
+	 * We keep the peek mode and slave engines for the core; so start
+	 * at the third slot in the configuration table. We also need to
+	 * skip the chip ID entry at the start of the address space.
+	 */
+	engine_addr = engine_page_size * 3;
+	for (i = 2; i < engine_page_size / sizeof(uint32_t); i++) {
+		uint8_t slots, version, type, crc;
+		struct fsi_device *dev;
+
+		rc = fsi_slave_read(slave, (i + 1) * sizeof(conf),
+				&conf, sizeof(conf));
+		if (rc) {
+			dev_warn(&slave->dev,
+				"error reading slave registers\n");
+			return -1;
+		}
+
+		crc = fsi_crc4(0, conf >> FSI_SLAVE_CONF_CRC_SHIFT,
+				FSI_SLAVE_CONF_DATA_BITS);
+		if (crc != (conf & FSI_SLAVE_CONF_CRC_MASK)) {
+			dev_warn(&slave->dev,
+				"crc error in slave register at 0x%04x\n",
+				i);
+			return -1;
+		}
+
+		slots = (conf & FSI_SLAVE_CONF_SLOTS_MASK)
+			>> FSI_SLAVE_CONF_SLOTS_SHIFT;
+		version = (conf & FSI_SLAVE_CONF_VERSION_MASK)
+			>> FSI_SLAVE_CONF_VERSION_SHIFT;
+		type = (conf & FSI_SLAVE_CONF_TYPE_MASK)
+			>> FSI_SLAVE_CONF_TYPE_SHIFT;
+
+		/*
+		 * Unused address areas are marked by a zero type value; this
+		 * skips the defined address areas
+		 */
+		if (type != 0) {
+
+			/* create device */
+			dev = fsi_create_device(slave);
+			if (!dev)
+				return -ENOMEM;
+
+			dev->slave = slave;
+			dev->engine_type = type;
+			dev->version = version;
+			dev->unit = i;
+			dev->addr = engine_addr;
+			dev->size = slots * engine_page_size;
+
+			dev_info(&slave->dev,
+			"engine[%i]: type %x, version %x, addr %x size %x\n",
+					dev->unit, dev->engine_type, version,
+					dev->addr, dev->size);
+
+			device_initialize(&dev->dev);
+			dev_set_name(&dev->dev, "%02x:%02x:%02x:%02x",
+					slave->master->idx, slave->link,
+					slave->id, i - 2);
+
+			rc = device_add(&dev->dev);
+			if (rc) {
+				dev_warn(&slave->dev, "add failed: %d\n", rc);
+				put_device(&dev->dev);
+			}
+		}
+
+		engine_addr += slots * engine_page_size;
+
+		if (!(conf & FSI_SLAVE_CONF_NEXT_MASK))
+			break;
+	}
+
+	return 0;
+}
+
 static void fsi_slave_release(struct device *dev)
 {
 	struct fsi_slave *slave = to_fsi_slave(dev);
@@ -110,7 +237,8 @@ static int fsi_slave_init(struct fsi_master *master,
 		return rc;
 	}
 
-	return rc;
+	fsi_slave_scan(slave);
+	return 0;
 }
 
 /* FSI master support */
diff --git a/include/linux/fsi.h b/include/linux/fsi.h
index 273cbf6..efa55ba 100644
--- a/include/linux/fsi.h
+++ b/include/linux/fsi.h
@@ -21,6 +21,10 @@ struct fsi_device {
 	struct device		dev;
 	u8			engine_type;
 	u8			version;
+	u8			unit;
+	struct fsi_slave	*slave;
+	uint32_t		addr;
+	uint32_t		size;
 };
 
 struct fsi_device_id {
-- 
1.8.2.2

[toc] | [prev] | [next] | [standalone]


#1537414

FromSebastian Reichel <sre@kernel.org>
Date2016-12-07 03:00 +0100
Message-ID<sLzGx-1ry-5@gated-at.bofh.it>
In reply to#1537372

[Multipart message — attachments visible in raw view] — view raw

Hi.

On Tue, Dec 06, 2016 at 06:14:21PM -0600, Chris Bostic wrote:
> [...]
>
> Introduction of the IBM 'Flexible Support Interface' (FSI) bus device
> driver. FSI is a high fan out serial bus consisting of a clock and a serial
> data line capable of running at speeds up to 166 MHz.
> 
> [...]

I would expect, that this information is added to Documentation/
and there should be Documentation/ABI/<stable or testing>/sysfs-bus-fsi

P.S.: I'm not sure, why I'm Cc'd.

-- Sebastian

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web