[PATCH 0/5] Add support for the STM32F7 I2C

[PATCH 0/5] Add support for the STM32F7 I2C

[M'boumba Cedric Madianga]

This patchset adds support for the I2C controller embedded in STM32F7xx SoC.
It enables I2C transfer in interrupt mode with Standard-mode, Fast-mode and
Fast-mode+ bus speed.

M'boumba Cedric Madianga (5):
  dt-bindings: i2c-stm32: Document the STM32F7 I2C bindings
  i2c: i2c-stm32f4: use generic definition of speed enum
  i2c: i2c-stm32f7: add driver
  ARM: dts: stm32: Add I2C1 support for STM32F746 SoC
  ARM: dts: stm32: Add I2C1 support for STM32F746 eval board

 .../devicetree/bindings/i2c/i2c-stm32.txt          |  22 +-
 arch/arm/boot/dts/stm32746g-eval.dts               |   6 +
 arch/arm/boot/dts/stm32f746.dtsi                   |  23 +
 drivers/i2c/busses/Kconfig                         |  10 +
 drivers/i2c/busses/Makefile                        |   1 +
 drivers/i2c/busses/i2c-stm32.h                     |  20 +
 drivers/i2c/busses/i2c-stm32f4.c                   |  18 +-
 drivers/i2c/busses/i2c-stm32f7.c                   | 562 +++++++++++++++++++++
 8 files changed, 648 insertions(+), 14 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-stm32.h
 create mode 100644 drivers/i2c/busses/i2c-stm32f7.c

-- 
1.9.1

[PATCH 1/5] dt-bindings: i2c-stm32: Document the STM32F7 I2C bindings

[M'boumba Cedric Madianga]

This patch adds the documentation of device tree bindings for STM32F7 I2C

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
---
 .../devicetree/bindings/i2c/i2c-stm32.txt          | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-stm32.txt b/Documentation/devicetree/bindings/i2c/i2c-stm32.txt
index 78eaf7b..8288724 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-stm32.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-stm32.txt
@@ -1,7 +1,9 @@
 * I2C controller embedded in STMicroelectronics STM32 I2C platform
 
 Required properties :
-- compatible : Must be "st,stm32f4-i2c"
+- compatible : Must be one of the following
+  - "st,stm32f4-i2c"
+  - "st,stm32f7-i2c"
 - reg : Offset and length of the register set for the device
 - interrupts : Must contain the interrupt id for I2C event and then the
   interrupt id for I2C error.
@@ -14,8 +16,22 @@ Required properties :
 
 Optional properties :
 - clock-frequency : Desired I2C bus clock frequency in Hz. If not specified,
-  the default 100 kHz frequency will be used. As only Normal and Fast modes
-  are supported, possible values are 100000 and 400000.
+  the default 100 kHz frequency will be used.
+  For STM32F4 SoC Standard-mode and Fast-mode are supported, possible values are
+  100000 and 400000.
+  For STM32F7 SoC, Standard-mode, Fast-mode and Fast-mode Plus are supported,
+  possible values are 100000, 400000 and 1000000.
+- st,i2c-timing : A 32-bit I2C timing refister value.
+  This value is only required for "st,stm32f7-i2c".
+  A specific external tool delivered by ST is used to compute the value to be
+  set here according to i2C speed frequency, i2c clock source frequency,
+  i2c rise time and i2c fall time inputs.
+  - bit 31:28: PRESC[3:0]: Timing prescaler
+  - bit 27:24: Reserved
+  - bit 23:20: SCLDEL[3:0]: Data setup time
+  - bit 19:16: SDADEL[3:0]: Data hold time
+  - bit 15:8:  SCLH[7:0]: SCL high period
+  - bit 7:0:   SCLL[7:0]: SCL low period
 
 Example :
 
-- 
1.9.1

Re: [PATCH 1/5] dt-bindings: i2c-stm32: Document the STM32F7 I2C bindings

[Rob Herring]

On Fri, Mar 17, 2017 at 10:58:54AM +0100, M'boumba Cedric Madianga wrote:
> This patch adds the documentation of device tree bindings for STM32F7 I2C
> 
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> ---
>  .../devicetree/bindings/i2c/i2c-stm32.txt          | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-stm32.txt b/Documentation/devicetree/bindings/i2c/i2c-stm32.txt
> index 78eaf7b..8288724 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-stm32.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-stm32.txt
> @@ -1,7 +1,9 @@
>  * I2C controller embedded in STMicroelectronics STM32 I2C platform
>  
>  Required properties :
> -- compatible : Must be "st,stm32f4-i2c"
> +- compatible : Must be one of the following
> +  - "st,stm32f4-i2c"
> +  - "st,stm32f7-i2c"
>  - reg : Offset and length of the register set for the device
>  - interrupts : Must contain the interrupt id for I2C event and then the
>    interrupt id for I2C error.
> @@ -14,8 +16,22 @@ Required properties :
>  
>  Optional properties :
>  - clock-frequency : Desired I2C bus clock frequency in Hz. If not specified,
> -  the default 100 kHz frequency will be used. As only Normal and Fast modes
> -  are supported, possible values are 100000 and 400000.
> +  the default 100 kHz frequency will be used.
> +  For STM32F4 SoC Standard-mode and Fast-mode are supported, possible values are
> +  100000 and 400000.
> +  For STM32F7 SoC, Standard-mode, Fast-mode and Fast-mode Plus are supported,
> +  possible values are 100000, 400000 and 1000000.
> +- st,i2c-timing : A 32-bit I2C timing refister value.

s/refister/register/

With that,

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


> +  This value is only required for "st,stm32f7-i2c".
> +  A specific external tool delivered by ST is used to compute the value to be
> +  set here according to i2C speed frequency, i2c clock source frequency,
> +  i2c rise time and i2c fall time inputs.
> +  - bit 31:28: PRESC[3:0]: Timing prescaler
> +  - bit 27:24: Reserved
> +  - bit 23:20: SCLDEL[3:0]: Data setup time
> +  - bit 19:16: SDADEL[3:0]: Data hold time
> +  - bit 15:8:  SCLH[7:0]: SCL high period
> +  - bit 7:0:   SCLL[7:0]: SCL low period
>  
>  Example :
>  
> -- 
> 1.9.1
> 

[PATCH 2/5] i2c: i2c-stm32f4: use generic definition of speed enum

[M'boumba Cedric Madianga]

This patch uses a more generic definition of speed enum for i2c-stm32f4
driver.

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Reviewed-by: Ludovic BARRE <ludovic.barre@st.com>
---
 drivers/i2c/busses/i2c-stm32.h   | 20 ++++++++++++++++++++
 drivers/i2c/busses/i2c-stm32f4.c | 18 +++++++-----------
 2 files changed, 27 insertions(+), 11 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-stm32.h

diff --git a/drivers/i2c/busses/i2c-stm32.h b/drivers/i2c/busses/i2c-stm32.h
new file mode 100644
index 0000000..dab5176
--- /dev/null
+++ b/drivers/i2c/busses/i2c-stm32.h
@@ -0,0 +1,20 @@
+/*
+ * i2c-stm32.h
+ *
+ * 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
+ */
+
+#ifndef _I2C_STM32_H
+#define _I2C_STM32_H
+
+enum stm32_i2c_speed {
+	STM32_I2C_SPEED_STANDARD, /* 100 kHz */
+	STM32_I2C_SPEED_FAST, /* 400 kHz */
+	STM32_I2C_SPEED_FAST_PLUS, /* 1 MHz */
+	STM32_I2C_SPEED_END,
+};
+
+#endif /* _I2C_STM32_H */
diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c
index f9dd7e8..b81557d 100644
--- a/drivers/i2c/busses/i2c-stm32f4.c
+++ b/drivers/i2c/busses/i2c-stm32f4.c
@@ -27,6 +27,8 @@
 #include <linux/platform_device.h>
 #include <linux/reset.h>
 
+#include "i2c-stm32.h"
+
 /* STM32F4 I2C offset registers */
 #define STM32F4_I2C_CR1			0x00
 #define STM32F4_I2C_CR2			0x04
@@ -90,12 +92,6 @@
 #define STM32F4_I2C_MAX_FREQ		46U
 #define HZ_TO_MHZ			1000000
 
-enum stm32f4_i2c_speed {
-	STM32F4_I2C_SPEED_STANDARD, /* 100 kHz */
-	STM32F4_I2C_SPEED_FAST, /* 400 kHz */
-	STM32F4_I2C_SPEED_END,
-};
-
 /**
  * struct stm32f4_i2c_msg - client specific data
  * @addr: 8-bit slave addr, including r/w bit
@@ -159,7 +155,7 @@ static int stm32f4_i2c_set_periph_clk_freq(struct stm32f4_i2c_dev *i2c_dev)
 	i2c_dev->parent_rate = clk_get_rate(i2c_dev->clk);
 	freq = DIV_ROUND_UP(i2c_dev->parent_rate, HZ_TO_MHZ);
 
-	if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD) {
+	if (i2c_dev->speed == STM32_I2C_SPEED_STANDARD) {
 		/*
 		 * To reach 100 kHz, the parent clk frequency should be between
 		 * a minimum value of 2 MHz and a maximum value of 46 MHz due
@@ -216,7 +212,7 @@ static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
 	 * is not higher than 46 MHz . As a result trise is at most 4 bits wide
 	 * and so fits into the TRISE bits [5:0].
 	 */
-	if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD)
+	if (i2c_dev->speed == STM32_I2C_SPEED_STANDARD)
 		trise = freq + 1;
 	else
 		trise = freq * 3 / 10 + 1;
@@ -230,7 +226,7 @@ static void stm32f4_i2c_set_speed_mode(struct stm32f4_i2c_dev *i2c_dev)
 	u32 val;
 	u32 ccr = 0;
 
-	if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD) {
+	if (i2c_dev->speed == STM32_I2C_SPEED_STANDARD) {
 		/*
 		 * In standard mode:
 		 * t_scl_high = t_scl_low = CCR * I2C parent clk period
@@ -808,10 +804,10 @@ static int stm32f4_i2c_probe(struct platform_device *pdev)
 	udelay(2);
 	reset_control_deassert(rst);
 
-	i2c_dev->speed = STM32F4_I2C_SPEED_STANDARD;
+	i2c_dev->speed = STM32_I2C_SPEED_STANDARD;
 	ret = of_property_read_u32(np, "clock-frequency", &clk_rate);
 	if (!ret && clk_rate >= 400000)
-		i2c_dev->speed = STM32F4_I2C_SPEED_FAST;
+		i2c_dev->speed = STM32_I2C_SPEED_FAST;
 
 	i2c_dev->dev = &pdev->dev;
 
-- 
1.9.1

[PATCH 3/5] i2c: i2c-stm32f7: add driver

[M'boumba Cedric Madianga]

This patch adds initial support for the STM32F7 I2C controller.

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
---
 drivers/i2c/busses/Kconfig       |  10 +
 drivers/i2c/busses/Makefile      |   1 +
 drivers/i2c/busses/i2c-stm32f7.c | 562 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 573 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-stm32f7.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 8adc0f1..ab0caec 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -897,6 +897,16 @@ config I2C_STM32F4
 	  This driver can also be built as module. If so, the module
 	  will be called i2c-stm32f4.
 
+config I2C_STM32F7
+	tristate "STMicroelectronics STM32F7 I2C support"
+	depends on ARCH_STM32 || COMPILE_TEST
+	help
+	  Enable this option to add support for STM32 I2C controller embedded
+	  in STM32F7 SoCs.
+
+	  This driver can also be built as module. If so, the module
+	  will be called i2c-stm32f7.
+
 config I2C_STU300
 	tristate "ST Microelectronics DDC I2C interface"
 	depends on MACH_U300
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 30b6085..5449ece 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -86,6 +86,7 @@ obj-$(CONFIG_I2C_SIMTEC)	+= i2c-simtec.o
 obj-$(CONFIG_I2C_SIRF)		+= i2c-sirf.o
 obj-$(CONFIG_I2C_ST)		+= i2c-st.o
 obj-$(CONFIG_I2C_STM32F4)	+= i2c-stm32f4.o
+obj-$(CONFIG_I2C_STM32F7)	+= i2c-stm32f7.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
new file mode 100644
index 0000000..844a98b
--- /dev/null
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -0,0 +1,562 @@
+/*
+ * Driver for STMicroelectronics STM32F7 I2C controller
+ *
+ * This I2C controller is described in the STM32F75xxx and STM32F74xxx Soc
+ * reference manual.
+ * Please see below a link to the documentation:
+ * http://www.st.com/resource/en/reference_manual/dm00124865.pdf
+ *
+ * Copyright (C) M'boumba Cedric Madianga 2017
+ * Author: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
+ *
+ * This driver is based on i2c-stm32f4.c
+ *
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#include "i2c-stm32.h"
+
+/* STM32F7 I2C registers */
+#define STM32F7_I2C_CR1				0x00
+#define STM32F7_I2C_CR2				0x04
+#define STM32F7_I2C_TIMINGR			0x10
+#define STM32F7_I2C_ISR				0x18
+#define STM32F7_I2C_ICR				0x1C
+#define STM32F7_I2C_RXDR			0x24
+#define STM32F7_I2C_TXDR			0x28
+
+/* STM32F7 I2C control 1 */
+#define STM32F7_I2C_CR1_ERRIE			BIT(7)
+#define STM32F7_I2C_CR1_TCIE			BIT(6)
+#define STM32F7_I2C_CR1_STOPIE			BIT(5)
+#define STM32F7_I2C_CR1_NACKIE			BIT(4)
+#define STM32F7_I2C_CR1_ADDRIE			BIT(3)
+#define STM32F7_I2C_CR1_RXIE			BIT(2)
+#define STM32F7_I2C_CR1_TXIE			BIT(1)
+#define STM32F7_I2C_CR1_PE			BIT(0)
+#define STM32F7_I2C_ALL_IRQ_MASK		(STM32F7_I2C_CR1_ERRIE \
+						| 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)
+#define STM32F7_I2C_CR2_NBYTES_MASK		GENMASK(23, 16)
+#define STM32F7_I2C_CR2_NBYTES(n)		(((n) & 0xff) << 16)
+#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_RD_WRN			BIT(10)
+#define STM32F7_I2C_CR2_SADD7_MASK		GENMASK(7, 1)
+#define STM32F7_I2C_CR2_SADD7(n)		(((n) & 0x7f) << 1)
+
+/* STM32F7 I2C Interrupt Status */
+#define STM32F7_I2C_ISR_BUSY			BIT(15)
+#define STM32F7_I2C_ISR_ARLO			BIT(9)
+#define STM32F7_I2C_ISR_BERR			BIT(8)
+#define STM32F7_I2C_ISR_TCR			BIT(7)
+#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_RXNE			BIT(2)
+#define STM32F7_I2C_ISR_TXIS			BIT(1)
+
+/* 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_MAX_LEN			0xff
+
+/**
+ * struct stm32f7_i2c_msg - client specific data
+ * @addr: 8-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;
+	u32 count;
+	u8 *buf;
+	int result;
+	bool stop;
+};
+
+/**
+ * struct stm32f7_i2c_dev - private data of the controller
+ * @adap: I2C adapter for this controller
+ * @dev: device for this controller
+ * @base: virtual memory area
+ * @complete: completion of I2C message
+ * @clk: hw i2c clock
+ * @speed: I2C clock frequency of the controller. Standard, Fast or Fast+
+ * @msg: Pointer to data to be written
+ * @msg_num: number of I2C messages to be executed
+ * @msg_id: message identifiant
+ * @f7_msg: customized i2c msg for driver usage
+ */
+struct stm32f7_i2c_dev {
+	struct i2c_adapter adap;
+	struct device *dev;
+	void __iomem *base;
+	struct completion complete;
+	struct clk *clk;
+	int speed;
+	struct i2c_msg *msg;
+	unsigned int msg_num;
+	unsigned int msg_id;
+	struct stm32f7_i2c_msg f7_msg;
+};
+
+static inline void stm32f7_i2c_set_bits(void __iomem *reg, u32 mask)
+{
+	writel_relaxed(readl_relaxed(reg) | mask, reg);
+}
+
+static inline void stm32f7_i2c_clr_bits(void __iomem *reg, u32 mask)
+{
+	writel_relaxed(readl_relaxed(reg) & ~mask, reg);
+}
+
+static int stm32f7_i2c_hw_config(struct stm32f7_i2c_dev *i2c_dev)
+{
+	struct device_node *of_node = i2c_dev->dev->of_node;
+	u32 timing;
+	int ret;
+
+	ret = of_property_read_u32(of_node, "st,i2c-timing", &timing);
+	if (ret) {
+		dev_err(i2c_dev->dev, "Error: missing i2c timing property\n");
+		return ret;
+	}
+
+	/* Timing settings */
+	writel_relaxed(timing, i2c_dev->base + STM32F7_I2C_TIMINGR);
+
+	/* Enable I2C */
+	writel_relaxed(STM32F7_I2C_CR1_PE, i2c_dev->base + STM32F7_I2C_CR1);
+
+	return 0;
+}
+
+static void stm32f7_i2c_write_tx_data(struct stm32f7_i2c_dev *i2c_dev)
+{
+	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	void __iomem *base = i2c_dev->base;
+
+	if (f7_msg->count) {
+		writeb_relaxed(*f7_msg->buf++, base + STM32F7_I2C_TXDR);
+		f7_msg->count--;
+	}
+}
+
+static void stm32f7_i2c_read_rx_data(struct stm32f7_i2c_dev *i2c_dev)
+{
+	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	void __iomem *base = i2c_dev->base;
+
+	if (f7_msg->count) {
+		*f7_msg->buf++ = readb_relaxed(base + STM32F7_I2C_RXDR);
+		f7_msg->count--;
+	}
+}
+
+static void stm32f7_i2c_reload(struct stm32f7_i2c_dev *i2c_dev)
+{
+	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	u32 cr2;
+
+	cr2 = readl_relaxed(i2c_dev->base + STM32F7_I2C_CR2);
+
+	cr2 &= ~STM32F7_I2C_CR2_NBYTES_MASK;
+	if (f7_msg->count > STM32F7_I2C_MAX_LEN) {
+		cr2 |= STM32F7_I2C_CR2_NBYTES(STM32F7_I2C_MAX_LEN);
+	} else {
+		cr2 &= ~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;
+	int ret;
+
+	ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F7_I2C_ISR,
+					 status,
+					 !(status & STM32F7_I2C_ISR_BUSY),
+					 10, 1000);
+	if (ret) {
+		dev_dbg(i2c_dev->dev, "bus busy\n");
+		ret = -EBUSY;
+	}
+
+	return ret;
+}
+
+static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
+				 struct i2c_msg *msg)
+{
+	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	void __iomem *base = i2c_dev->base;
+	u32 cr1, cr2;
+
+	f7_msg->addr = msg->addr;
+	f7_msg->buf = msg->buf;
+	f7_msg->count = msg->len;
+	f7_msg->result = 0;
+	f7_msg->stop = (i2c_dev->msg_id >= i2c_dev->msg_num - 1);
+
+	reinit_completion(&i2c_dev->complete);
+
+	cr1 = readl_relaxed(base + STM32F7_I2C_CR1);
+	cr2 = readl_relaxed(base + STM32F7_I2C_CR2);
+
+	/* Set transfer direction */
+	cr2 &= ~STM32F7_I2C_CR2_RD_WRN;
+	if (msg->flags & I2C_M_RD)
+		cr2 |= STM32F7_I2C_CR2_RD_WRN;
+
+	/* Set slave address */
+	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);
+	if (f7_msg->count > STM32F7_I2C_MAX_LEN) {
+		cr2 |= STM32F7_I2C_CR2_NBYTES(STM32F7_I2C_MAX_LEN);
+		cr2 |= STM32F7_I2C_CR2_RELOAD;
+	} else {
+		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 (msg->flags & I2C_M_RD)
+		cr1 |= STM32F7_I2C_CR1_RXIE;
+	else
+		cr1 |= STM32F7_I2C_CR1_TXIE;
+
+	/* Configure Start/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 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 irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
+{
+	struct stm32f7_i2c_dev *i2c_dev = data;
+	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	void __iomem *base = i2c_dev->base;
+	u32 status, mask;
+
+	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
+
+	/* Tx empty */
+	if (status & STM32F7_I2C_ISR_TXIS)
+		stm32f7_i2c_write_tx_data(i2c_dev);
+
+	/* RX not empty */
+	if (status & STM32F7_I2C_ISR_RXNE)
+		stm32f7_i2c_read_rx_data(i2c_dev);
+
+	/* 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);
+		f7_msg->result = -EBADE;
+	}
+
+	/* STOP detection flag */
+	if (status & STM32F7_I2C_ISR_STOPF) {
+		/* Disable interrupts */
+		stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
+
+		/* Clear STOP flag */
+		writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
+
+		complete(&i2c_dev->complete);
+	}
+
+	/* Transfer complete */
+	if (status & STM32F7_I2C_ISR_TC) {
+		if (f7_msg->stop) {
+			mask = STM32F7_I2C_CR2_STOP;
+			stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
+		} else {
+			i2c_dev->msg_id++;
+			i2c_dev->msg++;
+			stm32f7_i2c_xfer_msg(i2c_dev, i2c_dev->msg);
+		}
+	}
+
+	/*
+	 * 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);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
+{
+	struct stm32f7_i2c_dev *i2c_dev = 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;
+
+	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
+
+	/* Bus error */
+	if (status & STM32F7_I2C_ISR_BERR) {
+		dev_err(dev, "<%s>: Bus error\n", __func__);
+		writel_relaxed(STM32F7_I2C_ICR_BERRCF, base + STM32F7_I2C_ICR);
+		f7_msg->result = -EIO;
+	}
+
+	/* Arbitration loss */
+	if (status & STM32F7_I2C_ISR_ARLO) {
+		dev_err(dev, "<%s>: Arbitration loss\n", __func__);
+		writel_relaxed(STM32F7_I2C_ICR_ARLOCF, base + STM32F7_I2C_ICR);
+		f7_msg->result = -EAGAIN;
+	}
+
+	stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
+
+	complete(&i2c_dev->complete);
+
+	return IRQ_HANDLED;
+}
+
+static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
+			    struct i2c_msg msgs[], int num)
+{
+	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
+	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	unsigned long timeout;
+	int ret;
+
+	i2c_dev->msg = msgs;
+	i2c_dev->msg_num = num;
+	i2c_dev->msg_id = 0;
+
+	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;
+
+	stm32f7_i2c_xfer_msg(i2c_dev, msgs);
+
+	timeout = wait_for_completion_timeout(&i2c_dev->complete,
+					      i2c_dev->adap.timeout);
+	ret = f7_msg->result;
+
+	if (!timeout) {
+		dev_dbg(i2c_dev->dev, "Access to slave 0x%x timed out\n",
+			i2c_dev->msg->addr);
+		ret = -ETIMEDOUT;
+	}
+
+clk_free:
+	clk_disable(i2c_dev->clk);
+
+	return (ret < 0) ? ret : num;
+}
+
+static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static struct i2c_algorithm stm32f7_i2c_algo = {
+	.master_xfer = stm32f7_i2c_xfer,
+	.functionality = stm32f7_i2c_func,
+};
+
+static int stm32f7_i2c_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct stm32f7_i2c_dev *i2c_dev;
+	struct resource *res;
+	u32 irq_event, irq_error, clk_rate;
+	struct i2c_adapter *adap;
+	struct reset_control *rst;
+	int ret;
+
+	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
+	if (!i2c_dev)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	i2c_dev->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(i2c_dev->base))
+		return PTR_ERR(i2c_dev->base);
+
+	irq_event = irq_of_parse_and_map(np, 0);
+	if (!irq_event) {
+		dev_err(&pdev->dev, "IRQ event missing or invalid\n");
+		return -EINVAL;
+	}
+
+	irq_error = irq_of_parse_and_map(np, 1);
+	if (!irq_error) {
+		dev_err(&pdev->dev, "IRQ error missing or invalid\n");
+		return -EINVAL;
+	}
+
+	i2c_dev->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(i2c_dev->clk)) {
+		dev_err(&pdev->dev, "Error: Missing controller clock\n");
+		return PTR_ERR(i2c_dev->clk);
+	}
+	ret = clk_prepare_enable(i2c_dev->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to prepare_enable clock\n");
+		return ret;
+	}
+
+	rst = devm_reset_control_get(&pdev->dev, NULL);
+	if (IS_ERR(rst)) {
+		dev_err(&pdev->dev, "Error: Missing controller reset\n");
+		ret = PTR_ERR(rst);
+		goto clk_free;
+	}
+	reset_control_assert(rst);
+	udelay(2);
+	reset_control_deassert(rst);
+
+	i2c_dev->speed = STM32_I2C_SPEED_STANDARD;
+	ret = of_property_read_u32(np, "clock-frequency", &clk_rate);
+	if ((!ret) && (clk_rate == 400000))
+		i2c_dev->speed = STM32_I2C_SPEED_FAST;
+	else if ((!ret) && (clk_rate == 1000000))
+		i2c_dev->speed = STM32_I2C_SPEED_FAST_PLUS;
+
+	i2c_dev->dev = &pdev->dev;
+
+	ret = devm_request_irq(&pdev->dev, irq_event, stm32f7_i2c_isr_event, 0,
+			       pdev->name, i2c_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to request irq event %i\n",
+			irq_event);
+		goto clk_free;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq_error, stm32f7_i2c_isr_error, 0,
+			       pdev->name, i2c_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to request irq error %i\n",
+			irq_error);
+		goto clk_free;
+	}
+
+	ret = stm32f7_i2c_hw_config(i2c_dev);
+	if (ret)
+		goto clk_free;
+
+	adap = &i2c_dev->adap;
+	i2c_set_adapdata(adap, i2c_dev);
+	snprintf(adap->name, sizeof(adap->name), "STM32F7 I2C(%pa)",
+		 &res->start);
+	adap->owner = THIS_MODULE;
+	adap->timeout = 2 * HZ;
+	adap->retries = 0;
+	adap->algo = &stm32f7_i2c_algo;
+	adap->dev.parent = &pdev->dev;
+	adap->dev.of_node = pdev->dev.of_node;
+
+	init_completion(&i2c_dev->complete);
+
+	ret = i2c_add_adapter(adap);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to add adapter\n");
+		goto clk_free;
+	}
+
+	platform_set_drvdata(pdev, i2c_dev);
+
+	clk_disable(i2c_dev->clk);
+
+	dev_info(i2c_dev->dev, "STM32F7 I2C-%d driver registered\n", adap->nr);
+
+	return 0;
+
+clk_free:
+	clk_disable_unprepare(i2c_dev->clk);
+
+	return ret;
+}
+
+static int stm32f7_i2c_remove(struct platform_device *pdev)
+{
+	struct stm32f7_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
+
+	i2c_del_adapter(&i2c_dev->adap);
+
+	clk_unprepare(i2c_dev->clk);
+
+	return 0;
+}
+
+static const struct of_device_id stm32f7_i2c_match[] = {
+	{ .compatible = "st,stm32f7-i2c", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, stm32f7_i2c_match);
+
+static struct platform_driver stm32f7_i2c_driver = {
+	.driver = {
+		.name = "stm32f7-i2c",
+		.of_match_table = stm32f7_i2c_match,
+	},
+	.probe = stm32f7_i2c_probe,
+	.remove = stm32f7_i2c_remove,
+};
+
+module_platform_driver(stm32f7_i2c_driver);
+
+MODULE_AUTHOR("M'boumba Cedric Madianga <cedric.madianga@gmail.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32F7 I2C driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [PATCH 3/5] i2c: i2c-stm32f7: add driver

[Neil Armstrong]

On 03/17/2017 10:58 AM, M'boumba Cedric Madianga wrote:
> This patch adds initial support for the STM32F7 I2C controller.
> 
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> ---
>  drivers/i2c/busses/Kconfig       |  10 +
>  drivers/i2c/busses/Makefile      |   1 +
>  drivers/i2c/busses/i2c-stm32f7.c | 562 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 573 insertions(+)
>  create mode 100644 drivers/i2c/busses/i2c-stm32f7.c
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 8adc0f1..ab0caec 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -897,6 +897,16 @@ config I2C_STM32F4
>  	  This driver can also be built as module. If so, the module
>  	  will be called i2c-stm32f4.
>  
> +config I2C_STM32F7
> +	tristate "STMicroelectronics STM32F7 I2C support"
> +	depends on ARCH_STM32 || COMPILE_TEST
> +	help
> +	  Enable this option to add support for STM32 I2C controller embedded
> +	  in STM32F7 SoCs.
> +
> +	  This driver can also be built as module. If so, the module
> +	  will be called i2c-stm32f7.
> +
>  config I2C_STU300
>  	tristate "ST Microelectronics DDC I2C interface"
>  	depends on MACH_U300
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 30b6085..5449ece 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -86,6 +86,7 @@ obj-$(CONFIG_I2C_SIMTEC)	+= i2c-simtec.o
>  obj-$(CONFIG_I2C_SIRF)		+= i2c-sirf.o
>  obj-$(CONFIG_I2C_ST)		+= i2c-st.o
>  obj-$(CONFIG_I2C_STM32F4)	+= i2c-stm32f4.o
> +obj-$(CONFIG_I2C_STM32F7)	+= i2c-stm32f7.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
> new file mode 100644
> index 0000000..844a98b
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -0,0 +1,562 @@
> +/*
> + * Driver for STMicroelectronics STM32F7 I2C controller
> + *
> + * This I2C controller is described in the STM32F75xxx and STM32F74xxx Soc
> + * reference manual.
> + * Please see below a link to the documentation:
> + * http://www.st.com/resource/en/reference_manual/dm00124865.pdf
> + *
> + * Copyright (C) M'boumba Cedric Madianga 2017
> + * Author: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> + *
> + * This driver is based on i2c-stm32f4.c
> + *
> + * License terms:  GNU General Public License (GPL), version 2
> + */
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +#include "i2c-stm32.h"
> +
> +/* STM32F7 I2C registers */
> +#define STM32F7_I2C_CR1				0x00
> +#define STM32F7_I2C_CR2				0x04
> +#define STM32F7_I2C_TIMINGR			0x10
> +#define STM32F7_I2C_ISR				0x18
> +#define STM32F7_I2C_ICR				0x1C
> +#define STM32F7_I2C_RXDR			0x24
> +#define STM32F7_I2C_TXDR			0x28
> +
> +/* STM32F7 I2C control 1 */
> +#define STM32F7_I2C_CR1_ERRIE			BIT(7)
> +#define STM32F7_I2C_CR1_TCIE			BIT(6)
> +#define STM32F7_I2C_CR1_STOPIE			BIT(5)
> +#define STM32F7_I2C_CR1_NACKIE			BIT(4)
> +#define STM32F7_I2C_CR1_ADDRIE			BIT(3)
> +#define STM32F7_I2C_CR1_RXIE			BIT(2)
> +#define STM32F7_I2C_CR1_TXIE			BIT(1)
> +#define STM32F7_I2C_CR1_PE			BIT(0)
> +#define STM32F7_I2C_ALL_IRQ_MASK		(STM32F7_I2C_CR1_ERRIE \
> +						| 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)
> +#define STM32F7_I2C_CR2_NBYTES_MASK		GENMASK(23, 16)
> +#define STM32F7_I2C_CR2_NBYTES(n)		(((n) & 0xff) << 16)
> +#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_RD_WRN			BIT(10)
> +#define STM32F7_I2C_CR2_SADD7_MASK		GENMASK(7, 1)
> +#define STM32F7_I2C_CR2_SADD7(n)		(((n) & 0x7f) << 1)
> +
> +/* STM32F7 I2C Interrupt Status */
> +#define STM32F7_I2C_ISR_BUSY			BIT(15)
> +#define STM32F7_I2C_ISR_ARLO			BIT(9)
> +#define STM32F7_I2C_ISR_BERR			BIT(8)
> +#define STM32F7_I2C_ISR_TCR			BIT(7)
> +#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_RXNE			BIT(2)
> +#define STM32F7_I2C_ISR_TXIS			BIT(1)
> +
> +/* 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_MAX_LEN			0xff
> +
> +/**
> + * struct stm32f7_i2c_msg - client specific data
> + * @addr: 8-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;
> +	u32 count;
> +	u8 *buf;
> +	int result;
> +	bool stop;
> +};
> +
> +/**
> + * struct stm32f7_i2c_dev - private data of the controller
> + * @adap: I2C adapter for this controller
> + * @dev: device for this controller
> + * @base: virtual memory area
> + * @complete: completion of I2C message
> + * @clk: hw i2c clock
> + * @speed: I2C clock frequency of the controller. Standard, Fast or Fast+
> + * @msg: Pointer to data to be written
> + * @msg_num: number of I2C messages to be executed
> + * @msg_id: message identifiant
> + * @f7_msg: customized i2c msg for driver usage
> + */
> +struct stm32f7_i2c_dev {
> +	struct i2c_adapter adap;
> +	struct device *dev;
> +	void __iomem *base;
> +	struct completion complete;
> +	struct clk *clk;
> +	int speed;
> +	struct i2c_msg *msg;
> +	unsigned int msg_num;
> +	unsigned int msg_id;
> +	struct stm32f7_i2c_msg f7_msg;
> +};
> +
> +static inline void stm32f7_i2c_set_bits(void __iomem *reg, u32 mask)
> +{
> +	writel_relaxed(readl_relaxed(reg) | mask, reg);
> +}
> +
> +static inline void stm32f7_i2c_clr_bits(void __iomem *reg, u32 mask)
> +{
> +	writel_relaxed(readl_relaxed(reg) & ~mask, reg);
> +}
> +
> +static int stm32f7_i2c_hw_config(struct stm32f7_i2c_dev *i2c_dev)
> +{
> +	struct device_node *of_node = i2c_dev->dev->of_node;
> +	u32 timing;
> +	int ret;
> +
> +	ret = of_property_read_u32(of_node, "st,i2c-timing", &timing);
> +	if (ret) {
> +		dev_err(i2c_dev->dev, "Error: missing i2c timing property\n");
> +		return ret;
> +	}
> +
> +	/* Timing settings */
> +	writel_relaxed(timing, i2c_dev->base + STM32F7_I2C_TIMINGR);

Hi,

Using a register value in DT is quite ugly since the requirement to calculate the timings
is quite easy, and well documented.

I wrote it for Zephyr, you can find it here :
https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/i2c/i2c_stm32lx.c#L31

Another point, maybe you should find a better name for the driver, since this I2C IP is share
with the STM32Lx also and is not tied to STM32F7.

Thanks,
Neil

> +
> +	/* Enable I2C */
> +	writel_relaxed(STM32F7_I2C_CR1_PE, i2c_dev->base + STM32F7_I2C_CR1);
> +
> +	return 0;
> +}
> +
> +static void stm32f7_i2c_write_tx_data(struct stm32f7_i2c_dev *i2c_dev)
> +{
> +	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
> +	void __iomem *base = i2c_dev->base;
> +
> +	if (f7_msg->count) {
> +		writeb_relaxed(*f7_msg->buf++, base + STM32F7_I2C_TXDR);
> +		f7_msg->count--;
> +	}
> +}
> +
> +static void stm32f7_i2c_read_rx_data(struct stm32f7_i2c_dev *i2c_dev)
> +{
> +	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
> +	void __iomem *base = i2c_dev->base;
> +
> +	if (f7_msg->count) {
> +		*f7_msg->buf++ = readb_relaxed(base + STM32F7_I2C_RXDR);
> +		f7_msg->count--;
> +	}
> +}
> +
> +static void stm32f7_i2c_reload(struct stm32f7_i2c_dev *i2c_dev)
> +{
> +	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
> +	u32 cr2;
> +
> +	cr2 = readl_relaxed(i2c_dev->base + STM32F7_I2C_CR2);
> +
> +	cr2 &= ~STM32F7_I2C_CR2_NBYTES_MASK;
> +	if (f7_msg->count > STM32F7_I2C_MAX_LEN) {
> +		cr2 |= STM32F7_I2C_CR2_NBYTES(STM32F7_I2C_MAX_LEN);
> +	} else {
> +		cr2 &= ~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;
> +	int ret;
> +
> +	ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F7_I2C_ISR,
> +					 status,
> +					 !(status & STM32F7_I2C_ISR_BUSY),
> +					 10, 1000);
> +	if (ret) {
> +		dev_dbg(i2c_dev->dev, "bus busy\n");
> +		ret = -EBUSY;
> +	}
> +
> +	return ret;
> +}
> +
> +static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
> +				 struct i2c_msg *msg)
> +{
> +	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
> +	void __iomem *base = i2c_dev->base;
> +	u32 cr1, cr2;
> +
> +	f7_msg->addr = msg->addr;
> +	f7_msg->buf = msg->buf;
> +	f7_msg->count = msg->len;
> +	f7_msg->result = 0;
> +	f7_msg->stop = (i2c_dev->msg_id >= i2c_dev->msg_num - 1);
> +
> +	reinit_completion(&i2c_dev->complete);
> +
> +	cr1 = readl_relaxed(base + STM32F7_I2C_CR1);
> +	cr2 = readl_relaxed(base + STM32F7_I2C_CR2);
> +
> +	/* Set transfer direction */
> +	cr2 &= ~STM32F7_I2C_CR2_RD_WRN;
> +	if (msg->flags & I2C_M_RD)
> +		cr2 |= STM32F7_I2C_CR2_RD_WRN;
> +
> +	/* Set slave address */
> +	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);
> +	if (f7_msg->count > STM32F7_I2C_MAX_LEN) {
> +		cr2 |= STM32F7_I2C_CR2_NBYTES(STM32F7_I2C_MAX_LEN);
> +		cr2 |= STM32F7_I2C_CR2_RELOAD;
> +	} else {
> +		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 (msg->flags & I2C_M_RD)
> +		cr1 |= STM32F7_I2C_CR1_RXIE;
> +	else
> +		cr1 |= STM32F7_I2C_CR1_TXIE;
> +
> +	/* Configure Start/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 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 irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
> +{
> +	struct stm32f7_i2c_dev *i2c_dev = data;
> +	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
> +	void __iomem *base = i2c_dev->base;
> +	u32 status, mask;
> +
> +	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
> +
> +	/* Tx empty */
> +	if (status & STM32F7_I2C_ISR_TXIS)
> +		stm32f7_i2c_write_tx_data(i2c_dev);
> +
> +	/* RX not empty */
> +	if (status & STM32F7_I2C_ISR_RXNE)
> +		stm32f7_i2c_read_rx_data(i2c_dev);
> +
> +	/* 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);
> +		f7_msg->result = -EBADE;
> +	}
> +
> +	/* STOP detection flag */
> +	if (status & STM32F7_I2C_ISR_STOPF) {
> +		/* Disable interrupts */
> +		stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
> +
> +		/* Clear STOP flag */
> +		writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
> +
> +		complete(&i2c_dev->complete);
> +	}
> +
> +	/* Transfer complete */
> +	if (status & STM32F7_I2C_ISR_TC) {
> +		if (f7_msg->stop) {
> +			mask = STM32F7_I2C_CR2_STOP;
> +			stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
> +		} else {
> +			i2c_dev->msg_id++;
> +			i2c_dev->msg++;
> +			stm32f7_i2c_xfer_msg(i2c_dev, i2c_dev->msg);
> +		}
> +	}
> +
> +	/*
> +	 * 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);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
> +{
> +	struct stm32f7_i2c_dev *i2c_dev = 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;
> +
> +	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
> +
> +	/* Bus error */
> +	if (status & STM32F7_I2C_ISR_BERR) {
> +		dev_err(dev, "<%s>: Bus error\n", __func__);
> +		writel_relaxed(STM32F7_I2C_ICR_BERRCF, base + STM32F7_I2C_ICR);
> +		f7_msg->result = -EIO;
> +	}
> +
> +	/* Arbitration loss */
> +	if (status & STM32F7_I2C_ISR_ARLO) {
> +		dev_err(dev, "<%s>: Arbitration loss\n", __func__);
> +		writel_relaxed(STM32F7_I2C_ICR_ARLOCF, base + STM32F7_I2C_ICR);
> +		f7_msg->result = -EAGAIN;
> +	}
> +
> +	stm32f7_i2c_disable_irq(i2c_dev, STM32F7_I2C_ALL_IRQ_MASK);
> +
> +	complete(&i2c_dev->complete);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
> +			    struct i2c_msg msgs[], int num)
> +{
> +	struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
> +	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
> +	unsigned long timeout;
> +	int ret;
> +
> +	i2c_dev->msg = msgs;
> +	i2c_dev->msg_num = num;
> +	i2c_dev->msg_id = 0;
> +
> +	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;
> +
> +	stm32f7_i2c_xfer_msg(i2c_dev, msgs);
> +
> +	timeout = wait_for_completion_timeout(&i2c_dev->complete,
> +					      i2c_dev->adap.timeout);
> +	ret = f7_msg->result;
> +
> +	if (!timeout) {
> +		dev_dbg(i2c_dev->dev, "Access to slave 0x%x timed out\n",
> +			i2c_dev->msg->addr);
> +		ret = -ETIMEDOUT;
> +	}
> +
> +clk_free:
> +	clk_disable(i2c_dev->clk);
> +
> +	return (ret < 0) ? ret : num;
> +}
> +
> +static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
> +{
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> +}
> +
> +static struct i2c_algorithm stm32f7_i2c_algo = {
> +	.master_xfer = stm32f7_i2c_xfer,
> +	.functionality = stm32f7_i2c_func,
> +};
> +
> +static int stm32f7_i2c_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct stm32f7_i2c_dev *i2c_dev;
> +	struct resource *res;
> +	u32 irq_event, irq_error, clk_rate;
> +	struct i2c_adapter *adap;
> +	struct reset_control *rst;
> +	int ret;
> +
> +	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
> +	if (!i2c_dev)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	i2c_dev->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(i2c_dev->base))
> +		return PTR_ERR(i2c_dev->base);
> +
> +	irq_event = irq_of_parse_and_map(np, 0);
> +	if (!irq_event) {
> +		dev_err(&pdev->dev, "IRQ event missing or invalid\n");
> +		return -EINVAL;
> +	}
> +
> +	irq_error = irq_of_parse_and_map(np, 1);
> +	if (!irq_error) {
> +		dev_err(&pdev->dev, "IRQ error missing or invalid\n");
> +		return -EINVAL;
> +	}
> +
> +	i2c_dev->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(i2c_dev->clk)) {
> +		dev_err(&pdev->dev, "Error: Missing controller clock\n");
> +		return PTR_ERR(i2c_dev->clk);
> +	}
> +	ret = clk_prepare_enable(i2c_dev->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to prepare_enable clock\n");
> +		return ret;
> +	}
> +
> +	rst = devm_reset_control_get(&pdev->dev, NULL);
> +	if (IS_ERR(rst)) {
> +		dev_err(&pdev->dev, "Error: Missing controller reset\n");
> +		ret = PTR_ERR(rst);
> +		goto clk_free;
> +	}
> +	reset_control_assert(rst);
> +	udelay(2);
> +	reset_control_deassert(rst);
> +
> +	i2c_dev->speed = STM32_I2C_SPEED_STANDARD;
> +	ret = of_property_read_u32(np, "clock-frequency", &clk_rate);
> +	if ((!ret) && (clk_rate == 400000))
> +		i2c_dev->speed = STM32_I2C_SPEED_FAST;
> +	else if ((!ret) && (clk_rate == 1000000))
> +		i2c_dev->speed = STM32_I2C_SPEED_FAST_PLUS;
> +
> +	i2c_dev->dev = &pdev->dev;
> +
> +	ret = devm_request_irq(&pdev->dev, irq_event, stm32f7_i2c_isr_event, 0,
> +			       pdev->name, i2c_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to request irq event %i\n",
> +			irq_event);
> +		goto clk_free;
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, irq_error, stm32f7_i2c_isr_error, 0,
> +			       pdev->name, i2c_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to request irq error %i\n",
> +			irq_error);
> +		goto clk_free;
> +	}
> +
> +	ret = stm32f7_i2c_hw_config(i2c_dev);
> +	if (ret)
> +		goto clk_free;
> +
> +	adap = &i2c_dev->adap;
> +	i2c_set_adapdata(adap, i2c_dev);
> +	snprintf(adap->name, sizeof(adap->name), "STM32F7 I2C(%pa)",
> +		 &res->start);
> +	adap->owner = THIS_MODULE;
> +	adap->timeout = 2 * HZ;
> +	adap->retries = 0;
> +	adap->algo = &stm32f7_i2c_algo;
> +	adap->dev.parent = &pdev->dev;
> +	adap->dev.of_node = pdev->dev.of_node;
> +
> +	init_completion(&i2c_dev->complete);
> +
> +	ret = i2c_add_adapter(adap);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to add adapter\n");
> +		goto clk_free;
> +	}
> +
> +	platform_set_drvdata(pdev, i2c_dev);
> +
> +	clk_disable(i2c_dev->clk);
> +
> +	dev_info(i2c_dev->dev, "STM32F7 I2C-%d driver registered\n", adap->nr);
> +
> +	return 0;
> +
> +clk_free:
> +	clk_disable_unprepare(i2c_dev->clk);
> +
> +	return ret;
> +}
> +
> +static int stm32f7_i2c_remove(struct platform_device *pdev)
> +{
> +	struct stm32f7_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
> +
> +	i2c_del_adapter(&i2c_dev->adap);
> +
> +	clk_unprepare(i2c_dev->clk);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id stm32f7_i2c_match[] = {
> +	{ .compatible = "st,stm32f7-i2c", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, stm32f7_i2c_match);
> +
> +static struct platform_driver stm32f7_i2c_driver = {
> +	.driver = {
> +		.name = "stm32f7-i2c",
> +		.of_match_table = stm32f7_i2c_match,
> +	},
> +	.probe = stm32f7_i2c_probe,
> +	.remove = stm32f7_i2c_remove,
> +};
> +
> +module_platform_driver(stm32f7_i2c_driver);
> +
> +MODULE_AUTHOR("M'boumba Cedric Madianga <cedric.madianga@gmail.com>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32F7 I2C driver");
> +MODULE_LICENSE("GPL v2");
> 

Re: [PATCH 3/5] i2c: i2c-stm32f7: add driver

[M'boumba Cedric Madianga]

Hi,


>> +static int stm32f7_i2c_hw_config(struct stm32f7_i2c_dev *i2c_dev)
>> +{
>> +     struct device_node *of_node = i2c_dev->dev->of_node;
>> +     u32 timing;
>> +     int ret;
>> +
>> +     ret = of_property_read_u32(of_node, "st,i2c-timing", &timing);
>> +     if (ret) {
>> +             dev_err(i2c_dev->dev, "Error: missing i2c timing property\n");
>> +             return ret;
>> +     }
>> +
>> +     /* Timing settings */
>> +     writel_relaxed(timing, i2c_dev->base + STM32F7_I2C_TIMINGR);
>
> Hi,
>
> Using a register value in DT is quite ugly since the requirement to calculate the timings
> is quite easy, and well documented.
>
> I wrote it for Zephyr, you can find it here :
> https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/i2c/i2c_stm32lx.c#L31

Thanks for this code. It is very interesting.
With our formula, I just notice that we don't use any i2c rise time or
fall time but they have some impacts in i2c timing computation.
So, the user could not be able to select these values for him use case. Right ?


>
> Another point, maybe you should find a better name for the driver, since this I2C IP is share
> with the STM32Lx also and is not tied to STM32F7.

As far as I know, we don't have any STM32Lx SoC integrated in the
linux kernel mainline.
I choose i2c-stm32f7 name as it is the first Soc integrated in the
linux kernel mainline where this IP could be used.

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

Re: [PATCH 3/5] i2c: i2c-stm32f7: add driver

[Neil Armstrong]

On 03/17/2017 02:52 PM, M'boumba Cedric Madianga wrote:
> Hi,
> 
> 
>>> +static int stm32f7_i2c_hw_config(struct stm32f7_i2c_dev *i2c_dev)
>>> +{
>>> +     struct device_node *of_node = i2c_dev->dev->of_node;
>>> +     u32 timing;
>>> +     int ret;
>>> +
>>> +     ret = of_property_read_u32(of_node, "st,i2c-timing", &timing);
>>> +     if (ret) {
>>> +             dev_err(i2c_dev->dev, "Error: missing i2c timing property\n");
>>> +             return ret;
>>> +     }
>>> +
>>> +     /* Timing settings */
>>> +     writel_relaxed(timing, i2c_dev->base + STM32F7_I2C_TIMINGR);
>>
>> Hi,
>>
>> Using a register value in DT is quite ugly since the requirement to calculate the timings
>> is quite easy, and well documented.
>>
>> I wrote it for Zephyr, you can find it here :
>> https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/i2c/i2c_stm32lx.c#L31
> 
> Thanks for this code. It is very interesting.
> With our formula, I just notice that we don't use any i2c rise time or
> fall time but they have some impacts in i2c timing computation.
> So, the user could not be able to select these values for him use case. Right ?

Sorry I don't understand.
The value you use from the DT and the one calculated from the setup/hold/high/low value
with the algorithm I developed will set the same values.

The main difference is that you won't need the ST tool to calculate these, and even better
you can provide generic binding for whatever APB frequency the I2C peripheral is running
on.

> 
> 
>>
>> Another point, maybe you should find a better name for the driver, since this I2C IP is share
>> with the STM32Lx also and is not tied to STM32F7.
> 
> As far as I know, we don't have any STM32Lx SoC integrated in the
> linux kernel mainline.
> I choose i2c-stm32f7 name as it is the first Soc integrated in the
> linux kernel mainline where this IP could be used.

Having the STM32L4 running mainline won't be hard, maybe useless, but not hard.
My point is that you can somehow consider the STM32F0/F1/F4 I2C IP to be "v1" or "gen1",
and the L0/L4/F6 (and H7 ?) to be "v2" or "gen2" then prepend a generic compatible to
have something like :
compatible = "st,stm32-i2c-v2", "st,stm32f7-i2c"

> BR,
> Cedric
> 

Thanks,
Neil

Re: [PATCH 3/5] i2c: i2c-stm32f7: add driver

[M'boumba Cedric Madianga]

> Sorry I don't understand.
> The value you use from the DT and the one calculated from the setup/hold/high/low value
> with the algorithm I developed will set the same values.

With the ST tool, I could set the following values:
I2C speed mode (Master, Fast Mode, Fast Mode Plus)
I2C speed frequency
I2C clock source frequency
I2C rise time
I2C fall time

The values set in the DT in this patchset is 0x40202537 for the
following input values:
I2C speed mode  = Master mode
I2C speed frequency= 100 kHz
I2C clock source frequency = 48 MHz
I2C rise time = 25 ns
I2C fall time = 10 ns

If I keep all the above values and just change I2C rise time with 100
ns, I will have TIMINGR value at 0x10805E89

>
> The main difference is that you won't need the ST tool to calculate these, and even better
> you can provide generic binding for whatever APB frequency the I2C peripheral is running
> on.

As, I2C rise/fall time have some impacts in I2C timings value, the
question is: it is very relevant to let customer control these
parameters ?
If the answer is NO, I agree with you, it is better to use your
formula and remove this property from DT.
If the answer is YES, I think we should keep ST tool.

> Having the STM32L4 running mainline won't be hard, maybe useless, but not hard.
> My point is that you can somehow consider the STM32F0/F1/F4 I2C IP to be "v1" or "gen1",
> and the L0/L4/F6 (and H7 ?) to be "v2" or "gen2" then prepend a generic compatible to
> have something like :
> compatible = "st,stm32-i2c-v2", "st,stm32f7-i2c"

This is not the strategy chosen by the company.
They decided to push all driver with ip_name-stm32.c if the driver is
common for all Soc.
If it not the case, the suffix to be used is the name of the first
supported SoC that introduced the IP in the mainline kernel.

BR,
Cedric

Re: [PATCH 3/5] i2c: i2c-stm32f7: add driver

[Neil Armstrong]

Hi Cedric,

On 03/17/2017 04:35 PM, M'boumba Cedric Madianga wrote:
>> Sorry I don't understand.
>> The value you use from the DT and the one calculated from the setup/hold/high/low value
>> with the algorithm I developed will set the same values.
> 
> With the ST tool, I could set the following values:
> I2C speed mode (Master, Fast Mode, Fast Mode Plus)
> I2C speed frequency
> I2C clock source frequency
> I2C rise time
> I2C fall time
> 
> The values set in the DT in this patchset is 0x40202537 for the
> following input values:
> I2C speed mode  = Master mode
> I2C speed frequency= 100 kHz
> I2C clock source frequency = 48 MHz
> I2C rise time = 25 ns
> I2C fall time = 10 ns
> 
> If I keep all the above values and just change I2C rise time with 100
> ns, I will have TIMINGR value at 0x10805E89
>
>>
>> The main difference is that you won't need the ST tool to calculate these, and even better
>> you can provide generic binding for whatever APB frequency the I2C peripheral is running
>> on.
> 
> As, I2C rise/fall time have some impacts in I2C timings value, the
> question is: it is very relevant to let customer control these
> parameters ?

Actually, you could specify a different rise time in DT if it's relevant for
a specific design, this is why you have the following DT attributes :
- i2c-scl-falling-time-ns
- i2c-scl-internal-delay-ns
- i2c-scl-rising-time-ns
- i2c-sda-falling-time-ns

> If the answer is NO, I agree with you, it is better to use your
> formula and remove this property from DT.
> If the answer is YES, I think we should keep ST tool.

Note that the ST tool calculations are tied to the clock source frequency, so either
you provide a table for all the possible clock source frequencies or calculate dynamically.
Having a single parameter for a single frequency is, from my point of view, not acceptable.

And, I don't think it's a military secret to have (at least) a simplified algorithm from ST...
since you have very detailed explanations in the public manuals !

> 
>> Having the STM32L4 running mainline won't be hard, maybe useless, but not hard.
>> My point is that you can somehow consider the STM32F0/F1/F4 I2C IP to be "v1" or "gen1",
>> and the L0/L4/F6 (and H7 ?) to be "v2" or "gen2" then prepend a generic compatible to
>> have something like :
>> compatible = "st,stm32-i2c-v2", "st,stm32f7-i2c"
> 
> This is not the strategy chosen by the company.
> They decided to push all driver with ip_name-stm32.c if the driver is
> common for all Soc.
> If it not the case, the suffix to be used is the name of the first
> supported SoC that introduced the IP in the mainline kernel.

OK, but I think the I2C and DT maintainers are also involved in these kind of decisions.
They should also give their advice.

Neil

> 
> BR,
> Cedric
> 

Re: [PATCH 3/5] i2c: i2c-stm32f7: add driver

[M'boumba Cedric Madianga]

Hi Neil,

>> As, I2C rise/fall time have some impacts in I2C timings value, the
>> question is: it is very relevant to let customer control these
>> parameters ?
>
> Actually, you could specify a different rise time in DT if it's relevant for
> a specific design, this is why you have the following DT attributes :
> - i2c-scl-falling-time-ns
> - i2c-scl-internal-delay-ns
> - i2c-scl-rising-time-ns
> - i2c-sda-falling-time-ns
>
>> If the answer is NO, I agree with you, it is better to use your
>> formula and remove this property from DT.
>> If the answer is YES, I think we should keep ST tool.
>
> Note that the ST tool calculations are tied to the clock source frequency, so either
> you provide a table for all the possible clock source frequencies or calculate dynamically.
> Having a single parameter for a single frequency is, from my point of view, not acceptable.
>
> And, I don't think it's a military secret to have (at least) a simplified algorithm from ST...
> since you have very detailed explanations in the public manuals !

OK. I will do some trials with your algorithm and push it in the V2.
Thanks

>
> OK, but I think the I2C and DT maintainers are also involved in these kind of decisions.
> They should also give their advice.

I already upstream an I2C driver with this approach: "i2c-stm32f4".
I don't think that Wolfram or Rob will change the philosophy for this driver.
Then, I also don't think that the machine code for F0/F1/L0/L4 will be
pushed in the mainline as it will be completely useless to port a
linux kernel for this kind of chip.

BR,
Cedric

Re: [PATCH 3/5] i2c: i2c-stm32f7: add driver

[Wolfram Sang]

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

On Fri, Mar 17, 2017 at 10:58:56AM +0100, M'boumba Cedric Madianga wrote:
> This patch adds initial support for the STM32F7 I2C controller.
> 
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

So, the STM32F7 has a new I2C IP core compared to STM32F4?


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

Re: [PATCH 3/5] i2c: i2c-stm32f7: add driver

[M'boumba Cedric Madianga]

Hi Wolfram,

Yes the STM32F7 has a new I2C IP core compared to STM32F4.
The engine, the machine state are very different.
I tried few months ago to write a common driver but it was very very
difficilcut as the 2 IP are not so much things in common.

BR,
Cedric

2017-03-23 21:17 GMT+01:00 Wolfram Sang <wsa@the-dreams.de>:
> On Fri, Mar 17, 2017 at 10:58:56AM +0100, M'boumba Cedric Madianga wrote:
>> This patch adds initial support for the STM32F7 I2C controller.
>>
>> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>
> So, the STM32F7 has a new I2C IP core compared to STM32F4?
>

Re: [PATCH 3/5] i2c: i2c-stm32f7: add driver

[Wolfram Sang]

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


> Yes the STM32F7 has a new I2C IP core compared to STM32F4.
> The engine, the machine state are very different.
> I tried few months ago to write a common driver but it was very very
> difficilcut as the 2 IP are not so much things in common.

Good, thanks for the heads up!


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

[PATCH 4/5] ARM: dts: stm32: Add I2C1 support for STM32F746 SoC

[M'boumba Cedric Madianga]

This patch adds I2C1 support for STM32F746 SoC.

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
---
 arch/arm/boot/dts/stm32f746.dtsi | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
index f321ffe..89d3897 100644
--- a/arch/arm/boot/dts/stm32f746.dtsi
+++ b/arch/arm/boot/dts/stm32f746.dtsi
@@ -287,6 +287,29 @@
 					bias-disable;
 				};
 			};
+
+			i2c1_pins_b: i2c1@0 {
+				pins {
+					pinmux = <STM32F746_PB9_FUNC_I2C1_SDA>,
+						 <STM32F746_PB6_FUNC_I2C1_SCL>;
+					bias-disable;
+					drive-open-drain;
+					slew-rate = <3>;
+				};
+			};
+		};
+
+		i2c1: i2c@40005400 {
+			compatible = "st,stm32f7-i2c";
+			reg = <0x40005400 0x400>;
+			interrupts = <31>,
+				     <32>;
+			resets = <&rcc 149>;
+			clocks = <&rcc 0 405>;
+			st,i2c-timing = <0x40202537>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
 		};
 
 		rcc: rcc@40023800 {
-- 
1.9.1

[PATCH 5/5] ARM: dts: stm32: Add I2C1 support for STM32F746 eval board

[M'boumba Cedric Madianga]

This patch adds I2C1 support for STM32F746 eval board

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
---
 arch/arm/boot/dts/stm32746g-eval.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/stm32746g-eval.dts b/arch/arm/boot/dts/stm32746g-eval.dts
index aa03fac..19cb82d 100644
--- a/arch/arm/boot/dts/stm32746g-eval.dts
+++ b/arch/arm/boot/dts/stm32746g-eval.dts
@@ -94,3 +94,9 @@
 	pinctrl-names = "default";
 	status = "okay";
 };
+
+&i2c1 {
+	pinctrl-0 = <&i2c1_pins_b>;
+	pinctrl-names = "default";
+	status = "okay";
+};
-- 
1.9.1