Re: [PATCH] hxd8-tsl256x.patch

[Alec_Tsai]

Thanks for your guidance.
Alec

To: "Alec_Tsai" <[EMAIL PROTECTED]>
Cc: <openmoko-kernel@lists.openmoko.org>; "hsu matt" <[EMAIL PROTECTED]>
Sent: Wednesday, May 16, 2007 12:04 PM
Subject: Re: [PATCH] hxd8-tsl256x.patch

----- Original Message ----- From: "Harald Welte" <[EMAIL PROTECTED]>

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

@@ -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.

+++ linux-2.6.21/drivers/i2c/chips/tsl256x.c 2007-05-15 15:57:40.000000000 +0800

+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 <[EMAIL PROTECTED]> http://openmoko.org/

============================================================================

Software for the world's first truly open Free Software mobile phone

hxd8-tsl256x.patch
Description: Binary data