| Message ID | 201006171454.07593.roman.fietze@telemotive.de |
|---|---|
| State | Superseded |
| Headers | show |
Hi Roman, > Hallo Wan, > > Thanks for the review. I added an second patch here with the proposed > changes. Should I better provide an all-in-one patch instead? > Since your patch has not been applied please submit a revised version of the full patch rather than incremental updates. > On Thursday 17 June 2010 12:28:11 Wan ZongShun wrote: > >> There is no need to print your private driver info in Kernel, please >> get rid of it. > > Ok, I thought its good habit, just copied that 1:1 from rtc-pcf8563.c > as well as the stuff with the check for ->rtc beeing zero and the > order of the module macros. I can of course also provide a patch for > rtc-pcf8563.c with the same changes. > > > From 134f8488933f723489d5cf244c0b54bde1ce3622 Mon Sep 17 00:00:00 2001 > From: Roman Fietze <roman.fietze@telemotive.de> > Date: Thu, 17 Jun 2010 14:45:36 +0200 > Subject: [PATCH] rtc-isl12022: omit checking ->rtc in remove function, reorder module macros > > - isl12022->rtc cannot be NULL inside isl12022_remove > - move up module init and exit macros > - be less verbose inside probe function > > Signed-off-by: Roman Fietze <roman.fietze@telemotive.de> > --- > drivers/rtc/rtc-isl12022.c | 14 +++++--------- > 1 files changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c > index e0a340f..293d2c1 100644 > --- a/drivers/rtc/rtc-isl12022.c > +++ b/drivers/rtc/rtc-isl12022.c > @@ -236,8 +236,6 @@ static int isl12022_probe(struct i2c_client *client, > > int ret = 0; > > - dev_dbg(&client->dev, "%s\n", __func__); > - > if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) > return -ENODEV; > > @@ -245,7 +243,7 @@ static int isl12022_probe(struct i2c_client *client, > if (!isl12022) > return -ENOMEM; > > - dev_info(&client->dev, "chip found, driver version " DRV_VERSION "\n"); > + dev_dbg(&client->dev, "chip found, driver version " DRV_VERSION "\n"); > > i2c_set_clientdata(client, isl12022); > > @@ -269,9 +267,7 @@ static int isl12022_remove(struct i2c_client *client) > { > struct isl12022 *isl12022 = i2c_get_clientdata(client); > > - if (isl12022->rtc) > - rtc_device_unregister(isl12022->rtc); > - > + rtc_device_unregister(isl12022->rtc); > kfree(isl12022); > > return 0; > @@ -303,10 +299,10 @@ static void __exit isl12022_exit(void) > i2c_del_driver(&isl12022_driver); > } > > +module_init(isl12022_init); > +module_exit(isl12022_exit); > + > MODULE_AUTHOR("roman.fietze@telemotive.de"); > MODULE_DESCRIPTION("ISL 12022 RTC driver"); > MODULE_LICENSE("GPL"); > MODULE_VERSION(DRV_VERSION); > - > -module_init(isl12022_init); > -module_exit(isl12022_exit);
Hi Roman, I have to remind you to use scripts/checkpatch.pl' to check your patch format. There are more warning and errors in your patch. > >> Hallo Wan, >> >> Thanks for the review. I added an second patch here with the proposed >> changes. Should I better provide an all-in-one patch instead? >> > > Since your patch has not been applied please submit a revised version of > the full patch rather than incremental updates. > >> On Thursday 17 June 2010 12:28:11 Wan ZongShun wrote: >> >>> There is no need to print your private driver info in Kernel, please >>> get rid of it. >> >> Ok, I thought its good habit, just copied that 1:1 from rtc-pcf8563.c >> as well as the stuff with the check for ->rtc beeing zero and the >> order of the module macros. I can of course also provide a patch for >> rtc-pcf8563.c with the same changes. >> >> >> From 134f8488933f723489d5cf244c0b54bde1ce3622 Mon Sep 17 00:00:00 2001 >> From: Roman Fietze <roman.fietze@telemotive.de> >> Date: Thu, 17 Jun 2010 14:45:36 +0200 >> Subject: [PATCH] rtc-isl12022: omit checking ->rtc in remove function, reorder module macros >> >> - isl12022->rtc cannot be NULL inside isl12022_remove >> - move up module init and exit macros >> - be less verbose inside probe function >> > >> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de> >> --- >> drivers/rtc/rtc-isl12022.c | 14 +++++--------- >> 1 files changed, 5 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c >> index e0a340f..293d2c1 100644 >> --- a/drivers/rtc/rtc-isl12022.c >> +++ b/drivers/rtc/rtc-isl12022.c >> @@ -236,8 +236,6 @@ static int isl12022_probe(struct i2c_client *client, >> >> int ret = 0; >> >> - dev_dbg(&client->dev, "%s\n", __func__); >> - >> if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) >> return -ENODEV; >> >> @@ -245,7 +243,7 @@ static int isl12022_probe(struct i2c_client *client, >> if (!isl12022) >> return -ENOMEM; >> >> - dev_info(&client->dev, "chip found, driver version " DRV_VERSION "\n"); >> + dev_dbg(&client->dev, "chip found, driver version " DRV_VERSION "\n"); >> >> i2c_set_clientdata(client, isl12022); >> >> @@ -269,9 +267,7 @@ static int isl12022_remove(struct i2c_client *client) >> { >> struct isl12022 *isl12022 = i2c_get_clientdata(client); >> >> - if (isl12022->rtc) >> - rtc_device_unregister(isl12022->rtc); >> - >> + rtc_device_unregister(isl12022->rtc); >> kfree(isl12022); >> >> return 0; >> @@ -303,10 +299,10 @@ static void __exit isl12022_exit(void) >> i2c_del_driver(&isl12022_driver); >> } >> >> +module_init(isl12022_init); >> +module_exit(isl12022_exit); >> + >> MODULE_AUTHOR("roman.fietze@telemotive.de"); >> MODULE_DESCRIPTION("ISL 12022 RTC driver"); >> MODULE_LICENSE("GPL"); >> MODULE_VERSION(DRV_VERSION); >> - >> -module_init(isl12022_init); >> -module_exit(isl12022_exit); > >
diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c index e0a340f..293d2c1 100644 --- a/drivers/rtc/rtc-isl12022.c +++ b/drivers/rtc/rtc-isl12022.c @@ -236,8 +236,6 @@ static int isl12022_probe(struct i2c_client *client, int ret = 0; - dev_dbg(&client->dev, "%s\n", __func__); - if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) return -ENODEV; @@ -245,7 +243,7 @@ static int isl12022_probe(struct i2c_client *client, if (!isl12022) return -ENOMEM; - dev_info(&client->dev, "chip found, driver version " DRV_VERSION "\n"); + dev_dbg(&client->dev, "chip found, driver version " DRV_VERSION "\n"); i2c_set_clientdata(client, isl12022); @@ -269,9 +267,7 @@ static int isl12022_remove(struct i2c_client *client) { struct isl12022 *isl12022 = i2c_get_clientdata(client); - if (isl12022->rtc) - rtc_device_unregister(isl12022->rtc); - + rtc_device_unregister(isl12022->rtc); kfree(isl12022); return 0; @@ -303,10 +299,10 @@ static void __exit isl12022_exit(void) i2c_del_driver(&isl12022_driver); } +module_init(isl12022_init); +module_exit(isl12022_exit); + MODULE_AUTHOR("roman.fietze@telemotive.de"); MODULE_DESCRIPTION("ISL 12022 RTC driver"); MODULE_LICENSE("GPL"); MODULE_VERSION(DRV_VERSION); - -module_init(isl12022_init); -module_exit(isl12022_exit);