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.

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

> +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 //.

All the best,

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

Attachment: signature.asc
Description: Digital signature

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Reply via email to