[PATCH] hxd8-tsl256x.patch

[Alec_Tsai]

Thanks for your guidance.
Alec
----- Original Message ----- 
From: "Harald Welte" <laforge at openmoko.org>
To: "Alec_Tsai" <alec_tsai at fiwin.com.tw>
Cc: <openmoko-kernel at lists.openmoko.org>; "hsu matt" <matt_hsu at fiwin.com.tw>
Sent: Wednesday, May 16, 2007 12:04 PM
Subject: Re: [PATCH] hxd8-tsl256x.patch


> On Tue, May 15, 2007 at 06:51:53PM +0800, Alec_Tsai wrote:
>> Log:
>>
>> This is a light sensor (TSL256x) driver for HXD8.
>
> Index: linux-2.6.21/drivers/i2c/chips/tsl256x.c
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6.21/drivers/i2c/chips/tsl256x.c 2007-05-15 15:57:40.000000000 
> +0800
> @@ -0,0 +1,310 @@
> +/*
> + * tsl256x.c  --  TSL256x Light Sensor driver
> + *
> + * Copyright 2007 by Fiwin.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
>
>> +enum tsl256x_regs {
>> + TSL256X_REG_CONTROL = 0x80, // Control of basic functions
>
> please put those registers definitions into the header file and use /**/
> style comments instead of //
>
>> + switch (iType)
>> + {
>> + case 0: // T package
>> + if ((ratio >= 0) && (ratio <= K1T))
>> + {b=B1T; m=M1T;}
>> + else if (ratio <= K2T)
>> + {b=B2T; m=M2T;}
>> + else if (ratio <= K3T)
>> + {b=B3T; m=M3T;}
>
> this is not compliant with the kernel coding style since:
> 1) the opening '{' belongs on the same line as the switch()
> 2) the // style comment should be replaced with /* */
> 3) the body should look like
>
> if ((ratio >= 0) && (ratio <= K1T)) {
> b = B1T;
> m = M1T;
> } else if (ratio <= K2T) {
> ...
> }
>
>> + if (res = i2c_attach_client(new_client)) {
>> + printk(KERN_DEBUG "\n[%s]Error: during i2c_attach_client()\n",
>> + new_client->name);
> ...
>> + printk(KERN_INFO "\nTSL256X is attached to I2C bus.\n");
>
> please don't start printk's with a newline (\n).
>
> and from the header file:
>
>> +//...................................................
>
> please replace all comments with /* */ style comments.
>
> Once again, let me assure that I don't make those rules to make you go
> through some particular hardship ;)
>
> But if we want to have 'mainline style' then we have to follow those
> rules...
>
> Plase just resubmit the patch after incorporating those changes.  From
> the actual code itself it seems quite fine to me.
>
> -- 
> - Harald Welte <laforge at openmoko.org> 
> http://openmoko.org/
> ============================================================================
> Software for the world's first truly open Free Software mobile phone 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hxd8-tsl256x.patch
Type: application/octet-stream
Size: 15576 bytes
Desc: not available
Url : http://lists.openmoko.org/pipermail/openmoko-kernel/attachments/20070516/24ebbe40/attachment.obj