Skip to contentSkip to conversation

i2c-designware: Allow mixed endianness

PATCH Superseded Jean-Hugues Deschenes 3 participants 8 messages Jan 19, 2010 – Jan 24, 2010 3 messages not in the archive I2C subsystem host drivers

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Jean-Hugues Deschenes

author
#0 lore raw

v2 cover i2c-designware: Allow mixed endianness

This is the v2 version of the patch which adds support for the i2c Designware controller when its endianness differs from that of the cpu.

---
Changelog v1->v2
- rename i2c_dw_(read|write)l to dw_(read|write)l (Shinya Kuribayashi)
- rename dw_(read|write)l's addr argument to offset (Shinya Kuribayashi)
- replace (if constant == x) with (if x == constant) (Ben Dooks)
---

Signed-off-by Jean-Hugues Deschenes <[email protected]>
author
#1 lore raw

PATCH v2 1/3 i2c-designware: Use local version of readl & writel

Use local versions of readl & writel, so per-access manipulations may be performed

Signed-off-by Jean-Hugues Deschenes <[email protected]>
---
 drivers/i2c/busses/i2c-designware.c |   88 ++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 39 deletions(-)
drivers/i2c/busses/i2c-designware.c +49 -39
Index: linux-2.6_i2c/drivers/i2c/busses/i2c-designware.c
===================================================================
@@ -219,6 +219,16 @@ struct dw_i2c_dev {
 	unsigned int		rx_fifo_depth;
 };
 
+static u32 dw_readl(struct dw_i2c_dev *dev, int offset)
+{
+	return readl(dev->base + offset);
+}
+
+static void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
+{
+	writel(b, dev->base + offset);
+}
+
 static u32
 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
 {

@@ -288,7 +298,7 @@ static void i2c_dw_init(struct dw_i2c_de
 	u32 ic_con, hcnt, lcnt;
 
 	/* Disable the adapter */
-	writel(0, dev->base + DW_IC_ENABLE);
+	dw_writel(dev, 0, DW_IC_ENABLE);
 
 	/* set standard and fast speed deviders for high/low periods */
 

@@ -302,8 +312,8 @@ static void i2c_dw_init(struct dw_i2c_de
 				47,	/* tLOW = 4.7 us */
 				3,	/* tf = 0.3 us */
 				0);	/* No offset */
-	writel(hcnt, dev->base + DW_IC_SS_SCL_HCNT);
-	writel(lcnt, dev->base + DW_IC_SS_SCL_LCNT);
+	dw_writel(dev, hcnt, DW_IC_SS_SCL_HCNT);
+	dw_writel(dev, lcnt, DW_IC_SS_SCL_LCNT);
 	dev_dbg(dev->dev, "Standard-mode HCNT:LCNT = %d:%d\n", hcnt, lcnt);
 
 	/* Fast-mode */

@@ -316,18 +326,18 @@ static void i2c_dw_init(struct dw_i2c_de
 				13,	/* tLOW = 1.3 us */
 				3,	/* tf = 0.3 us */
 				0);	/* No offset */
-	writel(hcnt, dev->base + DW_IC_FS_SCL_HCNT);
-	writel(lcnt, dev->base + DW_IC_FS_SCL_LCNT);
+	dw_writel(dev, hcnt, DW_IC_FS_SCL_HCNT);
+	dw_writel(dev, lcnt, DW_IC_FS_SCL_LCNT);
 	dev_dbg(dev->dev, "Fast-mode HCNT:LCNT = %d:%d\n", hcnt, lcnt);
 
 	/* Configure Tx/Rx FIFO threshold levels */
-	writel(dev->tx_fifo_depth - 1, dev->base + DW_IC_TX_TL);
-	writel(0, dev->base + DW_IC_RX_TL);
+	dw_writel(dev, dev->tx_fifo_depth - 1, DW_IC_TX_TL);
+	dw_writel(dev, 0, DW_IC_RX_TL);
 
 	/* configure the i2c master */
 	ic_con = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
 		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
-	writel(ic_con, dev->base + DW_IC_CON);
+	dw_writel(dev, ic_con, DW_IC_CON);
 }
 
 /*

@@ -337,7 +347,7 @@ static int i2c_dw_wait_bus_not_busy(stru
 {
 	int timeout = TIMEOUT;
 
-	while (readl(dev->base + DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY) {
+	while (dw_readl(dev, DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY) {
 		if (timeout <= 0) {
 			dev_warn(dev->dev, "timeout waiting for bus ready\n");
 			return -ETIMEDOUT;

@@ -355,24 +365,24 @@ static void i2c_dw_xfer_init(struct dw_i
 	u32 ic_con;
 
 	/* Disable the adapter */
-	writel(0, dev->base + DW_IC_ENABLE);
+	dw_writel(dev, 0, DW_IC_ENABLE);
 
 	/* set the slave (target) address */
-	writel(msgs[dev->msg_write_idx].addr, dev->base + DW_IC_TAR);
+	dw_writel(dev, msgs[dev->msg_write_idx].addr, DW_IC_TAR);
 
 	/* if the slave address is ten bit address, enable 10BITADDR */
-	ic_con = readl(dev->base + DW_IC_CON);
+	ic_con = dw_readl(dev, DW_IC_CON);
 	if (msgs[dev->msg_write_idx].flags & I2C_M_TEN)
 		ic_con |= DW_IC_CON_10BITADDR_MASTER;
 	else
 		ic_con &= ~DW_IC_CON_10BITADDR_MASTER;
-	writel(ic_con, dev->base + DW_IC_CON);
+	dw_writel(dev, ic_con, DW_IC_CON);
 
 	/* Enable the adapter */
-	writel(1, dev->base + DW_IC_ENABLE);
+	dw_writel(dev, 1, DW_IC_ENABLE);
 
 	/* Enable interrupts */
-	writel(DW_IC_INTR_DEFAULT_MASK, dev->base + DW_IC_INTR_MASK);
+	dw_writel(dev, DW_IC_INTR_DEFAULT_MASK, DW_IC_INTR_MASK);
 }
 
 /*

@@ -419,15 +429,15 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 			buf_len = msgs[dev->msg_write_idx].len;
 		}
 
-		tx_limit = dev->tx_fifo_depth - readl(dev->base + DW_IC_TXFLR);
-		rx_limit = dev->rx_fifo_depth - readl(dev->base + DW_IC_RXFLR);
+		tx_limit = dev->tx_fifo_depth - dw_readl(dev, DW_IC_TXFLR);
+		rx_limit = dev->rx_fifo_depth - dw_readl(dev, DW_IC_RXFLR);
 
 		while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
 			if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
-				writel(0x100, dev->base + DW_IC_DATA_CMD);
+				dw_writel(dev, 0x100, DW_IC_DATA_CMD);
 				rx_limit--;
 			} else
-				writel(*buf++, dev->base + DW_IC_DATA_CMD);
+				dw_writel(dev, *buf++, DW_IC_DATA_CMD);
 			tx_limit--; buf_len--;
 		}
 

@@ -452,7 +462,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 	if (dev->msg_err)
 		intr_mask = 0;
 
-	writel(intr_mask, dev->base + DW_IC_INTR_MASK);
+	dw_writel(dev, intr_mask, DW_IC_INTR_MASK);
 }
 
 static void

@@ -476,10 +486,10 @@ i2c_dw_read(struct dw_i2c_dev *dev)
 			buf = dev->rx_buf;
 		}
 
-		rx_valid = readl(dev->base + DW_IC_RXFLR);
+		rx_valid = dw_readl(dev, DW_IC_RXFLR);
 
 		for (; len > 0 && rx_valid > 0; len--, rx_valid--)
-			*buf++ = readl(dev->base + DW_IC_DATA_CMD);
+			*buf++ = dw_readl(dev, DW_IC_DATA_CMD);
 
 		if (len > 0) {
 			dev->status |= STATUS_READ_IN_PROGRESS;

@@ -562,7 +572,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, st
 	/* no error */
 	if (likely(!dev->cmd_err)) {
 		/* Disable the adapter */
-		writel(0, dev->base + DW_IC_ENABLE);
+		dw_writel(dev, 0, DW_IC_ENABLE);
 		ret = num;
 		goto done;
 	}

@@ -606,7 +616,7 @@ static u32 i2c_dw_read_clear_intrbits(st
 	 *
 	 * The raw version might be useful for debugging purposes.
 	 */
-	stat = readl(dev->base + DW_IC_INTR_STAT);
+	stat = dw_readl(dev, DW_IC_INTR_STAT);
 
 	/*
 	 * Do not use the IC_CLR_INTR register to clear interrupts, or

@@ -616,31 +626,31 @@ static u32 i2c_dw_read_clear_intrbits(st
 	 * Instead, use the separately-prepared IC_CLR_* registers.
 	 */
 	if (stat & DW_IC_INTR_RX_UNDER)
-		readl(dev->base + DW_IC_CLR_RX_UNDER);
+		dw_readl(dev, DW_IC_CLR_RX_UNDER);
 	if (stat & DW_IC_INTR_RX_OVER)
-		readl(dev->base + DW_IC_CLR_RX_OVER);
+		dw_readl(dev, DW_IC_CLR_RX_OVER);
 	if (stat & DW_IC_INTR_TX_OVER)
-		readl(dev->base + DW_IC_CLR_TX_OVER);
+		dw_readl(dev, DW_IC_CLR_TX_OVER);
 	if (stat & DW_IC_INTR_RD_REQ)
-		readl(dev->base + DW_IC_CLR_RD_REQ);
+		dw_readl(dev, DW_IC_CLR_RD_REQ);
 	if (stat & DW_IC_INTR_TX_ABRT) {
 		/*
 		 * The IC_TX_ABRT_SOURCE register is cleared whenever
 		 * the IC_CLR_TX_ABRT is read.  Preserve it beforehand.
 		 */
-		dev->abort_source = readl(dev->base + DW_IC_TX_ABRT_SOURCE);
-		readl(dev->base + DW_IC_CLR_TX_ABRT);
+		dev->abort_source = dw_readl(dev, DW_IC_TX_ABRT_SOURCE);
+		dw_readl(dev, DW_IC_CLR_TX_ABRT);
 	}
 	if (stat & DW_IC_INTR_RX_DONE)
-		readl(dev->base + DW_IC_CLR_RX_DONE);
+		dw_readl(dev, DW_IC_CLR_RX_DONE);
 	if (stat & DW_IC_INTR_ACTIVITY)
-		readl(dev->base + DW_IC_CLR_ACTIVITY);
+		dw_readl(dev, DW_IC_CLR_ACTIVITY);
 	if (stat & DW_IC_INTR_STOP_DET)
-		readl(dev->base + DW_IC_CLR_STOP_DET);
+		dw_readl(dev, DW_IC_CLR_STOP_DET);
 	if (stat & DW_IC_INTR_START_DET)
-		readl(dev->base + DW_IC_CLR_START_DET);
+		dw_readl(dev, DW_IC_CLR_START_DET);
 	if (stat & DW_IC_INTR_GEN_CALL)
-		readl(dev->base + DW_IC_CLR_GEN_CALL);
+		dw_readl(dev, DW_IC_CLR_GEN_CALL);
 
 	return stat;
 }

@@ -665,7 +675,7 @@ static irqreturn_t i2c_dw_isr(int this_i
 		 * Anytime TX_ABRT is set, the contents of the tx/rx
 		 * buffers are flushed.  Make sure to skip them.
 		 */
-		writel(0, dev->base + DW_IC_INTR_MASK);
+		dw_writel(dev, 0, DW_IC_INTR_MASK);
 		goto tx_aborted;
 	}
 

@@ -746,14 +756,14 @@ static int __devinit dw_i2c_probe(struct
 		goto err_unuse_clocks;
 	}
 	{
-		u32 param1 = readl(dev->base + DW_IC_COMP_PARAM_1);
+		u32 param1 = dw_readl(dev, DW_IC_COMP_PARAM_1);
 
 		dev->tx_fifo_depth = ((param1 >> 16) & 0xff) + 1;
 		dev->rx_fifo_depth = ((param1 >> 8)  & 0xff) + 1;
 	}
 	i2c_dw_init(dev);
 
-	writel(0, dev->base + DW_IC_INTR_MASK); /* disable IRQ */
+	dw_writel(dev, 0, DW_IC_INTR_MASK); /* disable IRQ */
 	r = request_irq(dev->irq, i2c_dw_isr, IRQF_DISABLED, pdev->name, dev);
 	if (r) {
 		dev_err(&pdev->dev, "failure requesting irq %i\n", dev->irq);

@@ -809,7 +819,7 @@ static int __devexit dw_i2c_remove(struc
 	clk_put(dev->clk);
 	dev->clk = NULL;
 
-	writel(0, dev->base + DW_IC_ENABLE);
+	dw_writel(dev, 0, DW_IC_ENABLE);
 	free_irq(dev->irq, dev);
 	kfree(dev);
author
#2 lore raw

PATCH v2 2/3 i2c-designware: Check component type register

In reply to Jean-Hugues Deschenes

Designware component type register is checked before attaching to the device.

Signed-off-by Jean-Hugues Deschenes <[email protected]>
---
 drivers/i2c/busses/i2c-designware.c |   11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/i2c/busses/i2c-designware.c +11 -0
Index: linux-2.6_i2c/drivers/i2c/busses/i2c-designware.c
===================================================================
@@ -68,6 +68,7 @@
 #define DW_IC_TXFLR		0x74
 #define DW_IC_RXFLR		0x78
 #define DW_IC_COMP_PARAM_1	0xf4
+#define DW_IC_COMP_TYPE		0xfc
 #define DW_IC_TX_ABRT_SOURCE	0x80
 
 #define DW_IC_CON_MASTER		0x1

@@ -756,6 +757,16 @@ static int __devinit dw_i2c_probe(struct
 		goto err_unuse_clocks;
 	}
 	{
+		u32 comp_type = dw_readl(dev, DW_IC_COMP_TYPE);
+
+		if (comp_type != 0x44570140) {
+			dev_err(&pdev->dev, "Unknown Synopsys component type: "
+					"0x%08x\n",	comp_type);
+			r = -ENODEV;
+			goto err_iounmap;
+		}
+	}
+	{
 		u32 param1 = dw_readl(dev, DW_IC_COMP_PARAM_1);
 
 		dev->tx_fifo_depth = ((param1 >> 16) & 0xff) + 1;

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw

Re: [PATCH v2 2/3] i2c-designware: Check component type register

Quoted 23 lines · Jean-Hugues Deschenes

Designware component type register is checked before attaching to the device.

Signed-off-by Jean-Hugues Deschenes <[email protected]>
---
 drivers/i2c/busses/i2c-designware.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: linux-2.6_i2c/drivers/i2c/busses/i2c-designware.c
===================================================================

drivers/i2c/busses/i2c-designware.c +1 -0
@@ -68,6 +68,7 @@
 #define DW_IC_TXFLR		0x74
 #define DW_IC_RXFLR		0x78
 #define DW_IC_COMP_PARAM_1	0xf4
+#define DW_IC_COMP_TYPE		0xfc
 #define DW_IC_TX_ABRT_SOURCE	0x80
 
 #define DW_IC_CON_MASTER		0x1

@@ -756,6 +757,16 @@ static int __devinit dw_i2c_probe(struct
 		goto err_unuse_clocks;
 	}

drivers/i2c/busses/i2c-designware.c static int __devinit dw_i2c_probe(struct · line 768

Quoted 10 lines
drivers/i2c/busses/i2c-designware.c +9 -0
 	{
+		u32 comp_type = dw_readl(dev, DW_IC_COMP_TYPE);
+
+		if (comp_type != 0x44570140) {
+			dev_err(&pdev->dev, "Unknown Synopsys component type: "
+					"0x%08x\n",	comp_type);
+			r = -ENODEV;
+			goto err_iounmap;
+		}
+	}

having looked at the code around here, let's just use a single u32 reg or u32 val field at the top of dw_i2c_probe and avoid the need for the {} blocks in the probe.

Would like to see a patch fixing this added to the series.

Quoted 10 lines
drivers/i2c/busses/i2c-designware.c +1 -0
+	{
 		u32 param1 = dw_readl(dev, DW_IC_COMP_PARAM_1);
 
 		dev->tx_fifo_depth = ((param1 >> 16) & 0xff) + 1;
Signature · 5 lines
-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
Ben ([email protected], http://www.fluff.org/)

  'a smiley only costs 4 bytes'
author
#5 lore raw

PATCH v2 3/3 i2c-designware: Allow mixed endianness accesses

In reply to Jean-Hugues Deschenes

Allows CPUs of a given endianness to access a dw controller of a different endianness. Endianncess difference is detected at run time through the dw component type register.

Signed-off-by Jean-Hugues Deschenes <[email protected]>
---
 drivers/i2c/busses/i2c-designware.c |   17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-designware.c +15 -2
Index: linux-2.6_i2c/drivers/i2c/busses/i2c-designware.c
===================================================================
@@ -36,6 +36,7 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
+#include <linux/swab.h>
 
 /*
  * Registers offset

@@ -192,6 +193,7 @@ static char *abort_sources[] = {
  * @status: i2c master status, one of STATUS_*
  * @abort_source: copy of the TX_ABRT_SOURCE register
  * @irq: interrupt number for the i2c master
+ * @swab: true if the instantiated IP is of different endianess
  * @adapter: i2c subsystem adapter node
  * @tx_fifo_depth: depth of the hardware tx fifo
  * @rx_fifo_depth: depth of the hardware rx fifo

@@ -215,6 +217,7 @@ struct dw_i2c_dev {
 	unsigned int		status;
 	u32			abort_source;
 	int			irq;
+	int			swab;
 	struct i2c_adapter	adapter;
 	unsigned int		tx_fifo_depth;
 	unsigned int		rx_fifo_depth;

@@ -222,11 +225,19 @@ struct dw_i2c_dev {
 
 static u32 dw_readl(struct dw_i2c_dev *dev, int offset)
 {
-	return readl(dev->base + offset);
+	u32 value = readl(dev->base + offset);
+
+	if (dev->swab)
+		return swab32(value);
+	else
+		return value;
 }
 
 static void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
 {
+	if (dev->swab)
+		b = swab32(b);
+
 	writel(b, dev->base + offset);
 }
 

@@ -759,7 +770,9 @@ static int __devinit dw_i2c_probe(struct
 	{
 		u32 comp_type = dw_readl(dev, DW_IC_COMP_TYPE);
 
-		if (comp_type != 0x44570140) {
+		if (comp_type == ___constant_swab32(0x44570140))
+			dev->swab = 1;
+		else if (comp_type != 0x44570140) {
 			dev_err(&pdev->dev, "Unknown Synopsys component type: "
 					"0x%08x\n",	comp_type);
 			r = -ENODEV;

A message that is not in the archive (sent off-list or not yet received): [email protected]

#7 lore raw
Quoted 12 lines · Jean-Hugues Deschenes

This is the v2 version of the patch which adds support for the i2c Designware controller when its endianness differs from that of the cpu.

---
Changelog v1->v2
- rename i2c_dw_(read|write)l to dw_(read|write)l (Shinya Kuribayashi)
- rename dw_(read|write)l's addr argument to offset (Shinya Kuribayashi)
- replace (if constant == x) with (if x == constant) (Ben Dooks)
---

Quoted 1 line
Signed-off-by Jean-Hugues Deschenes <[email protected]>

All three patches look good, and work for my BE machines.

Signed-off-by Shinya Kuribayashi <[email protected]>

---

drivers/i2c/busses/i2c-designware.c +0 -0
index 362dd9e..ce20fc8 100644
@@ -778,6 +783,12 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)

			r = -ENODEV;
			goto err_iounmap;
		}

+
+ printk("### comp_type = 0x%08x\n", comp_type);
+ printk("### ___constant_swab32(0x44570140) = 0x%08x\n",
+ ___constant_swab32(0x44570140));
+ printk("### swab = %d\n", dev->swab);
+

	}
	{
		u32 param1 = dw_readl(dev, DW_IC_COMP_PARAM_1);

[...]

i2c /dev entries driver
### comp_type = 0x44570140
### ___constant_swab32(0x44570140) = 0x40015744
### swab = 0
### comp_type = 0x44570140
### ___constant_swab32(0x44570140) = 0x40015744
### swab = 0
### comp_type = 0x44570140
### ___constant_swab32(0x44570140) = 0x40015744
### swab = 0

Signature · 2 lines
Shinya Kuribayashi
NEC Electronics

A message that is not in the archive (sent off-list or not yet received): [email protected]

Acked-by
#9 lore raw
Quoted 13 lines · Shinya Kuribayashi
Quoted 12 lines · Jean-Hugues Deschenes

This is the v2 version of the patch which adds support for the i2c Designware controller when its endianness differs from that of the cpu.

---
Changelog v1->v2
- rename i2c_dw_(read|write)l to dw_(read|write)l (Shinya Kuribayashi)
- rename dw_(read|write)l's addr argument to offset (Shinya Kuribayashi)
- replace (if constant == x) with (if x == constant) (Ben Dooks)
---

Quoted 5 lines
Quoted 1 line
Signed-off-by Jean-Hugues Deschenes <[email protected]>

All three patches look good, and work for my BE machines.

Signed-off-by Shinya Kuribayashi <[email protected]>

Oops, wrong tag.

Acked-by Shinya Kuribayashi <[email protected]>
Signature · 2 lines
Shinya Kuribayashi
NEC Electronics
#10 lore raw
In reply to a message not in the archive
Quoted 4 lines · Jean-Hugues Deschenes

This is the v2 version of the patch which adds support for the i2c Designware controller when its endianness differs from that of the cpu.

apart from the one comment on the {}. the rest is fine.

Quoted 16 lines

---
Changelog v1->v2
- rename i2c_dw_(read|write)l to dw_(read|write)l (Shinya Kuribayashi)
- rename dw_(read|write)l's addr argument to offset (Shinya Kuribayashi)
- replace (if constant == x) with (if x == constant) (Ben Dooks)
---

Signed-off-by Jean-Hugues Deschenes <[email protected]>
Signature · 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
Ben ([email protected], http://www.fluff.org/)

  'a smiley only costs 4 bytes'