Thank you for your comments, Jean.
Jean Delvare wrote:
>Hi Mark,
>
>
>
>>+ This driver can also be built as a module. If so, the module
>>+ will be called i2c-m41t00.
>>
>>
>
>It'll actually be called m41t00, according to the Makefile.
>
Good catch.
>
>
>>+struct m41t00_data {
>>+ struct i2c_client client;
>>+};
>>
>>
>
>You don't have to do that. Including the i2c_client stucture in the data
>structure is a trick which let us get both allocated with a single
>kmalloc (and freed with a single kfree) while still respecting the
>arch-dependent alignment requirements. If you have no private data to
>carry around, you can do the kmalloc on the i2c_client structure
>directly, and have client->data point to NULL (which it actually already
>does thanks to memset). This will save some code in both the detection
>and the detach functions.
>
>However, if you know that, in a future update of this driver, you *will*
>have to store client-private data, then I guess you can keep it this way.
>
Its gone.
>>+ i2c_detach_client(client);
>>
>>
>
>This one supposedly can fail.
>
Okay, I'll check.
>
>
>>+ .name = "M41T00",
>>
>>
>
>No caps in name please (will be used in sysfs).
>
Done.
>
>
>
>>+static int __devinit
>>+m41t00_init(void)
>>+{
>>+ return i2c_add_driver(&m41t00_driver);
>>+}
>>
>>+static void __devexit
>>+m41t00_exit(void)
>>+{
>>+ i2c_del_driver(&m41t00_driver);
>>+ return;
>>+}
>>
>>
>
>Should be __init and __exit, respectively, unless I am mistaken. And the
>last return is usless.
>
I thought the __devxxx ones were the best ones to use but I don't know
for sure. I'll change them to __init/__exit.
>
>I'm also suspicious about the other __devexit and __devinit you used. No
>other i2c chip drivers has them.
>
>
Done.
Here is a replacement patch that should address Jean Delvare and Dick
Johnson's issues. Please let me know if there are any more issues.
Thanks,
Mark
Signed-off-by: Mark A. Greer <mgreer at mvista.com>
--
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: m41t00_2.patch
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20050201/a855c9c6/attachment.pl
