All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH V4 1/2] i2c: mxc: refactor i2c driver and support dm
@ 2015-05-14 10:03 Peng Fan
  2015-05-14 10:03 ` [U-Boot] [PATCH V4 2/2] imx: mx27 remove redundant macro Peng Fan
  2015-05-14 15:01 ` [U-Boot] [PATCH V4 1/2] i2c: mxc: refactor i2c driver and support dm Simon Glass
  0 siblings, 2 replies; 7+ messages in thread
From: Peng Fan @ 2015-05-14 10:03 UTC (permalink / raw)
  To: u-boot

1. Introduce a new structure `struct mxc_i2c_bus`, this structure will
   used for non-DM and DM.
2. Remove `struct mxc_i2c_regs` structure, but use register offset to access
   registers based on `base` entry of `struct mxc_i2c_bus`.
3. Remove most `#ifdef I2C_QUIRK_REG`. Using driver_data to contain platform
   flags. A new flag is introduced, I2C_QUIRK_FLAG.
4. Most functions use `struct mxc_i2c_bus` as one of the parameters.
   Make most functions common to DM and non-DM, try to avoid duplicated code.
5. Support DM, but pinctrl is not included. Pinmux setting is still set
   by setup_i2c, but we do not need bus_i2c_init for DM.
6. struct i2c_parms and struct sram_data are removed.
7. Remove bus_i2c_read bus_i2c_write prototype in header file. The frist
   paramter of bus_i2c_init is modified to i2c index. Add new prototype
   i2c_idle_bus and force_bus_idle. Since bus_i2c_init is not good for
   DM I2C and pinctrl is missed, we use a weak function for i2c_idle_bus.
   Board file take the responsibility to implement this function, like this:
   "
   int i2c_idle_bus(struct mxc_i2c_bus *i2c_bus)
   {
	   if (i2c_bus->index == 0)
		   force_bus_idle(i2c_pads_info0);
	   else if (i2c_bus->index == 1)
		   force_bus_idle(i2c_pads_info1);
	   else
		   xxxxxx
   }
   "
8. Introduce a weak function, enable_i2c_clk
9. Tested on an i.MX7 platform. Log info:
 => dm tree
 Class       Probed   Name
 ----------------------------------------
 root        [ + ]    root_driver
 simple_bus  [   ]    |-- soc
 simple_bus  [   ]    |   |-- aips-bus at 30000000
 simple_bus  [   ]    |   |   |-- anatop at 30360000
 simple_bus  [   ]    |   |   `-- snvs at 30370000
 simple_bus  [   ]    |   |-- aips-bus at 30400000
 simple_bus  [   ]    |   `-- aips-bus at 30800000
 i2c         [   ]    |       |-- i2c at 30a20000
 i2c         [   ]    |       `-- i2c at 30a40000
 simple_bus  [   ]    `-- regulators
 => i2c dev 0
 Setting bus to 0
 => i2c probe
 Valid chip addresses: 08 50
 => i2c md 8 31
 0031: 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08

Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
Acked-by: Simon Glass <sjg@chromium.org>
---

 Changes v4:
 1. fix build errors
 2. Introduce a weak function enable_i2c_clk.
 3. add Simon's Acked-by.
 4. Add test log in commit log.

 Changes v3:
  1. remove bus_i2c_init for DM, introuduce a weak function i2c_idle_bus.
  2. remove static return type for force_idle_bus, since we need to call
     it in i2c_idle_bus which may be implemented in board file. This does
     not hurt for non-DM.

 Changes v2:
  1. Refactor driver, remove register access based on structure, but use
    'base + offset'
  2. Introduce mxc_i2c_bus structure
  3. Introduce I2C_QUIRK_FLAG and remove most I2C_QUIRK_REG and use
     driver_data to contain the flags for different platforms
  4.  Avoid duplicated code between DM and non-DM part
  5. The function name i2c_init_transfer is not changed.
  6. Remove bus_i2c_read/write prototype from header file
  7. change bus_i2c_init's first parameter to i2c index
  8. Rename patch name, since refactor non-DM part.

 arch/arm/imx-common/i2c-mxv7.c            |   7 +-
 arch/arm/include/asm/imx-common/mxc_i2c.h |  38 +-
 drivers/i2c/mxc_i2c.c                     | 581 ++++++++++++++++++++----------
 3 files changed, 433 insertions(+), 193 deletions(-)

diff --git a/arch/arm/imx-common/i2c-mxv7.c b/arch/arm/imx-common/i2c-mxv7.c
index 1a632e7..f3a5c3f 100644
--- a/arch/arm/imx-common/i2c-mxv7.c
+++ b/arch/arm/imx-common/i2c-mxv7.c
@@ -12,7 +12,7 @@
 #include <asm/imx-common/mxc_i2c.h>
 #include <watchdog.h>
 
-static int force_idle_bus(void *priv)
+int force_idle_bus(void *priv)
 {
 	int i;
 	int sda, scl;
@@ -99,8 +99,9 @@ int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
 	if (ret)
 		goto err_idle;
 
-	bus_i2c_init(i2c_bases[i2c_index], speed, slave_addr,
-			force_idle_bus, p);
+#ifndef CONFIG_DM_I2C
+	bus_i2c_init(i2c_index, speed, slave_addr, force_idle_bus, p);
+#endif
 
 	return 0;
 
diff --git a/arch/arm/include/asm/imx-common/mxc_i2c.h b/arch/arm/include/asm/imx-common/mxc_i2c.h
index af86163..355b25e 100644
--- a/arch/arm/include/asm/imx-common/mxc_i2c.h
+++ b/arch/arm/include/asm/imx-common/mxc_i2c.h
@@ -19,6 +19,36 @@ struct i2c_pads_info {
 	struct i2c_pin_ctrl sda;
 };
 
+/*
+ * Information about i2c controller
+ * struct mxc_i2c_bus - information about the i2c[x] bus
+ * @index: i2c bus index
+ * @base: Address of I2C bus controller
+ * @driver_data: Flags for different platforms, such as I2C_QUIRK_FLAG.
+ * @speed: Speed of I2C bus
+ * @pads_info: pinctrl info for this i2c bus, will be used when pinctrl is ok.
+ * The following two is only to be compatible with non-DM part.
+ * @idle_bus_fn: function to force bus idle
+ * @idle_bus_data: parameter for idle_bus_fun
+ */
+struct mxc_i2c_bus {
+	/*
+	 * board file can use this index to locate which i2c_pads_info is for
+	 * i2c_idle_bus. When pinmux is implement, this entry can be
+	 * discarded. Here we do not use dev->seq, because we do not want to
+	 * export device to board file.
+	 */
+	int index;
+	ulong base;
+	ulong driver_data;
+	int speed;
+	struct i2c_pads_info *pads_info;
+#ifndef CONFIG_DM_I2C
+	int (*idle_bus_fn)(void *p);
+	void *idle_bus_data;
+#endif
+};
+
 #if defined(CONFIG_MX6QDL)
 #define I2C_PADS(name, scl_i2c, scl_gpio, scl_gp, sda_i2c, sda_gpio, sda_gp) \
 	struct i2c_pads_info mx6q_##name = {		\
@@ -54,10 +84,8 @@ struct i2c_pads_info {
 
 int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
 	      struct i2c_pads_info *p);
-void bus_i2c_init(void *base, int speed, int slave_addr,
+void bus_i2c_init(int index, int speed, int slave_addr,
 		int (*idle_bus_fn)(void *p), void *p);
-int bus_i2c_read(void *base, uchar chip, uint addr, int alen, uchar *buf,
-		int len);
-int bus_i2c_write(void *base, uchar chip, uint addr, int alen,
-		const uchar *buf, int len);
+int force_idle_bus(void *priv);
+int i2c_idle_bus(struct mxc_i2c_bus *i2c_bus);
 #endif
diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 42782cb..81adf6f 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -18,29 +18,25 @@
 #include <asm/arch/clock.h>
 #include <asm/arch/imx-regs.h>
 #include <asm/errno.h>
+#include <asm/imx-common/mxc_i2c.h>
 #include <asm/io.h>
 #include <i2c.h>
 #include <watchdog.h>
+#include <dm.h>
+#include <fdtdec.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#ifdef I2C_QUIRK_REG
-struct mxc_i2c_regs {
-	uint8_t		iadr;
-	uint8_t		ifdr;
-	uint8_t		i2cr;
-	uint8_t		i2sr;
-	uint8_t		i2dr;
-};
-#else
-struct mxc_i2c_regs {
-	uint32_t	iadr;
-	uint32_t	ifdr;
-	uint32_t	i2cr;
-	uint32_t	i2sr;
-	uint32_t	i2dr;
-};
-#endif
+#define I2C_QUIRK_FLAG		(1 << 0)
+
+#define IMX_I2C_REGSHIFT	2
+#define VF610_I2C_REGSHIFT	0
+/* Register index */
+#define IADR	0
+#define IFDR	1
+#define I2CR	2
+#define I2SR	3
+#define I2DR	4
 
 #define I2CR_IIEN	(1 << 6)
 #define I2CR_MSTA	(1 << 5)
@@ -104,7 +100,6 @@ static u16 i2c_clk_div[50][2] = {
 };
 #endif
 
-
 #ifndef CONFIG_SYS_MXC_I2C1_SPEED
 #define CONFIG_SYS_MXC_I2C1_SPEED 100000
 #endif
@@ -131,11 +126,10 @@ static u16 i2c_clk_div[50][2] = {
 #define CONFIG_SYS_MXC_I2C4_SLAVE 0
 #endif
 
-
 /*
  * Calculate and set proper clock divider
  */
-static uint8_t i2c_imx_get_clk(unsigned int rate)
+static uint8_t i2c_imx_get_clk(struct mxc_i2c_bus *i2c_bus, unsigned int rate)
 {
 	unsigned int i2c_clk_rate;
 	unsigned int div;
@@ -168,18 +162,20 @@ static uint8_t i2c_imx_get_clk(unsigned int rate)
 /*
  * Set I2C Bus speed
  */
-static int bus_i2c_set_bus_speed(void *base, int speed)
+static int bus_i2c_set_bus_speed(struct mxc_i2c_bus *i2c_bus, int speed)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
-	u8 clk_idx = i2c_imx_get_clk(speed);
+	ulong base = i2c_bus->base;
+	bool quirk = i2c_bus->driver_data & I2C_QUIRK_FLAG ? true : false;
+	u8 clk_idx = i2c_imx_get_clk(i2c_bus, speed);
 	u8 idx = i2c_clk_div[clk_idx][1];
+	int reg_shift = quirk ? VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
 
 	/* Store divider value */
-	writeb(idx, &i2c_regs->ifdr);
+	writeb(idx, base + (IFDR << reg_shift));
 
 	/* Reset module */
-	writeb(I2CR_IDIS, &i2c_regs->i2cr);
-	writeb(0, &i2c_regs->i2sr);
+	writeb(I2CR_IDIS, base + (I2CR << reg_shift));
+	writeb(0, base + (I2SR << reg_shift));
 	return 0;
 }
 
@@ -187,21 +183,26 @@ static int bus_i2c_set_bus_speed(void *base, int speed)
 #define ST_BUS_BUSY (I2SR_IBB | (I2SR_IBB << 8))
 #define ST_IIF (I2SR_IIF | (I2SR_IIF << 8))
 
-static int wait_for_sr_state(struct mxc_i2c_regs *i2c_regs, unsigned state)
+static int wait_for_sr_state(struct mxc_i2c_bus *i2c_bus, unsigned state)
 {
 	unsigned sr;
 	ulong elapsed;
+	bool quirk = i2c_bus->driver_data & I2C_QUIRK_FLAG ? true : false;
+	int reg_shift = quirk ? VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
+	ulong base = i2c_bus->base;
 	ulong start_time = get_timer(0);
 	for (;;) {
-		sr = readb(&i2c_regs->i2sr);
+		sr = readb(base + (I2SR << reg_shift));
 		if (sr & I2SR_IAL) {
-#ifdef I2C_QUIRK_REG
-			writeb(sr | I2SR_IAL, &i2c_regs->i2sr);
-#else
-			writeb(sr & ~I2SR_IAL, &i2c_regs->i2sr);
-#endif
+			if (quirk)
+				writeb(sr | I2SR_IAL, base +
+				       (I2SR << reg_shift));
+			else
+				writeb(sr & ~I2SR_IAL, base +
+				       (I2SR << reg_shift));
 			printf("%s: Arbitration lost sr=%x cr=%x state=%x\n",
-				__func__, sr, readb(&i2c_regs->i2cr), state);
+				__func__, sr, readb(base + (I2CR << reg_shift)),
+				state);
 			return -ERESTART;
 		}
 		if ((sr & (state >> 8)) == (unsigned char)state)
@@ -212,17 +213,21 @@ static int wait_for_sr_state(struct mxc_i2c_regs *i2c_regs, unsigned state)
 			break;
 	}
 	printf("%s: failed sr=%x cr=%x state=%x\n", __func__,
-			sr, readb(&i2c_regs->i2cr), state);
+	       sr, readb(base + (I2CR << reg_shift)), state);
 	return -ETIMEDOUT;
 }
 
-static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
+static int tx_byte(struct mxc_i2c_bus *i2c_bus, u8 byte)
 {
 	int ret;
+	int reg_shift = i2c_bus->driver_data & I2C_QUIRK_FLAG ?
+			VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
+	ulong base = i2c_bus->base;
 
-	writeb(I2SR_IIF_CLEAR, &i2c_regs->i2sr);
-	writeb(byte, &i2c_regs->i2dr);
-	ret = wait_for_sr_state(i2c_regs, ST_IIF);
+	writeb(I2SR_IIF_CLEAR, base + (I2SR << reg_shift));
+	writeb(byte, base + (I2DR << reg_shift));
+
+	ret = wait_for_sr_state(i2c_bus, ST_IIF);
 	if (ret < 0)
 		return ret;
 	if (ret & I2SR_RX_NO_AK)
@@ -231,16 +236,28 @@ static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
 }
 
 /*
+ * Stub implementations for outer i2c slave operations.
+ */
+void __i2c_force_reset_slave(void)
+{
+}
+void i2c_force_reset_slave(void)
+	__attribute__((weak, alias("__i2c_force_reset_slave")));
+
+/*
  * Stop I2C transaction
  */
-static void i2c_imx_stop(struct mxc_i2c_regs *i2c_regs)
+static void i2c_imx_stop(struct mxc_i2c_bus *i2c_bus)
 {
 	int ret;
-	unsigned int temp = readb(&i2c_regs->i2cr);
+	int reg_shift = i2c_bus->driver_data & I2C_QUIRK_FLAG ?
+			VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
+	ulong base = i2c_bus->base;
+	unsigned int temp = readb(base + (I2CR << reg_shift));
 
 	temp &= ~(I2CR_MSTA | I2CR_MTX);
-	writeb(temp, &i2c_regs->i2cr);
-	ret = wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
+	writeb(temp, base + (I2CR << reg_shift));
+	ret = wait_for_sr_state(i2c_bus, ST_BUS_IDLE);
 	if (ret < 0)
 		printf("%s:trigger stop failed\n", __func__);
 }
@@ -249,66 +266,96 @@ static void i2c_imx_stop(struct mxc_i2c_regs *i2c_regs)
  * Send start signal, chip address and
  * write register address
  */
-static int i2c_init_transfer_(struct mxc_i2c_regs *i2c_regs,
-		uchar chip, uint addr, int alen)
+static int i2c_init_transfer_(struct mxc_i2c_bus *i2c_bus, u8 chip,
+			      u32 addr, int alen)
 {
 	unsigned int temp;
 	int ret;
+	bool quirk = i2c_bus->driver_data & I2C_QUIRK_FLAG ? true : false;
+	ulong base = i2c_bus->base;
+	int reg_shift = quirk ? VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
+
+	/* Reset i2c slave */
+	i2c_force_reset_slave();
 
 	/* Enable I2C controller */
-#ifdef I2C_QUIRK_REG
-	if (readb(&i2c_regs->i2cr) & I2CR_IDIS) {
-#else
-	if (!(readb(&i2c_regs->i2cr) & I2CR_IEN)) {
-#endif
-		writeb(I2CR_IEN, &i2c_regs->i2cr);
+	if (quirk)
+		ret = readb(base + (I2CR << reg_shift)) & I2CR_IDIS;
+	else
+		ret = !(readb(base + (I2CR << reg_shift)) & I2CR_IEN);
+
+	if (ret) {
+		writeb(I2CR_IEN, base + (I2CR << reg_shift));
 		/* Wait for controller to be stable */
 		udelay(50);
 	}
-	if (readb(&i2c_regs->iadr) == (chip << 1))
-		writeb((chip << 1) ^ 2, &i2c_regs->iadr);
-	writeb(I2SR_IIF_CLEAR, &i2c_regs->i2sr);
-	ret = wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
+
+	if (readb(base + (IADR << reg_shift)) == (chip << 1))
+		writeb((chip << 1) ^ 2, base + (IADR << reg_shift));
+	writeb(I2SR_IIF_CLEAR, base + (I2SR << reg_shift));
+	ret = wait_for_sr_state(i2c_bus, ST_BUS_IDLE);
 	if (ret < 0)
 		return ret;
 
 	/* Start I2C transaction */
-	temp = readb(&i2c_regs->i2cr);
+	temp = readb(base + (I2CR << reg_shift));
 	temp |= I2CR_MSTA;
-	writeb(temp, &i2c_regs->i2cr);
+	writeb(temp, base + (I2CR << reg_shift));
 
-	ret = wait_for_sr_state(i2c_regs, ST_BUS_BUSY);
+	ret = wait_for_sr_state(i2c_bus, ST_BUS_BUSY);
 	if (ret < 0)
 		return ret;
 
 	temp |= I2CR_MTX | I2CR_TX_NO_AK;
-	writeb(temp, &i2c_regs->i2cr);
+	writeb(temp, base + (I2CR << reg_shift));
 
 	/* write slave address */
-	ret = tx_byte(i2c_regs, chip << 1);
+	ret = tx_byte(i2c_bus, chip << 1);
 	if (ret < 0)
 		return ret;
 
 	while (alen--) {
-		ret = tx_byte(i2c_regs, (addr >> (alen * 8)) & 0xff);
+		ret = tx_byte(i2c_bus, (addr >> (alen * 8)) & 0xff);
 		if (ret < 0)
 			return ret;
 	}
 	return 0;
 }
 
-static int i2c_idle_bus(void *base);
+#ifndef CONFIG_DM_I2C
+int i2c_idle_bus(struct mxc_i2c_bus *i2c_bus)
+{
+	if (i2c_bus && i2c_bus->idle_bus_fn)
+		return i2c_bus->idle_bus_fn(i2c_bus->idle_bus_data);
+	return 0;
+}
+#else
+/*
+ * Since pinmux is not supported, implement a weak function here.
+ * You can implement your i2c_bus_idle in board file. When pinctrl
+ * is supported, this can be removed.
+ */
+int __i2c_idle_bus(struct mxc_i2c_bus *i2c_bus)
+{
+	return 0;
+}
 
-static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
-		uchar chip, uint addr, int alen)
+int i2c_idle_bus(struct mxc_i2c_bus *i2c_bus)
+	__attribute__((weak, alias("__i2c_idle_bus")));
+#endif
+
+static int i2c_init_transfer(struct mxc_i2c_bus *i2c_bus, u8 chip,
+			     u32 addr, int alen)
 {
 	int retry;
 	int ret;
+	int reg_shift = i2c_bus->driver_data & I2C_QUIRK_FLAG ?
+			VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
 	for (retry = 0; retry < 3; retry++) {
-		ret = i2c_init_transfer_(i2c_regs, chip, addr, alen);
+		ret = i2c_init_transfer_(i2c_bus, chip, addr, alen);
 		if (ret >= 0)
 			return 0;
-		i2c_imx_stop(i2c_regs);
+		i2c_imx_stop(i2c_bus);
 		if (ret == -ENODEV)
 			return ret;
 
@@ -316,54 +363,67 @@ static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
 				retry);
 		if (ret != -ERESTART)
 			/* Disable controller */
-			writeb(I2CR_IDIS, &i2c_regs->i2cr);
+			writeb(I2CR_IDIS, i2c_bus->base + (I2CR << reg_shift));
 		udelay(100);
-		if (i2c_idle_bus(i2c_regs) < 0)
+		if (i2c_idle_bus(i2c_bus) < 0)
 			break;
 	}
-	printf("%s: give up i2c_regs=%p\n", __func__, i2c_regs);
+	printf("%s: give up i2c_regs=0x%lx\n", __func__, i2c_bus->base);
 	return ret;
 }
 
-/*
- * Read data from I2C device
- */
-int bus_i2c_read(void *base, uchar chip, uint addr, int alen, uchar *buf,
-		int len)
+
+static int i2c_write_data(struct mxc_i2c_bus *i2c_bus, u8 chip, const u8 *buf,
+			  int len)
+{
+	int i, ret = 0;
+
+	debug("i2c_write_data: chip=0x%x, len=0x%x\n", chip, len);
+	debug("write_data: ");
+	/* use rc for counter */
+	for (i = 0; i < len; ++i)
+		debug(" 0x%02x", buf[i]);
+	debug("\n");
+
+	for (i = 0; i < len; i++) {
+		ret = tx_byte(i2c_bus, buf[i]);
+		if (ret < 0) {
+			debug("i2c_write_data(): rc=%d\n", ret);
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static int i2c_read_data(struct mxc_i2c_bus *i2c_bus, uchar chip, uchar *buf,
+			 int len)
 {
 	int ret;
 	unsigned int temp;
 	int i;
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
-
-	ret = i2c_init_transfer(i2c_regs, chip, addr, alen);
-	if (ret < 0)
-		return ret;
+	int reg_shift = i2c_bus->driver_data & I2C_QUIRK_FLAG ?
+			VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
+	ulong base = i2c_bus->base;
 
-	temp = readb(&i2c_regs->i2cr);
-	temp |= I2CR_RSTA;
-	writeb(temp, &i2c_regs->i2cr);
-
-	ret = tx_byte(i2c_regs, (chip << 1) | 1);
-	if (ret < 0) {
-		i2c_imx_stop(i2c_regs);
-		return ret;
-	}
+	debug("i2c_read_data: chip=0x%x, len=0x%x\n", chip, len);
 
 	/* setup bus to read data */
-	temp = readb(&i2c_regs->i2cr);
+	temp = readb(base + (I2CR << reg_shift));
 	temp &= ~(I2CR_MTX | I2CR_TX_NO_AK);
 	if (len == 1)
 		temp |= I2CR_TX_NO_AK;
-	writeb(temp, &i2c_regs->i2cr);
-	writeb(I2SR_IIF_CLEAR, &i2c_regs->i2sr);
-	readb(&i2c_regs->i2dr);		/* dummy read to clear ICF */
+	writeb(temp, base + (I2CR << reg_shift));
+	writeb(I2SR_IIF_CLEAR, base + (I2SR << reg_shift));
+	/* dummy read to clear ICF */
+	readb(base + (I2DR << reg_shift));
 
 	/* read data */
 	for (i = 0; i < len; i++) {
-		ret = wait_for_sr_state(i2c_regs, ST_IIF);
+		ret = wait_for_sr_state(i2c_bus, ST_IIF);
 		if (ret < 0) {
-			i2c_imx_stop(i2c_regs);
+			debug("i2c_read_data(): ret=%d\n", ret);
+			i2c_imx_stop(i2c_bus);
 			return ret;
 		}
 
@@ -372,105 +432,111 @@ int bus_i2c_read(void *base, uchar chip, uint addr, int alen, uchar *buf,
 		 * controller from generating another clock cycle
 		 */
 		if (i == (len - 1)) {
-			i2c_imx_stop(i2c_regs);
+			i2c_imx_stop(i2c_bus);
 		} else if (i == (len - 2)) {
-			temp = readb(&i2c_regs->i2cr);
+			temp = readb(base + (I2CR << reg_shift));
 			temp |= I2CR_TX_NO_AK;
-			writeb(temp, &i2c_regs->i2cr);
+			writeb(temp, base + (I2CR << reg_shift));
 		}
-		writeb(I2SR_IIF_CLEAR, &i2c_regs->i2sr);
-		buf[i] = readb(&i2c_regs->i2dr);
+		writeb(I2SR_IIF_CLEAR, base + (I2SR << reg_shift));
+		buf[i] = readb(base + (I2DR << reg_shift));
 	}
-	i2c_imx_stop(i2c_regs);
+
+	/* reuse ret for counter*/
+	for (ret = 0; ret < len; ++ret)
+		debug(" 0x%02x", buf[ret]);
+	debug("\n");
+
+	i2c_imx_stop(i2c_bus);
 	return 0;
 }
 
+#ifndef CONFIG_DM_I2C
 /*
- * Write data to I2C device
+ * Read data from I2C device
  */
-int bus_i2c_write(void *base, uchar chip, uint addr, int alen,
-		const uchar *buf, int len)
+static int bus_i2c_read(struct mxc_i2c_bus *i2c_bus, u8 chip, u32 addr,
+			int alen, u8 *buf, int len)
 {
-	int ret;
-	int i;
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
+	int ret = 0;
+	u32 temp;
+	int reg_shift = i2c_bus->driver_data & I2C_QUIRK_FLAG ?
+		VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
+	ulong base = i2c_bus->base;
 
-	ret = i2c_init_transfer(i2c_regs, chip, addr, alen);
+	ret = i2c_init_transfer(i2c_bus, chip, addr, alen);
 	if (ret < 0)
 		return ret;
 
-	for (i = 0; i < len; i++) {
-		ret = tx_byte(i2c_regs, buf[i]);
-		if (ret < 0)
-			break;
+	temp = readb(base + (I2CR << reg_shift));
+	temp |= I2CR_RSTA;
+	writeb(temp, base + (I2CR << reg_shift));
+
+	ret = tx_byte(i2c_bus, (chip << 1) | 1);
+	if (ret < 0) {
+		i2c_imx_stop(i2c_bus);
+		return ret;
 	}
-	i2c_imx_stop(i2c_regs);
+
+	ret = i2c_read_data(i2c_bus, chip, buf, len);
+
+	i2c_imx_stop(i2c_bus);
+	return ret;
+}
+
+/*
+ * Write data to I2C device
+ */
+static int bus_i2c_write(struct mxc_i2c_bus *i2c_bus, u8 chip, u32 addr,
+			 int alen, const u8 *buf, int len)
+{
+	int ret = 0;
+
+	ret = i2c_init_transfer(i2c_bus, chip, addr, alen);
+	if (ret < 0)
+		return ret;
+
+	ret = i2c_write_data(i2c_bus, chip, buf, len);
+
+	i2c_imx_stop(i2c_bus);
+
 	return ret;
 }
 
-static void * const i2c_bases[] = {
+static struct mxc_i2c_bus mxc_i2c_buses[] = {
 #if defined(CONFIG_MX25)
-	(void *)IMX_I2C_BASE,
-	(void *)IMX_I2C2_BASE,
-	(void *)IMX_I2C3_BASE
+	{ 0, IMX_I2C_BASE },
+	{ 1, IMX_I2C2_BASE },
+	{ 2, IMX_I2C3_BASE },
 #elif defined(CONFIG_MX27)
-	(void *)IMX_I2C1_BASE,
-	(void *)IMX_I2C2_BASE
+	{ 0, IMX_I2C1_BASE },
+	{ 1, IMX_I2C2_BASE },
 #elif defined(CONFIG_MX31) || defined(CONFIG_MX35) || \
 	defined(CONFIG_MX51) || defined(CONFIG_MX53) ||	\
-	defined(CONFIG_MX6) || defined(CONFIG_LS102XA)
-	(void *)I2C1_BASE_ADDR,
-	(void *)I2C2_BASE_ADDR,
-	(void *)I2C3_BASE_ADDR
+	defined(CONFIG_MX6)
+	{ 0, I2C1_BASE_ADDR },
+	{ 1, I2C2_BASE_ADDR },
+	{ 2, I2C3_BASE_ADDR },
+#elif defined(CONFIG_LS102XA)
+	{ 0, I2C1_BASE_ADDR, I2C_QUIRK_FLAG },
+	{ 1, I2C2_BASE_ADDR, I2C_QUIRK_FLAG },
+	{ 2, I2C3_BASE_ADDR, I2C_QUIRK_FLAG },
 #elif defined(CONFIG_VF610)
-	(void *)I2C0_BASE_ADDR
+	{ 0, I2C0_BASE_ADDR, I2C_QUIRK_FLAG },
 #elif defined(CONFIG_FSL_LSCH3)
-	(void *)I2C1_BASE_ADDR,
-	(void *)I2C2_BASE_ADDR,
-	(void *)I2C3_BASE_ADDR,
-	(void *)I2C4_BASE_ADDR
+	{ 0, I2C1_BASE_ADDR, I2C_QUIRK_FLAG },
+	{ 1, I2C2_BASE_ADDR, I2C_QUIRK_FLAG },
+	{ 2, I2C3_BASE_ADDR, I2C_QUIRK_FLAG },
+	{ 3, I2C4_BASE_ADDR, I2C_QUIRK_FLAG },
 #else
 #error "architecture not supported"
 #endif
+	{ }
 };
 
-struct i2c_parms {
-	void *base;
-	void *idle_bus_data;
-	int (*idle_bus_fn)(void *p);
-};
-
-struct sram_data {
-	unsigned curr_i2c_bus;
-	struct i2c_parms i2c_data[ARRAY_SIZE(i2c_bases)];
-};
-
-void *i2c_get_base(struct i2c_adapter *adap)
-{
-	return i2c_bases[adap->hwadapnr];
-}
-
-static struct i2c_parms *i2c_get_parms(void *base)
-{
-	struct sram_data *srdata = (void *)gd->srdata;
-	int i = 0;
-	struct i2c_parms *p = srdata->i2c_data;
-	while (i < ARRAY_SIZE(srdata->i2c_data)) {
-		if (p->base == base)
-			return p;
-		p++;
-		i++;
-	}
-	printf("Invalid I2C base: %p\n", base);
-	return NULL;
-}
-
-static int i2c_idle_bus(void *base)
+struct mxc_i2c_bus *i2c_get_base(struct i2c_adapter *adap)
 {
-	struct i2c_parms *p = i2c_get_parms(base);
-	if (p && p->idle_bus_fn)
-		return p->idle_bus_fn(p->idle_bus_data);
-	return 0;
+	return &mxc_i2c_buses[adap->hwadapnr];
 }
 
 static int mxc_i2c_read(struct i2c_adapter *adap, uint8_t chip,
@@ -495,29 +561,33 @@ static int mxc_i2c_probe(struct i2c_adapter *adap, uint8_t chip)
 	return bus_i2c_write(i2c_get_base(adap), chip, 0, 0, NULL, 0);
 }
 
-void bus_i2c_init(void *base, int speed, int unused,
-		int (*idle_bus_fn)(void *p), void *idle_bus_data)
+int __enable_i2c_clk(unsigned char enable, unsigned i2c_num)
+{
+	return 1;
+}
+int enable_i2c_clk(unsigned char enable, unsigned i2c_num)
+	__attribute__((weak, alias("__enable_i2c_clk")));
+
+void bus_i2c_init(int index, int speed, int unused,
+		  int (*idle_bus_fn)(void *p), void *idle_bus_data)
 {
-	struct sram_data *srdata = (void *)gd->srdata;
-	int i = 0;
-	struct i2c_parms *p = srdata->i2c_data;
-	if (!base)
+	int ret;
+
+	if (index >= ARRAY_SIZE(mxc_i2c_buses)) {
+		debug("Error i2c index\n");
 		return;
-	for (;;) {
-		if (!p->base || (p->base == base)) {
-			p->base = base;
-			if (idle_bus_fn) {
-				p->idle_bus_fn = idle_bus_fn;
-				p->idle_bus_data = idle_bus_data;
-			}
-			break;
-		}
-		p++;
-		i++;
-		if (i >= ARRAY_SIZE(srdata->i2c_data))
-			return;
 	}
-	bus_i2c_set_bus_speed(base, speed);
+
+	mxc_i2c_buses[index].idle_bus_fn = idle_bus_fn;
+	mxc_i2c_buses[index].idle_bus_data = idle_bus_data;
+
+	ret = enable_i2c_clk(1, index);
+	if (ret < 0) {
+		debug("I2C-%d clk fail to enable.\n", index);
+		return;
+	}
+
+	bus_i2c_set_bus_speed(&mxc_i2c_buses[index], speed);
 }
 
 /*
@@ -525,13 +595,13 @@ void bus_i2c_init(void *base, int speed, int unused,
  */
 static void mxc_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr)
 {
-	bus_i2c_init(i2c_get_base(adap), speed, slaveaddr, NULL, NULL);
+	bus_i2c_init(adap->hwadapnr, speed, slaveaddr, NULL, NULL);
 }
 
 /*
  * Set I2C Speed
  */
-static uint mxc_i2c_set_bus_speed(struct i2c_adapter *adap, uint speed)
+static u32 mxc_i2c_set_bus_speed(struct i2c_adapter *adap, uint speed)
 {
 	return bus_i2c_set_bus_speed(i2c_get_base(adap), speed);
 }
@@ -556,6 +626,7 @@ U_BOOT_I2C_ADAP_COMPLETE(mxc2, mxc_i2c_init, mxc_i2c_probe,
 			 CONFIG_SYS_MXC_I2C3_SPEED,
 			 CONFIG_SYS_MXC_I2C3_SLAVE, 2)
 #endif
+
 #ifdef CONFIG_SYS_I2C_MXC_I2C4
 U_BOOT_I2C_ADAP_COMPLETE(mxc3, mxc_i2c_init, mxc_i2c_probe,
 			 mxc_i2c_read, mxc_i2c_write,
@@ -563,3 +634,143 @@ U_BOOT_I2C_ADAP_COMPLETE(mxc3, mxc_i2c_init, mxc_i2c_probe,
 			 CONFIG_SYS_MXC_I2C4_SPEED,
 			 CONFIG_SYS_MXC_I2C4_SLAVE, 3)
 #endif
+
+#else
+
+static int mxc_i2c_set_bus_speed(struct udevice *bus, unsigned int speed)
+{
+	struct mxc_i2c_bus *i2c_bus = dev_get_priv(bus);
+
+	return bus_i2c_set_bus_speed(i2c_bus, speed);
+}
+
+static int mxc_i2c_probe(struct udevice *bus)
+{
+	struct mxc_i2c_bus *i2c_bus = dev_get_priv(bus);
+	fdt_addr_t addr;
+	int ret;
+
+	i2c_bus->driver_data = dev_get_driver_data(bus);
+
+	addr = dev_get_addr(bus);
+	if (addr == FDT_ADDR_T_NONE)
+		return -ENODEV;
+
+	i2c_bus->base = addr;
+	i2c_bus->index = bus->seq;
+
+	/* Enable clk */
+	ret = enable_i2c_clk(1, bus->seq);
+	if (ret < 0)
+		return ret;
+
+	ret = i2c_idle_bus(i2c_bus);
+	if (ret < 0) {
+		/* Disable clk */
+		enable_i2c_clk(0, bus->seq);
+		return ret;
+	}
+
+	/*
+	 * Pinmux settings are in board file now, until pinmux is supported,
+	 * we can set pinmux here in probe function.
+	 */
+
+	debug("i2c : controller bus %d at %lu , speed %d: ",
+	      bus->seq, i2c_bus->base,
+	      i2c_bus->speed);
+
+	return 0;
+}
+
+static int mxc_i2c_probe_chip(struct udevice *bus, u32 chip_addr,
+			      u32 chip_flags)
+{
+	int ret;
+	struct mxc_i2c_bus *i2c_bus = dev_get_priv(bus);
+
+	ret = i2c_init_transfer(i2c_bus, chip_addr, 0, 0);
+	if (ret < 0) {
+		debug("%s failed, ret = %d\n", __func__, ret);
+		return ret;
+	}
+
+	i2c_imx_stop(i2c_bus);
+
+	return 0;
+}
+
+static int mxc_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, int nmsgs)
+{
+	struct mxc_i2c_bus *i2c_bus = dev_get_priv(bus);
+	int ret = 0;
+	ulong base = i2c_bus->base;
+	int reg_shift = i2c_bus->driver_data & I2C_QUIRK_FLAG ?
+		VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
+
+	/*
+	 * Here the 3rd parameter addr and the 4th one alen are set to 0,
+	 * because here we only want to send out chip address. The register
+	 * address is wrapped in msg.
+	 */
+	ret = i2c_init_transfer(i2c_bus, msg->addr, 0, 0);
+	if (ret < 0) {
+		debug("i2c_init_transfer error: %d\n", ret);
+		return ret;
+	}
+
+	for (; nmsgs > 0; nmsgs--, msg++) {
+		bool next_is_read = nmsgs > 1 && (msg[1].flags & I2C_M_RD);
+		debug("i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len);
+		if (msg->flags & I2C_M_RD)
+			ret = i2c_read_data(i2c_bus, msg->addr, msg->buf,
+					    msg->len);
+		else {
+			ret = i2c_write_data(i2c_bus, msg->addr, msg->buf,
+					     msg->len);
+			if (ret)
+				break;
+			if (next_is_read) {
+				/* Reuse ret */
+				ret = readb(base + (I2CR << reg_shift));
+				ret |= I2CR_RSTA;
+				writeb(ret, base + (I2CR << reg_shift));
+
+				ret = tx_byte(i2c_bus, (msg->addr << 1) | 1);
+				if (ret < 0) {
+					i2c_imx_stop(i2c_bus);
+					break;
+				}
+			}
+		}
+	}
+
+	if (ret)
+		debug("i2c_write: error sending\n");
+
+	i2c_imx_stop(i2c_bus);
+
+	return ret;
+}
+
+static const struct dm_i2c_ops mxc_i2c_ops = {
+	.xfer		= mxc_i2c_xfer,
+	.probe_chip	= mxc_i2c_probe_chip,
+	.set_bus_speed	= mxc_i2c_set_bus_speed,
+};
+
+static const struct udevice_id mxc_i2c_ids[] = {
+	{ .compatible = "fsl,imx21-i2c", },
+	{ .compatible = "fsl,vf610-i2c", .data = I2C_QUIRK_FLAG, },
+	{}
+};
+
+U_BOOT_DRIVER(i2c_mxc) = {
+	.name = "i2c_mxc",
+	.id = UCLASS_I2C,
+	.of_match = mxc_i2c_ids,
+	.probe = mxc_i2c_probe,
+	.priv_auto_alloc_size = sizeof(struct mxc_i2c_bus),
+	.ops = &mxc_i2c_ops,
+};
+#endif
-- 
1.8.4

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

* [U-Boot] [PATCH V4 2/2] imx: mx27 remove redundant macro
  2015-05-14 10:03 [U-Boot] [PATCH V4 1/2] i2c: mxc: refactor i2c driver and support dm Peng Fan
@ 2015-05-14 10:03 ` Peng Fan
  2015-05-14 15:01   ` Simon Glass
  2015-05-14 15:31   ` Fabio Estevam
  2015-05-14 15:01 ` [U-Boot] [PATCH V4 1/2] i2c: mxc: refactor i2c driver and support dm Simon Glass
  1 sibling, 2 replies; 7+ messages in thread
From: Peng Fan @ 2015-05-14 10:03 UTC (permalink / raw)
  To: u-boot

Use common macro in iomux-v3.h, remove redundant macro.

Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
---

Changes v4:
 New patch.
 we include mxc_i2c.h in driver/i2c/mxc_i2c.c in patch 1/2.
 mxc_i2c.h includes iomux-v3.h.
 Since iomux-v3.h have some macros which also exists in
 asm/arch-xx/imx-regs.h, this will introudce compile warnings
 such as "redefined macro".

Changes v3:
 none

Changes v2:
 none

 arch/arm/cpu/arm926ejs/mx27/generic.c      |  1 +
 arch/arm/include/asm/arch-mx27/imx-regs.h  | 22 ----------------------
 arch/arm/include/asm/imx-common/iomux-v3.h | 22 ++++++++++++++++------
 board/armadeus/apf27/apf27.c               |  1 +
 board/armadeus/apf27/fpga.c                |  1 +
 board/logicpd/imx27lite/imx27lite.c        |  1 +
 6 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/arch/arm/cpu/arm926ejs/mx27/generic.c b/arch/arm/cpu/arm926ejs/mx27/generic.c
index 5ee9f07..53d52e5 100644
--- a/arch/arm/cpu/arm926ejs/mx27/generic.c
+++ b/arch/arm/cpu/arm926ejs/mx27/generic.c
@@ -12,6 +12,7 @@
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/gpio.h>
+#include <asm/imx-common/iomux-v3.h>
 #ifdef CONFIG_MXC_MMC
 #include <asm/arch/mxcmmc.h>
 #endif
diff --git a/arch/arm/include/asm/arch-mx27/imx-regs.h b/arch/arm/include/asm/arch-mx27/imx-regs.h
index 92c847e..7402e31 100644
--- a/arch/arm/include/asm/arch-mx27/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx27/imx-regs.h
@@ -138,16 +138,6 @@ struct gpt_regs {
 	u32 gpt_tstat;
 };
 
-/*
- *  GPIO Module and I/O Multiplexer
- */
-#define PORTA 0
-#define PORTB 1
-#define PORTC 2
-#define PORTD 3
-#define PORTE 4
-#define PORTF 5
-
 /* IIM Control Registers */
 struct iim_regs {
 	u32 iim_stat;
@@ -449,18 +439,6 @@ struct fuse_bank0_regs {
 #define GPIO5_BASE_ADDR 0x10015400
 #define GPIO6_BASE_ADDR 0x10015500
 
-#define GPIO_PIN_MASK	0x1f
-
-#define GPIO_PORT_SHIFT	5
-#define GPIO_PORT_MASK	(0x7 << GPIO_PORT_SHIFT)
-
-#define GPIO_PORTA	(PORTA << GPIO_PORT_SHIFT)
-#define GPIO_PORTB	(PORTB << GPIO_PORT_SHIFT)
-#define GPIO_PORTC	(PORTC << GPIO_PORT_SHIFT)
-#define GPIO_PORTD	(PORTD << GPIO_PORT_SHIFT)
-#define GPIO_PORTE	(PORTE << GPIO_PORT_SHIFT)
-#define GPIO_PORTF	(PORTF << GPIO_PORT_SHIFT)
-
 #define GPIO_OUT	(1 << 8)
 #define GPIO_IN		(0 << 8)
 #define GPIO_PUEN	(1 << 9)
diff --git a/arch/arm/include/asm/imx-common/iomux-v3.h b/arch/arm/include/asm/imx-common/iomux-v3.h
index e0a49be..086486c 100644
--- a/arch/arm/include/asm/imx-common/iomux-v3.h
+++ b/arch/arm/include/asm/imx-common/iomux-v3.h
@@ -169,15 +169,25 @@ typedef u64 iomux_v3_cfg_t;
 
 #define IOMUX_CONFIG_SION	0x10
 
+/*
+ *  GPIO Module and I/O Multiplexer
+ */
+#define PORTA 0
+#define PORTB 1
+#define PORTC 2
+#define PORTD 3
+#define PORTE 4
+#define PORTF 5
+
 #define GPIO_PIN_MASK		0x1f
 #define GPIO_PORT_SHIFT		5
 #define GPIO_PORT_MASK		(0x7 << GPIO_PORT_SHIFT)
-#define GPIO_PORTA		(0 << GPIO_PORT_SHIFT)
-#define GPIO_PORTB		(1 << GPIO_PORT_SHIFT)
-#define GPIO_PORTC		(2 << GPIO_PORT_SHIFT)
-#define GPIO_PORTD		(3 << GPIO_PORT_SHIFT)
-#define GPIO_PORTE		(4 << GPIO_PORT_SHIFT)
-#define GPIO_PORTF		(5 << GPIO_PORT_SHIFT)
+#define GPIO_PORTA		(PORTA << GPIO_PORT_SHIFT)
+#define GPIO_PORTB		(PORTB << GPIO_PORT_SHIFT)
+#define GPIO_PORTC		(PORTC << GPIO_PORT_SHIFT)
+#define GPIO_PORTD		(PORTD << GPIO_PORT_SHIFT)
+#define GPIO_PORTE		(PORTE << GPIO_PORT_SHIFT)
+#define GPIO_PORTF		(PORTF << GPIO_PORT_SHIFT)
 
 void imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad);
 void imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list,
diff --git a/board/armadeus/apf27/apf27.c b/board/armadeus/apf27/apf27.c
index 30e720d..f718e5c 100644
--- a/board/armadeus/apf27/apf27.c
+++ b/board/armadeus/apf27/apf27.c
@@ -17,6 +17,7 @@
 #include <asm/arch/gpio.h>
 #include <asm/gpio.h>
 #include <asm/errno.h>
+#include <asm/imx-common/iomux-v3.h>
 #include "apf27.h"
 #include "crc.h"
 #include "fpga.h"
diff --git a/board/armadeus/apf27/fpga.c b/board/armadeus/apf27/fpga.c
index 65a4812..af68244 100644
--- a/board/armadeus/apf27/fpga.c
+++ b/board/armadeus/apf27/fpga.c
@@ -13,6 +13,7 @@
 
 #include <asm/arch/imx-regs.h>
 #include <asm/gpio.h>
+#include <asm/imx-common/iomux-v3.h>
 #include <asm/io.h>
 #include <command.h>
 #include <config.h>
diff --git a/board/logicpd/imx27lite/imx27lite.c b/board/logicpd/imx27lite/imx27lite.c
index 07b07a0..576187b 100644
--- a/board/logicpd/imx27lite/imx27lite.c
+++ b/board/logicpd/imx27lite/imx27lite.c
@@ -10,6 +10,7 @@
 #include <asm/io.h>
 #include <asm/arch/imx-regs.h>
 #include <asm/gpio.h>
+#include <asm/imx-common/iomux-v3.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-- 
1.8.4

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

* [U-Boot] [PATCH V4 2/2] imx: mx27 remove redundant macro
  2015-05-14 10:03 ` [U-Boot] [PATCH V4 2/2] imx: mx27 remove redundant macro Peng Fan
@ 2015-05-14 15:01   ` Simon Glass
  2015-05-14 15:31   ` Fabio Estevam
  1 sibling, 0 replies; 7+ messages in thread
From: Simon Glass @ 2015-05-14 15:01 UTC (permalink / raw)
  To: u-boot

On 14 May 2015 at 04:03, Peng Fan <Peng.Fan@freescale.com> wrote:
> Use common macro in iomux-v3.h, remove redundant macro.
>
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
> ---
>
> Changes v4:
>  New patch.
>  we include mxc_i2c.h in driver/i2c/mxc_i2c.c in patch 1/2.
>  mxc_i2c.h includes iomux-v3.h.
>  Since iomux-v3.h have some macros which also exists in
>  asm/arch-xx/imx-regs.h, this will introudce compile warnings
>  such as "redefined macro".
>
> Changes v3:
>  none
>
> Changes v2:
>  none
>
>  arch/arm/cpu/arm926ejs/mx27/generic.c      |  1 +
>  arch/arm/include/asm/arch-mx27/imx-regs.h  | 22 ----------------------
>  arch/arm/include/asm/imx-common/iomux-v3.h | 22 ++++++++++++++++------
>  board/armadeus/apf27/apf27.c               |  1 +
>  board/armadeus/apf27/fpga.c                |  1 +
>  board/logicpd/imx27lite/imx27lite.c        |  1 +
>  6 files changed, 20 insertions(+), 28 deletions(-)

Applied to u-boot-dm, thanks!

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

* [U-Boot] [PATCH V4 1/2] i2c: mxc: refactor i2c driver and support dm
  2015-05-14 10:03 [U-Boot] [PATCH V4 1/2] i2c: mxc: refactor i2c driver and support dm Peng Fan
  2015-05-14 10:03 ` [U-Boot] [PATCH V4 2/2] imx: mx27 remove redundant macro Peng Fan
@ 2015-05-14 15:01 ` Simon Glass
  1 sibling, 0 replies; 7+ messages in thread
From: Simon Glass @ 2015-05-14 15:01 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On 14 May 2015 at 04:03, Peng Fan <Peng.Fan@freescale.com> wrote:
> 1. Introduce a new structure `struct mxc_i2c_bus`, this structure will
>    used for non-DM and DM.
> 2. Remove `struct mxc_i2c_regs` structure, but use register offset to access
>    registers based on `base` entry of `struct mxc_i2c_bus`.
> 3. Remove most `#ifdef I2C_QUIRK_REG`. Using driver_data to contain platform
>    flags. A new flag is introduced, I2C_QUIRK_FLAG.
> 4. Most functions use `struct mxc_i2c_bus` as one of the parameters.
>    Make most functions common to DM and non-DM, try to avoid duplicated code.
> 5. Support DM, but pinctrl is not included. Pinmux setting is still set
>    by setup_i2c, but we do not need bus_i2c_init for DM.
> 6. struct i2c_parms and struct sram_data are removed.
> 7. Remove bus_i2c_read bus_i2c_write prototype in header file. The frist
>    paramter of bus_i2c_init is modified to i2c index. Add new prototype
>    i2c_idle_bus and force_bus_idle. Since bus_i2c_init is not good for
>    DM I2C and pinctrl is missed, we use a weak function for i2c_idle_bus.
>    Board file take the responsibility to implement this function, like this:
>    "
>    int i2c_idle_bus(struct mxc_i2c_bus *i2c_bus)
>    {
>            if (i2c_bus->index == 0)
>                    force_bus_idle(i2c_pads_info0);
>            else if (i2c_bus->index == 1)
>                    force_bus_idle(i2c_pads_info1);
>            else
>                    xxxxxx
>    }
>    "
> 8. Introduce a weak function, enable_i2c_clk
> 9. Tested on an i.MX7 platform. Log info:
>  => dm tree
>  Class       Probed   Name
>  ----------------------------------------
>  root        [ + ]    root_driver
>  simple_bus  [   ]    |-- soc
>  simple_bus  [   ]    |   |-- aips-bus at 30000000
>  simple_bus  [   ]    |   |   |-- anatop at 30360000
>  simple_bus  [   ]    |   |   `-- snvs at 30370000
>  simple_bus  [   ]    |   |-- aips-bus at 30400000
>  simple_bus  [   ]    |   `-- aips-bus at 30800000
>  i2c         [   ]    |       |-- i2c at 30a20000
>  i2c         [   ]    |       `-- i2c at 30a40000
>  simple_bus  [   ]    `-- regulators
>  => i2c dev 0
>  Setting bus to 0
>  => i2c probe
>  Valid chip addresses: 08 50
>  => i2c md 8 31
>  0031: 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
>
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
> Acked-by: Simon Glass <sjg@chromium.org>
> ---
>
>  Changes v4:
>  1. fix build errors
>  2. Introduce a weak function enable_i2c_clk.
>  3. add Simon's Acked-by.
>  4. Add test log in commit log.

I still get one build break here, fixed by the second commit:

02: i2c: mxc: refactor i2c driver and support dm
       arm:  +   apf27
+In file included from ../arch/arm/include/asm/imx-common/mxc_i2c.h:8:0,
+                 from ../drivers/i2c/mxc_i2c.c:21:
w+../arch/arm/include/asm/imx-common/iomux-v3.h:175:0: warning:
"GPIO_PORTA" redefined [enabled by default]
w+In file included from ../drivers/i2c/mxc_i2c.c:19:0:
w+include/asm/arch/imx-regs.h:457:0: note: this is the location of the
previous definition
w+../arch/arm/include/asm/imx-common/iomux-v3.h:176:0: warning:
"GPIO_PORTB" redefined [enabled by default]
w+include/asm/arch/imx-regs.h:458:0: note: this is the location of the
previous definition
w+../arch/arm/include/asm/imx-common/iomux-v3.h:177:0: warning:
"GPIO_PORTC" redefined [enabled by default]
w+include/asm/arch/imx-regs.h:459:0: note: this is the location of the
previous definition
w+../arch/arm/include/asm/imx-common/iomux-v3.h:178:0: warning:
"GPIO_PORTD" redefined [enabled by default]
w+include/asm/arch/imx-regs.h:460:0: note: this is the location of the
previous definition
w+../arch/arm/include/asm/imx-common/iomux-v3.h:179:0: warning:
"GPIO_PORTE" redefined [enabled by default]
w+include/asm/arch/imx-regs.h:461:0: note: this is the location of the
previous definition
w+../arch/arm/include/asm/imx-common/iomux-v3.h:180:0: warning:
"GPIO_PORTF" redefined [enabled by default]
w+include/asm/arch/imx-regs.h:462:0: note: this is the location of the
previous definition
03: imx: mx27 remove redundant macro
       arm:     apf27
-In file included from ../arch/arm/include/asm/imx-common/mxc_i2c.h:8:0,
-                 from ../drivers/i2c/mxc_i2c.c:21:
w-../arch/arm/include/asm/imx-common/iomux-v3.h:175:0: warning:
"GPIO_PORTA" redefined [enabled by default]
w-In file included from ../drivers/i2c/mxc_i2c.c:19:0:
w-include/asm/arch/imx-regs.h:457:0: note: this is the location of the
previous definition
w-../arch/arm/include/asm/imx-common/iomux-v3.h:176:0: warning:
"GPIO_PORTB" redefined [enabled by default]
w-include/asm/arch/imx-regs.h:458:0: note: this is the location of the
previous definition
w-../arch/arm/include/asm/imx-common/iomux-v3.h:177:0: warning:
"GPIO_PORTC" redefined [enabled by default]
w-include/asm/arch/imx-regs.h:459:0: note: this is the location of the
previous definition
w-../arch/arm/include/asm/imx-common/iomux-v3.h:178:0: warning:
"GPIO_PORTD" redefined [enabled by default]
w-include/asm/arch/imx-regs.h:460:0: note: this is the location of the
previous definition
w-../arch/arm/include/asm/imx-common/iomux-v3.h:179:0: warning:
"GPIO_PORTE" redefined [enabled by default]
w-include/asm/arch/imx-regs.h:461:0: note: this is the location of the
previous definition
w-../arch/arm/include/asm/imx-common/iomux-v3.h:180:0: warning:
"GPIO_PORTF" redefined [enabled by default]
w-include/asm/arch/imx-regs.h:462:0: note: this is the location of the
previous definition

I'll apply them in the other order.

>
>  Changes v3:
>   1. remove bus_i2c_init for DM, introuduce a weak function i2c_idle_bus.
>   2. remove static return type for force_idle_bus, since we need to call
>      it in i2c_idle_bus which may be implemented in board file. This does
>      not hurt for non-DM.
>
>  Changes v2:
>   1. Refactor driver, remove register access based on structure, but use
>     'base + offset'
>   2. Introduce mxc_i2c_bus structure
>   3. Introduce I2C_QUIRK_FLAG and remove most I2C_QUIRK_REG and use
>      driver_data to contain the flags for different platforms
>   4.  Avoid duplicated code between DM and non-DM part
>   5. The function name i2c_init_transfer is not changed.
>   6. Remove bus_i2c_read/write prototype from header file
>   7. change bus_i2c_init's first parameter to i2c index
>   8. Rename patch name, since refactor non-DM part.
>
>  arch/arm/imx-common/i2c-mxv7.c            |   7 +-
>  arch/arm/include/asm/imx-common/mxc_i2c.h |  38 +-
>  drivers/i2c/mxc_i2c.c                     | 581 ++++++++++++++++++++----------
>  3 files changed, 433 insertions(+), 193 deletions(-)
>

[snip]

Applied to u-boot-dm, thanks!

Regards,
Simon

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

* [U-Boot] [PATCH V4 2/2] imx: mx27 remove redundant macro
  2015-05-14 10:03 ` [U-Boot] [PATCH V4 2/2] imx: mx27 remove redundant macro Peng Fan
  2015-05-14 15:01   ` Simon Glass
@ 2015-05-14 15:31   ` Fabio Estevam
  2015-05-14 16:31     ` Simon Glass
  1 sibling, 1 reply; 7+ messages in thread
From: Fabio Estevam @ 2015-05-14 15:31 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On Thu, May 14, 2015 at 7:03 AM, Peng Fan <Peng.Fan@freescale.com> wrote:
> Use common macro in iomux-v3.h, remove redundant macro.

This is a bit misleading.

mx27 iomux is not compatible to iomux-v3.h at all.

Regards,

Fabio Estevam

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

* [U-Boot] [PATCH V4 2/2] imx: mx27 remove redundant macro
  2015-05-14 15:31   ` Fabio Estevam
@ 2015-05-14 16:31     ` Simon Glass
  2015-05-14 23:19       ` Fan Peng
  0 siblings, 1 reply; 7+ messages in thread
From: Simon Glass @ 2015-05-14 16:31 UTC (permalink / raw)
  To: u-boot

Hi,

On 14 May 2015 at 09:31, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Peng,
>
> On Thu, May 14, 2015 at 7:03 AM, Peng Fan <Peng.Fan@freescale.com> wrote:
>> Use common macro in iomux-v3.h, remove redundant macro.
>
> This is a bit misleading.
>
> mx27 iomux is not compatible to iomux-v3.h at all.

If you'd like to respin this patch I can accept it up until tomorrow
morning, when I hope to send a pull request. If you would rather that
I drop this patch, then I can drop both (since the other one depends
on this one).

Regards,
Simon

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

* [U-Boot] [PATCH V4 2/2] imx: mx27 remove redundant macro
  2015-05-14 16:31     ` Simon Glass
@ 2015-05-14 23:19       ` Fan Peng
  0 siblings, 0 replies; 7+ messages in thread
From: Fan Peng @ 2015-05-14 23:19 UTC (permalink / raw)
  To: u-boot

Hi,

On 5/15/2015 12:31 AM, Simon Glass wrote:
> Hi,
>
> On 14 May 2015 at 09:31, Fabio Estevam <festevam@gmail.com> wrote:
>> Hi Peng,
>>
>> On Thu, May 14, 2015 at 7:03 AM, Peng Fan <Peng.Fan@freescale.com> wrote:
>>> Use common macro in iomux-v3.h, remove redundant macro.
>> This is a bit misleading.
>>
>> mx27 iomux is not compatible to iomux-v3.h at all.
Ok. I am not familiar with mx27. I'll move the GPIO_PORT[A,B,Cxxx] to 
gpio.h, but not in imx-regs.h for mx27.
> If you'd like to respin this patch I can accept it up until tomorrow
> morning, when I hope to send a pull request. If you would rather that
> I drop this patch, then I can drop both (since the other one depends
> on this one).
I'll respin this patch. If this delays your PR, you can drop them first. 
I'll  resend out patch v5 soon.
>
> Regards,
> Simon
Regards,
Peng.

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

end of thread, other threads:[~2015-05-14 23:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-14 10:03 [U-Boot] [PATCH V4 1/2] i2c: mxc: refactor i2c driver and support dm Peng Fan
2015-05-14 10:03 ` [U-Boot] [PATCH V4 2/2] imx: mx27 remove redundant macro Peng Fan
2015-05-14 15:01   ` Simon Glass
2015-05-14 15:31   ` Fabio Estevam
2015-05-14 16:31     ` Simon Glass
2015-05-14 23:19       ` Fan Peng
2015-05-14 15:01 ` [U-Boot] [PATCH V4 1/2] i2c: mxc: refactor i2c driver and support dm Simon Glass

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