Add restart support to i2c-pnx

Add restart support to i2c-pnx

[matej.kupljen-nWYkTt+LGLL1KXRcyAk9cg]

To all,

this patch adds support for restart to the i2c-pnx.c code.

Any comments?

Regards,
Matej

Re: Add restart support to i2c-pnx

[Vitaly Wool]

Hi,

On Wed, Feb 2, 2011 at 11:41 AM,  <matej.kupljen-nWYkTt+LGLL1KXRcyAk9cg@public.gmane.org> wrote:
> To all,
>
> this patch adds support for restart to the i2c-pnx.c code.
>

Sorry, can't see the patch itself :)

Thanks,
   Vitaly

Re: Add restart support to i2c-pnx

[matej.kupljen-nWYkTt+LGLL1KXRcyAk9cg]

Hi,

>> To all,
>>
>> this patch adds support for restart to the i2c-pnx.c code.
>>
> 
> Sorry, can't see the patch itself :)

Hit send to quick :D

I hope now you can see it.

Regards,
Matej

Re: Add restart support to i2c-pnx

[Jean Delvare]

On Wed, 02 Feb 2011 12:43:03 +0100, matej.kupljen-nWYkTt+LGLL1KXRcyAk9cg@public.gmane.org wrote:
> Hi,
> 
> >> To all,
> >>
> >> this patch adds support for restart to the i2c-pnx.c code.
> >>
> > 
> > Sorry, can't see the patch itself :)
> 
> Hit send to quick :D
> 
> I hope now you can see it.

No, we still can't.

-- 
Jean Delvare

Re: Add restart support to i2c-pnx

[matej.kupljen-nWYkTt+LGLL1KXRcyAk9cg]

Hi,

>> >> To all,
>> >>
>> >> this patch adds support for restart to the i2c-pnx.c code.
>> >>
>> > 
>> > Sorry, can't see the patch itself :)
>> 
>> Hit send to quick :D
>> 
>> I hope now you can see it.
> 
> No, we still can't.

Something is removing my attachments :(

This time in-line:
---------------------------------------------------------------

Current code in i2c-pnx.c doesn't support sending messages
with restart condition in it, for example:
S - (address | W) - Sr - (address | R) - ... - P

This patch adds this to the current code. It has been
tested on a custom board with LPC3152 CPU (similar to
Embedded Artists EA313x board) with uBlox GPS receiver
connected to I2C bus. It has been running over the night
without errors. 
Also, the patch extends the timeout value, but this was
only needed, because there are a lot of data to be transferred
and the timeout occurred before all the data was transferred.

The pathch is against 2.6.28.2, but it should apply to the
cureent GIT version as well.

Signed-off-by: Matej Kupljen <matej.kupljen-nWYkTt+LGLL1KXRcyAk9cg@public.gmane.org>

--- i2c-pnx-orig.c	2010-12-14 17:48:40.000000000 +0100
+++ i2c-pnx.c	2011-02-01 23:48:42.000000000 +0100
@@ -54,7 +54,8 @@
 {
 	struct i2c_pnx_algo_data *data = adap->algo_data;
 	struct timer_list *timer = &data->mif.timer;
-	int expires = I2C_PNX_TIMEOUT / (1000 / HZ);
+	// We should probably take into account, how many bytes we need to
transfer!
+	int expires = I2C_PNX_TIMEOUT * 10 / (1000 / HZ);
 
 	del_timer_sync(timer);
 
@@ -74,7 +75,7 @@
  *
  * Generate a START signal in the desired mode.
  */
-static int i2c_pnx_start(unsigned char slave_addr, struct i2c_adapter
*adap)
+static int i2c_pnx_start(unsigned char slave_addr, struct i2c_adapter
*adap, int repeated)
 {
 	struct i2c_pnx_algo_data *alg_data = adap->algo_data;
 
@@ -89,8 +90,8 @@
 		return -EINVAL;
 	}
 
-	/* First, make sure bus is idle */
-	if (wait_timeout(I2C_PNX_TIMEOUT, alg_data)) {
+	/* First, make sure bus is idle, if we are not doing repeated start */
+	if ((!repeated) && (wait_timeout(I2C_PNX_TIMEOUT, alg_data))) {
 		/* Somebody else is monopolizing the bus */
 		dev_err(&adap->dev, "%s: Bus busy. Slave addr = %02x, "
 		       "cntrl = %x, stat = %x\n",
@@ -439,7 +440,7 @@
 i2c_pnx_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	struct i2c_msg *pmsg;
-	int rc = 0, completed = 0, i;
+	int rc = 0, completed = 0, i, repeated;
 	struct i2c_pnx_algo_data *alg_data = adap->algo_data;
 	u32 stat = ioread32(I2C_REG_STS(alg_data));
 
@@ -454,6 +455,7 @@
 
 		pmsg = &msgs[i];
 		addr = pmsg->addr;
+		repeated = 0;
 
 		if (pmsg->flags & I2C_M_TEN) {
 			dev_err(&adap->dev,
@@ -479,16 +481,22 @@
 		/* initialize the completion var */
 		init_completion(&alg_data->mif.complete);
 
-		/* Enable master interrupt */
-		iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_afie |
-				mcntrl_naie | mcntrl_drmie,
-			  I2C_REG_CTL(alg_data));
+		/* If this is not the only message, then we need to
+		 * send repeated start, but only if the flaga allow us
+		 */
+		if((i > 0) && !(pmsg->flags & I2C_M_NOSTART))
+			repeated = 1;
 
 		/* Put start-code and slave-address on the bus. */
-		rc = i2c_pnx_start(addr, adap);
+		rc = i2c_pnx_start(addr, adap, repeated);
 		if (rc < 0)
 			break;
 
+		/* Enable master interrupt */
+		iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_afie |
+				mcntrl_naie | mcntrl_drmie,
+			  I2C_REG_CTL(alg_data));
+
 		/* Wait for completion */
 		wait_for_completion(&alg_data->mif.complete);
 

RE: Add restart support to i2c-pnx

[Kevin Wells]

Hi Matel,

Thanks for posting this patch. See comments below..
I'll be happy to test your changes once the following items
are clarified.

> The pathch is against 2.6.28.2, but it should apply to the
> cureent GIT version as well.

This patch needs to be re-based against the latest version
of the driver as it has changes that this patch will revert
or won't apply to.

> 
> Signed-off-by: Matej Kupljen <matej.kupljen@maprel.si>
> 
> --- i2c-pnx-orig.c	2010-12-14 17:48:40.000000000 +0100
> +++ i2c-pnx.c	2011-02-01 23:48:42.000000000 +0100
> @@ -54,7 +54,8 @@
>  {
>  	struct i2c_pnx_algo_data *data = adap->algo_data;
>  	struct timer_list *timer = &data->mif.timer;
> -	int expires = I2C_PNX_TIMEOUT / (1000 / HZ);
> +	// We should probably take into account, how many bytes we need to
> transfer!

Use /* */ for comments instead of //
Regarding this comment, you should remove it or add some
support for size based timeouts. The minimum timeout with the
latest updates will always be 10mS waiting for a stop condition
or the bus to clear, but the timeout doesn't come into play for
the actual transfer.

> +	int expires = I2C_PNX_TIMEOUT * 10 / (1000 / HZ);

The logic for handling timeout was changed in a later version of
i2c-pnx driver. This might of already fixed the timeout issue.

> 
>  	del_timer_sync(timer);
> 
> @@ -74,7 +75,7 @@
>   *
>   * Generate a START signal in the desired mode.
>   */
> -static int i2c_pnx_start(unsigned char slave_addr, struct i2c_adapter
> *adap)
> +static int i2c_pnx_start(unsigned char slave_addr, struct i2c_adapter
> *adap, int repeated)
>  {
>  	struct i2c_pnx_algo_data *alg_data = adap->algo_data;
> 
> @@ -89,8 +90,8 @@
>  		return -EINVAL;
>  	}
> 
> -	/* First, make sure bus is idle */
> -	if (wait_timeout(I2C_PNX_TIMEOUT, alg_data)) {
> +	/* First, make sure bus is idle, if we are not doing repeated start
> */
> +	if ((!repeated) && (wait_timeout(I2C_PNX_TIMEOUT, alg_data))) {
>  		/* Somebody else is monopolizing the bus */
>  		dev_err(&adap->dev, "%s: Bus busy. Slave addr = %02x, "
>  		       "cntrl = %x, stat = %x\n",
> @@ -439,7 +440,7 @@
>  i2c_pnx_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>  {
>  	struct i2c_msg *pmsg;
> -	int rc = 0, completed = 0, i;
> +	int rc = 0, completed = 0, i, repeated;
>  	struct i2c_pnx_algo_data *alg_data = adap->algo_data;
>  	u32 stat = ioread32(I2C_REG_STS(alg_data));
> 
> @@ -454,6 +455,7 @@
> 
>  		pmsg = &msgs[i];
>  		addr = pmsg->addr;
> +		repeated = 0;
> 
>  		if (pmsg->flags & I2C_M_TEN) {
>  			dev_err(&adap->dev,
> @@ -479,16 +481,22 @@
>  		/* initialize the completion var */
>  		init_completion(&alg_data->mif.complete);
> 
> -		/* Enable master interrupt */
> -		iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_afie |
> -				mcntrl_naie | mcntrl_drmie,
> -			  I2C_REG_CTL(alg_data));
> +		/* If this is not the only message, then we need to
> +		 * send repeated start, but only if the flaga allow us
> +		 */

Comments than span multiple lines should be in the format
/*
 * Line 1
 * Line 2
 */
Single comment lines are in the format
/* Line 1 */
'Flaga allow us' should be 'flag allows it'

> +		if((i > 0) && !(pmsg->flags & I2C_M_NOSTART))
> +			repeated = 1;

All the repeated start logic seems like it will work ok.

> 
>  		/* Put start-code and slave-address on the bus. */
> -		rc = i2c_pnx_start(addr, adap);
> +		rc = i2c_pnx_start(addr, adap, repeated);

RE: Add restart support to i2c-pnx

[Matej Kupljen]

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

Kevin,

> Thanks for posting this patch. See comments below..
> I'll be happy to test your changes once the following items
> are clarified.
> 
> > The pathch is against 2.6.28.2, but it should apply to the
> > cureent GIT version as well.
> 
> This patch needs to be re-based against the latest version
> of the driver as it has changes that this patch will revert
> or won't apply to.

I re-based it against latest version of the kernel in GIT and 
also included your comments in the mail.

Is it O.K. now?

Regards,
Matej

P.S.: Changed the email client. I now attached the file as 
well as pasted it in-line.

-------------------------------------------------------------------
Current code in i2c-pnx.c doesn't support sending messages
with restart condition in it, for example:
S - (address | W) - Sr - (address | R) - ... - P

This patch adds this to the current code. It has been
tested on a custom board with LPC3152 CPU (similar to
Embedded Artists EA313x board) with uBlox GPS receiver
connected to I2C bus. It has been running over the night
without errors. 

Signed-off-by: Matej Kupljen <matej.kupljen-nWYkTt+LGLL1KXRcyAk9cg@public.gmane.org>

--- i2c-pnx-orig.c	2011-02-02 21:30:19.000000000 +0100
+++ i2c-pnx.c	2011-02-02 21:49:02.000000000 +0100
@@ -78,7 +78,7 @@
  * Generate a START signal in the desired mode.
  */
 static int i2c_pnx_start(unsigned char slave_addr,
-	struct i2c_pnx_algo_data *alg_data)
+	struct i2c_pnx_algo_data *alg_data, int repeated)
 {
 	dev_dbg(&alg_data->adapter.dev, "%s(): addr 0x%x mode %d\n", __func__,
 		slave_addr, alg_data->mif.mode);
@@ -91,8 +91,8 @@
 		return -EINVAL;
 	}
 
-	/* First, make sure bus is idle */
-	if (wait_timeout(I2C_PNX_TIMEOUT, alg_data)) {
+	/* First, make sure bus is idle, if we are not doing repeated start */
+	if ((!repeated) && (wait_timeout(I2C_PNX_TIMEOUT, alg_data))) {
 		/* Somebody else is monopolizing the bus */
 		dev_err(&alg_data->adapter.dev,
 			"%s: Bus busy. Slave addr = %02x, cntrl = %x, stat = %x\n",
@@ -441,7 +441,7 @@
 i2c_pnx_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	struct i2c_msg *pmsg;
-	int rc = 0, completed = 0, i;
+	int rc = 0, completed = 0, i, repeated;
 	struct i2c_pnx_algo_data *alg_data = adap->algo_data;
 	u32 stat = ioread32(I2C_REG_STS(alg_data));
 
@@ -457,6 +457,7 @@
 
 		pmsg = &msgs[i];
 		addr = pmsg->addr;
+		repeated = 0;
 
 		if (pmsg->flags & I2C_M_TEN) {
 			dev_err(&alg_data->adapter.dev,
@@ -481,16 +482,23 @@
 		/* initialize the completion var */
 		init_completion(&alg_data->mif.complete);
 
-		/* Enable master interrupt */
-		iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_afie |
-				mcntrl_naie | mcntrl_drmie,
-			  I2C_REG_CTL(alg_data));
+		/* 
+		 * If this is not the only message, then we need to
+		 * send repeated start, but only if the flag allows it
+		 */
+		if((i > 0) && !(pmsg->flags & I2C_M_NOSTART))
+			repeated = 1;
 
 		/* Put start-code and slave-address on the bus. */
-		rc = i2c_pnx_start(addr, alg_data);
+		rc = i2c_pnx_start(addr, alg_data, repeated);
 		if (rc < 0)
 			break;
 
+		/* Enable master interrupt */
+		iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_afie |
+				mcntrl_naie | mcntrl_drmie,
+			  I2C_REG_CTL(alg_data));
+
 		/* Wait for completion */
 		wait_for_completion(&alg_data->mif.complete);
 

[-- Attachment #2: i2c-pnx-restart-git.patch --]
[-- Type: text/x-patch, Size: 2640 bytes --]

Current code in i2c-pnx.c doesn't support sending messages
with restart condition in it, for example:
S - (address | W) - Sr - (address | R) - ... - P

This patch adds this to the current code. It has been
tested on a custom board with LPC3152 CPU (similar to
Embedded Artists EA313x board) with uBlox GPS receiver
connected to I2C bus. It has been running over the night
without errors. 

Signed-off-by: Matej Kupljen <matej.kupljen-nWYkTt+LGLL1KXRcyAk9cg@public.gmane.org>

--- i2c-pnx-orig.c	2011-02-02 21:30:19.000000000 +0100
+++ i2c-pnx.c	2011-02-02 21:49:02.000000000 +0100
@@ -78,7 +78,7 @@
  * Generate a START signal in the desired mode.
  */
 static int i2c_pnx_start(unsigned char slave_addr,
-	struct i2c_pnx_algo_data *alg_data)
+	struct i2c_pnx_algo_data *alg_data, int repeated)
 {
 	dev_dbg(&alg_data->adapter.dev, "%s(): addr 0x%x mode %d\n", __func__,
 		slave_addr, alg_data->mif.mode);
@@ -91,8 +91,8 @@
 		return -EINVAL;
 	}
 
-	/* First, make sure bus is idle */
-	if (wait_timeout(I2C_PNX_TIMEOUT, alg_data)) {
+	/* First, make sure bus is idle, if we are not doing repeated start */
+	if ((!repeated) && (wait_timeout(I2C_PNX_TIMEOUT, alg_data))) {
 		/* Somebody else is monopolizing the bus */
 		dev_err(&alg_data->adapter.dev,
 			"%s: Bus busy. Slave addr = %02x, cntrl = %x, stat = %x\n",
@@ -441,7 +441,7 @@
 i2c_pnx_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	struct i2c_msg *pmsg;
-	int rc = 0, completed = 0, i;
+	int rc = 0, completed = 0, i, repeated;
 	struct i2c_pnx_algo_data *alg_data = adap->algo_data;
 	u32 stat = ioread32(I2C_REG_STS(alg_data));
 
@@ -457,6 +457,7 @@
 
 		pmsg = &msgs[i];
 		addr = pmsg->addr;
+		repeated = 0;
 
 		if (pmsg->flags & I2C_M_TEN) {
 			dev_err(&alg_data->adapter.dev,
@@ -481,16 +482,23 @@
 		/* initialize the completion var */
 		init_completion(&alg_data->mif.complete);
 
-		/* Enable master interrupt */
-		iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_afie |
-				mcntrl_naie | mcntrl_drmie,
-			  I2C_REG_CTL(alg_data));
+		/* 
+		 * If this is not the only message, then we need to
+		 * send repeated start, but only if the flag allows it
+		 */
+		if((i > 0) && !(pmsg->flags & I2C_M_NOSTART))
+			repeated = 1;
 
 		/* Put start-code and slave-address on the bus. */
-		rc = i2c_pnx_start(addr, alg_data);
+		rc = i2c_pnx_start(addr, alg_data, repeated);
 		if (rc < 0)
 			break;
 
+		/* Enable master interrupt */
+		iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_afie |
+				mcntrl_naie | mcntrl_drmie,
+			  I2C_REG_CTL(alg_data));
+
 		/* Wait for completion */
 		wait_for_completion(&alg_data->mif.complete);
 

Re: Add restart support to i2c-pnx

[Ben Dooks]

On Wed, Feb 02, 2011 at 10:23:42PM +0100, Matej Kupljen wrote:
> Kevin,
> 
> > Thanks for posting this patch. See comments below..
> > I'll be happy to test your changes once the following items
> > are clarified.
> > 
> > > The pathch is against 2.6.28.2, but it should apply to the
> > > cureent GIT version as well.
> > 
> > This patch needs to be re-based against the latest version
> > of the driver as it has changes that this patch will revert
> > or won't apply to.
> 
> I re-based it against latest version of the kernel in GIT and 
> also included your comments in the mail.
> 
> Is it O.K. now?
> 
> Regards,
> Matej
> 
> P.S.: Changed the email client. I now attached the file as 
> well as pasted it in-line.
> 
> -------------------------------------------------------------------
> Current code in i2c-pnx.c doesn't support sending messages
> with restart condition in it, for example:
> S - (address | W) - Sr - (address | R) - ... - P
> 
> This patch adds this to the current code. It has been
> tested on a custom board with LPC3152 CPU (similar to
> Embedded Artists EA313x board) with uBlox GPS receiver
> connected to I2C bus. It has been running over the night
> without errors. 
> 
> Signed-off-by: Matej Kupljen <matej.kupljen-nWYkTt+LGLL1KXRcyAk9cg@public.gmane.org>
> 
> --- i2c-pnx-orig.c	2011-02-02 21:30:19.000000000 +0100
> +++ i2c-pnx.c	2011-02-02 21:49:02.000000000 +0100
> @@ -78,7 +78,7 @@
>   * Generate a START signal in the desired mode.
>   */
>  static int i2c_pnx_start(unsigned char slave_addr,
> -	struct i2c_pnx_algo_data *alg_data)
> +	struct i2c_pnx_algo_data *alg_data, int repeated)

bool repeated.

>  {
>  	dev_dbg(&alg_data->adapter.dev, "%s(): addr 0x%x mode %d\n", __func__,
>  		slave_addr, alg_data->mif.mode);
> @@ -91,8 +91,8 @@
>  		return -EINVAL;
>  	}
>  
> -	/* First, make sure bus is idle */
> -	if (wait_timeout(I2C_PNX_TIMEOUT, alg_data)) {
> +	/* First, make sure bus is idle, if we are not doing repeated start */
> +	if ((!repeated) && (wait_timeout(I2C_PNX_TIMEOUT, alg_data))) {

no need for () around !repeated.

>  		/* Somebody else is monopolizing the bus */
>  		dev_err(&alg_data->adapter.dev,
>  			"%s: Bus busy. Slave addr = %02x, cntrl = %x, stat = %x\n",
> @@ -441,7 +441,7 @@
>  i2c_pnx_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>  {
>  	struct i2c_msg *pmsg;
> -	int rc = 0, completed = 0, i;
> +	int rc = 0, completed = 0, i, repeated;

see above.

>  	struct i2c_pnx_algo_data *alg_data = adap->algo_data;
>  	u32 stat = ioread32(I2C_REG_STS(alg_data));
>  
> @@ -457,6 +457,7 @@
>  
>  		pmsg = &msgs[i];
>  		addr = pmsg->addr;
> +		repeated = 0;
>  
>  		if (pmsg->flags & I2C_M_TEN) {
>  			dev_err(&alg_data->adapter.dev,
> @@ -481,16 +482,23 @@
>  		/* initialize the completion var */
>  		init_completion(&alg_data->mif.complete);
>  
> -		/* Enable master interrupt */
> -		iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_afie |
> -				mcntrl_naie | mcntrl_drmie,
> -			  I2C_REG_CTL(alg_data));
> +		/* 
> +		 * If this is not the only message, then we need to
> +		 * send repeated start, but only if the flag allows it
> +		 */
> +		if((i > 0) && !(pmsg->flags & I2C_M_NOSTART))

no need for () around i > 0

> +			repeated = 1;
>  
>  		/* Put start-code and slave-address on the bus. */
> -		rc = i2c_pnx_start(addr, alg_data);
> +		rc = i2c_pnx_start(addr, alg_data, repeated);
>  		if (rc < 0)
>  			break;
>  
> +		/* Enable master interrupt */
> +		iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_afie |
> +				mcntrl_naie | mcntrl_drmie,
> +			  I2C_REG_CTL(alg_data));
> +
>  		/* Wait for completion */
>  		wait_for_completion(&alg_data->mif.complete);
  

-- 
Ben Dooks, ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

Re: Add restart support to i2c-pnx

[matej.kupljen-nWYkTt+LGLL1KXRcyAk9cg]

Ben,

>> -	struct i2c_pnx_algo_data *alg_data)
>> +	struct i2c_pnx_algo_data *alg_data, int repeated)
> 
> bool repeated.

I don't agree.
Since this is C and C++, we do not have a bool keyword.
And, there is no variable declared as bool in the source file this 
is why and defined this as int and not as bool.

>> +	if ((!repeated) && (wait_timeout(I2C_PNX_TIMEOUT, alg_data))) {
> 
> no need for () around !repeated.

Agreed, but for the clarity sake.

>> +		if((i > 0) && !(pmsg->flags & I2C_M_NOSTART))
> 
> no need for () around i > 0

Same as above. I my opinion it is better to use more parentheses then
fewer, since you do not rely on compiler implementation.

Regards,
Matej

Re: Add restart support to i2c-pnx

[Vitaly Wool]

On Thu, Feb 3, 2011 at 8:54 AM,  <matej.kupljen-nWYkTt+LGLL1KXRcyAk9cg@public.gmane.org> wrote:
>
> Same as above. I my opinion it is better to use more parentheses then
> fewer, since you do not rely on compiler implementation.

What? This is not a compiler implementation, this is the C language _standard_.

~Vitaly

Re: Add restart support to i2c-pnx

[Ben Dooks]

On Thu, Feb 03, 2011 at 08:54:30AM +0100, matej.kupljen-nWYkTt+LGLL1KXRcyAk9cg@public.gmane.org wrote:
> Ben,
> 
> >> -	struct i2c_pnx_algo_data *alg_data)
> >> +	struct i2c_pnx_algo_data *alg_data, int repeated)
> > 
> > bool repeated.
> 
> I don't agree.
> Since this is C and C++, we do not have a bool keyword.
> And, there is no variable declared as bool in the source file this 
> is why and defined this as int and not as bool.

the kernel has this.
 
> >> +	if ((!repeated) && (wait_timeout(I2C_PNX_TIMEOUT, alg_data))) {
> > 
> > no need for () around !repeated.
> 
> Agreed, but for the clarity sake.

Makes it less clear, there's more on each line.
 
> >> +		if((i > 0) && !(pmsg->flags & I2C_M_NOSTART))
> > 
> > no need for () around i > 0
> 
> Same as above. I my opinion it is better to use more parentheses then
> fewer, since you do not rely on compiler implementation.
> 
> Regards,
> Matej
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ben Dooks, ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.