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

[Simon Glass]

Hi Stephen,

On Thu, Jan 12, 2012 at 11:21 AM, Stephen Warren <swarren at nvidia.com> wrote:
> Simon Glass wrote at Thursday, January 12, 2012 12:10 PM:
>> 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:
> ...
>> >> 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.
>
> You mean change the function that's activated by CONFIG_SYS_I2C_INIT_BOARD
> to be something more like i2c_unhang_board()? That makes sense to me. I
> have no idea how much of a hassle it'd be to update any existing boards.

Well we should change the README or change the function name. I'm not
sure which is correct.

>
>> > 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.
>>
>> 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?
>
> It does make sense to me to at least parse out all the DT stuff early
> on, so there's a single place to do all the alias processing etc. Given
> what I just wrote in the other email about fixing the I2C bus speed
> stuff, would making those changes here help this at all?

I think the device tree processing should be done later because
i2c_init() is called very early, before the dcache is enabled. It is
very  s l o w  to touch device tree before then. To be honest, that's
another reason for leaving it as it is, with i2c_init() being a no-op.
But we might be able to init the bus on first use with enough if()
statements :-)

I will take a look. I am not going to refactor U-Boot I2C here,
although I think it could use a few tweaked...

Regards,
Simon
>
> --
> nvpublic
>