[PATCH 0/7] rtc: convert to c99 format

[PATCH 0/7] rtc: convert to c99 format

[Shubhrajyoti D]

The series tries to convert the i2c_msg to c99 struct.
This may avoid issues like below if someone tries to add an
element to the structure.
http://www.mail-archive.com/linux-i2c@vger.kernel.org/msg08972.html
 
Special thanks to Julia Lawall for helping it automate.
By the below script.
http://www.mail-archive.com/cocci@diku.dk/msg02753.html
 

Shubhrajyoti D (7):
  rtc: Convert struct i2c_msg initialization to C99 format
  rtc: Convert struct i2c_msg initialization to C99 format
  rtc: Convert struct i2c_msg initialization to C99 format
  rtc: Convert struct i2c_msg initialization to C99 format
  rtc: Convert struct i2c_msg initialization to C99 format
  rtc: Convert struct i2c_msg initialization to C99 format
  rtc: Convert struct i2c_msg initialization to C99 format

 drivers/rtc/rtc-ds1672.c  |   26 ++++++++++++++++++++++----
 drivers/rtc/rtc-em3027.c  |    6 +++---
 drivers/rtc/rtc-isl1208.c |    6 +++---
 drivers/rtc/rtc-pcf8563.c |    4 ++--
 drivers/rtc/rtc-rs5c372.c |    2 +-
 drivers/rtc/rtc-s35390a.c |    4 ++--
 drivers/rtc/rtc-x1205.c   |   28 ++++++++++++++--------------
 7 files changed, 47 insertions(+), 29 deletions(-)

-- 
1.7.5.4

[PATCH 1/7] rtc: Convert struct i2c_msg initialization to C99 format

[Shubhrajyoti D]

    Convert the struct i2c_msg initialization to C99 format. This makes
    maintaining and editing the code simpler. Also helps once other fields
    like transferred are added in future.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
 drivers/rtc/rtc-ds1672.c |   26 ++++++++++++++++++++++----
 1 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-ds1672.c b/drivers/rtc/rtc-ds1672.c
index 7fa67d0..b44b2a1 100644
--- a/drivers/rtc/rtc-ds1672.c
+++ b/drivers/rtc/rtc-ds1672.c
@@ -37,8 +37,18 @@ static int ds1672_get_datetime(struct i2c_client *client, struct rtc_time *tm)
 	unsigned char buf[4];
 
 	struct i2c_msg msgs[] = {
-		{client->addr, 0, 1, &addr},	/* setup read ptr */
-		{client->addr, I2C_M_RD, 4, buf},	/* read date */
+		{
+			.addr = client->addr,
+			.flags = 0,
+			.len = 1,
+			.buf = &addr
+		},	/* setup read ptr */
+		{
+			.addr = client->addr,
+			.flags = I2C_M_RD,
+			.len = 4,
+			.buf = buf
+		},	/* read date */
 	};
 
 	/* read date registers */
@@ -99,8 +109,16 @@ static int ds1672_get_control(struct i2c_client *client, u8 *status)
 	unsigned char addr = DS1672_REG_CONTROL;
 
 	struct i2c_msg msgs[] = {
-		{client->addr, 0, 1, &addr},	/* setup read ptr */
-		{client->addr, I2C_M_RD, 1, status},	/* read control */
+		{	.addr = client->addr,
+			.flags = 0,
+			.len = 1,
+			.buf = &addr
+		},	/* setup read ptr */
+		{	.addr = client->addr,
+			.flags = I2C_M_RD,
+			.len = 1,
+			.buf = status
+		},	/* read control */
 	};
 
 	/* read control register */
-- 
1.7.5.4

[PATCH 2/7] rtc: Convert struct i2c_msg initialization to C99 format

[Shubhrajyoti D]

    Convert the struct i2c_msg initialization to C99 format. This makes
    maintaining and editing the code simpler. Also helps once other fields
    like transferred are added in future.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
 drivers/rtc/rtc-em3027.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-em3027.c b/drivers/rtc/rtc-em3027.c
index 0104ea7..373f7e5 100644
--- a/drivers/rtc/rtc-em3027.c
+++ b/drivers/rtc/rtc-em3027.c
@@ -49,8 +49,8 @@ static int em3027_get_time(struct device *dev, struct rtc_time *tm)
 	unsigned char buf[7];
 
 	struct i2c_msg msgs[] = {
-		{client->addr, 0, 1, &addr},		/* setup read addr */
-		{client->addr, I2C_M_RD, 7, buf},	/* read time/date */
+		{.addr = client->addr, .flags = 0, .len = 1, .buf = &addr},		/* setup read addr */
+		{.addr = client->addr, .flags = I2C_M_RD, .len = 7, .buf = buf},	/* read time/date */
 	};
 
 	/* read time/date registers */
@@ -76,7 +76,7 @@ static int em3027_set_time(struct device *dev, struct rtc_time *tm)
 	unsigned char buf[8];
 
 	struct i2c_msg msg = {
-		client->addr, 0, 8, buf,	/* write time/date */
+		.addr = client->addr, .flags = 0, .len = 8, .buf = buf,	/* write time/date */
 	};
 
 	buf[0] = EM3027_REG_WATCH_SEC;
-- 
1.7.5.4

[PATCH 3/7] rtc: Convert struct i2c_msg initialization to C99 format

[Shubhrajyoti D]

    Convert the struct i2c_msg initialization to C99 format. This makes
    maintaining and editing the code simpler. Also helps once other fields
    like transferred are added in future.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
 drivers/rtc/rtc-isl1208.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index dd2aeee..c3a76ae 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -68,9 +68,9 @@ isl1208_i2c_read_regs(struct i2c_client *client, u8 reg, u8 buf[],
 {
 	u8 reg_addr[1] = { reg };
 	struct i2c_msg msgs[2] = {
-		{client->addr, 0, sizeof(reg_addr), reg_addr}
+		{.addr = client->addr, .flags = 0, .len = sizeof(reg_addr), .buf = reg_addr}
 		,
-		{client->addr, I2C_M_RD, len, buf}
+		{.addr = client->addr, .flags = I2C_M_RD, .len = len, .buf = buf}
 	};
 	int ret;
 
@@ -90,7 +90,7 @@ isl1208_i2c_set_regs(struct i2c_client *client, u8 reg, u8 const buf[],
 {
 	u8 i2c_buf[ISL1208_REG_USR2 + 2];
 	struct i2c_msg msgs[1] = {
-		{client->addr, 0, len + 1, i2c_buf}
+		{.addr = client->addr, .flags = 0, .len = len + 1, .buf = i2c_buf}
 	};
 	int ret;
 
-- 
1.7.5.4

[PATCH 4/7] rtc: Convert struct i2c_msg initialization to C99 format

[Shubhrajyoti D]

        Convert the struct i2c_msg initialization to C99 format. This makes
        maintaining and editing the code simpler. Also helps once other fields
        like transferred are added in future.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
 drivers/rtc/rtc-pcf8563.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
index c2fe426..7cc8092 100644
--- a/drivers/rtc/rtc-pcf8563.c
+++ b/drivers/rtc/rtc-pcf8563.c
@@ -78,8 +78,8 @@ static int pcf8563_get_datetime(struct i2c_client *client, struct rtc_time *tm)
 	unsigned char buf[13] = { PCF8563_REG_ST1 };
 
 	struct i2c_msg msgs[] = {
-		{ client->addr, 0, 1, buf },	/* setup read ptr */
-		{ client->addr, I2C_M_RD, 13, buf },	/* read status + date */
+		{ .addr = client->addr, .flags = 0, .len = 1, .buf = buf },	/* setup read ptr */
+		{ .addr = client->addr, .flags = I2C_M_RD, .len = 13, .buf = buf },	/* read status + date */
 	};
 
 	/* read registers */
-- 
1.7.5.4

[PATCH 5/7] rtc: Convert struct i2c_msg initialization to C99 format

[Shubhrajyoti D]

        Convert the struct i2c_msg initialization to C99 format. This makes
        maintaining and editing the code simpler. Also helps once other fields
        like transferred are added in future.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
 drivers/rtc/rtc-rs5c372.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
index fb4842c..68e6332 100644
--- a/drivers/rtc/rtc-rs5c372.c
+++ b/drivers/rtc/rtc-rs5c372.c
@@ -104,7 +104,7 @@ static int rs5c_get_regs(struct rs5c372 *rs5c)
 {
 	struct i2c_client	*client = rs5c->client;
 	struct i2c_msg		msgs[] = {
-		{ client->addr, I2C_M_RD, sizeof rs5c->buf, rs5c->buf },
+		{ .addr = client->addr, .flags = I2C_M_RD, .len = sizeof rs5c->buf, .buf = rs5c->buf },
 	};
 
 	/* This implements the third reading method from the datasheet, using
-- 
1.7.5.4

[PATCH 6/7] rtc: Convert struct i2c_msg initialization to C99 format

[Shubhrajyoti D]

        Convert the struct i2c_msg initialization to C99 format. This makes
        maintaining and editing the code simpler. Also helps once other fields
        like transferred are added in future.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
 drivers/rtc/rtc-s35390a.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index c9562ce..e8c835e 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -50,7 +50,7 @@ static int s35390a_set_reg(struct s35390a *s35390a, int reg, char *buf, int len)
 {
 	struct i2c_client *client = s35390a->client[reg];
 	struct i2c_msg msg[] = {
-		{ client->addr, 0, len, buf },
+		{ .addr = client->addr, .flags = 0, .len = len, .buf = buf },
 	};
 
 	if ((i2c_transfer(client->adapter, msg, 1)) != 1)
@@ -63,7 +63,7 @@ static int s35390a_get_reg(struct s35390a *s35390a, int reg, char *buf, int len)
 {
 	struct i2c_client *client = s35390a->client[reg];
 	struct i2c_msg msg[] = {
-		{ client->addr, I2C_M_RD, len, buf },
+		{ .addr = client->addr, .flags = I2C_M_RD, .len = len, .buf = buf },
 	};
 
 	if ((i2c_transfer(client->adapter, msg, 1)) != 1)
-- 
1.7.5.4

[PATCH 7/7] rtc: Convert struct i2c_msg initialization to C99 format

[Shubhrajyoti D]

        Convert the struct i2c_msg initialization to C99 format. This makes
        maintaining and editing the code simpler. Also helps once other fields
        like transferred are added in future.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
 drivers/rtc/rtc-x1205.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/rtc/rtc-x1205.c b/drivers/rtc/rtc-x1205.c
index 403b3d4..4ddd9a6 100644
--- a/drivers/rtc/rtc-x1205.c
+++ b/drivers/rtc/rtc-x1205.c
@@ -97,8 +97,8 @@ static int x1205_get_datetime(struct i2c_client *client, struct rtc_time *tm,
 	int i;
 
 	struct i2c_msg msgs[] = {
-		{ client->addr, 0, 2, dt_addr },	/* setup read ptr */
-		{ client->addr, I2C_M_RD, 8, buf },	/* read date */
+		{ .addr = client->addr, .flags = 0, .len = 2, .buf = dt_addr },	/* setup read ptr */
+		{ .addr = client->addr, .flags = I2C_M_RD, .len = 8, .buf = buf },	/* read date */
 	};
 
 	/* read date registers */
@@ -142,8 +142,8 @@ static int x1205_get_status(struct i2c_client *client, unsigned char *sr)
 	static unsigned char sr_addr[2] = { 0, X1205_REG_SR };
 
 	struct i2c_msg msgs[] = {
-		{ client->addr, 0, 2, sr_addr },	/* setup read ptr */
-		{ client->addr, I2C_M_RD, 1, sr },	/* read status */
+		{ .addr = client->addr, .flags = 0, .len = 2, .buf = sr_addr },	/* setup read ptr */
+		{ .addr = client->addr, .flags = I2C_M_RD, .len = 1, .buf = sr },	/* read status */
 	};
 
 	/* read status register */
@@ -279,8 +279,8 @@ static int x1205_get_dtrim(struct i2c_client *client, int *trim)
 	static unsigned char dtr_addr[2] = { 0, X1205_REG_DTR };
 
 	struct i2c_msg msgs[] = {
-		{ client->addr, 0, 2, dtr_addr },	/* setup read ptr */
-		{ client->addr, I2C_M_RD, 1, &dtr }, 	/* read dtr */
+		{ .addr = client->addr, .flags = 0, .len = 2, .buf = dtr_addr },	/* setup read ptr */
+		{ .addr = client->addr, .flags = I2C_M_RD, .len = 1, .buf = &dtr },	/* read dtr */
 	};
 
 	/* read dtr register */
@@ -311,8 +311,8 @@ static int x1205_get_atrim(struct i2c_client *client, int *trim)
 	static unsigned char atr_addr[2] = { 0, X1205_REG_ATR };
 
 	struct i2c_msg msgs[] = {
-		{ client->addr, 0, 2, atr_addr },	/* setup read ptr */
-		{ client->addr, I2C_M_RD, 1, &atr }, 	/* read atr */
+		{ .addr = client->addr, .flags = 0, .len = 2, .buf = atr_addr },	/* setup read ptr */
+		{ .addr = client->addr, .flags = I2C_M_RD, .len = 1, .buf = &atr },	/* read atr */
 	};
 
 	/* read atr register */
@@ -381,8 +381,8 @@ static int x1205_validate_client(struct i2c_client *client)
 		unsigned char addr[2] = { 0, probe_zero_pattern[i] };
 
 		struct i2c_msg msgs[2] = {
-			{ client->addr, 0, 2, addr },
-			{ client->addr, I2C_M_RD, 1, &buf },
+			{ .addr = client->addr, .flags = 0, .len = 2, .buf = addr },
+			{ .addr = client->addr, .flags = I2C_M_RD, .len = 1, .buf = &buf },
 		};
 
 		if ((xfer = i2c_transfer(client->adapter, msgs, 2)) != 2) {
@@ -409,8 +409,8 @@ static int x1205_validate_client(struct i2c_client *client)
 		unsigned char addr[2] = { 0, probe_limits_pattern[i].reg };
 
 		struct i2c_msg msgs[2] = {
-			{ client->addr, 0, 2, addr },
-			{ client->addr, I2C_M_RD, 1, &reg },
+			{ .addr = client->addr, .flags = 0, .len = 2, .buf = addr },
+			{ .addr = client->addr, .flags = I2C_M_RD, .len = 1, .buf = &reg },
 		};
 
 		if ((xfer = i2c_transfer(client->adapter, msgs, 2)) != 2) {
@@ -444,8 +444,8 @@ static int x1205_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	static unsigned char int_addr[2] = { 0, X1205_REG_INT };
 	struct i2c_client *client = to_i2c_client(dev);
 	struct i2c_msg msgs[] = {
-		{ client->addr, 0, 2, int_addr },        /* setup read ptr */
-		{ client->addr, I2C_M_RD, 1, &intreg },  /* read INT register */
+		{ .addr = client->addr, .flags = 0, .len = 2, .buf = int_addr },        /* setup read ptr */
+		{ .addr = client->addr, .flags = I2C_M_RD, .len = 1, .buf = &intreg },  /* read INT register */
 	};
 
 	/* read interrupt register and status register */
-- 
1.7.5.4

Re: [PATCH 1/7] rtc: Convert struct i2c_msg initialization to C99 format

[Ryan Mallon]

On 17/09/12 23:58, Shubhrajyoti D wrote:
>     Convert the struct i2c_msg initialization to C99 format. This makes
>     maintaining and editing the code simpler. Also helps once other fields
>     like transferred are added in future.
> 
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
>  drivers/rtc/rtc-ds1672.c |   26 ++++++++++++++++++++++----
>  1 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-ds1672.c b/drivers/rtc/rtc-ds1672.c
> index 7fa67d0..b44b2a1 100644
> --- a/drivers/rtc/rtc-ds1672.c
> +++ b/drivers/rtc/rtc-ds1672.c
> @@ -37,8 +37,18 @@ static int ds1672_get_datetime(struct i2c_client *client, struct rtc_time *tm)
>  	unsigned char buf[4];
>  
>  	struct i2c_msg msgs[] = {
> -		{client->addr, 0, 1, &addr},	/* setup read ptr */
> -		{client->addr, I2C_M_RD, 4, buf},	/* read date */
> +		{
> +			.addr = client->addr,
> +			.flags = 0,
> +			.len = 1,
> +			.buf = &addr
> +		},	/* setup read ptr */

It would be nice to tabify the fields, and put the comments on their own
lines while you are here. With the C99 format you can also omit fields
which are initialised to zero. Like this:

		{
			/* Setup read pointer */
			.addr	= client->addr,
			.len	= 1,
			.buf	= &addr,
		},

~Ryan

Re: [PATCH 3/7] rtc: Convert struct i2c_msg initialization to C99 format

[Ryan Mallon]

On 17/09/12 23:58, Shubhrajyoti D wrote:
>     Convert the struct i2c_msg initialization to C99 format. This makes
>     maintaining and editing the code simpler. Also helps once other fields
>     like transferred are added in future.
> 
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
>  drivers/rtc/rtc-isl1208.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> index dd2aeee..c3a76ae 100644
> --- a/drivers/rtc/rtc-isl1208.c
> +++ b/drivers/rtc/rtc-isl1208.c
> @@ -68,9 +68,9 @@ isl1208_i2c_read_regs(struct i2c_client *client, u8 reg, u8 buf[],
>  {
>  	u8 reg_addr[1] = { reg };
>  	struct i2c_msg msgs[2] = {
> -		{client->addr, 0, sizeof(reg_addr), reg_addr}
> +		{.addr = client->addr, .flags = 0, .len = sizeof(reg_addr), .buf = reg_addr}
>  		,

Putting the whole initialiser on one line is a bit ugly. Any reason not
to expand it over multiple lines as the previous patch (and majority of
other drivers) does?

~Ryan

Re: [PATCH 3/7] rtc: Convert struct i2c_msg initialization to C99 format

[Ryan Mallon]

On 17/09/12 23:58, Shubhrajyoti D wrote:
>     Convert the struct i2c_msg initialization to C99 format. This makes
>     maintaining and editing the code simpler. Also helps once other fields
>     like transferred are added in future.
> 
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
>  drivers/rtc/rtc-isl1208.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> index dd2aeee..c3a76ae 100644
> --- a/drivers/rtc/rtc-isl1208.c
> +++ b/drivers/rtc/rtc-isl1208.c
> @@ -68,9 +68,9 @@ isl1208_i2c_read_regs(struct i2c_client *client, u8 reg, u8 buf[],
>  {
>  	u8 reg_addr[1] = { reg };
>  	struct i2c_msg msgs[2] = {
> -		{client->addr, 0, sizeof(reg_addr), reg_addr}
> +		{.addr = client->addr, .flags = 0, .len = sizeof(reg_addr), .buf = reg_addr}

Actually, I wonder if it is useful to have something like:

	#define I2C_WRITE(_addr, _buf, _len) {	\
		.addr	= _addr,		\
		.buf	= _buf,			\
		.len	= _len,			\
	}

	#define I2C_READ(_addr, _buf, _len) {	\
		.addr	= _addr,		\
		.buf	= _buf,			\
		.len	= _len,			\
		.flags	= I2C_M_RD,		\
	}

and then write this as:

	struct i2c_msg msgs[2] = {
		I2C_WRITE(client->addr, reg_addr, sizeof(reg_addr)),
		I2C_READ(client->addr, buf, len),
	};

~Ryan

Re: [PATCH 3/7] rtc: Convert struct i2c_msg initialization to C99 format

[Shubhrajyoti]

On Tuesday 18 September 2012 07:15 AM, Ryan Mallon wrote:
>>  		,
> Putting the whole initialiser on one line is a bit ugly. Any reason not
> to expand it over multiple lines as the previous patch (and majority of
> other drivers) does?
Will do that.

Re: [PATCH 3/7] rtc: Convert struct i2c_msg initialization to C99 format

[Shubhrajyoti]

On Tuesday 18 September 2012 07:21 AM, Ryan Mallon wrote:
> Actually, I wonder if it is useful to have something like:.
Read and write differ only in the flag also it will be a deviation from
what $SUBJECT
would warrant.  So could be a separate patch.
>
> 	#define I2C_WRITE(_addr, _buf, _len) {	\
> 		.addr	= _addr,		\
> 		.buf	= _buf,			\
> 		.len	= _len,			\
> 	}
>
> 	#define I2C_READ(_addr, _buf, _len) {	\
> 		.addr	= _addr,		\
> 		.buf	= _buf,			\
> 		.len	= _len,			\
> 		.flags	= I2C_M_RD,		\
> 	}
>
> and then write this as:
>
> 	struct i2c_msg msgs[2] = {
> 		I2C_WRITE(client->addr, reg_addr, sizeof(reg_addr)),
> 		I2C_READ(client->addr, buf, len),
> 	};

Re: [PATCH 3/7] rtc: Convert struct i2c_msg initialization to C99 format

[Ryan Mallon]

On 18/09/12 15:40, Shubhrajyoti wrote:
> On Tuesday 18 September 2012 07:21 AM, Ryan Mallon wrote:
>> Actually, I wonder if it is useful to have something like:.
> Read and write differ only in the flag also it will be a deviation from
> what $SUBJECT
> would warrant.  So could be a separate patch.

Sure, but I think it would help make the code even more readable than
just converting to C99 initialisers (especially since putting the C99
initialiser all on one line is hard to read), and there is little sense
in doing one clean up and then replacing it later with a different clean up.

If you are worried about the duplication of code for a single flag
difference you could always do:

  #define I2C_OP(_addr, _buf, _len, _flags)	\
	...

  #define I2C_WRITE(addr, buf, len) I2C_OP(addr, buf, len, 0)
  #define I2C_READ(addr, buf, len)  I2C_OP(addr, buf, len, I2C_M_RD)

~Ryan

>>
>> 	#define I2C_WRITE(_addr, _buf, _len) {	\
>> 		.addr	= _addr,		\
>> 		.buf	= _buf,			\
>> 		.len	= _len,			\
>> 	}
>>
>> 	#define I2C_READ(_addr, _buf, _len) {	\
>> 		.addr	= _addr,		\
>> 		.buf	= _buf,			\
>> 		.len	= _len,			\
>> 		.flags	= I2C_M_RD,		\
>> 	}
>>
>> and then write this as:
>>
>> 	struct i2c_msg msgs[2] = {
>> 		I2C_WRITE(client->addr, reg_addr, sizeof(reg_addr)),
>> 		I2C_READ(client->addr, buf, len),
>> 	};
> 

Re: [PATCH 3/7] rtc: Convert struct i2c_msg initialization to C99 format

[Julia Lawall]

On Tue, 18 Sep 2012, Ryan Mallon wrote:

> On 18/09/12 15:40, Shubhrajyoti wrote:
>> On Tuesday 18 September 2012 07:21 AM, Ryan Mallon wrote:
>>> Actually, I wonder if it is useful to have something like:.
>> Read and write differ only in the flag also it will be a deviation from
>> what $SUBJECT
>> would warrant.  So could be a separate patch.
>
> Sure, but I think it would help make the code even more readable than
> just converting to C99 initialisers (especially since putting the C99
> initialiser all on one line is hard to read),

The one line thing is the fault of Coccinelle :)

julia

> and there is little sense
> in doing one clean up and then replacing it later with a different clean up.
>
> If you are worried about the duplication of code for a single flag
> difference you could always do:
>
>  #define I2C_OP(_addr, _buf, _len, _flags)	\
> 	...
>
>  #define I2C_WRITE(addr, buf, len) I2C_OP(addr, buf, len, 0)
>  #define I2C_READ(addr, buf, len)  I2C_OP(addr, buf, len, I2C_M_RD)
>
> ~Ryan
>
>>>
>>> 	#define I2C_WRITE(_addr, _buf, _len) {	\
>>> 		.addr	= _addr,		\
>>> 		.buf	= _buf,			\
>>> 		.len	= _len,			\
>>> 	}
>>>
>>> 	#define I2C_READ(_addr, _buf, _len) {	\
>>> 		.addr	= _addr,		\
>>> 		.buf	= _buf,			\
>>> 		.len	= _len,			\
>>> 		.flags	= I2C_M_RD,		\
>>> 	}
>>>
>>> and then write this as:
>>>
>>> 	struct i2c_msg msgs[2] = {
>>> 		I2C_WRITE(client->addr, reg_addr, sizeof(reg_addr)),
>>> 		I2C_READ(client->addr, buf, len),
>>> 	};
>>
>
>

Re: [PATCH 1/7] rtc: Convert struct i2c_msg initialization to C99 format

[Shubhrajyoti]

On Tuesday 18 September 2012 07:14 AM, Ryan Mallon wrote:
> On 17/09/12 23:58, Shubhrajyoti D wrote:
>>     Convert the struct i2c_msg initialization to C99 format. This makes
>>     maintaining and editing the code simpler. Also helps once other fields
>>     like transferred are added in future.
>>
>> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
>> ---
>>  drivers/rtc/rtc-ds1672.c |   26 ++++++++++++++++++++++----
>>  1 files changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-ds1672.c b/drivers/rtc/rtc-ds1672.c
>> index 7fa67d0..b44b2a1 100644
>> --- a/drivers/rtc/rtc-ds1672.c
>> +++ b/drivers/rtc/rtc-ds1672.c
>> @@ -37,8 +37,18 @@ static int ds1672_get_datetime(struct i2c_client *client, struct rtc_time *tm)
>>  	unsigned char buf[4];
>>  
>>  	struct i2c_msg msgs[] = {
>> -		{client->addr, 0, 1, &addr},	/* setup read ptr */
>> -		{client->addr, I2C_M_RD, 4, buf},	/* read date */
>> +		{
>> +			.addr = client->addr,
>> +			.flags = 0,
>> +			.len = 1,
>> +			.buf = &addr
>> +		},	/* setup read ptr */
> It would be nice to tabify the fields, and put the comments on their own
> lines while you are here. With the C99 format you can also omit fields
> which are initialised to zero. Like this:
For local structures also?
>
> 		{
> 			/* Setup read pointer */
> 			.addr	= client->addr,
> 			.len	= 1,
> 			.buf	= &addr,
> 		},
>
> ~Ryan
>
>

Re: [PATCH 1/7] rtc: Convert struct i2c_msg initialization to C99 format

[Ryan Mallon]

On 18/09/12 16:05, Shubhrajyoti wrote:
> On Tuesday 18 September 2012 07:14 AM, Ryan Mallon wrote:
>> On 17/09/12 23:58, Shubhrajyoti D wrote:
>>>     Convert the struct i2c_msg initialization to C99 format. This makes
>>>     maintaining and editing the code simpler. Also helps once other fields
>>>     like transferred are added in future.
>>>
>>> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
>>> ---
>>>  drivers/rtc/rtc-ds1672.c |   26 ++++++++++++++++++++++----
>>>  1 files changed, 22 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/rtc/rtc-ds1672.c b/drivers/rtc/rtc-ds1672.c
>>> index 7fa67d0..b44b2a1 100644
>>> --- a/drivers/rtc/rtc-ds1672.c
>>> +++ b/drivers/rtc/rtc-ds1672.c
>>> @@ -37,8 +37,18 @@ static int ds1672_get_datetime(struct i2c_client *client, struct rtc_time *tm)
>>>  	unsigned char buf[4];
>>>  
>>>  	struct i2c_msg msgs[] = {
>>> -		{client->addr, 0, 1, &addr},	/* setup read ptr */
>>> -		{client->addr, I2C_M_RD, 4, buf},	/* read date */
>>> +		{
>>> +			.addr = client->addr,
>>> +			.flags = 0,
>>> +			.len = 1,
>>> +			.buf = &addr
>>> +		},	/* setup read ptr */
>> It would be nice to tabify the fields, and put the comments on their own
>> lines while you are here. With the C99 format you can also omit fields
>> which are initialised to zero. Like this:
> For local structures also?

Yes, C99 initialisers automatically clear all unspecified fields to
zero. See:
http://stackoverflow.com/questions/3374446/what-happens-to-fields-not-named-by-a-designated-initializer

~Ryan