Hello Simon,
Simon Glass wrote:
Hi Heiko,
On Thu, Jan 12, 2012 at 11:25 PM, Heiko Schocher hs@denx.de wrote:
Hello Simon,
Simon Glass wrote:
From: Yen Lin yelin@nvidia.com
Add basic i2c driver for Tegra2 with 8- and 16-bit address support. The driver requires CONFIG_OF_CONTROL to obtain its configuration from the device tree.
(Simon Glass: sjg@chromium.org modified for upstream)
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Use DIV_ROUND_UP() instead of a home-grown macro
- Tidy comment style
- Change i2c array to static
- Adjust definitions to fit new peripheral clock bindings
- Remove i2c configuring using CONFIG (use fdt instead)
Why? Ah found it ... Hmm.. why we don't need the non OF case?
The intent is that Tegra boards will run with fdt turned on. It means that we should be able to build U-Boot for a Tegra platform using the Linux fdt file and not lots of manual config. Of course this works better for some peripherals than others, but I2C works well enough.
Ah, Ok, nice!
- Make i2c/dvc decision come from fdt
- Use new fdtdec alias decode function
- Simplify code in i2c_addr_ok()
- Return an error if an unavailable i2c bus is selected
arch/arm/include/asm/arch-tegra2/tegra2_i2c.h | 160 +++++++ drivers/i2c/Makefile | 1 + drivers/i2c/tegra2_i2c.c | 551 +++++++++++++++++++++++++ include/fdtdec.h | 2 + lib/fdtdec.c | 2 + 5 files changed, 716 insertions(+), 0 deletions(-) create mode 100644 arch/arm/include/asm/arch-tegra2/tegra2_i2c.h create mode 100644 drivers/i2c/tegra2_i2c.c
[...]
diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c new file mode 100644 index 0000000..a7db714 --- /dev/null +++ b/drivers/i2c/tegra2_i2c.c @@ -0,0 +1,551 @@
[...]
+static void i2c_init_controller(struct i2c_bus *i2c_bus) +{
/* TODO: Fix bug which makes us need to do this */clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_OSC,i2c_bus->speed * (8 * 2 - 1));Can you use here some defines?What is (8 * 2 - 1) ?
I will look up the bug and find out.
Not a bug, just I did not know what this values are for ...
+#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
Hmm.. see above question. Ok, if somebody need want to use this driver without CONFIG_OF_CONTROL it must be added ...
Yes and they need a way of getting the configuration in there. The fdt is nicer...
;-)
[...]
+void i2c_init(int speed, int slaveaddr) +{
debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);+}
Hmm... i2c_init is called to init the i2c subsystem ... you do nothing here ... and use i2c_init_board for init the i2c bus, right?
But i2c_init_board is not called from the driver ... ah, you do this in board code ... Ok ...
I think, you do this, because i2c_init is called very early, and so processing fdt is slow?
That's not the main reason but it is true. We have no need of early I2C on the platform. Also we don't want to set the speed as this is defined individually per port in the fdt.
Ok.
bye, Heiko