Skip to contentSkip to conversation

i2c: imx: use struct representing i2c clk{div, val} pair

PATCH New Jingchang Lu 3 participants 11 messages Aug 7, 2013 – Aug 15, 2013

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/8 i2c: imx: use struct representing i2c clk{div, val} pair

using struct representing the i2c clk{div, val} pair would make the i2c_clk_div array more clear.

---
 drivers/i2c/busses/i2c-imx.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)
drivers/i2c/busses/i2c-imx.c +13 -7
index e242797..9167d43 100644
@@ -30,6 +30,8 @@
  *	Copyright (C) 2007 RightHand Technologies, Inc.
  *	Copyright (C) 2008 Darius Augulis <[email protected]>
  *
+ *	Copyright 2013 Freescale Semiconductor, Inc.
+ *
  */
 
 /** Includes *******************************************************************
@@ -95,8 +97,12 @@
  *
  * Duplicated divider values removed from list
  */
+struct imx_i2c_clk_pair {
+	u16	div;
+	u16	val;
+};
 
-static u16 __initdata i2c_clk_div[50][2] = {
+static struct imx_i2c_clk_pair __initdata i2c_clk_div[] = {
 	{ 22,	0x20 }, { 24,	0x21 }, { 26,	0x22 }, { 28,	0x23 },
 	{ 30,	0x00 },	{ 32,	0x24 }, { 36,	0x25 }, { 40,	0x26 },
 	{ 42,	0x03 }, { 44,	0x27 },	{ 48,	0x28 }, { 52,	0x05 },
@@ -274,15 +280,15 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
 	/* Divider value calculation */
 	i2c_clk_rate = clk_get_rate(i2c_imx->clk);
 	div = (i2c_clk_rate + rate - 1) / rate;
-	if (div < i2c_clk_div[0][0])
+	if (div < i2c_clk_div[0].div)
 		i = 0;
-	else if (div > i2c_clk_div[ARRAY_SIZE(i2c_clk_div) - 1][0])
+	else if (div > i2c_clk_div[ARRAY_SIZE(i2c_clk_div) - 1].div)
 		i = ARRAY_SIZE(i2c_clk_div) - 1;
 	else
-		for (i = 0; i2c_clk_div[i][0] < div; i++);
+		for (i = 0; i2c_clk_div[i].div < div; i++);
 
 	/* Store divider value */
-	i2c_imx->ifdr = i2c_clk_div[i][1];
+	i2c_imx->ifdr = i2c_clk_div[i].val;
 
 	/*
 	 * There dummy delay is calculated.
@@ -290,7 +296,7 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
 	 * This delay is used in I2C bus disable function
 	 * to fix chip hardware bug.
 	 */
-	i2c_imx->disable_delay = (500000U * i2c_clk_div[i][0]
+	i2c_imx->disable_delay = (500000U * i2c_clk_div[i].div
 		+ (i2c_clk_rate / 2) - 1) / (i2c_clk_rate / 2);
 
 	/* dev_dbg() can't be used, because adapter is not yet registered */
@@ -298,7 +304,7 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
 	dev_dbg(&i2c_imx->adapter.dev, "<%s> I2C_CLK=%d, REQ DIV=%d\n",
 		__func__, i2c_clk_rate, div);
 	dev_dbg(&i2c_imx->adapter.dev, "<%s> IFDR[IC]=0x%x, REAL DIV=%d\n",
-		__func__, i2c_clk_div[i][1], i2c_clk_div[i][0]);
+		__func__, i2c_clk_div[i].val, i2c_clk_div[i].div);
 #endif
 }
 
Signature · 1 line
1.8.0
#1 lore raw

PATCH 2/8 i2c: imx: enable clk before write to registers

The module clk should be enabled before write to its register in probe(), and the clk can be disabled after.

---
 drivers/i2c/busses/i2c-imx.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/i2c/busses/i2c-imx.c +6 -0
index 9167d43..7adb351 100644
@@ -543,6 +543,11 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 		return PTR_ERR(i2c_imx->clk);
 	}
 
+	ret = clk_prepare_enable(i2c_imx->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "can't enable I2C clock\n");
+		return ret;
+	}
 	/* Request IRQ */
 	ret = devm_request_irq(&pdev->dev, irq, i2c_imx_isr, 0,
 				pdev->name, i2c_imx);
@@ -580,6 +585,7 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 
 	/* Set up platform driver data */
 	platform_set_drvdata(pdev, i2c_imx);
+	clk_disable_unprepare(i2c_imx->clk);
 
 	dev_dbg(&i2c_imx->adapter.dev, "claimed irq %d\n", irq);
 	dev_dbg(&i2c_imx->adapter.dev, "device resources from 0x%x to 0x%x\n",
Signature · 1 line
1.8.0
#2 lore raw

PATCH 3/8 i2c: imx: don't change platform device id_entry directly

In reply to Jingchang Lu

The id_entry field should be changed by platform driver core, driver should prevent changing it derectly. Use local variable to save and extract platform_device_id info of the dts devices instead.

---
 drivers/i2c/busses/i2c-imx.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-imx.c +6 -2
index 7adb351..cbea84b 100644
@@ -499,6 +499,7 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 	struct imx_i2c_struct *i2c_imx;
 	struct resource *res;
 	struct imxi2c_platform_data *pdata = pdev->dev.platform_data;
+	const struct platform_device_id *imx_id;
 	void __iomem *base;
 	int irq, ret;
 	u32 bitrate;
@@ -524,8 +525,11 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 	}
 
 	if (of_id)
-		pdev->id_entry = of_id->data;
-	i2c_imx->devtype = pdev->id_entry->driver_data;
+		imx_id = of_id->data;
+	else
+		imx_id = platform_get_device_id(pdev);
+
+	i2c_imx->devtype = imx_id->driver_data;
 
 	/* Setup i2c_imx driver structure */
 	strlcpy(i2c_imx->adapter.name, pdev->name, sizeof(i2c_imx->adapter.name));
Signature · 1 line
1.8.0
#3 lore raw

PATCH 4/8 i2c: imx: wrap registers read/write to inline function

In reply to Jingchang Lu

wrap the readb(), writeb() into inline function calls. It would make the driver more clearer to support platform with different register offset.

---
 drivers/i2c/busses/i2c-imx.c | 80 +++++++++++++++++++++++++-------------------
 1 file changed, 46 insertions(+), 34 deletions(-)
drivers/i2c/busses/i2c-imx.c +46 -34
index cbea84b..1a3c608 100644
@@ -160,6 +160,18 @@ static inline int is_imx1_i2c(struct imx_i2c_struct *i2c_imx)
 	return i2c_imx->devtype == IMX1_I2C;
 }
 
+static inline void imx_i2c_write_reg(unsigned int val,
+		struct imx_i2c_struct *i2c_imx, unsigned int reg)
+{
+	writeb(val, i2c_imx->base + reg);
+}
+
+static inline unsigned char imx_i2c_read_reg(struct imx_i2c_struct *i2c_imx,
+		unsigned int reg)
+{
+	return readb(i2c_imx->base + reg);
+}
+
 /** Functions for IMX I2C adapter driver ***************************************
 *******************************************************************************/
 
@@ -171,7 +183,7 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy)
 	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
 
 	while (1) {
-		temp = readb(i2c_imx->base + IMX_I2C_I2SR);
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
 		if (for_busy && (temp & I2SR_IBB))
 			break;
 		if (!for_busy && !(temp & I2SR_IBB))
@@ -202,7 +214,7 @@ static int i2c_imx_trx_complete(struct imx_i2c_struct *i2c_imx)
 
 static int i2c_imx_acked(struct imx_i2c_struct *i2c_imx)
 {
-	if (readb(i2c_imx->base + IMX_I2C_I2SR) & I2SR_RXAK) {
+	if (imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR) & I2SR_RXAK) {
 		dev_dbg(&i2c_imx->adapter.dev, "<%s> No ACK\n", __func__);
 		return -EIO;  /* No ACK */
 	}
@@ -219,25 +231,25 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
 	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
 
 	clk_prepare_enable(i2c_imx->clk);
-	writeb(i2c_imx->ifdr, i2c_imx->base + IMX_I2C_IFDR);
+	imx_i2c_write_reg(i2c_imx->ifdr, i2c_imx, IMX_I2C_IFDR);
 	/* Enable I2C controller */
-	writeb(0, i2c_imx->base + IMX_I2C_I2SR);
-	writeb(I2CR_IEN, i2c_imx->base + IMX_I2C_I2CR);
+	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2SR);
+	imx_i2c_write_reg(I2CR_IEN, i2c_imx, IMX_I2C_I2CR);
 
 	/* Wait controller to be stable */
 	udelay(50);
 
 	/* Start I2C transaction */
-	temp = readb(i2c_imx->base + IMX_I2C_I2CR);
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
 	temp |= I2CR_MSTA;
-	writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
 	result = i2c_imx_bus_busy(i2c_imx, 1);
 	if (result)
 		return result;
 	i2c_imx->stopped = 0;
 
 	temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
-	writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
 	return result;
 }
 
@@ -248,9 +260,9 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
 	if (!i2c_imx->stopped) {
 		/* Stop I2C transaction */
 		dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
-		temp = readb(i2c_imx->base + IMX_I2C_I2CR);
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
 		temp &= ~(I2CR_MSTA | I2CR_MTX);
-		writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
 	}
 	if (is_imx1_i2c(i2c_imx)) {
 		/*
@@ -266,7 +278,7 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
 	}
 
 	/* Disable I2C controller */
-	writeb(0, i2c_imx->base + IMX_I2C_I2CR);
+	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2CR);
 	clk_disable_unprepare(i2c_imx->clk);
 }
 
@@ -313,12 +325,12 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
 	struct imx_i2c_struct *i2c_imx = dev_id;
 	unsigned int temp;
 
-	temp = readb(i2c_imx->base + IMX_I2C_I2SR);
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
 	if (temp & I2SR_IIF) {
 		/* save status register */
 		i2c_imx->i2csr = temp;
 		temp &= ~I2SR_IIF;
-		writeb(temp, i2c_imx->base + IMX_I2C_I2SR);
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR);
 		wake_up(&i2c_imx->queue);
 		return IRQ_HANDLED;
 	}
@@ -334,7 +346,7 @@ static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 		__func__, msgs->addr << 1);
 
 	/* write slave address */
-	writeb(msgs->addr << 1, i2c_imx->base + IMX_I2C_I2DR);
+	imx_i2c_write_reg(msgs->addr << 1, i2c_imx, IMX_I2C_I2DR);
 	result = i2c_imx_trx_complete(i2c_imx);
 	if (result)
 		return result;
@@ -348,7 +360,7 @@ static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 		dev_dbg(&i2c_imx->adapter.dev,
 			"<%s> write byte: B%d=0x%X\n",
 			__func__, i, msgs->buf[i]);
-		writeb(msgs->buf[i], i2c_imx->base + IMX_I2C_I2DR);
+		imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
 		result = i2c_imx_trx_complete(i2c_imx);
 		if (result)
 			return result;
@@ -369,7 +381,7 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 		__func__, (msgs->addr << 1) | 0x01);
 
 	/* write slave address */
-	writeb((msgs->addr << 1) | 0x01, i2c_imx->base + IMX_I2C_I2DR);
+	imx_i2c_write_reg((msgs->addr << 1) | 0x01, i2c_imx, IMX_I2C_I2DR);
 	result = i2c_imx_trx_complete(i2c_imx);
 	if (result)
 		return result;
@@ -380,12 +392,12 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 	dev_dbg(&i2c_imx->adapter.dev, "<%s> setup bus\n", __func__);
 
 	/* setup bus to read data */
-	temp = readb(i2c_imx->base + IMX_I2C_I2CR);
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
 	temp &= ~I2CR_MTX;
 	if (msgs->len - 1)
 		temp &= ~I2CR_TXAK;
-	writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
-	readb(i2c_imx->base + IMX_I2C_I2DR); /* dummy read */
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+	imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */
 
 	dev_dbg(&i2c_imx->adapter.dev, "<%s> read data\n", __func__);
 
@@ -399,19 +411,19 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 			   controller from generating another clock cycle */
 			dev_dbg(&i2c_imx->adapter.dev,
 				"<%s> clear MSTA\n", __func__);
-			temp = readb(i2c_imx->base + IMX_I2C_I2CR);
+			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
 			temp &= ~(I2CR_MSTA | I2CR_MTX);
-			writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
+			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
 			i2c_imx_bus_busy(i2c_imx, 0);
 			i2c_imx->stopped = 1;
 		} else if (i == (msgs->len - 2)) {
 			dev_dbg(&i2c_imx->adapter.dev,
 				"<%s> set TXAK\n", __func__);
-			temp = readb(i2c_imx->base + IMX_I2C_I2CR);
+			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
 			temp |= I2CR_TXAK;
-			writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
+			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
 		}
-		msgs->buf[i] = readb(i2c_imx->base + IMX_I2C_I2DR);
+		msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
 		dev_dbg(&i2c_imx->adapter.dev,
 			"<%s> read byte: B%d=0x%X\n",
 			__func__, i, msgs->buf[i]);
@@ -438,9 +450,9 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
 		if (i) {
 			dev_dbg(&i2c_imx->adapter.dev,
 				"<%s> repeated start\n", __func__);
-			temp = readb(i2c_imx->base + IMX_I2C_I2CR);
+			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
 			temp |= I2CR_RSTA;
-			writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
+			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
 			result =  i2c_imx_bus_busy(i2c_imx, 1);
 			if (result)
 				goto fail0;
@@ -449,13 +461,13 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
 			"<%s> transfer message: %d\n", __func__, i);
 		/* write/read data */
 #ifdef CONFIG_I2C_DEBUG_BUS
-		temp = readb(i2c_imx->base + IMX_I2C_I2CR);
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
 		dev_dbg(&i2c_imx->adapter.dev, "<%s> CONTROL: IEN=%d, IIEN=%d, "
 			"MSTA=%d, MTX=%d, TXAK=%d, RSTA=%d\n", __func__,
 			(temp & I2CR_IEN ? 1 : 0), (temp & I2CR_IIEN ? 1 : 0),
 			(temp & I2CR_MSTA ? 1 : 0), (temp & I2CR_MTX ? 1 : 0),
 			(temp & I2CR_TXAK ? 1 : 0), (temp & I2CR_RSTA ? 1 : 0));
-		temp = readb(i2c_imx->base + IMX_I2C_I2SR);
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
 		dev_dbg(&i2c_imx->adapter.dev,
 			"<%s> STATUS: ICF=%d, IAAS=%d, IBB=%d, "
 			"IAL=%d, SRW=%d, IIF=%d, RXAK=%d\n", __func__,
@@ -575,8 +587,8 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 	i2c_imx_set_clk(i2c_imx, bitrate);
 
 	/* Set up chip registers to defaults */
-	writeb(0, i2c_imx->base + IMX_I2C_I2CR);
-	writeb(0, i2c_imx->base + IMX_I2C_I2SR);
+	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2CR);
+	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2SR);
 
 	/* Add I2C adapter */
 	ret = i2c_add_numbered_adapter(&i2c_imx->adapter);
@@ -612,10 +624,10 @@ static int __exit i2c_imx_remove(struct platform_device *pdev)
 	i2c_del_adapter(&i2c_imx->adapter);
 
 	/* setup chip registers to defaults */
-	writeb(0, i2c_imx->base + IMX_I2C_IADR);
-	writeb(0, i2c_imx->base + IMX_I2C_IFDR);
-	writeb(0, i2c_imx->base + IMX_I2C_I2CR);
-	writeb(0, i2c_imx->base + IMX_I2C_I2SR);
+	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IADR);
+	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IFDR);
+	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2CR);
+	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2SR);
 
 	return 0;
 }
Signature · 1 line
1.8.0
#4 lore raw

PATCH 5/8 i2c: imx: change register offset representation

In reply to Jingchang Lu

the I2C register offset may different between SoCs,
to provid support for all these chips, split the register offset into a fixed base address and a variable shift value, then the full register offset will be calculated by
reg_off = ( reg_base_addr << reg_shift)

---
 drivers/i2c/busses/i2c-imx.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)
drivers/i2c/busses/i2c-imx.c +16 -7
index 1a3c608..8a292a9 100644
@@ -64,12 +64,21 @@
 /* Default value */
 #define IMX_I2C_BIT_RATE	100000	/* 100kHz */
 
-/* IMX I2C registers */
+/* IMX I2C registers:
+ * the I2C register offset is different between SoCs,
+ * to provid support for all these chips, split the
+ * register offset into a fixed base address and a
+ * variable shift value, then the full register offset
+ * will be calculated by
+ * reg_off = ( reg_base_addr << reg_shift)
+ */
 #define IMX_I2C_IADR	0x00	/* i2c slave address */
-#define IMX_I2C_IFDR	0x04	/* i2c frequency divider */
-#define IMX_I2C_I2CR	0x08	/* i2c control */
-#define IMX_I2C_I2SR	0x0C	/* i2c status */
-#define IMX_I2C_I2DR	0x10	/* i2c transfer data */
+#define IMX_I2C_IFDR	0x01	/* i2c frequency divider */
+#define IMX_I2C_I2CR	0x02	/* i2c control */
+#define IMX_I2C_I2SR	0x03	/* i2c status */
+#define IMX_I2C_I2DR	0x04	/* i2c transfer data */
+
+#define IMX_I2C_REGSHIFT	2
 
 /* Bits of IMX I2C registers */
 #define I2SR_RXAK	0x01
@@ -163,13 +172,13 @@ static inline int is_imx1_i2c(struct imx_i2c_struct *i2c_imx)
 static inline void imx_i2c_write_reg(unsigned int val,
 		struct imx_i2c_struct *i2c_imx, unsigned int reg)
 {
-	writeb(val, i2c_imx->base + reg);
+	writeb(val, i2c_imx->base + (reg << IMX_I2C_REGSHIFT));
 }
 
 static inline unsigned char imx_i2c_read_reg(struct imx_i2c_struct *i2c_imx,
 		unsigned int reg)
 {
-	return readb(i2c_imx->base + reg);
+	return readb(i2c_imx->base + (reg << IMX_I2C_REGSHIFT));
 }
 
 /** Functions for IMX I2C adapter driver ***************************************
Signature · 1 line
1.8.0
#5 lore raw

PATCH 6/8 i2c: imx: add INT flag and IEN bit operatation codes

In reply to Jingchang Lu

This add bits operation macro that differ between SoCs. Interrupt flags clear operation in I2SR differ between SoCs:
write zero to clear(w0c) INT flag on i.MX,
but write one to clear(w1c) INT flag on Vybrid.
I2C module enable operation in I2CR also differ between SoCs:
set I2CR_IEN bit enable the module on i.MX,
but clear I2CR_IEN bit enable the module on Vybrid.

---
 drivers/i2c/busses/i2c-imx.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)
drivers/i2c/busses/i2c-imx.c +22 -5
index 8a292a9..dc9f2ec 100644
@@ -95,6 +95,22 @@
 #define I2CR_IIEN	0x40
 #define I2CR_IEN	0x80
 
+/* register bits different operating codes definition:
+ * 1) I2SR: Interrupt flags clear operation differ between SoCs:
+ * - write zero to clear(w0c) INT flag on i.MX,
+ * - but write one to clear(w1c) INT flag on Vybrid.
+ * 2) I2CR: I2C module enable operation also differ between SoCs:
+ * - set I2CR_IEN bit enable the module on i.MX,
+ * - but clear I2CR_IEN bit enable the module on Vybrid.
+ */
+#define I2SR_CLR_OPCODE_W0C	0x0
+#define I2SR_CLR_OPCODE_W1C	(I2SR_IAL | I2SR_IIF)
+#define I2CR_IEN_OPCODE_0	0x0
+#define I2CR_IEN_OPCODE_1	I2CR_IEN
+
+#define IMX_I2SR_CLR_OPCODE	I2SR_CLR_OPCODE_W0C
+#define IMX_I2CR_IEN_OPCODE	I2CR_IEN_OPCODE_1
+
 /** Variables ******************************************************************
 *******************************************************************************/
 
@@ -242,8 +258,8 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
 	clk_prepare_enable(i2c_imx->clk);
 	imx_i2c_write_reg(i2c_imx->ifdr, i2c_imx, IMX_I2C_IFDR);
 	/* Enable I2C controller */
-	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2SR);
-	imx_i2c_write_reg(I2CR_IEN, i2c_imx, IMX_I2C_I2CR);
+	imx_i2c_write_reg(IMX_I2SR_CLR_OPCODE, i2c_imx, IMX_I2C_I2SR);
+	imx_i2c_write_reg(IMX_I2CR_IEN_OPCODE, i2c_imx, IMX_I2C_I2CR);
 
 	/* Wait controller to be stable */
 	udelay(50);
@@ -287,7 +303,7 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
 	}
 
 	/* Disable I2C controller */
-	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2CR);
+	imx_i2c_write_reg(IMX_I2CR_IEN_OPCODE ^ I2CR_IEN, i2c_imx, IMX_I2C_I2CR);
 	clk_disable_unprepare(i2c_imx->clk);
 }
 
@@ -339,6 +355,7 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
 		/* save status register */
 		i2c_imx->i2csr = temp;
 		temp &= ~I2SR_IIF;
+		temp |= (IMX_I2SR_CLR_OPCODE & I2SR_IIF);
 		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR);
 		wake_up(&i2c_imx->queue);
 		return IRQ_HANDLED;
@@ -596,8 +613,8 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 	i2c_imx_set_clk(i2c_imx, bitrate);
 
 	/* Set up chip registers to defaults */
-	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2CR);
-	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2SR);
+	imx_i2c_write_reg(IMX_I2CR_IEN_OPCODE ^ I2CR_IEN, i2c_imx, IMX_I2C_I2CR);
+	imx_i2c_write_reg(IMX_I2SR_CLR_OPCODE, i2c_imx, IMX_I2C_I2SR);
 
 	/* Add I2C adapter */
 	ret = i2c_add_numbered_adapter(&i2c_imx->adapter);
Signature · 1 line
1.8.0
#6 lore raw

PATCH 7/8 i2c: imx: add struct to hold more configurable quirks

In reply to Jingchang Lu

This add struct imx_i2c_hwdata to hold more quirks data which may vary between SoCs, thus the driver can operate on more differences to support more SoCs.

---
 drivers/i2c/busses/i2c-imx.c | 77 ++++++++++++++++++++++++++++++--------------
 1 file changed, 52 insertions(+), 25 deletions(-)
drivers/i2c/busses/i2c-imx.c +52 -25
index dc9f2ec..ce31196 100644
@@ -108,9 +108,6 @@
 #define I2CR_IEN_OPCODE_0	0x0
 #define I2CR_IEN_OPCODE_1	I2CR_IEN
 
-#define IMX_I2SR_CLR_OPCODE	I2SR_CLR_OPCODE_W0C
-#define IMX_I2CR_IEN_OPCODE	I2CR_IEN_OPCODE_1
-
 /** Variables ******************************************************************
 *******************************************************************************/
 
@@ -127,7 +124,7 @@ struct imx_i2c_clk_pair {
 	u16	val;
 };
 
-static struct imx_i2c_clk_pair __initdata i2c_clk_div[] = {
+static struct imx_i2c_clk_pair imx_i2c_clk_div[] = {
 	{ 22,	0x20 }, { 24,	0x21 }, { 26,	0x22 }, { 28,	0x23 },
 	{ 30,	0x00 },	{ 32,	0x24 }, { 36,	0x25 }, { 40,	0x26 },
 	{ 42,	0x03 }, { 44,	0x27 },	{ 48,	0x28 }, { 52,	0x05 },
@@ -148,6 +145,15 @@ enum imx_i2c_type {
 	IMX21_I2C,
 };
 
+struct imx_i2c_hwdata {
+	enum imx_i2c_type	devtype;
+	unsigned		regshift;
+	struct imx_i2c_clk_pair	*clk_div;
+	unsigned		ndivs;
+	unsigned		i2sr_clr_opcode;
+	unsigned		i2cr_ien_opcode;
+};
+
 struct imx_i2c_struct {
 	struct i2c_adapter	adapter;
 	struct clk		*clk;
@@ -157,16 +163,36 @@ struct imx_i2c_struct {
 	unsigned int 		disable_delay;
 	int			stopped;
 	unsigned int		ifdr; /* IMX_I2C_IFDR */
-	enum imx_i2c_type	devtype;
+	const struct imx_i2c_hwdata	*hwdata;
+};
+
+static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
+	.devtype		= IMX1_I2C,
+	.regshift		= IMX_I2C_REGSHIFT,
+	.clk_div		= imx_i2c_clk_div,
+	.ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
+	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
+	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
+
+};
+
+static const struct imx_i2c_hwdata imx21_i2c_hwdata  = {
+	.devtype		= IMX21_I2C,
+	.regshift		= IMX_I2C_REGSHIFT,
+	.clk_div		= imx_i2c_clk_div,
+	.ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
+	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
+	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
+
 };
 
 static struct platform_device_id imx_i2c_devtype[] = {
 	{
 		.name = "imx1-i2c",
-		.driver_data = IMX1_I2C,
+		.driver_data = (kernel_ulong_t)&imx1_i2c_hwdata,
 	}, {
 		.name = "imx21-i2c",
-		.driver_data = IMX21_I2C,
+		.driver_data = (kernel_ulong_t)&imx21_i2c_hwdata,
 	}, {
 		/* sentinel */
 	}
@@ -174,27 +200,27 @@ static struct platform_device_id imx_i2c_devtype[] = {
 MODULE_DEVICE_TABLE(platform, imx_i2c_devtype);
 
 static const struct of_device_id i2c_imx_dt_ids[] = {
-	{ .compatible = "fsl,imx1-i2c", .data = &imx_i2c_devtype[IMX1_I2C], },
-	{ .compatible = "fsl,imx21-i2c", .data = &imx_i2c_devtype[IMX21_I2C], },
+	{ .compatible = "fsl,imx1-i2c", .data = &imx1_i2c_hwdata, },
+	{ .compatible = "fsl,imx21-i2c", .data = &imx21_i2c_hwdata, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, i2c_imx_dt_ids);
 
 static inline int is_imx1_i2c(struct imx_i2c_struct *i2c_imx)
 {
-	return i2c_imx->devtype == IMX1_I2C;
+	return i2c_imx->hwdata->devtype == IMX1_I2C;
 }
 
 static inline void imx_i2c_write_reg(unsigned int val,
 		struct imx_i2c_struct *i2c_imx, unsigned int reg)
 {
-	writeb(val, i2c_imx->base + (reg << IMX_I2C_REGSHIFT));
+	writeb(val, i2c_imx->base + (reg << i2c_imx->hwdata->regshift));
 }
 
 static inline unsigned char imx_i2c_read_reg(struct imx_i2c_struct *i2c_imx,
 		unsigned int reg)
 {
-	return readb(i2c_imx->base + (reg << IMX_I2C_REGSHIFT));
+	return readb(i2c_imx->base + (reg << i2c_imx->hwdata->regshift));
 }
 
 /** Functions for IMX I2C adapter driver ***************************************
@@ -258,8 +284,8 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
 	clk_prepare_enable(i2c_imx->clk);
 	imx_i2c_write_reg(i2c_imx->ifdr, i2c_imx, IMX_I2C_IFDR);
 	/* Enable I2C controller */
-	imx_i2c_write_reg(IMX_I2SR_CLR_OPCODE, i2c_imx, IMX_I2C_I2SR);
-	imx_i2c_write_reg(IMX_I2CR_IEN_OPCODE, i2c_imx, IMX_I2C_I2CR);
+	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR);
+	imx_i2c_write_reg(i2c_imx->hwdata->i2cr_ien_opcode, i2c_imx, IMX_I2C_I2CR);
 
 	/* Wait controller to be stable */
 	udelay(50);
@@ -303,13 +329,15 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
 	}
 
 	/* Disable I2C controller */
-	imx_i2c_write_reg(IMX_I2CR_IEN_OPCODE ^ I2CR_IEN, i2c_imx, IMX_I2C_I2CR);
+	temp = i2c_imx->hwdata->i2cr_ien_opcode ^ I2CR_IEN,
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
 	clk_disable_unprepare(i2c_imx->clk);
 }
 
 static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
 							unsigned int rate)
 {
+	struct imx_i2c_clk_pair *i2c_clk_div = i2c_imx->hwdata->clk_div;
 	unsigned int i2c_clk_rate;
 	unsigned int div;
 	int i;
@@ -319,8 +347,8 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
 	div = (i2c_clk_rate + rate - 1) / rate;
 	if (div < i2c_clk_div[0].div)
 		i = 0;
-	else if (div > i2c_clk_div[ARRAY_SIZE(i2c_clk_div) - 1].div)
-		i = ARRAY_SIZE(i2c_clk_div) - 1;
+	else if (div > i2c_clk_div[i2c_imx->hwdata->ndivs - 1].div)
+		i = i2c_imx->hwdata->ndivs - 1;
 	else
 		for (i = 0; i2c_clk_div[i].div < div; i++);
 
@@ -355,7 +383,7 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
 		/* save status register */
 		i2c_imx->i2csr = temp;
 		temp &= ~I2SR_IIF;
-		temp |= (IMX_I2SR_CLR_OPCODE & I2SR_IIF);
+		temp |= (i2c_imx->hwdata->i2sr_clr_opcode & I2SR_IIF);
 		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR);
 		wake_up(&i2c_imx->queue);
 		return IRQ_HANDLED;
@@ -537,7 +565,6 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 	struct imx_i2c_struct *i2c_imx;
 	struct resource *res;
 	struct imxi2c_platform_data *pdata = pdev->dev.platform_data;
-	const struct platform_device_id *imx_id;
 	void __iomem *base;
 	int irq, ret;
 	u32 bitrate;
@@ -563,11 +590,10 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 	}
 
 	if (of_id)
-		imx_id = of_id->data;
+		i2c_imx->hwdata = of_id->data;
 	else
-		imx_id = platform_get_device_id(pdev);
-
-	i2c_imx->devtype = imx_id->driver_data;
+		i2c_imx->hwdata = (struct imx_i2c_hwdata *)
+				platform_get_device_id(pdev)->driver_data;
 
 	/* Setup i2c_imx driver structure */
 	strlcpy(i2c_imx->adapter.name, pdev->name, sizeof(i2c_imx->adapter.name));
@@ -613,8 +639,9 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 	i2c_imx_set_clk(i2c_imx, bitrate);
 
 	/* Set up chip registers to defaults */
-	imx_i2c_write_reg(IMX_I2CR_IEN_OPCODE ^ I2CR_IEN, i2c_imx, IMX_I2C_I2CR);
-	imx_i2c_write_reg(IMX_I2SR_CLR_OPCODE, i2c_imx, IMX_I2C_I2SR);
+	imx_i2c_write_reg(i2c_imx->hwdata->i2cr_ien_opcode ^ I2CR_IEN,
+			i2c_imx, IMX_I2C_I2CR);
+	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR);
 
 	/* Add I2C adapter */
 	ret = i2c_add_numbered_adapter(&i2c_imx->adapter);
Signature · 1 line
1.8.0
#7 lore raw

PATCH 8/8 i2c: imx: Add Vybrid VF610 I2C controller support

In reply to Jingchang Lu

Add Freescale Vybrid VF610 I2C controller support to imx I2C driver framework.

---
 drivers/i2c/busses/i2c-imx.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
drivers/i2c/busses/i2c-imx.c +32 -0
index ce31196..9c0f8bd 100644
@@ -79,6 +79,7 @@
 #define IMX_I2C_I2DR	0x04	/* i2c transfer data */
 
 #define IMX_I2C_REGSHIFT	2
+#define VF610_I2C_REGSHIFT	0
 
 /* Bits of IMX I2C registers */
 #define I2SR_RXAK	0x01
@@ -140,9 +141,29 @@ static struct imx_i2c_clk_pair imx_i2c_clk_div[] = {
 	{ 3072,	0x1E }, { 3840,	0x1F }
 };
 
+/* Vybrid VF610 clock divider, register value pairs */
+static struct imx_i2c_clk_pair vf610_i2c_clk_div[] = {
+	{ 20,   0x00 }, { 22,   0x01 }, { 24,   0x02 }, { 26,   0x03 },
+	{ 28,   0x04 }, { 30,   0x05 }, { 32,   0x09 }, { 34,   0x06 },
+	{ 36,   0x0A }, { 40,   0x07 }, { 44,   0x0C }, { 48,   0x0D },
+	{ 52,   0x43 }, { 56,   0x0E }, { 60,   0x45 }, { 64,   0x12 },
+	{ 68,   0x0F }, { 72,   0x13 }, { 80,   0x14 }, { 88,   0x15 },
+	{ 96,   0x19 }, { 104,  0x16 }, { 112,  0x1A }, { 128,  0x17 },
+	{ 136,  0x4F }, { 144,  0x1C }, { 160,  0x1D }, { 176,  0x55 },
+	{ 192,  0x1E }, { 208,  0x56 }, { 224,  0x22 }, { 228,  0x24 },
+	{ 240,  0x1F }, { 256,  0x23 }, { 288,  0x5C }, { 320,  0x25 },
+	{ 384,  0x26 }, { 448,  0x2A }, { 480,  0x27 }, { 512,  0x2B },
+	{ 576,  0x2C }, { 640,  0x2D }, { 768,  0x31 }, { 896,  0x32 },
+	{ 960,  0x2F }, { 1024, 0x33 }, { 1152, 0x34 }, { 1280, 0x35 },
+	{ 1536, 0x36 }, { 1792, 0x3A }, { 1920, 0x37 }, { 2048, 0x3B },
+	{ 2304, 0x3C }, { 2560, 0x3D }, { 3072, 0x3E }, { 3584, 0x7A },
+	{ 3840, 0x3F }, { 4096, 0x7B }, { 5120, 0x7D }, { 6144, 0x7E },
+};
+
 enum imx_i2c_type {
 	IMX1_I2C,
 	IMX21_I2C,
+	VF610_I2C,
 };
 
 struct imx_i2c_hwdata {
@@ -186,6 +207,16 @@ static const struct imx_i2c_hwdata imx21_i2c_hwdata  = {
 
 };
 
+static struct imx_i2c_hwdata vf610_i2c_hwdata = {
+	.devtype		= VF610_I2C,
+	.regshift		= VF610_I2C_REGSHIFT,
+	.clk_div		= vf610_i2c_clk_div,
+	.ndivs			= ARRAY_SIZE(vf610_i2c_clk_div),
+	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W1C,
+	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_0,
+
+};
+
 static struct platform_device_id imx_i2c_devtype[] = {
 	{
 		.name = "imx1-i2c",
@@ -202,6 +233,7 @@ MODULE_DEVICE_TABLE(platform, imx_i2c_devtype);
 static const struct of_device_id i2c_imx_dt_ids[] = {
 	{ .compatible = "fsl,imx1-i2c", .data = &imx1_i2c_hwdata, },
 	{ .compatible = "fsl,imx21-i2c", .data = &imx21_i2c_hwdata, },
+	{ .compatible = "fsl,vf610-i2c", .data = &vf610_i2c_hwdata, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, i2c_imx_dt_ids);
Signature · 1 line
1.8.0
maintainer
#8 lore raw

PATCH 1/8 i2c: imx: use struct representing i2c clk{div, val} pair

In reply to Jingchang Lu

Nice improvement of this series.

There are some unnecessary blank lines in the definitions in 7/8 and 8/8, but otherwise:

Sascha

Quoted 78 lines · Jingchang Lu

using struct representing the i2c clk{div, val} pair would make the i2c_clk_div array more clear.

---
 drivers/i2c/busses/i2c-imx.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)
drivers/i2c/busses/i2c-imx.c +13 -7
index e242797..9167d43 100644
@@ -30,6 +30,8 @@
  *	Copyright (C) 2007 RightHand Technologies, Inc.
  *	Copyright (C) 2008 Darius Augulis <[email protected]>
  *
+ *	Copyright 2013 Freescale Semiconductor, Inc.
+ *
  */
 
 /** Includes *******************************************************************
@@ -95,8 +97,12 @@
  *
  * Duplicated divider values removed from list
  */
+struct imx_i2c_clk_pair {
+	u16	div;
+	u16	val;
+};
 
-static u16 __initdata i2c_clk_div[50][2] = {
+static struct imx_i2c_clk_pair __initdata i2c_clk_div[] = {
 	{ 22,	0x20 }, { 24,	0x21 }, { 26,	0x22 }, { 28,	0x23 },
 	{ 30,	0x00 },	{ 32,	0x24 }, { 36,	0x25 }, { 40,	0x26 },
 	{ 42,	0x03 }, { 44,	0x27 },	{ 48,	0x28 }, { 52,	0x05 },
@@ -274,15 +280,15 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
 	/* Divider value calculation */
 	i2c_clk_rate = clk_get_rate(i2c_imx->clk);
 	div = (i2c_clk_rate + rate - 1) / rate;
-	if (div < i2c_clk_div[0][0])
+	if (div < i2c_clk_div[0].div)
 		i = 0;
-	else if (div > i2c_clk_div[ARRAY_SIZE(i2c_clk_div) - 1][0])
+	else if (div > i2c_clk_div[ARRAY_SIZE(i2c_clk_div) - 1].div)
 		i = ARRAY_SIZE(i2c_clk_div) - 1;
 	else
-		for (i = 0; i2c_clk_div[i][0] < div; i++);
+		for (i = 0; i2c_clk_div[i].div < div; i++);
 
 	/* Store divider value */
-	i2c_imx->ifdr = i2c_clk_div[i][1];
+	i2c_imx->ifdr = i2c_clk_div[i].val;
 
 	/*
 	 * There dummy delay is calculated.
@@ -290,7 +296,7 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
 	 * This delay is used in I2C bus disable function
 	 * to fix chip hardware bug.
 	 */
-	i2c_imx->disable_delay = (500000U * i2c_clk_div[i][0]
+	i2c_imx->disable_delay = (500000U * i2c_clk_div[i].div
 		+ (i2c_clk_rate / 2) - 1) / (i2c_clk_rate / 2);
 
 	/* dev_dbg() can't be used, because adapter is not yet registered */
@@ -298,7 +304,7 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
 	dev_dbg(&i2c_imx->adapter.dev, "<%s> I2C_CLK=%d, REQ DIV=%d\n",
 		__func__, i2c_clk_rate, div);
 	dev_dbg(&i2c_imx->adapter.dev, "<%s> IFDR[IC]=0x%x, REAL DIV=%d\n",
-		__func__, i2c_clk_div[i][1], i2c_clk_div[i][0]);
+		__func__, i2c_clk_div[i].val, i2c_clk_div[i].div);
 #endif
 }
 
Signature · 1 line
1.8.0
Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
#9 lore raw

PATCH 1/8 i2c: imx: use struct representing i2c clk{div, val} pair

Quoted 9 lines

-----Original Message-----
From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
Sent: Thursday, August 08, 2013 1:54 AM
To: Lu Jingchang-B35083
Cc: wsa at the-dreams.de; shawn.guo at linaro.org; Estevam Fabio-R49496; linux- i2c at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH 1/8] i2c: imx: use struct representing i2c clk{div, val} pair

Quoted 6 lines

Nice improvement of this series.

There are some unnecessary blank lines in the definitions in 7/8 and 8/8, but otherwise:

[Lu Jingchang-B35083]
Thank you very much for your review.
I leave one blank lines between definitions for readability, could you please help to comment the unnecessary blank lines on that patches so that I can remove them and send out the next version, thanks.

Best Regards,
Jingchang

Applied
#10 lore raw

PATCH 1/8 i2c: imx: use struct representing i2c clk{div, val} pair

In reply to Jingchang Lu
Quoted 4 lines · Jingchang Lu

using struct representing the i2c clk{div, val} pair would make the i2c_clk_div array more clear.

Applied series to for-next, thanks! Please use --cover-letter next time.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130815/5ff52d7b/attachment.sig>