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

Previous thread: [git pull] drm-intel fixes for 2.6.34-rc4 by Eric Anholt on Thursday, April 22, 2010 - 2:58 pm. (2 messages)

Next thread: CONTACT ME VIA/WILLIAMWILCOX1943@HOTMAIL.COM by Rivera, Alicia on Thursday, April 22, 2010 - 2:45 pm. (1 message)
From: Farid Hammane
Date: Thursday, April 22, 2010 - 3:01 pm

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@gmail.com>
---
 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 @@ ...
From: Wolfram Sang
Date: Thursday, April 22, 2010 - 5:52 pm

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

Thanks!

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
From: Jean Delvare
Date: Friday, April 23, 2010 - 2:33 am

Hi Farid, Wolfram,


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)


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

From: Farid Hammane
Date: Saturday, April 24, 2010 - 7:07 am

Hi Jean,



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 !"


Thanks for you comments and for sharing your knowledge !

A patch V3 will be sent,

Regards,
Farid,
--

From: Wolfram Sang
Date: Monday, April 26, 2010 - 12:41 am

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/  |
Previous thread: [git pull] drm-intel fixes for 2.6.34-rc4 by Eric Anholt on Thursday, April 22, 2010 - 2:58 pm. (2 messages)

Next thread: CONTACT ME VIA/WILLIAMWILCOX1943@HOTMAIL.COM by Rivera, Alicia on Thursday, April 22, 2010 - 2:45 pm. (1 message)