LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] i2c: Adding mangling capability to i2c imx bus controller.
@ 2011-01-31 15:29 Fabien Marteau
  2011-03-02 13:28 ` Wolfram Sang
  0 siblings, 1 reply; 2+ messages in thread
From: Fabien Marteau @ 2011-01-31 15:29 UTC (permalink / raw)
  To: Jean Delvare (PC drivers, core), Ben Dooks (embedded platforms),
	"Uwe Kleine-König",
	Arnaud Patard, Tejun Heo
  Cc: linux-i2c, linux-kernel

Adding Mangling capability to i2c imx bus controller.

Signed-off-by: Fabien Marteau <fabien.marteau@armadeus.com>
---
Index: linux-2.6.36/drivers/i2c/busses/i2c-imx.c
===================================================================
--- linux-2.6.36.orig/drivers/i2c/busses/i2c-imx.c	2010-10-20 22:30:22.000000000 +0200
+++ linux-2.6.36/drivers/i2c/busses/i2c-imx.c	2011-01-31 16:23:34.000000000 +0100
@@ -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 = {

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

* Re: [PATCH] i2c: Adding mangling capability to i2c imx bus controller.
  2011-01-31 15:29 [PATCH] i2c: Adding mangling capability to i2c imx bus controller Fabien Marteau
@ 2011-03-02 13:28 ` Wolfram Sang
  0 siblings, 0 replies; 2+ messages in thread
From: Wolfram Sang @ 2011-03-02 13:28 UTC (permalink / raw)
  To: Fabien Marteau
  Cc: Jean Delvare (PC drivers, core), Ben Dooks (embedded platforms),
	"Uwe Kleine-König",
	Arnaud Patard, Tejun Heo, linux-i2c, linux-kernel

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

Hi Fabien,

thanks for your patch. Nice addition.

On Mon, Jan 31, 2011 at 04:29:53PM +0100, Fabien Marteau wrote:
> Adding Mangling capability to i2c imx bus controller.

Can you give a rough description how you tested it?

> Signed-off-by: Fabien Marteau <fabien.marteau@armadeus.com>
> ---
> Index: linux-2.6.36/drivers/i2c/busses/i2c-imx.c
> ===================================================================
> --- linux-2.6.36.orig/drivers/i2c/busses/i2c-imx.c	2010-10-20 22:30:22.000000000 +0200
> +++ linux-2.6.36/drivers/i2c/busses/i2c-imx.c	2011-01-31 16:23:34.000000000 +0100
> @@ -300,17 +300,28 @@

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

>  {
>  	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.

> +		/* 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.

> +		result = i2c_imx_trx_complete(i2c_imx);
> +		if (result)
> +			return result;
> +		result = i2c_imx_acked(i2c_imx);
> +		if (result != 0)

if (result)

> +			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))

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

>  			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) {

!flag

> +		/* 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.

> +		result = i2c_imx_trx_complete(i2c_imx);
> +		if (result)
> +			return result;
> +		result = i2c_imx_acked(i2c_imx);
> +		if (result != 0)

if (result)

> +			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)

>  			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 = {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

end of thread, other threads:[~2011-03-02 13:28 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-31 15:29 [PATCH] i2c: Adding mangling capability to i2c imx bus controller Fabien Marteau
2011-03-02 13:28 ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).