Messages in this thread | | | From | David Brownell <> | Subject | Re: [PATCH] OMAP: I2C driver for TI OMAP boards #2 | Date | Mon, 31 Jul 2006 16:53:47 -0700 |
| |
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 (non-blocking).
- 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@users.sourceforge.net>
- Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |