Re: [PATCH] I2C driver for IMX

[Darius <augulis.darius-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Wolfram Sang wrote:
> On Thu, Mar 13, 2008 at 06:07:45PM +0200, Darius wrote:
> 
> Hello,
> 
> (I know I am myself quite new here. I don't want to boast, just trying
> to be a little bit helpful.)
> 
> I found some things concerning the formal side:
> 
>> + * History: 2002/2/07 use msgs[]
> I have been told here that histories are not wanted as they are achieved
> through git.

ok, I'll remove history.

> 
>> +static struct platform_driver i2c_imx_driver = {
>> +	.probe		= i2c_imx_probe,
>> +	.remove		= i2c_imx_remove,
>> +	.driver		= {
>> +		.name	= DRIVER_NAME,
>> +        .owner	= THIS_MODULE,
> Indentation seems wrong here and in some other places...

stupid thing. It seems it's caused by Code::Blocks editor. I will 
correct text with simple text editor.

> 
>> +struct imx_i2c_struct {
>> +    struct i2c_adapter  adapter;
>> +	struct resource		*res;
>> +	void   __iomem		*base;
> ...like here and so on.
> 
>> +    #ifdef CONFIG_I2C_DEBUG_BUS
>> +        printk("I2C: <i2c_imx_bus_busy>\n");
>> +    #endif
> Maybe you can define a debug-printing-macro that only prints when
> DEBUG_BUS is set and is simply empty when it is unset? This should make
> the code more readable, I think (or use pr_debug).
> 
>> +	// chech or i2c bus is not busy
> CodingStyle suggests not using //.

this is from old driver code... I'll change it to /* */

> 
> All the best,
> 
>    Wolfram
> 

thanks for comments. What I should do with corrected patch? Place there 
again?


BR,
Darius


> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> i2c mailing list
> i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
> http://lists.lm-sensors.org/mailman/listinfo/i2c


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c