linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/5] ARM: Add GXP I2C Support
@ 2023-01-25 18:44 nick.hawkins
  2023-01-25 18:44 ` [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller nick.hawkins
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: nick.hawkins @ 2023-01-25 18:44 UTC (permalink / raw)
  To: verdun, nick.hawkins, robh+dt, krzysztof.kozlowski+dt, linux,
	linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel

From: Nick Hawkins <nick.hawkins@hpe.com>

The GXP SoC supports 10 I2C engines. Each I2C engine is completely
independent and can function both as an I2C master and I2C slave. The
I2C master can operate in a multi master environment. The engines support
a scalable speed from 8kHZ to 1.5 Mhz.

---

Changes since v3:
 *Switch engine variable to u32
 *Disable IRQ on device remove with register write instead
 *Provided even greater description with the use of Phandle
Changes since v2:
 *Disable IRQ on a device remove
 *Remove use of I2C_CLASS_DEPRECATED
 *Use i2c_parse_fw_timings instead of of_property_read_u32
 *Remove redundant dev_err_probe as platform_get_irq already has one
 *Used __iomem instead of res->start to find physical address
 *Use BIT in gxp_i2c_irq_handler
 *Made value u8 instead of u16 for u8 read
 *Provided a better description of Phandle in yaml
Changes since v1:
 *Removed yaml documentation of hpe,gxp-sysreg as it has been
  applied to syscon.yaml
 *Made i2cX a generic node name i2c in dts file
 *Added status field to the dtsi and the dts for i2c bus
 *Removed unnecessary size-cells and address-cells from yaml
 *Removed phandle from hpe,sysreg-phandle
 *Changed hpe,i2c-max-bus-freq to clock-frequency
 *Removed rogue tab in structure definition
 *Removed use of __iomem *base local variables as it was
  unnecessary
 *Switched #if IS_ENABLED() -> if (IS_ENABLED()) inside
  functions
 *Removed use of pr_* functions
 *Removed informational prints in register and unregister
  functions
 *Removed print from interrupt handler
 *Removed informational prints from probe function
 *Switched dev_err -> dev_err_probe in probe function
 *Used the respective helper for mapping the resource to
  __iomem

Nick Hawkins (5):
  i2c: hpe: Add GXP SoC I2C Controller
  dt-bindings: i2c: Add hpe,gxp-i2c
  ARM: dts: hpe: Add I2C Topology
  ARM: multi_v7_defconfig: add gxp i2c module
  MAINTAINERS: Add HPE GXP I2C Support

 .../devicetree/bindings/i2c/hpe,gxp-i2c.yaml  |  59 ++
 MAINTAINERS                                   |   2 +
 arch/arm/boot/dts/hpe-bmc-dl360gen10.dts      | 109 ++++
 arch/arm/boot/dts/hpe-gxp.dtsi                | 125 ++++
 arch/arm/configs/multi_v7_defconfig           |   1 +
 drivers/i2c/busses/Kconfig                    |   7 +
 drivers/i2c/busses/Makefile                   |   1 +
 drivers/i2c/busses/i2c-gxp.c                  | 603 ++++++++++++++++++
 8 files changed, 907 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml
 create mode 100644 drivers/i2c/busses/i2c-gxp.c

-- 
2.17.1


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

* [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller
  2023-01-25 18:44 [PATCH v4 0/5] ARM: Add GXP I2C Support nick.hawkins
@ 2023-01-25 18:44 ` nick.hawkins
  2023-02-15 20:29   ` Wolfram Sang
  2023-01-25 18:44 ` [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c nick.hawkins
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: nick.hawkins @ 2023-01-25 18:44 UTC (permalink / raw)
  To: verdun, nick.hawkins, robh+dt, krzysztof.kozlowski+dt, linux,
	linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel

From: Nick Hawkins <nick.hawkins@hpe.com>

The GXP SoC supports 10 I2C engines. Each I2C engine is completely
independent and can function both as an I2C master and I2C slave. The
I2C master can operate in a multi master environment. The engines support
a scalable speed from 8kHZ to 1.5 Mhz.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>

---

v4:
 *Disable IRQ on device remove by register write
 *Switch engine variable to u32
v3:
 *Disable IRQ on device remove
 *Remove use of I2C_CLASS_DEPRECATED
 *Use i2c_parse_fw_timings instead of of_property_read_u32
 *Remove redundant dev_err_probe as platform_get_irq already has one
 *Used __iomem instead of res->start to find physical address
 *Use BIT in gxp_i2c_irq_handler
 *Made value u8 instead of u16
v2:
 *Removed rogue tab in structure
 *Removed use of __iomem base local variables as it was unnecessary
 *Switched #if IS_ENABLED() -> if (IS_ENABLED()) inside functions
 *Removed use of pr_* functions
 *Removed informational prints in register and unregister functions
 *Removed print from interrupt handler
 *No informational prints in probe function
 *Switched dev_err for dev_err_probe in probe function
 *Use respective helper for mapping the resource to __iomem
---
 drivers/i2c/busses/Kconfig   |   7 +
 drivers/i2c/busses/Makefile  |   1 +
 drivers/i2c/busses/i2c-gxp.c | 603 +++++++++++++++++++++++++++++++++++
 3 files changed, 611 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-gxp.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index e50f9603d189..8b3951e0ca5c 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1457,4 +1457,11 @@ config I2C_VIRTIO
           This driver can also be built as a module. If so, the module
           will be called i2c-virtio.
 
+config I2C_GXP
+	tristate "GXP I2C Interface"
+	depends on ARCH_HPE_GXP || COMPILE_TEST
+	help
+	  This enables support for GXP I2C interface. The I2C engines can be
+	  either I2C master or I2C slaves.
+
 endmenu
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index e73cdb1d2b5a..dcc96eab6d68 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -127,6 +127,7 @@ obj-$(CONFIG_I2C_THUNDERX)	+= i2c-thunderx.o
 obj-$(CONFIG_I2C_XILINX)	+= i2c-xiic.o
 obj-$(CONFIG_I2C_XLP9XX)	+= i2c-xlp9xx.o
 obj-$(CONFIG_I2C_RCAR)		+= i2c-rcar.o
+obj-$(CONFIG_I2C_GXP)		+= i2c-gxp.o
 
 # External I2C/SMBus adapter drivers
 obj-$(CONFIG_I2C_DIOLAN_U2C)	+= i2c-diolan-u2c.o
diff --git a/drivers/i2c/busses/i2c-gxp.c b/drivers/i2c/busses/i2c-gxp.c
new file mode 100644
index 000000000000..37272261ce8d
--- /dev/null
+++ b/drivers/i2c/busses/i2c-gxp.c
@@ -0,0 +1,603 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P. */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+
+#define GXP_MAX_I2C_ENGINE 10
+static const char * const gxp_i2c_name[] = {
+	"gxp-i2c0", "gxp-i2c1", "gxp-i2c2", "gxp-i2c3",
+	"gxp-i2c4", "gxp-i2c5", "gxp-i2c6", "gxp-i2c7",
+	"gxp-i2c8", "gxp-i2c9" };
+
+/* GXP I2C Global interrupt status/enable register*/
+#define GXP_I2CINTSTAT		0x00
+#define GXP_I2CINTEN		0x04
+
+/* GXP I2C registers */
+#define GXP_I2CSTAT		0x00
+#define MASK_STOP_EVENT		0x20
+#define MASK_ACK		0x08
+#define MASK_RW			0x04
+#define GXP_I2CEVTERR		0x01
+#define MASK_SLAVE_CMD_EVENT	0x01
+#define MASK_SLAVE_DATA_EVENT	0x02
+#define MASK_MASTER_EVENT	0x10
+#define GXP_I2CSNPDAT		0x02
+#define GXP_I2CMCMD		0x04
+#define GXP_I2CSCMD		0x06
+#define GXP_I2CSNPAA		0x09
+#define GXP_I2CADVFEAT		0x0A
+#define GXP_I2COWNADR		0x0B
+#define GXP_I2CFREQDIV		0x0C
+#define GXP_I2CFLTFAIR		0x0D
+#define GXP_I2CTMOEDG		0x0E
+#define GXP_I2CCYCTIM		0x0F
+
+static bool i2c_global_init_done;
+
+enum {
+	GXP_I2C_IDLE = 0,
+	GXP_I2C_ADDR_PHASE,
+	GXP_I2C_RDATA_PHASE,
+	GXP_I2C_WDATA_PHASE,
+	GXP_I2C_ADDR_NACK,
+	GXP_I2C_DATA_NACK,
+	GXP_I2C_ERROR,
+	GXP_I2C_COMP
+};
+
+struct gxp_i2c_drvdata {
+	struct device *dev;
+	void __iomem *base;
+	struct i2c_timings t;
+	u32 engine;
+	int irq;
+	struct completion completion;
+	struct i2c_adapter adapter;
+	struct i2c_msg *curr_msg;
+	int msgs_remaining;
+	int msgs_num;
+	u8 *buf;
+	size_t buf_remaining;
+	unsigned char state;
+	struct i2c_client *slave;
+	unsigned char stopped;
+};
+
+static struct regmap *i2cg_map;
+
+static void gxp_i2c_start(struct gxp_i2c_drvdata *drvdata)
+{
+	u16 value;
+
+	drvdata->buf = drvdata->curr_msg->buf;
+	drvdata->buf_remaining = drvdata->curr_msg->len;
+
+	/* Note: Address in struct i2c_msg is 7 bits */
+	value = drvdata->curr_msg->addr << 9;
+
+	if (drvdata->curr_msg->flags & I2C_M_RD) {
+		/* Read */
+		value |= 0x05;
+	} else {
+		/* Write */
+		value |= 0x01;
+	}
+
+	drvdata->state = GXP_I2C_ADDR_PHASE;
+	writew(value, drvdata->base + GXP_I2CMCMD);
+}
+
+static int gxp_i2c_master_xfer(struct i2c_adapter *adapter,
+			       struct i2c_msg *msgs, int num)
+{
+	int ret;
+	struct gxp_i2c_drvdata *drvdata = i2c_get_adapdata(adapter);
+	unsigned long time_left;
+
+	drvdata->msgs_remaining = num;
+	drvdata->curr_msg = msgs;
+	drvdata->msgs_num = num;
+	reinit_completion(&drvdata->completion);
+
+	gxp_i2c_start(drvdata);
+
+	time_left = wait_for_completion_timeout(&drvdata->completion,
+						adapter->timeout);
+	ret = num - drvdata->msgs_remaining;
+	if (time_left == 0) {
+		switch (drvdata->state) {
+		case GXP_I2C_WDATA_PHASE:
+			dev_err(drvdata->dev,
+				"gxp_i2c_start:write Data phase timeout at msg[%d]\n",
+			ret);
+			break;
+		case GXP_I2C_RDATA_PHASE:
+			dev_err(drvdata->dev,
+				"gxp_i2c_start:read Data phase timeout at msg[%d]\n",
+			ret);
+			break;
+		case GXP_I2C_ADDR_PHASE:
+			dev_err(drvdata->dev,
+				"gxp_i2c_start:Addr phase timeout\n");
+			break;
+		default:
+			dev_err(drvdata->dev,
+				"gxp_i2c_start:i2c transfer timeout state=%d\n",
+			drvdata->state);
+			break;
+		}
+		return -ETIMEDOUT;
+	}
+
+	if (drvdata->state == GXP_I2C_ADDR_NACK) {
+		dev_err(drvdata->dev,
+			"gxp_i2c_start:No ACK for address phase\n");
+		return -EIO;
+	} else if (drvdata->state == GXP_I2C_DATA_NACK) {
+		dev_err(drvdata->dev, "gxp_i2c_start:No ACK for data phase\n");
+		return -EIO;
+	}
+
+	return ret;
+}
+
+static u32 gxp_i2c_func(struct i2c_adapter *adap)
+{
+	if (IS_ENABLED(CONFIG_I2C_SLAVE))
+		return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SLAVE;
+
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static int gxp_i2c_reg_slave(struct i2c_client *slave)
+{
+	struct gxp_i2c_drvdata *drvdata = i2c_get_adapdata(slave->adapter);
+
+	if (drvdata->slave)
+		return -EBUSY;
+
+	if (slave->flags & I2C_CLIENT_TEN)
+		return -EAFNOSUPPORT;
+
+	drvdata->slave = slave;
+
+	writeb(slave->addr << 1, drvdata->base + GXP_I2COWNADR);
+	writeb(0x69, drvdata->base + GXP_I2CSCMD);
+
+	return 0;
+}
+
+static int gxp_i2c_unreg_slave(struct i2c_client *slave)
+{
+	struct gxp_i2c_drvdata *drvdata = i2c_get_adapdata(slave->adapter);
+
+	WARN_ON(!drvdata->slave);
+
+	writeb(0x00, drvdata->base + GXP_I2COWNADR);
+	writeb(0xF0, drvdata->base + GXP_I2CSCMD);
+
+	drvdata->slave = NULL;
+
+	return 0;
+}
+#endif
+
+static const struct i2c_algorithm gxp_i2c_algo = {
+	.master_xfer   = gxp_i2c_master_xfer,
+	.functionality = gxp_i2c_func,
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	.reg_slave     = gxp_i2c_reg_slave,
+	.unreg_slave   = gxp_i2c_unreg_slave,
+#endif
+};
+
+static void gxp_i2c_stop(struct gxp_i2c_drvdata *drvdata)
+{
+	/* Clear event and send stop */
+	writeb(0x82, drvdata->base + GXP_I2CMCMD);
+
+	complete(&drvdata->completion);
+}
+
+static void gxp_i2c_restart(struct gxp_i2c_drvdata *drvdata)
+{
+	u16 value;
+
+	drvdata->buf = drvdata->curr_msg->buf;
+	drvdata->buf_remaining = drvdata->curr_msg->len;
+
+	value = drvdata->curr_msg->addr << 9;
+
+	if (drvdata->curr_msg->flags & I2C_M_RD) {
+		/* Read and clear master event */
+		value |= 0x85;
+	} else {
+		/* Write and clear master event */
+		value |= 0x81;
+	}
+
+	drvdata->state = GXP_I2C_ADDR_PHASE;
+
+	writew(value, drvdata->base + GXP_I2CMCMD);
+}
+
+static void gxp_i2c_chk_addr_ack(struct gxp_i2c_drvdata *drvdata)
+{
+	u16 value;
+
+	value = readb(drvdata->base + GXP_I2CSTAT);
+	if (!(value & MASK_ACK)) {
+		/* Got no ack, stop */
+		drvdata->state = GXP_I2C_ADDR_NACK;
+		gxp_i2c_stop(drvdata);
+		return;
+	}
+
+	if (drvdata->curr_msg->flags & I2C_M_RD) {
+		/* Start to read data from slave */
+		if (drvdata->buf_remaining == 0) {
+			/* No more data to read, stop */
+			drvdata->msgs_remaining--;
+			drvdata->state = GXP_I2C_COMP;
+			gxp_i2c_stop(drvdata);
+			return;
+		}
+		drvdata->state = GXP_I2C_RDATA_PHASE;
+
+		if (drvdata->buf_remaining == 1) {
+			/* The last data, do not ack */
+			writeb(0x84, drvdata->base + GXP_I2CMCMD);
+		} else {
+			/* Read data and ack it */
+			writeb(0x8C, drvdata->base + GXP_I2CMCMD);
+		}
+	} else {
+		/* Start to write first data to slave */
+		if (drvdata->buf_remaining == 0) {
+			/* No more data to write, stop */
+			drvdata->msgs_remaining--;
+			drvdata->state = GXP_I2C_COMP;
+			gxp_i2c_stop(drvdata);
+			return;
+		}
+		value = *drvdata->buf;
+		value = value << 8;
+		/* Clear master event */
+		value |= 0x80;
+		drvdata->buf++;
+		drvdata->buf_remaining--;
+		drvdata->state = GXP_I2C_WDATA_PHASE;
+		writew(value, drvdata->base + GXP_I2CMCMD);
+	}
+}
+
+static void gxp_i2c_ack_data(struct gxp_i2c_drvdata *drvdata)
+{
+	u8 value;
+
+	/* Store the data returned */
+	value = readb(drvdata->base + GXP_I2CSNPDAT);
+	*drvdata->buf = value;
+	drvdata->buf++;
+	drvdata->buf_remaining--;
+
+	if (drvdata->buf_remaining == 0) {
+		/* No more data, this message is completed. */
+		drvdata->msgs_remaining--;
+
+		if (drvdata->msgs_remaining == 0) {
+			/* No more messages, stop */
+			drvdata->state = GXP_I2C_COMP;
+			gxp_i2c_stop(drvdata);
+			return;
+		}
+		/* Move to next message and start transfer */
+		drvdata->curr_msg++;
+		gxp_i2c_restart(drvdata);
+		return;
+	}
+
+	/* Ack the slave to make it send next byte */
+	drvdata->state = GXP_I2C_RDATA_PHASE;
+	if (drvdata->buf_remaining == 1) {
+		/* The last data, do not ack */
+		writeb(0x84, drvdata->base + GXP_I2CMCMD);
+	} else {
+		/* Read data and ack it */
+		writeb(0x8C, drvdata->base + GXP_I2CMCMD);
+	}
+}
+
+static void gxp_i2c_chk_data_ack(struct gxp_i2c_drvdata *drvdata)
+{
+	u16 value;
+
+	value = readb(drvdata->base + GXP_I2CSTAT);
+	if (!(value & MASK_ACK)) {
+		/* Received No ack, stop */
+		drvdata->state = GXP_I2C_DATA_NACK;
+		gxp_i2c_stop(drvdata);
+		return;
+	}
+
+	/* Got ack, check if there is more data to write */
+	if (drvdata->buf_remaining == 0) {
+		/* No more data, this message is completed */
+		drvdata->msgs_remaining--;
+
+		if (drvdata->msgs_remaining == 0) {
+			/* No more messages, stop */
+			drvdata->state = GXP_I2C_COMP;
+			gxp_i2c_stop(drvdata);
+			return;
+		}
+		/* Move to next message and start transfer */
+		drvdata->curr_msg++;
+		gxp_i2c_restart(drvdata);
+		return;
+	}
+
+	/* Write data to slave */
+	value = *drvdata->buf;
+	value = value << 8;
+
+	/* Clear master event */
+	value |= 0x80;
+	drvdata->buf++;
+	drvdata->buf_remaining--;
+	drvdata->state = GXP_I2C_WDATA_PHASE;
+	writew(value, drvdata->base + GXP_I2CMCMD);
+}
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static bool gxp_i2c_slave_irq_handler(struct gxp_i2c_drvdata *drvdata)
+{
+	u8 value;
+	u8 buf;
+	int ret;
+
+	value = readb(drvdata->base + GXP_I2CEVTERR);
+
+	/* Received start or stop event */
+	if (value & MASK_SLAVE_CMD_EVENT) {
+		value = readb(drvdata->base + GXP_I2CSTAT);
+		/* Master sent stop */
+		if (value & MASK_STOP_EVENT) {
+			if (drvdata->stopped == 0)
+				i2c_slave_event(drvdata->slave, I2C_SLAVE_STOP, &buf);
+			writeb(0x69, drvdata->base + GXP_I2CSCMD);
+			drvdata->stopped = 1;
+		} else {
+			/* Master sent start and  wants to read */
+			drvdata->stopped = 0;
+			if (value & MASK_RW) {
+				i2c_slave_event(drvdata->slave,
+						I2C_SLAVE_READ_REQUESTED, &buf);
+				value = buf << 8 | 0x61;
+				writew(value, drvdata->base + GXP_I2CSCMD);
+			} else {
+				/* Master wants to write to us */
+				ret = i2c_slave_event(drvdata->slave,
+						      I2C_SLAVE_WRITE_REQUESTED, &buf);
+				if (!ret) {
+					/* Ack next byte from master */
+					writeb(0x69, drvdata->base + GXP_I2CSCMD);
+				} else {
+					/* Nack next byte from master */
+					writeb(0x61, drvdata->base + GXP_I2CSCMD);
+				}
+			}
+		}
+	} else if (value & MASK_SLAVE_DATA_EVENT) {
+		value = readb(drvdata->base + GXP_I2CSTAT);
+		/* Master wants to read */
+		if (value & MASK_RW) {
+			/* Master wants another byte */
+			if (value & MASK_ACK) {
+				i2c_slave_event(drvdata->slave,
+						I2C_SLAVE_READ_PROCESSED, &buf);
+				value = buf << 8 | 0x61;
+				writew(value, drvdata->base + GXP_I2CSCMD);
+			} else {
+				/* No more bytes needed */
+				writew(0x69, drvdata->base + GXP_I2CSCMD);
+			}
+		} else {
+			/* Master wants to write to us */
+			value = readb(drvdata->base + GXP_I2CSNPDAT);
+			buf = (uint8_t)value;
+			ret = i2c_slave_event(drvdata->slave,
+					      I2C_SLAVE_WRITE_RECEIVED, &buf);
+			if (!ret) {
+				/* Ack next byte from master */
+				writeb(0x69, drvdata->base + GXP_I2CSCMD);
+			} else {
+				/* Nack next byte from master */
+				writeb(0x61, drvdata->base + GXP_I2CSCMD);
+			}
+		}
+	} else {
+		return false;
+	}
+
+	return true;
+}
+#endif
+
+static irqreturn_t gxp_i2c_irq_handler(int irq, void *_drvdata)
+{
+	struct gxp_i2c_drvdata *drvdata = (struct gxp_i2c_drvdata *)_drvdata;
+	u32 value;
+
+	/* Check if the interrupt is for the current engine */
+	regmap_read(i2cg_map, GXP_I2CINTSTAT, &value);
+	if (!(value & BIT(drvdata->engine)))
+		return IRQ_NONE;
+
+	value = readb(drvdata->base + GXP_I2CEVTERR);
+
+	/* Error */
+	if (value & ~(MASK_MASTER_EVENT | MASK_SLAVE_CMD_EVENT |
+				MASK_SLAVE_DATA_EVENT)) {
+		/* Clear all events */
+		writeb(0x00, drvdata->base + GXP_I2CEVTERR);
+		drvdata->state = GXP_I2C_ERROR;
+		gxp_i2c_stop(drvdata);
+		return IRQ_HANDLED;
+	}
+
+	if (IS_ENABLED(CONFIG_I2C_SLAVE)) {
+		/* Slave mode */
+		if (value & (MASK_SLAVE_CMD_EVENT | MASK_SLAVE_DATA_EVENT)) {
+			if (gxp_i2c_slave_irq_handler(drvdata))
+				return IRQ_HANDLED;
+			return IRQ_NONE;
+		}
+	}
+
+	/*  Master mode */
+	switch (drvdata->state) {
+	case GXP_I2C_ADDR_PHASE:
+		gxp_i2c_chk_addr_ack(drvdata);
+		break;
+
+	case GXP_I2C_RDATA_PHASE:
+		gxp_i2c_ack_data(drvdata);
+		break;
+
+	case GXP_I2C_WDATA_PHASE:
+		gxp_i2c_chk_data_ack(drvdata);
+		break;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void gxp_i2c_init(struct gxp_i2c_drvdata *drvdata)
+{
+	drvdata->state = GXP_I2C_IDLE;
+	writeb(2000000 / drvdata->t.bus_freq_hz,
+	       drvdata->base + GXP_I2CFREQDIV);
+	writeb(0x32, drvdata->base + GXP_I2CFLTFAIR);
+	writeb(0x0a, drvdata->base + GXP_I2CTMOEDG);
+	writeb(0x00, drvdata->base + GXP_I2CCYCTIM);
+	writeb(0x00, drvdata->base + GXP_I2CSNPAA);
+	writeb(0x00, drvdata->base + GXP_I2CADVFEAT);
+	writeb(0xF0, drvdata->base + GXP_I2CSCMD);
+	writeb(0x80, drvdata->base + GXP_I2CMCMD);
+	writeb(0x00, drvdata->base + GXP_I2CEVTERR);
+	writeb(0x00, drvdata->base + GXP_I2COWNADR);
+}
+
+static int gxp_i2c_probe(struct platform_device *pdev)
+{
+	struct gxp_i2c_drvdata *drvdata;
+	int rc;
+	struct i2c_adapter *adapter;
+
+	if (!i2c_global_init_done) {
+		i2cg_map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+							   "hpe,sysreg");
+		if (IS_ERR(i2cg_map)) {
+			return dev_err_probe(&pdev->dev, IS_ERR(i2cg_map),
+					     "failed to map i2cg_handle\n");
+		}
+
+		/* Disable interrupt */
+		regmap_update_bits(i2cg_map, GXP_I2CINTEN, 0x00000FFF, 0);
+		i2c_global_init_done = true;
+	}
+
+	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata),
+			       GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, drvdata);
+	drvdata->dev = &pdev->dev;
+	init_completion(&drvdata->completion);
+
+	drvdata->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(drvdata->base))
+		return PTR_ERR(drvdata->base);
+
+	/* Use physical memory address to determine which I2C engine this is. */
+	drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8;
+
+	if (drvdata->engine >= GXP_MAX_I2C_ENGINE) {
+		return dev_err_probe(&pdev->dev, -EINVAL, "i2c engine% is unsupported\n",
+			drvdata->engine);
+	}
+
+	rc = platform_get_irq(pdev, 0);
+	if (rc < 0)
+		return rc;
+
+	drvdata->irq = rc;
+	rc = devm_request_irq(&pdev->dev, drvdata->irq, gxp_i2c_irq_handler,
+			      IRQF_SHARED, gxp_i2c_name[drvdata->engine], drvdata);
+	if (rc < 0)
+		return dev_err_probe(&pdev->dev, rc, "irq request failed\n");
+
+	i2c_parse_fw_timings(&pdev->dev, &drvdata->t, true);
+
+	gxp_i2c_init(drvdata);
+
+	/* Enable interrupt */
+	regmap_update_bits(i2cg_map, GXP_I2CINTEN, BIT(drvdata->engine),
+			   BIT(drvdata->engine));
+
+	adapter = &drvdata->adapter;
+	i2c_set_adapdata(adapter, drvdata);
+
+	adapter->owner = THIS_MODULE;
+	strscpy(adapter->name, "HPE GXP I2C adapter", sizeof(adapter->name));
+	adapter->algo = &gxp_i2c_algo;
+	adapter->dev.parent = &pdev->dev;
+	adapter->dev.of_node = pdev->dev.of_node;
+
+	rc = i2c_add_adapter(adapter);
+	if (rc)
+		return dev_err_probe(&pdev->dev, rc, "i2c add adapter failed\n");
+
+	return 0;
+}
+
+static int gxp_i2c_remove(struct platform_device *pdev)
+{
+	struct gxp_i2c_drvdata *drvdata = platform_get_drvdata(pdev);
+
+	/* Disable interrupt */
+	regmap_update_bits(i2cg_map, GXP_I2CINTEN, BIT(drvdata->engine), 0);
+	i2c_del_adapter(&drvdata->adapter);
+
+	return 0;
+}
+
+static const struct of_device_id gxp_i2c_of_match[] = {
+	{ .compatible = "hpe,gxp-i2c" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, gxp_i2c_of_match);
+
+static struct platform_driver gxp_i2c_driver = {
+	.probe	= gxp_i2c_probe,
+	.remove = gxp_i2c_remove,
+	.driver = {
+		.name = "gxp-i2c",
+		.of_match_table = gxp_i2c_of_match,
+	},
+};
+module_platform_driver(gxp_i2c_driver);
+
+MODULE_AUTHOR("Nick Hawkins <nick.hawkins@hpe.com>");
+MODULE_DESCRIPTION("HPE GXP I2C bus driver");
+MODULE_LICENSE("GPL");
-- 
2.17.1


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

* [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c
  2023-01-25 18:44 [PATCH v4 0/5] ARM: Add GXP I2C Support nick.hawkins
  2023-01-25 18:44 ` [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller nick.hawkins
@ 2023-01-25 18:44 ` nick.hawkins
  2023-01-25 21:18   ` Rob Herring
  2023-01-25 18:44 ` [PATCH v4 3/5] ARM: dts: hpe: Add I2C Topology nick.hawkins
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: nick.hawkins @ 2023-01-25 18:44 UTC (permalink / raw)
  To: verdun, nick.hawkins, robh+dt, krzysztof.kozlowski+dt, linux,
	linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel

From: Nick Hawkins <nick.hawkins@hpe.com>

Document compatibility string to support I2C controller
in GXP.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v4:
 *Provide even greater description with the use
  of Phandle
 *Reorder properties so they match the required
  order
v3:
 *Provide better description with use of Phandle
v2:
 *Removed uneccessary size-cells and address-cells
 *Removed phandle from hpe,sysreg-phandle
 *Changed hpe,i2c-max-bus-freq to clock-frequency
---
 .../devicetree/bindings/i2c/hpe,gxp-i2c.yaml  | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml

diff --git a/Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml b/Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml
new file mode 100644
index 000000000000..6604dcd47251
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/hpe,gxp-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HPE GXP SoC I2C Controller
+
+maintainers:
+  - Nick Hawkins <nick.hawkins@hpe.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    const: hpe,gxp-i2c
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clock-frequency:
+    default: 100000
+
+  hpe,sysreg:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to the global status and enable interrupt registers shared
+      between each I2C engine controller instance. It enables the I2C
+      engine controller to act as both a master or slave by being able to
+      arm and respond to interrupts from its engine. Each bit in the
+      registers represent the respective bit position.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c@2600 {
+        compatible = "hpe,gxp-i2c";
+        reg = <0x2500 0x70>;
+        interrupts = <9>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        hpe,sysreg = <&sysreg_system_controller>;
+        clock-frequency = <10000>;
+
+        eeprom@50 {
+            compatible = "atmel,24c128";
+            reg = <0x50>;
+        };
+    };
-- 
2.17.1


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

* [PATCH v4 3/5] ARM: dts: hpe: Add I2C Topology
  2023-01-25 18:44 [PATCH v4 0/5] ARM: Add GXP I2C Support nick.hawkins
  2023-01-25 18:44 ` [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller nick.hawkins
  2023-01-25 18:44 ` [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c nick.hawkins
@ 2023-01-25 18:44 ` nick.hawkins
  2023-01-25 18:44 ` [PATCH v4 4/5] ARM: multi_v7_defconfig: add gxp i2c module nick.hawkins
  2023-01-25 18:44 ` [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support nick.hawkins
  4 siblings, 0 replies; 15+ messages in thread
From: nick.hawkins @ 2023-01-25 18:44 UTC (permalink / raw)
  To: verdun, nick.hawkins, robh+dt, krzysztof.kozlowski+dt, linux,
	linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel

From: Nick Hawkins <nick.hawkins@hpe.com>

Add 9 I2C Engines, 2 MUXs, and a EEPROM to the device tree.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---

v4:
 *No change
v3:
 *No change
v2:
 *Made i2cX a generic node name i2c in the dts
 *Added status field to dtsi and dts for i2c bus
---
 arch/arm/boot/dts/hpe-bmc-dl360gen10.dts | 109 ++++++++++++++++++++
 arch/arm/boot/dts/hpe-gxp.dtsi           | 125 +++++++++++++++++++++++
 2 files changed, 234 insertions(+)

diff --git a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
index 3a7382ce40ef..1f2547fe9ae3 100644
--- a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
+++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
@@ -23,4 +23,113 @@
 		device_type = "memory";
 		reg = <0x40000000 0x20000000>;
 	};
+
+	i2cmux@4 {
+		compatible = "i2c-mux-reg";
+		i2c-parent = <&i2c4>;
+		reg = <0xd1000074 0x1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c@4 {
+			reg = <4>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+	};
+
+	i2cmux@6 {
+		compatible = "i2c-mux-reg";
+		i2c-parent = <&i2c6>;
+		reg = <0xd1000076 0x1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c@2 {
+			reg = <2>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c@4 {
+			reg = <4>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c@5 {
+			reg = <5>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+	};
+};
+
+&i2c0 {
+	status = "okay";
+};
+
+&i2c1 {
+	status = "okay";
+};
+
+&i2c2 {
+	status = "okay";
+	eeprom@50 {
+		compatible = "atmel,24c02";
+		pagesize = <8>;
+		reg = <0x50>;
+	};
+};
+
+&i2c3 {
+	status = "okay";
+};
+
+&i2c4 {
+	status = "okay";
+};
+
+&i2c5 {
+	status = "okay";
+};
+
+&i2c6 {
+	status = "okay";
+};
+
+&i2c7 {
+	status = "okay";
+};
+
+&i2c8 {
+	status = "okay";
+};
+
+&i2c9 {
+	status = "okay";
 };
diff --git a/arch/arm/boot/dts/hpe-gxp.dtsi b/arch/arm/boot/dts/hpe-gxp.dtsi
index cf735b3c4f35..3bc071149bae 100644
--- a/arch/arm/boot/dts/hpe-gxp.dtsi
+++ b/arch/arm/boot/dts/hpe-gxp.dtsi
@@ -122,6 +122,131 @@
 				interrupts = <6>;
 				interrupt-parent = <&vic0>;
 			};
+
+			sysreg_system_controller: syscon@f8 {
+				compatible = "hpe,gxp-sysreg", "syscon";
+				reg = <0xf8 0x8>;
+			};
+
+			i2c0: i2c@2000 {
+				compatible = "hpe,gxp-i2c";
+				reg = <0x2000 0x70>;
+				interrupts = <9>;
+				interrupt-parent = <&vic0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				status = "disabled";
+				hpe,sysreg = <&sysreg_system_controller>;
+				clock-frequency = <100000>;
+			};
+
+			i2c1: i2c@2100 {
+				compatible = "hpe,gxp-i2c";
+				reg = <0x2100 0x70>;
+				interrupts = <9>;
+				interrupt-parent = <&vic0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				status = "disabled";
+				hpe,sysreg = <&sysreg_system_controller>;
+				clock-frequency = <100000>;
+			};
+
+			i2c2: i2c@2200 {
+				compatible = "hpe,gxp-i2c";
+				reg = <0x2200 0x70>;
+				interrupts = <9>;
+				interrupt-parent = <&vic0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				status = "disabled";
+				hpe,sysreg = <&sysreg_system_controller>;
+				clock-frequency = <100000>;
+			};
+
+			i2c3: i2c@2300 {
+				compatible = "hpe,gxp-i2c";
+				reg = <0x2300 0x70>;
+				interrupts = <9>;
+				interrupt-parent = <&vic0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				status = "disabled";
+				hpe,sysreg = <&sysreg_system_controller>;
+				clock-frequency = <100000>;
+			};
+
+			i2c4: i2c@2400 {
+				compatible = "hpe,gxp-i2c";
+				reg = <0x2400 0x70>;
+				interrupts = <9>;
+				interrupt-parent = <&vic0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				status = "disabled";
+				hpe,sysreg = <&sysreg_system_controller>;
+				clock-frequency = <100000>;
+			};
+
+			i2c5: i2c@2500 {
+				compatible = "hpe,gxp-i2c";
+				reg = <0x2500 0x70>;
+				interrupts = <9>;
+				interrupt-parent = <&vic0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				status = "disabled";
+				hpe,sysreg = <&sysreg_system_controller>;
+				clock-frequency = <100000>;
+			};
+
+			i2c6: i2c@2600 {
+				compatible = "hpe,gxp-i2c";
+				reg = <0x2600 0x70>;
+				interrupts = <9>;
+				interrupt-parent = <&vic0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				status = "disabled";
+				hpe,sysreg = <&sysreg_system_controller>;
+				clock-frequency = <100000>;
+			};
+
+			i2c7: i2c@2700 {
+				compatible = "hpe,gxp-i2c";
+				reg = <0x2700 0x70>;
+				interrupts = <9>;
+				interrupt-parent = <&vic0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				status = "disabled";
+				hpe,sysreg = <&sysreg_system_controller>;
+				clock-frequency = <100000>;
+			};
+
+			i2c8: i2c@2800 {
+				compatible = "hpe,gxp-i2c";
+				reg = <0x2800 0x70>;
+				interrupts = <9>;
+				interrupt-parent = <&vic0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				status = "disabled";
+				hpe,sysreg = <&sysreg_system_controller>;
+				clock-frequency = <100000>;
+			};
+
+			i2c9: i2c@2900 {
+				compatible = "hpe,gxp-i2c";
+				reg = <0x2900 0x70>;
+				interrupts = <9>;
+				interrupt-parent = <&vic0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				status = "disabled";
+				hpe,sysreg = <&sysreg_system_controller>;
+				clock-frequency = <100000>;
+			};
 		};
 	};
 };
-- 
2.17.1


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

* [PATCH v4 4/5] ARM: multi_v7_defconfig: add gxp i2c module
  2023-01-25 18:44 [PATCH v4 0/5] ARM: Add GXP I2C Support nick.hawkins
                   ` (2 preceding siblings ...)
  2023-01-25 18:44 ` [PATCH v4 3/5] ARM: dts: hpe: Add I2C Topology nick.hawkins
@ 2023-01-25 18:44 ` nick.hawkins
  2023-01-25 18:44 ` [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support nick.hawkins
  4 siblings, 0 replies; 15+ messages in thread
From: nick.hawkins @ 2023-01-25 18:44 UTC (permalink / raw)
  To: verdun, nick.hawkins, robh+dt, krzysztof.kozlowski+dt, linux,
	linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel

From: Nick Hawkins <nick.hawkins@hpe.com>

Add the CONFIG_I2C_GXP symbol to enable the GXP SoC I2C capabilities.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---

v4:
 *No change
v3:
 *No change
v2:
 *No change
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index b61b2e3d116b..8a19b1dc10d0 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -411,6 +411,7 @@ CONFIG_I2C_DAVINCI=y
 CONFIG_I2C_DESIGNWARE_PLATFORM=y
 CONFIG_I2C_DIGICOLOR=m
 CONFIG_I2C_EMEV2=m
+CONFIG_I2C_GXP=m
 CONFIG_I2C_IMX=y
 CONFIG_I2C_MESON=y
 CONFIG_I2C_MV64XXX=y
-- 
2.17.1


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

* [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support
  2023-01-25 18:44 [PATCH v4 0/5] ARM: Add GXP I2C Support nick.hawkins
                   ` (3 preceding siblings ...)
  2023-01-25 18:44 ` [PATCH v4 4/5] ARM: multi_v7_defconfig: add gxp i2c module nick.hawkins
@ 2023-01-25 18:44 ` nick.hawkins
  2023-01-28 18:38   ` Wolfram Sang
  4 siblings, 1 reply; 15+ messages in thread
From: nick.hawkins @ 2023-01-25 18:44 UTC (permalink / raw)
  To: verdun, nick.hawkins, robh+dt, krzysztof.kozlowski+dt, linux,
	linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel

From: Nick Hawkins <nick.hawkins@hpe.com>

Add the I2C controller source and bindings.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---

v4:
 *No change
v3:
 *No change
v2:
 *No change
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1daadaa4d48b..d671a8b6968e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2217,12 +2217,14 @@ M:	Jean-Marie Verdun <verdun@hpe.com>
 M:	Nick Hawkins <nick.hawkins@hpe.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/arm/hpe,gxp.yaml
+F:	Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml
 F:	Documentation/devicetree/bindings/spi/hpe,gxp-spifi.yaml
 F:	Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
 F:	arch/arm/boot/dts/hpe-bmc*
 F:	arch/arm/boot/dts/hpe-gxp*
 F:	arch/arm/mach-hpe/
 F:	drivers/clocksource/timer-gxp.c
+F:	drivers/i2c/busses/i2c-gxp.c
 F:	drivers/spi/spi-gxp.c
 F:	drivers/watchdog/gxp-wdt.c
 
-- 
2.17.1


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

* Re: [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c
  2023-01-25 18:44 ` [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c nick.hawkins
@ 2023-01-25 21:18   ` Rob Herring
  2023-01-25 21:31     ` Hawkins, Nick
  0 siblings, 1 reply; 15+ messages in thread
From: Rob Herring @ 2023-01-25 21:18 UTC (permalink / raw)
  To: nick.hawkins
  Cc: verdun, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree,
	linux-kernel, linux-arm-kernel, joel

On Wed, Jan 25, 2023 at 12:44:35PM -0600, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Document compatibility string to support I2C controller
> in GXP.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> v4:
>  *Provide even greater description with the use
>   of Phandle
>  *Reorder properties so they match the required
>   order
> v3:
>  *Provide better description with use of Phandle
> v2:
>  *Removed uneccessary size-cells and address-cells
>  *Removed phandle from hpe,sysreg-phandle
>  *Changed hpe,i2c-max-bus-freq to clock-frequency
> ---
>  .../devicetree/bindings/i2c/hpe,gxp-i2c.yaml  | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml b/Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml
> new file mode 100644
> index 000000000000..6604dcd47251
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/hpe,gxp-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HPE GXP SoC I2C Controller
> +
> +maintainers:
> +  - Nick Hawkins <nick.hawkins@hpe.com>
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: hpe,gxp-i2c
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clock-frequency:
> +    default: 100000
> +
> +  hpe,sysreg:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to the global status and enable interrupt registers shared
> +      between each I2C engine controller instance. It enables the I2C
> +      engine controller to act as both a master or slave by being able to
> +      arm and respond to interrupts from its engine. Each bit in the
> +      registers represent the respective bit position.

Each bit represents the bit position?

AIUI, each I2C instance has a bit in it needs to control. How does the 
driver know what instance (and therefore the correct bit)? Typically you 
would have a 2nd cell here with that information.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c@2600 {
> +        compatible = "hpe,gxp-i2c";
> +        reg = <0x2500 0x70>;
> +        interrupts = <9>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        hpe,sysreg = <&sysreg_system_controller>;
> +        clock-frequency = <10000>;
> +
> +        eeprom@50 {
> +            compatible = "atmel,24c128";
> +            reg = <0x50>;
> +        };
> +    };
> -- 
> 2.17.1
> 

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

* Re: [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c
  2023-01-25 21:18   ` Rob Herring
@ 2023-01-25 21:31     ` Hawkins, Nick
  2023-01-26 13:38       ` Rob Herring
  0 siblings, 1 reply; 15+ messages in thread
From: Hawkins, Nick @ 2023-01-25 21:31 UTC (permalink / raw)
  To: Rob Herring
  Cc: Verdun, Jean-Marie, krzysztof.kozlowski+dt, linux, linux-i2c,
	devicetree, linux-kernel, linux-arm-kernel, joel

> > + hpe,sysreg:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description:
> > + Phandle to the global status and enable interrupt registers shared
> > + between each I2C engine controller instance. It enables the I2C
> > + engine controller to act as both a master or slave by being able to
> > + arm and respond to interrupts from its engine. Each bit in the
> > + registers represent the respective bit position.


> Each bit represents the bit position?

Yes what I mean here is that bit 0 represents engine 0, bit 1 represents
engine 1 and so on. I will reword this how you have below.

> AIUI, each I2C instance has a bit in it needs to control. How does the 
> driver know what instance (and therefore the correct bit)? Typically you 
> would have a 2nd cell here with that information.

We are currently using the memory area designated reg to determine
which engine we are on.

Here is a snippet from patch 1 of this patchset that introduces the driver:
/* Use physical memory address to determine which I2C engine this is. */
+	drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8;

This works because each engine is 0x100 apart.

I would however like to conform to a standard to designate the engine.
Is there an existing property I can leverage?

Thanks for your feedback,

-Nick Hawkins


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

* Re: [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c
  2023-01-25 21:31     ` Hawkins, Nick
@ 2023-01-26 13:38       ` Rob Herring
  0 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2023-01-26 13:38 UTC (permalink / raw)
  To: Hawkins, Nick
  Cc: Verdun, Jean-Marie, krzysztof.kozlowski+dt, linux, linux-i2c,
	devicetree, linux-kernel, linux-arm-kernel, joel

On Wed, Jan 25, 2023 at 3:32 PM Hawkins, Nick <nick.hawkins@hpe.com> wrote:
>
> > > + hpe,sysreg:
> > > + $ref: /schemas/types.yaml#/definitions/phandle
> > > + description:
> > > + Phandle to the global status and enable interrupt registers shared
> > > + between each I2C engine controller instance. It enables the I2C
> > > + engine controller to act as both a master or slave by being able to
> > > + arm and respond to interrupts from its engine. Each bit in the
> > > + registers represent the respective bit position.
>
>
> > Each bit represents the bit position?
>
> Yes what I mean here is that bit 0 represents engine 0, bit 1 represents
> engine 1 and so on. I will reword this how you have below.
>
> > AIUI, each I2C instance has a bit in it needs to control. How does the
> > driver know what instance (and therefore the correct bit)? Typically you
> > would have a 2nd cell here with that information.
>
> We are currently using the memory area designated reg to determine
> which engine we are on.
>
> Here is a snippet from patch 1 of this patchset that introduces the driver:
> /* Use physical memory address to determine which I2C engine this is. */
> +       drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8;
>
> This works because each engine is 0x100 apart.

Ah, that works fine then.

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support
  2023-01-25 18:44 ` [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support nick.hawkins
@ 2023-01-28 18:38   ` Wolfram Sang
  2023-02-07 21:53     ` Hawkins, Nick
  0 siblings, 1 reply; 15+ messages in thread
From: Wolfram Sang @ 2023-01-28 18:38 UTC (permalink / raw)
  To: nick.hawkins
  Cc: verdun, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c,
	devicetree, linux-kernel, linux-arm-kernel, joel

[-- Attachment #1: Type: text/plain, Size: 448 bytes --]

On Wed, Jan 25, 2023 at 12:44:38PM -0600, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add the I2C controller source and bindings.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

I assume this better goes via the arm-soc-tree as well to reduce merge
conflicts, so:

Acked-by: Wolfram Sang <wsa@kernel.org>

Let me know if I should pick this instead. Will review the driver in the
next days.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support
  2023-01-28 18:38   ` Wolfram Sang
@ 2023-02-07 21:53     ` Hawkins, Nick
  2023-02-15 19:26       ` Hawkins, Nick
  0 siblings, 1 reply; 15+ messages in thread
From: Hawkins, Nick @ 2023-02-07 21:53 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Verdun, Jean-Marie, robh+dt, krzysztof.kozlowski+dt, linux,
	linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel

> Let me know if I should pick this instead. Will review the driver in the
> next days.

Please include it. There may be warnings generated if MAINTAINERS
does not list the files at the time of your commit.

Thank you,

-Nick Hawkins




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

* RE: [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support
  2023-02-07 21:53     ` Hawkins, Nick
@ 2023-02-15 19:26       ` Hawkins, Nick
  0 siblings, 0 replies; 15+ messages in thread
From: Hawkins, Nick @ 2023-02-15 19:26 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Verdun, Jean-Marie, robh+dt, krzysztof.kozlowski+dt, linux,
	linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel


> > Let me know if I should pick this instead. Will review the driver in the
> > next days.

> Please include it. There may be warnings generated if MAINTAINERS
does not list the files at the time of your commit.

> Thank you,

> -Nick Hawkins

Greetings Wolfram,

Just a gentle reminder on this review.

Thank you for your time,

-Nick Hawkins

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

* Re: [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller
  2023-01-25 18:44 ` [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller nick.hawkins
@ 2023-02-15 20:29   ` Wolfram Sang
  2023-02-16 20:02     ` Hawkins, Nick
  0 siblings, 1 reply; 15+ messages in thread
From: Wolfram Sang @ 2023-02-15 20:29 UTC (permalink / raw)
  To: nick.hawkins
  Cc: verdun, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c,
	devicetree, linux-kernel, linux-arm-kernel, joel

[-- Attachment #1: Type: text/plain, Size: 7270 bytes --]

Hi Nick,

> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index e50f9603d189..8b3951e0ca5c 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -1457,4 +1457,11 @@ config I2C_VIRTIO
>            This driver can also be built as a module. If so, the module
>            will be called i2c-virtio.
>  
> +config I2C_GXP
> +	tristate "GXP I2C Interface"
> +	depends on ARCH_HPE_GXP || COMPILE_TEST
> +	help
> +	  This enables support for GXP I2C interface. The I2C engines can be
> +	  either I2C master or I2C slaves.
> +

Shouldn't this be in the "I2C system bus drivers (mostly embedded /
system-on-chip)" section? (alphabetically sorted)

> +static bool i2c_global_init_done;

Can't we avoid this static by checking if i2cg_map is NULL/non-NULL?

> +
> +enum {
> +	GXP_I2C_IDLE = 0,
> +	GXP_I2C_ADDR_PHASE,
> +	GXP_I2C_RDATA_PHASE,
> +	GXP_I2C_WDATA_PHASE,
> +	GXP_I2C_ADDR_NACK,
> +	GXP_I2C_DATA_NACK,
> +	GXP_I2C_ERROR,
> +	GXP_I2C_COMP
> +};
> +
> +struct gxp_i2c_drvdata {
> +	struct device *dev;
> +	void __iomem *base;
> +	struct i2c_timings t;
> +	u32 engine;
> +	int irq;
> +	struct completion completion;
> +	struct i2c_adapter adapter;
> +	struct i2c_msg *curr_msg;
> +	int msgs_remaining;
> +	int msgs_num;
> +	u8 *buf;
> +	size_t buf_remaining;
> +	unsigned char state;
> +	struct i2c_client *slave;
> +	unsigned char stopped;
> +};
> +
> +static struct regmap *i2cg_map;
> +
> +static void gxp_i2c_start(struct gxp_i2c_drvdata *drvdata)
> +{
> +	u16 value;
> +
> +	drvdata->buf = drvdata->curr_msg->buf;
> +	drvdata->buf_remaining = drvdata->curr_msg->len;
> +
> +	/* Note: Address in struct i2c_msg is 7 bits */
> +	value = drvdata->curr_msg->addr << 9;
> +
> +	if (drvdata->curr_msg->flags & I2C_M_RD) {
> +		/* Read */
> +		value |= 0x05;
> +	} else {
> +		/* Write */
> +		value |= 0x01;
> +	}

I'd write it more concise as:

	value |= drvdata->curr_msg->flags & I2C_M_RD ? 0x05 : 0x01;

But this is a matter of taste.

> +
> +	drvdata->state = GXP_I2C_ADDR_PHASE;
> +	writew(value, drvdata->base + GXP_I2CMCMD);
> +}
> +
> +static int gxp_i2c_master_xfer(struct i2c_adapter *adapter,
> +			       struct i2c_msg *msgs, int num)
> +{
> +	int ret;
> +	struct gxp_i2c_drvdata *drvdata = i2c_get_adapdata(adapter);
> +	unsigned long time_left;
> +
> +	drvdata->msgs_remaining = num;
> +	drvdata->curr_msg = msgs;
> +	drvdata->msgs_num = num;
> +	reinit_completion(&drvdata->completion);
> +
> +	gxp_i2c_start(drvdata);
> +
> +	time_left = wait_for_completion_timeout(&drvdata->completion,
> +						adapter->timeout);
> +	ret = num - drvdata->msgs_remaining;
> +	if (time_left == 0) {
> +		switch (drvdata->state) {
> +		case GXP_I2C_WDATA_PHASE:
> +			dev_err(drvdata->dev,
> +				"gxp_i2c_start:write Data phase timeout at msg[%d]\n",
> +			ret);

Please don't write error message to the log on timeouts. They can
happen. Think of an EEPROM which is busy because it needs to erase a
page. Upper layers need to handle this correctly, the user doesn't need
to know about it.

> +			break;
> +		case GXP_I2C_RDATA_PHASE:
> +			dev_err(drvdata->dev,
> +				"gxp_i2c_start:read Data phase timeout at msg[%d]\n",
> +			ret);
> +			break;
> +		case GXP_I2C_ADDR_PHASE:
> +			dev_err(drvdata->dev,
> +				"gxp_i2c_start:Addr phase timeout\n");
> +			break;
> +		default:
> +			dev_err(drvdata->dev,
> +				"gxp_i2c_start:i2c transfer timeout state=%d\n",
> +			drvdata->state);
> +			break;
> +		}
> +		return -ETIMEDOUT;
> +	}
> +
> +	if (drvdata->state == GXP_I2C_ADDR_NACK) {
> +		dev_err(drvdata->dev,
> +			"gxp_i2c_start:No ACK for address phase\n");
> +		return -EIO;
> +	} else if (drvdata->state == GXP_I2C_DATA_NACK) {
> +		dev_err(drvdata->dev, "gxp_i2c_start:No ACK for data phase\n");
> +		return -EIO;

Same here for NACK. Otherwise i2cdetect will flood your logfile.

> +	}
> +
> +	return ret;
> +}
> +
> +static u32 gxp_i2c_func(struct i2c_adapter *adap)
> +{
> +	if (IS_ENABLED(CONFIG_I2C_SLAVE))
> +		return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SLAVE;
> +
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> +}
> +
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +static int gxp_i2c_reg_slave(struct i2c_client *slave)
> +{
> +	struct gxp_i2c_drvdata *drvdata = i2c_get_adapdata(slave->adapter);
> +
> +	if (drvdata->slave)
> +		return -EBUSY;
> +
> +	if (slave->flags & I2C_CLIENT_TEN)
> +		return -EAFNOSUPPORT;
> +
> +	drvdata->slave = slave;
> +
> +	writeb(slave->addr << 1, drvdata->base + GXP_I2COWNADR);
> +	writeb(0x69, drvdata->base + GXP_I2CSCMD);

Does it make sense to have #defines for the magic values for I2CSCMD?
BTW, is the datasheet public?

...

> +static void gxp_i2c_init(struct gxp_i2c_drvdata *drvdata)
> +{
> +	drvdata->state = GXP_I2C_IDLE;
> +	writeb(2000000 / drvdata->t.bus_freq_hz,
> +	       drvdata->base + GXP_I2CFREQDIV);
> +	writeb(0x32, drvdata->base + GXP_I2CFLTFAIR);
> +	writeb(0x0a, drvdata->base + GXP_I2CTMOEDG);
> +	writeb(0x00, drvdata->base + GXP_I2CCYCTIM);
> +	writeb(0x00, drvdata->base + GXP_I2CSNPAA);
> +	writeb(0x00, drvdata->base + GXP_I2CADVFEAT);
> +	writeb(0xF0, drvdata->base + GXP_I2CSCMD);
> +	writeb(0x80, drvdata->base + GXP_I2CMCMD);
> +	writeb(0x00, drvdata->base + GXP_I2CEVTERR);
> +	writeb(0x00, drvdata->base + GXP_I2COWNADR);

Also here, lots of magic values. Can we do something about it?

> +}
> +
> +static int gxp_i2c_probe(struct platform_device *pdev)
> +{
> +	struct gxp_i2c_drvdata *drvdata;
> +	int rc;
> +	struct i2c_adapter *adapter;
> +
> +	if (!i2c_global_init_done) {
> +		i2cg_map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +							   "hpe,sysreg");
> +		if (IS_ERR(i2cg_map)) {
> +			return dev_err_probe(&pdev->dev, IS_ERR(i2cg_map),
> +					     "failed to map i2cg_handle\n");
> +		}
> +
> +		/* Disable interrupt */
> +		regmap_update_bits(i2cg_map, GXP_I2CINTEN, 0x00000FFF, 0);
> +		i2c_global_init_done = true;
> +	}
> +
> +	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata),
> +			       GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, drvdata);
> +	drvdata->dev = &pdev->dev;
> +	init_completion(&drvdata->completion);
> +
> +	drvdata->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(drvdata->base))
> +		return PTR_ERR(drvdata->base);
> +
> +	/* Use physical memory address to determine which I2C engine this is. */
> +	drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8;

This breaks on my 64-bit test-build, so it will also fail with
COMPILE_TEST.

drivers/i2c/busses/i2c-gxp.c: In function ‘gxp_i2c_probe’:
drivers/i2c/busses/i2c-gxp.c:533:28: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
  533 |         drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8;

The rest looks good to me.

Except for removing the hardcoded values, the other fixes should be
fairly simple, I think. So, hardcoded values could be changed
incrementally maybe. If this is possible at all. I still plan to have
this driver in 6.3.

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller
  2023-02-15 20:29   ` Wolfram Sang
@ 2023-02-16 20:02     ` Hawkins, Nick
  2023-02-16 20:47       ` Wolfram Sang
  0 siblings, 1 reply; 15+ messages in thread
From: Hawkins, Nick @ 2023-02-16 20:02 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Verdun, Jean-Marie, robh+dt, krzysztof.kozlowski+dt, linux,
	linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel

> Does it make sense to have #defines for the magic values for I2CSCMD?
> BTW, is the datasheet public?

I will work on the defines and get rid of all the magic values.
Unfortunately, there is no public spec available currently.
Hopefully, we will have one someday though.

> > + /* Use physical memory address to determine which I2C engine this is. */
> > + drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8;


> This breaks on my 64-bit test-build, so it will also fail with
> COMPILE_TEST.


> drivers/i2c/busses/i2c-gxp.c: In function ‘gxp_i2c_probe’:
> drivers/i2c/busses/i2c-gxp.c:533:28: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
> 533 | drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8;

I am currently unable to reproduce this error. I even set W=2.
Would replacing (u32) with (unsigned long) resolve it?

Thanks,

-Nick Hawkins






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

* Re: [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller
  2023-02-16 20:02     ` Hawkins, Nick
@ 2023-02-16 20:47       ` Wolfram Sang
  0 siblings, 0 replies; 15+ messages in thread
From: Wolfram Sang @ 2023-02-16 20:47 UTC (permalink / raw)
  To: Hawkins, Nick
  Cc: Verdun, Jean-Marie, robh+dt, krzysztof.kozlowski+dt, linux,
	linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel

[-- Attachment #1: Type: text/plain, Size: 770 bytes --]

Hi Nick,

> I will work on the defines and get rid of all the magic values.

Cool, thank you!

> Unfortunately, there is no public spec available currently.
> Hopefully, we will have one someday though.

Then, the #defines are even more helpful!

> > drivers/i2c/busses/i2c-gxp.c: In function ‘gxp_i2c_probe’:
> > drivers/i2c/busses/i2c-gxp.c:533:28: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
> > 533 | drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8;
> 
> I am currently unable to reproduce this error. I even set W=2.

Interesting. I have this version:

gcc (Debian 10.2.1-6) 10.2.1 20210110

> Would replacing (u32) with (unsigned long) resolve it?

Or size_t.

Happy hacking,

   Wolfram

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2023-02-16 20:47 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-25 18:44 [PATCH v4 0/5] ARM: Add GXP I2C Support nick.hawkins
2023-01-25 18:44 ` [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller nick.hawkins
2023-02-15 20:29   ` Wolfram Sang
2023-02-16 20:02     ` Hawkins, Nick
2023-02-16 20:47       ` Wolfram Sang
2023-01-25 18:44 ` [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c nick.hawkins
2023-01-25 21:18   ` Rob Herring
2023-01-25 21:31     ` Hawkins, Nick
2023-01-26 13:38       ` Rob Herring
2023-01-25 18:44 ` [PATCH v4 3/5] ARM: dts: hpe: Add I2C Topology nick.hawkins
2023-01-25 18:44 ` [PATCH v4 4/5] ARM: multi_v7_defconfig: add gxp i2c module nick.hawkins
2023-01-25 18:44 ` [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support nick.hawkins
2023-01-28 18:38   ` Wolfram Sang
2023-02-07 21:53     ` Hawkins, Nick
2023-02-15 19:26       ` Hawkins, Nick

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).