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@xxxxxxxxx> > > --- > > 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, -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
