Patchwork isl12022: added Intersil ISL12022 RTC

login
register
mail settings
Submitter Roman Fietze
Date June 17, 2010, 12:54 p.m.
Message ID <201006171454.07593.roman.fietze@telemotive.de>
Download mbox | patch
Permalink /patch/56033/
State New
Headers show

Comments

Roman Fietze - June 17, 2010, 12:54 p.m.
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?

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(-)
Wan ZongShun - June 18, 2010, 1:33 a.m.
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);
Wan ZongShun - June 18, 2010, 2:08 a.m.
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);
>
>

Patch

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);