[PATCH] hxd8-tsl256x.patch

Harald Welte laforge at openmoko.org
Wed May 16 06:04:35 CEST 2007


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




More information about the openmoko-kernel mailing list