[PATCH V2] i2c-algo-pca: fix coding style issues in i2c-algo-pca.c

[PATCH V2] i2c-algo-pca: fix coding style issues in i2c-algo-pca.c

[Farid Hammane]

This patch fixes coding style issues found by checkpatch.pl.
i2c-algo-pca.c has been built successfully after applying this patch.

Signed-off-by: Farid Hammane <farid.hammane-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/algos/i2c-algo-pca.c |   36 ++++++++++++++++++------------------
 1 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/algos/i2c-algo-pca.c b/drivers/i2c/algos/i2c-algo-pca.c
index dcdaf8e..ca817f8 100644
--- a/drivers/i2c/algos/i2c-algo-pca.c
+++ b/drivers/i2c/algos/i2c-algo-pca.c
@@ -37,15 +37,15 @@
 
 static int i2c_debug;
 
-#define pca_outw(adap, reg, val) adap->write_byte(adap->data, reg, val)
-#define pca_inw(adap, reg) adap->read_byte(adap->data, reg)
+#define pca_outw(adap, reg, val) (adap->write_byte(adap->data, reg, val))
+#define pca_inw(adap, reg) (adap->read_byte(adap->data, reg))
 
 #define pca_status(adap) pca_inw(adap, I2C_PCA_STA)
-#define pca_clock(adap) adap->i2c_clock
+#define pca_clock(adap) (adap->i2c_clock)
 #define pca_set_con(adap, val) pca_outw(adap, I2C_PCA_CON, val)
 #define pca_get_con(adap) pca_inw(adap, I2C_PCA_CON)
-#define pca_wait(adap) adap->wait_for_completion(adap->data)
-#define pca_reset(adap) adap->reset_chip(adap->data)
+#define pca_wait(adap) (adap->wait_for_completion(adap->data))
+#define pca_reset(adap) (adap->reset_chip(adap->data))
 
 static void pca9665_reset(void *pd)
 {
@@ -114,8 +114,8 @@ static int pca_address(struct i2c_algo_pca_data *adap,
 	int sta = pca_get_con(adap);
 	int addr;
 
-	addr = ( (0x7f & msg->addr) << 1 );
-	if (msg->flags & I2C_M_RD )
+	addr = ((0x7f & msg->addr) << 1);
+	if (msg->flags & I2C_M_RD)
 		addr |= 1;
 	DEB2("=== SLAVE ADDRESS %#04x+%c=%#04x\n",
 	     msg->addr, msg->flags & I2C_M_RD ? 'R' : 'W', addr);
@@ -170,7 +170,7 @@ static int pca_rx_ack(struct i2c_algo_pca_data *adap,
 
 	sta &= ~(I2C_PCA_CON_STO|I2C_PCA_CON_STA|I2C_PCA_CON_SI|I2C_PCA_CON_AA);
 
-	if ( ack )
+	if (ack)
 		sta |= I2C_PCA_CON_AA;
 
 	pca_set_con(adap, sta);
@@ -181,9 +181,9 @@ static int pca_xfer(struct i2c_adapter *i2c_adap,
                     struct i2c_msg *msgs,
                     int num)
 {
-        struct i2c_algo_pca_data *adap = i2c_adap->algo_data;
-        struct i2c_msg *msg = NULL;
-        int curmsg;
+	struct i2c_algo_pca_data *adap = i2c_adap->algo_data;
+	struct i2c_msg *msg = NULL;
+	int curmsg;
 	int numbytes = 0;
 	int state;
 	int ret;
@@ -202,21 +202,21 @@ static int pca_xfer(struct i2c_adapter *i2c_adap,
 
 	DEB1("{{{ XFER %d messages\n", num);
 
-	if (i2c_debug>=2) {
+	if (i2c_debug >= 2) {
 		for (curmsg = 0; curmsg < num; curmsg++) {
 			int addr, i;
 			msg = &msgs[curmsg];
 
 			addr = (0x7f & msg->addr) ;
 
-			if (msg->flags & I2C_M_RD )
+			if (msg->flags & I2C_M_RD)
 				printk(KERN_INFO "    [%02d] RD %d bytes from %#02x [%#02x, ...]\n",
-				       curmsg, msg->len, addr, (addr<<1) | 1);
+				       curmsg, msg->len, addr, (addr << 1) | 1);
 			else {
 				printk(KERN_INFO "    [%02d] WR %d bytes to %#02x [%#02x%s",
-				       curmsg, msg->len, addr, addr<<1,
+				       curmsg, msg->len, addr, addr << 1,
 				       msg->len == 0 ? "" : ", ");
-				for(i=0; i < msg->len; i++)
+				for (i = 0; i < msg->len; i++)
 					printk("%#04x%s", msg->buf[i], i == msg->len - 1 ? "" : ", ");
 				printk("]\n");
 			}
@@ -305,7 +305,7 @@ static int pca_xfer(struct i2c_adapter *i2c_adap,
 			goto out;
 
 		case 0x58: /* Data byte has been received; NOT ACK has been returned */
-			if ( numbytes == msg->len - 1 ) {
+			if (numbytes == msg->len - 1) {
 				pca_rx_byte(adap, &msg->buf[numbytes], 0);
 				curmsg++; numbytes = 0;
 				if (curmsg == num)
@@ -352,7 +352,7 @@ static int pca_xfer(struct i2c_adapter *i2c_adap,
 
 static u32 pca_func(struct i2c_adapter *adap)
 {
-        return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
 }
 
 static const struct i2c_algorithm pca_algo = {
-- 
1.7.0

Re: [PATCH V2] i2c-algo-pca: fix coding style issues in i2c-algo-pca.c

[Wolfram Sang]

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

On Fri, Apr 23, 2010 at 12:01:55AM +0200, Farid Hammane wrote:
> This patch fixes coding style issues found by checkpatch.pl.
> i2c-algo-pca.c has been built successfully after applying this patch.
> 
> Signed-off-by: Farid Hammane <farid.hammane-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Thanks!

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

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

Re: [PATCH V2] i2c-algo-pca: fix coding style issues in i2c-algo-pca.c

[Jean Delvare]

Hi Farid, Wolfram,

On Fri, 23 Apr 2010 00:01:55 +0200, Farid Hammane wrote:
> This patch fixes coding style issues found by checkpatch.pl.
> i2c-algo-pca.c has been built successfully after applying this patch.
> 
> Signed-off-by: Farid Hammane <farid.hammane-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/i2c/algos/i2c-algo-pca.c |   36 ++++++++++++++++++------------------
>  1 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/i2c/algos/i2c-algo-pca.c b/drivers/i2c/algos/i2c-algo-pca.c
> index dcdaf8e..ca817f8 100644
> --- a/drivers/i2c/algos/i2c-algo-pca.c
> +++ b/drivers/i2c/algos/i2c-algo-pca.c
> @@ -37,15 +37,15 @@
>  
>  static int i2c_debug;
>  
> -#define pca_outw(adap, reg, val) adap->write_byte(adap->data, reg, val)
> -#define pca_inw(adap, reg) adap->read_byte(adap->data, reg)
> +#define pca_outw(adap, reg, val) (adap->write_byte(adap->data, reg, val))
> +#define pca_inw(adap, reg) (adap->read_byte(adap->data, reg))

I'm confused by these changes. For one thing, macros which are
shortcuts for function calls normally don't need surrounding
parentheses. If checkpatch.pl complains about that, I would call it a
false positive, unless someone can prove me wrong with a real-world
case where these surrounding parentheses help.

For another, macro _parameters_ normally need parentheses for each
non-trivial use. I would thus expect the following to be correct:

#define pca_outw(adap, reg, val) (adap)->write_byte((adap)->data, reg, val)
#define pca_inw(adap, reg) (adap)->read_byte((adap)->data, reg)

Or is it just me?

>  
>  #define pca_status(adap) pca_inw(adap, I2C_PCA_STA)
> -#define pca_clock(adap) adap->i2c_clock
> +#define pca_clock(adap) (adap->i2c_clock)
>  #define pca_set_con(adap, val) pca_outw(adap, I2C_PCA_CON, val)
>  #define pca_get_con(adap) pca_inw(adap, I2C_PCA_CON)
> -#define pca_wait(adap) adap->wait_for_completion(adap->data)
> -#define pca_reset(adap) adap->reset_chip(adap->data)
> +#define pca_wait(adap) (adap->wait_for_completion(adap->data))
> +#define pca_reset(adap) (adap->reset_chip(adap->data))

Same here...

I'm fine with all other changes. But checkpatch.pl spouts 2 more errors
to me, which we've discussed before. I'm curious why you didn't fix
them? Just replace each block of 8 spaces with one tab.

ERROR: code indent should use tabs where possible
#181: FILE: i2c/algos/i2c-algo-pca.c:181:
+                    struct i2c_msg *msgs,$

ERROR: code indent should use tabs where possible
#182: FILE: i2c/algos/i2c-algo-pca.c:182:
+                    int num)$

-- 
Jean Delvare

Re: [PATCH V2] i2c-algo-pca: fix coding style issues in i2c-algo-pca.c

[Farid Hammane]

Hi Jean,

On Friday 23 April 2010 11:33:26 you wrote:
> Hi Farid, Wolfram,
> 
> On Fri, 23 Apr 2010 00:01:55 +0200, Farid Hammane wrote:
> > This patch fixes coding style issues found by checkpatch.pl.
> > i2c-algo-pca.c has been built successfully after applying this patch.
> >
> > Signed-off-by: Farid Hammane <farid.hammane-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >  drivers/i2c/algos/i2c-algo-pca.c |   36
> > ++++++++++++++++++------------------ 1 files changed, 18 insertions(+),
> > 18 deletions(-)
> >
> > diff --git a/drivers/i2c/algos/i2c-algo-pca.c
> > b/drivers/i2c/algos/i2c-algo-pca.c index dcdaf8e..ca817f8 100644
> > --- a/drivers/i2c/algos/i2c-algo-pca.c
> > +++ b/drivers/i2c/algos/i2c-algo-pca.c
> > @@ -37,15 +37,15 @@
> >
> >  static int i2c_debug;
> >
> > -#define pca_outw(adap, reg, val) adap->write_byte(adap->data, reg, val)
> > -#define pca_inw(adap, reg) adap->read_byte(adap->data, reg)
> > +#define pca_outw(adap, reg, val) (adap->write_byte(adap->data, reg,
> > val)) +#define pca_inw(adap, reg) (adap->read_byte(adap->data, reg))
> 
> I'm confused by these changes. For one thing, macros which are
> shortcuts for function calls normally don't need surrounding
> parentheses. If checkpatch.pl complains about that, I would call it a
> false positive, unless someone can prove me wrong with a real-world
> case where these surrounding parentheses help.
> 
> For another, macro _parameters_ normally need parentheses for each
> non-trivial use. I would thus expect the following to be correct:
> 
> #define pca_outw(adap, reg, val) (adap)->write_byte((adap)->data, reg, val)
> #define pca_inw(adap, reg) (adap)->read_byte((adap)->data, reg)
> 
> Or is it just me?

You are right ! This will be deleted from the patch.

> 
> >  #define pca_status(adap) pca_inw(adap, I2C_PCA_STA)
> > -#define pca_clock(adap) adap->i2c_clock
> > +#define pca_clock(adap) (adap->i2c_clock)
> >  #define pca_set_con(adap, val) pca_outw(adap, I2C_PCA_CON, val)
> >  #define pca_get_con(adap) pca_inw(adap, I2C_PCA_CON)
> > -#define pca_wait(adap) adap->wait_for_completion(adap->data)
> > -#define pca_reset(adap) adap->reset_chip(adap->data)
> > +#define pca_wait(adap) (adap->wait_for_completion(adap->data))
> > +#define pca_reset(adap) (adap->reset_chip(adap->data))
> 
> Same here...
> 
> I'm fine with all other changes. But checkpatch.pl spouts 2 more errors
> to me, which we've discussed before. I'm curious why you didn't fix
> them? Just replace each block of 8 spaces with one tab.

I thought you expected just one tab and spaces. You said :
"Better use tab + spaces and align on the opening parenthesis. What
checkpatch.pl complains about here isn't the alignment, it's the use of
more than 8 consecutive spaces."
I understood here : "don't care about this warning !"

So, I'll fix it.

> 
> ERROR: code indent should use tabs where possible
> #181: FILE: i2c/algos/i2c-algo-pca.c:181:
> +                    struct i2c_msg *msgs,$
> 
> ERROR: code indent should use tabs where possible
> #182: FILE: i2c/algos/i2c-algo-pca.c:182:
> +                    int num)$
> 

Thanks for you comments and for sharing your knowledge !

A patch V3 will be sent,

Regards,
Farid,

Re: [PATCH V2] i2c-algo-pca: fix coding style issues in i2c-algo-pca.c

[Wolfram Sang]

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


Geez, that will teach me to review whitespace-fixes while working somewhere
else :( Thanks for the catch!

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

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