[U-Boot] [PATCH 1/2] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore

[Tom Warren]

Stephen,

On Tue, Jan 29, 2013 at 2:08 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 01/29/2013 01:40 PM, Tom Warren wrote:
>> Stephen,
>>
>> On Tue, Jan 29, 2013 at 1:09 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>> On 01/29/2013 12:19 PM, Tom Warren wrote:
>>>> Note that T114 does not have a separate/different DVC (power I2C)
>>>> controller like T20 - all 5 I2C controllers are identical, but
>>>> I2C5 is used to designate the controller intended for power
>>>> control (PWR_I2C in the schematics).
>
>>> If you do keep this node, it needs the clocks property filled in.
>>>
>>>> +     i2c at 7000c000 {
>>>> +             compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c";
>>>
>>> The I2C nodes also aren't backwards-compatible.
>>
>> They were on T30 (same compatible string there, except of course
>> s/114/30/). And the tegra20-i2c is needed for fdtdec.c to find it
>> (compat_names table).
>
> Yes, I believe the HW changes between Tegra20 and Tegra30 were such that
> the Tegra20 driver would run unmodified on Tegra30 without issue. I
> don't believe that's the case for Tegra114 though. The solution would be
> to add the Tegra114 compatible value to the I2C driver so that it can
> search for both.

Looking at the TRMs for both T30 and T114, I2C looks nearly identical,
except for some additional registers tacked onto the end for bus clear
support. 99% of the bits appear exactly the same, too (with the
addition of a bus clear int bit on T114 in an used bit position).  I
know next to nothing about I2C, but it appears to me that the current
tegra_I2C.c driver should work fine on T114 (and does, since I can
probe addresses with it).  I don't see a need for a different driver
or compat value here.

>
>>>> +             reg = <0x7000c000 0x100>;
>>>
>>> The interrupts property is missing here.
>>
>> I didn't have an interrupts property for I2C on the T30 dts files,
>> either.
>
> That sounds like a bug in the DT.
>
>> What is it used for in U-Boot?
>
> U-Boot doesn't use interrupts at least on Tegra, so it's not used.
> However, it'd be best to keep the U-Boot DT in line with the kernel DT
> so we can easily spot differences. There has been talk of trying to use
> the same DT for both too, although we're a long long way off from that.

I'll add it, but it seems silly to add unused values, especially when
your past review critiques of DT files have asked for minimum
functionality, which I agree with.

>
>> If it's really needed, can I
>> just crib it from the kernel dts file(s)?
>
> Yes, but the Tegra114 patches aren't all checked in yet. See the
> following proposed patch for what you need:
>
> https://lkml.org/lkml/2013/1/29/235

The only diff between Laxman's dtsi file in the proposed patch and
mine is the interrupts property. I added it (it's exactly the same as
the T30 values, BTW, so I'll add interrupts to the T30 dtsi file in a
separate patch).

>
> (note that patch is missing the clocks properties since the clock
> binding isn't finalized and checked in yet)
>
>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts
>>>
>>> Board changes would usually be a separate patch to an SoC change, but I
>>> guess not a big deal.
>>
>> Actually, this is the way I've always done DT files (in a separate
>> patch), and then the config files in a separate patch the enable the
>> feature.
>
> I meant that tegra114.dtsi and tegra114-dalmore.dts would usually be
> separate patches.
>
>>> All 5 I2C ports are used on the board and only support 100KHz? That
>>> seems unusual, but I suppose it's possible.
>>
>> That's the way I have it on T30, and looking at
>> arch/arm/boot/dts/tegra30.dtsi in the kernel, all 5 ports are set to
>> 100KHz.
>
> But this is board-specific; it depends on which of the I2C controllers
> are actually pinmux'd out onto the board (-> which to enable), and which
> devices are attached to those buses (-> max I2C clock rate). I don't see
> how Tegra30 nor tegra30.dtsi are relevant here.

I see your point, but I don't know exactly how to find out the proper
clock & disable settings for E1611 Dalmore. I'll talk to Yen & peruse
the docs.

Thanks,

Tom