Skip to contentSkip to conversation

i2c: Adding mangling capability to i2c imx bus controller.

PATCH In review Fabien Marteau 2 participants 2 messages Jan 31, 2011 – Mar 2, 2011

Subsystems (3)
Arm i.MX and Layerscape I2C subsystem host drivers Freescale i.MX I2C driver
lore
Share & tools
lore

Fabien Marteau

author
#0 lore raw

PATCH 1/1 i2c: Adding mangling capability to i2c imx bus controller.

Adding Mangling capability to i2c imx bus controller.

Signed-off-by Fabien Marteau <[email protected]>

---

drivers/i2c/busses/i2c-imx.c +45 -25
Index: linux-2.6.36/drivers/i2c/busses/i2c-imx.c
===================================================================
@@ -300,17 +300,28 @@
 {
 	int i, result;
 
-	dev_dbg(&i2c_imx->adapter.dev, "<%s> write slave address: addr=0x%x\n",
-		__func__, msgs->addr << 1);
+	if ((msgs->flags & I2C_M_NOSTART) == 0) {
+		/* write slave address */
+		if (msgs->flags & I2C_M_REV_DIR_ADDR) {
+			dev_dbg(&i2c_imx->adapter.dev,
+				"<%s> write slave address: addr=0x%x\n",
+				__func__, msgs->addr << 1 | 0x01);
+			writeb((msgs->addr << 1) | 0x01,
+			       i2c_imx->base + IMX_I2C_I2DR);
+		} else {
+			dev_dbg(&i2c_imx->adapter.dev,
+				"<%s> write slave address: addr=0x%x\n",
+				__func__, msgs->addr << 1);
+			writeb((msgs->addr << 1), i2c_imx->base + IMX_I2C_I2DR);
+		}
+		result = i2c_imx_trx_complete(i2c_imx);
+		if (result)
+			return result;
+		result = i2c_imx_acked(i2c_imx);
+		if (result != 0)
+			return result;
+	}
 
-	/* write slave address */
-	writeb(msgs->addr << 1, i2c_imx->base + IMX_I2C_I2DR);
-	result = i2c_imx_trx_complete(i2c_imx);
-	if (result)
-		return result;
-	result = i2c_imx_acked(i2c_imx);
-	if (result)
-		return result;
 	dev_dbg(&i2c_imx->adapter.dev, "<%s> write data\n", __func__);
 
 	/* write data */

@@ -323,7 +334,7 @@
 		if (result)
 			return result;
 		result = i2c_imx_acked(i2c_imx);
-		if (result)
+		if ((result != 0) && ((msgs[0].flags & I2C_M_IGNORE_NAK) == 0))
 			return result;
 	}
 	return 0;

@@ -334,18 +345,27 @@
 	int i, result;
 	unsigned int temp;
 
-	dev_dbg(&i2c_imx->adapter.dev,
-		"<%s> write slave address: addr=0x%x\n",
-		__func__, (msgs->addr << 1) | 0x01);
-
-	/* write slave address */
-	writeb((msgs->addr << 1) | 0x01, i2c_imx->base + IMX_I2C_I2DR);
-	result = i2c_imx_trx_complete(i2c_imx);
-	if (result)
-		return result;
-	result = i2c_imx_acked(i2c_imx);
-	if (result)
-		return result;
+	if ((msgs->flags & I2C_M_NOSTART) == 0) {
+		/* write slave address */
+		if (msgs->flags & I2C_M_REV_DIR_ADDR) {
+			dev_dbg(&i2c_imx->adapter.dev,
+			"<%s> write slave address: addr=0x%x\n",
+			__func__, (msgs->addr << 1));
+			writeb((msgs->addr << 1), i2c_imx->base + IMX_I2C_I2DR);
+		} else {
+			dev_dbg(&i2c_imx->adapter.dev,
+			"<%s> write slave address: addr=0x%x\n",
+			__func__, (msgs->addr << 1) | 0x01);
+			writeb((msgs->addr << 1) | 0x01,
+			       i2c_imx->base + IMX_I2C_I2DR);
+		}
+		result = i2c_imx_trx_complete(i2c_imx);
+		if (result)
+			return result;
+		result = i2c_imx_acked(i2c_imx);
+		if (result != 0)
+			return result;
+	}
 
 	dev_dbg(&i2c_imx->adapter.dev, "<%s> setup bus\n", __func__);
 

@@ -405,7 +425,7 @@
 
 	/* read/write data */
 	for (i = 0; i < num; i++) {
-		if (i) {
+		if (i && ((msgs[i].flags & I2C_M_NOSTART) == 0)) {
 			dev_dbg(&i2c_imx->adapter.dev,
 				"<%s> repeated start\n", __func__);
 			temp = readb(i2c_imx->base + IMX_I2C_I2CR);

@@ -454,7 +474,7 @@
 
 static u32 i2c_imx_func(struct i2c_adapter *adapter)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
 }
 
 static struct i2c_algorithm i2c_imx_algo = {
#1 lore raw

Hi Fabien,

thanks for your patch. Nice addition.

Quoted 1 line · Fabien Marteau

Adding Mangling capability to i2c imx bus controller.

Can you give a rough description how you tested it?

drivers/i2c/busses/i2c-imx.c

Quoted 7 lines
Signed-off-by Fabien Marteau <[email protected]>

---
Index: linux-2.6.36/drivers/i2c/busses/i2c-imx.c
===================================================================

drivers/i2c/busses/i2c-imx.c +0 -0
@@ -300,17 +300,28 @@

Please use "--show-c-function" for diff/quilt. This makes reviewing a tad easier.

drivers/i2c/busses/i2c-imx.c line 303

Quoted 6 lines
drivers/i2c/busses/i2c-imx.c +1 -2
 {
 	int i, result;
 
-	dev_dbg(&i2c_imx->adapter.dev, "<%s> write slave address: addr=0x%x\n",
-		__func__, msgs->addr << 1);
+	if ((msgs->flags & I2C_M_NOSTART) == 0) {

Please use !flag instead of == 0 as it is done in the rest of the driver.

drivers/i2c/busses/i2c-imx.c line 316

Quoted 13 lines
drivers/i2c/busses/i2c-imx.c +13 -0
+		/* write slave address */
+		if (msgs->flags & I2C_M_REV_DIR_ADDR) {
+			dev_dbg(&i2c_imx->adapter.dev,
+				"<%s> write slave address: addr=0x%x\n",
+				__func__, msgs->addr << 1 | 0x01);
+			writeb((msgs->addr << 1) | 0x01,
+			       i2c_imx->base + IMX_I2C_I2DR);
+		} else {
+			dev_dbg(&i2c_imx->adapter.dev,
+				"<%s> write slave address: addr=0x%x\n",
+				__func__, msgs->addr << 1);
+			writeb((msgs->addr << 1), i2c_imx->base + IMX_I2C_I2DR);
+		}

What about:

	dev_dbg();
	writeb(msgs->addr << 1 | !!(msg->flags & I2C_M_REV_DIR_ADDR));

Hmm, probably more readable would be

	write_flag = (msg->flags & I2C_M_REV_DIR_ADDR) ? I2C_SMBUS_READ : I2C_SMBUS_WRITE

and use that? Also, not sure about the additional dev_dbg.

drivers/i2c/busses/i2c-imx.c line 321

Quoted 5 lines
drivers/i2c/busses/i2c-imx.c +5 -0
+		result = i2c_imx_trx_complete(i2c_imx);
+		if (result)
+			return result;
+		result = i2c_imx_acked(i2c_imx);
+		if (result != 0)

if (result)

Quoted 15 lines
drivers/i2c/busses/i2c-imx.c +2 -8
+			return result;
+	}
 
-	/* write slave address */
-	writeb(msgs->addr << 1, i2c_imx->base + IMX_I2C_I2DR);
-	result = i2c_imx_trx_complete(i2c_imx);
-	if (result)
-		return result;
-	result = i2c_imx_acked(i2c_imx);
-	if (result)
-		return result;
 	dev_dbg(&i2c_imx->adapter.dev, "<%s> write data\n", __func__);
 
 	/* write data */

drivers/i2c/busses/i2c-imx.c line 337

Quoted 5 lines
drivers/i2c/busses/i2c-imx.c +1 -1
@@ -323,7 +334,7 @@
 		if (result)
 			return result;
 		result = i2c_imx_acked(i2c_imx);
-		if (result)
+		if ((result != 0) && ((msgs[0].flags & I2C_M_IGNORE_NAK) == 0))

if (result && !(msgs[0].flags & I2C_M_IGNORE_NAK))

Quoted 10 lines
drivers/i2c/busses/i2c-imx.c +0 -3
 			return result;
 	}
 	return 0;

@@ -334,18 +345,27 @@
 	int i, result;
 	unsigned int temp;
 
-	dev_dbg(&i2c_imx->adapter.dev,
-		"<%s> write slave address: addr=0x%x\n",
-		__func__, (msgs->addr << 1) | 0x01);

drivers/i2c/busses/i2c-imx.c line 348

Quoted 10 lines
drivers/i2c/busses/i2c-imx.c +1 -9
-
-	/* write slave address */
-	writeb((msgs->addr << 1) | 0x01, i2c_imx->base + IMX_I2C_I2DR);
-	result = i2c_imx_trx_complete(i2c_imx);
-	if (result)
-		return result;
-	result = i2c_imx_acked(i2c_imx);
-	if (result)
-		return result;
+	if ((msgs->flags & I2C_M_NOSTART) == 0) {

!flag

drivers/i2c/busses/i2c-imx.c line 361

Quoted 13 lines
drivers/i2c/busses/i2c-imx.c +13 -0
+		/* write slave address */
+		if (msgs->flags & I2C_M_REV_DIR_ADDR) {
+			dev_dbg(&i2c_imx->adapter.dev,
+			"<%s> write slave address: addr=0x%x\n",
+			__func__, (msgs->addr << 1));
+			writeb((msgs->addr << 1), i2c_imx->base + IMX_I2C_I2DR);
+		} else {
+			dev_dbg(&i2c_imx->adapter.dev,
+			"<%s> write slave address: addr=0x%x\n",
+			__func__, (msgs->addr << 1) | 0x01);
+			writeb((msgs->addr << 1) | 0x01,
+			       i2c_imx->base + IMX_I2C_I2DR);
+		}

Please fix similar to above.

drivers/i2c/busses/i2c-imx.c line 321

Quoted 5 lines
drivers/i2c/busses/i2c-imx.c +5 -0
+		result = i2c_imx_trx_complete(i2c_imx);
+		if (result)
+			return result;
+		result = i2c_imx_acked(i2c_imx);
+		if (result != 0)

if (result)

drivers/i2c/busses/i2c-imx.c line 428

Quoted 11 lines
drivers/i2c/busses/i2c-imx.c +3 -1
+			return result;
+	}
 
 	dev_dbg(&i2c_imx->adapter.dev, "<%s> setup bus\n", __func__);
 

@@ -405,7 +425,7 @@
 
 	/* read/write data */
 	for (i = 0; i < num; i++) {
-		if (i) {
+		if (i && ((msgs[i].flags & I2C_M_NOSTART) == 0)) {

if (i && !(flag))

(Please check for other occasions, I might have missed one)

Quoted 13 lines
drivers/i2c/busses/i2c-imx.c +1 -2
 			dev_dbg(&i2c_imx->adapter.dev,
 				"<%s> repeated start\n", __func__);
 			temp = readb(i2c_imx->base + IMX_I2C_I2CR);

@@ -454,7 +474,7 @@
 
 static u32 i2c_imx_func(struct i2c_adapter *adapter)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
 }
 
 static struct i2c_algorithm i2c_imx_algo = {
--
Quoted 4 lines

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Regards,

Wolfram

Signature · 2 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |