Re: [rtc-linux] [PATCH] isl12022: added Intersil ISL12022 RTC

[Wan ZongShun]

 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@xxxxxxxxxxxxx>
>> 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@xxxxxxxxxxxxx>
>> ---
>> Â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@xxxxxxxxxxxxx");
>> ÂMODULE_DESCRIPTION("ISL 12022 RTC driver");
>> ÂMODULE_LICENSE("GPL");
>> ÂMODULE_VERSION(DRV_VERSION);
>> -
>> -module_init(isl12022_init);
>> -module_exit(isl12022_exit);
>
>



-- 
*linux-arm-kernel mailing list
mail addr:linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
you can subscribe by:
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

* linux-arm-NUC900 mailing list
mail addr:NUC900@xxxxxxxxxxxxxxxx
main web: https://groups.google.com/group/NUC900
you can subscribe it by sending me mail:
mcuos.com@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/