Hello Simon, Simon Glass wrote: > Hi Heiko, > > On Thu, Jan 12, 2012 at 11:25 PM, Heiko Schocher <h...@denx.de> wrote: >> Hello Simon, >> >> Simon Glass wrote: >>> From: Yen Lin <ye...@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: s...@chromium.org modified for upstream) >>> >>> Signed-off-by: Simon Glass <s...@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 -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot