All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm
@ 2017-07-26 17:13 Eddie James
  2017-07-26 17:13   ` Eddie James
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Eddie James @ 2017-07-26 17:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, devicetree, wsa, robh+dt, jk, joel, andrew, cbostic,
	eajames, Edward A. James

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

This series adds an algorithm for an I2C master physically located on an FSI
slave device. The I2C master has multiple ports, each of which may be connected
to an I2C slave. Access to the I2C master registers is achieved over FSI bus.

Due to the multi-port nature of the I2C master, the driver instantiates a new
I2C adapter for each port connected to a slave. The connected ports should be
defined in the device tree under the I2C master device.

Changes since v4:
 - Add SMBUS flags to functionality mask.
 - Improve probing by continuing parsing the dt if we fail one adapter
   registration. Also don't fail for ENOMEM, as we may have registered some
   ports already.
 - Fix remove() by checking for empty list before iterating.

Edward A. James (6):
  drivers/i2c: Add FSI-attached I2C master algorithm
  drivers/i2c: Add port structure to FSI algorithm
  drivers/i2c: Add transfer implementation for FSI algorithm
  drivers/i2c: Add I2C master locking to FSI algorithm
  drivers/i2c: Add bus recovery for FSI algorithm
  dt-bindings: i2c: Add FSI-attached I2C master dt binding documentation

 Documentation/devicetree/bindings/i2c/i2c-fsi.txt |  40 ++
 drivers/i2c/busses/Kconfig                        |  11 +
 drivers/i2c/busses/Makefile                       |   1 +
 drivers/i2c/busses/i2c-fsi.c                      | 643 ++++++++++++++++++++++
 4 files changed, 695 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-fsi.txt
 create mode 100644 drivers/i2c/busses/i2c-fsi.c

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v5 1/6] drivers/i2c: Add FSI-attached I2C master algorithm
@ 2017-07-26 17:13   ` Eddie James
  0 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2017-07-26 17:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, devicetree, wsa, robh+dt, jk, joel, andrew, cbostic,
	eajames, Edward A. James

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

Add register definitions for FSI-attached I2C master and functions to
access those registers over FSI. Add an FSI driver so that our I2C bus
is probed up during an FSI scan.

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

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 1006b23..07503b4 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1290,4 +1290,15 @@ config I2C_ZX2967
 	  This driver can also be built as a module. If so, the module will be
 	  called i2c-zx2967.
 
+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 1b2fc81..aa5f7b5 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -131,5 +131,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..79475f8
--- /dev/null
+++ b/drivers/i2c/busses/i2c-fsi.c
@@ -0,0 +1,244 @@
+/*
+ * 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/device.h>
+#include <linux/errno.h>
+#include <linux/fsi.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#define FSI_ENGID_I2C		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
+
+/* i2c registers */
+#define I2C_FSI_FIFO		0x00
+#define I2C_FSI_CMD		0x04
+#define I2C_FSI_MODE		0x08
+#define I2C_FSI_WATER_MARK	0x0C
+#define I2C_FSI_INT_MASK	0x10
+#define I2C_FSI_INT_COND	0x14
+#define I2C_FSI_OR_INT_MASK	0x14
+#define I2C_FSI_INTS		0x18
+#define I2C_FSI_AND_INT_MASK	0x18
+#define I2C_FSI_STAT		0x1C
+#define I2C_FSI_RESET_I2C	0x1C
+#define I2C_FSI_ESTAT		0x20
+#define I2C_FSI_RESET_ERR	0x20
+#define I2C_FSI_RESID_LEN	0x24
+#define I2C_FSI_SET_SCL		0x24
+#define I2C_FSI_PORT_BUSY	0x28
+#define I2C_FSI_RESET_SCL	0x2C
+#define I2C_FSI_SET_SDA		0x30
+#define I2C_FSI_RESET_SDA	0x34
+
+/* cmd register */
+#define I2C_CMD_WITH_START	0x80000000
+#define I2C_CMD_WITH_ADDR	0x40000000
+#define I2C_CMD_RD_CONT		0x20000000
+#define I2C_CMD_WITH_STOP	0x10000000
+#define I2C_CMD_FORCELAUNCH	0x08000000
+#define I2C_CMD_ADDR		0x00fe0000
+#define I2C_CMD_READ		0x00010000
+#define I2C_CMD_LEN		0x0000ffff
+
+/* mode register */
+#define I2C_MODE_CLKDIV		0xffff0000
+#define I2C_MODE_PORT		0x0000fc00
+#define I2C_MODE_ENHANCED	0x00000008
+#define I2C_MODE_DIAG		0x00000004
+#define I2C_MODE_PACE_ALLOW	0x00000002
+#define I2C_MODE_WRAP		0x00000001
+
+/* watermark register */
+#define I2C_WATERMARK_HI	0x0000f000
+#define I2C_WATERMARK_LO	0x000000f0
+
+#define I2C_FIFO_HI_LVL		4
+#define I2C_FIFO_LO_LVL		4
+
+/* interrupt register */
+#define I2C_INT_INV_CMD		0x00008000
+#define I2C_INT_PARITY		0x00004000
+#define I2C_INT_BE_OVERRUN	0x00002000
+#define I2C_INT_BE_ACCESS	0x00001000
+#define I2C_INT_LOST_ARB	0x00000800
+#define I2C_INT_NACK		0x00000400
+#define I2C_INT_DAT_REQ		0x00000200
+#define I2C_INT_CMD_COMP	0x00000100
+#define I2C_INT_STOP_ERR	0x00000080
+#define I2C_INT_BUSY		0x00000040
+#define I2C_INT_IDLE		0x00000020
+
+#define I2C_INT_ENABLE		0x0000ff80
+#define I2C_INT_ERR		0x0000fcc0
+
+/* status register */
+#define I2C_STAT_INV_CMD	0x80000000
+#define I2C_STAT_PARITY		0x40000000
+#define I2C_STAT_BE_OVERRUN	0x20000000
+#define I2C_STAT_BE_ACCESS	0x10000000
+#define I2C_STAT_LOST_ARB	0x08000000
+#define I2C_STAT_NACK		0x04000000
+#define I2C_STAT_DAT_REQ	0x02000000
+#define I2C_STAT_CMD_COMP	0x01000000
+#define I2C_STAT_STOP_ERR	0x00800000
+#define I2C_STAT_MAX_PORT	0x000f0000
+#define I2C_STAT_ANY_INT	0x00008000
+#define I2C_STAT_SCL_IN		0x00000800
+#define I2C_STAT_SDA_IN		0x00000400
+#define I2C_STAT_PORT_BUSY	0x00000200
+#define I2C_STAT_SELF_BUSY	0x00000100
+#define I2C_STAT_FIFO_COUNT	0x000000ff
+
+#define I2C_STAT_ERR		0xfc800000
+#define I2C_STAT_ANY_RESP	0xff800000
+
+/* extended status register */
+#define I2C_ESTAT_FIFO_SZ	0xff000000
+#define I2C_ESTAT_SCL_IN_SY	0x00008000
+#define I2C_ESTAT_SDA_IN_SY	0x00004000
+#define I2C_ESTAT_S_SCL		0x00002000
+#define I2C_ESTAT_S_SDA		0x00001000
+#define I2C_ESTAT_M_SCL		0x00000800
+#define I2C_ESTAT_M_SDA		0x00000400
+#define I2C_ESTAT_HI_WATER	0x00000200
+#define I2C_ESTAT_LO_WATER	0x00000100
+#define I2C_ESTAT_PORT_BUSY	0x00000080
+#define I2C_ESTAT_SELF_BUSY	0x00000040
+#define I2C_ESTAT_VERSION	0x0000001f
+
+struct fsi_i2c_master {
+	struct fsi_device	*fsi;
+	u8			fifo_size;
+};
+
+static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
+			    u32 *data)
+{
+	int rc;
+	__be32 data_be;
+
+	rc = fsi_device_read(fsi, reg, &data_be, sizeof(data_be));
+	if (rc)
+		return rc;
+
+	*data = be32_to_cpu(data_be);
+
+	return 0;
+}
+
+static int fsi_i2c_write_reg(struct fsi_device *fsi, unsigned int reg,
+			     u32 *data)
+{
+	__be32 data_be = cpu_to_be32(*data);
+
+	return fsi_device_write(fsi, reg, &data_be, sizeof(data_be));
+}
+
+static int fsi_i2c_dev_init(struct fsi_i2c_master *i2c)
+{
+	int rc;
+	u32 mode = I2C_MODE_ENHANCED, extended_status, watermark = 0;
+	u32 interrupt = 0;
+
+	/* since we use polling, disable interrupts */
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_INT_MASK, &interrupt);
+	if (rc)
+		return rc;
+
+	mode = SETFIELD(I2C_MODE_CLKDIV, mode, I2C_DEFAULT_CLK_DIV);
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
+	if (rc)
+		return rc;
+
+	rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_ESTAT, &extended_status);
+	if (rc)
+		return rc;
+
+	i2c->fifo_size = GETFIELD(I2C_ESTAT_FIFO_SZ, extended_status);
+	watermark = SETFIELD(I2C_WATERMARK_HI, watermark,
+			     i2c->fifo_size - I2C_FIFO_HI_LVL);
+	watermark = SETFIELD(I2C_WATERMARK_LO, watermark,
+			     I2C_FIFO_LO_LVL);
+
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_WATER_MARK, &watermark);
+
+	return rc;
+}
+
+static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+			int num)
+{
+	return -EOPNOTSUPP;
+}
+
+static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_10BIT_ADDR
+		| I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+static const struct i2c_algorithm fsi_i2c_algorithm = {
+	.master_xfer = fsi_i2c_xfer,
+	.functionality = fsi_i2c_functionality,
+};
+
+static int fsi_i2c_probe(struct device *dev)
+{
+	struct fsi_i2c_master *i2c;
+	int rc;
+
+	i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
+	if (!i2c)
+		return -ENOMEM;
+
+	i2c->fsi = to_fsi_dev(dev);
+
+	rc = fsi_i2c_dev_init(i2c);
+	if (rc)
+		return rc;
+
+	dev_set_drvdata(dev, i2c);
+
+	return 0;
+}
+
+static const struct fsi_device_id fsi_i2c_ids[] = {
+	{ FSI_ENGID_I2C, FSI_VERSION_ANY },
+	{ 0 }
+};
+
+static struct fsi_driver fsi_i2c_driver = {
+	.id_table = fsi_i2c_ids,
+	.drv = {
+		.name = "i2c-fsi",
+		.bus = &fsi_bus_type,
+		.probe = fsi_i2c_probe,
+	},
+};
+
+module_fsi_driver(fsi_i2c_driver);
+
+MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
+MODULE_DESCRIPTION("FSI attached I2C master");
+MODULE_LICENSE("GPL");
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 1/6] drivers/i2c: Add FSI-attached I2C master algorithm
@ 2017-07-26 17:13   ` Eddie James
  0 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2017-07-26 17:13 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, jk-mnsaURCQ41sdnm+yROfE0A,
	joel-U3u1mxZcP9KHXe+LvDLADg, andrew-zrmu5oMJ5Fs,
	cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	eajames-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Edward A. James

From: "Edward A. James" <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Add register definitions for FSI-attached I2C master and functions to
access those registers over FSI. Add an FSI driver so that our I2C bus
is probed up during an FSI scan.

Signed-off-by: Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 drivers/i2c/busses/Kconfig   |  11 ++
 drivers/i2c/busses/Makefile  |   1 +
 drivers/i2c/busses/i2c-fsi.c | 244 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 256 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-fsi.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 1006b23..07503b4 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1290,4 +1290,15 @@ config I2C_ZX2967
 	  This driver can also be built as a module. If so, the module will be
 	  called i2c-zx2967.
 
+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 1b2fc81..aa5f7b5 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -131,5 +131,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..79475f8
--- /dev/null
+++ b/drivers/i2c/busses/i2c-fsi.c
@@ -0,0 +1,244 @@
+/*
+ * Copyright 2017 IBM Corporation
+ *
+ * Eddie James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
+ *
+ * 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/device.h>
+#include <linux/errno.h>
+#include <linux/fsi.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#define FSI_ENGID_I2C		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
+
+/* i2c registers */
+#define I2C_FSI_FIFO		0x00
+#define I2C_FSI_CMD		0x04
+#define I2C_FSI_MODE		0x08
+#define I2C_FSI_WATER_MARK	0x0C
+#define I2C_FSI_INT_MASK	0x10
+#define I2C_FSI_INT_COND	0x14
+#define I2C_FSI_OR_INT_MASK	0x14
+#define I2C_FSI_INTS		0x18
+#define I2C_FSI_AND_INT_MASK	0x18
+#define I2C_FSI_STAT		0x1C
+#define I2C_FSI_RESET_I2C	0x1C
+#define I2C_FSI_ESTAT		0x20
+#define I2C_FSI_RESET_ERR	0x20
+#define I2C_FSI_RESID_LEN	0x24
+#define I2C_FSI_SET_SCL		0x24
+#define I2C_FSI_PORT_BUSY	0x28
+#define I2C_FSI_RESET_SCL	0x2C
+#define I2C_FSI_SET_SDA		0x30
+#define I2C_FSI_RESET_SDA	0x34
+
+/* cmd register */
+#define I2C_CMD_WITH_START	0x80000000
+#define I2C_CMD_WITH_ADDR	0x40000000
+#define I2C_CMD_RD_CONT		0x20000000
+#define I2C_CMD_WITH_STOP	0x10000000
+#define I2C_CMD_FORCELAUNCH	0x08000000
+#define I2C_CMD_ADDR		0x00fe0000
+#define I2C_CMD_READ		0x00010000
+#define I2C_CMD_LEN		0x0000ffff
+
+/* mode register */
+#define I2C_MODE_CLKDIV		0xffff0000
+#define I2C_MODE_PORT		0x0000fc00
+#define I2C_MODE_ENHANCED	0x00000008
+#define I2C_MODE_DIAG		0x00000004
+#define I2C_MODE_PACE_ALLOW	0x00000002
+#define I2C_MODE_WRAP		0x00000001
+
+/* watermark register */
+#define I2C_WATERMARK_HI	0x0000f000
+#define I2C_WATERMARK_LO	0x000000f0
+
+#define I2C_FIFO_HI_LVL		4
+#define I2C_FIFO_LO_LVL		4
+
+/* interrupt register */
+#define I2C_INT_INV_CMD		0x00008000
+#define I2C_INT_PARITY		0x00004000
+#define I2C_INT_BE_OVERRUN	0x00002000
+#define I2C_INT_BE_ACCESS	0x00001000
+#define I2C_INT_LOST_ARB	0x00000800
+#define I2C_INT_NACK		0x00000400
+#define I2C_INT_DAT_REQ		0x00000200
+#define I2C_INT_CMD_COMP	0x00000100
+#define I2C_INT_STOP_ERR	0x00000080
+#define I2C_INT_BUSY		0x00000040
+#define I2C_INT_IDLE		0x00000020
+
+#define I2C_INT_ENABLE		0x0000ff80
+#define I2C_INT_ERR		0x0000fcc0
+
+/* status register */
+#define I2C_STAT_INV_CMD	0x80000000
+#define I2C_STAT_PARITY		0x40000000
+#define I2C_STAT_BE_OVERRUN	0x20000000
+#define I2C_STAT_BE_ACCESS	0x10000000
+#define I2C_STAT_LOST_ARB	0x08000000
+#define I2C_STAT_NACK		0x04000000
+#define I2C_STAT_DAT_REQ	0x02000000
+#define I2C_STAT_CMD_COMP	0x01000000
+#define I2C_STAT_STOP_ERR	0x00800000
+#define I2C_STAT_MAX_PORT	0x000f0000
+#define I2C_STAT_ANY_INT	0x00008000
+#define I2C_STAT_SCL_IN		0x00000800
+#define I2C_STAT_SDA_IN		0x00000400
+#define I2C_STAT_PORT_BUSY	0x00000200
+#define I2C_STAT_SELF_BUSY	0x00000100
+#define I2C_STAT_FIFO_COUNT	0x000000ff
+
+#define I2C_STAT_ERR		0xfc800000
+#define I2C_STAT_ANY_RESP	0xff800000
+
+/* extended status register */
+#define I2C_ESTAT_FIFO_SZ	0xff000000
+#define I2C_ESTAT_SCL_IN_SY	0x00008000
+#define I2C_ESTAT_SDA_IN_SY	0x00004000
+#define I2C_ESTAT_S_SCL		0x00002000
+#define I2C_ESTAT_S_SDA		0x00001000
+#define I2C_ESTAT_M_SCL		0x00000800
+#define I2C_ESTAT_M_SDA		0x00000400
+#define I2C_ESTAT_HI_WATER	0x00000200
+#define I2C_ESTAT_LO_WATER	0x00000100
+#define I2C_ESTAT_PORT_BUSY	0x00000080
+#define I2C_ESTAT_SELF_BUSY	0x00000040
+#define I2C_ESTAT_VERSION	0x0000001f
+
+struct fsi_i2c_master {
+	struct fsi_device	*fsi;
+	u8			fifo_size;
+};
+
+static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
+			    u32 *data)
+{
+	int rc;
+	__be32 data_be;
+
+	rc = fsi_device_read(fsi, reg, &data_be, sizeof(data_be));
+	if (rc)
+		return rc;
+
+	*data = be32_to_cpu(data_be);
+
+	return 0;
+}
+
+static int fsi_i2c_write_reg(struct fsi_device *fsi, unsigned int reg,
+			     u32 *data)
+{
+	__be32 data_be = cpu_to_be32(*data);
+
+	return fsi_device_write(fsi, reg, &data_be, sizeof(data_be));
+}
+
+static int fsi_i2c_dev_init(struct fsi_i2c_master *i2c)
+{
+	int rc;
+	u32 mode = I2C_MODE_ENHANCED, extended_status, watermark = 0;
+	u32 interrupt = 0;
+
+	/* since we use polling, disable interrupts */
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_INT_MASK, &interrupt);
+	if (rc)
+		return rc;
+
+	mode = SETFIELD(I2C_MODE_CLKDIV, mode, I2C_DEFAULT_CLK_DIV);
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
+	if (rc)
+		return rc;
+
+	rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_ESTAT, &extended_status);
+	if (rc)
+		return rc;
+
+	i2c->fifo_size = GETFIELD(I2C_ESTAT_FIFO_SZ, extended_status);
+	watermark = SETFIELD(I2C_WATERMARK_HI, watermark,
+			     i2c->fifo_size - I2C_FIFO_HI_LVL);
+	watermark = SETFIELD(I2C_WATERMARK_LO, watermark,
+			     I2C_FIFO_LO_LVL);
+
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_WATER_MARK, &watermark);
+
+	return rc;
+}
+
+static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+			int num)
+{
+	return -EOPNOTSUPP;
+}
+
+static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_10BIT_ADDR
+		| I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+static const struct i2c_algorithm fsi_i2c_algorithm = {
+	.master_xfer = fsi_i2c_xfer,
+	.functionality = fsi_i2c_functionality,
+};
+
+static int fsi_i2c_probe(struct device *dev)
+{
+	struct fsi_i2c_master *i2c;
+	int rc;
+
+	i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
+	if (!i2c)
+		return -ENOMEM;
+
+	i2c->fsi = to_fsi_dev(dev);
+
+	rc = fsi_i2c_dev_init(i2c);
+	if (rc)
+		return rc;
+
+	dev_set_drvdata(dev, i2c);
+
+	return 0;
+}
+
+static const struct fsi_device_id fsi_i2c_ids[] = {
+	{ FSI_ENGID_I2C, FSI_VERSION_ANY },
+	{ 0 }
+};
+
+static struct fsi_driver fsi_i2c_driver = {
+	.id_table = fsi_i2c_ids,
+	.drv = {
+		.name = "i2c-fsi",
+		.bus = &fsi_bus_type,
+		.probe = fsi_i2c_probe,
+	},
+};
+
+module_fsi_driver(fsi_i2c_driver);
+
+MODULE_AUTHOR("Eddie James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("FSI attached I2C master");
+MODULE_LICENSE("GPL");
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 2/6] drivers/i2c: Add port structure to FSI algorithm
  2017-07-26 17:13 [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm Eddie James
  2017-07-26 17:13   ` Eddie James
@ 2017-07-26 17:13 ` Eddie James
  2017-07-26 17:13 ` [PATCH v5 3/6] drivers/i2c: Add transfer implementation for " Eddie James
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2017-07-26 17:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, devicetree, wsa, robh+dt, jk, joel, andrew, cbostic,
	eajames, Edward A. James

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

Add and initialize I2C adapters for each port on the FSI-attached I2C
master. Ports for each master are defined in the devicetree.

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

diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
index 79475f8..4ad5d68 100644
--- a/drivers/i2c/busses/i2c-fsi.c
+++ b/drivers/i2c/busses/i2c-fsi.c
@@ -14,7 +14,9 @@
 #include <linux/fsi.h>
 #include <linux/i2c.h>
 #include <linux/kernel.h>
+#include <linux/list.h>
 #include <linux/module.h>
+#include <linux/of.h>
 
 #define FSI_ENGID_I2C		0x7
 
@@ -130,6 +132,14 @@
 struct fsi_i2c_master {
 	struct fsi_device	*fsi;
 	u8			fifo_size;
+	struct list_head	ports;
+};
+
+struct fsi_i2c_port {
+	struct list_head	list;
+	struct i2c_adapter	adapter;
+	struct fsi_i2c_master	*master;
+	u16			port;
 };
 
 static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
@@ -186,9 +196,44 @@ static int fsi_i2c_dev_init(struct fsi_i2c_master *i2c)
 	return rc;
 }
 
+static int fsi_i2c_set_port(struct fsi_i2c_port *port)
+{
+	int rc;
+	struct fsi_device *fsi = port->master->fsi;
+	u32 mode, dummy = 0;
+	u16 old_port;
+
+	rc = fsi_i2c_read_reg(fsi, I2C_FSI_MODE, &mode);
+	if (rc)
+		return rc;
+
+	old_port = GETFIELD(I2C_MODE_PORT, mode);
+
+	if (old_port != port->port) {
+		mode = SETFIELD(I2C_MODE_PORT, mode, port->port);
+		rc = fsi_i2c_write_reg(fsi, I2C_FSI_MODE, &mode);
+		if (rc)
+			return rc;
+
+		/* reset engine when port is changed */
+		rc = fsi_i2c_write_reg(fsi, I2C_FSI_RESET_ERR, &dummy);
+		if (rc)
+			return rc;
+	}
+
+	return rc;
+}
+
 static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 			int num)
 {
+	int rc;
+	struct fsi_i2c_port *port = adap->algo_data;
+
+	rc = fsi_i2c_set_port(port);
+	if (rc)
+		return rc;
+
 	return -EOPNOTSUPP;
 }
 
@@ -206,23 +251,73 @@ static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
 static int fsi_i2c_probe(struct device *dev)
 {
 	struct fsi_i2c_master *i2c;
+	struct fsi_i2c_port *port;
+	struct device_node *np;
 	int rc;
+	u32 port_no;
 
 	i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
 	if (!i2c)
 		return -ENOMEM;
 
 	i2c->fsi = to_fsi_dev(dev);
+	INIT_LIST_HEAD(&i2c->ports);
 
 	rc = fsi_i2c_dev_init(i2c);
 	if (rc)
 		return rc;
 
+	/* Add adapter for each i2c port of the master. */
+	for_each_available_child_of_node(dev->of_node, np) {
+		rc = of_property_read_u32(np, "reg", &port_no);
+		if (rc || port_no > USHRT_MAX)
+			continue;
+
+		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+		if (!port)
+			break;
+
+		port->master = i2c;
+		port->port = port_no;
+
+		port->adapter.owner = THIS_MODULE;
+		port->adapter.dev.of_node = np;
+		port->adapter.dev.parent = dev;
+		port->adapter.algo = &fsi_i2c_algorithm;
+		port->adapter.algo_data = port;
+
+		snprintf(port->adapter.name, sizeof(port->adapter.name),
+			 "i2c_bus-%u", port_no);
+
+		rc = i2c_add_adapter(&port->adapter);
+		if (rc < 0) {
+			dev_err(dev, "Failed to register adapter: %d\n", rc);
+			devm_kfree(dev, port);
+			continue;
+		}
+
+		list_add(&port->list, &i2c->ports);
+	}
+
 	dev_set_drvdata(dev, i2c);
 
 	return 0;
 }
 
+static int fsi_i2c_remove(struct device *dev)
+{
+	struct fsi_i2c_master *i2c = dev_get_drvdata(dev);
+	struct fsi_i2c_port *port;
+
+	if (!list_empty(&i2c->ports)) {
+		list_for_each_entry(port, &i2c->ports, list) {
+			i2c_del_adapter(&port->adapter);
+		}
+	}
+
+	return 0;
+}
+
 static const struct fsi_device_id fsi_i2c_ids[] = {
 	{ FSI_ENGID_I2C, FSI_VERSION_ANY },
 	{ 0 }
@@ -234,6 +329,7 @@ static int fsi_i2c_probe(struct device *dev)
 		.name = "i2c-fsi",
 		.bus = &fsi_bus_type,
 		.probe = fsi_i2c_probe,
+		.remove = fsi_i2c_remove,
 	},
 };
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 3/6] drivers/i2c: Add transfer implementation for FSI algorithm
  2017-07-26 17:13 [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm Eddie James
  2017-07-26 17:13   ` Eddie James
  2017-07-26 17:13 ` [PATCH v5 2/6] drivers/i2c: Add port structure to FSI algorithm Eddie James
@ 2017-07-26 17:13 ` Eddie James
  2017-07-26 17:13 ` [PATCH v5 4/6] drivers/i2c: Add I2C master locking to " Eddie James
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2017-07-26 17:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, devicetree, wsa, robh+dt, jk, joel, andrew, cbostic,
	eajames, Edward A. James

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

Execute I2C transfers from the FSI-attached I2C master. Use polling
instead of interrupts as we have no hardware IRQ over FSI.

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

diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
index 4ad5d68..d37f2c8 100644
--- a/drivers/i2c/busses/i2c-fsi.c
+++ b/drivers/i2c/busses/i2c-fsi.c
@@ -13,10 +13,12 @@
 #include <linux/errno.h>
 #include <linux/fsi.h>
 #include <linux/i2c.h>
+#include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/sched.h>
 
 #define FSI_ENGID_I2C		0x7
 
@@ -140,6 +142,7 @@ struct fsi_i2c_port {
 	struct i2c_adapter	adapter;
 	struct fsi_i2c_master	*master;
 	u16			port;
+	u16			xfrd;
 };
 
 static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
@@ -224,17 +227,207 @@ static int fsi_i2c_set_port(struct fsi_i2c_port *port)
 	return rc;
 }
 
+static int fsi_i2c_start(struct fsi_i2c_port *port, struct i2c_msg *msg,
+			 bool stop)
+{
+	int rc;
+	struct fsi_i2c_master *i2c = port->master;
+	u32 cmd = I2C_CMD_WITH_START | I2C_CMD_WITH_ADDR;
+
+	port->xfrd = 0;
+
+	if (msg->flags & I2C_M_RD)
+		cmd |= I2C_CMD_READ;
+
+	if (stop || msg->flags & I2C_M_STOP)
+		cmd |= I2C_CMD_WITH_STOP;
+
+	cmd = SETFIELD(I2C_CMD_ADDR, cmd, msg->addr >> 1);
+	cmd = SETFIELD(I2C_CMD_LEN, cmd, msg->len);
+
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_CMD, &cmd);
+
+	return rc;
+}
+
+static int fsi_i2c_write_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
+			      u8 fifo_count)
+{
+	int write;
+	int rc = 0;
+	struct fsi_i2c_master *i2c = port->master;
+	int bytes_to_write = i2c->fifo_size - fifo_count;
+	int bytes_remaining = msg->len - port->xfrd;
+
+	if (bytes_to_write > bytes_remaining)
+		bytes_to_write = bytes_remaining;
+
+	while (bytes_to_write > 0) {
+		write = bytes_to_write;
+		/* fsi limited to max 4 byte aligned ops */
+		if (bytes_to_write > 4)
+			write = 4;
+		else if (write == 3)
+			write = 2;
+
+		rc = fsi_device_write(i2c->fsi, I2C_FSI_FIFO,
+				      &msg->buf[port->xfrd], write);
+		if (rc)
+			return rc;
+
+		port->xfrd += write;
+		bytes_to_write -= write;
+	}
+
+	return rc;
+}
+
+static int fsi_i2c_read_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
+			     u8 fifo_count)
+{
+	int read;
+	int rc = 0;
+	struct fsi_i2c_master *i2c = port->master;
+	int xfr_remaining = msg->len - port->xfrd;
+	u32 dummy;
+
+	while (fifo_count) {
+		read = fifo_count;
+		/* fsi limited to max 4 byte aligned ops */
+		if (fifo_count > 4)
+			read = 4;
+		else if (read == 3)
+			read = 2;
+
+		if (xfr_remaining) {
+			if (xfr_remaining < read)
+				read = xfr_remaining;
+
+			rc = fsi_device_read(i2c->fsi, I2C_FSI_FIFO,
+					     &msg->buf[port->xfrd], read);
+			if (rc)
+				return rc;
+
+			port->xfrd += read;
+			xfr_remaining -= read;
+		} else {
+			/* no more buffer but data in fifo, need to clear it */
+			rc = fsi_device_read(i2c->fsi, I2C_FSI_FIFO, &dummy,
+					     read);
+			if (rc)
+				return rc;
+		}
+
+		fifo_count -= read;
+	}
+
+	return rc;
+}
+
+static int fsi_i2c_handle_status(struct fsi_i2c_port *port,
+				 struct i2c_msg *msg, u32 status)
+{
+	int rc;
+	u8 fifo_count;
+	struct fsi_i2c_master *i2c = port->master;
+	u32 dummy = 0;
+
+	if (status & I2C_STAT_ERR) {
+		rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_ERR, &dummy);
+		if (rc)
+			return rc;
+
+		if (status & I2C_STAT_NACK)
+			return -EFAULT;
+
+		return -EIO;
+	}
+
+	if (status & I2C_STAT_DAT_REQ) {
+		fifo_count = GETFIELD(I2C_STAT_FIFO_COUNT, status);
+
+		if (msg->flags & I2C_M_RD)
+			rc = fsi_i2c_read_fifo(port, msg, fifo_count);
+		else
+			rc = fsi_i2c_write_fifo(port, msg, fifo_count);
+
+		return rc;
+	}
+
+	if (status & I2C_STAT_CMD_COMP) {
+		if (port->xfrd < msg->len)
+			rc = -ENODATA;
+		else
+			rc = msg->len;
+
+		return rc;
+	}
+
+	return 0;
+}
+
+static int fsi_i2c_wait(struct fsi_i2c_port *port, struct i2c_msg *msg,
+			unsigned long timeout)
+{
+	const unsigned long local_timeout = 2; /* jiffies */
+	u32 status = 0;
+	int rc;
+
+	do {
+		rc = fsi_i2c_read_reg(port->master->fsi, I2C_FSI_STAT,
+				      &status);
+		if (rc)
+			return rc;
+
+		if (status & I2C_STAT_ANY_RESP) {
+			rc = fsi_i2c_handle_status(port, 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)
 {
-	int rc;
+	int i, rc;
+	unsigned long start_time;
 	struct fsi_i2c_port *port = adap->algo_data;
+	struct i2c_msg *msg;
 
 	rc = fsi_i2c_set_port(port);
 	if (rc)
 		return rc;
 
-	return -EOPNOTSUPP;
+	for (i = 0; i < num; ++i) {
+		msg = msgs + i;
+		start_time = jiffies;
+
+		rc = fsi_i2c_start(port, msg, i == num - 1);
+		if (rc)
+			return rc;
+
+		rc = fsi_i2c_wait(port, msg,
+				  adap->timeout - (jiffies - start_time));
+		if (rc)
+			return rc;
+	}
+
+	return 0;
 }
 
 static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 4/6] drivers/i2c: Add I2C master locking to FSI algorithm
  2017-07-26 17:13 [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm Eddie James
                   ` (2 preceding siblings ...)
  2017-07-26 17:13 ` [PATCH v5 3/6] drivers/i2c: Add transfer implementation for " Eddie James
@ 2017-07-26 17:13 ` Eddie James
  2017-07-26 17:13 ` [PATCH v5 5/6] drivers/i2c: Add bus recovery for " Eddie James
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2017-07-26 17:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, devicetree, wsa, robh+dt, jk, joel, andrew, cbostic,
	eajames, Edward A. James

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

Since there are many ports per master, each with it's own adapter and
chardev, we need some locking to prevent transfers from changing the\
master state while other transfers are in progress.

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

diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
index d37f2c8..3a1f317 100644
--- a/drivers/i2c/busses/i2c-fsi.c
+++ b/drivers/i2c/busses/i2c-fsi.c
@@ -19,6 +19,8 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/sched.h>
+#include <linux/semaphore.h>
+#include <linux/wait.h>
 
 #define FSI_ENGID_I2C		0x7
 
@@ -135,6 +137,8 @@ struct fsi_i2c_master {
 	struct fsi_device	*fsi;
 	u8			fifo_size;
 	struct list_head	ports;
+	wait_queue_head_t	wait;
+	struct semaphore	lock;
 };
 
 struct fsi_i2c_port {
@@ -168,6 +172,29 @@ static int fsi_i2c_write_reg(struct fsi_device *fsi, unsigned int reg,
 	return fsi_device_write(fsi, reg, &data_be, sizeof(data_be));
 }
 
+static int fsi_i2c_lock_master(struct fsi_i2c_master *i2c, int timeout)
+{
+	int rc;
+
+	rc = down_trylock(&i2c->lock);
+	if (!rc)
+		return 0;
+
+	rc = wait_event_interruptible_timeout(i2c->wait,
+					      !down_trylock(&i2c->lock),
+					      timeout);
+	if (rc > 0)
+		return 0;
+
+	return -EBUSY;
+}
+
+static void fsi_i2c_unlock_master(struct fsi_i2c_master *i2c)
+{
+	up(&i2c->lock);
+	wake_up(&i2c->wait);
+}
+
 static int fsi_i2c_dev_init(struct fsi_i2c_master *i2c)
 {
 	int rc;
@@ -409,25 +436,31 @@ static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 	struct fsi_i2c_port *port = adap->algo_data;
 	struct i2c_msg *msg;
 
-	rc = fsi_i2c_set_port(port);
+	rc = fsi_i2c_lock_master(port->master, adap->timeout);
 	if (rc)
 		return rc;
 
+	rc = fsi_i2c_set_port(port);
+	if (rc)
+		goto unlock;
+
 	for (i = 0; i < num; ++i) {
 		msg = msgs + i;
 		start_time = jiffies;
 
 		rc = fsi_i2c_start(port, msg, i == num - 1);
 		if (rc)
-			return rc;
+			goto unlock;
 
 		rc = fsi_i2c_wait(port, msg,
 				  adap->timeout - (jiffies - start_time));
 		if (rc)
-			return rc;
+			goto unlock;
 	}
 
-	return 0;
+unlock:
+	fsi_i2c_unlock_master(port->master);
+	return rc;
 }
 
 static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
@@ -453,6 +486,8 @@ static int fsi_i2c_probe(struct device *dev)
 	if (!i2c)
 		return -ENOMEM;
 
+	init_waitqueue_head(&i2c->wait);
+	sema_init(&i2c->lock, 1);
 	i2c->fsi = to_fsi_dev(dev);
 	INIT_LIST_HEAD(&i2c->ports);
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 5/6] drivers/i2c: Add bus recovery for FSI algorithm
  2017-07-26 17:13 [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm Eddie James
                   ` (3 preceding siblings ...)
  2017-07-26 17:13 ` [PATCH v5 4/6] drivers/i2c: Add I2C master locking to " Eddie James
@ 2017-07-26 17:13 ` Eddie James
  2017-07-26 17:13   ` Eddie James
  2017-08-15  6:36 ` [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm Peter Rosin
  6 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2017-07-26 17:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, devicetree, wsa, robh+dt, jk, joel, andrew, cbostic,
	eajames, Edward A. James

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

Bus recovery should reset the engine and force clock the bus 9 times
to recover most situations.

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

diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
index 3a1f317..1af9c01 100644
--- a/drivers/i2c/busses/i2c-fsi.c
+++ b/drivers/i2c/busses/i2c-fsi.c
@@ -469,6 +469,80 @@ static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
 		| I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
 }
 
+static int fsi_i2c_low_level_recover_bus(struct fsi_i2c_master *i2c)
+{
+	int i, rc;
+	u32 mode, dummy = 0;
+
+	rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_MODE, &mode);
+	if (rc)
+		return rc;
+
+	mode |= I2C_MODE_DIAG;
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
+	if (rc)
+		return rc;
+
+	for (i = 0; i < 9; ++i) {
+		rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_SCL, &dummy);
+		if (rc)
+			return rc;
+
+		rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_SET_SCL, &dummy);
+		if (rc)
+			return rc;
+	}
+
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_SCL, &dummy);
+	if (rc)
+		return rc;
+
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_SDA, &dummy);
+	if (rc)
+		return rc;
+
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_SET_SCL, &dummy);
+	if (rc)
+		return rc;
+
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_SET_SDA, &dummy);
+	if (rc)
+		return rc;
+
+	mode &= ~I2C_MODE_DIAG;
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
+
+	return rc;
+}
+
+static int fsi_i2c_recover_bus(struct i2c_adapter *adap)
+{
+	int rc;
+	u32 dummy = 0;
+	struct fsi_i2c_port *port = adap->algo_data;
+	struct fsi_i2c_master *i2c = port->master;
+
+	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_I2C, &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_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_ERR, &dummy);
+
+	return rc;
+}
+
+static struct i2c_bus_recovery_info fsi_i2c_bus_recovery_info = {
+	.recover_bus = fsi_i2c_recover_bus,
+};
+
 static const struct i2c_algorithm fsi_i2c_algorithm = {
 	.master_xfer = fsi_i2c_xfer,
 	.functionality = fsi_i2c_functionality,
@@ -512,6 +586,7 @@ static int fsi_i2c_probe(struct device *dev)
 		port->adapter.dev.of_node = np;
 		port->adapter.dev.parent = dev;
 		port->adapter.algo = &fsi_i2c_algorithm;
+		port->adapter.bus_recovery_info = &fsi_i2c_bus_recovery_info;
 		port->adapter.algo_data = port;
 
 		snprintf(port->adapter.name, sizeof(port->adapter.name),
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 6/6] dt-bindings: i2c: Add FSI-attached I2C master dt binding documentation
@ 2017-07-26 17:13   ` Eddie James
  0 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2017-07-26 17:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, devicetree, wsa, robh+dt, jk, joel, andrew, cbostic,
	eajames, Edward A. James

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

Document the bindings.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/i2c/i2c-fsi.txt | 40 +++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-fsi.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-fsi.txt b/Documentation/devicetree/bindings/i2c/i2c-fsi.txt
new file mode 100644
index 0000000..b1be2ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-fsi.txt
@@ -0,0 +1,40 @@
+Device-tree bindings for FSI-attached I2C master and busses
+-----------------------------------------------------------
+
+Required properties:
+ - compatible = "ibm,i2c-fsi";
+ - reg = < address size >;		: The FSI CFAM address and address
+					  space size.
+ - #address-cells = <1>;		: Number of address cells in child
+					  nodes.
+ - #size-cells = <0>;			: Number of size cells in child nodes.
+ - child nodes				: Nodes to describe busses off the I2C
+					  master.
+
+Child node required properties:
+ - reg = < port number >		: The port number on the I2C master.
+
+Child node optional properties:
+ - child nodes				: Nodes to describe devices on the I2C
+					  bus.
+
+Examples:
+
+    i2c@1800 {
+        compatible = "ibm,i2c-fsi";
+        reg = < 0x1800 0x400 >;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c-bus@0 {
+            reg = <0>;
+        };
+
+        i2c-bus@1 {
+            reg = <1>;
+
+            eeprom@50 {
+                compatible = "vendor,dev-name";
+            };
+        };
+    };
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 6/6] dt-bindings: i2c: Add FSI-attached I2C master dt binding documentation
@ 2017-07-26 17:13   ` Eddie James
  0 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2017-07-26 17:13 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, jk-mnsaURCQ41sdnm+yROfE0A,
	joel-U3u1mxZcP9KHXe+LvDLADg, andrew-zrmu5oMJ5Fs,
	cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	eajames-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Edward A. James

From: "Edward A. James" <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Document the bindings.

Signed-off-by: Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/i2c/i2c-fsi.txt | 40 +++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-fsi.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-fsi.txt b/Documentation/devicetree/bindings/i2c/i2c-fsi.txt
new file mode 100644
index 0000000..b1be2ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-fsi.txt
@@ -0,0 +1,40 @@
+Device-tree bindings for FSI-attached I2C master and busses
+-----------------------------------------------------------
+
+Required properties:
+ - compatible = "ibm,i2c-fsi";
+ - reg = < address size >;		: The FSI CFAM address and address
+					  space size.
+ - #address-cells = <1>;		: Number of address cells in child
+					  nodes.
+ - #size-cells = <0>;			: Number of size cells in child nodes.
+ - child nodes				: Nodes to describe busses off the I2C
+					  master.
+
+Child node required properties:
+ - reg = < port number >		: The port number on the I2C master.
+
+Child node optional properties:
+ - child nodes				: Nodes to describe devices on the I2C
+					  bus.
+
+Examples:
+
+    i2c@1800 {
+        compatible = "ibm,i2c-fsi";
+        reg = < 0x1800 0x400 >;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c-bus@0 {
+            reg = <0>;
+        };
+
+        i2c-bus@1 {
+            reg = <1>;
+
+            eeprom@50 {
+                compatible = "vendor,dev-name";
+            };
+        };
+    };
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm
  2017-07-26 17:13 [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm Eddie James
                   ` (5 preceding siblings ...)
  2017-07-26 17:13   ` Eddie James
@ 2017-08-15  6:36 ` Peter Rosin
  2017-08-15  8:10   ` Joel Stanley
  6 siblings, 1 reply; 15+ messages in thread
From: Peter Rosin @ 2017-08-15  6:36 UTC (permalink / raw)
  To: Eddie James, linux-kernel
  Cc: linux-i2c, devicetree, wsa, robh+dt, jk, joel, andrew, cbostic,
	Edward A. James

On 2017-07-26 19:13, Eddie James wrote:
> From: "Edward A. James" <eajames@us.ibm.com>
> 
> This series adds an algorithm for an I2C master physically located on an FSI
> slave device. The I2C master has multiple ports, each of which may be connected
> to an I2C slave. Access to the I2C master registers is achieved over FSI bus.
> 
> Due to the multi-port nature of the I2C master, the driver instantiates a new
> I2C adapter for each port connected to a slave. The connected ports should be
> defined in the device tree under the I2C master device.

Hmmm, AFAIU fsi is a bus, and on this bus you have some "client" device that
happens to be an i2c master, and this is a driver for that "client". Is it
totally inconceivable to have some other client device in the future that is
implementing an i2c master differently, but still using the fsi bus?

With that in mind, is it wise to pick the driver name from the bus that the
device is connected to, and nothing else without further qualification?

I don't see any "i2c-usb" driver, but I think there are a couple of i2c master
drivers that communicate via usb.

Cheers,
Peter

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm
  2017-08-15  6:36 ` [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm Peter Rosin
@ 2017-08-15  8:10   ` Joel Stanley
  2017-08-15 16:28     ` Christopher Bostic
  0 siblings, 1 reply; 15+ messages in thread
From: Joel Stanley @ 2017-08-15  8:10 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Eddie James, Linux Kernel Mailing List, linux-i2c, devicetree,
	Wolfram Sang, Rob Herring, Jeremy Kerr, Andrew Jeffery,
	Christopher Bostic, Edward A. James

On Tue, Aug 15, 2017 at 4:06 PM, Peter Rosin <peda@axentia.se> wrote:
> On 2017-07-26 19:13, Eddie James wrote:
>> From: "Edward A. James" <eajames@us.ibm.com>
>>
>> This series adds an algorithm for an I2C master physically located on an FSI
>> slave device. The I2C master has multiple ports, each of which may be connected
>> to an I2C slave. Access to the I2C master registers is achieved over FSI bus.
>>
>> Due to the multi-port nature of the I2C master, the driver instantiates a new
>> I2C adapter for each port connected to a slave. The connected ports should be
>> defined in the device tree under the I2C master device.
>
> Hmmm, AFAIU fsi is a bus, and on this bus you have some "client" device that
> happens to be an i2c master, and this is a driver for that "client". Is it
> totally inconceivable to have some other client device in the future that is
> implementing an i2c master differently, but still using the fsi bus?
>
> With that in mind, is it wise to pick the driver name from the bus that the
> device is connected to, and nothing else without further qualification?
>
> I don't see any "i2c-usb" driver, but I think there are a couple of i2c master
> drivers that communicate via usb.

You make a fair point. When I did a prototype of this driver I called
it i2c-cfam, as it is part of the CFAM hardware unit inside of the
Power8/Power9 processors.

The documentation does call it FSI_I2CM, so that's an argument for the
current name.

I'm not sure how accurate that name is. Chris, Eddie, do you have any
other suggestions?

Cheers,

Joel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm
  2017-08-15  8:10   ` Joel Stanley
@ 2017-08-15 16:28     ` Christopher Bostic
  2017-08-15 17:35       ` Peter Rosin
  0 siblings, 1 reply; 15+ messages in thread
From: Christopher Bostic @ 2017-08-15 16:28 UTC (permalink / raw)
  To: Joel Stanley, Peter Rosin
  Cc: Eddie James, Linux Kernel Mailing List, linux-i2c, devicetree,
	Wolfram Sang, Rob Herring, Jeremy Kerr, Andrew Jeffery,
	Edward A. James



On 8/15/17 3:10 AM, Joel Stanley wrote:
> On Tue, Aug 15, 2017 at 4:06 PM, Peter Rosin <peda@axentia.se> wrote:
>> On 2017-07-26 19:13, Eddie James wrote:
>>> From: "Edward A. James" <eajames@us.ibm.com>
>>>
>>> This series adds an algorithm for an I2C master physically located on an FSI
>>> slave device. The I2C master has multiple ports, each of which may be connected
>>> to an I2C slave. Access to the I2C master registers is achieved over FSI bus.
>>>
>>> Due to the multi-port nature of the I2C master, the driver instantiates a new
>>> I2C adapter for each port connected to a slave. The connected ports should be
>>> defined in the device tree under the I2C master device.
>> Hmmm, AFAIU fsi is a bus, and on this bus you have some "client" device that
>> happens to be an i2c master, and this is a driver for that "client". Is it
>> totally inconceivable to have some other client device in the future that is
>> implementing an i2c master differently, but still using the fsi bus?
>>
>> With that in mind, is it wise to pick the driver name from the bus that the
>> device is connected to, and nothing else without further qualification?
>>
>> I don't see any "i2c-usb" driver, but I think there are a couple of i2c master
>> drivers that communicate via usb.
> You make a fair point. When I did a prototype of this driver I called
> it i2c-cfam, as it is part of the CFAM hardware unit inside of the
> Power8/Power9 processors.
>
> The documentation does call it FSI_I2CM, so that's an argument for the
> current name.
>
> I'm not sure how accurate that name is. Chris, Eddie, do you have any
> other suggestions?
The I2C engine up to now has been always accessed via the FSI bus so 
historically I assume that's why its labelled as FSI_I2CM in the p8/p9 
specs.  There isn't any reason this I2C device couldn't be implemented 
in some other topology independent of FSI / CFAMs.  In other words there 
are no FSI details internal to this I2C engine, an argument for removing 
the 'FSI' tag.

Thanks,
Chris

>
> Cheers,
>
> Joel
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm
  2017-08-15 16:28     ` Christopher Bostic
@ 2017-08-15 17:35       ` Peter Rosin
  2017-08-15 17:53           ` Eddie James
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Rosin @ 2017-08-15 17:35 UTC (permalink / raw)
  To: Christopher Bostic, Joel Stanley
  Cc: Eddie James, Linux Kernel Mailing List, linux-i2c, devicetree,
	Wolfram Sang, Rob Herring, Jeremy Kerr, Andrew Jeffery,
	Edward A. James

On 2017-08-15 18:28, Christopher Bostic wrote:
> On 8/15/17 3:10 AM, Joel Stanley wrote:
>> On Tue, Aug 15, 2017 at 4:06 PM, Peter Rosin <peda@axentia.se> wrote:
>>> On 2017-07-26 19:13, Eddie James wrote:
>>>> From: "Edward A. James" <eajames@us.ibm.com>
>>>>
>>>> This series adds an algorithm for an I2C master physically located on an FSI
>>>> slave device. The I2C master has multiple ports, each of which may be connected
>>>> to an I2C slave. Access to the I2C master registers is achieved over FSI bus.
>>>>
>>>> Due to the multi-port nature of the I2C master, the driver instantiates a new
>>>> I2C adapter for each port connected to a slave. The connected ports should be
>>>> defined in the device tree under the I2C master device.
>>> Hmmm, AFAIU fsi is a bus, and on this bus you have some "client" device that
>>> happens to be an i2c master, and this is a driver for that "client". Is it
>>> totally inconceivable to have some other client device in the future that is
>>> implementing an i2c master differently, but still using the fsi bus?
>>>
>>> With that in mind, is it wise to pick the driver name from the bus that the
>>> device is connected to, and nothing else without further qualification?
>>>
>>> I don't see any "i2c-usb" driver, but I think there are a couple of i2c master
>>> drivers that communicate via usb.
>> You make a fair point. When I did a prototype of this driver I called
>> it i2c-cfam, as it is part of the CFAM hardware unit inside of the
>> Power8/Power9 processors.
>>
>> The documentation does call it FSI_I2CM, so that's an argument for the
>> current name.
>>
>> I'm not sure how accurate that name is. Chris, Eddie, do you have any
>> other suggestions?
> The I2C engine up to now has been always accessed via the FSI bus so 
> historically I assume that's why its labelled as FSI_I2CM in the p8/p9 
> specs.  There isn't any reason this I2C device couldn't be implemented 
> in some other topology independent of FSI / CFAMs.  In other words there 
> are no FSI details internal to this I2C engine, an argument for removing 
> the 'FSI' tag.

Note that I wasn't primarily concerned with this i2c engine growing some other
non-fsi interface (like many devices have both i2c and spi interface versions).
I was more concerned with some future and totally different i2c engine that
naturally sports a totally different register map but still uses the fsi bus.

But you have a point. If this i2c engine evolves and ends up supporting some
other interface, then that too would be cause to regret the i2c-fsi name.

Cheers,
peda

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm
@ 2017-08-15 17:53           ` Eddie James
  0 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2017-08-15 17:53 UTC (permalink / raw)
  To: Peter Rosin, Christopher Bostic, Joel Stanley
  Cc: Linux Kernel Mailing List, linux-i2c, devicetree, Wolfram Sang,
	Rob Herring, Jeremy Kerr, Andrew Jeffery, Edward A. James



On 08/15/2017 12:35 PM, Peter Rosin wrote:
> On 2017-08-15 18:28, Christopher Bostic wrote:
>> On 8/15/17 3:10 AM, Joel Stanley wrote:
>>> On Tue, Aug 15, 2017 at 4:06 PM, Peter Rosin <peda@axentia.se> wrote:
>>>> On 2017-07-26 19:13, Eddie James wrote:
>>>>> From: "Edward A. James" <eajames@us.ibm.com>
>>>>>
>>>>> This series adds an algorithm for an I2C master physically located on an FSI
>>>>> slave device. The I2C master has multiple ports, each of which may be connected
>>>>> to an I2C slave. Access to the I2C master registers is achieved over FSI bus.
>>>>>
>>>>> Due to the multi-port nature of the I2C master, the driver instantiates a new
>>>>> I2C adapter for each port connected to a slave. The connected ports should be
>>>>> defined in the device tree under the I2C master device.
>>>> Hmmm, AFAIU fsi is a bus, and on this bus you have some "client" device that
>>>> happens to be an i2c master, and this is a driver for that "client". Is it
>>>> totally inconceivable to have some other client device in the future that is
>>>> implementing an i2c master differently, but still using the fsi bus?
>>>>
>>>> With that in mind, is it wise to pick the driver name from the bus that the
>>>> device is connected to, and nothing else without further qualification?
>>>>
>>>> I don't see any "i2c-usb" driver, but I think there are a couple of i2c master
>>>> drivers that communicate via usb.
>>> You make a fair point. When I did a prototype of this driver I called
>>> it i2c-cfam, as it is part of the CFAM hardware unit inside of the
>>> Power8/Power9 processors.
>>>
>>> The documentation does call it FSI_I2CM, so that's an argument for the
>>> current name.
>>>
>>> I'm not sure how accurate that name is. Chris, Eddie, do you have any
>>> other suggestions?
>> The I2C engine up to now has been always accessed via the FSI bus so
>> historically I assume that's why its labelled as FSI_I2CM in the p8/p9
>> specs.  There isn't any reason this I2C device couldn't be implemented
>> in some other topology independent of FSI / CFAMs.  In other words there
>> are no FSI details internal to this I2C engine, an argument for removing
>> the 'FSI' tag.
> Note that I wasn't primarily concerned with this i2c engine growing some other
> non-fsi interface (like many devices have both i2c and spi interface versions).
> I was more concerned with some future and totally different i2c engine that
> naturally sports a totally different register map but still uses the fsi bus.
>
> But you have a point. If this i2c engine evolves and ends up supporting some
> other interface, then that too would be cause to regret the i2c-fsi name.

i2c-cfam would work, though it would also be possible for another type 
of i2c engine to exist on a CFAM... it has been done in the past.

Historically, this driver was called "iic-boe" for "Boeblingen". I'm not 
suggesting it's a good name for this driver, but it is unique...

Edduie

>
> Cheers,
> peda
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm
@ 2017-08-15 17:53           ` Eddie James
  0 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2017-08-15 17:53 UTC (permalink / raw)
  To: Peter Rosin, Christopher Bostic, Joel Stanley
  Cc: Linux Kernel Mailing List, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree, Wolfram Sang, Rob Herring, Jeremy Kerr,
	Andrew Jeffery, Edward A. James



On 08/15/2017 12:35 PM, Peter Rosin wrote:
> On 2017-08-15 18:28, Christopher Bostic wrote:
>> On 8/15/17 3:10 AM, Joel Stanley wrote:
>>> On Tue, Aug 15, 2017 at 4:06 PM, Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org> wrote:
>>>> On 2017-07-26 19:13, Eddie James wrote:
>>>>> From: "Edward A. James" <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>>>>>
>>>>> This series adds an algorithm for an I2C master physically located on an FSI
>>>>> slave device. The I2C master has multiple ports, each of which may be connected
>>>>> to an I2C slave. Access to the I2C master registers is achieved over FSI bus.
>>>>>
>>>>> Due to the multi-port nature of the I2C master, the driver instantiates a new
>>>>> I2C adapter for each port connected to a slave. The connected ports should be
>>>>> defined in the device tree under the I2C master device.
>>>> Hmmm, AFAIU fsi is a bus, and on this bus you have some "client" device that
>>>> happens to be an i2c master, and this is a driver for that "client". Is it
>>>> totally inconceivable to have some other client device in the future that is
>>>> implementing an i2c master differently, but still using the fsi bus?
>>>>
>>>> With that in mind, is it wise to pick the driver name from the bus that the
>>>> device is connected to, and nothing else without further qualification?
>>>>
>>>> I don't see any "i2c-usb" driver, but I think there are a couple of i2c master
>>>> drivers that communicate via usb.
>>> You make a fair point. When I did a prototype of this driver I called
>>> it i2c-cfam, as it is part of the CFAM hardware unit inside of the
>>> Power8/Power9 processors.
>>>
>>> The documentation does call it FSI_I2CM, so that's an argument for the
>>> current name.
>>>
>>> I'm not sure how accurate that name is. Chris, Eddie, do you have any
>>> other suggestions?
>> The I2C engine up to now has been always accessed via the FSI bus so
>> historically I assume that's why its labelled as FSI_I2CM in the p8/p9
>> specs.  There isn't any reason this I2C device couldn't be implemented
>> in some other topology independent of FSI / CFAMs.  In other words there
>> are no FSI details internal to this I2C engine, an argument for removing
>> the 'FSI' tag.
> Note that I wasn't primarily concerned with this i2c engine growing some other
> non-fsi interface (like many devices have both i2c and spi interface versions).
> I was more concerned with some future and totally different i2c engine that
> naturally sports a totally different register map but still uses the fsi bus.
>
> But you have a point. If this i2c engine evolves and ends up supporting some
> other interface, then that too would be cause to regret the i2c-fsi name.

i2c-cfam would work, though it would also be possible for another type 
of i2c engine to exist on a CFAM... it has been done in the past.

Historically, this driver was called "iic-boe" for "Boeblingen". I'm not 
suggesting it's a good name for this driver, but it is unique...

Edduie

>
> Cheers,
> peda
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-08-15 17:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-26 17:13 [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm Eddie James
2017-07-26 17:13 ` [PATCH v5 1/6] " Eddie James
2017-07-26 17:13   ` Eddie James
2017-07-26 17:13 ` [PATCH v5 2/6] drivers/i2c: Add port structure to FSI algorithm Eddie James
2017-07-26 17:13 ` [PATCH v5 3/6] drivers/i2c: Add transfer implementation for " Eddie James
2017-07-26 17:13 ` [PATCH v5 4/6] drivers/i2c: Add I2C master locking to " Eddie James
2017-07-26 17:13 ` [PATCH v5 5/6] drivers/i2c: Add bus recovery for " Eddie James
2017-07-26 17:13 ` [PATCH v5 6/6] dt-bindings: i2c: Add FSI-attached I2C master dt binding documentation Eddie James
2017-07-26 17:13   ` Eddie James
2017-08-15  6:36 ` [PATCH v5 0/6] drivers/i2c: Add FSI-attached I2C master algorithm Peter Rosin
2017-08-15  8:10   ` Joel Stanley
2017-08-15 16:28     ` Christopher Bostic
2017-08-15 17:35       ` Peter Rosin
2017-08-15 17:53         ` Eddie James
2017-08-15 17:53           ` Eddie James

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.