[PATCH v2 0/6] Add different features for I2C

[PATCH v2 0/6] Add different features for I2C

[Pierre-Yves MORDRET]

Append new I2C STM32F7 feature set. This includes 10 bit support, slave
support, SMBBus protocols support, DMA Support and eventually an I2C recovery
mechanism.

---
  Version history:
    v1:
        * Initial
    v2:
        * Kbuild test robot issue
        * I2C master recovery mechanism 
---
Pierre-Yves MORDRET (6):
  i2c: i2c-stm32f7: Add 10-bit address support
  i2c: i2c-stm32f7: Add slave support
  i2c: i2c-stm32f7: Add initial SMBus protocols support
  i2c: i2c-stm32: Add generic DMA API
  i2c: i2c-stm32f7: Add DMA support
  i2c: i2c-stm32f7: Implement I2C recovery mechanism

 drivers/i2c/busses/Kconfig       |    1 +
 drivers/i2c/busses/Makefile      |    3 +-
 drivers/i2c/busses/i2c-stm32.c   |  153 ++++++
 drivers/i2c/busses/i2c-stm32.h   |   37 ++
 drivers/i2c/busses/i2c-stm32f7.c | 1040 +++++++++++++++++++++++++++++++++++++-
 5 files changed, 1207 insertions(+), 27 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-stm32.c

-- 
2.7.4

[PATCH v2 1/6] i2c: i2c-stm32f7: Add 10-bit address support

[Pierre-Yves MORDRET]

This patch adds support for 10-bit device address for STM32F7 I2C

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
---
  Version history:
     v1:
        * Initial
     v2:
---
---
 drivers/i2c/busses/i2c-stm32f7.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index f273e28..ae0d15c 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -65,7 +65,12 @@
 #define STM32F7_I2C_CR2_NACK			BIT(15)
 #define STM32F7_I2C_CR2_STOP			BIT(14)
 #define STM32F7_I2C_CR2_START			BIT(13)
+#define STM32F7_I2C_CR2_HEAD10R			BIT(12)
+#define STM32F7_I2C_CR2_ADD10			BIT(11)
 #define STM32F7_I2C_CR2_RD_WRN			BIT(10)
+#define STM32F7_I2C_CR2_SADD10_MASK		GENMASK(9, 0)
+#define STM32F7_I2C_CR2_SADD10(n)		(((n) & \
+						STM32F7_I2C_CR2_SADD10_MASK))
 #define STM32F7_I2C_CR2_SADD7_MASK		GENMASK(7, 1)
 #define STM32F7_I2C_CR2_SADD7(n)		(((n) & 0x7f) << 1)
 
@@ -176,14 +181,14 @@ struct stm32f7_i2c_timings {
 
 /**
  * struct stm32f7_i2c_msg - client specific data
- * @addr: 8-bit slave addr, including r/w bit
+ * @addr: 8-bit or 10-bit slave addr, including r/w bit
  * @count: number of bytes to be transferred
  * @buf: data buffer
  * @result: result of the transfer
  * @stop: last I2C msg to be sent, i.e. STOP to be generated
  */
 struct stm32f7_i2c_msg {
-	u8 addr;
+	u16 addr;
 	u32 count;
 	u8 *buf;
 	int result;
@@ -629,8 +634,15 @@ static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
 		cr2 |= STM32F7_I2C_CR2_RD_WRN;
 
 	/* Set slave address */
-	cr2 &= ~STM32F7_I2C_CR2_SADD7_MASK;
-	cr2 |= STM32F7_I2C_CR2_SADD7(f7_msg->addr);
+	cr2 &= ~(STM32F7_I2C_CR2_HEAD10R | STM32F7_I2C_CR2_ADD10);
+	if (msg->flags & I2C_M_TEN) {
+		cr2 &= ~STM32F7_I2C_CR2_SADD10_MASK;
+		cr2 |= STM32F7_I2C_CR2_SADD10(f7_msg->addr);
+		cr2 |= STM32F7_I2C_CR2_ADD10;
+	} else {
+		cr2 &= ~STM32F7_I2C_CR2_SADD7_MASK;
+		cr2 |= STM32F7_I2C_CR2_SADD7(f7_msg->addr);
+	}
 
 	/* Set nb bytes to transfer and reload if needed */
 	cr2 &= ~(STM32F7_I2C_CR2_NBYTES_MASK | STM32F7_I2C_CR2_RELOAD);
@@ -798,7 +810,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
 
 static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR;
 }
 
 static struct i2c_algorithm stm32f7_i2c_algo = {
-- 
2.7.4

[PATCH v2 2/6] i2c: i2c-stm32f7: Add slave support

[Pierre-Yves MORDRET]

This patch adds slave support for I2C controller embedded in STM32F7 SoC

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
---
  Version history:
     v1:
        * Initial
     v2:
---
---
 drivers/i2c/busses/Kconfig       |   1 +
 drivers/i2c/busses/i2c-stm32f7.c | 434 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 429 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index e2954fb..118b9be 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -964,6 +964,7 @@ config I2C_STM32F4
 config I2C_STM32F7
 	tristate "STMicroelectronics STM32F7 I2C support"
 	depends on ARCH_STM32 || COMPILE_TEST
+	select I2C_SLAVE
 	help
 	  Enable this option to add support for STM32 I2C controller embedded
 	  in STM32F7 SoCs.
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index ae0d15c..9bf676e 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -35,6 +35,8 @@
 /* STM32F7 I2C registers */
 #define STM32F7_I2C_CR1				0x00
 #define STM32F7_I2C_CR2				0x04
+#define STM32F7_I2C_OAR1			0x08
+#define STM32F7_I2C_OAR2			0x0C
 #define STM32F7_I2C_TIMINGR			0x10
 #define STM32F7_I2C_ISR				0x18
 #define STM32F7_I2C_ICR				0x1C
@@ -42,6 +44,7 @@
 #define STM32F7_I2C_TXDR			0x28
 
 /* STM32F7 I2C control 1 */
+#define STM32F7_I2C_CR1_SBC			BIT(16)
 #define STM32F7_I2C_CR1_ANFOFF			BIT(12)
 #define STM32F7_I2C_CR1_ERRIE			BIT(7)
 #define STM32F7_I2C_CR1_TCIE			BIT(6)
@@ -57,6 +60,11 @@
 						| STM32F7_I2C_CR1_NACKIE \
 						| STM32F7_I2C_CR1_RXIE \
 						| STM32F7_I2C_CR1_TXIE)
+#define STM32F7_I2C_XFER_IRQ_MASK		(STM32F7_I2C_CR1_TCIE \
+						| STM32F7_I2C_CR1_STOPIE \
+						| STM32F7_I2C_CR1_NACKIE \
+						| STM32F7_I2C_CR1_RXIE \
+						| STM32F7_I2C_CR1_TXIE)
 
 /* STM32F7 I2C control 2 */
 #define STM32F7_I2C_CR2_RELOAD			BIT(24)
@@ -74,7 +82,34 @@
 #define STM32F7_I2C_CR2_SADD7_MASK		GENMASK(7, 1)
 #define STM32F7_I2C_CR2_SADD7(n)		(((n) & 0x7f) << 1)
 
+/* STM32F7 I2C Own Address 1 */
+#define STM32F7_I2C_OAR1_OA1EN			BIT(15)
+#define STM32F7_I2C_OAR1_OA1MODE		BIT(10)
+#define STM32F7_I2C_OAR1_OA1_10_MASK		GENMASK(9, 0)
+#define STM32F7_I2C_OAR1_OA1_10(n)		(((n) & \
+						STM32F7_I2C_OAR1_OA1_10_MASK))
+#define STM32F7_I2C_OAR1_OA1_7_MASK		GENMASK(7, 1)
+#define STM32F7_I2C_OAR1_OA1_7(n)		(((n) & 0x7f) << 1)
+#define STM32F7_I2C_OAR1_MASK			(STM32F7_I2C_OAR1_OA1_7_MASK \
+						| STM32F7_I2C_OAR1_OA1_10_MASK \
+						| STM32F7_I2C_OAR1_OA1EN \
+						| STM32F7_I2C_OAR1_OA1MODE)
+
+/* STM32F7 I2C Own Address 2 */
+#define STM32F7_I2C_OAR2_OA2EN			BIT(15)
+#define STM32F7_I2C_OAR2_OA2MSK_MASK		GENMASK(10, 8)
+#define STM32F7_I2C_OAR2_OA2MSK(n)		(((n) & 0x7) << 8)
+#define STM32F7_I2C_OAR2_OA2_7_MASK		GENMASK(7, 1)
+#define STM32F7_I2C_OAR2_OA2_7(n)		(((n) & 0x7f) << 1)
+#define STM32F7_I2C_OAR2_MASK			(STM32F7_I2C_OAR2_OA2MSK_MASK \
+						| STM32F7_I2C_OAR2_OA2_7_MASK \
+						| STM32F7_I2C_OAR2_OA2EN)
+
 /* STM32F7 I2C Interrupt Status */
+#define STM32F7_I2C_ISR_ADDCODE_MASK		GENMASK(23, 17)
+#define STM32F7_I2C_ISR_ADDCODE_GET(n) \
+				(((n) & STM32F7_I2C_ISR_ADDCODE_MASK) >> 17)
+#define STM32F7_I2C_ISR_DIR			BIT(16)
 #define STM32F7_I2C_ISR_BUSY			BIT(15)
 #define STM32F7_I2C_ISR_ARLO			BIT(9)
 #define STM32F7_I2C_ISR_BERR			BIT(8)
@@ -82,14 +117,17 @@
 #define STM32F7_I2C_ISR_TC			BIT(6)
 #define STM32F7_I2C_ISR_STOPF			BIT(5)
 #define STM32F7_I2C_ISR_NACKF			BIT(4)
+#define STM32F7_I2C_ISR_ADDR			BIT(3)
 #define STM32F7_I2C_ISR_RXNE			BIT(2)
 #define STM32F7_I2C_ISR_TXIS			BIT(1)
+#define STM32F7_I2C_ISR_TXE			BIT(0)
 
 /* STM32F7 I2C Interrupt Clear */
 #define STM32F7_I2C_ICR_ARLOCF			BIT(9)
 #define STM32F7_I2C_ICR_BERRCF			BIT(8)
 #define STM32F7_I2C_ICR_STOPCF			BIT(5)
 #define STM32F7_I2C_ICR_NACKCF			BIT(4)
+#define STM32F7_I2C_ICR_ADDRCF			BIT(3)
 
 /* STM32F7 I2C Timing */
 #define STM32F7_I2C_TIMINGR_PRESC(n)		(((n) & 0xf) << 28)
@@ -99,6 +137,7 @@
 #define STM32F7_I2C_TIMINGR_SCLL(n)		((n) & 0xff)
 
 #define STM32F7_I2C_MAX_LEN			0xff
+#define STM32F7_I2C_MAX_SLAVE			0x2
 
 #define STM32F7_I2C_DNF_DEFAULT			0
 #define STM32F7_I2C_DNF_MAX			16
@@ -209,6 +248,11 @@ struct stm32f7_i2c_msg {
  * @f7_msg: customized i2c msg for driver usage
  * @setup: I2C timing input setup
  * @timing: I2C computed timings
+ * @slave: list of slave devices registered on the I2C bus
+ * @slave_running: slave device currently used
+ * @slave_dir: transfer direction for the current slave device
+ * @master_mode: boolean to know in which mode the I2C is running (master or
+ * slave)
  */
 struct stm32f7_i2c_dev {
 	struct i2c_adapter adap;
@@ -223,6 +267,10 @@ struct stm32f7_i2c_dev {
 	struct stm32f7_i2c_msg f7_msg;
 	struct stm32f7_i2c_setup setup;
 	struct stm32f7_i2c_timings timing;
+	struct i2c_client *slave[STM32F7_I2C_MAX_SLAVE];
+	struct i2c_client *slave_running;
+	u32 slave_dir;
+	bool master_mode;
 };
 
 /**
@@ -288,6 +336,11 @@ static inline void stm32f7_i2c_clr_bits(void __iomem *reg, u32 mask)
 	writel_relaxed(readl_relaxed(reg) & ~mask, reg);
 }
 
+static void stm32f7_i2c_disable_irq(struct stm32f7_i2c_dev *i2c_dev, u32 mask)
+{
+	stm32f7_i2c_clr_bits(i2c_dev->base + STM32F7_I2C_CR1, mask);
+}
+
 static int stm32f7_i2c_compute_timing(struct stm32f7_i2c_dev *i2c_dev,
 				      struct stm32f7_i2c_setup *setup,
 				      struct stm32f7_i2c_timings *output)
@@ -572,6 +625,9 @@ static void stm32f7_i2c_read_rx_data(struct stm32f7_i2c_dev *i2c_dev)
 	if (f7_msg->count) {
 		*f7_msg->buf++ = readb_relaxed(base + STM32F7_I2C_RXDR);
 		f7_msg->count--;
+	} else {
+		/* Flush RX buffer has no data is expected */
+		readb_relaxed(base + STM32F7_I2C_RXDR);
 	}
 }
 
@@ -669,14 +725,250 @@ static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
 	/* Configure Start/Repeated Start */
 	cr2 |= STM32F7_I2C_CR2_START;
 
+	i2c_dev->master_mode = true;
+
 	/* Write configurations registers */
 	writel_relaxed(cr1, base + STM32F7_I2C_CR1);
 	writel_relaxed(cr2, base + STM32F7_I2C_CR2);
 }
 
-static void stm32f7_i2c_disable_irq(struct stm32f7_i2c_dev *i2c_dev, u32 mask)
+static bool stm32f7_i2c_is_addr_match(struct i2c_client *slave, u32 addcode)
 {
-	stm32f7_i2c_clr_bits(i2c_dev->base + STM32F7_I2C_CR1, mask);
+	u32 addr;
+
+	if (!slave)
+		return false;
+
+	if (slave->flags & I2C_CLIENT_TEN) {
+		/*
+		 * For 10-bit addr, addcode = 11110XY with
+		 * X = Bit 9 of slave address
+		 * Y = Bit 8 of slave address
+		 */
+		addr = slave->addr >> 8;
+		addr |= 0x78;
+		if (addr == addcode)
+			return true;
+	} else {
+		addr = slave->addr & 0x7f;
+		if (addr == addcode)
+			return true;
+	}
+
+	return false;
+}
+
+static void stm32f7_i2c_slave_start(struct stm32f7_i2c_dev *i2c_dev)
+{
+	struct i2c_client *slave = i2c_dev->slave_running;
+	void __iomem *base = i2c_dev->base;
+	u32 mask;
+	u8 value = 0;
+
+	if (i2c_dev->slave_dir) {
+		/* Notify i2c slave that new read transfer is starting */
+		i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
+
+		/*
+		 * Disable slave TX config in case of I2C combined message
+		 * (I2C Write followed by I2C Read)
+		 */
+		mask = STM32F7_I2C_CR2_RELOAD;
+		stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR2, mask);
+		mask = STM32F7_I2C_CR1_SBC | STM32F7_I2C_CR1_RXIE |
+		       STM32F7_I2C_CR1_TCIE;
+		stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR1, mask);
+
+		/* Enable TX empty, STOP, NACK interrupts */
+		mask =  STM32F7_I2C_CR1_STOPIE | STM32F7_I2C_CR1_NACKIE |
+			STM32F7_I2C_CR1_TXIE;
+		stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, mask);
+
+	} else {
+		/* Notify i2c slave that new write transfer is starting */
+		i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
+
+		/* Set reload mode to be able to ACK/NACK each received byte */
+		mask = STM32F7_I2C_CR2_RELOAD;
+		stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
+
+		/*
+		 * Set STOP, NACK, RX empty and transfer complete interrupts.*
+		 * Set Slave Byte Control to be able to ACK/NACK each data
+		 * byte received
+		 */
+		mask =  STM32F7_I2C_CR1_STOPIE | STM32F7_I2C_CR1_NACKIE |
+			STM32F7_I2C_CR1_SBC | STM32F7_I2C_CR1_RXIE |
+			STM32F7_I2C_CR1_TCIE;
+		stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, mask);
+	}
+}
+
+static void stm32f7_i2c_slave_addr(struct stm32f7_i2c_dev *i2c_dev)
+{
+	void __iomem *base = i2c_dev->base;
+	u32 isr, addcode, dir, mask;
+	int i;
+
+	isr = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
+	addcode = STM32F7_I2C_ISR_ADDCODE_GET(isr);
+	dir = isr & STM32F7_I2C_ISR_DIR;
+
+	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
+		if (stm32f7_i2c_is_addr_match(i2c_dev->slave[i], addcode)) {
+			i2c_dev->slave_running = i2c_dev->slave[i];
+			i2c_dev->slave_dir = dir;
+
+			/* Start I2C slave processing */
+			stm32f7_i2c_slave_start(i2c_dev);
+
+			/* Clear ADDR flag */
+			mask = STM32F7_I2C_ICR_ADDRCF;
+			writel_relaxed(mask, base + STM32F7_I2C_ICR);
+			break;
+		}
+	}
+}
+
+static int stm32f7_i2c_get_slave_id(struct stm32f7_i2c_dev *i2c_dev,
+				    struct i2c_client *slave, int *id)
+{
+	int i;
+
+	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
+		if (i2c_dev->slave[i] == slave) {
+			*id = i;
+			return 0;
+		}
+	}
+
+	dev_err(i2c_dev->dev, "Slave 0x%x not registered\n", slave->addr);
+
+	return -ENODEV;
+}
+
+static int stm32f7_i2c_get_free_slave_id(struct stm32f7_i2c_dev *i2c_dev,
+					 struct i2c_client *slave, int *id)
+{
+	struct device *dev = i2c_dev->dev;
+	int i;
+
+	/*
+	 * slave[0] supports 7-bit and 10-bit slave address
+	 * slave[1] supports 7-bit slave address only
+	 */
+	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
+		if (i == 1 && (slave->flags & I2C_CLIENT_PEC))
+			continue;
+		if (!i2c_dev->slave[i]) {
+			*id = i;
+			return 0;
+		}
+	}
+
+	dev_err(dev, "Slave 0x%x could not be registered\n", slave->addr);
+
+	return -EINVAL;
+}
+
+static bool stm32f7_i2c_is_slave_registered(struct stm32f7_i2c_dev *i2c_dev)
+{
+	int i;
+
+	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
+		if (i2c_dev->slave[i])
+			return true;
+	}
+
+	return false;
+}
+
+static bool stm32f7_i2c_is_slave_busy(struct stm32f7_i2c_dev *i2c_dev)
+{
+	int i, busy;
+
+	busy = 0;
+	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
+		if (i2c_dev->slave[i])
+			busy++;
+	}
+
+	return i == busy;
+}
+
+static irqreturn_t stm32f7_i2c_slave_isr_event(struct stm32f7_i2c_dev *i2c_dev)
+{
+	void __iomem *base = i2c_dev->base;
+	u32 cr2, status, mask;
+	u8 val;
+	int ret;
+
+	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
+
+	/* Slave transmitter mode */
+	if (status & STM32F7_I2C_ISR_TXIS) {
+		i2c_slave_event(i2c_dev->slave_running,
+				I2C_SLAVE_READ_PROCESSED,
+				&val);
+
+		/* Write data byte */
+		writel_relaxed(val, base + STM32F7_I2C_TXDR);
+	}
+
+	/* Transfer Complete Reload for Slave receiver mode */
+	if (status & STM32F7_I2C_ISR_TCR || status & STM32F7_I2C_ISR_RXNE) {
+		/*
+		 * Read data byte then set NBYTES to receive next byte or NACK
+		 * the current received byte
+		 */
+		val = readb_relaxed(i2c_dev->base + STM32F7_I2C_RXDR);
+		ret = i2c_slave_event(i2c_dev->slave_running,
+				      I2C_SLAVE_WRITE_RECEIVED,
+				      &val);
+		if (!ret) {
+			cr2 = readl_relaxed(i2c_dev->base + STM32F7_I2C_CR2);
+			cr2 |= STM32F7_I2C_CR2_NBYTES(1);
+			writel_relaxed(cr2, i2c_dev->base + STM32F7_I2C_CR2);
+		} else {
+			mask = STM32F7_I2C_CR2_NACK;
+			stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
+		}
+	}
+
+	/* NACK received */
+	if (status & STM32F7_I2C_ISR_NACKF) {
+		dev_dbg(i2c_dev->dev, "<%s>: Receive NACK\n", __func__);
+		writel_relaxed(STM32F7_I2C_ICR_NACKCF, base + STM32F7_I2C_ICR);
+	}
+
+	/* STOP received */
+	if (status & STM32F7_I2C_ISR_STOPF) {
+		/* Disable interrupts */
+		stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_XFER_IRQ_MASK);
+
+		if (i2c_dev->slave_dir) {
+			/*
+			 * Flush TX buffer in order to not used the byte in
+			 * TXDR for the next transfer
+			 */
+			mask = STM32F7_I2C_ISR_TXE;
+			stm32f7_i2c_set_bits(base + STM32F7_I2C_ISR, mask);
+		}
+
+		/* Clear STOP flag */
+		writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
+
+		/* Notify i2c slave that a STOP flag has been detected */
+		i2c_slave_event(i2c_dev->slave_running, I2C_SLAVE_STOP, &val);
+
+		i2c_dev->slave_running = NULL;
+	}
+
+	/* Address match received */
+	if (status & STM32F7_I2C_ISR_ADDR)
+		stm32f7_i2c_slave_addr(i2c_dev);
+
+	return IRQ_HANDLED;
 }
 
 static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
@@ -685,6 +977,13 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
 	void __iomem *base = i2c_dev->base;
 	u32 status, mask;
+	int ret;
+
+	/* Check if the interrupt if for a slave device */
+	if (!i2c_dev->master_mode) {
+		ret = stm32f7_i2c_slave_isr_event(i2c_dev);
+		return ret;
+	}
 
 	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
 
@@ -706,11 +1005,16 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 	/* STOP detection flag */
 	if (status & STM32F7_I2C_ISR_STOPF) {
 		/* Disable interrupts */
-		stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
+		if (stm32f7_i2c_is_slave_registered(i2c_dev))
+			mask = STM32F7_I2C_XFER_IRQ_MASK;
+		else
+			mask = STM32F7_I2C_ALL_IRQ_MASK;
+		stm32f7_i2c_disable_irq(i2c_dev, mask);
 
 		/* Clear STOP flag */
 		writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
 
+		i2c_dev->master_mode = false;
 		complete(&i2c_dev->complete);
 	}
 
@@ -743,7 +1047,7 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
 	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
 	void __iomem *base = i2c_dev->base;
 	struct device *dev = i2c_dev->dev;
-	u32 status;
+	u32 mask, status;
 
 	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
 
@@ -761,8 +1065,14 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
 		f7_msg->result = -EAGAIN;
 	}
 
-	stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
+	/* Disable interrupts */
+	if (stm32f7_i2c_is_slave_registered(i2c_dev))
+		mask = STM32F7_I2C_XFER_IRQ_MASK;
+	else
+		mask = STM32F7_I2C_ALL_IRQ_MASK;
+	stm32f7_i2c_disable_irq(i2c_dev, mask);
 
+	i2c_dev->master_mode = false;
 	complete(&i2c_dev->complete);
 
 	return IRQ_HANDLED;
@@ -808,14 +1118,126 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
 	return (ret < 0) ? ret : num;
 }
 
+static int stm32f7_i2c_reg_slave(struct i2c_client *slave)
+{
+	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(slave->adapter);
+	void __iomem *base = i2c_dev->base;
+	struct device *dev = i2c_dev->dev;
+	u32 oar1, oar2, mask;
+	int id, ret;
+
+	if (slave->flags & I2C_CLIENT_PEC) {
+		dev_err(dev, "SMBus PEC not supported in slave mode\n");
+		return -EINVAL;
+	}
+
+	if (stm32f7_i2c_is_slave_busy(i2c_dev)) {
+		dev_err(dev, "Too much slave registered\n");
+		return -EBUSY;
+	}
+
+	ret = stm32f7_i2c_get_free_slave_id(i2c_dev, slave, &id);
+	if (ret)
+		return ret;
+
+	if (!(stm32f7_i2c_is_slave_registered(i2c_dev))) {
+		ret = clk_enable(i2c_dev->clk);
+		if (ret) {
+			dev_err(dev, "Failed to enable clock\n");
+			return ret;
+		}
+	}
+
+	if (id == 0) {
+		/* Configure Own Address 1 */
+		oar1 = readl_relaxed(i2c_dev->base + STM32F7_I2C_OAR1);
+		oar1 &= ~STM32F7_I2C_OAR1_MASK;
+		if (slave->flags & I2C_CLIENT_TEN) {
+			oar1 |= STM32F7_I2C_OAR1_OA1_10(slave->addr);
+			oar1 |= STM32F7_I2C_OAR1_OA1MODE;
+		} else {
+			oar1 |= STM32F7_I2C_OAR1_OA1_7(slave->addr);
+		}
+		oar1 |= STM32F7_I2C_OAR1_OA1EN;
+		i2c_dev->slave[id] = slave;
+		writel_relaxed(oar1, i2c_dev->base + STM32F7_I2C_OAR1);
+	} else if (id == 1) {
+		/* Configure Own Address 2 */
+		oar2 = readl_relaxed(i2c_dev->base + STM32F7_I2C_OAR2);
+		oar2 &= ~STM32F7_I2C_OAR2_MASK;
+		if (slave->flags & I2C_CLIENT_TEN) {
+			ret = -EOPNOTSUPP;
+			goto exit;
+		}
+
+		oar2 |= STM32F7_I2C_OAR2_OA2_7(slave->addr);
+		oar2 |= STM32F7_I2C_OAR2_OA2EN;
+		i2c_dev->slave[id] = slave;
+		writel_relaxed(oar2, i2c_dev->base + STM32F7_I2C_OAR2);
+	} else {
+		ret = -ENODEV;
+		goto exit;
+	}
+
+	/* Enable ACK */
+	stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR2, STM32F7_I2C_CR2_NACK);
+
+	/* Enable Address match interrupt, error interrupt and enable I2C  */
+	mask = STM32F7_I2C_CR1_ADDRIE | STM32F7_I2C_CR1_ERRIE |
+		STM32F7_I2C_CR1_PE;
+	stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, mask);
+
+	return 0;
+
+exit:
+	if (!(stm32f7_i2c_is_slave_registered(i2c_dev)))
+		clk_disable(i2c_dev->clk);
+
+	return ret;
+}
+
+static int stm32f7_i2c_unreg_slave(struct i2c_client *slave)
+{
+	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(slave->adapter);
+	void __iomem *base = i2c_dev->base;
+	u32 mask;
+	int id, ret;
+
+	ret = stm32f7_i2c_get_slave_id(i2c_dev, slave, &id);
+	if (ret)
+		return ret;
+
+	WARN_ON(!i2c_dev->slave[id]);
+
+	if (id == 0) {
+		mask = STM32F7_I2C_OAR1_OA1EN;
+		stm32f7_i2c_clr_bits(base + STM32F7_I2C_OAR1, mask);
+	} else {
+		mask = STM32F7_I2C_OAR2_OA2EN;
+		stm32f7_i2c_clr_bits(base + STM32F7_I2C_OAR2, mask);
+	}
+
+	i2c_dev->slave[id] = NULL;
+
+	if (!(stm32f7_i2c_is_slave_registered(i2c_dev))) {
+		stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
+		clk_disable(i2c_dev->clk);
+	}
+
+	return 0;
+}
+
 static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR |
+		I2C_FUNC_SLAVE;
 }
 
 static struct i2c_algorithm stm32f7_i2c_algo = {
 	.master_xfer = stm32f7_i2c_xfer,
 	.functionality = stm32f7_i2c_func,
+	.reg_slave = stm32f7_i2c_reg_slave,
+	.unreg_slave = stm32f7_i2c_unreg_slave,
 };
 
 static int stm32f7_i2c_probe(struct platform_device *pdev)
-- 
2.7.4

[PATCH v2 3/6] i2c: i2c-stm32f7: Add initial SMBus protocols support

[Pierre-Yves MORDRET]

This patch adds SMBus support for I2C controller embedded in STM32F7 Soc.
All SMBus protocols are implemented except SMBus-specific protocols.

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
---
  Version history:
    v1:
       * Initial
    v2:
       * fix Kbuild test robot issue (Unneeded semicolon)
---

fixup! i2c: i2c-stm32f7: Add initial SMBus protocols support
---
 drivers/i2c/busses/i2c-stm32f7.c | 377 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 368 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 9bf676e..0b81b5d 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -37,6 +37,7 @@
 #define STM32F7_I2C_CR2				0x04
 #define STM32F7_I2C_OAR1			0x08
 #define STM32F7_I2C_OAR2			0x0C
+#define STM32F7_I2C_PECR			0x20
 #define STM32F7_I2C_TIMINGR			0x10
 #define STM32F7_I2C_ISR				0x18
 #define STM32F7_I2C_ICR				0x1C
@@ -44,6 +45,7 @@
 #define STM32F7_I2C_TXDR			0x28
 
 /* STM32F7 I2C control 1 */
+#define STM32F7_I2C_CR1_PECEN			BIT(23)
 #define STM32F7_I2C_CR1_SBC			BIT(16)
 #define STM32F7_I2C_CR1_ANFOFF			BIT(12)
 #define STM32F7_I2C_CR1_ERRIE			BIT(7)
@@ -67,6 +69,7 @@
 						| STM32F7_I2C_CR1_TXIE)
 
 /* STM32F7 I2C control 2 */
+#define STM32F7_I2C_CR2_PECBYTE			BIT(26)
 #define STM32F7_I2C_CR2_RELOAD			BIT(24)
 #define STM32F7_I2C_CR2_NBYTES_MASK		GENMASK(23, 16)
 #define STM32F7_I2C_CR2_NBYTES(n)		(((n) & 0xff) << 16)
@@ -111,6 +114,7 @@
 				(((n) & STM32F7_I2C_ISR_ADDCODE_MASK) >> 17)
 #define STM32F7_I2C_ISR_DIR			BIT(16)
 #define STM32F7_I2C_ISR_BUSY			BIT(15)
+#define STM32F7_I2C_ISR_PECERR			BIT(11)
 #define STM32F7_I2C_ISR_ARLO			BIT(9)
 #define STM32F7_I2C_ISR_BERR			BIT(8)
 #define STM32F7_I2C_ISR_TCR			BIT(7)
@@ -123,6 +127,7 @@
 #define STM32F7_I2C_ISR_TXE			BIT(0)
 
 /* STM32F7 I2C Interrupt Clear */
+#define STM32F7_I2C_ICR_PECCF			BIT(11)
 #define STM32F7_I2C_ICR_ARLOCF			BIT(9)
 #define STM32F7_I2C_ICR_BERRCF			BIT(8)
 #define STM32F7_I2C_ICR_STOPCF			BIT(5)
@@ -225,6 +230,14 @@ struct stm32f7_i2c_timings {
  * @buf: data buffer
  * @result: result of the transfer
  * @stop: last I2C msg to be sent, i.e. STOP to be generated
+ * @smbus: boolean to know if the I2C IP is used in SMBus mode
+ * @size: type of SMBus protocol
+ * @read_write: direction of SMBus protocol
+ * SMBus block read and SMBus block write - block read process call protocols
+ * @smbus_buff: buffer to be used for SMBus protocol transfer. It will
+ * contain a maximum of 32 bytes of data + byte command + byte count + PEC
+ * This buffer has to be 32-bit aligned to be compliant with memory address
+ * register in DMA mode.
  */
 struct stm32f7_i2c_msg {
 	u16 addr;
@@ -232,6 +245,10 @@ struct stm32f7_i2c_msg {
 	u8 *buf;
 	int result;
 	bool stop;
+	bool smbus;
+	int size;
+	char read_write;
+	u8 smbus_buf[I2C_SMBUS_BLOCK_MAX + 3] __aligned(4);
 };
 
 /**
@@ -649,6 +666,29 @@ static void stm32f7_i2c_reload(struct stm32f7_i2c_dev *i2c_dev)
 	writel_relaxed(cr2, i2c_dev->base + STM32F7_I2C_CR2);
 }
 
+static void stm32f7_i2c_smbus_reload(struct stm32f7_i2c_dev *i2c_dev)
+{
+	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	u32 cr2;
+	u8 *val;
+
+	/*
+	 * For I2C_SMBUS_BLOCK_DATA && I2C_SMBUS_BLOCK_PROC_CALL, the first
+	 * data received inform us how many data will follow.
+	 */
+	stm32f7_i2c_read_rx_data(i2c_dev);
+
+	/*
+	 * Update NBYTES with the value read to continue the transfer
+	 */
+	val = f7_msg->buf - sizeof(u8);
+	f7_msg->count = *val;
+	cr2 = readl_relaxed(i2c_dev->base + STM32F7_I2C_CR2);
+	cr2 &= ~(STM32F7_I2C_CR2_NBYTES_MASK | STM32F7_I2C_CR2_RELOAD);
+	cr2 |= STM32F7_I2C_CR2_NBYTES(f7_msg->count);
+	writel_relaxed(cr2, i2c_dev->base + STM32F7_I2C_CR2);
+}
+
 static int stm32f7_i2c_wait_free_bus(struct stm32f7_i2c_dev *i2c_dev)
 {
 	u32 status;
@@ -732,6 +772,237 @@ static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
 	writel_relaxed(cr2, base + STM32F7_I2C_CR2);
 }
 
+static int stm32f7_i2c_smbus_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
+				      unsigned short flags, u8 command,
+				      union i2c_smbus_data *data)
+{
+	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	struct device *dev = i2c_dev->dev;
+	void __iomem *base = i2c_dev->base;
+	u32 cr1, cr2;
+	int i;
+
+	f7_msg->result = 0;
+	reinit_completion(&i2c_dev->complete);
+
+	cr2 = readl_relaxed(base + STM32F7_I2C_CR2);
+	cr1 = readl_relaxed(base + STM32F7_I2C_CR1);
+
+	/* Set transfer direction */
+	cr2 &= ~STM32F7_I2C_CR2_RD_WRN;
+	if (f7_msg->read_write)
+		cr2 |= STM32F7_I2C_CR2_RD_WRN;
+
+	/* Set slave address */
+	cr2 &= ~(STM32F7_I2C_CR2_ADD10 | STM32F7_I2C_CR2_SADD7_MASK);
+	cr2 |= STM32F7_I2C_CR2_SADD7(f7_msg->addr);
+
+	f7_msg->smbus_buf[0] = command;
+	switch (f7_msg->size) {
+	case I2C_SMBUS_QUICK:
+		f7_msg->stop = true;
+		f7_msg->count = 0;
+		break;
+	case I2C_SMBUS_BYTE:
+		f7_msg->stop = true;
+		f7_msg->count = 1;
+		break;
+	case I2C_SMBUS_BYTE_DATA:
+		if (f7_msg->read_write) {
+			f7_msg->stop = false;
+			f7_msg->count = 1;
+			cr2 &= ~STM32F7_I2C_CR2_RD_WRN;
+		} else {
+			f7_msg->stop = true;
+			f7_msg->count = 2;
+			f7_msg->smbus_buf[1] = data->byte;
+		}
+		break;
+	case I2C_SMBUS_WORD_DATA:
+		if (f7_msg->read_write) {
+			f7_msg->stop = false;
+			f7_msg->count = 1;
+			cr2 &= ~STM32F7_I2C_CR2_RD_WRN;
+		} else {
+			f7_msg->stop = true;
+			f7_msg->count = 3;
+			f7_msg->smbus_buf[1] = data->word & 0xff;
+			f7_msg->smbus_buf[2] = data->word >> 8;
+		}
+		break;
+	case I2C_SMBUS_BLOCK_DATA:
+		if (f7_msg->read_write) {
+			f7_msg->stop = false;
+			f7_msg->count = 1;
+			cr2 &= ~STM32F7_I2C_CR2_RD_WRN;
+		} else {
+			f7_msg->stop = true;
+			if (data->block[0] > I2C_SMBUS_BLOCK_MAX ||
+			    !data->block[0]) {
+				dev_err(dev, "Invalid block write size %d\n",
+					data->block[0]);
+				return -EINVAL;
+			}
+			f7_msg->count = data->block[0] + 2;
+			for (i = 1; i < f7_msg->count; i++)
+				f7_msg->smbus_buf[i] = data->block[i - 1];
+		}
+		break;
+	case I2C_SMBUS_PROC_CALL:
+		f7_msg->stop = false;
+		f7_msg->count = 3;
+		f7_msg->smbus_buf[1] = data->word & 0xff;
+		f7_msg->smbus_buf[2] = data->word >> 8;
+		cr2 &= ~STM32F7_I2C_CR2_RD_WRN;
+		f7_msg->read_write = I2C_SMBUS_READ;
+		break;
+	case I2C_SMBUS_BLOCK_PROC_CALL:
+		f7_msg->stop = false;
+		if (data->block[0] > I2C_SMBUS_BLOCK_MAX - 1) {
+			dev_err(dev, "Invalid block write size %d\n",
+				data->block[0]);
+			return -EINVAL;
+		}
+		f7_msg->count = data->block[0] + 2;
+		for (i = 1; i < f7_msg->count; i++)
+			f7_msg->smbus_buf[i] = data->block[i - 1];
+		cr2 &= ~STM32F7_I2C_CR2_RD_WRN;
+		f7_msg->read_write = I2C_SMBUS_READ;
+		break;
+	default:
+		dev_err(dev, "Unsupported smbus protocol %d\n", f7_msg->size);
+		return -EOPNOTSUPP;
+	}
+
+	f7_msg->buf = f7_msg->smbus_buf;
+
+	/* Configure PEC */
+	if ((flags & I2C_CLIENT_PEC) && f7_msg->size != I2C_SMBUS_QUICK) {
+		cr1 |= STM32F7_I2C_CR1_PECEN;
+		cr2 |= STM32F7_I2C_CR2_PECBYTE;
+		if (!f7_msg->read_write)
+			f7_msg->count++;
+	} else {
+		cr1 &= ~STM32F7_I2C_CR1_PECEN;
+		cr2 &= ~STM32F7_I2C_CR2_PECBYTE;
+	}
+
+	/* Set number of bytes to be transferred */
+	cr2 &= ~(STM32F7_I2C_CR2_NBYTES_MASK | STM32F7_I2C_CR2_RELOAD);
+	cr2 |= STM32F7_I2C_CR2_NBYTES(f7_msg->count);
+
+	/* Enable NACK, STOP, error and transfer complete interrupts */
+	cr1 |= STM32F7_I2C_CR1_ERRIE | STM32F7_I2C_CR1_TCIE |
+		STM32F7_I2C_CR1_STOPIE | STM32F7_I2C_CR1_NACKIE;
+
+	/* Clear TX/RX interrupt */
+	cr1 &= ~(STM32F7_I2C_CR1_RXIE | STM32F7_I2C_CR1_TXIE);
+
+	/* Enable RX/TX interrupt according to msg direction */
+	if (cr2 & STM32F7_I2C_CR2_RD_WRN)
+		cr1 |= STM32F7_I2C_CR1_RXIE;
+	else
+		cr1 |= STM32F7_I2C_CR1_TXIE;
+
+	/* Set Start bit */
+	cr2 |= STM32F7_I2C_CR2_START;
+
+	i2c_dev->master_mode = true;
+
+	/* Write configurations registers */
+	writel_relaxed(cr1, base + STM32F7_I2C_CR1);
+	writel_relaxed(cr2, base + STM32F7_I2C_CR2);
+
+	return 0;
+}
+
+static void stm32f7_i2c_smbus_rep_start(struct stm32f7_i2c_dev *i2c_dev)
+{
+	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	void __iomem *base = i2c_dev->base;
+	u32 cr1, cr2;
+
+	cr2 = readl_relaxed(base + STM32F7_I2C_CR2);
+	cr1 = readl_relaxed(base + STM32F7_I2C_CR1);
+
+	/* Set transfer direction */
+	cr2 |= STM32F7_I2C_CR2_RD_WRN;
+
+	switch (f7_msg->size) {
+	case I2C_SMBUS_BYTE_DATA:
+		f7_msg->count = 1;
+		break;
+	case I2C_SMBUS_WORD_DATA:
+	case I2C_SMBUS_PROC_CALL:
+		f7_msg->count = 2;
+		break;
+	case I2C_SMBUS_BLOCK_DATA:
+	case I2C_SMBUS_BLOCK_PROC_CALL:
+		f7_msg->count = 1;
+		cr2 |= STM32F7_I2C_CR2_RELOAD;
+		break;
+	}
+
+	f7_msg->buf = f7_msg->smbus_buf;
+	f7_msg->stop = true;
+
+	/* Add one byte for PEC if needed */
+	if (cr1 & STM32F7_I2C_CR1_PECEN)
+		f7_msg->count++;
+
+	/* Set number of bytes to be transferred */
+	cr2 &= ~(STM32F7_I2C_CR2_NBYTES_MASK);
+	cr2 |= STM32F7_I2C_CR2_NBYTES(f7_msg->count);
+
+	/*
+	 * Configure RX/TX interrupt:
+	 */
+	cr1 &= ~(STM32F7_I2C_CR1_RXIE | STM32F7_I2C_CR1_TXIE);
+	cr1 |= STM32F7_I2C_CR1_RXIE;
+
+	/* Configure Repeated Start */
+	cr2 |= STM32F7_I2C_CR2_START;
+
+	/* Write configurations registers */
+	writel_relaxed(cr1, base + STM32F7_I2C_CR1);
+	writel_relaxed(cr2, base + STM32F7_I2C_CR2);
+}
+
+static int stm32f7_i2c_smbus_check_pec(struct stm32f7_i2c_dev *i2c_dev)
+{
+	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	u8 count, internal_pec, received_pec;
+
+	internal_pec = readl_relaxed(i2c_dev->base + STM32F7_I2C_PECR);
+
+	switch (f7_msg->size) {
+	case I2C_SMBUS_BYTE:
+	case I2C_SMBUS_BYTE_DATA:
+		received_pec = f7_msg->smbus_buf[1];
+		break;
+	case I2C_SMBUS_WORD_DATA:
+	case I2C_SMBUS_PROC_CALL:
+		received_pec = f7_msg->smbus_buf[2];
+		break;
+	case I2C_SMBUS_BLOCK_DATA:
+	case I2C_SMBUS_BLOCK_PROC_CALL:
+		count = f7_msg->smbus_buf[0];
+		received_pec = f7_msg->smbus_buf[count];
+		break;
+	default:
+		dev_err(i2c_dev->dev, "Unsupported smbus protocol for PEC\n");
+		return -EINVAL;
+	}
+
+	if (internal_pec != received_pec) {
+		dev_err(i2c_dev->dev, "Bad PEC 0x%02x vs. 0x%02x\n",
+			internal_pec, received_pec);
+		return -EBADMSG;
+	}
+
+	return 0;
+}
+
 static bool stm32f7_i2c_is_addr_match(struct i2c_client *slave, u32 addcode)
 {
 	u32 addr;
@@ -1023,6 +1294,8 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 		if (f7_msg->stop) {
 			mask = STM32F7_I2C_CR2_STOP;
 			stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
+		} else if (f7_msg->smbus) {
+			stm32f7_i2c_smbus_rep_start(i2c_dev);
 		} else {
 			i2c_dev->msg_id++;
 			i2c_dev->msg++;
@@ -1030,13 +1303,12 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 		}
 	}
 
-	/*
-	 * Transfer Complete Reload: 255 data bytes have been transferred
-	 * We have to prepare the I2C controller to transfer the remaining
-	 * data.
-	 */
-	if (status & STM32F7_I2C_ISR_TCR)
-		stm32f7_i2c_reload(i2c_dev);
+	if (status & STM32F7_I2C_ISR_TCR) {
+		if (f7_msg->smbus)
+			stm32f7_i2c_smbus_reload(i2c_dev);
+		else
+			stm32f7_i2c_reload(i2c_dev);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -1065,6 +1337,12 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
 		f7_msg->result = -EAGAIN;
 	}
 
+	if (status & STM32F7_I2C_ISR_PECERR) {
+		dev_err(dev, "<%s>: PEC error in reception\n", __func__);
+		writel_relaxed(STM32F7_I2C_ICR_PECCF, base + STM32F7_I2C_ICR);
+		f7_msg->result = -EINVAL;
+	}
+
 	/* Disable interrupts */
 	if (stm32f7_i2c_is_slave_registered(i2c_dev))
 		mask = STM32F7_I2C_XFER_IRQ_MASK;
@@ -1089,6 +1367,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
 	i2c_dev->msg = msgs;
 	i2c_dev->msg_num = num;
 	i2c_dev->msg_id = 0;
+	f7_msg->smbus = false;
 
 	ret = clk_enable(i2c_dev->clk);
 	if (ret) {
@@ -1118,6 +1397,82 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
 	return (ret < 0) ? ret : num;
 }
 
+static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
+				  unsigned short flags, char read_write,
+				  u8 command, int size,
+				  union i2c_smbus_data *data)
+{
+	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(adapter);
+	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	struct device *dev = i2c_dev->dev;
+	unsigned long timeout;
+	int i, ret;
+
+	f7_msg->addr = addr;
+	f7_msg->size = size;
+	f7_msg->read_write = read_write;
+	f7_msg->smbus = true;
+
+	ret = clk_enable(i2c_dev->clk);
+	if (ret) {
+		dev_err(i2c_dev->dev, "Failed to enable clock\n");
+		return ret;
+	}
+
+	ret = stm32f7_i2c_wait_free_bus(i2c_dev);
+	if (ret)
+		goto clk_free;
+
+	ret = stm32f7_i2c_smbus_xfer_msg(i2c_dev, flags, command, data);
+	if (ret)
+		goto clk_free;
+
+	timeout = wait_for_completion_timeout(&i2c_dev->complete,
+					      i2c_dev->adap.timeout);
+	ret = f7_msg->result;
+	if (ret)
+		goto clk_free;
+
+	if (!timeout) {
+		dev_dbg(dev, "Access to slave 0x%x timed out\n", f7_msg->addr);
+		ret = -ETIMEDOUT;
+		goto clk_free;
+	}
+
+	/* Check PEC */
+	if ((flags & I2C_CLIENT_PEC) && size != I2C_SMBUS_QUICK && read_write) {
+		ret = stm32f7_i2c_smbus_check_pec(i2c_dev);
+		if (ret)
+			goto clk_free;
+	}
+
+	if (read_write && size != I2C_SMBUS_QUICK) {
+		switch (size) {
+		case I2C_SMBUS_BYTE:
+		case I2C_SMBUS_BYTE_DATA:
+			data->byte = f7_msg->smbus_buf[0];
+		break;
+		case I2C_SMBUS_WORD_DATA:
+		case I2C_SMBUS_PROC_CALL:
+			data->word = f7_msg->smbus_buf[0] |
+				(f7_msg->smbus_buf[1] << 8);
+		break;
+		case I2C_SMBUS_BLOCK_DATA:
+		case I2C_SMBUS_BLOCK_PROC_CALL:
+		for (i = 0; i <= f7_msg->smbus_buf[0]; i++)
+			data->block[i] = f7_msg->smbus_buf[i];
+		break;
+		default:
+			dev_err(dev, "Unsupported smbus transaction\n");
+			ret = -EINVAL;
+		}
+	}
+
+clk_free:
+	clk_disable(i2c_dev->clk);
+	return ret;
+}
+
 static int stm32f7_i2c_reg_slave(struct i2c_client *slave)
 {
 	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(slave->adapter);
@@ -1229,12 +1584,16 @@ static int stm32f7_i2c_unreg_slave(struct i2c_client *slave)
 
 static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR |
-		I2C_FUNC_SLAVE;
+	return I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR | I2C_FUNC_SLAVE |
+		I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
+		I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
+		I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
+		I2C_FUNC_SMBUS_PROC_CALL | I2C_FUNC_SMBUS_PEC;
 }
 
 static struct i2c_algorithm stm32f7_i2c_algo = {
 	.master_xfer = stm32f7_i2c_xfer,
+	.smbus_xfer = stm32f7_i2c_smbus_xfer,
 	.functionality = stm32f7_i2c_func,
 	.reg_slave = stm32f7_i2c_reg_slave,
 	.unreg_slave = stm32f7_i2c_unreg_slave,
-- 
2.7.4

[PATCH v2 4/6] i2c: i2c-stm32: Add generic DMA API

[Pierre-Yves MORDRET]

This patch adds a generic DMA API to implement DMA support for i2c-stm32fx
drivers

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
---
  Version history:
    v1:
       * Initial
    v2:
---
---
 drivers/i2c/busses/i2c-stm32.c | 153 +++++++++++++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-stm32.h |  37 ++++++++++
 2 files changed, 190 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-stm32.c

diff --git a/drivers/i2c/busses/i2c-stm32.c b/drivers/i2c/busses/i2c-stm32.c
new file mode 100644
index 0000000..d75fbcb
--- /dev/null
+++ b/drivers/i2c/busses/i2c-stm32.c
@@ -0,0 +1,153 @@
+/*
+ * i2c-stm32.c
+ *
+ * Copyright (C) M'boumba Cedric Madianga 2017
+ * Author: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
+ *
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include "i2c-stm32.h"
+
+/* Functions for DMA support */
+struct stm32_i2c_dma *stm32_i2c_dma_request(struct device *dev,
+					    dma_addr_t phy_addr,
+					    u32 txdr_offset,
+					    u32 rxdr_offset)
+{
+	struct stm32_i2c_dma *dma;
+	struct dma_slave_config dma_sconfig;
+	int ret;
+
+	dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
+	if (!dma)
+		return NULL;
+
+	/* Request and configure I2C TX dma channel */
+	dma->chan_tx = dma_request_slave_channel(dev, "tx");
+	if (!dma->chan_tx) {
+		dev_dbg(dev, "can't request DMA tx channel\n");
+		ret = -EINVAL;
+		goto fail_al;
+	}
+
+	memset(&dma_sconfig, 0, sizeof(dma_sconfig));
+	dma_sconfig.dst_addr = phy_addr + txdr_offset;
+	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_maxburst = 1;
+	dma_sconfig.direction = DMA_MEM_TO_DEV;
+	ret = dmaengine_slave_config(dma->chan_tx, &dma_sconfig);
+	if (ret < 0) {
+		dev_err(dev, "can't configure tx channel\n");
+		goto fail_tx;
+	}
+
+	/* Request and configure I2C RX dma channel */
+	dma->chan_rx = dma_request_slave_channel(dev, "rx");
+	if (!dma->chan_rx) {
+		dev_err(dev, "can't request DMA rx channel\n");
+		ret = -EINVAL;
+		goto fail_tx;
+	}
+
+	memset(&dma_sconfig, 0, sizeof(dma_sconfig));
+	dma_sconfig.src_addr = phy_addr + rxdr_offset;
+	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.src_maxburst = 1;
+	dma_sconfig.direction = DMA_DEV_TO_MEM;
+	ret = dmaengine_slave_config(dma->chan_rx, &dma_sconfig);
+	if (ret < 0) {
+		dev_err(dev, "can't configure rx channel\n");
+		goto fail_rx;
+	}
+
+	init_completion(&dma->dma_complete);
+
+	dev_info(dev, "using %s (tx) and %s (rx) for DMA transfers\n",
+		 dma_chan_name(dma->chan_tx), dma_chan_name(dma->chan_rx));
+
+	return dma;
+
+fail_rx:
+	dma_release_channel(dma->chan_rx);
+fail_tx:
+	dma_release_channel(dma->chan_tx);
+fail_al:
+	devm_kfree(dev, dma);
+	dev_info(dev, "can't use DMA\n");
+
+	return NULL;
+}
+
+void stm32_i2c_dma_free(struct stm32_i2c_dma *dma)
+{
+	dma->dma_buf = 0;
+	dma->dma_len = 0;
+
+	dma_release_channel(dma->chan_tx);
+	dma->chan_tx = NULL;
+
+	dma_release_channel(dma->chan_rx);
+	dma->chan_rx = NULL;
+
+	dma->chan_using = NULL;
+}
+
+int stm32_i2c_prep_dma_xfer(struct device *dev, struct stm32_i2c_dma *dma,
+			    bool rd_wr, u32 len, u8 *buf,
+			    dma_async_tx_callback callback,
+			    void *dma_async_param)
+{
+	struct dma_async_tx_descriptor *txdesc;
+	struct device *chan_dev;
+	int ret;
+
+	if (rd_wr) {
+		dma->chan_using = dma->chan_rx;
+		dma->dma_transfer_dir = DMA_DEV_TO_MEM;
+		dma->dma_data_dir = DMA_FROM_DEVICE;
+	} else {
+		dma->chan_using = dma->chan_tx;
+		dma->dma_transfer_dir = DMA_MEM_TO_DEV;
+		dma->dma_data_dir = DMA_TO_DEVICE;
+	}
+
+	dma->dma_len = len;
+	chan_dev = dma->chan_using->device->dev;
+
+	dma->dma_buf = dma_map_single(chan_dev, buf, dma->dma_len,
+				      dma->dma_data_dir);
+	if (dma_mapping_error(chan_dev, dma->dma_buf)) {
+		dev_err(dev, "DMA mapping failed\n");
+		return -EINVAL;
+	}
+
+	txdesc = dmaengine_prep_slave_single(dma->chan_using, dma->dma_buf,
+					     dma->dma_len,
+					     dma->dma_transfer_dir,
+					     DMA_PREP_INTERRUPT);
+	if (!txdesc) {
+		dev_err(dev, "Not able to get desc for DMA xfer\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	reinit_completion(&dma->dma_complete);
+
+	txdesc->callback = callback;
+	txdesc->callback_param = dma_async_param;
+	ret = dma_submit_error(dmaengine_submit(txdesc));
+	if (ret < 0) {
+		dev_err(dev, "DMA submit failed\n");
+		goto err;
+	}
+
+	dma_async_issue_pending(dma->chan_using);
+
+	return 0;
+
+err:
+	dma_unmap_single(chan_dev, dma->dma_buf, dma->dma_len,
+			 dma->dma_data_dir);
+	return ret;
+}
diff --git a/drivers/i2c/busses/i2c-stm32.h b/drivers/i2c/busses/i2c-stm32.h
index d4f9cef..868755f 100644
--- a/drivers/i2c/busses/i2c-stm32.h
+++ b/drivers/i2c/busses/i2c-stm32.h
@@ -11,6 +11,10 @@
 #ifndef _I2C_STM32_H
 #define _I2C_STM32_H
 
+#include <linux/dma-direction.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+
 enum stm32_i2c_speed {
 	STM32_I2C_SPEED_STANDARD, /* 100 kHz */
 	STM32_I2C_SPEED_FAST, /* 400 kHz */
@@ -18,4 +22,37 @@ enum stm32_i2c_speed {
 	STM32_I2C_SPEED_END,
 };
 
+/**
+ * struct stm32_i2c_dma - DMA specific data
+ * @chan_tx: dma channel for TX transfer
+ * @chan_rx: dma channel for RX transfer
+ * @chan_using: dma channel used for the current transfer (TX or RX)
+ * @dma_buf: dma buffer
+ * @dma_len: dma buffer len
+ * @dma_transfer_dir: dma transfer direction indicator
+ * @dma_data_dir: dma transfer mode indicator
+ * @dma_complete: dma transfer completion
+ */
+struct stm32_i2c_dma {
+	struct dma_chan *chan_tx;
+	struct dma_chan *chan_rx;
+	struct dma_chan *chan_using;
+	dma_addr_t dma_buf;
+	unsigned int dma_len;
+	enum dma_transfer_direction dma_transfer_dir;
+	enum dma_data_direction dma_data_dir;
+	struct completion dma_complete;
+};
+
+struct stm32_i2c_dma *stm32_i2c_dma_request(struct device *dev,
+					    dma_addr_t phy_addr,
+					    u32 txdr_offset, u32 rxdr_offset);
+
+void stm32_i2c_dma_free(struct stm32_i2c_dma *dma);
+
+int stm32_i2c_prep_dma_xfer(struct device *dev, struct stm32_i2c_dma *dma,
+			    bool rd_wr, u32 len, u8 *buf,
+			    dma_async_tx_callback callback,
+			    void *dma_async_param);
+
 #endif /* _I2C_STM32_H */
-- 
2.7.4

[PATCH v2 5/6] i2c: i2c-stm32f7: Add DMA support

[Pierre-Yves MORDRET]

This patch adds DMA support for i2c-stm32f7 driver

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
---
  Version history:
    v1:
       * Initial
    v2:
       * fix kbuild test robot issue (format)
---

fixup! i2c: i2c-stm32f7: Add DMA support
---
 drivers/i2c/busses/Makefile      |   3 +-
 drivers/i2c/busses/i2c-stm32f7.c | 214 +++++++++++++++++++++++++++++++++++----
 2 files changed, 196 insertions(+), 21 deletions(-)

diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 2ce8576..8dbfaf0 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -95,7 +95,8 @@ obj-$(CONFIG_I2C_SIRF)		+= i2c-sirf.o
 obj-$(CONFIG_I2C_SPRD)		+= i2c-sprd.o
 obj-$(CONFIG_I2C_ST)		+= i2c-st.o
 obj-$(CONFIG_I2C_STM32F4)	+= i2c-stm32f4.o
-obj-$(CONFIG_I2C_STM32F7)	+= i2c-stm32f7.o
+i2c-stm32f7-drv-objs := i2c-stm32f7.o i2c-stm32.o
+obj-$(CONFIG_I2C_STM32F7)	+= i2c-stm32f7-drv.o
 obj-$(CONFIG_I2C_STU300)	+= i2c-stu300.o
 obj-$(CONFIG_I2C_SUN6I_P2WI)	+= i2c-sun6i-p2wi.o
 obj-$(CONFIG_I2C_TEGRA)		+= i2c-tegra.o
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 0b81b5d..91f73e0 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -47,6 +47,8 @@
 /* STM32F7 I2C control 1 */
 #define STM32F7_I2C_CR1_PECEN			BIT(23)
 #define STM32F7_I2C_CR1_SBC			BIT(16)
+#define STM32F7_I2C_CR1_RXDMAEN			BIT(15)
+#define STM32F7_I2C_CR1_TXDMAEN			BIT(14)
 #define STM32F7_I2C_CR1_ANFOFF			BIT(12)
 #define STM32F7_I2C_CR1_ERRIE			BIT(7)
 #define STM32F7_I2C_CR1_TCIE			BIT(6)
@@ -142,6 +144,7 @@
 #define STM32F7_I2C_TIMINGR_SCLL(n)		((n) & 0xff)
 
 #define STM32F7_I2C_MAX_LEN			0xff
+#define STM32F7_I2C_DMA_LEN_MIN			0x1
 #define STM32F7_I2C_MAX_SLAVE			0x2
 
 #define STM32F7_I2C_DNF_DEFAULT			0
@@ -270,6 +273,8 @@ struct stm32f7_i2c_msg {
  * @slave_dir: transfer direction for the current slave device
  * @master_mode: boolean to know in which mode the I2C is running (master or
  * slave)
+ * @dma: dma data
+ * @use_dma: boolean to know if dma is used in the current transfer
  */
 struct stm32f7_i2c_dev {
 	struct i2c_adapter adap;
@@ -288,6 +293,8 @@ struct stm32f7_i2c_dev {
 	struct i2c_client *slave_running;
 	u32 slave_dir;
 	bool master_mode;
+	struct stm32_i2c_dma *dma;
+	bool use_dma;
 };
 
 /**
@@ -599,6 +606,25 @@ static int stm32f7_i2c_setup_timing(struct stm32f7_i2c_dev *i2c_dev,
 	return 0;
 }
 
+static void stm32f7_i2c_disable_dma_req(struct stm32f7_i2c_dev *i2c_dev)
+{
+	void __iomem *base = i2c_dev->base;
+	u32 mask = STM32F7_I2C_CR1_RXDMAEN | STM32F7_I2C_CR1_TXDMAEN;
+
+	stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR1, mask);
+}
+
+static void stm32f7_i2c_dma_callback(void *arg)
+{
+	struct stm32f7_i2c_dev *i2c_dev = (struct stm32f7_i2c_dev *)arg;
+	struct stm32_i2c_dma *dma = i2c_dev->dma;
+	struct device *dev = dma->chan_using->device->dev;
+
+	stm32f7_i2c_disable_dma_req(i2c_dev);
+	dma_unmap_single(dev, dma->dma_buf, dma->dma_len, dma->dma_data_dir);
+	complete(&dma->dma_complete);
+}
+
 static void stm32f7_i2c_hw_config(struct stm32f7_i2c_dev *i2c_dev)
 {
 	struct stm32f7_i2c_timings *t = &i2c_dev->timing;
@@ -653,6 +679,9 @@ static void stm32f7_i2c_reload(struct stm32f7_i2c_dev *i2c_dev)
 	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
 	u32 cr2;
 
+	if (i2c_dev->use_dma)
+		f7_msg->count -= STM32F7_I2C_MAX_LEN;
+
 	cr2 = readl_relaxed(i2c_dev->base + STM32F7_I2C_CR2);
 
 	cr2 &= ~STM32F7_I2C_CR2_NBYTES_MASK;
@@ -712,6 +741,7 @@ static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
 	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
 	void __iomem *base = i2c_dev->base;
 	u32 cr1, cr2;
+	int ret;
 
 	f7_msg->addr = msg->addr;
 	f7_msg->buf = msg->buf;
@@ -753,14 +783,35 @@ static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
 	cr1 |= STM32F7_I2C_CR1_ERRIE | STM32F7_I2C_CR1_TCIE |
 		STM32F7_I2C_CR1_STOPIE | STM32F7_I2C_CR1_NACKIE;
 
-	/* Clear TX/RX interrupt */
-	cr1 &= ~(STM32F7_I2C_CR1_RXIE | STM32F7_I2C_CR1_TXIE);
+	/* Clear DMA req and TX/RX interrupt */
+	cr1 &= ~(STM32F7_I2C_CR1_RXIE | STM32F7_I2C_CR1_TXIE |
+			STM32F7_I2C_CR1_RXDMAEN | STM32F7_I2C_CR1_TXDMAEN);
+
+	/* Configure DMA or enable RX/TX interrupt */
+	i2c_dev->use_dma = false;
+	if (i2c_dev->dma && f7_msg->count >= STM32F7_I2C_DMA_LEN_MIN) {
+		ret = stm32_i2c_prep_dma_xfer(i2c_dev->dev, i2c_dev->dma,
+					      msg->flags & I2C_M_RD,
+					      f7_msg->count, f7_msg->buf,
+					      stm32f7_i2c_dma_callback,
+					      i2c_dev);
+		if (!ret)
+			i2c_dev->use_dma = true;
+		else
+			dev_warn(i2c_dev->dev, "can't use DMA\n");
+	}
 
-	/* Enable RX/TX interrupt according to msg direction */
-	if (msg->flags & I2C_M_RD)
-		cr1 |= STM32F7_I2C_CR1_RXIE;
-	else
-		cr1 |= STM32F7_I2C_CR1_TXIE;
+	if (!i2c_dev->use_dma) {
+		if (msg->flags & I2C_M_RD)
+			cr1 |= STM32F7_I2C_CR1_RXIE;
+		else
+			cr1 |= STM32F7_I2C_CR1_TXIE;
+	} else {
+		if (msg->flags & I2C_M_RD)
+			cr1 |= STM32F7_I2C_CR1_RXDMAEN;
+		else
+			cr1 |= STM32F7_I2C_CR1_TXDMAEN;
+	}
 
 	/* Configure Start/Repeated Start */
 	cr2 |= STM32F7_I2C_CR2_START;
@@ -780,7 +831,7 @@ static int stm32f7_i2c_smbus_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
 	struct device *dev = i2c_dev->dev;
 	void __iomem *base = i2c_dev->base;
 	u32 cr1, cr2;
-	int i;
+	int i, ret;
 
 	f7_msg->result = 0;
 	reinit_completion(&i2c_dev->complete);
@@ -895,14 +946,35 @@ static int stm32f7_i2c_smbus_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
 	cr1 |= STM32F7_I2C_CR1_ERRIE | STM32F7_I2C_CR1_TCIE |
 		STM32F7_I2C_CR1_STOPIE | STM32F7_I2C_CR1_NACKIE;
 
-	/* Clear TX/RX interrupt */
-	cr1 &= ~(STM32F7_I2C_CR1_RXIE | STM32F7_I2C_CR1_TXIE);
+	/* Clear DMA req and TX/RX interrupt */
+	cr1 &= ~(STM32F7_I2C_CR1_RXIE | STM32F7_I2C_CR1_TXIE |
+			STM32F7_I2C_CR1_RXDMAEN | STM32F7_I2C_CR1_TXDMAEN);
+
+	/* Configure DMA or enable RX/TX interrupt */
+	i2c_dev->use_dma = false;
+	if (i2c_dev->dma && f7_msg->count >= STM32F7_I2C_DMA_LEN_MIN) {
+		ret = stm32_i2c_prep_dma_xfer(i2c_dev->dev, i2c_dev->dma,
+					      cr2 & STM32F7_I2C_CR2_RD_WRN,
+					      f7_msg->count, f7_msg->buf,
+					      stm32f7_i2c_dma_callback,
+					      i2c_dev);
+		if (!ret)
+			i2c_dev->use_dma = true;
+		else
+			dev_warn(i2c_dev->dev, "can't use DMA\n");
+	}
 
-	/* Enable RX/TX interrupt according to msg direction */
-	if (cr2 & STM32F7_I2C_CR2_RD_WRN)
-		cr1 |= STM32F7_I2C_CR1_RXIE;
-	else
-		cr1 |= STM32F7_I2C_CR1_TXIE;
+	if (!i2c_dev->use_dma) {
+		if (cr2 & STM32F7_I2C_CR2_RD_WRN)
+			cr1 |= STM32F7_I2C_CR1_RXIE;
+		else
+			cr1 |= STM32F7_I2C_CR1_TXIE;
+	} else {
+		if (cr2 & STM32F7_I2C_CR2_RD_WRN)
+			cr1 |= STM32F7_I2C_CR1_RXDMAEN;
+		else
+			cr1 |= STM32F7_I2C_CR1_TXDMAEN;
+	}
 
 	/* Set Start bit */
 	cr2 |= STM32F7_I2C_CR2_START;
@@ -921,6 +993,7 @@ static void stm32f7_i2c_smbus_rep_start(struct stm32f7_i2c_dev *i2c_dev)
 	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
 	void __iomem *base = i2c_dev->base;
 	u32 cr1, cr2;
+	int ret;
 
 	cr2 = readl_relaxed(base + STM32F7_I2C_CR2);
 	cr1 = readl_relaxed(base + STM32F7_I2C_CR1);
@@ -960,6 +1033,35 @@ static void stm32f7_i2c_smbus_rep_start(struct stm32f7_i2c_dev *i2c_dev)
 	cr1 &= ~(STM32F7_I2C_CR1_RXIE | STM32F7_I2C_CR1_TXIE);
 	cr1 |= STM32F7_I2C_CR1_RXIE;
 
+	/*
+	 * Configure DMA or enable RX/TX interrupt:
+	 * For I2C_SMBUS_BLOCK_DATA and I2C_SMBUS_BLOCK_PROC_CALL we don't use
+	 * dma as we don't know in advance how many data will be received
+	 */
+	cr1 &= ~(STM32F7_I2C_CR1_RXIE | STM32F7_I2C_CR1_TXIE |
+		 STM32F7_I2C_CR1_RXDMAEN | STM32F7_I2C_CR1_TXDMAEN);
+
+	i2c_dev->use_dma = false;
+	if (i2c_dev->dma && f7_msg->count >= STM32F7_I2C_DMA_LEN_MIN &&
+	    f7_msg->size != I2C_SMBUS_BLOCK_DATA &&
+	    f7_msg->size != I2C_SMBUS_BLOCK_PROC_CALL) {
+		ret = stm32_i2c_prep_dma_xfer(i2c_dev->dev, i2c_dev->dma,
+					      cr2 & STM32F7_I2C_CR2_RD_WRN,
+					      f7_msg->count, f7_msg->buf,
+					      stm32f7_i2c_dma_callback,
+					      i2c_dev);
+
+		if (!ret)
+			i2c_dev->use_dma = true;
+		else
+			dev_warn(i2c_dev->dev, "can't use DMA\n");
+	}
+
+	if (!i2c_dev->use_dma)
+		cr1 |= STM32F7_I2C_CR1_RXIE;
+	else
+		cr1 |= STM32F7_I2C_CR1_RXDMAEN;
+
 	/* Configure Repeated Start */
 	cr2 |= STM32F7_I2C_CR2_START;
 
@@ -1248,7 +1350,7 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
 	void __iomem *base = i2c_dev->base;
 	u32 status, mask;
-	int ret;
+	int ret = IRQ_HANDLED;
 
 	/* Check if the interrupt if for a slave device */
 	if (!i2c_dev->master_mode) {
@@ -1285,8 +1387,12 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 		/* Clear STOP flag */
 		writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
 
-		i2c_dev->master_mode = false;
-		complete(&i2c_dev->complete);
+		if (i2c_dev->use_dma) {
+			ret = IRQ_WAKE_THREAD;
+		} else {
+			i2c_dev->master_mode = false;
+			complete(&i2c_dev->complete);
+		}
 	}
 
 	/* Transfer complete */
@@ -1294,6 +1400,8 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 		if (f7_msg->stop) {
 			mask = STM32F7_I2C_CR2_STOP;
 			stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
+		} else if (i2c_dev->use_dma) {
+			ret = IRQ_WAKE_THREAD;
 		} else if (f7_msg->smbus) {
 			stm32f7_i2c_smbus_rep_start(i2c_dev);
 		} else {
@@ -1310,6 +1418,44 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 			stm32f7_i2c_reload(i2c_dev);
 	}
 
+	return ret;
+}
+
+static irqreturn_t stm32f7_i2c_isr_event_thread(int irq, void *data)
+{
+	struct stm32f7_i2c_dev *i2c_dev = data;
+	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	struct stm32_i2c_dma *dma = i2c_dev->dma;
+	u32 status;
+	int ret;
+
+	/*
+	 * Wait for dma transfer completion before sending next message or
+	 * notity the end of xfer to the client
+	 */
+	ret = wait_for_completion_timeout(&i2c_dev->dma->dma_complete, HZ);
+	if (!ret) {
+		dev_dbg(i2c_dev->dev, "<%s>: Timed out\n", __func__);
+		stm32f7_i2c_disable_dma_req(i2c_dev);
+		dmaengine_terminate_all(dma->chan_using);
+		f7_msg->result = -ETIMEDOUT;
+	}
+
+	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
+
+	if (status & STM32F7_I2C_ISR_TC) {
+		if (f7_msg->smbus) {
+			stm32f7_i2c_smbus_rep_start(i2c_dev);
+		} else {
+			i2c_dev->msg_id++;
+			i2c_dev->msg++;
+			stm32f7_i2c_xfer_msg(i2c_dev, i2c_dev->msg);
+		}
+	} else {
+		i2c_dev->master_mode = false;
+		complete(&i2c_dev->complete);
+	}
+
 	return IRQ_HANDLED;
 }
 
@@ -1319,6 +1465,7 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
 	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
 	void __iomem *base = i2c_dev->base;
 	struct device *dev = i2c_dev->dev;
+	struct stm32_i2c_dma *dma = i2c_dev->dma;
 	u32 mask, status;
 
 	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
@@ -1350,6 +1497,12 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
 		mask = STM32F7_I2C_ALL_IRQ_MASK;
 	stm32f7_i2c_disable_irq(i2c_dev, mask);
 
+	/* Disable dma */
+	if (i2c_dev->use_dma) {
+		stm32f7_i2c_disable_dma_req(i2c_dev);
+		dmaengine_terminate_all(dma->chan_using);
+	}
+
 	i2c_dev->master_mode = false;
 	complete(&i2c_dev->complete);
 
@@ -1361,6 +1514,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
 {
 	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
 	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	struct stm32_i2c_dma *dma = i2c_dev->dma;
 	unsigned long time_left;
 	int ret;
 
@@ -1388,6 +1542,8 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
 	if (!time_left) {
 		dev_dbg(i2c_dev->dev, "Access to slave 0x%x timed out\n",
 			i2c_dev->msg->addr);
+		if (i2c_dev->use_dma)
+			dmaengine_terminate_all(dma->chan_using);
 		ret = -ETIMEDOUT;
 	}
 
@@ -1404,6 +1560,7 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
 {
 	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(adapter);
 	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	struct stm32_i2c_dma *dma = i2c_dev->dma;
 	struct device *dev = i2c_dev->dev;
 	unsigned long timeout;
 	int i, ret;
@@ -1435,6 +1592,8 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
 
 	if (!timeout) {
 		dev_dbg(dev, "Access to slave 0x%x timed out\n", f7_msg->addr);
+		if (i2c_dev->use_dma)
+			dmaengine_terminate_all(dma->chan_using);
 		ret = -ETIMEDOUT;
 		goto clk_free;
 	}
@@ -1608,6 +1767,7 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
 	u32 irq_error, irq_event, clk_rate, rise_time, fall_time;
 	struct i2c_adapter *adap;
 	struct reset_control *rst;
+	dma_addr_t phy_addr;
 	int ret;
 
 	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
@@ -1618,6 +1778,7 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
 	i2c_dev->base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(i2c_dev->base))
 		return PTR_ERR(i2c_dev->base);
+	phy_addr = (dma_addr_t)res->start;
 
 	irq_event = irq_of_parse_and_map(np, 0);
 	if (!irq_event) {
@@ -1664,8 +1825,11 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
 
 	i2c_dev->dev = &pdev->dev;
 
-	ret = devm_request_irq(&pdev->dev, irq_event, stm32f7_i2c_isr_event, 0,
-			       pdev->name, i2c_dev);
+	ret = devm_request_threaded_irq(&pdev->dev, irq_event,
+					stm32f7_i2c_isr_event,
+					stm32f7_i2c_isr_event_thread,
+					IRQF_ONESHOT,
+					pdev->name, i2c_dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to request irq event %i\n",
 			irq_event);
@@ -1717,6 +1881,11 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
 
 	init_completion(&i2c_dev->complete);
 
+	/* Init DMA config if supported */
+	i2c_dev->dma = stm32_i2c_dma_request(i2c_dev->dev, phy_addr,
+					     STM32F7_I2C_TXDR,
+					     STM32F7_I2C_RXDR);
+
 	ret = i2c_add_adapter(adap);
 	if (ret)
 		goto clk_free;
@@ -1739,6 +1908,11 @@ static int stm32f7_i2c_remove(struct platform_device *pdev)
 {
 	struct stm32f7_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
 
+	if (i2c_dev->dma) {
+		stm32_i2c_dma_free(i2c_dev->dma);
+		i2c_dev->dma = NULL;
+	}
+
 	i2c_del_adapter(&i2c_dev->adap);
 
 	clk_unprepare(i2c_dev->clk);
-- 
2.7.4

[PATCH v2 6/6] i2c: i2c-stm32f7: Implement I2C recovery mechanism

[Pierre-Yves MORDRET]

Feature prevents I2C lock-ups. Mechanism resets I2C state machine
and releases SCL/SDA signals but preserves I2C registers.

Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
---
  Version history:
    v1:
       * Initial
    v2:
       * Don't use i2c engine recovery mechanism since driver
         procedure only recover master and not the slave.
---
---
 drivers/i2c/busses/i2c-stm32f7.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 91f73e0..9a9c469 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -718,6 +718,20 @@ static void stm32f7_i2c_smbus_reload(struct stm32f7_i2c_dev *i2c_dev)
 	writel_relaxed(cr2, i2c_dev->base + STM32F7_I2C_CR2);
 }
 
+static int stm32f7_i2c_recover_bus(struct i2c_adapter *i2c_adap)
+{
+	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
+
+	dev_info(i2c_dev->dev, "Trying to recover bus\n");
+
+	stm32f7_i2c_clr_bits(i2c_dev->base + STM32F7_I2C_CR1,
+			     STM32F7_I2C_CR1_PE);
+
+	stm32f7_i2c_hw_config(i2c_dev);
+
+	return 0;
+}
+
 static int stm32f7_i2c_wait_free_bus(struct stm32f7_i2c_dev *i2c_dev)
 {
 	u32 status;
@@ -727,12 +741,18 @@ static int stm32f7_i2c_wait_free_bus(struct stm32f7_i2c_dev *i2c_dev)
 					 status,
 					 !(status & STM32F7_I2C_ISR_BUSY),
 					 10, 1000);
+	if (!ret)
+		return 0;
+
+	dev_info(i2c_dev->dev, "bus busy\n");
+
+	ret = stm32f7_i2c_recover_bus(&i2c_dev->adap);
 	if (ret) {
-		dev_dbg(i2c_dev->dev, "bus busy\n");
-		ret = -EBUSY;
+		dev_err(i2c_dev->dev, "Failed to recover the bus (%d)\n", ret);
+		return ret;
 	}
 
-	return ret;
+	return -EBUSY;
 }
 
 static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
@@ -1474,6 +1494,7 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
 	if (status & STM32F7_I2C_ISR_BERR) {
 		dev_err(dev, "<%s>: Bus error\n", __func__);
 		writel_relaxed(STM32F7_I2C_ICR_BERRCF, base + STM32F7_I2C_ICR);
+		stm32f7_i2c_recover_bus(&i2c_dev->adap);
 		f7_msg->result = -EIO;
 	}
 
-- 
2.7.4

Re: [PATCH v2 1/6] i2c: i2c-stm32f7: Add 10-bit address support

[Wolfram Sang]

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

On Wed, Mar 21, 2018 at 05:48:55PM +0100, Pierre-Yves MORDRET wrote:
> This patch adds support for 10-bit device address for STM32F7 I2C
> 
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>

Out of curiosity: how did you test this patch? I never managed to find a
10-bit client (except for an SoC with 10-bit slave mode).

> ---
>   Version history:
>      v1:
>         * Initial
>      v2:
> ---
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
> index f273e28..ae0d15c 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -65,7 +65,12 @@
>  #define STM32F7_I2C_CR2_NACK			BIT(15)
>  #define STM32F7_I2C_CR2_STOP			BIT(14)
>  #define STM32F7_I2C_CR2_START			BIT(13)
> +#define STM32F7_I2C_CR2_HEAD10R			BIT(12)
> +#define STM32F7_I2C_CR2_ADD10			BIT(11)
>  #define STM32F7_I2C_CR2_RD_WRN			BIT(10)
> +#define STM32F7_I2C_CR2_SADD10_MASK		GENMASK(9, 0)
> +#define STM32F7_I2C_CR2_SADD10(n)		(((n) & \
> +						STM32F7_I2C_CR2_SADD10_MASK))
>  #define STM32F7_I2C_CR2_SADD7_MASK		GENMASK(7, 1)
>  #define STM32F7_I2C_CR2_SADD7(n)		(((n) & 0x7f) << 1)
>  
> @@ -176,14 +181,14 @@ struct stm32f7_i2c_timings {
>  
>  /**
>   * struct stm32f7_i2c_msg - client specific data
> - * @addr: 8-bit slave addr, including r/w bit
> + * @addr: 8-bit or 10-bit slave addr, including r/w bit
>   * @count: number of bytes to be transferred
>   * @buf: data buffer
>   * @result: result of the transfer
>   * @stop: last I2C msg to be sent, i.e. STOP to be generated
>   */
>  struct stm32f7_i2c_msg {
> -	u8 addr;
> +	u16 addr;
>  	u32 count;
>  	u8 *buf;
>  	int result;
> @@ -629,8 +634,15 @@ static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
>  		cr2 |= STM32F7_I2C_CR2_RD_WRN;
>  
>  	/* Set slave address */
> -	cr2 &= ~STM32F7_I2C_CR2_SADD7_MASK;
> -	cr2 |= STM32F7_I2C_CR2_SADD7(f7_msg->addr);
> +	cr2 &= ~(STM32F7_I2C_CR2_HEAD10R | STM32F7_I2C_CR2_ADD10);
> +	if (msg->flags & I2C_M_TEN) {
> +		cr2 &= ~STM32F7_I2C_CR2_SADD10_MASK;
> +		cr2 |= STM32F7_I2C_CR2_SADD10(f7_msg->addr);
> +		cr2 |= STM32F7_I2C_CR2_ADD10;
> +	} else {
> +		cr2 &= ~STM32F7_I2C_CR2_SADD7_MASK;
> +		cr2 |= STM32F7_I2C_CR2_SADD7(f7_msg->addr);
> +	}
>  
>  	/* Set nb bytes to transfer and reload if needed */
>  	cr2 &= ~(STM32F7_I2C_CR2_NBYTES_MASK | STM32F7_I2C_CR2_RELOAD);
> @@ -798,7 +810,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
>  
>  static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
>  {
> -	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR;
>  }
>  
>  static struct i2c_algorithm stm32f7_i2c_algo = {
> -- 
> 2.7.4
> 

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

Re: [PATCH v2 3/6] i2c: i2c-stm32f7: Add initial SMBus protocols support

[Wolfram Sang]

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

On Wed, Mar 21, 2018 at 05:48:57PM +0100, Pierre-Yves MORDRET wrote:
> This patch adds SMBus support for I2C controller embedded in STM32F7 Soc.

> All SMBus protocols are implemented except SMBus-specific protocols.

What does that mean?

> 
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
> ---
>   Version history:
>     v1:
>        * Initial
>     v2:
>        * fix Kbuild test robot issue (Unneeded semicolon)
> ---
> 
> fixup! i2c: i2c-stm32f7: Add initial SMBus protocols support
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 377 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 368 insertions(+), 9 deletions(-)

That is quite some complexity considering we have I2C_FUNC_SMBUS_EMUL. I
don't mind, but you really want that?


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

Re: [PATCH v2 4/6] i2c: i2c-stm32: Add generic DMA API

[Wolfram Sang]

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

On Wed, Mar 21, 2018 at 05:48:58PM +0100, Pierre-Yves MORDRET wrote:
> This patch adds a generic DMA API to implement DMA support for i2c-stm32fx
> drivers
> 
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>

In case you haven't read it so far, there is a new document about I2C &
DMA -> Documentation/i2c/DMA-considerations

Just so you know...


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

Re: [PATCH v2 6/6] i2c: i2c-stm32f7: Implement I2C recovery mechanism

[Wolfram Sang]

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

On Wed, Mar 21, 2018 at 05:49:00PM +0100, Pierre-Yves MORDRET wrote:
> Feature prevents I2C lock-ups. Mechanism resets I2C state machine
> and releases SCL/SDA signals but preserves I2C registers.
> 
> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
> ---
>   Version history:
>     v1:
>        * Initial
>     v2:
>        * Don't use i2c engine recovery mechanism since driver
>          procedure only recover master and not the slave.

s/recovery/release/ throughout the patch, please. Recovery is really
something else. Also, I think the dev_info's are too noisy in the log
files. I'd think the whole driver could be lifted from quite some
logging...

> ---
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
> index 91f73e0..9a9c469 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -718,6 +718,20 @@ static void stm32f7_i2c_smbus_reload(struct stm32f7_i2c_dev *i2c_dev)
>  	writel_relaxed(cr2, i2c_dev->base + STM32F7_I2C_CR2);
>  }
>  
> +static int stm32f7_i2c_recover_bus(struct i2c_adapter *i2c_adap)
> +{
> +	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
> +
> +	dev_info(i2c_dev->dev, "Trying to recover bus\n");
> +
> +	stm32f7_i2c_clr_bits(i2c_dev->base + STM32F7_I2C_CR1,
> +			     STM32F7_I2C_CR1_PE);
> +
> +	stm32f7_i2c_hw_config(i2c_dev);
> +
> +	return 0;
> +}
> +
>  static int stm32f7_i2c_wait_free_bus(struct stm32f7_i2c_dev *i2c_dev)
>  {
>  	u32 status;
> @@ -727,12 +741,18 @@ static int stm32f7_i2c_wait_free_bus(struct stm32f7_i2c_dev *i2c_dev)
>  					 status,
>  					 !(status & STM32F7_I2C_ISR_BUSY),
>  					 10, 1000);
> +	if (!ret)
> +		return 0;
> +
> +	dev_info(i2c_dev->dev, "bus busy\n");
> +
> +	ret = stm32f7_i2c_recover_bus(&i2c_dev->adap);
>  	if (ret) {
> -		dev_dbg(i2c_dev->dev, "bus busy\n");
> -		ret = -EBUSY;
> +		dev_err(i2c_dev->dev, "Failed to recover the bus (%d)\n", ret);
> +		return ret;
>  	}
>  
> -	return ret;
> +	return -EBUSY;
>  }
>  
>  static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
> @@ -1474,6 +1494,7 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
>  	if (status & STM32F7_I2C_ISR_BERR) {
>  		dev_err(dev, "<%s>: Bus error\n", __func__);
>  		writel_relaxed(STM32F7_I2C_ICR_BERRCF, base + STM32F7_I2C_ICR);
> +		stm32f7_i2c_recover_bus(&i2c_dev->adap);
>  		f7_msg->result = -EIO;
>  	}
>  
> -- 
> 2.7.4
> 

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

Re: [PATCH v2 0/6] Add different features for I2C

[Wolfram Sang]

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

On Wed, Mar 21, 2018 at 05:48:54PM +0100, Pierre-Yves MORDRET wrote:
> Append new I2C STM32F7 feature set. This includes 10 bit support, slave
> support, SMBBus protocols support, DMA Support and eventually an I2C recovery
> mechanism.

So, I gave a few comments. For hardware details (especially DMA), some
additional review from Maxime or Alexandre would be great.


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

Re: [PATCH v2 2/6] i2c: i2c-stm32f7: Add slave support

[Wolfram Sang]

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

On Wed, Mar 21, 2018 at 05:48:56PM +0100, Pierre-Yves MORDRET wrote:
> This patch adds slave support for I2C controller embedded in STM32F7 SoC
> 
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>

Looks OK from a first look. What kind of tests did you do?


> ---
>   Version history:
>      v1:
>         * Initial
>      v2:
> ---
> ---
>  drivers/i2c/busses/Kconfig       |   1 +
>  drivers/i2c/busses/i2c-stm32f7.c | 434 ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 429 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index e2954fb..118b9be 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -964,6 +964,7 @@ config I2C_STM32F4
>  config I2C_STM32F7
>  	tristate "STMicroelectronics STM32F7 I2C support"
>  	depends on ARCH_STM32 || COMPILE_TEST
> +	select I2C_SLAVE
>  	help
>  	  Enable this option to add support for STM32 I2C controller embedded
>  	  in STM32F7 SoCs.
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
> index ae0d15c..9bf676e 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -35,6 +35,8 @@
>  /* STM32F7 I2C registers */
>  #define STM32F7_I2C_CR1				0x00
>  #define STM32F7_I2C_CR2				0x04
> +#define STM32F7_I2C_OAR1			0x08
> +#define STM32F7_I2C_OAR2			0x0C
>  #define STM32F7_I2C_TIMINGR			0x10
>  #define STM32F7_I2C_ISR				0x18
>  #define STM32F7_I2C_ICR				0x1C
> @@ -42,6 +44,7 @@
>  #define STM32F7_I2C_TXDR			0x28
>  
>  /* STM32F7 I2C control 1 */
> +#define STM32F7_I2C_CR1_SBC			BIT(16)
>  #define STM32F7_I2C_CR1_ANFOFF			BIT(12)
>  #define STM32F7_I2C_CR1_ERRIE			BIT(7)
>  #define STM32F7_I2C_CR1_TCIE			BIT(6)
> @@ -57,6 +60,11 @@
>  						| STM32F7_I2C_CR1_NACKIE \
>  						| STM32F7_I2C_CR1_RXIE \
>  						| STM32F7_I2C_CR1_TXIE)
> +#define STM32F7_I2C_XFER_IRQ_MASK		(STM32F7_I2C_CR1_TCIE \
> +						| STM32F7_I2C_CR1_STOPIE \
> +						| STM32F7_I2C_CR1_NACKIE \
> +						| STM32F7_I2C_CR1_RXIE \
> +						| STM32F7_I2C_CR1_TXIE)
>  
>  /* STM32F7 I2C control 2 */
>  #define STM32F7_I2C_CR2_RELOAD			BIT(24)
> @@ -74,7 +82,34 @@
>  #define STM32F7_I2C_CR2_SADD7_MASK		GENMASK(7, 1)
>  #define STM32F7_I2C_CR2_SADD7(n)		(((n) & 0x7f) << 1)
>  
> +/* STM32F7 I2C Own Address 1 */
> +#define STM32F7_I2C_OAR1_OA1EN			BIT(15)
> +#define STM32F7_I2C_OAR1_OA1MODE		BIT(10)
> +#define STM32F7_I2C_OAR1_OA1_10_MASK		GENMASK(9, 0)
> +#define STM32F7_I2C_OAR1_OA1_10(n)		(((n) & \
> +						STM32F7_I2C_OAR1_OA1_10_MASK))
> +#define STM32F7_I2C_OAR1_OA1_7_MASK		GENMASK(7, 1)
> +#define STM32F7_I2C_OAR1_OA1_7(n)		(((n) & 0x7f) << 1)
> +#define STM32F7_I2C_OAR1_MASK			(STM32F7_I2C_OAR1_OA1_7_MASK \
> +						| STM32F7_I2C_OAR1_OA1_10_MASK \
> +						| STM32F7_I2C_OAR1_OA1EN \
> +						| STM32F7_I2C_OAR1_OA1MODE)
> +
> +/* STM32F7 I2C Own Address 2 */
> +#define STM32F7_I2C_OAR2_OA2EN			BIT(15)
> +#define STM32F7_I2C_OAR2_OA2MSK_MASK		GENMASK(10, 8)
> +#define STM32F7_I2C_OAR2_OA2MSK(n)		(((n) & 0x7) << 8)
> +#define STM32F7_I2C_OAR2_OA2_7_MASK		GENMASK(7, 1)
> +#define STM32F7_I2C_OAR2_OA2_7(n)		(((n) & 0x7f) << 1)
> +#define STM32F7_I2C_OAR2_MASK			(STM32F7_I2C_OAR2_OA2MSK_MASK \
> +						| STM32F7_I2C_OAR2_OA2_7_MASK \
> +						| STM32F7_I2C_OAR2_OA2EN)
> +
>  /* STM32F7 I2C Interrupt Status */
> +#define STM32F7_I2C_ISR_ADDCODE_MASK		GENMASK(23, 17)
> +#define STM32F7_I2C_ISR_ADDCODE_GET(n) \
> +				(((n) & STM32F7_I2C_ISR_ADDCODE_MASK) >> 17)
> +#define STM32F7_I2C_ISR_DIR			BIT(16)
>  #define STM32F7_I2C_ISR_BUSY			BIT(15)
>  #define STM32F7_I2C_ISR_ARLO			BIT(9)
>  #define STM32F7_I2C_ISR_BERR			BIT(8)
> @@ -82,14 +117,17 @@
>  #define STM32F7_I2C_ISR_TC			BIT(6)
>  #define STM32F7_I2C_ISR_STOPF			BIT(5)
>  #define STM32F7_I2C_ISR_NACKF			BIT(4)
> +#define STM32F7_I2C_ISR_ADDR			BIT(3)
>  #define STM32F7_I2C_ISR_RXNE			BIT(2)
>  #define STM32F7_I2C_ISR_TXIS			BIT(1)
> +#define STM32F7_I2C_ISR_TXE			BIT(0)
>  
>  /* STM32F7 I2C Interrupt Clear */
>  #define STM32F7_I2C_ICR_ARLOCF			BIT(9)
>  #define STM32F7_I2C_ICR_BERRCF			BIT(8)
>  #define STM32F7_I2C_ICR_STOPCF			BIT(5)
>  #define STM32F7_I2C_ICR_NACKCF			BIT(4)
> +#define STM32F7_I2C_ICR_ADDRCF			BIT(3)
>  
>  /* STM32F7 I2C Timing */
>  #define STM32F7_I2C_TIMINGR_PRESC(n)		(((n) & 0xf) << 28)
> @@ -99,6 +137,7 @@
>  #define STM32F7_I2C_TIMINGR_SCLL(n)		((n) & 0xff)
>  
>  #define STM32F7_I2C_MAX_LEN			0xff
> +#define STM32F7_I2C_MAX_SLAVE			0x2
>  
>  #define STM32F7_I2C_DNF_DEFAULT			0
>  #define STM32F7_I2C_DNF_MAX			16
> @@ -209,6 +248,11 @@ struct stm32f7_i2c_msg {
>   * @f7_msg: customized i2c msg for driver usage
>   * @setup: I2C timing input setup
>   * @timing: I2C computed timings
> + * @slave: list of slave devices registered on the I2C bus
> + * @slave_running: slave device currently used
> + * @slave_dir: transfer direction for the current slave device
> + * @master_mode: boolean to know in which mode the I2C is running (master or
> + * slave)
>   */
>  struct stm32f7_i2c_dev {
>  	struct i2c_adapter adap;
> @@ -223,6 +267,10 @@ struct stm32f7_i2c_dev {
>  	struct stm32f7_i2c_msg f7_msg;
>  	struct stm32f7_i2c_setup setup;
>  	struct stm32f7_i2c_timings timing;
> +	struct i2c_client *slave[STM32F7_I2C_MAX_SLAVE];
> +	struct i2c_client *slave_running;
> +	u32 slave_dir;
> +	bool master_mode;
>  };
>  
>  /**
> @@ -288,6 +336,11 @@ static inline void stm32f7_i2c_clr_bits(void __iomem *reg, u32 mask)
>  	writel_relaxed(readl_relaxed(reg) & ~mask, reg);
>  }
>  
> +static void stm32f7_i2c_disable_irq(struct stm32f7_i2c_dev *i2c_dev, u32 mask)
> +{
> +	stm32f7_i2c_clr_bits(i2c_dev->base + STM32F7_I2C_CR1, mask);
> +}
> +
>  static int stm32f7_i2c_compute_timing(struct stm32f7_i2c_dev *i2c_dev,
>  				      struct stm32f7_i2c_setup *setup,
>  				      struct stm32f7_i2c_timings *output)
> @@ -572,6 +625,9 @@ static void stm32f7_i2c_read_rx_data(struct stm32f7_i2c_dev *i2c_dev)
>  	if (f7_msg->count) {
>  		*f7_msg->buf++ = readb_relaxed(base + STM32F7_I2C_RXDR);
>  		f7_msg->count--;
> +	} else {
> +		/* Flush RX buffer has no data is expected */
> +		readb_relaxed(base + STM32F7_I2C_RXDR);
>  	}
>  }
>  
> @@ -669,14 +725,250 @@ static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
>  	/* Configure Start/Repeated Start */
>  	cr2 |= STM32F7_I2C_CR2_START;
>  
> +	i2c_dev->master_mode = true;
> +
>  	/* Write configurations registers */
>  	writel_relaxed(cr1, base + STM32F7_I2C_CR1);
>  	writel_relaxed(cr2, base + STM32F7_I2C_CR2);
>  }
>  
> -static void stm32f7_i2c_disable_irq(struct stm32f7_i2c_dev *i2c_dev, u32 mask)
> +static bool stm32f7_i2c_is_addr_match(struct i2c_client *slave, u32 addcode)
>  {
> -	stm32f7_i2c_clr_bits(i2c_dev->base + STM32F7_I2C_CR1, mask);
> +	u32 addr;
> +
> +	if (!slave)
> +		return false;
> +
> +	if (slave->flags & I2C_CLIENT_TEN) {
> +		/*
> +		 * For 10-bit addr, addcode = 11110XY with
> +		 * X = Bit 9 of slave address
> +		 * Y = Bit 8 of slave address
> +		 */
> +		addr = slave->addr >> 8;
> +		addr |= 0x78;
> +		if (addr == addcode)
> +			return true;
> +	} else {
> +		addr = slave->addr & 0x7f;
> +		if (addr == addcode)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static void stm32f7_i2c_slave_start(struct stm32f7_i2c_dev *i2c_dev)
> +{
> +	struct i2c_client *slave = i2c_dev->slave_running;
> +	void __iomem *base = i2c_dev->base;
> +	u32 mask;
> +	u8 value = 0;
> +
> +	if (i2c_dev->slave_dir) {
> +		/* Notify i2c slave that new read transfer is starting */
> +		i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
> +
> +		/*
> +		 * Disable slave TX config in case of I2C combined message
> +		 * (I2C Write followed by I2C Read)
> +		 */
> +		mask = STM32F7_I2C_CR2_RELOAD;
> +		stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR2, mask);
> +		mask = STM32F7_I2C_CR1_SBC | STM32F7_I2C_CR1_RXIE |
> +		       STM32F7_I2C_CR1_TCIE;
> +		stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR1, mask);
> +
> +		/* Enable TX empty, STOP, NACK interrupts */
> +		mask =  STM32F7_I2C_CR1_STOPIE | STM32F7_I2C_CR1_NACKIE |
> +			STM32F7_I2C_CR1_TXIE;
> +		stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, mask);
> +
> +	} else {
> +		/* Notify i2c slave that new write transfer is starting */
> +		i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
> +
> +		/* Set reload mode to be able to ACK/NACK each received byte */
> +		mask = STM32F7_I2C_CR2_RELOAD;
> +		stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
> +
> +		/*
> +		 * Set STOP, NACK, RX empty and transfer complete interrupts.*
> +		 * Set Slave Byte Control to be able to ACK/NACK each data
> +		 * byte received
> +		 */
> +		mask =  STM32F7_I2C_CR1_STOPIE | STM32F7_I2C_CR1_NACKIE |
> +			STM32F7_I2C_CR1_SBC | STM32F7_I2C_CR1_RXIE |
> +			STM32F7_I2C_CR1_TCIE;
> +		stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, mask);
> +	}
> +}
> +
> +static void stm32f7_i2c_slave_addr(struct stm32f7_i2c_dev *i2c_dev)
> +{
> +	void __iomem *base = i2c_dev->base;
> +	u32 isr, addcode, dir, mask;
> +	int i;
> +
> +	isr = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
> +	addcode = STM32F7_I2C_ISR_ADDCODE_GET(isr);
> +	dir = isr & STM32F7_I2C_ISR_DIR;
> +
> +	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
> +		if (stm32f7_i2c_is_addr_match(i2c_dev->slave[i], addcode)) {
> +			i2c_dev->slave_running = i2c_dev->slave[i];
> +			i2c_dev->slave_dir = dir;
> +
> +			/* Start I2C slave processing */
> +			stm32f7_i2c_slave_start(i2c_dev);
> +
> +			/* Clear ADDR flag */
> +			mask = STM32F7_I2C_ICR_ADDRCF;
> +			writel_relaxed(mask, base + STM32F7_I2C_ICR);
> +			break;
> +		}
> +	}
> +}
> +
> +static int stm32f7_i2c_get_slave_id(struct stm32f7_i2c_dev *i2c_dev,
> +				    struct i2c_client *slave, int *id)
> +{
> +	int i;
> +
> +	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
> +		if (i2c_dev->slave[i] == slave) {
> +			*id = i;
> +			return 0;
> +		}
> +	}
> +
> +	dev_err(i2c_dev->dev, "Slave 0x%x not registered\n", slave->addr);
> +
> +	return -ENODEV;
> +}
> +
> +static int stm32f7_i2c_get_free_slave_id(struct stm32f7_i2c_dev *i2c_dev,
> +					 struct i2c_client *slave, int *id)
> +{
> +	struct device *dev = i2c_dev->dev;
> +	int i;
> +
> +	/*
> +	 * slave[0] supports 7-bit and 10-bit slave address
> +	 * slave[1] supports 7-bit slave address only
> +	 */
> +	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
> +		if (i == 1 && (slave->flags & I2C_CLIENT_PEC))
> +			continue;
> +		if (!i2c_dev->slave[i]) {
> +			*id = i;
> +			return 0;
> +		}
> +	}
> +
> +	dev_err(dev, "Slave 0x%x could not be registered\n", slave->addr);
> +
> +	return -EINVAL;
> +}
> +
> +static bool stm32f7_i2c_is_slave_registered(struct stm32f7_i2c_dev *i2c_dev)
> +{
> +	int i;
> +
> +	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
> +		if (i2c_dev->slave[i])
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static bool stm32f7_i2c_is_slave_busy(struct stm32f7_i2c_dev *i2c_dev)
> +{
> +	int i, busy;
> +
> +	busy = 0;
> +	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
> +		if (i2c_dev->slave[i])
> +			busy++;
> +	}
> +
> +	return i == busy;
> +}
> +
> +static irqreturn_t stm32f7_i2c_slave_isr_event(struct stm32f7_i2c_dev *i2c_dev)
> +{
> +	void __iomem *base = i2c_dev->base;
> +	u32 cr2, status, mask;
> +	u8 val;
> +	int ret;
> +
> +	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
> +
> +	/* Slave transmitter mode */
> +	if (status & STM32F7_I2C_ISR_TXIS) {
> +		i2c_slave_event(i2c_dev->slave_running,
> +				I2C_SLAVE_READ_PROCESSED,
> +				&val);
> +
> +		/* Write data byte */
> +		writel_relaxed(val, base + STM32F7_I2C_TXDR);
> +	}
> +
> +	/* Transfer Complete Reload for Slave receiver mode */
> +	if (status & STM32F7_I2C_ISR_TCR || status & STM32F7_I2C_ISR_RXNE) {
> +		/*
> +		 * Read data byte then set NBYTES to receive next byte or NACK
> +		 * the current received byte
> +		 */
> +		val = readb_relaxed(i2c_dev->base + STM32F7_I2C_RXDR);
> +		ret = i2c_slave_event(i2c_dev->slave_running,
> +				      I2C_SLAVE_WRITE_RECEIVED,
> +				      &val);
> +		if (!ret) {
> +			cr2 = readl_relaxed(i2c_dev->base + STM32F7_I2C_CR2);
> +			cr2 |= STM32F7_I2C_CR2_NBYTES(1);
> +			writel_relaxed(cr2, i2c_dev->base + STM32F7_I2C_CR2);
> +		} else {
> +			mask = STM32F7_I2C_CR2_NACK;
> +			stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
> +		}
> +	}
> +
> +	/* NACK received */
> +	if (status & STM32F7_I2C_ISR_NACKF) {
> +		dev_dbg(i2c_dev->dev, "<%s>: Receive NACK\n", __func__);
> +		writel_relaxed(STM32F7_I2C_ICR_NACKCF, base + STM32F7_I2C_ICR);
> +	}
> +
> +	/* STOP received */
> +	if (status & STM32F7_I2C_ISR_STOPF) {
> +		/* Disable interrupts */
> +		stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_XFER_IRQ_MASK);
> +
> +		if (i2c_dev->slave_dir) {
> +			/*
> +			 * Flush TX buffer in order to not used the byte in
> +			 * TXDR for the next transfer
> +			 */
> +			mask = STM32F7_I2C_ISR_TXE;
> +			stm32f7_i2c_set_bits(base + STM32F7_I2C_ISR, mask);
> +		}
> +
> +		/* Clear STOP flag */
> +		writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
> +
> +		/* Notify i2c slave that a STOP flag has been detected */
> +		i2c_slave_event(i2c_dev->slave_running, I2C_SLAVE_STOP, &val);
> +
> +		i2c_dev->slave_running = NULL;
> +	}
> +
> +	/* Address match received */
> +	if (status & STM32F7_I2C_ISR_ADDR)
> +		stm32f7_i2c_slave_addr(i2c_dev);
> +
> +	return IRQ_HANDLED;
>  }
>  
>  static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
> @@ -685,6 +977,13 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
>  	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
>  	void __iomem *base = i2c_dev->base;
>  	u32 status, mask;
> +	int ret;
> +
> +	/* Check if the interrupt if for a slave device */
> +	if (!i2c_dev->master_mode) {
> +		ret = stm32f7_i2c_slave_isr_event(i2c_dev);
> +		return ret;
> +	}
>  
>  	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
>  
> @@ -706,11 +1005,16 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
>  	/* STOP detection flag */
>  	if (status & STM32F7_I2C_ISR_STOPF) {
>  		/* Disable interrupts */
> -		stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
> +		if (stm32f7_i2c_is_slave_registered(i2c_dev))
> +			mask = STM32F7_I2C_XFER_IRQ_MASK;
> +		else
> +			mask = STM32F7_I2C_ALL_IRQ_MASK;
> +		stm32f7_i2c_disable_irq(i2c_dev, mask);
>  
>  		/* Clear STOP flag */
>  		writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
>  
> +		i2c_dev->master_mode = false;
>  		complete(&i2c_dev->complete);
>  	}
>  
> @@ -743,7 +1047,7 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
>  	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
>  	void __iomem *base = i2c_dev->base;
>  	struct device *dev = i2c_dev->dev;
> -	u32 status;
> +	u32 mask, status;
>  
>  	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
>  
> @@ -761,8 +1065,14 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
>  		f7_msg->result = -EAGAIN;
>  	}
>  
> -	stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
> +	/* Disable interrupts */
> +	if (stm32f7_i2c_is_slave_registered(i2c_dev))
> +		mask = STM32F7_I2C_XFER_IRQ_MASK;
> +	else
> +		mask = STM32F7_I2C_ALL_IRQ_MASK;
> +	stm32f7_i2c_disable_irq(i2c_dev, mask);
>  
> +	i2c_dev->master_mode = false;
>  	complete(&i2c_dev->complete);
>  
>  	return IRQ_HANDLED;
> @@ -808,14 +1118,126 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
>  	return (ret < 0) ? ret : num;
>  }
>  
> +static int stm32f7_i2c_reg_slave(struct i2c_client *slave)
> +{
> +	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(slave->adapter);
> +	void __iomem *base = i2c_dev->base;
> +	struct device *dev = i2c_dev->dev;
> +	u32 oar1, oar2, mask;
> +	int id, ret;
> +
> +	if (slave->flags & I2C_CLIENT_PEC) {
> +		dev_err(dev, "SMBus PEC not supported in slave mode\n");
> +		return -EINVAL;
> +	}
> +
> +	if (stm32f7_i2c_is_slave_busy(i2c_dev)) {
> +		dev_err(dev, "Too much slave registered\n");
> +		return -EBUSY;
> +	}
> +
> +	ret = stm32f7_i2c_get_free_slave_id(i2c_dev, slave, &id);
> +	if (ret)
> +		return ret;
> +
> +	if (!(stm32f7_i2c_is_slave_registered(i2c_dev))) {
> +		ret = clk_enable(i2c_dev->clk);
> +		if (ret) {
> +			dev_err(dev, "Failed to enable clock\n");
> +			return ret;
> +		}
> +	}
> +
> +	if (id == 0) {
> +		/* Configure Own Address 1 */
> +		oar1 = readl_relaxed(i2c_dev->base + STM32F7_I2C_OAR1);
> +		oar1 &= ~STM32F7_I2C_OAR1_MASK;
> +		if (slave->flags & I2C_CLIENT_TEN) {
> +			oar1 |= STM32F7_I2C_OAR1_OA1_10(slave->addr);
> +			oar1 |= STM32F7_I2C_OAR1_OA1MODE;
> +		} else {
> +			oar1 |= STM32F7_I2C_OAR1_OA1_7(slave->addr);
> +		}
> +		oar1 |= STM32F7_I2C_OAR1_OA1EN;
> +		i2c_dev->slave[id] = slave;
> +		writel_relaxed(oar1, i2c_dev->base + STM32F7_I2C_OAR1);
> +	} else if (id == 1) {
> +		/* Configure Own Address 2 */
> +		oar2 = readl_relaxed(i2c_dev->base + STM32F7_I2C_OAR2);
> +		oar2 &= ~STM32F7_I2C_OAR2_MASK;
> +		if (slave->flags & I2C_CLIENT_TEN) {
> +			ret = -EOPNOTSUPP;
> +			goto exit;
> +		}
> +
> +		oar2 |= STM32F7_I2C_OAR2_OA2_7(slave->addr);
> +		oar2 |= STM32F7_I2C_OAR2_OA2EN;
> +		i2c_dev->slave[id] = slave;
> +		writel_relaxed(oar2, i2c_dev->base + STM32F7_I2C_OAR2);
> +	} else {
> +		ret = -ENODEV;
> +		goto exit;
> +	}
> +
> +	/* Enable ACK */
> +	stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR2, STM32F7_I2C_CR2_NACK);
> +
> +	/* Enable Address match interrupt, error interrupt and enable I2C  */
> +	mask = STM32F7_I2C_CR1_ADDRIE | STM32F7_I2C_CR1_ERRIE |
> +		STM32F7_I2C_CR1_PE;
> +	stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, mask);
> +
> +	return 0;
> +
> +exit:
> +	if (!(stm32f7_i2c_is_slave_registered(i2c_dev)))
> +		clk_disable(i2c_dev->clk);
> +
> +	return ret;
> +}
> +
> +static int stm32f7_i2c_unreg_slave(struct i2c_client *slave)
> +{
> +	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(slave->adapter);
> +	void __iomem *base = i2c_dev->base;
> +	u32 mask;
> +	int id, ret;
> +
> +	ret = stm32f7_i2c_get_slave_id(i2c_dev, slave, &id);
> +	if (ret)
> +		return ret;
> +
> +	WARN_ON(!i2c_dev->slave[id]);
> +
> +	if (id == 0) {
> +		mask = STM32F7_I2C_OAR1_OA1EN;
> +		stm32f7_i2c_clr_bits(base + STM32F7_I2C_OAR1, mask);
> +	} else {
> +		mask = STM32F7_I2C_OAR2_OA2EN;
> +		stm32f7_i2c_clr_bits(base + STM32F7_I2C_OAR2, mask);
> +	}
> +
> +	i2c_dev->slave[id] = NULL;
> +
> +	if (!(stm32f7_i2c_is_slave_registered(i2c_dev))) {
> +		stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
> +		clk_disable(i2c_dev->clk);
> +	}
> +
> +	return 0;
> +}
> +
>  static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
>  {
> -	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR;
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR |
> +		I2C_FUNC_SLAVE;
>  }
>  
>  static struct i2c_algorithm stm32f7_i2c_algo = {
>  	.master_xfer = stm32f7_i2c_xfer,
>  	.functionality = stm32f7_i2c_func,
> +	.reg_slave = stm32f7_i2c_reg_slave,
> +	.unreg_slave = stm32f7_i2c_unreg_slave,
>  };
>  
>  static int stm32f7_i2c_probe(struct platform_device *pdev)
> -- 
> 2.7.4
> 

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

Re: [PATCH v2 1/6] i2c: i2c-stm32f7: Add 10-bit address support

[Pierre Yves MORDRET]

On 03/24/2018 11:43 PM, Wolfram Sang wrote:
> On Wed, Mar 21, 2018 at 05:48:55PM +0100, Pierre-Yves MORDRET wrote:
>> This patch adds support for 10-bit device address for STM32F7 I2C
>>
>> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
> 
> Out of curiosity: how did you test this patch? I never managed to find a
> 10-bit client (except for an SoC with 10-bit slave mode).

I don't have a 10-bit device either. For testing 10-bit I'm using 2 I2C
instances from the SoC, one in master mode and the other in slave mode.

> 
>> ---
>>   Version history:
>>      v1:
>>         * Initial
>>      v2:
>> ---
>> ---
>>  drivers/i2c/busses/i2c-stm32f7.c | 22 +++++++++++++++++-----
>>  1 file changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
>> index f273e28..ae0d15c 100644
>> --- a/drivers/i2c/busses/i2c-stm32f7.c
>> +++ b/drivers/i2c/busses/i2c-stm32f7.c
>> @@ -65,7 +65,12 @@
>>  #define STM32F7_I2C_CR2_NACK			BIT(15)
>>  #define STM32F7_I2C_CR2_STOP			BIT(14)
>>  #define STM32F7_I2C_CR2_START			BIT(13)
>> +#define STM32F7_I2C_CR2_HEAD10R			BIT(12)
>> +#define STM32F7_I2C_CR2_ADD10			BIT(11)
>>  #define STM32F7_I2C_CR2_RD_WRN			BIT(10)
>> +#define STM32F7_I2C_CR2_SADD10_MASK		GENMASK(9, 0)
>> +#define STM32F7_I2C_CR2_SADD10(n)		(((n) & \
>> +						STM32F7_I2C_CR2_SADD10_MASK))
>>  #define STM32F7_I2C_CR2_SADD7_MASK		GENMASK(7, 1)
>>  #define STM32F7_I2C_CR2_SADD7(n)		(((n) & 0x7f) << 1)
>>  
>> @@ -176,14 +181,14 @@ struct stm32f7_i2c_timings {
>>  
>>  /**
>>   * struct stm32f7_i2c_msg - client specific data
>> - * @addr: 8-bit slave addr, including r/w bit
>> + * @addr: 8-bit or 10-bit slave addr, including r/w bit
>>   * @count: number of bytes to be transferred
>>   * @buf: data buffer
>>   * @result: result of the transfer
>>   * @stop: last I2C msg to be sent, i.e. STOP to be generated
>>   */
>>  struct stm32f7_i2c_msg {
>> -	u8 addr;
>> +	u16 addr;
>>  	u32 count;
>>  	u8 *buf;
>>  	int result;
>> @@ -629,8 +634,15 @@ static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
>>  		cr2 |= STM32F7_I2C_CR2_RD_WRN;
>>  
>>  	/* Set slave address */
>> -	cr2 &= ~STM32F7_I2C_CR2_SADD7_MASK;
>> -	cr2 |= STM32F7_I2C_CR2_SADD7(f7_msg->addr);
>> +	cr2 &= ~(STM32F7_I2C_CR2_HEAD10R | STM32F7_I2C_CR2_ADD10);
>> +	if (msg->flags & I2C_M_TEN) {
>> +		cr2 &= ~STM32F7_I2C_CR2_SADD10_MASK;
>> +		cr2 |= STM32F7_I2C_CR2_SADD10(f7_msg->addr);
>> +		cr2 |= STM32F7_I2C_CR2_ADD10;
>> +	} else {
>> +		cr2 &= ~STM32F7_I2C_CR2_SADD7_MASK;
>> +		cr2 |= STM32F7_I2C_CR2_SADD7(f7_msg->addr);
>> +	}
>>  
>>  	/* Set nb bytes to transfer and reload if needed */
>>  	cr2 &= ~(STM32F7_I2C_CR2_NBYTES_MASK | STM32F7_I2C_CR2_RELOAD);
>> @@ -798,7 +810,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
>>  
>>  static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
>>  {
>> -	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
>> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR;
>>  }
>>  
>>  static struct i2c_algorithm stm32f7_i2c_algo = {
>> -- 
>> 2.7.4
>>

Re: [PATCH v2 1/6] i2c: i2c-stm32f7: Add 10-bit address support

[Wolfram Sang]

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


> > Out of curiosity: how did you test this patch? I never managed to find a
> > 10-bit client (except for an SoC with 10-bit slave mode).
> 
> I don't have a 10-bit device either. For testing 10-bit I'm using 2 I2C
> instances from the SoC, one in master mode and the other in slave mode.

Nice!


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

Re: [PATCH v2 3/6] i2c: i2c-stm32f7: Add initial SMBus protocols support

[Pierre Yves MORDRET]

On 03/24/2018 11:49 PM, Wolfram Sang wrote:
> On Wed, Mar 21, 2018 at 05:48:57PM +0100, Pierre-Yves MORDRET wrote:
>> This patch adds SMBus support for I2C controller embedded in STM32F7 Soc.
> 
>> All SMBus protocols are implemented except SMBus-specific protocols.
> 
> What does that mean?

It miss SMBus Host Notification and SMBBus Alert. They are almost ready but I'm
struggling to put them back to operational state after recent changes related to
SMBust Host Notification. A more "classic" interrupt base solution has been put
in place but I fail to use implement it in my side.
Another patch set is going to be delivered for these 2 commands.

> 
>>
>> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
>> ---
>>   Version history:
>>     v1:
>>        * Initial
>>     v2:
>>        * fix Kbuild test robot issue (Unneeded semicolon)
>> ---
>>
>> fixup! i2c: i2c-stm32f7: Add initial SMBus protocols support
>> ---
>>  drivers/i2c/busses/i2c-stm32f7.c | 377 ++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 368 insertions(+), 9 deletions(-)
> 
> That is quite some complexity considering we have I2C_FUNC_SMBUS_EMUL. I
> don't mind, but you really want that?
> 

All SMBBus commands are implemented as such. I never try to emulation commands.
Should we use emulation SMBus commands or real commands... Don't know.

Re: [PATCH v2 4/6] i2c: i2c-stm32: Add generic DMA API

[Pierre Yves MORDRET]

Thanks for the heads-up !
My driver supports DMA, but hardly used in the system.

But it's worth to mentioned it.

On 03/24/2018 11:51 PM, Wolfram Sang wrote:
> On Wed, Mar 21, 2018 at 05:48:58PM +0100, Pierre-Yves MORDRET wrote:
>> This patch adds a generic DMA API to implement DMA support for i2c-stm32fx
>> drivers
>>
>> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
> 
> In case you haven't read it so far, there is a new document about I2C &
> DMA -> Documentation/i2c/DMA-considerations
> 
> Just so you know...
> 

Re: [PATCH v2 6/6] i2c: i2c-stm32f7: Implement I2C recovery mechanism

[Pierre Yves MORDRET]

On 03/24/2018 11:56 PM, Wolfram Sang wrote:
> On Wed, Mar 21, 2018 at 05:49:00PM +0100, Pierre-Yves MORDRET wrote:
>> Feature prevents I2C lock-ups. Mechanism resets I2C state machine
>> and releases SCL/SDA signals but preserves I2C registers.
>>
>> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
>> ---
>>   Version history:
>>     v1:
>>        * Initial
>>     v2:
>>        * Don't use i2c engine recovery mechanism since driver
>>          procedure only recover master and not the slave.
> 
> s/recovery/release/ throughout the patch, please. Recovery is really
> something else. Also, I think the dev_info's are too noisy in the log
> files. I'd think the whole driver could be lifted from quite some
> logging...
> 

ok. will do.
Do you have example of info's that we should get rid of ?

>> ---
>> ---
>>  drivers/i2c/busses/i2c-stm32f7.c | 27 ++++++++++++++++++++++++---
>>  1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
>> index 91f73e0..9a9c469 100644
>> --- a/drivers/i2c/busses/i2c-stm32f7.c
>> +++ b/drivers/i2c/busses/i2c-stm32f7.c
>> @@ -718,6 +718,20 @@ static void stm32f7_i2c_smbus_reload(struct stm32f7_i2c_dev *i2c_dev)
>>  	writel_relaxed(cr2, i2c_dev->base + STM32F7_I2C_CR2);
>>  }
>>  
>> +static int stm32f7_i2c_recover_bus(struct i2c_adapter *i2c_adap)
>> +{
>> +	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
>> +
>> +	dev_info(i2c_dev->dev, "Trying to recover bus\n");
>> +
>> +	stm32f7_i2c_clr_bits(i2c_dev->base + STM32F7_I2C_CR1,
>> +			     STM32F7_I2C_CR1_PE);
>> +
>> +	stm32f7_i2c_hw_config(i2c_dev);
>> +
>> +	return 0;
>> +}
>> +
>>  static int stm32f7_i2c_wait_free_bus(struct stm32f7_i2c_dev *i2c_dev)
>>  {
>>  	u32 status;
>> @@ -727,12 +741,18 @@ static int stm32f7_i2c_wait_free_bus(struct stm32f7_i2c_dev *i2c_dev)
>>  					 status,
>>  					 !(status & STM32F7_I2C_ISR_BUSY),
>>  					 10, 1000);
>> +	if (!ret)
>> +		return 0;
>> +
>> +	dev_info(i2c_dev->dev, "bus busy\n");
>> +
>> +	ret = stm32f7_i2c_recover_bus(&i2c_dev->adap);
>>  	if (ret) {
>> -		dev_dbg(i2c_dev->dev, "bus busy\n");
>> -		ret = -EBUSY;
>> +		dev_err(i2c_dev->dev, "Failed to recover the bus (%d)\n", ret);
>> +		return ret;
>>  	}
>>  
>> -	return ret;
>> +	return -EBUSY;
>>  }
>>  
>>  static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
>> @@ -1474,6 +1494,7 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
>>  	if (status & STM32F7_I2C_ISR_BERR) {
>>  		dev_err(dev, "<%s>: Bus error\n", __func__);
>>  		writel_relaxed(STM32F7_I2C_ICR_BERRCF, base + STM32F7_I2C_ICR);
>> +		stm32f7_i2c_recover_bus(&i2c_dev->adap);
>>  		f7_msg->result = -EIO;
>>  	}
>>  
>> -- 
>> 2.7.4
>>

Re: [PATCH v2 2/6] i2c: i2c-stm32f7: Add slave support

[Pierre Yves MORDRET]

On 03/25/2018 08:16 PM, Wolfram Sang wrote:
> On Wed, Mar 21, 2018 at 05:48:56PM +0100, Pierre-Yves MORDRET wrote:
>> This patch adds slave support for I2C controller embedded in STM32F7 SoC
>>
>> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
> 
> Looks OK from a first look. What kind of tests did you do?

As mentioned for 10-bit, I'm using 2 I2C instances from the SoC.
Here are the tests I dit:
 - Master/slave send in 7 and 10 bits
 - master/slave recv in 7 and 10 bits
 - E2PROM Read/Write



> 
> 
>> ---
>>   Version history:
>>      v1:
>>         * Initial
>>      v2:
>> ---
>> ---
>>  drivers/i2c/busses/Kconfig       |   1 +
>>  drivers/i2c/busses/i2c-stm32f7.c | 434 ++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 429 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index e2954fb..118b9be 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -964,6 +964,7 @@ config I2C_STM32F4
>>  config I2C_STM32F7
>>  	tristate "STMicroelectronics STM32F7 I2C support"
>>  	depends on ARCH_STM32 || COMPILE_TEST
>> +	select I2C_SLAVE
>>  	help
>>  	  Enable this option to add support for STM32 I2C controller embedded
>>  	  in STM32F7 SoCs.
>> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
>> index ae0d15c..9bf676e 100644
>> --- a/drivers/i2c/busses/i2c-stm32f7.c
>> +++ b/drivers/i2c/busses/i2c-stm32f7.c
>> @@ -35,6 +35,8 @@
>>  /* STM32F7 I2C registers */
>>  #define STM32F7_I2C_CR1				0x00
>>  #define STM32F7_I2C_CR2				0x04
>> +#define STM32F7_I2C_OAR1			0x08
>> +#define STM32F7_I2C_OAR2			0x0C
>>  #define STM32F7_I2C_TIMINGR			0x10
>>  #define STM32F7_I2C_ISR				0x18
>>  #define STM32F7_I2C_ICR				0x1C
>> @@ -42,6 +44,7 @@
>>  #define STM32F7_I2C_TXDR			0x28
>>  
>>  /* STM32F7 I2C control 1 */
>> +#define STM32F7_I2C_CR1_SBC			BIT(16)
>>  #define STM32F7_I2C_CR1_ANFOFF			BIT(12)
>>  #define STM32F7_I2C_CR1_ERRIE			BIT(7)
>>  #define STM32F7_I2C_CR1_TCIE			BIT(6)
>> @@ -57,6 +60,11 @@
>>  						| STM32F7_I2C_CR1_NACKIE \
>>  						| STM32F7_I2C_CR1_RXIE \
>>  						| STM32F7_I2C_CR1_TXIE)
>> +#define STM32F7_I2C_XFER_IRQ_MASK		(STM32F7_I2C_CR1_TCIE \
>> +						| STM32F7_I2C_CR1_STOPIE \
>> +						| STM32F7_I2C_CR1_NACKIE \
>> +						| STM32F7_I2C_CR1_RXIE \
>> +						| STM32F7_I2C_CR1_TXIE)
>>  
>>  /* STM32F7 I2C control 2 */
>>  #define STM32F7_I2C_CR2_RELOAD			BIT(24)
>> @@ -74,7 +82,34 @@
>>  #define STM32F7_I2C_CR2_SADD7_MASK		GENMASK(7, 1)
>>  #define STM32F7_I2C_CR2_SADD7(n)		(((n) & 0x7f) << 1)
>>  
>> +/* STM32F7 I2C Own Address 1 */
>> +#define STM32F7_I2C_OAR1_OA1EN			BIT(15)
>> +#define STM32F7_I2C_OAR1_OA1MODE		BIT(10)
>> +#define STM32F7_I2C_OAR1_OA1_10_MASK		GENMASK(9, 0)
>> +#define STM32F7_I2C_OAR1_OA1_10(n)		(((n) & \
>> +						STM32F7_I2C_OAR1_OA1_10_MASK))
>> +#define STM32F7_I2C_OAR1_OA1_7_MASK		GENMASK(7, 1)
>> +#define STM32F7_I2C_OAR1_OA1_7(n)		(((n) & 0x7f) << 1)
>> +#define STM32F7_I2C_OAR1_MASK			(STM32F7_I2C_OAR1_OA1_7_MASK \
>> +						| STM32F7_I2C_OAR1_OA1_10_MASK \
>> +						| STM32F7_I2C_OAR1_OA1EN \
>> +						| STM32F7_I2C_OAR1_OA1MODE)
>> +
>> +/* STM32F7 I2C Own Address 2 */
>> +#define STM32F7_I2C_OAR2_OA2EN			BIT(15)
>> +#define STM32F7_I2C_OAR2_OA2MSK_MASK		GENMASK(10, 8)
>> +#define STM32F7_I2C_OAR2_OA2MSK(n)		(((n) & 0x7) << 8)
>> +#define STM32F7_I2C_OAR2_OA2_7_MASK		GENMASK(7, 1)
>> +#define STM32F7_I2C_OAR2_OA2_7(n)		(((n) & 0x7f) << 1)
>> +#define STM32F7_I2C_OAR2_MASK			(STM32F7_I2C_OAR2_OA2MSK_MASK \
>> +						| STM32F7_I2C_OAR2_OA2_7_MASK \
>> +						| STM32F7_I2C_OAR2_OA2EN)
>> +
>>  /* STM32F7 I2C Interrupt Status */
>> +#define STM32F7_I2C_ISR_ADDCODE_MASK		GENMASK(23, 17)
>> +#define STM32F7_I2C_ISR_ADDCODE_GET(n) \
>> +				(((n) & STM32F7_I2C_ISR_ADDCODE_MASK) >> 17)
>> +#define STM32F7_I2C_ISR_DIR			BIT(16)
>>  #define STM32F7_I2C_ISR_BUSY			BIT(15)
>>  #define STM32F7_I2C_ISR_ARLO			BIT(9)
>>  #define STM32F7_I2C_ISR_BERR			BIT(8)
>> @@ -82,14 +117,17 @@
>>  #define STM32F7_I2C_ISR_TC			BIT(6)
>>  #define STM32F7_I2C_ISR_STOPF			BIT(5)
>>  #define STM32F7_I2C_ISR_NACKF			BIT(4)
>> +#define STM32F7_I2C_ISR_ADDR			BIT(3)
>>  #define STM32F7_I2C_ISR_RXNE			BIT(2)
>>  #define STM32F7_I2C_ISR_TXIS			BIT(1)
>> +#define STM32F7_I2C_ISR_TXE			BIT(0)
>>  
>>  /* STM32F7 I2C Interrupt Clear */
>>  #define STM32F7_I2C_ICR_ARLOCF			BIT(9)
>>  #define STM32F7_I2C_ICR_BERRCF			BIT(8)
>>  #define STM32F7_I2C_ICR_STOPCF			BIT(5)
>>  #define STM32F7_I2C_ICR_NACKCF			BIT(4)
>> +#define STM32F7_I2C_ICR_ADDRCF			BIT(3)
>>  
>>  /* STM32F7 I2C Timing */
>>  #define STM32F7_I2C_TIMINGR_PRESC(n)		(((n) & 0xf) << 28)
>> @@ -99,6 +137,7 @@
>>  #define STM32F7_I2C_TIMINGR_SCLL(n)		((n) & 0xff)
>>  
>>  #define STM32F7_I2C_MAX_LEN			0xff
>> +#define STM32F7_I2C_MAX_SLAVE			0x2
>>  
>>  #define STM32F7_I2C_DNF_DEFAULT			0
>>  #define STM32F7_I2C_DNF_MAX			16
>> @@ -209,6 +248,11 @@ struct stm32f7_i2c_msg {
>>   * @f7_msg: customized i2c msg for driver usage
>>   * @setup: I2C timing input setup
>>   * @timing: I2C computed timings
>> + * @slave: list of slave devices registered on the I2C bus
>> + * @slave_running: slave device currently used
>> + * @slave_dir: transfer direction for the current slave device
>> + * @master_mode: boolean to know in which mode the I2C is running (master or
>> + * slave)
>>   */
>>  struct stm32f7_i2c_dev {
>>  	struct i2c_adapter adap;
>> @@ -223,6 +267,10 @@ struct stm32f7_i2c_dev {
>>  	struct stm32f7_i2c_msg f7_msg;
>>  	struct stm32f7_i2c_setup setup;
>>  	struct stm32f7_i2c_timings timing;
>> +	struct i2c_client *slave[STM32F7_I2C_MAX_SLAVE];
>> +	struct i2c_client *slave_running;
>> +	u32 slave_dir;
>> +	bool master_mode;
>>  };
>>  
>>  /**
>> @@ -288,6 +336,11 @@ static inline void stm32f7_i2c_clr_bits(void __iomem *reg, u32 mask)
>>  	writel_relaxed(readl_relaxed(reg) & ~mask, reg);
>>  }
>>  
>> +static void stm32f7_i2c_disable_irq(struct stm32f7_i2c_dev *i2c_dev, u32 mask)
>> +{
>> +	stm32f7_i2c_clr_bits(i2c_dev->base + STM32F7_I2C_CR1, mask);
>> +}
>> +
>>  static int stm32f7_i2c_compute_timing(struct stm32f7_i2c_dev *i2c_dev,
>>  				      struct stm32f7_i2c_setup *setup,
>>  				      struct stm32f7_i2c_timings *output)
>> @@ -572,6 +625,9 @@ static void stm32f7_i2c_read_rx_data(struct stm32f7_i2c_dev *i2c_dev)
>>  	if (f7_msg->count) {
>>  		*f7_msg->buf++ = readb_relaxed(base + STM32F7_I2C_RXDR);
>>  		f7_msg->count--;
>> +	} else {
>> +		/* Flush RX buffer has no data is expected */
>> +		readb_relaxed(base + STM32F7_I2C_RXDR);
>>  	}
>>  }
>>  
>> @@ -669,14 +725,250 @@ static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
>>  	/* Configure Start/Repeated Start */
>>  	cr2 |= STM32F7_I2C_CR2_START;
>>  
>> +	i2c_dev->master_mode = true;
>> +
>>  	/* Write configurations registers */
>>  	writel_relaxed(cr1, base + STM32F7_I2C_CR1);
>>  	writel_relaxed(cr2, base + STM32F7_I2C_CR2);
>>  }
>>  
>> -static void stm32f7_i2c_disable_irq(struct stm32f7_i2c_dev *i2c_dev, u32 mask)
>> +static bool stm32f7_i2c_is_addr_match(struct i2c_client *slave, u32 addcode)
>>  {
>> -	stm32f7_i2c_clr_bits(i2c_dev->base + STM32F7_I2C_CR1, mask);
>> +	u32 addr;
>> +
>> +	if (!slave)
>> +		return false;
>> +
>> +	if (slave->flags & I2C_CLIENT_TEN) {
>> +		/*
>> +		 * For 10-bit addr, addcode = 11110XY with
>> +		 * X = Bit 9 of slave address
>> +		 * Y = Bit 8 of slave address
>> +		 */
>> +		addr = slave->addr >> 8;
>> +		addr |= 0x78;
>> +		if (addr == addcode)
>> +			return true;
>> +	} else {
>> +		addr = slave->addr & 0x7f;
>> +		if (addr == addcode)
>> +			return true;
>> +	}
>> +
>> +	return false;
>> +}
>> +
>> +static void stm32f7_i2c_slave_start(struct stm32f7_i2c_dev *i2c_dev)
>> +{
>> +	struct i2c_client *slave = i2c_dev->slave_running;
>> +	void __iomem *base = i2c_dev->base;
>> +	u32 mask;
>> +	u8 value = 0;
>> +
>> +	if (i2c_dev->slave_dir) {
>> +		/* Notify i2c slave that new read transfer is starting */
>> +		i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
>> +
>> +		/*
>> +		 * Disable slave TX config in case of I2C combined message
>> +		 * (I2C Write followed by I2C Read)
>> +		 */
>> +		mask = STM32F7_I2C_CR2_RELOAD;
>> +		stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR2, mask);
>> +		mask = STM32F7_I2C_CR1_SBC | STM32F7_I2C_CR1_RXIE |
>> +		       STM32F7_I2C_CR1_TCIE;
>> +		stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR1, mask);
>> +
>> +		/* Enable TX empty, STOP, NACK interrupts */
>> +		mask =  STM32F7_I2C_CR1_STOPIE | STM32F7_I2C_CR1_NACKIE |
>> +			STM32F7_I2C_CR1_TXIE;
>> +		stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, mask);
>> +
>> +	} else {
>> +		/* Notify i2c slave that new write transfer is starting */
>> +		i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
>> +
>> +		/* Set reload mode to be able to ACK/NACK each received byte */
>> +		mask = STM32F7_I2C_CR2_RELOAD;
>> +		stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
>> +
>> +		/*
>> +		 * Set STOP, NACK, RX empty and transfer complete interrupts.*
>> +		 * Set Slave Byte Control to be able to ACK/NACK each data
>> +		 * byte received
>> +		 */
>> +		mask =  STM32F7_I2C_CR1_STOPIE | STM32F7_I2C_CR1_NACKIE |
>> +			STM32F7_I2C_CR1_SBC | STM32F7_I2C_CR1_RXIE |
>> +			STM32F7_I2C_CR1_TCIE;
>> +		stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, mask);
>> +	}
>> +}
>> +
>> +static void stm32f7_i2c_slave_addr(struct stm32f7_i2c_dev *i2c_dev)
>> +{
>> +	void __iomem *base = i2c_dev->base;
>> +	u32 isr, addcode, dir, mask;
>> +	int i;
>> +
>> +	isr = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
>> +	addcode = STM32F7_I2C_ISR_ADDCODE_GET(isr);
>> +	dir = isr & STM32F7_I2C_ISR_DIR;
>> +
>> +	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
>> +		if (stm32f7_i2c_is_addr_match(i2c_dev->slave[i], addcode)) {
>> +			i2c_dev->slave_running = i2c_dev->slave[i];
>> +			i2c_dev->slave_dir = dir;
>> +
>> +			/* Start I2C slave processing */
>> +			stm32f7_i2c_slave_start(i2c_dev);
>> +
>> +			/* Clear ADDR flag */
>> +			mask = STM32F7_I2C_ICR_ADDRCF;
>> +			writel_relaxed(mask, base + STM32F7_I2C_ICR);
>> +			break;
>> +		}
>> +	}
>> +}
>> +
>> +static int stm32f7_i2c_get_slave_id(struct stm32f7_i2c_dev *i2c_dev,
>> +				    struct i2c_client *slave, int *id)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
>> +		if (i2c_dev->slave[i] == slave) {
>> +			*id = i;
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	dev_err(i2c_dev->dev, "Slave 0x%x not registered\n", slave->addr);
>> +
>> +	return -ENODEV;
>> +}
>> +
>> +static int stm32f7_i2c_get_free_slave_id(struct stm32f7_i2c_dev *i2c_dev,
>> +					 struct i2c_client *slave, int *id)
>> +{
>> +	struct device *dev = i2c_dev->dev;
>> +	int i;
>> +
>> +	/*
>> +	 * slave[0] supports 7-bit and 10-bit slave address
>> +	 * slave[1] supports 7-bit slave address only
>> +	 */
>> +	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
>> +		if (i == 1 && (slave->flags & I2C_CLIENT_PEC))
>> +			continue;
>> +		if (!i2c_dev->slave[i]) {
>> +			*id = i;
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	dev_err(dev, "Slave 0x%x could not be registered\n", slave->addr);
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static bool stm32f7_i2c_is_slave_registered(struct stm32f7_i2c_dev *i2c_dev)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
>> +		if (i2c_dev->slave[i])
>> +			return true;
>> +	}
>> +
>> +	return false;
>> +}
>> +
>> +static bool stm32f7_i2c_is_slave_busy(struct stm32f7_i2c_dev *i2c_dev)
>> +{
>> +	int i, busy;
>> +
>> +	busy = 0;
>> +	for (i = 0; i < STM32F7_I2C_MAX_SLAVE; i++) {
>> +		if (i2c_dev->slave[i])
>> +			busy++;
>> +	}
>> +
>> +	return i == busy;
>> +}
>> +
>> +static irqreturn_t stm32f7_i2c_slave_isr_event(struct stm32f7_i2c_dev *i2c_dev)
>> +{
>> +	void __iomem *base = i2c_dev->base;
>> +	u32 cr2, status, mask;
>> +	u8 val;
>> +	int ret;
>> +
>> +	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
>> +
>> +	/* Slave transmitter mode */
>> +	if (status & STM32F7_I2C_ISR_TXIS) {
>> +		i2c_slave_event(i2c_dev->slave_running,
>> +				I2C_SLAVE_READ_PROCESSED,
>> +				&val);
>> +
>> +		/* Write data byte */
>> +		writel_relaxed(val, base + STM32F7_I2C_TXDR);
>> +	}
>> +
>> +	/* Transfer Complete Reload for Slave receiver mode */
>> +	if (status & STM32F7_I2C_ISR_TCR || status & STM32F7_I2C_ISR_RXNE) {
>> +		/*
>> +		 * Read data byte then set NBYTES to receive next byte or NACK
>> +		 * the current received byte
>> +		 */
>> +		val = readb_relaxed(i2c_dev->base + STM32F7_I2C_RXDR);
>> +		ret = i2c_slave_event(i2c_dev->slave_running,
>> +				      I2C_SLAVE_WRITE_RECEIVED,
>> +				      &val);
>> +		if (!ret) {
>> +			cr2 = readl_relaxed(i2c_dev->base + STM32F7_I2C_CR2);
>> +			cr2 |= STM32F7_I2C_CR2_NBYTES(1);
>> +			writel_relaxed(cr2, i2c_dev->base + STM32F7_I2C_CR2);
>> +		} else {
>> +			mask = STM32F7_I2C_CR2_NACK;
>> +			stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
>> +		}
>> +	}
>> +
>> +	/* NACK received */
>> +	if (status & STM32F7_I2C_ISR_NACKF) {
>> +		dev_dbg(i2c_dev->dev, "<%s>: Receive NACK\n", __func__);
>> +		writel_relaxed(STM32F7_I2C_ICR_NACKCF, base + STM32F7_I2C_ICR);
>> +	}
>> +
>> +	/* STOP received */
>> +	if (status & STM32F7_I2C_ISR_STOPF) {
>> +		/* Disable interrupts */
>> +		stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_XFER_IRQ_MASK);
>> +
>> +		if (i2c_dev->slave_dir) {
>> +			/*
>> +			 * Flush TX buffer in order to not used the byte in
>> +			 * TXDR for the next transfer
>> +			 */
>> +			mask = STM32F7_I2C_ISR_TXE;
>> +			stm32f7_i2c_set_bits(base + STM32F7_I2C_ISR, mask);
>> +		}
>> +
>> +		/* Clear STOP flag */
>> +		writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
>> +
>> +		/* Notify i2c slave that a STOP flag has been detected */
>> +		i2c_slave_event(i2c_dev->slave_running, I2C_SLAVE_STOP, &val);
>> +
>> +		i2c_dev->slave_running = NULL;
>> +	}
>> +
>> +	/* Address match received */
>> +	if (status & STM32F7_I2C_ISR_ADDR)
>> +		stm32f7_i2c_slave_addr(i2c_dev);
>> +
>> +	return IRQ_HANDLED;
>>  }
>>  
>>  static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
>> @@ -685,6 +977,13 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
>>  	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
>>  	void __iomem *base = i2c_dev->base;
>>  	u32 status, mask;
>> +	int ret;
>> +
>> +	/* Check if the interrupt if for a slave device */
>> +	if (!i2c_dev->master_mode) {
>> +		ret = stm32f7_i2c_slave_isr_event(i2c_dev);
>> +		return ret;
>> +	}
>>  
>>  	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
>>  
>> @@ -706,11 +1005,16 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
>>  	/* STOP detection flag */
>>  	if (status & STM32F7_I2C_ISR_STOPF) {
>>  		/* Disable interrupts */
>> -		stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
>> +		if (stm32f7_i2c_is_slave_registered(i2c_dev))
>> +			mask = STM32F7_I2C_XFER_IRQ_MASK;
>> +		else
>> +			mask = STM32F7_I2C_ALL_IRQ_MASK;
>> +		stm32f7_i2c_disable_irq(i2c_dev, mask);
>>  
>>  		/* Clear STOP flag */
>>  		writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
>>  
>> +		i2c_dev->master_mode = false;
>>  		complete(&i2c_dev->complete);
>>  	}
>>  
>> @@ -743,7 +1047,7 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
>>  	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
>>  	void __iomem *base = i2c_dev->base;
>>  	struct device *dev = i2c_dev->dev;
>> -	u32 status;
>> +	u32 mask, status;
>>  
>>  	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
>>  
>> @@ -761,8 +1065,14 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
>>  		f7_msg->result = -EAGAIN;
>>  	}
>>  
>> -	stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
>> +	/* Disable interrupts */
>> +	if (stm32f7_i2c_is_slave_registered(i2c_dev))
>> +		mask = STM32F7_I2C_XFER_IRQ_MASK;
>> +	else
>> +		mask = STM32F7_I2C_ALL_IRQ_MASK;
>> +	stm32f7_i2c_disable_irq(i2c_dev, mask);
>>  
>> +	i2c_dev->master_mode = false;
>>  	complete(&i2c_dev->complete);
>>  
>>  	return IRQ_HANDLED;
>> @@ -808,14 +1118,126 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
>>  	return (ret < 0) ? ret : num;
>>  }
>>  
>> +static int stm32f7_i2c_reg_slave(struct i2c_client *slave)
>> +{
>> +	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(slave->adapter);
>> +	void __iomem *base = i2c_dev->base;
>> +	struct device *dev = i2c_dev->dev;
>> +	u32 oar1, oar2, mask;
>> +	int id, ret;
>> +
>> +	if (slave->flags & I2C_CLIENT_PEC) {
>> +		dev_err(dev, "SMBus PEC not supported in slave mode\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (stm32f7_i2c_is_slave_busy(i2c_dev)) {
>> +		dev_err(dev, "Too much slave registered\n");
>> +		return -EBUSY;
>> +	}
>> +
>> +	ret = stm32f7_i2c_get_free_slave_id(i2c_dev, slave, &id);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (!(stm32f7_i2c_is_slave_registered(i2c_dev))) {
>> +		ret = clk_enable(i2c_dev->clk);
>> +		if (ret) {
>> +			dev_err(dev, "Failed to enable clock\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	if (id == 0) {
>> +		/* Configure Own Address 1 */
>> +		oar1 = readl_relaxed(i2c_dev->base + STM32F7_I2C_OAR1);
>> +		oar1 &= ~STM32F7_I2C_OAR1_MASK;
>> +		if (slave->flags & I2C_CLIENT_TEN) {
>> +			oar1 |= STM32F7_I2C_OAR1_OA1_10(slave->addr);
>> +			oar1 |= STM32F7_I2C_OAR1_OA1MODE;
>> +		} else {
>> +			oar1 |= STM32F7_I2C_OAR1_OA1_7(slave->addr);
>> +		}
>> +		oar1 |= STM32F7_I2C_OAR1_OA1EN;
>> +		i2c_dev->slave[id] = slave;
>> +		writel_relaxed(oar1, i2c_dev->base + STM32F7_I2C_OAR1);
>> +	} else if (id == 1) {
>> +		/* Configure Own Address 2 */
>> +		oar2 = readl_relaxed(i2c_dev->base + STM32F7_I2C_OAR2);
>> +		oar2 &= ~STM32F7_I2C_OAR2_MASK;
>> +		if (slave->flags & I2C_CLIENT_TEN) {
>> +			ret = -EOPNOTSUPP;
>> +			goto exit;
>> +		}
>> +
>> +		oar2 |= STM32F7_I2C_OAR2_OA2_7(slave->addr);
>> +		oar2 |= STM32F7_I2C_OAR2_OA2EN;
>> +		i2c_dev->slave[id] = slave;
>> +		writel_relaxed(oar2, i2c_dev->base + STM32F7_I2C_OAR2);
>> +	} else {
>> +		ret = -ENODEV;
>> +		goto exit;
>> +	}
>> +
>> +	/* Enable ACK */
>> +	stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR2, STM32F7_I2C_CR2_NACK);
>> +
>> +	/* Enable Address match interrupt, error interrupt and enable I2C  */
>> +	mask = STM32F7_I2C_CR1_ADDRIE | STM32F7_I2C_CR1_ERRIE |
>> +		STM32F7_I2C_CR1_PE;
>> +	stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, mask);
>> +
>> +	return 0;
>> +
>> +exit:
>> +	if (!(stm32f7_i2c_is_slave_registered(i2c_dev)))
>> +		clk_disable(i2c_dev->clk);
>> +
>> +	return ret;
>> +}
>> +
>> +static int stm32f7_i2c_unreg_slave(struct i2c_client *slave)
>> +{
>> +	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(slave->adapter);
>> +	void __iomem *base = i2c_dev->base;
>> +	u32 mask;
>> +	int id, ret;
>> +
>> +	ret = stm32f7_i2c_get_slave_id(i2c_dev, slave, &id);
>> +	if (ret)
>> +		return ret;
>> +
>> +	WARN_ON(!i2c_dev->slave[id]);
>> +
>> +	if (id == 0) {
>> +		mask = STM32F7_I2C_OAR1_OA1EN;
>> +		stm32f7_i2c_clr_bits(base + STM32F7_I2C_OAR1, mask);
>> +	} else {
>> +		mask = STM32F7_I2C_OAR2_OA2EN;
>> +		stm32f7_i2c_clr_bits(base + STM32F7_I2C_OAR2, mask);
>> +	}
>> +
>> +	i2c_dev->slave[id] = NULL;
>> +
>> +	if (!(stm32f7_i2c_is_slave_registered(i2c_dev))) {
>> +		stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
>> +		clk_disable(i2c_dev->clk);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
>>  {
>> -	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR;
>> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR |
>> +		I2C_FUNC_SLAVE;
>>  }
>>  
>>  static struct i2c_algorithm stm32f7_i2c_algo = {
>>  	.master_xfer = stm32f7_i2c_xfer,
>>  	.functionality = stm32f7_i2c_func,
>> +	.reg_slave = stm32f7_i2c_reg_slave,
>> +	.unreg_slave = stm32f7_i2c_unreg_slave,
>>  };
>>  
>>  static int stm32f7_i2c_probe(struct platform_device *pdev)
>> -- 
>> 2.7.4
>>

Re: [PATCH v2 2/6] i2c: i2c-stm32f7: Add slave support

[Wolfram Sang]

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

On Mon, Mar 26, 2018 at 10:41:51AM +0200, Pierre Yves MORDRET wrote:
> 
> 
> On 03/25/2018 08:16 PM, Wolfram Sang wrote:
> > On Wed, Mar 21, 2018 at 05:48:56PM +0100, Pierre-Yves MORDRET wrote:
> >> This patch adds slave support for I2C controller embedded in STM32F7 SoC
> >>
> >> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> >> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
> > 
> > Looks OK from a first look. What kind of tests did you do?
> 
> As mentioned for 10-bit, I'm using 2 I2C instances from the SoC.
> Here are the tests I dit:
>  - Master/slave send in 7 and 10 bits
>  - master/slave recv in 7 and 10 bits
>  - E2PROM Read/Write

As far as I understand the code now, both instances can be master /
slave simultanously on the same bus?


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

Re: [PATCH v2 3/6] i2c: i2c-stm32f7: Add initial SMBus protocols support

[Wolfram Sang]

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


> >> All SMBus protocols are implemented except SMBus-specific protocols.
> > 
> > What does that mean?
> 
> It miss SMBus Host Notification and SMBBus Alert. They are almost ready but I'm
> struggling to put them back to operational state after recent changes related to
> SMBust Host Notification. A more "classic" interrupt base solution has been put
> in place but I fail to use implement it in my side.
> Another patch set is going to be delivered for these 2 commands.

This is totally fine to implement it incrementally. Please just update the
commit message with the more detailed explanation above.

> > That is quite some complexity considering we have I2C_FUNC_SMBUS_EMUL. I
> > don't mind, but you really want that?
> > 
> 
> All SMBBus commands are implemented as such. I never try to emulation commands.
> Should we use emulation SMBus commands or real commands... Don't know.

You won't see any difference on the wire. I don't know your HW. It might
be that SMBus mode is more "automatic" and uses less interrupts. Or
stuff like Alert or HostNotification only works in this mode. If you and
the driver maintainers think it is worth the added complexity, I am
fine, too.


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

Re: [PATCH v2 5/6] i2c: i2c-stm32f7: Add DMA support

[Wolfram Sang]

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


> +#define STM32F7_I2C_DMA_LEN_MIN			0x1
...

> +	if (i2c_dev->dma && f7_msg->count >= STM32F7_I2C_DMA_LEN_MIN) {

Are you using DMA for every message with a length >= 1? The setup of
that might be more expensive than the DMA gain, if so.


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

Re: [PATCH v2 2/6] i2c: i2c-stm32f7: Add slave support

[Pierre Yves MORDRET]

On 04/03/2018 05:26 PM, Wolfram Sang wrote:
> On Mon, Mar 26, 2018 at 10:41:51AM +0200, Pierre Yves MORDRET wrote:
>>
>>
>> On 03/25/2018 08:16 PM, Wolfram Sang wrote:
>>> On Wed, Mar 21, 2018 at 05:48:56PM +0100, Pierre-Yves MORDRET wrote:
>>>> This patch adds slave support for I2C controller embedded in STM32F7 SoC
>>>>
>>>> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>>>> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
>>>
>>> Looks OK from a first look. What kind of tests did you do?
>>
>> As mentioned for 10-bit, I'm using 2 I2C instances from the SoC.
>> Here are the tests I dit:
>>  - Master/slave send in 7 and 10 bits
>>  - master/slave recv in 7 and 10 bits
>>  - E2PROM Read/Write
> 
> As far as I understand the code now, both instances can be master /
> slave simultanously on the same bus?
> 

Correct.

Re: [PATCH v2 3/6] i2c: i2c-stm32f7: Add initial SMBus protocols support

[Pierre Yves MORDRET]

On 04/03/2018 05:31 PM, Wolfram Sang wrote:
> 
>>>> All SMBus protocols are implemented except SMBus-specific protocols.
>>>
>>> What does that mean?
>>
>> It miss SMBus Host Notification and SMBBus Alert. They are almost ready but I'm
>> struggling to put them back to operational state after recent changes related to
>> SMBust Host Notification. A more "classic" interrupt base solution has been put
>> in place but I fail to use implement it in my side.
>> Another patch set is going to be delivered for these 2 commands.
> 
> This is totally fine to implement it incrementally. Please just update the
> commit message with the more detailed explanation above.
> 

Ok. I get.

>>> That is quite some complexity considering we have I2C_FUNC_SMBUS_EMUL. I
>>> don't mind, but you really want that?
>>>
>>
>> All SMBBus commands are implemented as such. I never try to emulation commands.
>> Should we use emulation SMBus commands or real commands... Don't know.
> 
> You won't see any difference on the wire. I don't know your HW. It might
> be that SMBus mode is more "automatic" and uses less interrupts. Or
> stuff like Alert or HostNotification only works in this mode. If you and
> the driver maintainers think it is worth the added complexity, I am
> fine, too.
> 
Ok. I see.
I am the maintainer. So yes I keep it as such ... with this complexity ;)

Re: [PATCH v2 5/6] i2c: i2c-stm32f7: Add DMA support

[Pierre Yves MORDRET]

On 04/03/2018 05:39 PM, Wolfram Sang wrote:
> 
>> +#define STM32F7_I2C_DMA_LEN_MIN			0x1
> ...
> 
>> +	if (i2c_dev->dma && f7_msg->count >= STM32F7_I2C_DMA_LEN_MIN) {
> 
> Are you using DMA for every message with a length >= 1? The setup of
> that might be more expensive than the DMA gain, if so.
> 
Well yes. I am in charge of DMA IPs as well. I2C is the only devices that I had
to test DMA outside standard DMA Engine test. Quite convenient.
I believe to stress both I2C and DMA, this value was relevant.
Now I agree this value can be tuned a little bit. I might raise this threshold
in V3.

Re: [PATCH v2 3/6] i2c: i2c-stm32f7: Add initial SMBus protocols support

[Pierre Yves MORDRET]

On 04/03/2018 05:31 PM, Wolfram Sang wrote:
> 
>>>> All SMBus protocols are implemented except SMBus-specific protocols.
>>>
>>> What does that mean?
>>
>> It miss SMBus Host Notification and SMBBus Alert. They are almost ready but I'm
>> struggling to put them back to operational state after recent changes related to
>> SMBust Host Notification. A more "classic" interrupt base solution has been put
>> in place but I fail to use implement it in my side.
>> Another patch set is going to be delivered for these 2 commands.
> 
> This is totally fine to implement it incrementally. Please just update the
> commit message with the more detailed explanation above.
> 

Ok. I get.

>>> That is quite some complexity considering we have I2C_FUNC_SMBUS_EMUL. I
>>> don't mind, but you really want that?
>>>
>>
>> All SMBBus commands are implemented as such. I never try to emulation commands.
>> Should we use emulation SMBus commands or real commands... Don't know.
> 
> You won't see any difference on the wire. I don't know your HW. It might
> be that SMBus mode is more "automatic" and uses less interrupts. Or
> stuff like Alert or HostNotification only works in this mode. If you and
> the driver maintainers think it is worth the added complexity, I am
> fine, too.
> 
Ok. I see.
I am the maintainer. So yes I keep it as such ... with this complexity ;)

Re: [PATCH v2 2/6] i2c: i2c-stm32f7: Add slave support

[Wolfram Sang]

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


> > As far as I understand the code now, both instances can be master /
> > slave simultanously on the same bus?
> 
> Correct.

Very nice! :)


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

Re: [PATCH v2 3/6] i2c: i2c-stm32f7: Add initial SMBus protocols support

[Wolfram Sang]

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


> I am the maintainer. So yes I keep it as such ... with this complexity ;)

BTW you are not listed as such. Can you send an incremental patch for
MAINTAINERS?


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