| Message ID | 1317416168-23637-1-git-send-email-x0132446@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Sep 30, 2011 at 03:56:08PM -0500, David Anders wrote: > this patch cleans up some comment and formating issues that were > reported via checkpatch.pl > > Signed-off-by: David Anders <x0132446@ti.com> > --- Does this patch depend on the other ds1307-patches from recently? If so, it should be mentioned below the dashed line. > drivers/rtc/rtc-ds1307.c | 9 +++++++-- > 1 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c > index 2a98601..811e870 100644 > --- a/drivers/rtc/rtc-ds1307.c > +++ b/drivers/rtc/rtc-ds1307.c > @@ -37,7 +37,7 @@ enum ds_type { > mcp7941x, > rx_8025, > last_ds_type /* always last */ > - // rs5c372 too? different address... > + /* rs5c372 too? different address... */ > }; > > > @@ -366,6 +366,10 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t) > | DS1340_BIT_CENTURY; > break; > case mcp7941x: > + /* these bits were cleared when preparing the date/time > + * values and need to be set again before writing the > + * buffer out to the device. > + */ Common multi-line comments are like /* * line 1 * line 2 */ I think new comments should follow it. Unsure if it is worth fixing the existing ones. I'll leave that up to you :) > buf[DS1307_REG_SECS] |= MCP7941X_BIT_ST; > buf[DS1307_REG_WDAY] |= MCP7941X_BIT_VBATEN; > break; > @@ -624,7 +628,8 @@ static int __devinit ds1307_probe(struct i2c_client *client, > && !i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) > return -EIO; > > - if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL))) > + ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL); > + if (!(ds1307)) No need for parens around 'ds1307' Might be worth to fix other minor problems as well in this patch which can be found visually. E.g the first line of the remove function has indentation problems. Thanks, Wolfram
Wolfram, On Fri, Sep 30, 2011 at 4:08 PM, Wolfram Sang <w.sang@pengutronix.de> wrote: > On Fri, Sep 30, 2011 at 03:56:08PM -0500, David Anders wrote: > > this patch cleans up some comment and formating issues that were > > reported via checkpatch.pl > > > > Signed-off-by: David Anders <x0132446@ti.com> > > --- > > Does this patch depend on the other ds1307-patches from recently? > If so, it should be mentioned below the dashed line. > > right, i'll add that on v2. > > drivers/rtc/rtc-ds1307.c | 9 +++++++-- > > 1 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c > > index 2a98601..811e870 100644 > > --- a/drivers/rtc/rtc-ds1307.c > > +++ b/drivers/rtc/rtc-ds1307.c > > @@ -37,7 +37,7 @@ enum ds_type { > > mcp7941x, > > rx_8025, > > last_ds_type /* always last */ > > - // rs5c372 too? different address... > > + /* rs5c372 too? different address... */ > > }; > > > > > > @@ -366,6 +366,10 @@ static int ds1307_set_time(struct device *dev, > struct rtc_time *t) > > | DS1340_BIT_CENTURY; > > break; > > case mcp7941x: > > + /* these bits were cleared when preparing the date/time > > + * values and need to be set again before writing the > > + * buffer out to the device. > > + */ > > Common multi-line comments are like > > /* > * line 1 > * line 2 > */ > > I think new comments should follow it. Unsure if it is worth fixing the > existing ones. I'll leave that up to you :) > ahh right, i knew this but wasn't paying attention. guess will i'm hacking around might as well clean them up. > > > buf[DS1307_REG_SECS] |= MCP7941X_BIT_ST; > > buf[DS1307_REG_WDAY] |= MCP7941X_BIT_VBATEN; > > break; > > @@ -624,7 +628,8 @@ static int __devinit ds1307_probe(struct i2c_client > *client, > > && !i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) > > return -EIO; > > > > - if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL))) > > + ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL); > > + if (!(ds1307)) > > No need for parens around 'ds1307' > > fixed on v2 > Might be worth to fix other minor problems as well in this patch which can > be > found visually. E.g the first line of the remove function has indentation > problems. > > should i remove all the tabs in declarations? there appears to be tabs on virtually every function.... > Thanks, > > Wolfram > > -- > Pengutronix e.K. | Wolfram Sang | > Industrial Linux Solutions | http://www.pengutronix.de/ | > thanks Dave
Hi Dave, (you are sending HTML mails again) > should i remove all the tabs in declarations? there appears to be tabs on > virtually every function.... I'd think this is too intrusive? BTW there is an unneccessary forward declaration of ds1307_driver. This could go in your patch, too. Regards, Wolfram
Wolfram, On Fri, Sep 30, 2011 at 5:09 PM, Wolfram Sang <w.sang@pengutronix.de> wrote: > > Hi Dave, > > (you are sending HTML mails again) seems gmail's web client is doing something weird when replying to list posts. > > > should i remove all the tabs in declarations? there appears to be tabs on > > virtually every function.... > > I'd think this is too intrusive? BTW there is an unneccessary forward > declaration of ds1307_driver. This could go in your patch, too. > ok i'll just fix the ones you have pointed out. > Regards, > > Wolfram > > -- > Pengutronix e.K. | Wolfram Sang | > Industrial Linux Solutions | http://www.pengutronix.de/ | thanks Dave
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c index 2a98601..811e870 100644 --- a/drivers/rtc/rtc-ds1307.c +++ b/drivers/rtc/rtc-ds1307.c @@ -37,7 +37,7 @@ enum ds_type { mcp7941x, rx_8025, last_ds_type /* always last */ - // rs5c372 too? different address... + /* rs5c372 too? different address... */ }; @@ -366,6 +366,10 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t) | DS1340_BIT_CENTURY; break; case mcp7941x: + /* these bits were cleared when preparing the date/time + * values and need to be set again before writing the + * buffer out to the device. + */ buf[DS1307_REG_SECS] |= MCP7941X_BIT_ST; buf[DS1307_REG_WDAY] |= MCP7941X_BIT_VBATEN; break; @@ -624,7 +628,8 @@ static int __devinit ds1307_probe(struct i2c_client *client, && !i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) return -EIO; - if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL))) + ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL); + if (!(ds1307)) return -ENOMEM; i2c_set_clientdata(client, ds1307);
this patch cleans up some comment and formating issues that were reported via checkpatch.pl Signed-off-by: David Anders <x0132446@ti.com> --- drivers/rtc/rtc-ds1307.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)