[U-Boot] [PATCH 7/7] tegra: Enable I2C on Seaboard

[Heiko Schocher]

Hello Simon,

Simon Glass wrote:
> +U-Boot, which I seemed to have dropped by mistake
> 
> Hi Stephen,
> 
> On Thu, Jan 12, 2012 at 10:59 AM, Stephen Warren <swarren at nvidia.com> wrote:
>> Simon Glass wrote at Wednesday, January 11, 2012 8:00 PM:
>>> On Mon, Jan 9, 2012 at 1:45 PM, Stephen Warren <swarren at nvidia.com> wrote:
>>>> On 12/26/2011 11:11 AM, Simon Glass wrote:
>>>>> This enables I2C on Seaboard.
>>>> ...
>>>>> diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
>>>> ...
>>>>> +#define CONFIG_SYS_I2C_INIT_BOARD
>>>> I don't think that option is correct for Seaboard; the description in
>>>> the README indicates this causes a function named i2c_init_board() to be
>>>> called from boards/xxx/board.c, which is supposed to use GPIOs to unhang
>>>> the I2C bus. However, this raises a couple of issues:
>>>>
>>>> 1) Patch 5 in this series calls i2c_init_board() ifdef CONFIG_TEGRA2_I2C
>>>> rather than depending on this CONFIG option.
>>>>
>>>> 2) Tegra's i2c_init_board() doesn't appear to be anything to do with bus
>>>> unhanging, but is instead regular I2C initialization. Perhaps the
>>>> function should be renamed?
>>> I have had a bit of a look at this. From what I can see the problem is
>>> that i2c_init() is passed a bus speed, but this is just
>>> CONFIG_SYS_I2C_SPEED. We want to control the speed individually for
>>> each port. Yes would could use i2c_init() and ignore the speed, but
>>> that seems odd also. At least with the way it is we don't ignore the
>>> setting.
>>>
>>> We also don't define CONFIG_HARD_I2C which again seems odd, but I
>>> suppose is for the same reason (we don't want to call i2c_init()).
>> Hmm. It sounds like we should replace CONFIG_TEGRA2_I2C with
>> CONFIG_HARD_I2C given the README description of the latter?
> 
> Well we could, but we would need to ignore the speed argument. Do you
> think that is better?

Without defining CONFIG_HARD_i2C, i2c_init() never gets called ...
naming CONFIG_TEGRA2_I2C is absolutely OK for enabling the tegra i2c
driver.

>>> Also U-Boot tends to call i2c_init() before every use of i2c, whereas
>>> we just want to init once and be done with it.
>>>
>>> I think the function name is correct, but perhaps I should change the
>>> #ifdef you mention in 1 above to CONFIG_SYS_I2C_INIT_BOARD. But for
>>> i2c to function on Tegra boards, this must be defined, so in fact this
>>> might be counterproductive. So perhaps a check that it is defined is
>>> best?
>> But README explicitly says that i2c_init_board() is for bus unhanging
>> which isn't what the Tegra implementation does. How can the function
>> name be correct given that?
>>
> Well we should rename the function IMO. It seems to me that with a a
> name like that it is for initing i2c on the board.

Yes, the name is missleading. On the other hand is a deblocking also
some sort of "init" ... but I see no problem in using the i2c_init_board()
for doing board specific i2c inits too ... maybe the README should be
adapted ... but this problem results, that you don't want to use
i2c_init() ...

>> Don't we just want to make i2c_init() use a global/static variable to
>> determine whether the device has already been initialized, and defer all
>> the init until first usage or something like that? That seems in line
>> with U-Boot's desire not to initialize drivers until they're actually
>> used.

Yep, that would be inline with that ... but how would you use "i2c reset"
command? That only calls i2c_init() ... so, you never can reset/reinit
your i2c bus controller.

> Actually that might work - if we keep i2c_init() a no-op, and wait
> until we get a request for a bus before we look up the fdt and init
> that port. But I suspect we might need to init port 0 immediately
> since there is no explicit call to i2c_set_bus_num() for that. It's a
> little wonky whatever we do. What do you think?

Yep, the clean way would be to use the multibus/multiadapter branch:

http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2_20111112

but as I said in a previous EMail ... there is some work to do,
before it is mainline acceptable ...

[...]

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany