Skip to contentSkip to conversation

i2c-algo-pca: fix coding style issues in i2c-algo-pca.c

PATCH Superseded Farid Hammane 3 participants 5 messages Apr 22, 2010 – Apr 26, 2010 I2C subsystem host drivers

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Farid Hammane

author
#0 lore raw

PATCH v2 1/1 i2c-algo-pca: fix coding style issues in i2c-algo-pca.c

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 <[email protected]>
---
 drivers/i2c/algos/i2c-algo-pca.c |   36 ++++++++++++++++++------------------
 1 files changed, 18 insertions(+), 18 deletions(-)
drivers/i2c/algos/i2c-algo-pca.c +18 -18
index dcdaf8e..ca817f8 100644
@@ -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 = {
Signature · 1 line
1.7.0
Reviewed-by on v2 1/1
#1 lore raw
Quoted 4 lines · 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 <[email protected]>
Reviewed-by Wolfram Sang <[email protected]>

Thanks!

Signature · 2 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
#2 lore raw
In reply to Farid Hammane

Hi Farid, Wolfram,

Quoted 13 lines · 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 <[email protected]>
---
 drivers/i2c/algos/i2c-algo-pca.c |   36 ++++++++++++++++++------------------
 1 files changed, 18 insertions(+), 18 deletions(-)
drivers/i2c/algos/i2c-algo-pca.c +0 -0
index dcdaf8e..ca817f8 100644

drivers/i2c/algos/i2c-algo-pca.c line 41

Quoted 7 lines
drivers/i2c/algos/i2c-algo-pca.c +2 -2
@@ -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?

drivers/i2c/algos/i2c-algo-pca.c line 48

Quoted 10 lines
drivers/i2c/algos/i2c-algo-pca.c +3 -3
 #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)$

Signature · 1 line
Jean Delvare
author
#3 lore raw

Hi Jean,

Quoted 31 lines · On Friday 23 April 2010 11:33:26 you wrote:

Hi Farid, Wolfram,

Quoted 14 lines · 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 <[email protected]>
---
 drivers/i2c/algos/i2c-algo-pca.c |   36

++++++++++++++++++------------------ 1 files changed, 18 insertions(+), 18 deletions(-)

(quoted diff) +0 -0

b/drivers/i2c/algos/i2c-algo-pca.c index dcdaf8e..ca817f8 100644

drivers/i2c/algos/i2c-algo-pca.c line 41

Quoted 7 lines
drivers/i2c/algos/i2c-algo-pca.c +1 -2
@@ -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.

Quoted 7 lines

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.

Quoted 13 lines

drivers/i2c/algos/i2c-algo-pca.c line 48

Quoted 10 lines
drivers/i2c/algos/i2c-algo-pca.c +3 -3
 #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...

Quoted 3 lines

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.

Quoted 9 lines

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,

#4 lore raw
In reply to Jean Delvare

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

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