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

login
register
mail settings
Submitter Datta, Shubhrajyoti
Date Sept. 17, 2012, 1:58 p.m.
Message ID <1347890294-28467-2-git-send-email-shubhrajyoti@ti.com>
Download mbox | patch
Permalink /patch/184410/
State New
Headers show

Comments

Datta, Shubhrajyoti - Sept. 17, 2012, 1:58 p.m.
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(-)
Ryan Mallon - Sept. 18, 2012, 1:44 a.m.
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
Datta, Shubhrajyoti - Sept. 18, 2012, 6:05 a.m.
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
>
>
Ryan Mallon - Sept. 18, 2012, 6:10 a.m.
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

Patch

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