Re: [PATCH] OMAP: I2C driver for TI OMAP boards #2

From: David Brownell
Date: Mon Jul 31 2006 - 19:57:45 EST

On Monday 31 July 2006 9:13 am, Jean Delvare wrote:
> If you want things to improve, please help by
> reviewing Komal's driver. I think I understand you already commented on
> it, but I'd like you to really review it, and add a formal approval to
> it (e.g. Signed-off-by or Acked-by). Then I'll review it for merge.

The issues noted in the code are still almost all low priority

- The FIXME about choosing the address is very low priority,
and would affect only multi-master systems. The fix would
involve defining a new i2c-specific struct for platform_data,
updating various boards to use it (e.g. OSK can use 400 MHz),
and wouldn't change behavior for any board I've ever seen.

- Likewise with the REVISIT for the bus speed to use. They'd
be fixed with the same patch.

- The REVISIT about maybe a better way to probe is also low
priority; someone with a board that needs better probing
could address it at that time. (Then restest any changes
on multiple generations of silicion ... which IMO is the
role the linux-omap tree should play.)

- The revisit about adap->retries is still up in the air,
and was a question in my submission from last year.
How exactly is that supposed to be used? Right now
it's neither initialized (except to zero) nor tested.

Re coding style issues, I didn't give it a detailed nitpick
but I did easily notice two things worth fixing:

- Some lines are more than 80 characters, so they'll wrap
on standard editor windows.

- There are a couple instances of hidden whitespace to
remove: at end of line, or space-before-tab.

This doesn't include the drivers/Makefile change to push i2c
linkage up near the beginning with other "system" busses,
but that can be a separate patch in any case (assuming that
it's still needed).

Assuming those two coding style things get resolved first,

Signed-off-by: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>

- Dave
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at