[U-Boot] [PATCH 2/2] WIP: tegra: i2c: Enable new CONFIG_SYS_I2C framework

[Simon Glass]

Hi Heiko,

On Wed, Nov 7, 2012 at 11:02 PM, Heiko Schocher <hs at denx.de> wrote:
> Hello Simon,
>
>
> On 05.11.2012 21:39, Simon Glass wrote:
>>
>> Hi,
>>
>> On Thu, Nov 1, 2012 at 10:03 AM, Stephen Warren<swarren at wwwdotorg.org>
>> wrote:
>>>
>>> On 11/01/2012 01:42 AM, Heiko Schocher wrote:
>>>>
>>>> Hello Stephen,
>>>>
>>>> On 31.10.2012 17:25, Stephen Warren wrote:
>>>>>
>>>>> On 10/31/2012 09:56 AM, Simon Glass wrote:
>>>>>>
>>>>>> Hi Stephen,
>>>>>>
>>>>>> On Wed, Oct 31, 2012 at 8:41 AM, Stephen
>
> [...]
>
>>>> should be possible (not yet coded, but tried in an older version) to add
>>>> i2c busses after relocation, or while interpret DT ...
>>>>
>>>> something like I did in
>>>>
>>>> http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=commitdiff;h=ccb680c8cf9002232bc459e4003e3b47db5e1bf4#patch13
>>>
>>>
>>> I guess my question is: Why even have a CONFIG_SYS_I2C_ADAPTERS or
>>> CONFIG_SYS_I2C_BUSSES variable at all? Surely if we intend to make this
>>> dynamic in the long run we should just make it dynamic from the start as
>>> part of the API rework. If we don't, we'll just have to do another pass
>>> over all the drivers converting them to dynamic registration later
>>> anyway.
>>>
>>> I'd suggest having a CONFIG_SYS_I2C_DRIVERS variable at most, and each
>>> driver registers an arbitrary number of adapters and/or buses during its
>>> initialization.
>>>
>>> We could probably even get away without CONFIG_SYS_I2C_DRIVERS at all;
>>> just have each driver define a structure that gets put into a special
>>> linker section, so that the I2C core can iterate over all linked drivers
>>> at boot time and call their initialization functions.
>>>
>>> Even if we don't get rid of CONFIG_SYS_I2C_ADAPTERS, I really think we
>>> should get rid of CONFIG_SYS_I2C_BUSSES and do that dynamically.
>>>
>>> BTW, isn't buses spelled "buses" not "busses"? Thunderbird's
>>> spell-checker certainly thinks so!
>>>
>>>> function:
>>>> int i2c_add_one_bus(char *buf)
>>>>
>>>> ->  while interpreting DT i2c info for above board will result in
>>>> calling:
>>>>     i2c_add_one_bus("tegra-i2c-1");
>>>>     i2c_add_one_bus("tegra-i2c-3");
>>>>
>>>> and results in two new i2c busses 0 and 1 ...
>>>> Maybe this is a way to go?
>>>
>>>
>>> Yes, that sounds like the right kind of direction.
>>
>>
>> Well it certainly is useful to have dynamic I2C, but it might be
>> tricky until we have a pre-reloc malloc()?
>
>
> Yep ... not only this, we maybe have not enough "RAM" for it, because
> RAM is not initialized and we are using some SRAM, cache, ... for
> stack ...
>
> (/dreaming on
>  Sometimes I think, we should switch completely to boot basically with
>  SPL for all boards (in SPL we do not need mutliadapter/mutlibus support),
>  so we could start U-Boot directly relocated in RAM from SPL with an arch
>  independent board.c and all RAM availiable, which would solve some
>  problems ...
>  /dreaming off)
>
>
>> We don't need to do everything at once. It would be nice to get the
>> adapter parameter in there though, to avoid two steps of driver
>
>
> did this, see:
>
> http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2_20121030
>
>
>> change. But I would encourage you Heiko to get this tidied up a bit
>> and merged, since it is already a big step forward.
>
>
> Ok, we should vote here, if we
>
> a) go ahead with this patchset
> b) wait for DM is ready
> c) rework this patchserie here completely new (dynamic registration)
>    as stephen suggested

I vote for a. It is a big step forward, and we can then build on that
for dynamic registration, possibly making a few more changes as
needed. Since most drivers will not convert immediately, this should
have little impact on everyone else.

I don't see a huge benefit to doing dynamic registration until DM is
ready. At best it will just hold us up, at worst it will be thrown out
when DM arrives.

Regards,
Simon

>
>
>>>>> U-Boot shell; there will be I2C busses that exist but cannot be used.
>>>>> Is
>>>>> this what we want? Perhaps it is in fact a good idea to always make the
>>>>
>>>>
>>>> Now, this is wrong! You mix here "i2c bus" with "i2c adapter"! We have
>>>> some i2c adapters which are defined but (maybe) not used ...
>>>>
>>>>> U-Boot shell's I2C bus IDs be the same as the HW's, and hence leave
>>>>> gaps
>>>>> when some ports aren't enabled? That would be a departure from the way
>>>>> USB is handled today though.
>>>>
>>>>
>>>> Hmm.. but is this possible? If we have a board with 2 (or more)
>>>> different
>>>> i2c adapters (which is now possible with the new framework), for example
>>>> 2 i2c soft adapters + 4 tegra i2c adapter ... if we say the i2c tegra
>>>> adapters are the first 4 i2c busses, so we cannot longer say the
>>>> two soft i2c adapters are starting from 0 too (and vice versa) ...
>>>
>>>
>>> I was talking about the I2C HW in the SoC itself. There isn't any
>>> concept of an expected I2C bus/adapter ID for anything outside the SoC.
>>
>>
>> Well the device tree should take care of this (in the alias section).
>> The numbering of i2c ports should be done that way. See Tegra USB for
>> how this works, as an example (board_usb_init() calls
>> fdtdec_find_aliases_for_id(), then add_port() to add each of the ports
>> it finds).
>
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany