[U-Boot] [PATCH v3 8/9] tegra: i2c: Enable new CONFIG_SYS_I2C framework

[Heiko Schocher]

Hello Simon,

Am 31.07.2013 00:05, schrieb Simon Glass:
> Hi Stephen,
>
> On Tue, Jul 30, 2013 at 3:51 PM, Stephen Warren<swarren at wwwdotorg.org>wrote:
>
>> On 07/30/2013 03:46 PM, Simon Glass wrote:
>>> On Tue, Jul 30, 2013 at 3:32 PM, Stephen Warren<swarren at wwwdotorg.org
>>> <mailto:swarren at wwwdotorg.org>>  wrote:
>>>
>>>      On 07/30/2013 03:21 PM, Simon Glass wrote:
>>>      >  On Tue, Jul 30, 2013 at 2:00 PM, Stephen Warren
>>>      <swarren at wwwdotorg.org<mailto:swarren at wwwdotorg.org>
>>>      >  <mailto:swarren at wwwdotorg.org<mailto:swarren at wwwdotorg.org>>>
>> wrote:
[...]
>>>      In practice, perhaps we can assume that it will work on Tegra
>> because we
>>>      know the DRAM is already set up, but then that makes Tegra work in
>> some
>>>      strange special-case way, and completely violates the constraints
>>>      described in crt0.S. We should be striving to unify how all the
>>>      different chips work, rather than adding yet more strange
>> special-cases
>>>      to the initialization sequence to hack around systemic problems.
>>>
>>>
>>> Sure, this is up to you. I was just suggesting something that works and
>>> requires little effort. It isn't pure, agreed.
>>
>> The simplest approach is probably to revert the patch in question, since
>> it clearly violates how U-Boot is supposed to work.
>>
>> It's not really up to me; I think someone like Albert should make the
>> decision since he controls the ARM U-Boot architecture, or Tom as Tegra
>> maintainer, or perhaps you as your patch broke the code.
>>
>
> My '(just for illustration, please don't merge)' patch from last October?
> :-)

:-(

I thought you are testing the tegra plattforms with this patchset,
as you triggered a few times, when this will go to mainline ...

> I did offer to look at this for seaboard if it helps, once we agree on a
> solution, but if you have a solution in mind, please go ahead.

Yes, lets go ahead.

A goal of the i2c rework is to get rid of all i2c_init* calls
around the code, and only use i2c_set_bus_num() before accessing
the i2c subsystem (which looks if it needs to switch to this bus,
is it initialized, muxes setup, ... ) ... maybe add the "bus"
parameter as a second step to the i2c api, and i2c_set_bus_num()
is no longer needed, just internal in the i2c subsystem ...

Maybe we can make i2c_init_func() weak (in the CONFIG_SYS_I2C case),
and define it only on such boards, which need i2c so early?
(Or remove it for the CONFIG_SYS_I2C completely, so boards which
  need early i2c access and switch to the new framework would add
  a i2c_set_bus_num() where they need it ...)

Hmm.. each i2c adapter has its own init function ... why the tegra
driver do not use it? And do the necessary inits in it? So we
initialize an adapater only if we use it, which is also a rule
for u-boot ...

I have no hw, but it should be possible to add to process_nodes()
a parameter "controller_number" and call
process_nodes(controller_number, ...) from the i2c drivers init
function ...

So I see two steps:
- make i2c_init_func weak, and define this function only on boards
   which need i2c so early, or remove it completely ... and see
   which boards fail.
- change i2c tegra driver as described above

Maybe this is a way to go?

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