[PATCH linux v4 00/20] FSI device driver introduction

[PATCH linux v4 00/20] FSI device driver introduction

[christopher.lee.bostic]

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

Patch Descriptions:

Patches 01-14 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 14 was written by Jeremy Kerr contrary to what the commit
message indicates. Contains edits to the existing crc4 utilities
to handle arbitrary length input data.

Patch 15 contains minimal text formatting changes to deal with
checkpatch and git am warnings.

Patch 16 adds function to set up read/write access to a given CFAM.

Patch 17 sets up slave smode during scanning operations to allow
config space access.

Patch 18 covers implementation of the FSI GPIO master.

Patch 19 core additions for client driver registration.

Patch 20 SCOM client device driver creation


General Information:

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.

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 Client driver
    A device driver that registers with the FSI core so that it can access
    devices it owns on an FSI bus.

Chris Bostic (7):
  drivers/fsi: CRC Utility Updates
  drivers/fsi: Fix some text formatting
  drivers/fsi: Set up links for slave communication
  drivers/fsi: Set slave SMODE to init communications
  drivers/fsi: Add GPIO FSI master functionality
  drivers/fsi: Add client driver register utilities
  drivers/fsi: Add SCOM client driver

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    |  21 +
 arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts      |  30 ++
 drivers/Kconfig                                    |   2 +
 drivers/Makefile                                   |   1 +
 drivers/fsi/Kconfig                                |  35 ++
 drivers/fsi/Makefile                               |   5 +
 drivers/fsi/fsi-core.c                             | 496 +++++++++++++++++++++
 drivers/fsi/fsi-master-fake.c                      |  95 ++++
 drivers/fsi/fsi-master-gpio.c                      | 465 +++++++++++++++++++
 drivers/fsi/fsi-master.h                           |  60 +++
 drivers/fsi/fsi-scom.c                             | 158 +++++++
 include/linux/fsi.h                                |  63 +++
 12 files changed, 1431 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 drivers/fsi/fsi-scom.c
 create mode 100644 include/linux/fsi.h

-- 
1.8.2.2

[PATCH linux v4 01/20] 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 v4 02/20] 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 v4 03/20] 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 v4 04/20] 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 v4 05/20] 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 v4 06/20] 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 v4 07/20] 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 v4 08/20] 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 v4 09/20] 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 v4 10/20] 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 v4 11/20] 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 v4 12/20] 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 v4 13/20] 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 v4 14/20] drivers/fsi: CRC Utility Updates

[christopher.lee.bostic]

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

Provided by Jeremy Kerr.  Revise the CRC calculation utilities
to take variable bit length input.

Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 drivers/fsi/fsi-core.c   | 16 +++++++++-------
 drivers/fsi/fsi-master.h | 21 +++++++++++++++++++++
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 10bf817..07f5034 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -111,21 +111,23 @@ static const uint8_t crc4_tab[] = {
 	0x1, 0x6, 0xf, 0x8, 0xa, 0xd, 0x4, 0x3,
 };
 
-static uint8_t crc4(uint32_t x)
+uint8_t fsi_crc4(uint8_t c, uint64_t x, int bits)
 {
-	uint8_t c = 0;
 	int i;
 
+	/* ALign to 4-bits */
+	bits = (bits + 3) & ~0x3;
+
 	/* Calculate crc4 over four-bit nibbles, starting at the MSbit */
-	for (i = 28; i >= 0; i -= 4)
+	for (i = bits; i >= 0; i -= 4)
 		c = crc4_tab[c ^ ((x >> i) & 0xf)];
 
 	return c;
 }
 
-static bool check_crc4(uint32_t x)
+static bool check_crc4_u32(uint32_t x)
 {
-	return crc4(x) == 0;
+	return fsi_crc4(0, x, 32) == 0;
 }
 
 /* FSI slave support */
@@ -171,7 +173,7 @@ static int fsi_slave_scan(struct fsi_slave *slave)
 
 		conf = be32_to_cpu(conf);
 
-		if (!check_crc4(conf)) {
+		if (!check_crc4_u32(conf)) {
 			dev_warn(&slave->dev,
 				"crc error in slave register at 0x%04x\n",
 					i);
@@ -247,7 +249,7 @@ static int fsi_slave_init(struct fsi_master *master,
 	}
 
 	chip_id = be32_to_cpu(chip_id);
-	if (!check_crc4(chip_id)) {
+	if (!check_crc4_u32(chip_id)) {
 		dev_warn(master->dev, "slave %02x:%02x: invalid chip id CRC!\n",
 				link, slave_id);
 		return -EIO;
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index e75a810..cafb433 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -34,4 +34,25 @@ struct fsi_master {
 extern int fsi_master_register(struct fsi_master *master);
 extern void fsi_master_unregister(struct fsi_master *master);
 
+/**
+ * crc4 helper: Given a starting crc4 state @c, calculate the crc4 vaue of @x,
+ * which is @bits in length. This may be required by master implementations
+ * that do not provide their own hardware checksums.
+ *
+ * The crc4 is performed on 4-bit chunks (which is all we need for FSI
+ * calculations). Typically, we'll want a starting state of 0:
+ *
+ *  c = fsi_crc4(0, msg, len);
+ *
+ * To crc4 a message that includes a single start bit, initialise crc4 state
+ * with:
+ *
+ *  c = fsi_crc4(0, 1, 1);
+ *
+ * Then update with message data:
+ *
+ *  c = fsi_crc4(c, msg, len);
+ */
+uint8_t fsi_crc4(uint8_t c, uint64_t x, int bits);
+
 #endif /* DRIVERS_FSI_MASTER_H */
-- 
1.8.2.2

[PATCH linux v4 15/20] drivers/fsi: Fix some text formatting

[christopher.lee.bostic]

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

Cleaned up a few formatting warnings like blank lines at EOF
and block comment style.

Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 drivers/fsi/fsi-core.c        | 8 ++++++--
 drivers/fsi/fsi-master-fake.c | 1 -
 drivers/fsi/fsi-master-gpio.c | 1 -
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 07f5034..5cbe78d 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -87,6 +87,7 @@ int fsi_device_peek(struct fsi_device *dev, void *val)
 static void fsi_device_release(struct device *_device)
 {
 	struct fsi_device *device = to_fsi_dev(_device);
+
 	kfree(device);
 }
 
@@ -187,8 +188,10 @@ static int fsi_slave_scan(struct fsi_slave *slave)
 		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 */
@@ -231,6 +234,7 @@ static int fsi_slave_scan(struct fsi_slave *slave)
 static void fsi_slave_release(struct device *dev)
 {
 	struct fsi_slave *slave = to_fsi_slave(dev);
+
 	kfree(slave);
 }
 
diff --git a/drivers/fsi/fsi-master-fake.c b/drivers/fsi/fsi-master-fake.c
index 50aac0b..ec1ed5e 100644
--- a/drivers/fsi/fsi-master-fake.c
+++ b/drivers/fsi/fsi-master-fake.c
@@ -93,4 +93,3 @@ static int __init fsi_master_fake_init(void)
 }
 
 module_init(fsi_master_fake_init);
-
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index 3855829..5c3d5cd 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -89,4 +89,3 @@ static struct platform_driver fsi_master_gpio_driver = {
 };
 
 module_platform_driver(fsi_master_gpio_driver);
-
-- 
1.8.2.2

[PATCH linux v4 16/20] drivers/fsi: Set up links for slave communication

[christopher.lee.bostic]

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

Enable each link and send a break command in preparation
for scanning each link for slaves.

V3 - Remove definition of BREAK command from fsi-master.h

   - Remove definitions of fsi_master_fake for set_smode
     and break

   - Remove master->set_smode master dependent function and
     moved to a generic base master set_smode.

   - Add fsi_master_link_enable with master type dependencies

V4 - Remove all references to set smode

   - Remove file fsi-slave.h

   - Move link break and enable up into master scan

   - Change rc = func(); return rc; coding to return func();
     Note this is still in place in fsi_master_send_break in
     anticipation of changes coming up in next patch of this
     series.

   - Fix comment spelling error

   - Use dev_dbg instead of dev_info when link enable or break
     fails

   - Remove explicit set of master->break and
     master->link_enable to NULL due to kzalloc

Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 drivers/fsi/fsi-core.c        | 43 ++++++++++++++++++++++++++++++++++++++++---
 drivers/fsi/fsi-master-gpio.c | 31 +++++++++++++++++++++++++++++++
 drivers/fsi/fsi-master.h      |  2 ++
 3 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 5cbe78d..0195373 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -287,16 +287,53 @@ static int fsi_slave_init(struct fsi_master *master,
 
 /* FSI master support */
 
+static int fsi_master_link_enable(struct fsi_master *master, int link)
+{
+	if (master->link_enable)
+		return master->link_enable(master, link);
+
+	return -EINVAL;
+}
+
+/*
+ * Issue a break command on this link
+ */
+static int fsi_master_break(struct fsi_master *master, int link)
+{
+	int rc;
+
+	if (!master->send_break)
+		return -EINVAL;
+
+	rc =  master->send_break(master, link);
+	if (rc)
+		dev_info(master->dev, "break failed with:%d\n", rc);
+
+	return rc;
+}
+
 static int fsi_master_scan(struct fsi_master *master)
 {
-	int link, slave_id;
+	int link, slave_id, rc;
 
-	for (link = 0; link < master->n_links; link++)
+	for (link = 0; link < master->n_links; link++) {
+		rc = fsi_master_link_enable(master, link);
+		if (rc) {
+			dev_dbg(master->dev,
+				"Enable of link:%d failed with:%d\n", link, rc);
+			continue;
+		}
+		rc = fsi_master_break(master, link);
+		if (rc) {
+			dev_dbg(master->dev,
+				"break to link:%d failed with:%d\n", link, rc);
+			continue;
+		}
 		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)
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index 5c3d5cd..f8e5ca4 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -50,6 +50,35 @@ static int fsi_master_gpio_write(struct fsi_master *_master, int link,
 	return 0;
 }
 
+/*
+ * Issue a break command on link
+ */
+static int fsi_master_gpio_break(struct fsi_master *_master, int link)
+{
+	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+
+	if (link != 0)
+		return -ENODEV;
+
+	/* todo: send the break pattern over gpio */
+	(void)master;
+
+	return 0;
+}
+
+static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
+{
+	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+
+	if (link != 0)
+		return -ENODEV;
+
+	/* todo: set the enable pin */
+	(void)master;
+
+	return 0;
+}
+
 static int fsi_master_gpio_probe(struct platform_device *pdev)
 {
 	struct fsi_master_gpio *master;
@@ -71,6 +100,8 @@ 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.send_break = fsi_master_gpio_break;
+	master->master.link_enable = fsi_master_gpio_link_enable;
 
 	return fsi_master_register(&master->master);
 }
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index cafb433..56aad0e 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -29,6 +29,8 @@ struct fsi_master {
 	int		(*write)(struct fsi_master *, int link,
 				uint8_t slave, uint32_t addr,
 				const void *val, size_t size);
+	int		(*send_break)(struct fsi_master *, int link);
+	int		(*link_enable)(struct fsi_master *, int link);
 };
 
 extern int fsi_master_register(struct fsi_master *master);
-- 
1.8.2.2

[PATCH linux v4 17/20] drivers/fsi: Set slave SMODE to init communications

[christopher.lee.bostic]

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

Set CFAM to appropriate ID so that the controlling master
can manage link memory ranges.  Add slave engine register
definitions.

V3 - Remove master->set_smode op and make it generic for
     all masters.

V4 - 'Change rc = func(); return rc;' code to 'return func();'

   - Change set_smode_defaults to return value instead of
     passing back via reference.

   - Rename fsi_master_set_smode to fsi_slave_set_smode

   - Pass in slave id to fsi_slave_set_smode instead of
     making assumptions internal to the function.

   - Only allow slave inits on id 0 - a todo to accommodate
     all slave ID's.

Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 drivers/fsi/fsi-core.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 89 insertions(+), 3 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 0195373..185aff4 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -33,6 +33,7 @@
 #define FSI_SLAVE_CONF_TYPE_SHIFT	4
 
 #define FSI_PEEK_BASE			0x410
+#define FSI_SLAVE_BASE			0x800
 
 static const int engine_page_size = 0x400;
 
@@ -52,6 +53,25 @@ static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
 static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
 		const void *val, size_t size);
 
+/*
+ * FSI slave engine control register offsets
+ */
+#define	FSI_SMODE		0x0	/* R/W: Mode register */
+
+/*
+ * SMODE fields
+ */
+#define	FSI_SMODE_WSC		0x80000000	/* Warm start done */
+#define	FSI_SMODE_ECRC		0x20000000      /* Enable hdware 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 */
+
 /* FSI endpoint-device support */
 int fsi_device_read(struct fsi_device *dev, uint32_t addr, void *val,
 		size_t size)
@@ -132,6 +152,30 @@ static bool check_crc4_u32(uint32_t x)
 }
 
 /* FSI slave support */
+
+/* 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;
+}
 static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
 		void *val, size_t size)
 {
@@ -238,6 +282,22 @@ static void fsi_slave_release(struct device *dev)
 	kfree(slave);
 }
 
+static uint32_t set_smode_defaults(struct fsi_master *master)
+{
+	return FSI_SMODE_WSC | FSI_SMODE_ECRC
+		| fsi_smode_echodly(0xf) | fsi_smode_senddly(0xf)
+		| fsi_smode_lbcrr(1);
+}
+
+static int fsi_slave_set_smode(struct fsi_master *master, int link, int id)
+{
+	uint32_t smode = set_smode_defaults(master);
+
+	smode |= fsi_smode_sid(id);
+	return master->write(master, link, 0, FSI_SLAVE_BASE + FSI_SMODE,
+			&smode, sizeof(smode));
+}
+
 static int fsi_slave_init(struct fsi_master *master,
 		int link, uint8_t slave_id)
 {
@@ -245,6 +305,21 @@ static int fsi_slave_init(struct fsi_master *master,
 	uint32_t chip_id;
 	int rc;
 
+	/*
+	 * todo: Due to CFAM hardware issues related to BREAK commands we're
+	 * limited to only one CFAM per link.  Once issues are resolved this
+	 * restriction can be removed.
+	 */
+	if (slave_id > 0)
+		return 0;
+
+	rc = fsi_slave_set_smode(master, link, slave_id);
+	if (rc) {
+		dev_warn(master->dev, "can't set smode on slave%02x:%02x: %d\n",
+				link, slave_id, rc);
+		return -ENODEV;
+	}
+
 	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",
@@ -301,15 +376,26 @@ static int fsi_master_link_enable(struct fsi_master *master, int link)
 static int fsi_master_break(struct fsi_master *master, int link)
 {
 	int rc;
+	uint32_t smode;
 
 	if (!master->send_break)
 		return -EINVAL;
 
-	rc =  master->send_break(master, link);
-	if (rc)
+	rc = master->send_break(master, link);
+	if (rc) {
 		dev_info(master->dev, "break failed with:%d\n", rc);
+		return rc;
+	}
+	/* Verify can read slave at default ID location */
+	rc = master->read(master, link, 3, FSI_SLAVE_BASE + FSI_SMODE,
+			&smode, sizeof(smode));
 
-	return rc;
+	/* If read fails then nothing on other end of link */
+	if (rc)
+		return -ENODEV;
+
+	dev_dbg(master->dev, "smode at ID 3 after break:%08x\n", smode);
+	return 0;
 }
 
 static int fsi_master_scan(struct fsi_master *master)
-- 
1.8.2.2

[PATCH linux v4 18/20] drivers/fsi: Add GPIO FSI master functionality

[christopher.lee.bostic]

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

Add setup of the GPIO pins for FSI master function. Setup I/O directions,
define all pins and setup their initial directions. Define serial out
operation. Encode and decode all transactions on the SDA line.

V3 - Added remainder of base I/O ops (excluding dev tree and crc calcs).

   - Add encoding of all read/write commands and send data over sda

   - Check for and decode response from slave.

   - Add set enable pin in master->link_enable() master specific function.

V4 - Remove unnecessary comments about delays in various locations

   - Define non clock and data pins as optional and check for gpio
     descriptors == NULL before accessing.  Don't return error if
     optional pins cannot be retrieved via devm.

   - serial_in: rearrange order of msg shift and input bit OR op.

   - serial_out: Fix mirror image issue on shift data out and set
     data out as necessary for bit count to be sent.

   - serial_out: make message parm a const

   - serial_out: fix 3x 'msg & data' repeat

   - poll_for_response: bits remaining was calculated with
     sizeof(size) should be size.

   - fsi_master_gpio_break: remove smode read

   - Replace dev_info with dev_dbg in proper places.

   - Add a bit count parm to serial_in, increment msg->bits
     on every bit received.

   - Remove magic numbers in poll_for_response that indicate
     how many bits to receive

   - Utilize the crc utilities to check input data and set
     crc for output data

   - Remove add_crc stub

Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 .../devicetree/bindings/fsi/fsi-master-gpio.txt    |  13 +
 arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts      |  30 ++
 drivers/fsi/fsi-master-gpio.c                      | 365 ++++++++++++++++++++-
 3 files changed, 397 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt b/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
index 44762a3..4cb2c81 100644
--- a/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
+++ b/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
@@ -1,8 +1,21 @@
 Device-tree bindings for gpio-based FSI master driver
 -----------------------------------------------------
 
+Required properties:
+	- compatible = "ibm,fsi-master-gpio";
+	- clk-gpio;
+	- data-gpio;
+
+Optional properties:
+	- enable-gpio;
+	- trans-gpio;
+	- mux-gpio;
+
 fsi-master {
 	compatible = "ibm,fsi-master", "ibm,fsi-master-gpio";
 	clk-gpio = <&gpio 0>;
 	data-gpio = <&gpio 1>;
+	enable-gpio = <&gpio 2>;
+	trans-gpio = <&gpio 3>;
+	mux-gpio = <&gpio 4>;
 }
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
index 21619fd..e74dd6b 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
@@ -167,6 +167,36 @@
 		output-low;
 		line-name = "func_mode2";
 	};
+
+	pin_clk {
+		gpios = <ASPEED_GPIO(A, 4)  GPIO_ACTIVE_HIGH>;
+		output-low;
+		line-name = "clk";
+	};
+
+	pin_data {
+		gpios = <ASPEED_GPIO(A, 5)  GPIO_ACTIVE_HIGH>;
+		output-low;
+		line-name = "data";
+	};
+
+	pin_trans {
+		gpios = <ASPEED_GPIO(H, 6)  GPIO_ACTIVE_HIGH>;
+		output-low;
+		line-name = "trans";
+	};
+
+	pin_enable {
+		gpios = <ASPEED_GPIO(D, 0)  GPIO_ACTIVE_HIGH>;
+		output-low;
+		line-name = "enable";
+	};
+
+	pin_mux {
+		gpios = <ASPEED_GPIO(A, 6)  GPIO_ACTIVE_HIGH>;
+		output-low;
+		line-name = "mux";
+	};
 };
 
 &vuart {
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index f8e5ca4..58540b8 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -5,13 +5,51 @@
 #include <linux/platform_device.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
+#include <linux/delay.h>
 
 #include "fsi-master.h"
 
+#define	FSI_ECHO_DELAY_CLOCKS	16	/* Number clocks for echo delay */
+#define	FSI_PRE_BREAK_CLOCKS	50	/* Number clocks to prep for break */
+#define	FSI_BREAK_CLOCKS	256	/* Number of clocks to issue break */
+#define	FSI_POST_BREAK_CLOCKS	16000	/* Number clocks to set up cfam */
+#define	FSI_INIT_CLOCKS		5000	/* Clock out any old data */
+#define	FSI_GPIO_STD_DELAY	10	/* Standard GPIO delay in nS */
+					/* todo: adjust down as low as */
+					/* possible or eliminate */
+#define	FSI_GPIO_CMD_DPOLL	0x0000808A
+#define	FSI_GPIO_CMD_DPOLL_SIZE	10
+#define	FSI_GPIO_DPOLL_CLOCKS	24      /* < 21 will cause slave to hang */
+#define	FSI_GPIO_CMD_DEFAULT	0x2000000000000000ULL
+#define	FSI_GPIO_CMD_WRITE	0
+#define	FSI_GPIO_CMD_READ	0x0400000000000000ULL
+#define	FSI_GPIO_CMD_SLAVE_MASK	0xC000000000000000ULL
+#define	FSI_GPIO_CMD_ADDR_SHIFT	3
+#define	FSI_GPIO_CMD_SIZE_16	0x0000001000000000ULL
+#define	FSI_GPIO_CMD_SIZE_32	0x0000003000000000ULL
+#define	FSI_GPIO_CMD_DATA_SHIFT	28
+#define	FSI_GPIO_CMD_DFLT_LEN	32
+#define	FSI_GPIO_RESP_BUSY	1
+#define	FSI_GPIO_RESP_ERRA	2
+#define	FSI_GPIO_RESP_ERRC	3
+#define	FSI_GPIO_RESP_ACK	0
+#define	FSI_GPIO_RESP_ACKD	4
+#define	FSI_GPIO_MAX_BUSY	100
+#define	FSI_GPIO_MTOE_COUNT	1000
+#define	FSI_GPIO_MTOE		1
+#define	FSI_GPIO_DRAIN_BITS	20
+#define	FSI_GPIO_CRC_SIZE	4
+#define FSI_GPIO_MSG_ID_SIZE		2
+#define FSI_GPIO_MSG_RESPID_SIZE	2
+#define FSI_GPIO_CRC_INVAL		5
+
 struct fsi_master_gpio {
 	struct fsi_master	master;
 	struct gpio_desc	*gpio_clk;
 	struct gpio_desc	*gpio_data;
+	struct gpio_desc	*gpio_trans;	/* Voltage translator */
+	struct gpio_desc	*gpio_enable;	/* FSI enable */
+	struct gpio_desc	*gpio_mux;	/* Mux control */
 };
 
 #define to_fsi_master_gpio(m) container_of(m, struct fsi_master_gpio, master)
@@ -21,33 +59,303 @@ struct fsi_gpio_msg {
 	uint8_t		bits;
 };
 
+static void set_clock(struct fsi_master_gpio *master)
+{
+	ndelay(FSI_GPIO_STD_DELAY);
+	gpiod_set_value(master->gpio_clk, 1);
+}
+
+static void clear_clock(struct fsi_master_gpio *master)
+{
+	ndelay(FSI_GPIO_STD_DELAY);
+	gpiod_set_value(master->gpio_clk, 0);
+}
+
+static void clock_toggle(struct fsi_master_gpio *master, int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++) {
+		clear_clock(master);
+		set_clock(master);
+	}
+}
+
+static int sda_in(struct fsi_master_gpio *master)
+{
+	return gpiod_get_value(master->gpio_data);
+}
+
+static void sda_out(struct fsi_master_gpio *master, int value)
+{
+	gpiod_set_value(master->gpio_data, value);
+	ndelay(FSI_GPIO_STD_DELAY);
+}
+
+static void set_sda_input(struct fsi_master_gpio *master)
+{
+	gpiod_direction_input(master->gpio_data);
+	if (master->gpio_trans)
+		gpiod_direction_output(master->gpio_trans, 0);
+}
+static void set_sda_output(struct fsi_master_gpio *master,
+				int value)
+{
+	gpiod_direction_output(master->gpio_data, value);
+	if (master->gpio_trans)
+		gpiod_direction_output(master->gpio_trans, 1);
+}
+
+static void serial_in(struct fsi_master_gpio *master, struct fsi_gpio_msg *cmd,
+			uint8_t num_bits)
+{
+	uint8_t bit;
+	uint64_t msg = 0;
+	uint8_t in_bit = 0;
+
+	set_sda_input(master);
+	cmd->bits = 0;
+
+	for (bit = 0; bit < num_bits; bit++) {
+		clock_toggle(master, 1);
+		in_bit = sda_in(master);
+		cmd->bits++;
+		msg <<= 1;
+		msg |= in_bit;
+	}
+	cmd->msg = ~msg;		/*  Data is negative active */
+}
+
+static void serial_out(struct fsi_master_gpio *master,
+			const struct fsi_gpio_msg *cmd)
+{
+	uint8_t bit;
+	uint64_t msg = ~cmd->msg;	/* Data is negative active */
+	uint64_t sda_mask;
+	uint64_t last_bit = ~0;
+	int next_bit;
+
+	if (!cmd->bits) {
+		dev_warn(master->master.dev, "trying to output 0 bits\n");
+		return;
+	}
+	sda_mask = BIT(cmd->bits - 1);
+	set_sda_output(master, 0);
+
+	/* Send the start bit */
+	sda_out(master, 1);
+	clock_toggle(master, 1);
+
+	/* Send the message */
+	for (bit = 0; bit < cmd->bits; bit++) {
+		next_bit = (msg & sda_mask) >> (cmd->bits - 1);
+		if (last_bit ^ next_bit) {
+			sda_out(master, next_bit);
+			last_bit = next_bit;
+		}
+		clock_toggle(master, 1);
+		msg <<= 1;
+	}
+}
+
+/*
+ * Clock out some 0's after every message to ride out line reflections
+ */
+static void echo_delay(struct fsi_master_gpio *master)
+{
+	set_sda_output(master, 0);
+	clock_toggle(master, FSI_ECHO_DELAY_CLOCKS);
+}
+
+/*
+ * Used in bus error cases only.  Clears out any remaining data the slave
+ * is attempting to send
+ */
+static void drain_response(struct fsi_master_gpio *master)
+{
+	struct fsi_gpio_msg msg;
+
+	serial_in(master, &msg, FSI_GPIO_DRAIN_BITS);
+}
+
+/*
+ * Store information on master errors so handler can detect and clean
+ * up the bus
+ */
+static void fsi_master_gpio_error(struct fsi_master_gpio *master, int error)
+{
+
+}
+
+static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,
+			uint8_t size, void *data)
+{
+	int busy_count = 0, i;
+	struct fsi_gpio_msg response, cmd;
+	int bits_remaining = 0;
+	uint64_t resp = 0;
+	uint8_t bits_received = 1 + FSI_GPIO_MSG_ID_SIZE +
+				FSI_GPIO_MSG_RESPID_SIZE;
+	uint8_t crc_in;
+
+	do {
+		response.msg = 0;
+		for (i = 0; i < FSI_GPIO_MTOE_COUNT; i++) {
+			serial_in(master, &response, 1);
+			resp = response.msg;
+			if (response.msg)
+				break;
+		}
+		if (unlikely(i >= FSI_GPIO_MTOE_COUNT)) {
+			dev_info(master->master.dev,
+			"Master time out waiting for response\n");
+			drain_response(master);
+			fsi_master_gpio_error(master, FSI_GPIO_MTOE);
+			return FSI_GPIO_MTOE;
+		}
+
+		/* Response received */
+		response.msg = 0;
+		serial_in(master, &response, FSI_GPIO_MSG_ID_SIZE);
+		dev_info(master->master.dev, "cfam id:%d\n", (int)response.msg);
+		resp <<= FSI_GPIO_MSG_ID_SIZE;
+		resp |= response.msg;
+
+		response.msg = 0;
+		serial_in(master, &response, FSI_GPIO_MSG_RESPID_SIZE);
+		dev_info(master->master.dev, "response id:%d\n",
+				(int)response.msg);
+		resp <<= FSI_GPIO_MSG_RESPID_SIZE;
+		resp |= response.msg;
+
+		switch (response.msg) {
+		case FSI_GPIO_RESP_ACK:
+			if (expected == FSI_GPIO_RESP_ACKD)
+				bits_remaining = 8 * size;
+			break;
+
+		case FSI_GPIO_RESP_BUSY:
+			/*
+			 * Its necessary to clock slave before issuing
+			 * d-poll, not indicated in the hardware protocol
+			 * spec. < 20 clocks causes slave to hang, 21 ok.
+			 */
+			set_sda_output(master, 0);
+			clock_toggle(master, FSI_GPIO_DPOLL_CLOCKS);
+			cmd.msg = FSI_GPIO_CMD_DPOLL;
+			cmd.bits = FSI_GPIO_CMD_DPOLL_SIZE;
+			serial_out(master, &cmd);
+			continue;
+
+		case FSI_GPIO_RESP_ERRA:
+		case FSI_GPIO_RESP_ERRC:
+			dev_info(master->master.dev, "ERR received: %d\n",
+				(int)response.msg);
+			drain_response(master);
+			fsi_master_gpio_error(master, response.msg);
+			return response.msg;
+		}
+
+		/* Read in the data field if applicable */
+		if (bits_remaining) {
+			response.msg = 0;
+			serial_in(master, &response, bits_remaining);
+			resp <<= bits_remaining;
+			resp |= response.msg;
+			bits_received += bits_remaining;
+		}
+
+		/* Read in the crc and check it */
+		response.msg = 0;
+		serial_in(master, &response, FSI_GPIO_CRC_SIZE);
+
+		crc_in = fsi_crc4(0, resp, bits_received);
+		if (crc_in != response.msg) {
+			/* CRC's don't match */
+			dev_info(master->master.dev, "ERR response CRC\n");
+			fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
+			return FSI_GPIO_CRC_INVAL;
+		}
+		return 0;
+
+	} while (busy_count++ < FSI_GPIO_MAX_BUSY);
+
+	return busy_count;
+}
+
+static void build_command(struct fsi_gpio_msg *cmd, uint64_t mode,
+		uint8_t slave, uint32_t addr, size_t size,
+		const void *data)
+{
+	uint8_t crc;
+	uint64_t msg_with_start;
+
+	cmd->bits = FSI_GPIO_CMD_DFLT_LEN;
+	cmd->msg = FSI_GPIO_CMD_DEFAULT;
+	cmd->msg |= mode;
+
+	/* todo: handle more than just slave id 0 */
+	cmd->msg &= ~FSI_GPIO_CMD_SLAVE_MASK;
+
+	cmd->msg |= (addr << FSI_GPIO_CMD_ADDR_SHIFT);
+	if (size == sizeof(uint8_t)) {
+		if (data)
+			cmd->msg |=
+				*((uint8_t *)data) >> FSI_GPIO_CMD_DATA_SHIFT;
+	} else if (size == sizeof(uint16_t)) {
+		cmd->msg |= FSI_GPIO_CMD_SIZE_16;
+		if (data)
+			cmd->msg |=
+				*((uint16_t *)data) >> FSI_GPIO_CMD_DATA_SHIFT;
+	} else {
+		cmd->msg |= FSI_GPIO_CMD_SIZE_32;
+		if (data)
+			cmd->msg |=
+				*((uint32_t *)data) >> FSI_GPIO_CMD_DATA_SHIFT;
+	}
+
+	if (mode == FSI_GPIO_CMD_WRITE)
+		cmd->bits += (8 * size);
+
+	/* Start bit isn't considered part of command but we need to */
+	/* account for it in crc calcs */
+	msg_with_start = 0x1 << cmd->bits;
+	msg_with_start |= cmd->msg;
+	crc = fsi_crc4(0, msg_with_start, cmd->bits);
+	cmd->msg |= crc >> cmd->bits;
+	cmd->bits += FSI_GPIO_CRC_SIZE;
+}
+
 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);
+	struct fsi_gpio_msg cmd;
 
 	if (link != 0)
 		return -ENODEV;
 
-	/* todo: format read into a message, send, poll for response */
-	(void)master;
+	build_command(&cmd, FSI_GPIO_CMD_READ, slave, addr, size, NULL);
+	serial_out(master, &cmd);
+	echo_delay(master);
 
-
-	return 0;
+	return poll_for_response(master, FSI_GPIO_RESP_ACKD, size, val);
 }
 
 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);
+	struct fsi_gpio_msg cmd;
 
 	if (link != 0)
 		return -ENODEV;
 
-	/* todo: format write into a message, send, poll for response */
-	(void)master;
+	build_command(&cmd, FSI_GPIO_CMD_WRITE, slave, addr, size, val);
+	serial_out(master, &cmd);
+	echo_delay(master);
 
-	return 0;
+	return poll_for_response(master, FSI_GPIO_RESP_ACK, size, NULL);
 }
 
 /*
@@ -60,12 +368,31 @@ static int fsi_master_gpio_break(struct fsi_master *_master, int link)
 	if (link != 0)
 		return -ENODEV;
 
-	/* todo: send the break pattern over gpio */
-	(void)master;
+	set_sda_output(master, 0);
+	clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
+	sda_out(master, 1);
+	clock_toggle(master, FSI_BREAK_CLOCKS);
+	echo_delay(master);
+	sda_out(master, 0);
+	clock_toggle(master, FSI_POST_BREAK_CLOCKS);
+	udelay(200);		/* Wait for logic reset to take effect */
 
 	return 0;
 }
 
+static void fsi_master_gpio_init(struct fsi_master_gpio *master)
+{
+	if (master->gpio_mux)
+		gpiod_direction_output(master->gpio_mux, 1);
+	gpiod_direction_output(master->gpio_clk, 1);
+	set_sda_output(master, 1);
+	if (master->gpio_enable)
+		gpiod_direction_output(master->gpio_enable, 0);
+
+	/* todo: evaluate if clocks can be reduced */
+	clock_toggle(master, FSI_INIT_CLOCKS);
+}
+
 static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
 {
 	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
@@ -73,8 +400,8 @@ static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
 	if (link != 0)
 		return -ENODEV;
 
-	/* todo: set the enable pin */
-	(void)master;
+	if (master->gpio_enable)
+		gpiod_set_value(master->gpio_enable, 1);
 
 	return 0;
 }
@@ -98,11 +425,27 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
 		return PTR_ERR(gpio);
 	master->gpio_data = gpio;
 
+	/* Optional pins */
+
+	gpio = devm_gpiod_get(&pdev->dev, "trans", 0);
+	if (!IS_ERR(gpio))
+		master->gpio_trans = gpio;
+
+	gpio = devm_gpiod_get(&pdev->dev, "enable", 0);
+	if (!IS_ERR(gpio))
+		master->gpio_enable = gpio;
+
+	gpio = devm_gpiod_get(&pdev->dev, "mux", 0);
+	if (!IS_ERR(gpio))
+		master->gpio_mux = gpio;
+
 	master->master.read = fsi_master_gpio_read;
 	master->master.write = fsi_master_gpio_write;
 	master->master.send_break = fsi_master_gpio_break;
 	master->master.link_enable = fsi_master_gpio_link_enable;
 
+	fsi_master_gpio_init(master);
+
 	return fsi_master_register(&master->master);
 }
 
-- 
1.8.2.2

[PATCH linux v4 19/20] drivers/fsi: Add client driver register utilities

[christopher.lee.bostic]

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

Add driver_register and driver_unregister wrappers for FSI.

V4 - Separated out SCOM client driver registration for a follow
     on patch.

Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 drivers/fsi/fsi-core.c | 17 +++++++++++++++++
 include/linux/fsi.h    |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 185aff4..38935c0 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -459,6 +459,23 @@ static int fsi_bus_match(struct device *dev, struct device_driver *drv)
 	return 0;
 }
 
+int fsidrv_register(struct fsi_driver *fsi_drv)
+{
+	if (!fsi_drv)
+		return -EINVAL;
+	if (!fsi_drv->id_table)
+		return -EINVAL;
+
+	return driver_register(&fsi_drv->drv);
+}
+EXPORT_SYMBOL_GPL(fsidrv_register);
+
+void fsidrv_unregister(struct fsi_driver *fsi_drv)
+{
+	driver_unregister(&fsi_drv->drv);
+}
+EXPORT_SYMBOL_GPL(fsidrv_unregister);
+
 struct bus_type fsi_bus_type = {
 	.name		= "fsi",
 	.match		= fsi_bus_match,
diff --git a/include/linux/fsi.h b/include/linux/fsi.h
index 47af0af..09670b7 100644
--- a/include/linux/fsi.h
+++ b/include/linux/fsi.h
@@ -55,6 +55,9 @@ struct fsi_driver {
 #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 int fsidrv_register(struct fsi_driver *);
+extern void fsidrv_unregister(struct fsi_driver *);
+
 extern struct bus_type fsi_bus_type;
 
 #endif /* LINUX_FSI_H */
-- 
1.8.2.2

[PATCH linux v4 20/20] drivers/fsi: Add SCOM client driver

[christopher.lee.bostic]

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

Add a bare bones SCOM engine device driver that registers
with the FSI bus.

V4 - Add put_scom and get_scom ops

Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 drivers/fsi/Kconfig    |   6 ++
 drivers/fsi/Makefile   |   1 +
 drivers/fsi/fsi-scom.c | 158 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 165 insertions(+)
 create mode 100644 drivers/fsi/fsi-scom.c

diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
index 69e7ee8..719a217 100644
--- a/drivers/fsi/Kconfig
+++ b/drivers/fsi/Kconfig
@@ -24,6 +24,12 @@ config FSI_MASTER_GPIO
 	select GPIO_DEVRES
 	---help---
 	This option enables a FSI master driver, using GPIO lines directly.
+
+config FSI_SCOM
+	tristate "SCOM FSI client"
+	depends on FSI
+	---help---
+	This option enables the SCOM FSI client device driver.
 endif
 
 endmenu
diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
index 2021ce5..3e31d9a 100644
--- a/drivers/fsi/Makefile
+++ b/drivers/fsi/Makefile
@@ -2,3 +2,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
+obj-$(CONFIG_FSI_SCOM) += fsi-scom.o
diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c
new file mode 100644
index 0000000..244e450
--- /dev/null
+++ b/drivers/fsi/fsi-scom.c
@@ -0,0 +1,158 @@
+/*
+ * SCOM FSI Client device 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
+ * MERGCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/fsi.h>
+#include <linux/module.h>
+#include <linux/cdev.h>
+#include <linux/delay.h>
+
+#define FSI_ENGID_SCOM		0x5
+
+#define SCOM_FSI2PIB_DELAY	50
+
+/* SCOM engine register set */
+#define SCOM_DATA0_REG		0x00
+#define SCOM_DATA1_REG		0x04
+#define SCOM_CMD_REG		0x08
+#define SCOM_RESET_REG		0x1C
+
+#define SCOM_RESET_CMD		0x80000000
+#define SCOM_WRITE_CMD		0x80000000
+
+static int scom_probe(struct device *);
+
+struct scom_device {
+	struct fsi_device *fsi_dev;
+	struct cdev *cdev;
+};
+
+struct scom_device scom_dev;
+
+struct fsi_device_id scom_ids[] = {
+	{
+		.engine_type = FSI_ENGID_SCOM,
+		.version = FSI_VERSION_ANY,
+	},
+	{ 0 }
+};
+
+struct fsi_driver scom_drv = {
+	.id_table = scom_ids,
+	.drv = {
+		.name = "scom",
+		.bus = &fsi_bus_type,
+		.probe = scom_probe,
+	}
+};
+
+static int put_scom(uint64_t value, uint32_t addr)
+{
+	int rc;
+	uint32_t data = SCOM_RESET_CMD;
+	struct scom_device *scom = &scom_dev;
+
+	rc = fsi_device_write(scom->fsi_dev, SCOM_RESET_REG, &data,
+				sizeof(uint32_t));
+	if (rc)
+		return rc;
+
+	data = (value >> 32) & 0xffffffff;
+	rc = fsi_device_write(scom->fsi_dev, SCOM_DATA0_REG, &data,
+				sizeof(uint32_t));
+	if (rc)
+		return rc;
+
+	data = value & 0xffffffff;
+	rc = fsi_device_write(scom->fsi_dev, SCOM_DATA1_REG, &data,
+				sizeof(uint32_t));
+	if (rc)
+		return rc;
+
+	data = SCOM_WRITE_CMD | addr;
+	return fsi_device_write(scom->fsi_dev, SCOM_CMD_REG, &data,
+				sizeof(uint32_t));
+}
+
+
+static int get_scom(uint64_t *value, uint32_t addr)
+{
+	uint32_t result, data;
+	struct scom_device *scom = &scom_dev;
+	int rc;
+
+	udelay(SCOM_FSI2PIB_DELAY);
+
+	data = addr;
+	rc = fsi_device_write(scom->fsi_dev, SCOM_CMD_REG, &data,
+				sizeof(uint32_t));
+	if (rc)
+		return rc;
+
+	rc = fsi_device_read(scom->fsi_dev, SCOM_DATA0_REG, &result,
+				sizeof(uint32_t));
+	if (rc)
+		return rc;
+
+	*value |= (uint64_t) result << 32;
+	rc = fsi_device_read(scom->fsi_dev, SCOM_DATA1_REG, &result,
+				sizeof(uint32_t));
+	if (rc)
+		return rc;
+
+	*value |= result;
+
+	return 0;
+}
+
+static int scom_probe(struct device *dev)
+{
+	struct fsi_device *fsi_dev = to_fsi_dev(dev);
+	struct scom_device *scom = &scom_dev;
+	int rc;
+	uint64_t data;
+
+	scom->fsi_dev = fsi_dev;
+	scom->cdev = cdev_alloc();
+	if (!scom->cdev)
+		return -ENOMEM;
+
+	scom->cdev->owner = THIS_MODULE;
+
+	/* todo: fill in and register cdev */
+	/* open, read, write, close file ops */
+
+	/* todo: user space linkage to get_scom / put_scom */
+	/*  engine init */
+
+	/* todo: how to enter test values? */
+	rc = get_scom(&data, 0);
+	data = 0;
+	rc = put_scom(data, 0);
+
+	return 0;
+}
+
+static int scom_init(void)
+{
+	return fsidrv_register(&scom_drv);
+}
+
+static void scom_exit(void)
+{
+	fsidrv_unregister(&scom_drv);
+}
+
+module_init(scom_init);
+module_exit(scom_exit);
-- 
1.8.2.2

Re: [PATCH linux v4 17/20] drivers/fsi: Set slave SMODE to init communications

[Jeremy Kerr]

Hi Chris,

> @@ -301,15 +376,26 @@ static int fsi_master_link_enable(struct fsi_master *master, int link)
>  static int fsi_master_break(struct fsi_master *master, int link)
>  {
>  	int rc;
> +	uint32_t smode;
>  
>  	if (!master->send_break)
>  		return -EINVAL;
>  
> -	rc =  master->send_break(master, link);
> -	if (rc)
> +	rc = master->send_break(master, link);
> +	if (rc) {
>  		dev_info(master->dev, "break failed with:%d\n", rc);
> +		return rc;
> +	}
> +	/* Verify can read slave at default ID location */
> +	rc = master->read(master, link, 3, FSI_SLAVE_BASE + FSI_SMODE,
> +			&smode, sizeof(smode));
>  
> -	return rc;
> +	/* If read fails then nothing on other end of link */
> +	if (rc)
> +		return -ENODEV;
> +
> +	dev_dbg(master->dev, "smode at ID 3 after break:%08x\n", smode);
> +	return 0;
>  }

Should this smode read logic be in fsi_master_break() at all?

I'd be inclined to keep the slave detection separate. If it's necessary
to check for slave presence, we can do that in the caller.

Cheers,


Jeremy

Re: [PATCH linux v4 20/20] drivers/fsi: Add SCOM client driver

[Jeremy Kerr]

Hi Chris,

> +	/* todo: fill in and register cdev */
> +	/* open, read, write, close file ops */
> +
> +	/* todo: user space linkage to get_scom / put_scom */
> +	/*  engine init */

We have a couple of options for this, but I think a simple chardev would
work fine. Were you thinking just a read/write interface, where the
off_t is the SCOM address?

Cheers,


Jeremy

Re: [PATCH linux v4 18/20] drivers/fsi: Add GPIO FSI master functionality

[Jeremy Kerr]

Hi Chris,

> +
> +	pin_clk {
> +		gpios = <ASPEED_GPIO(A, 4)  GPIO_ACTIVE_HIGH>;
> +		output-low;
> +		line-name = "clk";

The names "clk", "data", "trans", "enable" and "mux" are super generic,
and could apply to any of the other bajillion functions on an AST chip.
Should we use fsi-clk, fsi-data, etc?

> +static void serial_in(struct fsi_master_gpio *master, struct fsi_gpio_msg *cmd,
> +			uint8_t num_bits)
> +{
> +	uint8_t bit;
> +	uint64_t msg = 0;
> +	uint8_t in_bit = 0;
> +
> +	set_sda_input(master);
> +	cmd->bits = 0;
> +
> +	for (bit = 0; bit < num_bits; bit++) {
> +		clock_toggle(master, 1);
> +		in_bit = sda_in(master);
> +		cmd->bits++;
> +		msg <<= 1;
> +		msg |= in_bit;
> +	}
> +	cmd->msg = ~msg;		/*  Data is negative active */

Really minor: I'd do that logic inversion when you or-in the bit:

		msg |= ~in_bit & 0x1;

(or even alter sda_in to suit).

This means that that the unused bits (for messages less than 64 bits
long) are left at zero, which will be slightly more readable when
debugging.

> +static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,
> +			uint8_t size, void *data)
> +{

I haven't reviewed this for protocol correctness, as you're the FSI
expert. I assume all is good :)

> +	int busy_count = 0, i;
> +	struct fsi_gpio_msg response, cmd;
> +	int bits_remaining = 0;
> +	uint64_t resp = 0;
> +	uint8_t bits_received = 1 + FSI_GPIO_MSG_ID_SIZE +
> +				FSI_GPIO_MSG_RESPID_SIZE;
> +	uint8_t crc_in;
> +
> +	do {
> +		response.msg = 0;
> +		for (i = 0; i < FSI_GPIO_MTOE_COUNT; i++) {
> +			serial_in(master, &response, 1);
> +			resp = response.msg;
> +			if (response.msg)
> +				break;
> +		}
> +		if (unlikely(i >= FSI_GPIO_MTOE_COUNT)) {

No need for unlikely() here.

> +			dev_info(master->master.dev,
> +			"Master time out waiting for response\n");

If you can (ie., without breaking the string), can you indent this
continuation line?

> +			drain_response(master);
> +			fsi_master_gpio_error(master, FSI_GPIO_MTOE);
> +			return FSI_GPIO_MTOE;

What does the return value for this function represent? What's a 'busy
count' and how does it relate to MTOE?

> +		}
> +
> +		/* Response received */
> +		response.msg = 0;
> +		serial_in(master, &response, FSI_GPIO_MSG_ID_SIZE);
> +		dev_info(master->master.dev, "cfam id:%d\n", (int)response.msg);
> +		resp <<= FSI_GPIO_MSG_ID_SIZE;
> +		resp |= response.msg;
> +
> +		response.msg = 0;
> +		serial_in(master, &response, FSI_GPIO_MSG_RESPID_SIZE);
> +		dev_info(master->master.dev, "response id:%d\n",
> +				(int)response.msg);
> +		resp <<= FSI_GPIO_MSG_RESPID_SIZE;
> +		resp |= response.msg;
> +
> +		switch (response.msg) {
> +		case FSI_GPIO_RESP_ACK:
> +			if (expected == FSI_GPIO_RESP_ACKD)
> +				bits_remaining = 8 * size;
> +			break;
> +
> +		case FSI_GPIO_RESP_BUSY:
> +			/*
> +			 * Its necessary to clock slave before issuing
> +			 * d-poll, not indicated in the hardware protocol
> +			 * spec. < 20 clocks causes slave to hang, 21 ok.
> +			 */
> +			set_sda_output(master, 0);
> +			clock_toggle(master, FSI_GPIO_DPOLL_CLOCKS);
> +			cmd.msg = FSI_GPIO_CMD_DPOLL;
> +			cmd.bits = FSI_GPIO_CMD_DPOLL_SIZE;
> +			serial_out(master, &cmd);
> +			continue;
> +
> +		case FSI_GPIO_RESP_ERRA:
> +		case FSI_GPIO_RESP_ERRC:
> +			dev_info(master->master.dev, "ERR received: %d\n",
> +				(int)response.msg);
> +			drain_response(master);
> +			fsi_master_gpio_error(master, response.msg);
> +			return response.msg;

Now I'm really confused about the return value :/

> +		}
> +
> +		/* Read in the data field if applicable */
> +		if (bits_remaining) {
> +			response.msg = 0;
> +			serial_in(master, &response, bits_remaining);
> +			resp <<= bits_remaining;
> +			resp |= response.msg;
> +			bits_received += bits_remaining;
> +		}
> +
> +		/* Read in the crc and check it */
> +		response.msg = 0;
> +		serial_in(master, &response, FSI_GPIO_CRC_SIZE);

serial_in() should always set response.msg, right? Do we need to set this
before every call?

> +
> +		crc_in = fsi_crc4(0, resp, bits_received);
> +		if (crc_in != response.msg) {
> +			/* CRC's don't match */
> +			dev_info(master->master.dev, "ERR response CRC\n");
> +			fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
> +			return FSI_GPIO_CRC_INVAL;
> +		}
> +		return 0;
> +
> +	} while (busy_count++ < FSI_GPIO_MAX_BUSY);
> +
> +	return busy_count;
> +}
> +
> +static void build_command(struct fsi_gpio_msg *cmd, uint64_t mode,
> +		uint8_t slave, uint32_t addr, size_t size,
> +		const void *data)

Can we indicate that we're building an ABS_AR command here?

[A future optimisation for this driver would be to cache the last address
used, and perform relative reads & writes if suitable...]

> +{
> +	uint8_t crc;
> +	uint64_t msg_with_start;
> +
> +	cmd->bits = FSI_GPIO_CMD_DFLT_LEN;
> +	cmd->msg = FSI_GPIO_CMD_DEFAULT;
> +	cmd->msg |= mode;
> +
> +	/* todo: handle more than just slave id 0 */
> +	cmd->msg &= ~FSI_GPIO_CMD_SLAVE_MASK;
> +
> +	cmd->msg |= (addr << FSI_GPIO_CMD_ADDR_SHIFT);
> +	if (size == sizeof(uint8_t)) {
> +		if (data)
> +			cmd->msg |=
> +				*((uint8_t *)data) >> FSI_GPIO_CMD_DATA_SHIFT;
> +	} else if (size == sizeof(uint16_t)) {
> +		cmd->msg |= FSI_GPIO_CMD_SIZE_16;
> +		if (data)
> +			cmd->msg |=
> +				*((uint16_t *)data) >> FSI_GPIO_CMD_DATA_SHIFT;
> +	} else {
> +		cmd->msg |= FSI_GPIO_CMD_SIZE_32;
> +		if (data)
> +			cmd->msg |=
> +				*((uint32_t *)data) >> FSI_GPIO_CMD_DATA_SHIFT;
> +	}
> +
> +	if (mode == FSI_GPIO_CMD_WRITE)
> +		cmd->bits += (8 * size);
> +
> +	/* Start bit isn't considered part of command but we need to */
> +	/* account for it in crc calcs */
> +	msg_with_start = 0x1 << cmd->bits;
> +	msg_with_start |= cmd->msg;
> +	crc = fsi_crc4(0, msg_with_start, cmd->bits);
> +	cmd->msg |= crc >> cmd->bits;
> +	cmd->bits += FSI_GPIO_CRC_SIZE;
> +}
> +
>  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);
> +	struct fsi_gpio_msg cmd;
>  
>  	if (link != 0)
>  		return -ENODEV;
>  
> -	/* todo: format read into a message, send, poll for response */
> -	(void)master;
> +	build_command(&cmd, FSI_GPIO_CMD_READ, slave, addr, size, NULL);
> +	serial_out(master, &cmd);
> +	echo_delay(master);
>  
> -
> -	return 0;
> +	return poll_for_response(master, FSI_GPIO_RESP_ACKD, size, val);
>  }
>  
>  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);
> +	struct fsi_gpio_msg cmd;
>  
>  	if (link != 0)
>  		return -ENODEV;
>  
> -	/* todo: format write into a message, send, poll for response */
> -	(void)master;
> +	build_command(&cmd, FSI_GPIO_CMD_WRITE, slave, addr, size, val);
> +	serial_out(master, &cmd);
> +	echo_delay(master);
>  
> -	return 0;
> +	return poll_for_response(master, FSI_GPIO_RESP_ACK, size, NULL);
>  }
>  
>  /*
> @@ -60,12 +368,31 @@ static int fsi_master_gpio_break(struct fsi_master *_master, int link)
>  	if (link != 0)
>  		return -ENODEV;
>  
> -	/* todo: send the break pattern over gpio */
> -	(void)master;
> +	set_sda_output(master, 0);
> +	clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
> +	sda_out(master, 1);
> +	clock_toggle(master, FSI_BREAK_CLOCKS);
> +	echo_delay(master);
> +	sda_out(master, 0);
> +	clock_toggle(master, FSI_POST_BREAK_CLOCKS);
> +	udelay(200);		/* Wait for logic reset to take effect */

Minor: Better to put comments like this above their code:

	/* Wait for logic reset to take effect */
	udelay(200);

Cheers,


Jeremy

Re: [PATCH linux v4 00/20] FSI device driver introduction

[Jeremy Kerr]

Hi Chris,

Overall, this series is getting to look much better - I'd say we're in
good shape for upstream submission soon.

I have some further comments on patches, but we should start thinking
about how we'd like to construct the upstream submission. Here's what
I'd suggest:

 - I take the corrections that you've made to my series, and integrate
   them into their relevant original patches, and update the commits to
   indicate that you've also contributed to these

 - I then update my fsi branch on github with the slightly-updated
   series of 13 base patches.

 - Because it's only a skeleton implementation, I drop the
   fsi_master_gpio patch from my series, and you squash (my) 13 with
   (your) 18, which makes a proper master driver. You take authorship of
   the resulting patch.

 - You pull from my github repo, and rebase your 14-20 on top of that.
   Some of those will then be no-ops, as the changes will already be in
   the base series.

How does that sound?

Cheers,


Jeremy

Re: [PATCH linux v4 19/20] drivers/fsi: Add client driver register utilities

[Jeremy Kerr]

Hi Chris,

> +int fsidrv_register(struct fsi_driver *fsi_drv)
> +{
> +	if (!fsi_drv)
> +		return -EINVAL;
> +	if (!fsi_drv->id_table)
> +		return -EINVAL;
> +
> +	return driver_register(&fsi_drv->drv);
> +}
> +EXPORT_SYMBOL_GPL(fsidrv_register);
> +
> +void fsidrv_unregister(struct fsi_driver *fsi_drv)
> +{
> +	driver_unregister(&fsi_drv->drv);
> +}
> +EXPORT_SYMBOL_GPL(fsidrv_unregister);
> +

Minor, but I think fsi_driver_register / fsi_driver_unregister
would be better names; we don't use 'fsidrv' elsewhere.

Do we want a module_fsi_driver macro (similar to module_platform_driver)
to reduce boilerplate code?

Cheers,


Jeremy

Re: [PATCH linux v4 00/20] FSI device driver introduction

[Joel Stanley]

Hey Chris,

On Mon, Oct 17, 2016 at 5:58 PM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Overall, this series is getting to look much better - I'd say we're in
> good shape for upstream submission soon.

I second this! You and Jeremy have made some good progress this past
week or two. Great work from both of you.

I came the OpenFSI spec[1] on the OpenPower Foundation website the
other day. I suggest you include a reference to it in your commit
message and cover letter so others can use it as a reference.

The fact that it's publicly available is particularly useful when
making the upstream submission, as it allows others to check our
implementation against a public specification.

[1] http://openpowerfoundation.org/wp-content/uploads/resources/OpenFSI-spec/content/ch_preface.html

Cheers,

Joel

Re: [PATCH linux v4 17/20] drivers/fsi: Set slave SMODE to init communications

[Christopher Bostic]

On Mon, Oct 17, 2016 at 2:04 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>> @@ -301,15 +376,26 @@ static int fsi_master_link_enable(struct fsi_master *master, int link)
>>  static int fsi_master_break(struct fsi_master *master, int link)
>>  {
>>       int rc;
>> +     uint32_t smode;
>>
>>       if (!master->send_break)
>>               return -EINVAL;
>>
>> -     rc =  master->send_break(master, link);
>> -     if (rc)
>> +     rc = master->send_break(master, link);
>> +     if (rc) {
>>               dev_info(master->dev, "break failed with:%d\n", rc);
>> +             return rc;
>> +     }
>> +     /* Verify can read slave at default ID location */
>> +     rc = master->read(master, link, 3, FSI_SLAVE_BASE + FSI_SMODE,
>> +                     &smode, sizeof(smode));
>>
>> -     return rc;
>> +     /* If read fails then nothing on other end of link */
>> +     if (rc)
>> +             return -ENODEV;
>> +
>> +     dev_dbg(master->dev, "smode at ID 3 after break:%08x\n", smode);
>> +     return 0;
>>  }
>
> Should this smode read logic be in fsi_master_break() at all?
>
> I'd be inclined to keep the slave detection separate. If it's necessary
> to check for slave presence, we can do that in the caller.

Hi Jeremy,

OK, will move.

Thanks,
-Chris

>
> Cheers,
>
>
> Jeremy

Re: [PATCH linux v4 20/20] drivers/fsi: Add SCOM client driver

[Christopher Bostic]

On Mon, Oct 17, 2016 at 2:27 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>> +     /* todo: fill in and register cdev */
>> +     /* open, read, write, close file ops */
>> +
>> +     /* todo: user space linkage to get_scom / put_scom */
>> +     /*  engine init */
>
> We have a couple of options for this, but I think a simple chardev would
> work fine. Were you thinking just a read/write interface, where the
> off_t is the SCOM address?
>

Hi Jeremy,

Yes, the idea was to create the char dev and use the read/write syscalls.
Might be a good topic for discussion in tonight's call.  Want to make sure
this adheres to the general openBMC plan for user space access to
the kernel.

Thanks,
-Chris

> Cheers,
>
>
> Jeremy

Re: [PATCH linux v4 18/20] drivers/fsi: Add GPIO FSI master functionality

[Christopher Bostic]

On Mon, Oct 17, 2016 at 2:28 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>> +
>> +     pin_clk {
>> +             gpios = <ASPEED_GPIO(A, 4)  GPIO_ACTIVE_HIGH>;
>> +             output-low;
>> +             line-name = "clk";
>
> The names "clk", "data", "trans", "enable" and "mux" are super generic,
> and could apply to any of the other bajillion functions on an AST chip.
> Should we use fsi-clk, fsi-data, etc?
>

Hi Jeremy,

Will rename as suggested.

>> +static void serial_in(struct fsi_master_gpio *master, struct fsi_gpio_msg *cmd,
>> +                     uint8_t num_bits)
>> +{
>> +     uint8_t bit;
>> +     uint64_t msg = 0;
>> +     uint8_t in_bit = 0;
>> +
>> +     set_sda_input(master);
>> +     cmd->bits = 0;
>> +
>> +     for (bit = 0; bit < num_bits; bit++) {
>> +             clock_toggle(master, 1);
>> +             in_bit = sda_in(master);
>> +             cmd->bits++;
>> +             msg <<= 1;
>> +             msg |= in_bit;
>> +     }
>> +     cmd->msg = ~msg;                /*  Data is negative active */
>
> Really minor: I'd do that logic inversion when you or-in the bit:
>
>                 msg |= ~in_bit & 0x1;
>
> (or even alter sda_in to suit).
>
> This means that that the unused bits (for messages less than 64 bits
> long) are left at zero, which will be slightly more readable when
> debugging.
>

OK, will change.

>> +static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,
>> +                     uint8_t size, void *data)
>> +{
>
> I haven't reviewed this for protocol correctness, as you're the FSI
> expert. I assume all is good :)
>

This works on prototype hardware per previous implementation.

>> +     int busy_count = 0, i;
>> +     struct fsi_gpio_msg response, cmd;
>> +     int bits_remaining = 0;
>> +     uint64_t resp = 0;
>> +     uint8_t bits_received = 1 + FSI_GPIO_MSG_ID_SIZE +s
>> +                             FSI_GPIO_MSG_RESPID_SIZE;
>> +     uint8_t crc_in;
>> +
>> +     do {
>> +             response.msg = 0;
>> +             for (i = 0; i < FSI_GPIO_MTOE_COUNT; i++) {
>> +                     serial_in(master, &response, 1);
>> +                     resp = response.msg;
>> +                     if (response.msg)
>> +                             break;
>> +             }
>> +             if (unlikely(i >= FSI_GPIO_MTOE_COUNT)) {
>
> No need for unlikely() here.
>

Will remove.

>> +                     dev_info(master->master.dev,
>> +                     "Master time out waiting for response\n");
>
> If you can (ie., without breaking the string), can you indent this
> continuation line?
>

OK, will indent.

>> +                     drain_response(master);
>> +                     fsi_master_gpio_error(master, FSI_GPIO_MTOE);
>> +                     return FSI_GPIO_MTOE;
>
> What does the return value for this function represent? What's a 'busy
> count' and how does it relate to MTOE?
>

Busy in this case means the slave replied with 'BUSY' too many times and
never responded with anything else.  MTOE means the slave never responded
at all to any requests for information.

The idea is to have poll_for_response( ) return non-zero when some form
of response failure occurs.   The types of failures reported are:

Master timeout error (MTOE) - No response at all from slave after x attempts
Any Error (ERRA) - Slave returns an ERRA as response meaning you need to
   query the slave engine registers to find out what went wrong
CRC Error (ERRC) - Slave indicates that it didn't like the crc field the master
   sent.
CRC invalid - Master receives slave's crc and thinks its a garbled message
BUSY -  Slave responds with busy on d-poll and never responds with
   anything else.

One thing I can make clearer and will fix is that instead of returning
'busy_count'
I'll just return the literal FSI_GPIO_MAX_BUSY


>> +             }
>> +
>> +             /* Response received */
>> +             response.msg = 0;
>> +             serial_in(master, &response, FSI_GPIO_MSG_ID_SIZE);
>> +             dev_info(master->master.dev, "cfam id:%d\n", (int)response.msg);
>> +             resp <<= FSI_GPIO_MSG_ID_SIZE;
>> +             resp |= response.msg;
>> +
>> +             response.msg = 0;
>> +             serial_in(master, &response, FSI_GPIO_MSG_RESPID_SIZE);
>> +             dev_info(master->master.dev, "response id:%d\n",
>> +                             (int)response.msg);
>> +             resp <<= FSI_GPIO_MSG_RESPID_SIZE;
>> +             resp |= response.msg;
>> +
>> +             switch (response.msg) {
>> +             case FSI_GPIO_RESP_ACK:
>> +                     if (expected == FSI_GPIO_RESP_ACKD)
>> +                             bits_remaining = 8 * size;
>> +                     break;
>> +
>> +             case FSI_GPIO_RESP_BUSY:
>> +                     /*
>> +                      * Its necessary to clock slave before issuing
>> +                      * d-poll, not indicated in the hardware protocol
>> +                      * spec. < 20 clocks causes slave to hang, 21 ok.
>> +                      */
>> +                     set_sda_output(master, 0);
>> +                     clock_toggle(master, FSI_GPIO_DPOLL_CLOCKS);
>> +                     cmd.msg = FSI_GPIO_CMD_DPOLL;
>> +                     cmd.bits = FSI_GPIO_CMD_DPOLL_SIZE;
>> +                     serial_out(master, &cmd);
>> +                     continue;
>> +
>> +             case FSI_GPIO_RESP_ERRA:
>> +             case FSI_GPIO_RESP_ERRC:
>> +                     dev_info(master->master.dev, "ERR received: %d\n",
>> +                             (int)response.msg);
>> +                     drain_response(master);
>> +                     fsi_master_gpio_error(master, response.msg);
>> +                     return response.msg;
>
> Now I'm really confused about the return value :/

response.msg is going to be either ERRA or ERRC,  I just return the variable
so I don't have to do a check for what type of error.

>
>> +             }
>> +
>> +             /* Read in the data field if applicable */
>> +             if (bits_remaining) {
>> +                     response.msg = 0;
>> +                     serial_in(master, &response, bits_remaining);
>> +                     resp <<= bits_remaining;
>> +                     resp |= response.msg;
>> +                     bits_received += bits_remaining;
>> +             }
>> +
>> +             /* Read in the crc and check it */
>> +             response.msg = 0;
>> +             serial_in(master, &response, FSI_GPIO_CRC_SIZE);
>
> serial_in() should always set response.msg, right? Do we need to set this
> before every call?
>

True, will remove those.

>> +
>> +             crc_in = fsi_crc4(0, resp, bits_received);
>> +             if (crc_in != response.msg) {
>> +                     /* CRC's don't match */
>> +                     dev_info(master->master.dev, "ERR response CRC\n");
>> +                     fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
>> +                     return FSI_GPIO_CRC_INVAL;
>> +             }
>> +             return 0;
>> +
>> +     } while (busy_count++ < FSI_GPIO_MAX_BUSY);
>> +
>> +     return busy_count;
>> +}
>> +
>> +static void build_command(struct fsi_gpio_msg *cmd, uint64_t mode,
>> +             uint8_t slave, uint32_t addr, size_t size,
>> +             const void *data)
>
> Can we indicate that we're building an ABS_AR command here?
>

OK, will change the name to build_abs_ar_command().

> [A future optimisation for this driver would be to cache the last address
> used, and perform relative reads & writes if suitable...]
>


>> @@ -60,12 +368,31 @@ static int fsi_master_gpio_break(struct fsi_master *_master, int link)
>>       if (link != 0)
>>               return -ENODEV;
>>
>> -     /* todo: send the break pattern over gpio */
>> -     (void)master;
>> +     set_sda_output(master, 0);
>> +     clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
>> +     sda_out(master, 1);
>> +     clock_toggle(master, FSI_BREAK_CLOCKS);
>> +     echo_delay(master);
>> +     sda_out(master, 0);
>> +     clock_toggle(master, FSI_POST_BREAK_CLOCKS);
>> +     udelay(200);            /* Wait for logic reset to take effect */
>
> Minor: Better to put comments like this above their code:
>
>         /* Wait for logic reset to take effect */
>         udelay(200);
>

OK, will change.

Thanks,
-Chris


> Cheers,
>
>
> Jeremy

Re: [PATCH linux v4 00/20] FSI device driver introduction

[Christopher Bostic]

On Mon, Oct 17, 2016 at 2:28 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
> Overall, this series is getting to look much better - I'd say we're in
> good shape for upstream submission soon.
>
> I have some further comments on patches, but we should start thinking
> about how we'd like to construct the upstream submission. Here's what
> I'd suggest:
>
>  - I take the corrections that you've made to my series, and integrate
>    them into their relevant original patches, and update the commits to
>    indicate that you've also contributed to these

Hi Jeremy,

Would this mean doing a git commit amend on your patches?

>
>  - I then update my fsi branch on github with the slightly-updated
>    series of 13 base patches.
>
>  - Because it's only a skeleton implementation, I drop the
>    fsi_master_gpio patch from my series, and you squash (my) 13 with
>    (your) 18, which makes a proper master driver. You take authorship of
>    the resulting patch.
>
>  - You pull from my github repo, and rebase your 14-20 on top of that.
>    Some of those will then be no-ops, as the changes will already be in
>    the base series.
>
> How does that sound?

Sounds like a plan -Thanks
Chris

>
> Cheers,
>
>
> Jeremy

Re: [PATCH linux v4 19/20] drivers/fsi: Add client driver register utilities

[Christopher Bostic]

On Mon, Oct 17, 2016 at 3:12 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>> +int fsidrv_register(struct fsi_driver *fsi_drv)
>> +{
>> +     if (!fsi_drv)
>> +             return -EINVAL;
>> +     if (!fsi_drv->id_table)
>> +             return -EINVAL;
>> +
>> +     return driver_register(&fsi_drv->drv);
>> +}
>> +EXPORT_SYMBOL_GPL(fsidrv_register);
>> +
>> +void fsidrv_unregister(struct fsi_driver *fsi_drv)
>> +{
>> +     driver_unregister(&fsi_drv->drv);
>> +}
>> +EXPORT_SYMBOL_GPL(fsidrv_unregister);
>> +
>
> Minor, but I think fsi_driver_register / fsi_driver_unregister
> would be better names; we don't use 'fsidrv' elsewhere.

Hi Jeremy,

OK will change.

>
> Do we want a module_fsi_driver macro (similar to module_platform_driver)
> to reduce boilerplate code?
>

Will add.

Thanks,
Chris

> Cheers,
>
>
> Jeremy

Re: [PATCH linux v4 00/20] FSI device driver introduction

[Christopher Bostic]

On Mon, Oct 17, 2016 at 5:18 AM, Joel Stanley <joel@jms.id.au> wrote:
> Hey Chris,
>
> On Mon, Oct 17, 2016 at 5:58 PM, Jeremy Kerr <jk@ozlabs.org> wrote:
>> Overall, this series is getting to look much better - I'd say we're in
>> good shape for upstream submission soon.
>
> I second this! You and Jeremy have made some good progress this past
> week or two. Great work from both of you.
>
> I came the OpenFSI spec[1] on the OpenPower Foundation website the
> other day. I suggest you include a reference to it in your commit
> message and cover letter so others can use it as a reference.
>
> The fact that it's publicly available is particularly useful when
> making the upstream submission, as it allows others to check our
> implementation against a public specification.
>
> [1] http://openpowerfoundation.org/wp-content/uploads/resources/OpenFSI-spec/content/ch_preface.html
>

Hi Joel,

Sure, will add that.

Thanks,
Chris

> Cheers,
>
> Joel

Re: [PATCH linux v4 00/20] FSI device driver introduction

[Jeremy Kerr]

Hi Chris,

>> I have some further comments on patches, but we should start thinking
>> about how we'd like to construct the upstream submission. Here's what
>> I'd suggest:
>>
>>  - I take the corrections that you've made to my series, and integrate
>>    them into their relevant original patches, and update the commits to
>>    indicate that you've also contributed to these
> 
> Would this mean doing a git commit amend on your patches?

Kinda. I'll be doing a `rebase -i` and squashing the fixes into their
relevant (original) changes.

>> How does that sound?
> 
> Sounds like a plan -Thanks

Excellent! I'll get that done today, and let you know when my branch is
ready to re-pull.

Cheers,


Jeremy

Re: [PATCH linux v4 00/20] FSI device driver introduction

[Jeremy Kerr]

Hi Chris & Joel,

> I came the OpenFSI spec[1] on the OpenPower Foundation website the
> other day. I suggest you include a reference to it in your commit
> message and cover letter so others can use it as a reference.
> 
> The fact that it's publicly available is particularly useful when
> making the upstream submission, as it allows others to check our
> implementation against a public specification.
> 
> [1] http://openpowerfoundation.org/wp-content/uploads/resources/OpenFSI-spec/content/ch_preface.html

Good idea.

Chris: you also have some useful info in your cover letter (00/20),
under the Common FSI Terminology section.  However, this will be lost
once the series is merged. So, I'd suggest putting that info in a new
patch to add Documentation/fsi.txt, with the link above, and a little
introductory text.

Cheers,


Jeremy

Re: [PATCH linux v4 00/20] FSI device driver introduction

[Jeremy Kerr]

Hi Chris,

>> Sounds like a plan -Thanks
> 
> Excellent! I'll get that done today, and let you know when my branch is
> ready to re-pull.

OK, done. The branch at:

  https://github.com/jk-ozlabs/linux/tree/openbmc/fsi

Now has an updated base set of commits for you to rebase onto. To do
that:

   git rebase -i --onto 2b5b3ae2 <SHA>

Where <SHA> is the start of your local changes in your tree (ie,
corresponding to 16/20). In that rebase, you'll probably want to pick
111f387 (my skeleton GPIO driver), which should still be an object in
your tree, and squash 18/20 into that.

Something like this for the rebase plan:

  pick cfa927f fsi: Add empty fsi bus definitions
  pick fd18ef2 fsi: Add device & driver definitions
  pick 05d2f6d fsi: add driver to device matches
  pick 918007d fsi: Add fsi master definition
  pick 1493896 fsi: Add fake master driver
  pick cd8c30f fsi: enable debug
  pick 287b519 fsi: Add slave definition
  pick db6d868 fsi: Add empty master scan
  pick 60e3513 fsi: Add crc4 helpers
  pick 57314b2 fsi: Implement slave initialisation
  pick 47a25ff fsi: scan slaves & register devices
  pick 2b5b3ae fsi: Add device read/write/peek functions
  pick <16/20> drivers/fsi: Set up links for slave communication
  pick <17/20> drivers/fsi: Set slave SMODE to init communications
  pick 111f387 fsi: Add GPIO master driver
  squash <18/20> drivers/fsi: Add GPIO FSI master functionality
  pick <19/20> drivers/fsi: Add client driver register utilities
  pick <20/20> drivers/fsi: Add SCOM client driver

Where <xx/20> is the actual SHA of the commit, of course. These will
automatically be provided by the rebase.

Then, when squashing the GPIO patch, set the author to you, keep my
signed-off-by, and reword the commit message to the entire change.

Cheers,


Jeremy

Re: [PATCH linux v4 00/20] FSI device driver introduction

[Christopher Bostic]

On Mon, Oct 17, 2016 at 8:56 PM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>>> Sounds like a plan -Thanks
>>
>> Excellent! I'll get that done today, and let you know when my branch is
>> ready to re-pull.
>
> OK, done. The branch at:
>
>   https://github.com/jk-ozlabs/linux/tree/openbmc/fsi
>
> Now has an updated base set of commits for you to rebase onto. To do
> that:
>
>    git rebase -i --onto 2b5b3ae2 <SHA>
>
> Where <SHA> is the start of your local changes in your tree (ie,
> corresponding to 16/20). In that rebase, you'll probably want to pick
> 111f387 (my skeleton GPIO driver), which should still be an object in
> your tree, and squash 18/20 into that.
>
> Something like this for the rebase plan:
>
>   pick cfa927f fsi: Add empty fsi bus definitions
>   pick fd18ef2 fsi: Add device & driver definitions
>   pick 05d2f6d fsi: add driver to device matches
>   pick 918007d fsi: Add fsi master definition
>   pick 1493896 fsi: Add fake master driver
>   pick cd8c30f fsi: enable debug
>   pick 287b519 fsi: Add slave definition
>   pick db6d868 fsi: Add empty master scan
>   pick 60e3513 fsi: Add crc4 helpers

Hi Jeremy,

Not sure what may have happened but looks like the crc4 helper
commit 60e3513 has a build issue.

Changes made to correct:
* Added a semi colon for function prototype in fsi-master.h
* Removed the open bracket in for loop of fsi_crc4
* Renamed two instances of check_crc4 to check_crc4_u32

I had made these updates in the last patch round (v4) but I figured
you may want to have those updates in place prior to me applying 16-20.

Thanks,
Chris

>   pick 57314b2 fsi: Implement slave initialisation
>   pick 47a25ff fsi: scan slaves & register devices
>   pick 2b5b3ae fsi: Add device read/write/peek functions
>   pick <16/20> drivers/fsi: Set up links for slave communication
>   pick <17/20> drivers/fsi: Set slave SMODE to init communications
>   pick 111f387 fsi: Add GPIO master driver
>   squash <18/20> drivers/fsi: Add GPIO FSI master functionality
>   pick <19/20> drivers/fsi: Add client driver register utilities
>   pick <20/20> drivers/fsi: Add SCOM client driver
>
> Where <xx/20> is the actual SHA of the commit, of course. These will
> automatically be provided by the rebase.
>
> Then, when squashing the GPIO patch, set the author to you, keep my
> signed-off-by, and reword the commit message to the entire change.
>
> Cheers,
>
>
> Jeremy

Re: [PATCH linux v4 00/20] FSI device driver introduction

[Jeremy Kerr]

Hi Chris,

> Not sure what may have happened but looks like the crc4 helper
> commit 60e3513 has a build issue.

Sorry about that, looks like I'd picked from an older branch. I've fixed
this up and re-pushed, top commit is now 6e5d6ba0.

Cheers,


Jeremy