[PATCH linux 00/15] FSI device driver introduction

[PATCH linux 00/15] FSI device driver introduction

[christopher.lee.bostic]

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.

Patches 01-13 of 15 have been provided by Jeremy Kerr as an example of
how FSI function should be phased in. This includes the basic framework
to add FSI extensions to the Linux bus and device models.  Patch 13
is a beginning definition of a GPIO controlled master controlled in
firmware.  Patch 14 contains only my recommended changes to Jeremy's set.
Patch 15 adds function to set up read/write access to a given CFAM.

Note patch 05/15 and patch 13/15 have blank lines at EOF and so
git am will warn on application of these two.  In order to make it
clear what changes Jeremy submitted and preserve his change history
I left them as is.  As per my understanding we wanted to make it
clear what changes were Jeremy's versus mine.

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.

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
    *  Host configuration allowing for various hardware / firmware
       emulation implementations of the FSI master.

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.

* Link
    The combination of a serial clock and data line constituting one
    FSI communications element. For each link there is a master on one
    end and a CFAM/slave on the other end.

* Engine
    A self contained hardware function found within a CFAM.  Examples
    include I2C masters, UARTs, GPIOs, etc...

* Client
    A device driver requiring access to its hardware via an FSI bus.
    For example an I2C client would be a device driver requiring
    access to an I2C master engine on a remote CFAM accessible via
    an FSI link.  Clients register with the FSI bus and will receive
    notifications of bus state changes as well as hot plug events related
    to engines of interest to them.

* Hub / Hub master
    Provides extension to the existing primary FSI master.  Allows for
    several chained FSI links in series to a target device thus increasing
    potential fan out.

* Cascaded master
    A subset of functionality of the  hub master.  Cascaded masters can
    access only a limited address range compared to hub masters.  This was
    the first generation implementation, essentially, of hub type function.

---

Chris Bostic (2):
  drivers/fsi: Rename slave to cfam
  drivers/fsi: Initialize CFAMs for read/write access

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

 .../devicetree/bindings/fsi/fsi-master-gpio.txt    |   8 +
 drivers/Kconfig                                    |   2 +
 drivers/Makefile                                   |   1 +
 drivers/fsi/Kconfig                                |  29 ++
 drivers/fsi/Makefile                               |   4 +
 drivers/fsi/fsi-cfam.h                             |  72 ++++
 drivers/fsi/fsi-core.c                             | 386 +++++++++++++++++++++
 drivers/fsi/fsi-master-fake.c                      |  99 ++++++
 drivers/fsi/fsi-master-gpio.c                      |  95 +++++
 drivers/fsi/fsi-master.h                           |  42 +++
 drivers/fsi/fsi-slave.h                            |  62 ++++
 include/linux/fsi.h                                |  60 ++++
 12 files changed, 860 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-cfam.h
 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 drivers/fsi/fsi-slave.h
 create mode 100644 include/linux/fsi.h

-- 
1.8.2.2

[PATCH linux 01/15] fsi: Add empty fsi bus definitions

[christopher.lee.bostic]

From: Jeremy Kerr <jk@ozlabs.org>

This change adds the initial (empty) fsi bus definition, and introduces
drivers/fsi/.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 drivers/Kconfig        |  2 ++
 drivers/Makefile       |  1 +
 drivers/fsi/Kconfig    | 12 ++++++++++++
 drivers/fsi/Makefile   |  2 ++
 drivers/fsi/fsi-core.c | 38 ++++++++++++++++++++++++++++++++++++++
 include/linux/fsi.h    | 23 +++++++++++++++++++++++
 6 files changed, 78 insertions(+)
 create mode 100644 drivers/fsi/Kconfig
 create mode 100644 drivers/fsi/Makefile
 create mode 100644 drivers/fsi/fsi-core.c
 create mode 100644 include/linux/fsi.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index e1e2066..117ca14c 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -202,4 +202,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
 
 source "drivers/fpga/Kconfig"
 
+source "drivers/fsi/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 0b6f3d6..7a1c96f 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -173,3 +173,4 @@ obj-$(CONFIG_STM)		+= hwtracing/stm/
 obj-$(CONFIG_ANDROID)		+= android/
 obj-$(CONFIG_NVMEM)		+= nvmem/
 obj-$(CONFIG_FPGA)		+= fpga/
+obj-$(CONFIG_FSI)		+= fsi/
diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
new file mode 100644
index 0000000..04c1a0e
--- /dev/null
+++ b/drivers/fsi/Kconfig
@@ -0,0 +1,12 @@
+#
+# FSI subsystem
+#
+
+menu "FSI support"
+
+config FSI
+	tristate "FSI support"
+	---help---
+	  FSI - the FRU Support Interface - is a simple bus for low-level
+	  access to POWER-based hardware.
+endmenu
diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
new file mode 100644
index 0000000..db0e5e7
--- /dev/null
+++ b/drivers/fsi/Makefile
@@ -0,0 +1,2 @@
+
+obj-$(CONFIG_FSI) += fsi-core.o
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
new file mode 100644
index 0000000..3e45306
--- /dev/null
+++ b/drivers/fsi/fsi-core.c
@@ -0,0 +1,38 @@
+/*
+ * FSI core driver
+ *
+ * Copyright (C) IBM Corporation 2016
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/fsi.h>
+#include <linux/module.h>
+
+/* FSI core & Linux bus type definitions */
+
+struct bus_type fsi_bus_type = {
+	.name		= "fsi",
+};
+EXPORT_SYMBOL_GPL(fsi_bus_type);
+
+static int fsi_init(void)
+{
+	return bus_register(&fsi_bus_type);
+}
+
+static void fsi_exit(void)
+{
+	bus_unregister(&fsi_bus_type);
+}
+
+module_init(fsi_init);
+module_exit(fsi_exit);
diff --git a/include/linux/fsi.h b/include/linux/fsi.h
new file mode 100644
index 0000000..8e8bdea
--- /dev/null
+++ b/include/linux/fsi.h
@@ -0,0 +1,23 @@
+/*
+ * FSI device & driver interfaces
+ *
+ * Copyright (C) IBM Corporation 2016
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef LINUX_FSI_H
+#define LINUX_FSI_H
+
+#include <linux/device.h>
+
+extern struct bus_type fsi_bus_type;
+
+#endif /* LINUX_FSI_H */
-- 
1.8.2.2

[PATCH linux 02/15] fsi: Add device & driver definitions

[christopher.lee.bostic]

From: Jeremy Kerr <jk@ozlabs.org>

Add structs for fsi devices & drivers, and struct device conversion
functions.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 include/linux/fsi.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/fsi.h b/include/linux/fsi.h
index 8e8bdea..66dcf25 100644
--- a/include/linux/fsi.h
+++ b/include/linux/fsi.h
@@ -18,6 +18,17 @@
 
 #include <linux/device.h>
 
+struct fsi_device {
+	struct device dev;
+};
+
+struct fsi_driver {
+	struct device_driver drv;
+};
+
+#define to_fsi_dev(devp) container_of(devp, struct fsi_device, dev)
+#define to_fsi_drv(drvp) container_of(drvp, struct fsi_driver, drv)
+
 extern struct bus_type fsi_bus_type;
 
 #endif /* LINUX_FSI_H */
-- 
1.8.2.2

[PATCH linux 03/15] fsi: add driver to device matches

[christopher.lee.bostic]

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>
---
 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 66dcf25..6d843d4 100644
--- a/include/linux/fsi.h
+++ b/include/linux/fsi.h
@@ -19,11 +19,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

[PATCH linux 04/15] fsi: Add fsi master definition

[christopher.lee.bostic]

From: Jeremy Kerr <jk@ozlabs.org>

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 drivers/fsi/fsi-core.c   | 20 ++++++++++++++++++++
 drivers/fsi/fsi-master.h | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)
 create mode 100644 drivers/fsi/fsi-master.h

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 3d55bd5..ce9428d 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -17,6 +17,26 @@
 #include <linux/fsi.h>
 #include <linux/module.h>
 
+#include "fsi-master.h"
+
+static atomic_t master_idx = ATOMIC_INIT(-1);
+
+/* FSI master support */
+
+int fsi_master_register(struct fsi_master *master)
+{
+	master->idx = atomic_inc_return(&master_idx);
+	get_device(master->dev);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fsi_master_register);
+
+void fsi_master_unregister(struct fsi_master *master)
+{
+	put_device(master->dev);
+}
+EXPORT_SYMBOL_GPL(fsi_master_unregister);
+
 /* FSI core & Linux bus type definitions */
 
 static int fsi_bus_match(struct device *dev, struct device_driver *drv)
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
new file mode 100644
index 0000000..e75a810
--- /dev/null
+++ b/drivers/fsi/fsi-master.h
@@ -0,0 +1,37 @@
+/*
+ * FSI master definitions. These comprise the core <--> master interface,
+ * to allow the core to interact with the (hardware-specific) masters.
+ *
+ * Copyright (C) IBM Corporation 2016
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef DRIVERS_FSI_MASTER_H
+#define DRIVERS_FSI_MASTER_H
+
+#include <linux/device.h>
+
+struct fsi_master {
+	struct device	*dev;
+	int		idx;
+	int		n_links;
+	int		(*read)(struct fsi_master *, int link,
+				uint8_t slave, uint32_t addr,
+				void *val, size_t size);
+	int		(*write)(struct fsi_master *, int link,
+				uint8_t slave, uint32_t addr,
+				const void *val, size_t size);
+};
+
+extern int fsi_master_register(struct fsi_master *master);
+extern void fsi_master_unregister(struct fsi_master *master);
+
+#endif /* DRIVERS_FSI_MASTER_H */
-- 
1.8.2.2

[PATCH linux 05/15] fsi: Add fake master driver

[christopher.lee.bostic]

From: Jeremy Kerr <jk@ozlabs.org>

For debugging, add a fake master driver, that only supports reads,
returning a fixed set of data.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 drivers/fsi/Kconfig           | 10 +++++
 drivers/fsi/Makefile          |  1 +
 drivers/fsi/fsi-master-fake.c | 96 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+)
 create mode 100644 drivers/fsi/fsi-master-fake.c

diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
index 04c1a0e..f065dbe 100644
--- a/drivers/fsi/Kconfig
+++ b/drivers/fsi/Kconfig
@@ -9,4 +9,14 @@ config FSI
 	---help---
 	  FSI - the FRU Support Interface - is a simple bus for low-level
 	  access to POWER-based hardware.
+
+if FSI
+
+config FSI_MASTER_FAKE
+	tristate "Fake FSI master"
+	depends on FSI
+	---help---
+	This option enables a fake FSI master driver for debugging.
+endif
+
 endmenu
diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
index db0e5e7..847c00c 100644
--- a/drivers/fsi/Makefile
+++ b/drivers/fsi/Makefile
@@ -1,2 +1,3 @@
 
 obj-$(CONFIG_FSI) += fsi-core.o
+obj-$(CONFIG_FSI_MASTER_FAKE) += fsi-master-fake.o
diff --git a/drivers/fsi/fsi-master-fake.c b/drivers/fsi/fsi-master-fake.c
new file mode 100644
index 0000000..50aac0b
--- /dev/null
+++ b/drivers/fsi/fsi-master-fake.c
@@ -0,0 +1,96 @@
+/*
+ * Fake FSI master driver for FSI development
+ *
+ * Copyright (C) IBM Corporation 2016
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+
+#include "fsi-master.h"
+
+const uint8_t data[] = {
+	0xc0, 0x02, 0x08, 0x03,	/* chip id */
+	0x80, 0x01, 0x11, 0x00,	/* peek */
+	0x80, 0x01, 0x20, 0x3e,	/* slave */
+	0x00, 0x01, 0x10, 0xa5,	/* i2c */
+};
+
+
+static int fsi_master_fake_read(struct fsi_master *_master, int link,
+		uint8_t slave, uint32_t addr, void *val, size_t size)
+{
+	if (link != 0)
+		return -ENODEV;
+
+	if (addr + size > sizeof(data))
+		memset(val, 0, size);
+	else
+		memcpy(val, data + addr, size);
+
+	return 0;
+}
+
+static int fsi_master_fake_write(struct fsi_master *_master, int link,
+		uint8_t slave, uint32_t addr, const void *val, size_t size)
+{
+	if (link != 0)
+		return -ENODEV;
+
+	return -EACCES;
+}
+
+static int fsi_master_fake_probe(struct platform_device *pdev)
+{
+	struct fsi_master *master;
+
+	master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
+	if (!master)
+		return -ENOMEM;
+
+	master->dev = &pdev->dev;
+	master->n_links = 1;
+	master->read = fsi_master_fake_read;
+	master->write = fsi_master_fake_write;
+
+	return fsi_master_register(master);
+}
+
+static const struct of_device_id fsi_master_fake_match[] = {
+	{ .compatible = "ibm,fsi-master-fake" },
+	{ },
+};
+
+static struct platform_driver fsi_master_fake_driver = {
+	.driver = {
+		.name		= "fsi-master-fake",
+		.of_match_table	= fsi_master_fake_match,
+	},
+	.probe	= fsi_master_fake_probe,
+};
+
+static int __init fsi_master_fake_init(void)
+{
+	struct device_node *np;
+
+	platform_driver_register(&fsi_master_fake_driver);
+
+	for_each_compatible_node(np, NULL, "ibm,fsi-master-fake")
+		of_platform_device_create(np, NULL, NULL);
+
+	return 0;
+}
+
+module_init(fsi_master_fake_init);
+
-- 
1.8.2.2

[PATCH linux 06/15] fsi: enable debug

[christopher.lee.bostic]

From: Jeremy Kerr <jk@ozlabs.org>

Enable debug for the fsi core during development. Remove before
submission.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 drivers/fsi/fsi-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index ce9428d..db1a1ce 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -13,6 +13,8 @@
  * GNU General Public License for more details.
  */
 
+#define DEBUG
+
 #include <linux/device.h>
 #include <linux/fsi.h>
 #include <linux/module.h>
-- 
1.8.2.2

[PATCH linux 07/15] fsi: Add slave defintion

[christopher.lee.bostic]

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>
---
 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 db1a1ce..7df4291 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -23,6 +23,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

[PATCH linux 08/15] fsi: Add empty master scan

[christopher.lee.bostic]

From: Jeremy Kerr <jk@ozlabs.org>

When a new fsi master is added, we will need to scan its links, and
slaves attached to those links. This change introduces a little shell to
iterate the links, which we will populate with the actual slave scan in
a later change.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 drivers/fsi/fsi-core.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 7df4291..9744a55 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -21,6 +21,8 @@
 
 #include "fsi-master.h"
 
+#define FSI_N_SLAVES	4
+
 static atomic_t master_idx = ATOMIC_INIT(-1);
 
 struct fsi_slave {
@@ -32,12 +34,34 @@ struct fsi_slave {
 
 #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+/* FSI slave support */
+static int fsi_slave_init(struct fsi_master *master,
+		int link, uint8_t slave_id)
+{
+	/* todo: initialise slave device, perform engine scan */
+
+	return -ENODEV;
+}
+
 /* FSI master support */
 
+static int fsi_master_scan(struct fsi_master *master)
+{
+	int link, slave_id;
+
+	for (link = 0; link < master->n_links; link++)
+		for (slave_id = 0; slave_id < FSI_N_SLAVES; slave_id++)
+			fsi_slave_init(master, link, slave_id);
+
+	return 0;
+
+}
+
 int fsi_master_register(struct fsi_master *master)
 {
 	master->idx = atomic_inc_return(&master_idx);
 	get_device(master->dev);
+	fsi_master_scan(master);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(fsi_master_register);
-- 
1.8.2.2

[PATCH linux 09/15] fsi: Add crc4 helpers

[christopher.lee.bostic]

From: Jeremy Kerr <jk@ozlabs.org>

Add some helpers for the crc checks for the slave configuration table.
This works 4-bits-at-a-time, using a simple table approach.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 drivers/fsi/fsi-core.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 9744a55..24538ae 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -34,6 +34,29 @@ struct fsi_slave {
 
 #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+/* crc helpers */
+static const uint8_t crc4_tab[] = {
+	0x0, 0x7, 0xe, 0x9, 0xb, 0xc, 0x5, 0x2,
+	0x1, 0x6, 0xf, 0x8, 0xa, 0xd, 0x4, 0x3,
+};
+
+static uint8_t crc4(uint32_t x)
+{
+	uint8_t c = 0;
+	int i;
+
+	/* Calculate crc4 over four-bit nibbles, starting at the MSbit */
+	for (i = 28; i >= 0; i -= 4)
+		c = crc4_tab[c ^ ((x >> i) & 0xf)];
+
+	return c;
+}
+
+static bool check_crc4(uint32_t x)
+{
+	return crc4(x) == 0;
+}
+
 /* FSI slave support */
 static int fsi_slave_init(struct fsi_master *master,
 		int link, uint8_t slave_id)
-- 
1.8.2.2

[PATCH linux 10/15] fsi: Implement slave initialisation

[christopher.lee.bostic]

From: Jeremy Kerr <jk@ozlabs.org>

Create fsi_slave devices during the master scan.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 drivers/fsi/fsi-core.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 24538ae..49a8e14 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -18,6 +18,7 @@
 #include <linux/device.h>
 #include <linux/fsi.h>
 #include <linux/module.h>
+#include <linux/slab.h>
 
 #include "fsi-master.h"
 
@@ -58,12 +59,57 @@ static bool check_crc4(uint32_t x)
 }
 
 /* FSI slave support */
+
+static void fsi_slave_release(struct device *dev)
+{
+	struct fsi_slave *slave = to_fsi_slave(dev);
+	kfree(slave);
+}
+
 static int fsi_slave_init(struct fsi_master *master,
 		int link, uint8_t slave_id)
 {
-	/* todo: initialise slave device, perform engine scan */
+	struct fsi_slave *slave;
+	uint32_t chip_id;
+	int rc;
+
+	rc = master->read(master, link, slave_id, 0, &chip_id, sizeof(chip_id));
+	if (rc) {
+		dev_warn(master->dev, "can't read slave %02x:%02x: %d\n",
+				link, slave_id, rc);
+		return -ENODEV;
+	}
+
+	chip_id = be32_to_cpu(chip_id);
+	if (!check_crc4(chip_id)) {
+		dev_warn(master->dev, "slave %02x:%02x: invalid chip id CRC!\n",
+				link, slave_id);
+		return -EIO;
+	}
+
+	pr_debug("fsi: found chip %08x at %02x:%02x:%02x\n",
+			master->idx, chip_id, link, slave_id);
+
+	/* we can communicate with a slave; create devices and scan */
+	slave = kzalloc(sizeof(*slave), GFP_KERNEL);
+	if (!slave)
+		return -ENOMEM;
+
+	slave->master = master;
+	slave->id = slave_id;
+	slave->dev.parent = master->dev;
+	slave->dev.release = fsi_slave_release;
+
+	dev_set_name(&slave->dev, "slave@%02x:%02x", link, slave_id);
+	rc = device_register(&slave->dev);
+	if (rc < 0) {
+		dev_warn(master->dev, "failed to create slave device: %d\n",
+				rc);
+		put_device(&slave->dev);
+		return rc;
+	}
 
-	return -ENODEV;
+	return rc;
 }
 
 /* FSI master support */
-- 
1.8.2.2

[PATCH linux 11/15] fsi: scan slaves & register devices

[christopher.lee.bostic]

From: Jeremy Kerr <jk@ozlabs.org>

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

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 drivers/fsi/fsi-core.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/fsi.h    |   4 ++
 2 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 49a8e14..63d26b5 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -24,6 +24,16 @@
 
 #define FSI_N_SLAVES	4
 
+#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
+
+static const int engine_page_size = 0x400;
+
 static atomic_t master_idx = ATOMIC_INIT(-1);
 
 struct fsi_slave {
@@ -35,6 +45,29 @@ 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 +92,95 @@ static bool check_crc4(uint32_t x)
 }
 
 /* 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;
+		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;
+		}
+
+		conf = be32_to_cpu(conf);
+
+		if (!check_crc4(conf)) {
+			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->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)
 {
@@ -109,7 +231,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 6d843d4..dfd3513 100644
--- a/include/linux/fsi.h
+++ b/include/linux/fsi.h
@@ -22,6 +22,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

[PATCH linux 12/15] fsi: Add device read/write/peek functions

[christopher.lee.bostic]

From: Jeremy Kerr <jk@ozlabs.org>

This change introduces the fsi device API: simple read, write and peek
accessors for the devices' address spaces.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 drivers/fsi/fsi-core.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fsi.h    |  7 ++++++-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 63d26b5..10bf817 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -32,6 +32,8 @@
 #define FSI_SLAVE_CONF_TYPE_MASK	0x00000ff0
 #define FSI_SLAVE_CONF_TYPE_SHIFT	4
 
+#define FSI_PEEK_BASE			0x410
+
 static const int engine_page_size = 0x400;
 
 static atomic_t master_idx = ATOMIC_INIT(-1);
@@ -45,7 +47,42 @@ struct fsi_slave {
 
 #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
+		void *val, size_t size);
+static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
+		const void *val, size_t size);
+
 /* FSI endpoint-device support */
+int fsi_device_read(struct fsi_device *dev, uint32_t addr, void *val,
+		size_t size)
+{
+	if (addr > dev->size)
+		return -EINVAL;
+
+	if (addr + size > dev->size)
+		return -EINVAL;
+
+	return fsi_slave_read(dev->slave, dev->addr + addr, val, size);
+}
+
+int fsi_device_write(struct fsi_device *dev, uint32_t addr, const void *val,
+		size_t size)
+{
+	if (addr > dev->size)
+		return -EINVAL;
+
+	if (addr + size > dev->size)
+		return -EINVAL;
+
+	return fsi_slave_write(dev->slave, dev->addr + addr, val, size);
+}
+
+int fsi_device_peek(struct fsi_device *dev, void *val)
+{
+	uint32_t addr = FSI_PEEK_BASE + ((dev->unit - 2) * sizeof(uint32_t));
+
+	return fsi_slave_read(dev->slave, addr, val, sizeof(uint32_t));
+}
 
 static void fsi_device_release(struct device *_device)
 {
@@ -99,6 +136,13 @@ static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
 			slave->id, addr, val, size);
 }
 
+static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
+		const void *val, size_t size)
+{
+	return slave->master->write(slave->master, slave->link,
+			slave->id, addr, val, size);
+}
+
 static int fsi_slave_scan(struct fsi_slave *slave)
 {
 	uint32_t engine_addr;
diff --git a/include/linux/fsi.h b/include/linux/fsi.h
index dfd3513..47af0af 100644
--- a/include/linux/fsi.h
+++ b/include/linux/fsi.h
@@ -28,6 +28,12 @@ struct fsi_device {
 	uint32_t		size;
 };
 
+extern int fsi_device_read(struct fsi_device *dev, uint32_t addr,
+		void *val, size_t size);
+extern int fsi_device_write(struct fsi_device *dev, uint32_t addr,
+		const void *val, size_t size);
+extern int fsi_device_peek(struct fsi_device *dev, void *val);
+
 struct fsi_device_id {
 	u8	engine_type;
 	u8	version;
@@ -41,7 +47,6 @@ struct fsi_device_id {
 #define FSI_DEVICE_VERSIONED(t, v) \
 	.engine_type = (t), .version = (v),
 
-
 struct fsi_driver {
 	struct device_driver		drv;
 	const struct fsi_device_id	*id_table;
-- 
1.8.2.2

[PATCH linux 13/15] fsi: Add GPIO master driver

[christopher.lee.bostic]

From: Jeremy Kerr <jk@ozlabs.org>

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 .../devicetree/bindings/fsi/fsi-master-gpio.txt    |  8 ++
 drivers/fsi/Kconfig                                |  7 ++
 drivers/fsi/Makefile                               |  1 +
 drivers/fsi/fsi-master-gpio.c                      | 92 ++++++++++++++++++++++
 4 files changed, 108 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
 create mode 100644 drivers/fsi/fsi-master-gpio.c

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..44762a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
@@ -0,0 +1,8 @@
+Device-tree bindings for gpio-based FSI master driver
+-----------------------------------------------------
+
+fsi-master {
+	compatible = "ibm,fsi-master", "ibm,fsi-master-gpio";
+	clk-gpio = <&gpio 0>;
+	data-gpio = <&gpio 1>;
+}
diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
index f065dbe..69e7ee8 100644
--- a/drivers/fsi/Kconfig
+++ b/drivers/fsi/Kconfig
@@ -17,6 +17,13 @@ config FSI_MASTER_FAKE
 	depends on FSI
 	---help---
 	This option enables a fake FSI master driver for debugging.
+
+config FSI_MASTER_GPIO
+	tristate "GPIO-based FSI master"
+	depends on FSI
+	select GPIO_DEVRES
+	---help---
+	This option enables a FSI master driver, using GPIO lines directly.
 endif
 
 endmenu
diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
index 847c00c..2021ce5 100644
--- a/drivers/fsi/Makefile
+++ b/drivers/fsi/Makefile
@@ -1,3 +1,4 @@
 
 obj-$(CONFIG_FSI) += fsi-core.o
 obj-$(CONFIG_FSI_MASTER_FAKE) += fsi-master-fake.o
+obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
new file mode 100644
index 0000000..3855829
--- /dev/null
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -0,0 +1,92 @@
+/*
+ * A FSI master controller, using a simple GPIO bit-banging interface
+ */
+
+#include <linux/platform_device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+
+#include "fsi-master.h"
+
+struct fsi_master_gpio {
+	struct fsi_master	master;
+	struct gpio_desc	*gpio_clk;
+	struct gpio_desc	*gpio_data;
+};
+
+#define to_fsi_master_gpio(m) container_of(m, struct fsi_master_gpio, master)
+
+struct fsi_gpio_msg {
+	uint64_t	msg;
+	uint8_t		bits;
+};
+
+static int fsi_master_gpio_read(struct fsi_master *_master, int link,
+		uint8_t slave, uint32_t addr, void *val, size_t size)
+{
+	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+
+	if (link != 0)
+		return -ENODEV;
+
+	/* todo: format read into a message, send, poll for response */
+	(void)master;
+
+
+	return 0;
+}
+
+static int fsi_master_gpio_write(struct fsi_master *_master, int link,
+		uint8_t slave, uint32_t addr, const void *val, size_t size)
+{
+	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+
+	if (link != 0)
+		return -ENODEV;
+
+	/* todo: format write into a message, send, poll for response */
+	(void)master;
+
+	return 0;
+}
+
+static int fsi_master_gpio_probe(struct platform_device *pdev)
+{
+	struct fsi_master_gpio *master;
+	struct gpio_desc *gpio;
+
+	master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
+	if (!master)
+		return -ENOMEM;
+
+	gpio = devm_gpiod_get(&pdev->dev, "clk", 0);
+	if (IS_ERR(gpio))
+		return PTR_ERR(gpio);
+	master->gpio_clk = gpio;
+
+	gpio = devm_gpiod_get(&pdev->dev, "data", 0);
+	if (IS_ERR(gpio))
+		return PTR_ERR(gpio);
+	master->gpio_data = gpio;
+
+	master->master.read = fsi_master_gpio_read;
+	master->master.write = fsi_master_gpio_write;
+
+	return fsi_master_register(&master->master);
+}
+
+static const struct of_device_id fsi_master_gpio_match[] = {
+	{ .compatible = "ibm,fsi-master-gpio" },
+	{ },
+};
+
+static struct platform_driver fsi_master_gpio_driver = {
+	.driver = {
+		.name		= "fsi-master-gpio",
+		.of_match_table	= fsi_master_gpio_match,
+	},
+	.probe	= fsi_master_gpio_probe,
+};
+
+module_platform_driver(fsi_master_gpio_driver);
+
-- 
1.8.2.2

[PATCH linux 14/15] drivers/fsi: Rename slave to cfam

[christopher.lee.bostic]

From: Chris Bostic <cbostic@us.ibm.com>

Suggested changes to the patch set so far as provided by Jeremy Kerr.
Rename slave to cfam to distinguish between the slave engine and its
containing cfam.  A few formatting changes to make checkpatch accept
patches 0001-0013.

Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 drivers/fsi/fsi-cfam.h        |  71 +++++++++++++++++++++
 drivers/fsi/fsi-core.c        | 144 ++++++++++++++++++------------------------
 drivers/fsi/fsi-master-fake.c |   4 +-
 drivers/fsi/fsi-master-gpio.c |   4 +-
 drivers/fsi/fsi-master.h      |   4 +-
 include/linux/fsi.h           |   2 +-
 6 files changed, 140 insertions(+), 89 deletions(-)
 create mode 100644 drivers/fsi/fsi-cfam.h

diff --git a/drivers/fsi/fsi-cfam.h b/drivers/fsi/fsi-cfam.h
new file mode 100644
index 0000000..a542f60
--- /dev/null
+++ b/drivers/fsi/fsi-cfam.h
@@ -0,0 +1,71 @@
+/*
+ * FSI CFAM definitions
+ *
+ * Copyright (C) IBM Corporation 2016
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef DRIVERS_FSI_CFAM_H
+#define DRIVERS_FSI_CFAM_H
+
+#include <linux/device.h>
+
+#include "fsi-master.h"
+
+#define FSI_SLAVE_ENG_ID	2
+
+#define FSI_MAX_CFAMS_PER_LINK	4
+#define FSI_CFAM_SIZE		(FSI_LINK_SIZE / FSI_MAX_CFAMS_PER_LINK)
+#define FSI_PEEK_BASE		0x410
+
+/* Config space decoding */
+#define FSI_CFG_NEXT_MASK	0x80000000
+#define FSI_CFG_SLOT_MASK	0x00ff0000
+#define FSI_CFG_SLOT_SHIFT	16
+#define FSI_CFG_VERS_MASK	0x0000f000
+#define FSI_CFG_VERS_SHIFT	12
+#define FSI_CFG_TYPE_MASK	0x00000ff0
+#define FSI_CFG_TYPE_SHIFT	4
+
+/*
+ * Return number of slots in the configuration word
+ */
+static inline uint8_t fsi_cfg_slot(uint32_t x)
+{
+	return (x & FSI_CFG_SLOT_MASK) >> FSI_CFG_SLOT_SHIFT;
+}
+
+/*
+ * Return version in the configuration word
+ */
+static inline uint8_t fsi_cfg_version(uint32_t x)
+{
+	return (x & FSI_CFG_VERS_MASK) >> FSI_CFG_VERS_SHIFT;
+}
+
+/*
+ * Return type field in the configuration word
+ */
+static inline uint8_t fsi_cfg_type(uint32_t x)
+{
+	return (x & FSI_CFG_TYPE_MASK) >> FSI_CFG_TYPE_SHIFT;
+}
+
+struct fsi_cfam {
+	struct device		dev;
+	struct fsi_master	*master;
+	uint8_t			link;
+	uint8_t			id;
+};
+
+#define to_fsi_cfam(d) container_of(d, struct fsi_cfam, dev)
+
+#endif /* DRIVERS_FSI_MASTER_H */
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 10bf817..a7f23a5 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -21,16 +21,7 @@
 #include <linux/slab.h>
 
 #include "fsi-master.h"
-
-#define FSI_N_SLAVES	4
-
-#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
+#include "fsi-cfam.h"
 
 #define FSI_PEEK_BASE			0x410
 
@@ -38,18 +29,9 @@ static const int engine_page_size = 0x400;
 
 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)
-
-static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
+static int fsi_cfam_read(struct fsi_cfam *cfam, uint32_t addr,
 		void *val, size_t size);
-static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
+static int fsi_cfam_write(struct fsi_cfam *cfam, uint32_t addr,
 		const void *val, size_t size);
 
 /* FSI endpoint-device support */
@@ -62,7 +44,7 @@ int fsi_device_read(struct fsi_device *dev, uint32_t addr, void *val,
 	if (addr + size > dev->size)
 		return -EINVAL;
 
-	return fsi_slave_read(dev->slave, dev->addr + addr, val, size);
+	return fsi_cfam_read(dev->cfam, dev->addr + addr, val, size);
 }
 
 int fsi_device_write(struct fsi_device *dev, uint32_t addr, const void *val,
@@ -74,14 +56,14 @@ int fsi_device_write(struct fsi_device *dev, uint32_t addr, const void *val,
 	if (addr + size > dev->size)
 		return -EINVAL;
 
-	return fsi_slave_write(dev->slave, dev->addr + addr, val, size);
+	return fsi_cfam_write(dev->cfam, dev->addr + addr, val, size);
 }
 
 int fsi_device_peek(struct fsi_device *dev, void *val)
 {
 	uint32_t addr = FSI_PEEK_BASE + ((dev->unit - 2) * sizeof(uint32_t));
 
-	return fsi_slave_read(dev->slave, addr, val, sizeof(uint32_t));
+	return fsi_cfam_read(dev->cfam, addr, val, sizeof(uint32_t));
 }
 
 static void fsi_device_release(struct device *_device)
@@ -90,7 +72,7 @@ static void fsi_device_release(struct device *_device)
 	kfree(device);
 }
 
-static struct fsi_device *fsi_create_device(struct fsi_slave *slave)
+static struct fsi_device *fsi_create_device(struct fsi_cfam *cfam)
 {
 	struct fsi_device *dev;
 
@@ -98,7 +80,7 @@ static struct fsi_device *fsi_create_device(struct fsi_slave *slave)
 	if (!dev)
 		return NULL;
 
-	dev->dev.parent = &slave->dev;
+	dev->dev.parent = &cfam->dev;
 	dev->dev.bus = &fsi_bus_type;
 	dev->dev.release = fsi_device_release;
 
@@ -128,22 +110,22 @@ static bool check_crc4(uint32_t x)
 	return crc4(x) == 0;
 }
 
-/* FSI slave support */
-static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
+/* FSI CFAM support */
+static int fsi_cfam_read(struct fsi_cfam *cfam, uint32_t addr,
 		void *val, size_t size)
 {
-	return slave->master->read(slave->master, slave->link,
-			slave->id, addr, val, size);
+	return cfam->master->read(cfam->master, cfam->link,
+			cfam->id, addr, val, size);
 }
 
-static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
+static int fsi_cfam_write(struct fsi_cfam *cfam, uint32_t addr,
 		const void *val, size_t size)
 {
-	return slave->master->write(slave->master, slave->link,
-			slave->id, addr, val, size);
+	return cfam->master->write(cfam->master, cfam->link,
+			cfam->id, addr, val, size);
 }
 
-static int fsi_slave_scan(struct fsi_slave *slave)
+static int fsi_cfam_scan(struct fsi_cfam *cfam)
 {
 	uint32_t engine_addr;
 	uint32_t conf;
@@ -157,40 +139,37 @@ static int fsi_slave_scan(struct fsi_slave *slave)
 	 * 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++) {
+	for (i = FSI_SLAVE_ENG_ID; i < engine_page_size / sizeof(uint32_t);
+							i++) {
 		uint8_t slots, version, type;
 		struct fsi_device *dev;
 
-		rc = fsi_slave_read(slave, (i + 1) * sizeof(conf),
+		rc = fsi_cfam_read(cfam, (i + 1) * sizeof(conf),
 				&conf, sizeof(conf));
 		if (rc) {
-			dev_warn(&slave->dev,
-					"error reading slave registers\n");
+			dev_warn(&cfam->dev,
+					"error reading cfam registers\n");
 			return -1;
 		}
 
 		conf = be32_to_cpu(conf);
 
 		if (!check_crc4(conf)) {
-			dev_warn(&slave->dev,
-				"crc error in slave register at 0x%04x\n",
+			dev_warn(&cfam->dev,
+				"crc error in cfam register at 0x%04x\n",
 					i);
 			return -1;
 		}
+		slots = fsi_cfg_slot(conf);
+		version = fsi_cfg_version(conf);
+		type = fsi_cfg_type(conf);
 
-		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 */
+		/* 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);
+			dev = fsi_create_device(cfam);
 			if (!dev)
 				return -ENOMEM;
 
@@ -200,82 +179,83 @@ static int fsi_slave_scan(struct fsi_slave *slave)
 			dev->addr = engine_addr;
 			dev->size = slots * engine_page_size;
 
-			dev_info(&slave->dev,
+			dev_info(&cfam->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);
+					cfam->master->idx, cfam->link,
+					cfam->id, i - 2);
 
 			rc = device_add(&dev->dev);
 			if (rc) {
-				dev_warn(&slave->dev, "add failed: %d\n", rc);
+				dev_warn(&cfam->dev, "add failed: %d\n", rc);
 				put_device(&dev->dev);
 			}
 		}
 
 		engine_addr += slots * engine_page_size;
 
-		if (!(conf & FSI_SLAVE_CONF_NEXT_MASK))
+		if (!(conf & FSI_CFG_NEXT_MASK))
 			break;
 	}
 
 	return 0;
 }
 
-static void fsi_slave_release(struct device *dev)
+static void fsi_cfam_release(struct device *dev)
 {
-	struct fsi_slave *slave = to_fsi_slave(dev);
-	kfree(slave);
+	struct fsi_cfam *cfam = to_fsi_cfam(dev);
+
+	kfree(cfam);
 }
 
-static int fsi_slave_init(struct fsi_master *master,
-		int link, uint8_t slave_id)
+static int fsi_cfam_init(struct fsi_master *master,
+		int link, uint8_t cfam_id)
 {
-	struct fsi_slave *slave;
+	struct fsi_cfam *cfam = NULL;
 	uint32_t chip_id;
 	int rc;
 
-	rc = master->read(master, link, slave_id, 0, &chip_id, sizeof(chip_id));
+	rc = master->read(master, link, cfam_id, 0, &chip_id, sizeof(chip_id));
 	if (rc) {
-		dev_warn(master->dev, "can't read slave %02x:%02x: %d\n",
-				link, slave_id, rc);
+		dev_warn(master->dev, "can't read cfam %02x:%02x: %d\n",
+				link, cfam_id, rc);
 		return -ENODEV;
 	}
 
 	chip_id = be32_to_cpu(chip_id);
 	if (!check_crc4(chip_id)) {
-		dev_warn(master->dev, "slave %02x:%02x: invalid chip id CRC!\n",
-				link, slave_id);
+		dev_warn(master->dev, "cfam %02x:%02x: invalid chip id CRC!\n",
+				link, cfam_id);
 		return -EIO;
 	}
 
 	pr_debug("fsi: found chip %08x at %02x:%02x:%02x\n",
-			master->idx, chip_id, link, slave_id);
+			master->idx, chip_id, link, cfam_id);
 
-	/* we can communicate with a slave; create devices and scan */
-	slave = kzalloc(sizeof(*slave), GFP_KERNEL);
-	if (!slave)
+	/* we can communicate with a cfam; create devices and scan */
+	cfam = kzalloc(sizeof(*cfam), GFP_KERNEL);
+	if (!cfam)
 		return -ENOMEM;
 
-	slave->master = master;
-	slave->id = slave_id;
-	slave->dev.parent = master->dev;
-	slave->dev.release = fsi_slave_release;
+	cfam->master = master;
+	cfam->id = cfam_id;
+	cfam->dev.parent = master->dev;
+	cfam->dev.release = fsi_cfam_release;
 
-	dev_set_name(&slave->dev, "slave@%02x:%02x", link, slave_id);
-	rc = device_register(&slave->dev);
+	dev_set_name(&cfam->dev, "cfam@%02x:%02x", link, cfam_id);
+	rc = device_register(&cfam->dev);
 	if (rc < 0) {
-		dev_warn(master->dev, "failed to create slave device: %d\n",
+		dev_warn(master->dev, "failed to create cfam device: %d\n",
 				rc);
-		put_device(&slave->dev);
+		put_device(&cfam->dev);
 		return rc;
 	}
 
-	fsi_slave_scan(slave);
+	fsi_cfam_scan(cfam);
 	return 0;
 }
 
@@ -283,11 +263,11 @@ static int fsi_slave_init(struct fsi_master *master,
 
 static int fsi_master_scan(struct fsi_master *master)
 {
-	int link, slave_id;
+	int link, cfam_id;
 
 	for (link = 0; link < master->n_links; link++)
-		for (slave_id = 0; slave_id < FSI_N_SLAVES; slave_id++)
-			fsi_slave_init(master, link, slave_id);
+		for (cfam_id = 0; cfam_id < FSI_MAX_CFAMS_PER_LINK; cfam_id++)
+			fsi_cfam_init(master, link, cfam_id);
 
 	return 0;
 
diff --git a/drivers/fsi/fsi-master-fake.c b/drivers/fsi/fsi-master-fake.c
index 50aac0b..658a5f6 100644
--- a/drivers/fsi/fsi-master-fake.c
+++ b/drivers/fsi/fsi-master-fake.c
@@ -29,7 +29,7 @@ const uint8_t data[] = {
 
 
 static int fsi_master_fake_read(struct fsi_master *_master, int link,
-		uint8_t slave, uint32_t addr, void *val, size_t size)
+		uint8_t cfam, uint32_t addr, void *val, size_t size)
 {
 	if (link != 0)
 		return -ENODEV;
@@ -43,7 +43,7 @@ static int fsi_master_fake_read(struct fsi_master *_master, int link,
 }
 
 static int fsi_master_fake_write(struct fsi_master *_master, int link,
-		uint8_t slave, uint32_t addr, const void *val, size_t size)
+		uint8_t cfam, uint32_t addr, const void *val, size_t size)
 {
 	if (link != 0)
 		return -ENODEV;
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index 3855829..3e1aeb6 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -22,7 +22,7 @@ struct fsi_gpio_msg {
 };
 
 static int fsi_master_gpio_read(struct fsi_master *_master, int link,
-		uint8_t slave, uint32_t addr, void *val, size_t size)
+		uint8_t cfam, uint32_t addr, void *val, size_t size)
 {
 	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
 
@@ -37,7 +37,7 @@ static int fsi_master_gpio_read(struct fsi_master *_master, int link,
 }
 
 static int fsi_master_gpio_write(struct fsi_master *_master, int link,
-		uint8_t slave, uint32_t addr, const void *val, size_t size)
+		uint8_t cfam, uint32_t addr, const void *val, size_t size)
 {
 	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
 
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index e75a810..4cdc4b4 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -24,10 +24,10 @@ struct fsi_master {
 	int		idx;
 	int		n_links;
 	int		(*read)(struct fsi_master *, int link,
-				uint8_t slave, uint32_t addr,
+				uint8_t cfam, uint32_t addr,
 				void *val, size_t size);
 	int		(*write)(struct fsi_master *, int link,
-				uint8_t slave, uint32_t addr,
+				uint8_t cfam, uint32_t addr,
 				const void *val, size_t size);
 };
 
diff --git a/include/linux/fsi.h b/include/linux/fsi.h
index 47af0af..c919058 100644
--- a/include/linux/fsi.h
+++ b/include/linux/fsi.h
@@ -23,7 +23,7 @@ struct fsi_device {
 	u8			engine_type;
 	u8			version;
 	u8			unit;
-	struct fsi_slave	*slave;
+	struct fsi_cfam		*cfam;
 	uint32_t		addr;
 	uint32_t		size;
 };
-- 
1.8.2.2

[PATCH linux 15/15] drivers/fsi: Initialize CFAMs for read/write access

[christopher.lee.bostic]

From: Chris Bostic <cbostic@us.ibm.com>

Send break command to CFAMs to reset their logic.  If break
fails this serves as an indication there is no CFAM present.
Set various slave engine mode register defaults.

Add new fields to struct fsi_master to manage hardware
workarounds for break ids, target addresses and cfam IDs.

Limit each link scan to one CFAM.  Due to limitations in
hardware break commands only one can be recognized per link.

Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 drivers/fsi/fsi-cfam.h        |  1 +
 drivers/fsi/fsi-core.c        | 58 +++++++++++++++++++++++++++++++++++++++-
 drivers/fsi/fsi-master-fake.c |  3 +++
 drivers/fsi/fsi-master-gpio.c |  3 +++
 drivers/fsi/fsi-master.h      |  5 ++++
 drivers/fsi/fsi-slave.h       | 62 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 drivers/fsi/fsi-slave.h

diff --git a/drivers/fsi/fsi-cfam.h b/drivers/fsi/fsi-cfam.h
index a542f60..628c5de 100644
--- a/drivers/fsi/fsi-cfam.h
+++ b/drivers/fsi/fsi-cfam.h
@@ -25,6 +25,7 @@
 #define FSI_MAX_CFAMS_PER_LINK	4
 #define FSI_CFAM_SIZE		(FSI_LINK_SIZE / FSI_MAX_CFAMS_PER_LINK)
 #define FSI_PEEK_BASE		0x410
+#define FSI_SLAVE_BASE		0x800
 
 /* Config space decoding */
 #define FSI_CFG_NEXT_MASK	0x80000000
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index a7f23a5..41c5a78 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -19,9 +19,11 @@
 #include <linux/fsi.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/delay.h>
 
 #include "fsi-master.h"
 #include "fsi-cfam.h"
+#include "fsi-slave.h"
 
 #define FSI_PEEK_BASE			0x410
 
@@ -212,13 +214,67 @@ static void fsi_cfam_release(struct device *dev)
 	kfree(cfam);
 }
 
+/*
+ * Issue a break command to this link
+ */
+static int fsi_master_break(struct fsi_master *master, int link)
+{
+	int rc;
+	uint32_t cmd = FSI_CMD_BREAK;
+
+	rc = master->write(master, link, master->break_id, master->break_offset,
+			   &cmd, sizeof(cmd));
+	if (rc) {
+		dev_warn(master->dev, "break to %02x:%02x failed\n",
+			 link, master->break_id);
+		return -ENODEV;
+	}
+	udelay(200);		/* wait for logic reset to take effect */
+
+	rc = master->read(master, link, master->break_id,
+			  FSI_SLAVE_BASE + FSI_SMODE, &cmd, sizeof(cmd));
+	dev_info(master->dev, "smode after break:%08x rc:%d\n", cmd, rc);
+	if (rc) {
+		dev_warn(master->dev, "read smode after break failed\n");
+		return -ENODEV;
+	}
+
+	return rc;
+}
+
+static void set_smode_defaults(struct fsi_master *master, uint32_t *smode)
+{
+	*smode = FSI_SMODE_WSC | FSI_SMODE_ECRC
+		| fsi_smode_echodly(0xf) | fsi_smode_senddly(0xf)
+		| fsi_smode_lbcrr(1) | fsi_smode_sid(master->cfam_id);
+}
+
 static int fsi_cfam_init(struct fsi_master *master,
 		int link, uint8_t cfam_id)
 {
 	struct fsi_cfam *cfam = NULL;
-	uint32_t chip_id;
+	uint32_t chip_id, smode;
 	int rc;
 
+	/*
+	 * todo: Due to CFAM hardware issues related to BREAK commands we're
+	 * limited to only one CFAM per link.  This can be removed once
+	 * issue is resolved.
+	 */
+	if (cfam_id > 0)
+		return 0;
+
+	rc = fsi_master_break(master, link);
+	if (rc) {
+		dev_warn(master->dev, "no cfam detected at %02x:%02x\n",
+				link, cfam_id);
+		return -ENODEV;
+	}
+
+	set_smode_defaults(master, &smode);
+	rc = master->write(master, link, master->break_id,
+			   FSI_SLAVE_BASE + FSI_SMODE, &smode, sizeof(smode));
+
 	rc = master->read(master, link, cfam_id, 0, &chip_id, sizeof(chip_id));
 	if (rc) {
 		dev_warn(master->dev, "can't read cfam %02x:%02x: %d\n",
diff --git a/drivers/fsi/fsi-master-fake.c b/drivers/fsi/fsi-master-fake.c
index 658a5f6..3663a0a 100644
--- a/drivers/fsi/fsi-master-fake.c
+++ b/drivers/fsi/fsi-master-fake.c
@@ -63,6 +63,9 @@ static int fsi_master_fake_probe(struct platform_device *pdev)
 	master->n_links = 1;
 	master->read = fsi_master_fake_read;
 	master->write = fsi_master_fake_write;
+	master->cfam_id = 0;
+	master->break_id = 0;
+	master->break_offset = 0;
 
 	return fsi_master_register(master);
 }
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index 3e1aeb6..9769960 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -71,6 +71,9 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
 
 	master->master.read = fsi_master_gpio_read;
 	master->master.write = fsi_master_gpio_write;
+	master->master.cfam_id = 0;
+	master->master.break_id = 0;
+	master->master.break_offset = 0;
 
 	return fsi_master_register(&master->master);
 }
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index 4cdc4b4..41a5dc5 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -19,10 +19,15 @@
 
 #include <linux/device.h>
 
+#define	FSI_CMD_BREAK		0xc0de0000	/* Break command */
+
 struct fsi_master {
 	struct device	*dev;
 	int		idx;
 	int		n_links;
+	int		cfam_id;	/* 1 CFAM per link hw workaround */
+	int		break_id;	/* Break to this CFAM ID */
+	uint32_t	break_offset;	/* Where to write break */
 	int		(*read)(struct fsi_master *, int link,
 				uint8_t cfam, uint32_t addr,
 				void *val, size_t size);
diff --git a/drivers/fsi/fsi-slave.h b/drivers/fsi/fsi-slave.h
new file mode 100644
index 0000000..ea7a760
--- /dev/null
+++ b/drivers/fsi/fsi-slave.h
@@ -0,0 +1,62 @@
+/*
+ * FSI slave engine definitions.
+ *
+ * Copyright (C) IBM Corporation 2016
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef DRIVERS_FSI_SLAVE_H
+#define DRIVERS_FSI_SLAVE_H
+
+/*
+ * FSI slave engine control register offsets
+ */
+#define	FSI_SMODE		0x0	/* R/W: Mode register */
+
+/*
+ * SMODE fields
+ */
+#define	FSI_SMODE_WSC		0x80000000	/* Warm start completed */
+#define	FSI_SMODE_ECRC		0x20000000	/* Enable hardware CRC check */
+#define	FSI_SMODE_SID_SHIFT	24		/* ID shift */
+#define	FSI_SMODE_SID_MASK	3		/* ID mask */
+#define	FSI_SMODE_ED_SHIFT	20		/* Echo delay shift */
+#define	FSI_SMODE_ED_MASK	0xf		/* Echo delay mask */
+#define	FSI_SMODE_SD_SHIFT	16		/* Send delay shift */
+#define	FSI_SMODE_SD_MASK	0xf		/* Send delay mask */
+#define	FSI_SMODE_LBCRR_SHIFT	8		/* Clock rate ratio shift */
+#define	FSI_SMODE_LBCRR_MASK	0xf		/* Clock rate ratio mask */
+
+/* Encode slave local bus echo delay */
+static inline uint32_t fsi_smode_echodly(int x)
+{
+	return (x & FSI_SMODE_ED_MASK) << FSI_SMODE_ED_SHIFT;
+}
+
+/* Encode slave local bus send delay */
+static inline uint32_t fsi_smode_senddly(int x)
+{
+	return (x & FSI_SMODE_SD_MASK) << FSI_SMODE_SD_SHIFT;
+}
+
+/* Encode slave local bus clock rate ratio */
+static inline uint32_t fsi_smode_lbcrr(int x)
+{
+	return (x & FSI_SMODE_LBCRR_MASK) << FSI_SMODE_LBCRR_SHIFT;
+}
+
+/* Encode slave ID */
+static inline uint32_t fsi_smode_sid(int x)
+{
+	return (x & FSI_SMODE_SID_MASK) << FSI_SMODE_SID_SHIFT;
+}
+
+#endif /* DRIVERS_FSI_SLAVE_H */
-- 
1.8.2.2

Re: [PATCH linux 15/15] drivers/fsi: Initialize CFAMs for read/write access

[Jeremy Kerr]

Hi Chris,

> Send break command to CFAMs to reset their logic.  If break
> fails this serves as an indication there is no CFAM present.
> Set various slave engine mode register defaults.
> 
> Add new fields to struct fsi_master to manage hardware
> workarounds for break ids, target addresses and cfam IDs.
> 
> Limit each link scan to one CFAM.  Due to limitations in
> hardware break commands only one can be recognized per link.

I'm not sure what the restriction is there. We can only send a break
over one link? Or something else?

> +/*
> + * Issue a break command to this link
> + */
> +static int fsi_master_break(struct fsi_master *master, int link)
> +{
> +	int rc;
> +	uint32_t cmd = FSI_CMD_BREAK;
> +
> +	rc = master->write(master, link, master->break_id, master->break_offset,
> +			   &cmd, sizeof(cmd));
> +	if (rc) {
> +		dev_warn(master->dev, "break to %02x:%02x failed\n",
> +			 link, master->break_id);
> +		return -ENODEV;
> +	}
> +	udelay(200);		/* wait for logic reset to take effect */
> +
> +	rc = master->read(master, link, master->break_id,
> +			  FSI_SLAVE_BASE + FSI_SMODE, &cmd, sizeof(cmd));
> +	dev_info(master->dev, "smode after break:%08x rc:%d\n", cmd, rc);
> +	if (rc) {
> +		dev_warn(master->dev, "read smode after break failed\n");
> +		return -ENODEV;
> +	}
> +
> +	return rc;
> +}

This seems very specific to one particular master implementation. What
is a GPIO-based FSI master supposed to do here? Decode the write to
specific master addresses and then perform the GPIO sequences for a
proper break?

Instead, the break should be added as an operation on a struct
fsi_master:

  struct fsi_master {
  	struct device	*dev;
  	int		idx;
  	int		n_links;
  	int		(*read)(struct fsi_master *, int link,
  				uint8_t slave, uint32_t addr,
  				void *val, size_t size);
  	int		(*write)(struct fsi_master *, int link,
  				uint8_t slave, uint32_t addr,
  				const void *val, size_t size);
+ 	int		(*break)(struct fsi_master *, int link);
  };

- then the FSI master implementations can do what's required for
sending a break. Then, the core's defintion of fsi_master_break()
becomes:

static int fsi_master_break(struct fsi_master *master, int link)
{
	int rc = 0;

	if (master->break)
		master->break(master, link)

	return rc;
}

Cheers,


Jeremy

Re: [PATCH linux 14/15] drivers/fsi: Rename slave to cfam

[Jeremy Kerr]

Hi Chris,

> Suggested changes to the patch set so far as provided by Jeremy Kerr.
> Rename slave to cfam to distinguish between the slave engine and its
> containing cfam.

I'm still not 100% on that. As is my understanding, a (hardware) CFAM
contains multiple slaves, and that not what `struct fsi_slave`
represents (it's a single slave).

> A few formatting changes to make checkpatch accept
> patches 0001-0013.

Those would be better split out. If you like, I can then take that
change and re-roll my original skeleton series with that incorporated.

Cheers,


Jeremy

Re: [PATCH linux 15/15] drivers/fsi: Initialize CFAMs for read/write access

[Jeremy Kerr]

Hi Chris,

> static int fsi_master_break(struct fsi_master *master, int link)
> {
> 	int rc = 0;
> 
> 	if (master->break)
> 		master->break(master, link)

Sorry, that should be:
 		rc = master->break(master, link);

Cheers,


Jeremy

Re: [PATCH linux 15/15] drivers/fsi: Initialize CFAMs for read/write access

[Christopher Bostic]

On Thu, Oct 6, 2016 at 1:53 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>> Send break command to CFAMs to reset their logic.  If break
>> fails this serves as an indication there is no CFAM present.
>> Set various slave engine mode register defaults.
>>
>> Add new fields to struct fsi_master to manage hardware
>> workarounds for break ids, target addresses and cfam IDs.
>>
>> Limit each link scan to one CFAM.  Due to limitations in
>> hardware break commands only one can be recognized per link.
>
> I'm not sure what the restriction is there. We can only send a break
> over one link? Or something else?
>
A break can be issued on any available link.

Due to hardware bugs that vary depending on the combination of
master type and cfam type the break command must be issued to
different ID's and addresses accordingly.  This was the reason
for adding the cfam_id, break_id, etc.. to the fsi_master struct.

For example, an FSP master must issue a break to the base
address of the 3rd cfam slot on the link.  Offset 0x00600000
on a link that covers 0x00800000 of address space.
There are different requirements for cascaded masters and their
downstream cfams.   Due to this break command limitation
there cannot be more than one cfam per link.  There are also
similar issues with 'terminate' commands that are issued to
reset a cfam out of its locked bus error state.

Additionally, some cfams such as the P8/P9 have a much
larger address window that takes up nearly all of the 0x00800000
address space for a given link.  As such there can only
ever be one cfam on that particular link.

Once the hardware issues are resolved related to break and
terminate commands then some links would be able to support
multiple cfams.

Since the first fsi code delivery will deal with basic one cfam per
link configuration platforms to start with I don't think this is
something to gate release of a first pass fsi device driver.

>> +/*
>> + * Issue a break command to this link
>> + */
>> +static int fsi_master_break(struct fsi_master *master, int link)
>> +{
>> +     int rc;
>> +     uint32_t cmd = FSI_CMD_BREAK;
>> +
>> +     rc = master->write(master, link, master->break_id, master->break_offset,
>> +                        &cmd, sizeof(cmd));
>> +     if (rc) {
>> +             dev_warn(master->dev, "break to %02x:%02x failed\n",
>> +                      link, master->break_id);
>> +             return -ENODEV;
>> +     }
>> +     udelay(200);            /* wait for logic reset to take effect */
>> +
>> +     rc = master->read(master, link, master->break_id,
>> +                       FSI_SLAVE_BASE + FSI_SMODE, &cmd, sizeof(cmd));
>> +     dev_info(master->dev, "smode after break:%08x rc:%d\n", cmd, rc);
>> +     if (rc) {
>> +             dev_warn(master->dev, "read smode after break failed\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     return rc;
>> +}
>
> This seems very specific to one particular master implementation. What
> is a GPIO-based FSI master supposed to do here? Decode the write to
> specific master addresses and then perform the GPIO sequences for a
> proper break?
>
> Instead, the break should be added as an operation on a struct
> fsi_master:
>
>   struct fsi_master {
>         struct device   *dev;
>         int             idx;
>         int             n_links;
>         int             (*read)(struct fsi_master *, int link,
>                                 uint8_t slave, uint32_t addr,
>                                 void *val, size_t size);
>         int             (*write)(struct fsi_master *, int link,
>                                 uint8_t slave, uint32_t addr,
>                                 const void *val, size_t size);
> +       int             (*break)(struct fsi_master *, int link);
>   };
>
> - then the FSI master implementations can do what's required for
> sending a break. Then, the core's defintion of fsi_master_break()
> becomes:
>
> static int fsi_master_break(struct fsi_master *master, int link)
> {
>         int rc = 0;
>
>         if (master->break)
>                 master->break(master, link)
>
>         return rc;
> }

OK, I'll make that change.

>
> Cheers,
>
>
> Jeremy

Re: [PATCH linux 14/15] drivers/fsi: Rename slave to cfam

[Christopher Bostic]

On Thu, Oct 6, 2016 at 1:58 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>> Suggested changes to the patch set so far as provided by Jeremy Kerr.
>> Rename slave to cfam to distinguish between the slave engine and its
>> containing cfam.
>
> I'm still not 100% on that. As is my understanding, a (hardware) CFAM
> contains multiple slaves, and that not what `struct fsi_slave`
> represents (it's a single slave).

In redundant configurations yes there are two slaves.  However, any
controlling device driver would only ever use one of them.  A redundant
system with its own fsi device driver would be controlling the other slave.

I still maintain that there is bound to be confusion with slave->read( )
etc... which are meant for accessing more than just the slave engine.
cfam->read makes it more clear I think that you are accessing some
range within the cfam, not just the slave engine.  A cfam acts as a slave
in a general sense but it doesn't distinguish itself from the slave engine.

If this is a big sticking point I'll rename it to slave and deal with the
potential ambiguity that may come up later. I'd like to move ahead
with the rest of the implementation.

>
>> A few formatting changes to make checkpatch accept
>> patches 0001-0013.
>
> Those would be better split out. If you like, I can then take that
> change and re-roll my original skeleton series with that incorporated.
>

I'm not sure what split out means in this case.  Git am applies each patch
in sequence so those warnings will be hit before any patches I add to reverse
something are applied.

I know we had discussed last week about making it clear what changes
you had made versus mine.  As per discussion I left your changes as
is in the patch series, then made my recommended changes as the
next in the series.   A bit confusing on how to deal with checkpatch,
git am, ... in those circumstances.

> Cheers,
>
>
> Jeremy

Re: [PATCH linux 14/15] drivers/fsi: Rename slave to cfam

[Christopher Bostic]

On Thu, Oct 6, 2016 at 1:58 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>> Suggested changes to the patch set so far as provided by Jeremy Kerr.
>> Rename slave to cfam to distinguish between the slave engine and its
>> containing cfam.
>
> I'm still not 100% on that. As is my understanding, a (hardware) CFAM
> contains multiple slaves, and that not what `struct fsi_slave`
> represents (it's a single slave).

One other thing that would make a cfam struct that is separate from the
slave engine useful is the fact that there are several different hardware
quirks for which the only way for firmware to know what to do is to check
the cfam type.

P8, P9, zNEW cfam types

* Controlling master's MATRB bit encoding is different versus other cfams.
This is not a function of the master type, only cfam type.

   I'd propose something like:



   uint32_t cfam->decode_matrb(master->matrb)   possibly.

* Any master that has cascaded masters downstream must set its
MSIEP registers differently than if there was no cascaded master.
This is to work around a known hardware bug in interrupt signaling.
The only way to determine this is to see if downstream cfam is a type
that has a cascaded master.  Again independent of a controlling
master's type.

   bool cfam->has_cascaded_master(master->cfam);

Several cfam engine device drivers have to work around known
hardware bugs on specific hardware.  There are cases where
the same hardware engine (id and version same) behaves differently
on two different cfam types:

* IOU mailbox engine has same engine id, version number but
has different behavior on a CFAM-S type versus an IOU type.
This would require some form of fsi client query of cfam type
or initialization of some op.

* The P9 iic boe engine did not have its version ID incremented
from P8 even though there are differences in the command
register set.  There is no way for the iic driver to know which
engine its on unless it knows the cfam type.

The IMD iic engine with the same engine ID and version will
have differences in iic clocks due to differences in cfam
type.  The iic bus clock is derived from the parent fsi master
clock which is divided down differently on different cfam types.
In order for iic driver to know what frequency its buses are
operating at has to know the cfam type.







>
>
> Jeremy

Re: [PATCH linux 15/15] drivers/fsi: Initialize CFAMs for read/write access

[Jeremy Kerr]

Hi Chris,

> A break can be issued on any available link.

OK.

> Due to hardware bugs that vary depending on the combination of
> master type and cfam type the break command must be issued to
> different ID's and addresses accordingly.  This was the reason for
> adding the cfam_id, break_id, etc.. to the fsi_master struct.

But (according to the docs I have) a break is just a logical 1 for 256
clocks - it contains no addressing ID / information. What do the
cfam_ids and break_ids represent "on the wire"?

Cheers,


Jeremy

Re: [PATCH linux 14/15] drivers/fsi: Rename slave to cfam

[Jeremy Kerr]

Hi Chris,

>>> Suggested changes to the patch set so far as provided by Jeremy Kerr.
>>> Rename slave to cfam to distinguish between the slave engine and its
>>> containing cfam.
>>
>> I'm still not 100% on that. As is my understanding, a (hardware) CFAM
>> contains multiple slaves, and that not what `struct fsi_slave`
>> represents (it's a single slave).
> 
> In redundant configurations yes there are two slaves.  However, any
> controlling device driver would only ever use one of them.  A redundant
> system with its own fsi device driver would be controlling the other slave.
> 
> I still maintain that there is bound to be confusion with slave->read( )
> etc... which are meant for accessing more than just the slave engine.
> cfam->read makes it more clear I think that you are accessing some
> range within the cfam, not just the slave engine.
>
> A cfam acts as a slave
> in a general sense but it doesn't distinguish itself from the slave engine.

Right, and that's where our confusion is coming from there.

In my initial series, I've used `struct fsi_slave` to represent *just*
the slave on the FSI bus, which is logically separated from the *slave
engines*. In this case, a "slave" is a different logical unit from a
"slave engine"; the latter being one of the actual functional units made
available behind the slave.

This separation is important, as each of the engines is what will be
bound to a FSI device driver, whereas the slave itself will be managed
by the core.

So we have:

  struct fsi_slave -> core slave logic, knows little about engines

  struct fsi_device -> engine logic, to be bound to a driver

(whereas a CFAM is slave + engines, right?)

Nothing outside of the core will be doing slave->read; FSI engine
drivers will be using the fsi_device API.

> I'm not sure what split out means in this case.  Git am applies each patch
> in sequence so those warnings will be hit before any patches I add to reverse
> something are applied.
> 
> I know we had discussed last week about making it clear what changes
> you had made versus mine.  As per discussion I left your changes as
> is in the patch series, then made my recommended changes as the
> next in the series.   A bit confusing on how to deal with checkpatch,
> git am, ... in those circumstances.

Yep, and if there are fixups to those patches, keep them separate from
other changes, as I'll apply them directly to my tree.

From your follow-up email, it sounds like we want some identification of
the slave HW implementation, to allow the core and drivers to work
around any HW issues.

Cheers,


Jeremy

Re: [PATCH linux 15/15] drivers/fsi: Initialize CFAMs for read/write access

[Christopher Bostic]

On Thu, Oct 6, 2016 at 8:21 PM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>> A break can be issued on any available link.
>
> OK.
>
>> Due to hardware bugs that vary depending on the combination of
>> master type and cfam type the break command must be issued to
>> different ID's and addresses accordingly.  This was the reason for
>> adding the cfam_id, break_id, etc.. to the fsi_master struct.
>
> But (according to the docs I have) a break is just a logical 1 for 256
> clocks - it contains no addressing ID / information. What do the
> cfam_ids and break_ids represent "on the wire"?

Right, at the protocol level a break is simply 256 1's.   One of the
cascaded masters on P8,P9 though has quirks where you must
adjust the target address and ID when writing to the actual hardware.
I don't know the hardware specifics as to what the bug is but
the logic responsible for the translation of a firmware initiated MMIO
onto a link that results in the 256 1's has a bug that corrupts that
message in some way.  Note that this is even an issue for the gpio
master since the bug I just described is in the P8,P9 cascaded master
hardware.

Sounds like we need  to schedule a meeting to probably
go over all the hardware quirks that need to be dealt with.  Exchanging
emails probably isn't the quickest way.

Long story short we'll still need a master->break( ) operation defined
for each master type.

-Chris

>
> Cheers,
>
>
> Jeremy

Re: [PATCH linux 14/15] drivers/fsi: Rename slave to cfam

[Christopher Bostic]

On Thu, Oct 6, 2016 at 8:22 PM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>>>> Suggested changes to the patch set so far as provided by Jeremy Kerr.
>>>> Rename slave to cfam to distinguish between the slave engine and its
>>>> containing cfam.
>>>
>>> I'm still not 100% on that. As is my understanding, a (hardware) CFAM
>>> contains multiple slaves, and that not what `struct fsi_slave`
>>> represents (it's a single slave).
>>
>> In redundant configurations yes there are two slaves.  However, any
>> controlling device driver would only ever use one of them.  A redundant
>> system with its own fsi device driver would be controlling the other slave.
>>
>> I still maintain that there is bound to be confusion with slave->read( )
>> etc... which are meant for accessing more than just the slave engine.
>> cfam->read makes it more clear I think that you are accessing some
>> range within the cfam, not just the slave engine.
>>
>> A cfam acts as a slave
>> in a general sense but it doesn't distinguish itself from the slave engine.
>
> Right, and that's where our confusion is coming from there.
>
> In my initial series, I've used `struct fsi_slave` to represent *just*
> the slave on the FSI bus, which is logically separated from the *slave
> engines*. In this case, a "slave" is a different logical unit from a
> "slave engine"; the latter being one of the actual functional units made
> available behind the slave.
>
> This separation is important, as each of the engines is what will be
> bound to a FSI device driver, whereas the slave itself will be managed
> by the core.
> So we have:
>
>   struct fsi_slave -> core slave logic, knows little about engines
>
>   struct fsi_device -> engine logic, to be bound to a driver
>
> (whereas a CFAM is slave + engines, right?)
>
> Nothing outside of the core will be doing slave->read; FSI engine
> drivers will be using the fsi_device API.

Hi Jeremy,

That makes sense.  I wasn't getting what you were thinking.  I'll
keep your 'struct fsi_slave' as is.   The previous note regarding
hardware workarounds, though, will possibly require some concept
of a separate 'struct cfam'.   That or some other construct to
deal in a common way with all the quirks related to a particular
cfam.  Can you make Chris Austen's call Tuesday morning
your time?  We can discuss that further.

>
>> I'm not sure what split out means in this case.  Git am applies each patch
>> in sequence so those warnings will be hit before any patches I add to reverse
>> something are applied.
>>
>> I know we had discussed last week about making it clear what changes
>> you had made versus mine.  As per discussion I left your changes as
>> is in the patch series, then made my recommended changes as the
>> next in the series.   A bit confusing on how to deal with checkpatch,
>> git am, ... in those circumstances.
>
> Yep, and if there are fixups to those patches, keep them separate from
> other changes, as I'll apply them directly to my tree.

So the plan is,  patches 01-12 your set.  patch 13 are my fixups to that.
Patches 14+ my content.

>
> From your follow-up email, it sounds like we want some identification of
> the slave HW implementation, to allow the core and drivers to work
> around any HW issues.
>
> Cheers,
>
>
> Jeremy