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
