[RFC linux dev-4.7 0/4] drivers: i2c: FSI-attached I2C master algorithm

[RFC linux dev-4.7 0/4] drivers: i2c: FSI-attached I2C master algorithm

[Eddie James]

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

This patch series adds an I2C bus algorithm to drive the I2C master located on
POWER CPUs. The master is accessed over FSI bus from the service processor.

The driver is functional for some basic read/write ops. There are a couple of
details regarding the device tree and chardev entries that need working out.

Mainly, do we want a char device for each port? There are 15 ports off one P9
master, so that'll be a lot of /dev/i2c-<x> entries. One issue is that
i2c_add_adapter always names it's device i2c-%d, regardless of adapter name.
Furthermore, you can specify the number to label it with
i2c_add_numbered_adapter, but how to prevent collisions with regular i2c
devices?

The driver will currently set up one chardev for each master (each proc).
The port is then specified by hacking the addr field of i2c_msg. It works,
but not a clean solution.

FSI clocking details are not defined in the device tree, so FSI clock is
hardcoded for now.

Edward A. James (4):
  drivers: fsi: Add function to get FSI clock rate
  drivers: i2c: Add FSI-attached I2C master algorithm
  drivers: i2c: Add transfer implementation for FSI algorithm
  drivers: i2c: Add bus recovery for FSI algorithm

 drivers/Makefile             |   2 +-
 drivers/fsi/fsi-core.c       |   7 +
 drivers/i2c/busses/Kconfig   |  11 +
 drivers/i2c/busses/Makefile  |   1 +
 drivers/i2c/busses/i2c-fsi.c | 608 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/fsi.h          |   1 +
 6 files changed, 629 insertions(+), 1 deletion(-)
 create mode 100644 drivers/i2c/busses/i2c-fsi.c

-- 
1.8.3.1

[RFC linux dev-4.7 1/5] drivers: fsi: Add function to get FSI clock rate

[Eddie James]

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

client drivers (I2C, etc) may need to know FSI bus clock rate.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
 drivers/fsi/fsi-core.c | 7 +++++++
 include/linux/fsi.h    | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 3d382e6..492c9e9 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -139,6 +139,13 @@ int fsi_device_peek(struct fsi_device *dev, void *val)
 	return fsi_slave_read(dev->slave, addr, val, sizeof(uint32_t));
 }
 
+unsigned long fsi_device_get_clk_rate(struct fsi_device *dev)
+{
+	/* TODO: get clk from dts */
+	return 8333333;
+}
+EXPORT_SYMBOL_GPL(fsi_device_get_clk_rate);
+
 static void fsi_device_release(struct device *_device)
 {
 	struct fsi_device *device = to_fsi_dev(_device);
diff --git a/include/linux/fsi.h b/include/linux/fsi.h
index d22d0c5..57c8fdb 100644
--- a/include/linux/fsi.h
+++ b/include/linux/fsi.h
@@ -35,6 +35,7 @@ extern int fsi_device_read(struct fsi_device *dev, uint32_t addr,
 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);
+extern unsigned long fsi_device_get_clk_rate(struct fsi_device *dev);
 
 struct fsi_device_id {
 	u8	engine_type;
-- 
1.8.3.1

[RFC linux dev-4.7 2/5] drivers: i2c: Add FSI-attached I2C master algorithm

[Eddie James]

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

Initial startup code for the I2C algorithm to drive the I2C master
located on POWER CPUs over FSI bus.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
 drivers/Makefile             |   2 +-
 drivers/i2c/busses/Kconfig   |  11 ++
 drivers/i2c/busses/Makefile  |   1 +
 drivers/i2c/busses/i2c-fsi.c | 302 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 315 insertions(+), 1 deletion(-)
 create mode 100644 drivers/i2c/busses/i2c-fsi.c

diff --git a/drivers/Makefile b/drivers/Makefile
index 26f18ac..b1db6d3 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -103,6 +103,7 @@ obj-$(CONFIG_SERIO)		+= input/serio/
 obj-$(CONFIG_GAMEPORT)		+= input/gameport/
 obj-$(CONFIG_INPUT)		+= input/
 obj-$(CONFIG_RTC_LIB)		+= rtc/
+obj-$(CONFIG_FSI)		+= fsi/
 obj-y				+= i2c/ media/
 obj-$(CONFIG_PPS)		+= pps/
 obj-$(CONFIG_PTP_1588_CLOCK)	+= ptp/
@@ -173,4 +174,3 @@ 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/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 835480b..c248eded 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1210,4 +1210,15 @@ config I2C_OPAL
 	  This driver can also be built as a module. If so, the module will be
 	  called as i2c-opal.
 
+config I2C_FSI
+	tristate "FSI I2C driver"
+	depends on FSI
+	help
+	  Driver for FSI bus attached I2C masters. These are I2C masters that
+	  are connected to the system over a FSI bus, instead of the more
+	  common PCI or MMIO interface.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called as i2c-fsi.
+
 endmenu
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 49631cd..b74b585 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -119,5 +119,6 @@ obj-$(CONFIG_I2C_PCA_ISA)	+= i2c-pca-isa.o
 obj-$(CONFIG_I2C_SIBYTE)	+= i2c-sibyte.o
 obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o
 obj-$(CONFIG_SCx200_ACB)	+= scx200_acb.o
+obj-$(CONFIG_I2C_FSI)		+= i2c-fsi.o
 
 ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
new file mode 100644
index 0000000..c47cd234
--- /dev/null
+++ b/drivers/i2c/busses/i2c-fsi.c
@@ -0,0 +1,302 @@
+/*
+ * Copyright 2017 IBM Corporation
+ *
+ * Eddie James <eajames@us.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/fsi.h>
+#include <linux/i2c.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/sched.h>
+
+#define FSI_ENGID_I2CM		0x7
+
+/* Find left shift from first set bit in m */
+#define MASK_TO_LSH(m)          (__builtin_ffsll(m) - 1ULL)
+
+/* Extract field m from v */
+#define GETFIELD(m, v)          (((v) & (m)) >> MASK_TO_LSH(m))
+
+/* Set field m of v to val */
+#define SETFIELD(m, v, val)                             \
+	(((v) & ~(m)) | ((((typeof(v))(val)) << MASK_TO_LSH(m)) & (m)))
+
+#define I2C_DEFAULT_CLK_DIV	6
+#define I2C_DEFAULT_CLK		333333
+#define I2C_ADDR_PORT		0xff00
+#define I2C_BOE_FIFO_HI_LVL	4
+#define I2C_BOE_FIFO_LO_LVL	4
+
+/* i2c registers */
+#define I2C_BOE_FIFO		0x00
+#define I2C_BOE_CMD		0x04
+#define I2C_BOE_MODE		0x08
+#define I2C_BOE_WATER_MARK	0x0C
+#define I2C_BOE_INT_MASK	0x10
+#define I2C_BOE_INT_COND	0x14
+#define I2C_BOE_OR_INT_MASK	0x14
+#define I2C_BOE_INTS		0x18
+#define I2C_BOE_AND_INT_MASK	0x18
+#define I2C_BOE_STAT		0x1C
+#define I2C_BOE_RESET_I2C	0x1C
+#define I2C_BOE_ESTAT		0x20
+#define I2C_BOE_RESET_ERR	0x20
+#define I2C_BOE_RESID_LEN	0x24
+#define I2C_BOE_SET_SCL		0x24
+#define I2C_BOE_PORT_BUSY	0x28
+#define I2C_BOE_RESET_SCL	0x2C
+#define I2C_BOE_SET_SDA		0x30
+#define I2C_BOE_RESET_SDA	0x34
+
+/* cmd register */
+#define I2C_BOE_CMD_WITH_START	0x80000000
+#define I2C_BOE_CMD_WITH_ADDR	0x40000000
+#define I2C_BOE_CMD_RD_CONT	0x20000000
+#define I2C_BOE_CMD_WITH_STOP	0x10000000
+#define I2C_BOE_CMD_FORCELAUNCH	0x08000000
+#define I2C_BOE_CMD_ADDR	0x00fe0000
+#define I2C_BOE_CMD_READ	0x00010000
+#define I2C_BOE_CMD_LEN		0x0000ffff
+
+/* mode register */
+#define I2C_BOE_MODE_CLKDIV	0xffff0000
+#define I2C_BOE_MODE_PORT	0x0000fc00
+#define I2C_BOE_MODE_ENHANCED	0x00000008
+#define I2C_BOE_MODE_DIAG	0x00000004
+#define I2C_BOE_MODE_PACE_ALLOW	0x00000002
+#define I2C_BOE_MODE_WRAP	0x00000001
+
+/* watermark register */
+#define I2C_BOE_WATERMARK_HI	0x0000f000
+#define I2C_BOE_WATERMARK_LO	0x000000f0
+
+/* interrupt register */
+#define I2C_BOE_INT_INV_CMD	0x00008000
+#define I2C_BOE_INT_PARITY	0x00004000
+#define I2C_BOE_INT_BE_OVERRUN	0x00002000
+#define I2C_BOE_INT_BE_ACCESS	0x00001000
+#define I2C_BOE_INT_LOST_ARB	0x00000800
+#define I2C_BOE_INT_NACK	0x00000400
+#define I2C_BOE_INT_DAT_REQ	0x00000200
+#define I2C_BOE_INT_CMD_COMP	0x00000100
+#define I2C_BOE_INT_STOP_ERR	0x00000080
+#define I2C_BOE_INT_BUSY	0x00000040
+#define I2C_BOE_INT_IDLE	0x00000020
+
+#define I2C_BOE_INT_ENABLE	0x0000ff80
+#define I2C_BOE_INT_ERR		0x0000fcc0
+
+/* status register */
+#define I2C_BOE_STAT_INV_CMD	0x80000000
+#define I2C_BOE_STAT_PARITY	0x40000000
+#define I2C_BOE_STAT_BE_OVERRUN	0x20000000
+#define I2C_BOE_STAT_BE_ACCESS	0x10000000
+#define I2C_BOE_STAT_LOST_ARB	0x08000000
+#define I2C_BOE_STAT_NACK	0x04000000
+#define I2C_BOE_STAT_DAT_REQ	0x02000000
+#define I2C_BOE_STAT_CMD_COMP	0x01000000
+#define I2C_BOE_STAT_STOP_ERR	0x00800000
+#define I2C_BOE_STAT_MAX_PORT	0x000f0000
+#define I2C_BOE_STAT_ANY_INT	0x00008000
+#define I2C_BOE_STAT_SCL_IN	0x00000800
+#define I2C_BOE_STAT_SDA_IN	0x00000400
+#define I2C_BOE_STAT_PORT_BUSY	0x00000200
+#define I2C_BOE_STAT_SELF_BUSY	0x00000100
+#define I2C_BOE_STAT_FIFO_COUNT	0x000000ff
+
+#define I2C_BOE_STAT_ERR	0xfc800000
+#define I2C_BOE_STAT_ANY_RESP	0xff800000
+
+/* extended status register */
+#define I2C_BOE_ESTAT_FIFO_SZ	0xff000000
+#define I2C_BOE_ESTAT_SCL_IN_SY	0x00008000
+#define I2C_BOE_ESTAT_SDA_IN_SY	0x00004000
+#define I2C_BOE_ESTAT_S_SCL	0x00002000
+#define I2C_BOE_ESTAT_S_SDA	0x00001000
+#define I2C_BOE_ESTAT_M_SCL	0x00000800
+#define I2C_BOE_ESTAT_M_SDA	0x00000400
+#define I2C_BOE_ESTAT_HI_WATER	0x00000200
+#define I2C_BOE_ESTAT_LO_WATER	0x00000100
+#define I2C_BOE_ESTAT_PORT_BUSY	0x00000080
+#define I2C_BOE_ESTAT_SELF_BUSY	0x00000040
+#define I2C_BOE_ESTAT_VERSION	0x0000001f
+
+struct fsi_i2c_bus {
+	struct i2c_adapter	adapter;
+	struct fsi_device	*fsi;
+	u8			fifo_size;
+	unsigned long		clk;
+};
+
+static unsigned long fsi_i2c_get_clk_div(struct fsi_i2c_bus *i2c)
+{
+	unsigned long clk_div = I2C_DEFAULT_CLK_DIV;
+	unsigned long fsi_clk = fsi_device_get_clk_rate(i2c->fsi);
+
+	if (fsi_clk > i2c->clk)
+		clk_div = ((fsi_clk / i2c->clk) - 1) / 4;
+
+	return clk_div;
+}
+
+static int fsi_i2c_dev_init(struct fsi_i2c_bus *i2c)
+{
+	int rc;
+	unsigned long clk_div;
+	u32 mode = I2C_BOE_MODE_ENHANCED, extended_status, watermark = 0;
+	u32 interrupt = 0;
+
+	/* since we use polling, disable interrupts */
+	rc = fsi_device_write(i2c->fsi, I2C_BOE_INT_MASK, &interrupt,
+			      sizeof(interrupt));
+	if (rc)
+		return rc;
+
+	clk_div = fsi_i2c_get_clk_div(i2c);
+	mode = SETFIELD(I2C_BOE_MODE_CLKDIV, mode, clk_div);
+	rc = fsi_device_write(i2c->fsi, I2C_BOE_MODE, &mode, sizeof(mode));
+	if (rc)
+		return rc;
+
+	rc = fsi_device_read(i2c->fsi, I2C_BOE_ESTAT, &extended_status,
+			     sizeof(extended_status));
+	if (rc)
+		return rc;
+
+	i2c->fifo_size = GETFIELD(I2C_BOE_ESTAT_FIFO_SZ, extended_status);
+	watermark = SETFIELD(I2C_BOE_WATERMARK_HI, watermark,
+			     i2c->fifo_size - I2C_BOE_FIFO_HI_LVL);
+	watermark = SETFIELD(I2C_BOE_WATERMARK_LO, watermark,
+			     I2C_BOE_FIFO_LO_LVL);
+
+	rc = fsi_device_write(i2c->fsi, I2C_BOE_WATER_MARK, &watermark,
+			      sizeof(watermark));
+
+	return rc;
+}
+
+static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+			int num)
+{
+	return -ENOSYS;
+}
+
+static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_10BIT_ADDR;
+}
+
+static const struct i2c_algorithm fsi_i2c_algorithm = {
+	.master_xfer = fsi_i2c_xfer,
+	.functionality = fsi_i2c_functionality,
+};
+
+static const char *compatible_strings[] = {
+	"ibm,power8-i2cm",
+	"ibm,power9-i2cm",
+};
+
+static int fsi_i2c_probe(struct device *dev)
+{
+	struct fsi_i2c_bus *i2c;
+	int rc, i;
+	struct device_node *np, *node = NULL;
+	u32 addr, clk;
+
+	i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
+	if (!i2c)
+		return -ENOMEM;
+
+	i2c->fsi = to_fsi_dev(dev);
+	i2c->clk = I2C_DEFAULT_CLK;
+
+	i2c->adapter.owner = THIS_MODULE;
+	i2c->adapter.dev.parent = dev;
+	i2c->adapter.algo = &fsi_i2c_algorithm;
+	i2c->adapter.algo_data = i2c;
+	strncpy(i2c->adapter.name, "power_i2cm", sizeof(i2c->adapter.name));
+
+	/* find node that matches FSI address */
+	for (i = 0; i < ARRAY_SIZE(compatible_strings) && !node; ++i) {
+		for_each_compatible_node(np, NULL, compatible_strings[i]) {
+			rc = of_property_read_u32(np, "reg", &addr);
+			if (rc)
+				continue;
+
+			if (addr == i2c->fsi->addr) {
+				node = np;
+				break;
+			}
+		}
+	}
+
+	if (node) {
+		rc = of_property_read_u32(node, "clock-frequency", &clk);
+		if (!rc)
+			i2c->clk = clk;
+	}
+
+	rc = fsi_i2c_dev_init(i2c);
+	if (rc)
+		return rc;
+
+	/* TODO: should we add an adapter for each port?? A lot of adapters...
+	 * numbering isn't ideal either, automatically set to i2c-%d. So no
+	 * way to distinguish fsi-i2c devs.
+	 */
+	rc = i2c_add_adapter(&i2c->adapter);
+	if (rc < 0)
+		return rc;
+
+	dev_set_drvdata(dev, i2c);
+
+	return 0;
+}
+
+static int fsi_i2c_remove(struct device *dev)
+{
+	struct fsi_i2c_bus *i2c = dev_get_drvdata(dev);
+
+	i2c_del_adapter(&i2c->adapter);
+
+	return 0;
+}
+
+static const struct fsi_device_id fsi_i2c_ids[] = {
+	{ FSI_ENGID_I2CM, FSI_VERSION_ANY },
+	{ 0 }
+};
+
+static struct fsi_driver fsi_i2c_driver = {
+	.id_table = fsi_i2c_ids,
+	.drv = {
+		.name = "power_i2cm",
+		.bus = &fsi_bus_type,
+		.probe = fsi_i2c_probe,
+		.remove = fsi_i2c_remove,
+	},
+};
+
+static int fsi_i2c_init(void)
+{
+	return fsi_driver_register(&fsi_i2c_driver);
+}
+
+static void fsi_i2c_exit(void)
+{
+	fsi_driver_unregister(&fsi_i2c_driver);
+}
+
+module_init(fsi_i2c_init);
+module_exit(fsi_i2c_exit);
+
+MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
+MODULE_DESCRIPTION("FSI attached I2C master");
+MODULE_LICENSE("GPL");
-- 
1.8.3.1

[RFC linux dev-4.7 3/5] drivers: i2c: Add transfer implementation for FSI algorithm

[Eddie James]

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

Use polling instead of interrupts; we have no hardware IRQ over FSI.

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

diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
index c47cd234..ebdfa72 100644
--- a/drivers/i2c/busses/i2c-fsi.c
+++ b/drivers/i2c/busses/i2c-fsi.c
@@ -132,6 +132,7 @@ struct fsi_i2c_bus {
 	struct i2c_adapter	adapter;
 	struct fsi_device	*fsi;
 	u8			fifo_size;
+	u16			xfrd;
 	unsigned long		clk;
 };
 
@@ -182,10 +183,233 @@ static int fsi_i2c_dev_init(struct fsi_i2c_bus *i2c)
 	return rc;
 }
 
+static int fsi_i2c_set_port(struct fsi_device *dev, int port)
+{
+	int rc;
+	u32 mode, dummy = 0;
+	int old_port;
+
+	rc = fsi_device_read(dev, I2C_BOE_MODE, &mode, sizeof(mode));
+	if (rc)
+		return rc;
+
+	old_port = GETFIELD(I2C_BOE_MODE_PORT, mode);
+
+	if (old_port != port) {
+		mode = SETFIELD(I2C_BOE_MODE_PORT, mode, port);
+		rc = fsi_device_write(dev, I2C_BOE_MODE, &mode,
+				      sizeof(mode));
+		if (rc)
+			return rc;
+
+		/* reset engine when port is changed */
+		rc = fsi_device_write(dev, I2C_BOE_RESET_ERR, &dummy,
+				      sizeof(dummy));
+		if (rc)
+			return rc;
+	}
+
+	return rc;
+}
+
+static int fsi_i2c_start(struct fsi_i2c_bus *i2c, struct i2c_msg *msg,
+			 bool stop)
+{
+	int rc;
+	u32 cmd = I2C_BOE_CMD_WITH_START | I2C_BOE_CMD_WITH_ADDR;
+
+	i2c->xfrd = 0;
+
+	if (msg->flags & I2C_M_RD)
+		cmd |= I2C_BOE_CMD_READ;
+
+	if (stop || msg->flags & I2C_M_STOP)
+		cmd |= I2C_BOE_CMD_WITH_STOP;
+
+	cmd = SETFIELD(I2C_BOE_CMD_ADDR, cmd, msg->addr >> 1);
+	cmd = SETFIELD(I2C_BOE_CMD_LEN, cmd, msg->len);
+
+	rc = fsi_device_write(i2c->fsi, I2C_BOE_CMD, &cmd, sizeof(cmd));
+
+	return rc;
+}
+
+static int fsi_i2c_write_fifo(struct fsi_i2c_bus *i2c, struct i2c_msg *msg,
+			      u8 fifo_count)
+{
+	int write;
+	int rc = 0;
+	int bytes_to_write = i2c->fifo_size - fifo_count;
+	int bytes_remaining = msg->len - i2c->xfrd;
+
+	if (bytes_to_write > bytes_remaining)
+		bytes_to_write = bytes_remaining;
+
+	while (bytes_to_write > 0) {
+		write = bytes_to_write;
+		/* fsi limited to 4 byte ops */
+		if (bytes_to_write > 4)
+			write = 4;
+
+		rc = fsi_device_write(i2c->fsi, I2C_BOE_FIFO,
+				      &msg->buf[i2c->xfrd], write);
+		if (rc)
+			return rc;
+
+		i2c->xfrd += write;
+		bytes_to_write -= write;
+	}
+
+	return rc;
+}
+
+static int fsi_i2c_read_fifo(struct fsi_i2c_bus *i2c, struct i2c_msg *msg,
+			     u8 fifo_count)
+{
+	int read;
+	int rc = 0;
+	int xfr_remaining = msg->len - i2c->xfrd;
+	u32 dummy;
+
+	while (fifo_count) {
+		read = fifo_count;
+		/* fsi limited to 4 byte ops */
+		if (fifo_count > 4)
+			read = 4;
+
+		if (xfr_remaining) {
+			if (xfr_remaining < read)
+				read = xfr_remaining;
+
+			rc = fsi_device_read(i2c->fsi, I2C_BOE_FIFO,
+					     &msg->buf[i2c->xfrd], read);
+			if (rc)
+				return rc;
+
+			i2c->xfrd += read;
+			xfr_remaining -= read;
+		} else {
+			/* no more buffer but data in fifo, need to clear it */
+			rc = fsi_device_read(i2c->fsi, I2C_BOE_FIFO,
+					     &dummy, read);
+			if (rc)
+				return rc;
+		}
+
+		fifo_count -= read;
+	}
+
+	return rc;
+}
+
+static int fsi_i2c_handle_status(struct fsi_i2c_bus *i2c, struct i2c_msg *msg,
+				 u32 status)
+{
+	int rc;
+	u8 fifo_count;
+	u32 dummy = 0;
+
+	if (status & I2C_BOE_STAT_ERR) {
+		rc = fsi_device_write(i2c->fsi, I2C_BOE_RESET_ERR, &dummy,
+				      sizeof(dummy));
+		if (rc)
+			return rc;
+
+		if (status & I2C_BOE_STAT_NACK)
+			return -EFAULT;
+
+		return -EIO;
+	}
+
+	if (status & I2C_BOE_STAT_DAT_REQ) {
+		fifo_count = GETFIELD(I2C_BOE_STAT_FIFO_COUNT, status);
+
+		if (msg->flags & I2C_M_RD)
+			rc = fsi_i2c_read_fifo(i2c, msg, fifo_count);
+		else
+			rc = fsi_i2c_write_fifo(i2c, msg, fifo_count);
+
+		return rc;
+	}
+
+	if (status & I2C_BOE_STAT_CMD_COMP) {
+		if (i2c->xfrd < msg->len)
+			rc = -ENODATA;
+		else
+			rc = msg->len;
+	}
+
+	return rc;
+}
+
+static int fsi_i2c_wait(struct fsi_i2c_bus *i2c, struct i2c_msg *msg,
+			unsigned long timeout)
+{
+	const unsigned long local_timeout = 2; /* jiffies */
+	u32 status = 0;
+	int rc;
+
+	do {
+		rc = fsi_device_read(i2c->fsi, I2C_BOE_STAT, &status,
+				     sizeof(status));
+		if (rc)
+			return rc;
+
+		if (status & I2C_BOE_STAT_ANY_RESP) {
+			rc = fsi_i2c_handle_status(i2c, msg, status);
+			if (rc < 0)
+				return rc;
+
+			/* cmd complete and all data xfrd */
+			if (rc == msg->len)
+				return 0;
+
+			/* need to xfr more data, but maybe don't need wait */
+			continue;
+		}
+
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		schedule_timeout(local_timeout);
+		timeout = (timeout < local_timeout) ? 0 :
+			timeout - local_timeout;
+	} while (timeout);
+
+	return -ETIME;
+}
+
 static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 			int num)
 {
-	return -ENOSYS;
+	int i;
+	int rc = 0;
+	int port;
+	unsigned long start_time;
+	struct fsi_i2c_bus *i2c = adap->algo_data;
+	struct i2c_msg *msg;
+
+	if (num) {
+		/* TODO: how else to get port information? this works though */
+		port = GETFIELD(I2C_ADDR_PORT, msgs[0].addr);
+		rc = fsi_i2c_set_port(i2c->fsi, port);
+		if (rc)
+			return rc;
+	}
+
+	for (i = 0; i < num; ++i) {
+		msg = msgs + i;
+		start_time = jiffies;
+
+		rc = fsi_i2c_start(i2c, msg, i == num - 1);
+		if (rc)
+			return rc;
+
+		rc = fsi_i2c_wait(i2c, msg,
+				  adap->timeout - (jiffies - start_time));
+		if (rc)
+			return rc;
+	}
+
+	return rc;
 }
 
 static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
-- 
1.8.3.1

[RFC linux dev-4.7 4/5] drivers: i2c: Add bus recovery for FSI algorithm

[Eddie James]

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

Reset engine and force the bus to clock 9 times to recover in most
situations.

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

diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
index ebdfa72..545d507 100644
--- a/drivers/i2c/busses/i2c-fsi.c
+++ b/drivers/i2c/busses/i2c-fsi.c
@@ -417,11 +417,92 @@ static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
 	return I2C_FUNC_I2C | I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_10BIT_ADDR;
 }
 
+static int fsi_i2c_low_level_recover_bus(struct fsi_i2c_bus *i2c)
+{
+	int i, rc;
+	u32 mode, dummy = 0;
+
+	rc = fsi_device_read(i2c->fsi, I2C_BOE_MODE, &mode, sizeof(mode));
+	if (rc)
+		return rc;
+
+	mode |= I2C_BOE_MODE_DIAG;
+	rc = fsi_device_write(i2c->fsi, I2C_BOE_MODE, &mode, sizeof(mode));
+	if (rc)
+		return rc;
+
+	for (i = 0; i < 9; ++i) {
+		rc = fsi_device_write(i2c->fsi, I2C_BOE_RESET_SCL, &dummy,
+				      sizeof(dummy));
+		if (rc)
+			return rc;
+
+		rc = fsi_device_write(i2c->fsi, I2C_BOE_SET_SCL, &dummy,
+				      sizeof(dummy));
+		if (rc)
+			return rc;
+	}
+
+	rc = fsi_device_write(i2c->fsi, I2C_BOE_RESET_SCL, &dummy,
+			      sizeof(dummy));
+	if (rc)
+		return rc;
+
+	rc = fsi_device_write(i2c->fsi, I2C_BOE_RESET_SDA, &dummy,
+			      sizeof(dummy));
+	if (rc)
+		return rc;
+
+	rc = fsi_device_write(i2c->fsi, I2C_BOE_SET_SCL, &dummy,
+			      sizeof(dummy));
+	if (rc)
+		return rc;
+
+	rc = fsi_device_write(i2c->fsi, I2C_BOE_SET_SDA, &dummy,
+			      sizeof(dummy));
+	if (rc)
+		return rc;
+
+	mode &= ~I2C_BOE_MODE_DIAG;
+	rc = fsi_device_write(i2c->fsi, I2C_BOE_MODE, &mode, sizeof(mode));
+
+	return rc;
+}
+
+static int fsi_i2c_recover_bus(struct i2c_adapter *adap)
+{
+	int rc;
+	u32 dummy = 0;
+	struct fsi_i2c_bus *i2c = adap->algo_data;
+
+	rc = fsi_device_write(i2c->fsi, I2C_BOE_RESET_I2C, &dummy,
+			      sizeof(dummy));
+	if (rc)
+		return rc;
+
+	rc = fsi_i2c_dev_init(i2c);
+	if (rc)
+		return rc;
+
+	rc = fsi_i2c_low_level_recover_bus(i2c);
+	if (rc)
+		return rc;
+
+	rc = fsi_device_write(i2c->fsi, I2C_BOE_RESET_ERR, &dummy,
+			      sizeof(dummy));
+
+	return rc;
+}
+
 static const struct i2c_algorithm fsi_i2c_algorithm = {
 	.master_xfer = fsi_i2c_xfer,
 	.functionality = fsi_i2c_functionality,
 };
 
+static struct i2c_bus_recovery_info fsi_i2c_bus_recovery_info = {
+	.recover_bus = fsi_i2c_recover_bus,
+};
+
 static const char *compatible_strings[] = {
 	"ibm,power8-i2cm",
 	"ibm,power9-i2cm",
@@ -445,6 +526,7 @@ static int fsi_i2c_probe(struct device *dev)
 	i2c->adapter.dev.parent = dev;
 	i2c->adapter.algo = &fsi_i2c_algorithm;
 	i2c->adapter.algo_data = i2c;
+	i2c->adapter.bus_recovery_info = &fsi_i2c_bus_recovery_info;
 	strncpy(i2c->adapter.name, "power_i2cm", sizeof(i2c->adapter.name));
 
 	/* find node that matches FSI address */
-- 
1.8.3.1

[RFC linux dev-4.7 5/5] ARM: dts: aspeed: witherspoon: Add i2cm as FSI device child

[Eddie James]

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

Primarily an example of how new i2c algorithm expects an i2c master
entry in the devicetree.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
 arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index 4d26d47..759c77e 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -57,6 +57,14 @@
 		mux-gpios = <&gpio ASPEED_GPIO(A, 6) GPIO_ACTIVE_HIGH>;
 		enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
 		trans-gpios = <&gpio ASPEED_GPIO(R, 2) GPIO_ACTIVE_HIGH>;
+
+		i2cm@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			compatible = "ibm,power9-i2cm";
+			reg = <0x00001800>;
+		};
 	};
 };
 
-- 
1.8.3.1

Re: [RFC linux dev-4.7 0/4] drivers: i2c: FSI-attached I2C master algorithm

[Brad Bishop]

> On Mar 10, 2017, at 4:48 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
> 
> From: "Edward A. James" <eajames@us.ibm.com>
> 
> This patch series adds an I2C bus algorithm to drive the I2C master located on
> POWER CPUs. The master is accessed over FSI bus from the service processor.
> 
> The driver is functional for some basic read/write ops. There are a couple of
> details regarding the device tree and chardev entries that need working out.
> 
> Mainly, do we want a char device for each port?  There are 15 ports off one P9
> master, so that'll be a lot of /dev/i2c-<x> entries.

IMHO I don’t see this being a problem.

> One issue is that
> i2c_add_adapter always names it's device i2c-%d, regardless of adapter name.

One can always add udev rules to get alternate naming schemes.

> Furthermore, you can specify the number to label it with
> i2c_add_numbered_adapter, but how to prevent collisions with regular i2c
> devices?

I think this is also probably solvable with udev.

> 
> The driver will currently set up one chardev for each master (each proc).
> The port is then specified by hacking the addr field of i2c_msg. It works,
> but not a clean solution.

I’d say we just go with one chardev per port, unless others have experience
that recommends otherwise.

> 
> FSI clocking details are not defined in the device tree, so FSI clock is
> hardcoded for now.
> 
> Edward A. James (4):
>  drivers: fsi: Add function to get FSI clock rate
>  drivers: i2c: Add FSI-attached I2C master algorithm
>  drivers: i2c: Add transfer implementation for FSI algorithm
>  drivers: i2c: Add bus recovery for FSI algorithm
> 
> drivers/Makefile             |   2 +-
> drivers/fsi/fsi-core.c       |   7 +
> drivers/i2c/busses/Kconfig   |  11 +
> drivers/i2c/busses/Makefile  |   1 +
> drivers/i2c/busses/i2c-fsi.c | 608 +++++++++++++++++++++++++++++++++++++++++++
> include/linux/fsi.h          |   1 +
> 6 files changed, 629 insertions(+), 1 deletion(-)
> create mode 100644 drivers/i2c/busses/i2c-fsi.c
> 
> -- 
> 1.8.3.1

Re: [RFC linux dev-4.7 0/4] drivers: i2c: FSI-attached I2C master algorithm

[Eddie James]

On 03/10/2017 08:16 PM, Brad Bishop wrote:
>> On Mar 10, 2017, at 4:48 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
>>
>> From: "Edward A. James" <eajames@us.ibm.com>
>>
>> This patch series adds an I2C bus algorithm to drive the I2C master located on
>> POWER CPUs. The master is accessed over FSI bus from the service processor.
>>
>> The driver is functional for some basic read/write ops. There are a couple of
>> details regarding the device tree and chardev entries that need working out.
>>
>> Mainly, do we want a char device for each port?  There are 15 ports off one P9
>> master, so that'll be a lot of /dev/i2c-<x> entries.
> IMHO I don’t see this being a problem.
>
>> One issue is that
>> i2c_add_adapter always names it's device i2c-%d, regardless of adapter name.
> One can always add udev rules to get alternate naming schemes.
>
>> Furthermore, you can specify the number to label it with
>> i2c_add_numbered_adapter, but how to prevent collisions with regular i2c
>> devices?
> I think this is also probably solvable with udev.
>
>> The driver will currently set up one chardev for each master (each proc).
>> The port is then specified by hacking the addr field of i2c_msg. It works,
>> but not a clean solution.
> I’d say we just go with one chardev per port, unless others have experience
> that recommends otherwise.

Thought of a potential issue with this approach. This will require some 
custom bus locking solution, as we need to prevent multiple transfers 
from setting the master regs at the same time. Obviously the kernel i2c 
driver handles locking on each chardev to prevent simultaneous access, 
but since we will have multiple chardevs that have to lock each other 
out, we need to do it ourselves.

I think having our own locking is pretty do-able, I just wonder if the 
kernel guys will not be happy with it?

Eddie
>
>> FSI clocking details are not defined in the device tree, so FSI clock is
>> hardcoded for now.
>>
>> Edward A. James (4):
>>   drivers: fsi: Add function to get FSI clock rate
>>   drivers: i2c: Add FSI-attached I2C master algorithm
>>   drivers: i2c: Add transfer implementation for FSI algorithm
>>   drivers: i2c: Add bus recovery for FSI algorithm
>>
>> drivers/Makefile             |   2 +-
>> drivers/fsi/fsi-core.c       |   7 +
>> drivers/i2c/busses/Kconfig   |  11 +
>> drivers/i2c/busses/Makefile  |   1 +
>> drivers/i2c/busses/i2c-fsi.c | 608 +++++++++++++++++++++++++++++++++++++++++++
>> include/linux/fsi.h          |   1 +
>> 6 files changed, 629 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/i2c/busses/i2c-fsi.c
>>
>> -- 
>> 1.8.3.1

Re: [RFC linux dev-4.7 1/5] drivers: fsi: Add function to get FSI clock rate

[Joel Stanley]

On Sat, Mar 11, 2017 at 8:18 AM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
> From: "Edward A. James" <eajames@us.ibm.com>
>
> client drivers (I2C, etc) may need to know FSI bus clock rate.

We need to make sure the device tree bindings expose this.

Are we able to discover the clock rate through reading some registers?
Or is it hardcoded on all systems?

I suggest you put something like this in the device tree under the FSI
node your testing:

                 fsi_clock {
                        #clock-cells = <0>;
                        compatible = "fixed-clock";
                        clock-frequency = <8333333>;
                };

And retrieve the clock from there. This way when we add the code for
determining the clock rate your driver won't need to change.

Cheers,

Joel

>
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> ---
>  drivers/fsi/fsi-core.c | 7 +++++++
>  include/linux/fsi.h    | 1 +
>  2 files changed, 8 insertions(+)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 3d382e6..492c9e9 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -139,6 +139,13 @@ int fsi_device_peek(struct fsi_device *dev, void *val)
>         return fsi_slave_read(dev->slave, addr, val, sizeof(uint32_t));
>  }
>
> +unsigned long fsi_device_get_clk_rate(struct fsi_device *dev)
> +{
> +       /* TODO: get clk from dts */
> +       return 8333333;
> +}
> +EXPORT_SYMBOL_GPL(fsi_device_get_clk_rate);
> +
>  static void fsi_device_release(struct device *_device)
>  {
>         struct fsi_device *device = to_fsi_dev(_device);
> diff --git a/include/linux/fsi.h b/include/linux/fsi.h
> index d22d0c5..57c8fdb 100644
> --- a/include/linux/fsi.h
> +++ b/include/linux/fsi.h
> @@ -35,6 +35,7 @@ extern int fsi_device_read(struct fsi_device *dev, uint32_t addr,
>  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);
> +extern unsigned long fsi_device_get_clk_rate(struct fsi_device *dev);
>
>  struct fsi_device_id {
>         u8      engine_type;
> --
> 1.8.3.1
>

Re: [RFC linux dev-4.7 2/5] drivers: i2c: Add FSI-attached I2C master algorithm

[Joel Stanley]

On Sat, Mar 11, 2017 at 8:18 AM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
> From: "Edward A. James" <eajames@us.ibm.com>
>
> Initial startup code for the I2C algorithm to drive the I2C master
> located on POWER CPUs over FSI bus.

I was leaning towards calling it i2c-cfam, as it's an i2c master in the cfam?

Other ideas are welcome too.

>
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> ---
>  drivers/Makefile             |   2 +-
>  drivers/i2c/busses/Kconfig   |  11 ++
>  drivers/i2c/busses/Makefile  |   1 +
>  drivers/i2c/busses/i2c-fsi.c | 302 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 315 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/i2c/busses/i2c-fsi.c
>
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 26f18ac..b1db6d3 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -103,6 +103,7 @@ obj-$(CONFIG_SERIO)         += input/serio/
>  obj-$(CONFIG_GAMEPORT)         += input/gameport/
>  obj-$(CONFIG_INPUT)            += input/
>  obj-$(CONFIG_RTC_LIB)          += rtc/
> +obj-$(CONFIG_FSI)              += fsi/
>  obj-y                          += i2c/ media/
>  obj-$(CONFIG_PPS)              += pps/
>  obj-$(CONFIG_PTP_1588_CLOCK)   += ptp/
> @@ -173,4 +174,3 @@ 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/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 835480b..c248eded 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -1210,4 +1210,15 @@ config I2C_OPAL
>           This driver can also be built as a module. If so, the module will be
>           called as i2c-opal.
>
> +config I2C_FSI
> +       tristate "FSI I2C driver"
> +       depends on FSI
> +       help
> +         Driver for FSI bus attached I2C masters. These are I2C masters that
> +         are connected to the system over a FSI bus, instead of the more
> +         common PCI or MMIO interface.

That's a good description.

> +
> +         This driver can also be built as a module. If so, the module will be
> +         called as i2c-fsi.
> +
>  endmenu
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 49631cd..b74b585 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -119,5 +119,6 @@ obj-$(CONFIG_I2C_PCA_ISA)   += i2c-pca-isa.o
>  obj-$(CONFIG_I2C_SIBYTE)       += i2c-sibyte.o
>  obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o
>  obj-$(CONFIG_SCx200_ACB)       += scx200_acb.o
> +obj-$(CONFIG_I2C_FSI)          += i2c-fsi.o
>
>  ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
> new file mode 100644
> index 0000000..c47cd234
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-fsi.c
> @@ -0,0 +1,302 @@
> +/*
> + * Copyright 2017 IBM Corporation
> + *
> + * Eddie James <eajames@us.ibm.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/fsi.h>
> +#include <linux/i2c.h>
> +#include <linux/jiffies.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/sched.h>
> +
> +#define FSI_ENGID_I2CM         0x7
> +
> +/* Find left shift from first set bit in m */
> +#define MASK_TO_LSH(m)          (__builtin_ffsll(m) - 1ULL)
> +
> +/* Extract field m from v */
> +#define GETFIELD(m, v)          (((v) & (m)) >> MASK_TO_LSH(m))
> +
> +/* Set field m of v to val */
> +#define SETFIELD(m, v, val)                             \
> +       (((v) & ~(m)) | ((((typeof(v))(val)) << MASK_TO_LSH(m)) & (m)))
> +
> +#define I2C_DEFAULT_CLK_DIV    6
> +#define I2C_DEFAULT_CLK                333333
> +#define I2C_ADDR_PORT          0xff00
> +#define I2C_BOE_FIFO_HI_LVL    4
> +#define I2C_BOE_FIFO_LO_LVL    4
> +
> +/* i2c registers */
> +#define I2C_BOE_FIFO           0x00
> +#define I2C_BOE_CMD            0x04
> +#define I2C_BOE_MODE           0x08
> +#define I2C_BOE_WATER_MARK     0x0C
> +#define I2C_BOE_INT_MASK       0x10
> +#define I2C_BOE_INT_COND       0x14
> +#define I2C_BOE_OR_INT_MASK    0x14
> +#define I2C_BOE_INTS           0x18
> +#define I2C_BOE_AND_INT_MASK   0x18
> +#define I2C_BOE_STAT           0x1C
> +#define I2C_BOE_RESET_I2C      0x1C
> +#define I2C_BOE_ESTAT          0x20
> +#define I2C_BOE_RESET_ERR      0x20
> +#define I2C_BOE_RESID_LEN      0x24
> +#define I2C_BOE_SET_SCL                0x24
> +#define I2C_BOE_PORT_BUSY      0x28
> +#define I2C_BOE_RESET_SCL      0x2C
> +#define I2C_BOE_SET_SDA                0x30
> +#define I2C_BOE_RESET_SDA      0x34

I don't think the BOE label makes any sense here, unless we want to
call the driver i2c-boe?

> +
> +/* cmd register */
> +#define I2C_BOE_CMD_WITH_START 0x80000000
> +#define I2C_BOE_CMD_WITH_ADDR  0x40000000
> +#define I2C_BOE_CMD_RD_CONT    0x20000000
> +#define I2C_BOE_CMD_WITH_STOP  0x10000000
> +#define I2C_BOE_CMD_FORCELAUNCH        0x08000000
> +#define I2C_BOE_CMD_ADDR       0x00fe0000
> +#define I2C_BOE_CMD_READ       0x00010000
> +#define I2C_BOE_CMD_LEN                0x0000ffff

Most f these are bits. I like to see the BIT(n) macro to make that clear.

You can also use the GENMASK(hi, low) macro for creating ranges. That
one I leave to you as it's not always a readability win.

> +
> +/* mode register */
> +#define I2C_BOE_MODE_CLKDIV    0xffff0000
> +#define I2C_BOE_MODE_PORT      0x0000fc00
> +#define I2C_BOE_MODE_ENHANCED  0x00000008
> +#define I2C_BOE_MODE_DIAG      0x00000004
> +#define I2C_BOE_MODE_PACE_ALLOW        0x00000002
> +#define I2C_BOE_MODE_WRAP      0x00000001
> +
> +/* watermark register */
> +#define I2C_BOE_WATERMARK_HI   0x0000f000
> +#define I2C_BOE_WATERMARK_LO   0x000000f0
> +
> +/* interrupt register */
> +#define I2C_BOE_INT_INV_CMD    0x00008000
> +#define I2C_BOE_INT_PARITY     0x00004000
> +#define I2C_BOE_INT_BE_OVERRUN 0x00002000
> +#define I2C_BOE_INT_BE_ACCESS  0x00001000
> +#define I2C_BOE_INT_LOST_ARB   0x00000800
> +#define I2C_BOE_INT_NACK       0x00000400
> +#define I2C_BOE_INT_DAT_REQ    0x00000200
> +#define I2C_BOE_INT_CMD_COMP   0x00000100
> +#define I2C_BOE_INT_STOP_ERR   0x00000080
> +#define I2C_BOE_INT_BUSY       0x00000040
> +#define I2C_BOE_INT_IDLE       0x00000020
> +
> +#define I2C_BOE_INT_ENABLE     0x0000ff80
> +#define I2C_BOE_INT_ERR                0x0000fcc0
> +
> +/* status register */
> +#define I2C_BOE_STAT_INV_CMD   0x80000000
> +#define I2C_BOE_STAT_PARITY    0x40000000
> +#define I2C_BOE_STAT_BE_OVERRUN        0x20000000
> +#define I2C_BOE_STAT_BE_ACCESS 0x10000000
> +#define I2C_BOE_STAT_LOST_ARB  0x08000000
> +#define I2C_BOE_STAT_NACK      0x04000000
> +#define I2C_BOE_STAT_DAT_REQ   0x02000000
> +#define I2C_BOE_STAT_CMD_COMP  0x01000000
> +#define I2C_BOE_STAT_STOP_ERR  0x00800000
> +#define I2C_BOE_STAT_MAX_PORT  0x000f0000
> +#define I2C_BOE_STAT_ANY_INT   0x00008000
> +#define I2C_BOE_STAT_SCL_IN    0x00000800
> +#define I2C_BOE_STAT_SDA_IN    0x00000400
> +#define I2C_BOE_STAT_PORT_BUSY 0x00000200
> +#define I2C_BOE_STAT_SELF_BUSY 0x00000100
> +#define I2C_BOE_STAT_FIFO_COUNT        0x000000ff
> +
> +#define I2C_BOE_STAT_ERR       0xfc800000
> +#define I2C_BOE_STAT_ANY_RESP  0xff800000
> +
> +/* extended status register */
> +#define I2C_BOE_ESTAT_FIFO_SZ  0xff000000
> +#define I2C_BOE_ESTAT_SCL_IN_SY        0x00008000
> +#define I2C_BOE_ESTAT_SDA_IN_SY        0x00004000
> +#define I2C_BOE_ESTAT_S_SCL    0x00002000
> +#define I2C_BOE_ESTAT_S_SDA    0x00001000
> +#define I2C_BOE_ESTAT_M_SCL    0x00000800
> +#define I2C_BOE_ESTAT_M_SDA    0x00000400
> +#define I2C_BOE_ESTAT_HI_WATER 0x00000200
> +#define I2C_BOE_ESTAT_LO_WATER 0x00000100
> +#define I2C_BOE_ESTAT_PORT_BUSY        0x00000080
> +#define I2C_BOE_ESTAT_SELF_BUSY        0x00000040
> +#define I2C_BOE_ESTAT_VERSION  0x0000001f
> +
> +struct fsi_i2c_bus {
> +       struct i2c_adapter      adapter;
> +       struct fsi_device       *fsi;
> +       u8                      fifo_size;
> +       unsigned long           clk;
> +};
> +
> +static unsigned long fsi_i2c_get_clk_div(struct fsi_i2c_bus *i2c)
> +{
> +       unsigned long clk_div = I2C_DEFAULT_CLK_DIV;
> +       unsigned long fsi_clk = fsi_device_get_clk_rate(i2c->fsi);
> +
> +       if (fsi_clk > i2c->clk)
> +               clk_div = ((fsi_clk / i2c->clk) - 1) / 4;
> +
> +       return clk_div;
> +}
> +
> +static int fsi_i2c_dev_init(struct fsi_i2c_bus *i2c)
> +{
> +       int rc;
> +       unsigned long clk_div;
> +       u32 mode = I2C_BOE_MODE_ENHANCED, extended_status, watermark = 0;
> +       u32 interrupt = 0;
> +
> +       /* since we use polling, disable interrupts */
> +       rc = fsi_device_write(i2c->fsi, I2C_BOE_INT_MASK, &interrupt,
> +                             sizeof(interrupt));
> +       if (rc)
> +               return rc;
> +
> +       clk_div = fsi_i2c_get_clk_div(i2c);
> +       mode = SETFIELD(I2C_BOE_MODE_CLKDIV, mode, clk_div);
> +       rc = fsi_device_write(i2c->fsi, I2C_BOE_MODE, &mode, sizeof(mode));
> +       if (rc)
> +               return rc;
> +
> +       rc = fsi_device_read(i2c->fsi, I2C_BOE_ESTAT, &extended_status,
> +                            sizeof(extended_status));
> +       if (rc)
> +               return rc;
> +
> +       i2c->fifo_size = GETFIELD(I2C_BOE_ESTAT_FIFO_SZ, extended_status);
> +       watermark = SETFIELD(I2C_BOE_WATERMARK_HI, watermark,
> +                            i2c->fifo_size - I2C_BOE_FIFO_HI_LVL);
> +       watermark = SETFIELD(I2C_BOE_WATERMARK_LO, watermark,
> +                            I2C_BOE_FIFO_LO_LVL);
> +
> +       rc = fsi_device_write(i2c->fsi, I2C_BOE_WATER_MARK, &watermark,
> +                             sizeof(watermark));
> +
> +       return rc;
> +}
> +
> +static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> +                       int num)
> +{
> +       return -ENOSYS;
> +}
> +
> +static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
> +{
> +       return I2C_FUNC_I2C | I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_10BIT_ADDR;
> +}
> +
> +static const struct i2c_algorithm fsi_i2c_algorithm = {
> +       .master_xfer = fsi_i2c_xfer,
> +       .functionality = fsi_i2c_functionality,
> +};
> +
> +static const char *compatible_strings[] = {
> +       "ibm,power8-i2cm",
> +       "ibm,power9-i2cm",
> +};
> +
> +static int fsi_i2c_probe(struct device *dev)
> +{
> +       struct fsi_i2c_bus *i2c;
> +       int rc, i;
> +       struct device_node *np, *node = NULL;
> +       u32 addr, clk;
> +
> +       i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
> +       if (!i2c)
> +               return -ENOMEM;
> +
> +       i2c->fsi = to_fsi_dev(dev);
> +       i2c->clk = I2C_DEFAULT_CLK;
> +
> +       i2c->adapter.owner = THIS_MODULE;
> +       i2c->adapter.dev.parent = dev;
> +       i2c->adapter.algo = &fsi_i2c_algorithm;
> +       i2c->adapter.algo_data = i2c;
> +       strncpy(i2c->adapter.name, "power_i2cm", sizeof(i2c->adapter.name));

Here is another name. Perhaps we should call the driver i2c-power?

> +
> +       /* find node that matches FSI address */
> +       for (i = 0; i < ARRAY_SIZE(compatible_strings) && !node; ++i) {
> +               for_each_compatible_node(np, NULL, compatible_strings[i]) {
> +                       rc = of_property_read_u32(np, "reg", &addr);

This is the right idea. I'm not sure if we want to perform the
matching in the FSI core instead of having it in the downstream
driver. Jeremy?


> +                       if (rc)
> +                               continue;
> +
> +                       if (addr == i2c->fsi->addr) {
> +                               node = np;
> +                               break;
> +                       }
> +               }
> +       }
> +
> +       if (node) {
> +               rc = of_property_read_u32(node, "clock-frequency", &clk);
> +               if (!rc)
> +                       i2c->clk = clk;
> +       }

I see that you're trying to leave i2c->clk at the default. Perhaps this instead?

if (!(node && of_property_read_u32(node, "clock-frequency", &i2c->clk)))
   i2c->clk = I2C_DEFAULT_CLK;

That isn't great though. At least set the default just above here so
it's obvious what is going on.

> +
> +       rc = fsi_i2c_dev_init(i2c);
> +       if (rc)
> +               return rc;
> +
> +       /* TODO: should we add an adapter for each port?? A lot of adapters...
> +        * numbering isn't ideal either, automatically set to i2c-%d. So no
> +        * way to distinguish fsi-i2c devs.
> +        */

As Brad said, using udev is probably the right solution here.

> +       rc = i2c_add_adapter(&i2c->adapter);
> +       if (rc < 0)
> +               return rc;
> +
> +       dev_set_drvdata(dev, i2c);
> +
> +       return 0;
> +}
> +
> +static int fsi_i2c_remove(struct device *dev)
> +{
> +       struct fsi_i2c_bus *i2c = dev_get_drvdata(dev);
> +
> +       i2c_del_adapter(&i2c->adapter);
> +
> +       return 0;
> +}
> +
> +static const struct fsi_device_id fsi_i2c_ids[] = {
> +       { FSI_ENGID_I2CM, FSI_VERSION_ANY },
> +       { 0 }
> +};
> +
> +static struct fsi_driver fsi_i2c_driver = {
> +       .id_table = fsi_i2c_ids,
> +       .drv = {
> +               .name = "power_i2cm",
> +               .bus = &fsi_bus_type,
> +               .probe = fsi_i2c_probe,
> +               .remove = fsi_i2c_remove,
> +       },
> +};
> +
> +static int fsi_i2c_init(void)
> +{
> +       return fsi_driver_register(&fsi_i2c_driver);
> +}
> +
> +static void fsi_i2c_exit(void)
> +{
> +       fsi_driver_unregister(&fsi_i2c_driver);
> +}
> +
> +module_init(fsi_i2c_init);
> +module_exit(fsi_i2c_exit);
> +
> +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
> +MODULE_DESCRIPTION("FSI attached I2C master");
> +MODULE_LICENSE("GPL");
> --
> 1.8.3.1
>

Re: [RFC linux dev-4.7 0/4] drivers: i2c: FSI-attached I2C master algorithm

[Brad Bishop]

[snip]

>> 
>>> The driver will currently set up one chardev for each master (each proc).
>>> The port is then specified by hacking the addr field of i2c_msg. It works,
>>> but not a clean solution.
>> I’d say we just go with one chardev per port, unless others have experience
>> that recommends otherwise.
> 
> Thought of a potential issue with this approach. This will require some custom bus locking solution, as we need to prevent multiple transfers from setting the master regs at the same time.

Does each port have its own reg(s) or do bits n-m in reg x correlate to ports n-m?

> Obviously the kernel i2c driver handles locking on each chardev to prevent simultaneous access, but since we will have multiple chardevs that have to lock each other out, we need to do it ourselves.
> 
> I think having our own locking is pretty do-able, I just wonder if the kernel guys will not be happy with it?
> 
> Eddie
>> 
>>> FSI clocking details are not defined in the device tree, so FSI clock is
>>> hardcoded for now.
>>> 
>>> Edward A. James (4):
>>>  drivers: fsi: Add function to get FSI clock rate
>>>  drivers: i2c: Add FSI-attached I2C master algorithm
>>>  drivers: i2c: Add transfer implementation for FSI algorithm
>>>  drivers: i2c: Add bus recovery for FSI algorithm
>>> 
>>> drivers/Makefile             |   2 +-
>>> drivers/fsi/fsi-core.c       |   7 +
>>> drivers/i2c/busses/Kconfig   |  11 +
>>> drivers/i2c/busses/Makefile  |   1 +
>>> drivers/i2c/busses/i2c-fsi.c | 608 +++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/fsi.h          |   1 +
>>> 6 files changed, 629 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/i2c/busses/i2c-fsi.c
>>> 
>>> -- 
>>> 1.8.3.1

Re: [RFC linux dev-4.7 0/4] drivers: i2c: FSI-attached I2C master algorithm

[Eddie James]

On 03/16/2017 07:44 PM, Brad Bishop wrote:
> [snip]
>
>>>> The driver will currently set up one chardev for each master (each proc).
>>>> The port is then specified by hacking the addr field of i2c_msg. It works,
>>>> but not a clean solution.
>>> I’d say we just go with one chardev per port, unless others have experience
>>> that recommends otherwise.
>> Thought of a potential issue with this approach. This will require some custom bus locking solution, as we need to prevent multiple transfers from setting the master regs at the same time.
> Does each port have its own reg(s) or do bits n-m in reg x correlate to ports n-m?

All the regs are shared between the ports for that master. On the P9, we 
have 15 ports per master. You have to set the port number in the mode 
register. All the registers just control the master, which does the 
necessary operations on the specified port.

So there will be access of the same registers for different port chardevs.

>
>> Obviously the kernel i2c driver handles locking on each chardev to prevent simultaneous access, but since we will have multiple chardevs that have to lock each other out, we need to do it ourselves.
>>
>> I think having our own locking is pretty do-able, I just wonder if the kernel guys will not be happy with it?
>>
>> Eddie
>>>> FSI clocking details are not defined in the device tree, so FSI clock is
>>>> hardcoded for now.
>>>>
>>>> Edward A. James (4):
>>>>   drivers: fsi: Add function to get FSI clock rate
>>>>   drivers: i2c: Add FSI-attached I2C master algorithm
>>>>   drivers: i2c: Add transfer implementation for FSI algorithm
>>>>   drivers: i2c: Add bus recovery for FSI algorithm
>>>>
>>>> drivers/Makefile             |   2 +-
>>>> drivers/fsi/fsi-core.c       |   7 +
>>>> drivers/i2c/busses/Kconfig   |  11 +
>>>> drivers/i2c/busses/Makefile  |   1 +
>>>> drivers/i2c/busses/i2c-fsi.c | 608 +++++++++++++++++++++++++++++++++++++++++++
>>>> include/linux/fsi.h          |   1 +
>>>> 6 files changed, 629 insertions(+), 1 deletion(-)
>>>> create mode 100644 drivers/i2c/busses/i2c-fsi.c
>>>>
>>>> -- 
>>>> 1.8.3.1