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

[Stephen Warren]

On 02/07/2013 12:05 PM, Tom Warren wrote:
> Stephen,
> 
> On Thu, Feb 7, 2013 at 11:20 AM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> On 02/07/2013 11:14 AM, Tom Warren wrote:
>>> Stephen,
>>>
>>> On Thu, Feb 7, 2013 at 11:07 AM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>> On 02/07/2013 09:17 AM, Tom Warren wrote:
>>>>> Laxman,
>>>>>
>>>>> On Thu, Feb 7, 2013 at 7:57 AM, Laxman Dewangan <ldewangan at nvidia.com> wrote:
>>>>>> On Thursday 07 February 2013 04:56 AM, 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).
>>>>>>>
>>>>>>> Signed-off-by: Tom Warren <twarren at nvidia.com>
>>>>>>> ---
>>>>>>
>>>>>>
>>>>>>
>>>>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts
>>>>>>> b/board/nvidia/dts/tegra114-dalmore.dts
>>>>>>> index 7315577..13b07f3 100644
>>>>>>> --- a/board/nvidia/dts/tegra114-dalmore.dts
>>>>>>> +++ b/board/nvidia/dts/tegra114-dalmore.dts
>>>>>>> @@ -6,8 +6,41 @@
>>>>>>>         model =NVIDIA Dalmore";
>>>>>>>         compatible =nvidia,dalmore", "nvidia,tegra114";
>>>>>>>
>>>>>>>   +     aliases {
>>>>>>> +               i2c0 =/i2c at 7000d000";
>>>>>>> +               i2c1 =/i2c at 7000c000";
>>>>>>> +               i2c2 =/i2c at 7000c400";
>>>>>>> +               i2c3 =/i2c at 7000c500";
>>>>>>> +               i2c4 =/i2c at 7000c700";
>>>>>>> +       };
>>>>>>
>>>>>>
>>>>>> Can we move this to tegar114.dtsi file.
>>>>>
>>>>> I could, but why? Most, if not all, of the U-Boot boards that use DT
>>>>> are putting their aliases in the .dts file in the board directory.
>>>>
>>>> Laxman, the issue here is that right now in U-Boot, I believe, if a
>>>> particular board only uses I2C adapters 0, 2, and 4, then only U-Boot
>>>> device IDs 0, 1, and 2 are defined, rather than IDs 0, 2, and 4. That's
>>>> why the aliases are in the per-board file for now, because the actual
>>>> set of I2C adapters enabled is board-specific.
>>>>
>>>> Tom, as background for Laxman's request, for other devices (e.g. serial
>>>> ports), customer engineers have pushed back on that naming scheme, and
>>>> always want a specific HW device to end up with a static name,
>>>> irrespective of which other devices of the same type are used, if any.
>>>> For that reason, in the kernel, we have aliases for the serial ports in
>>>> tegra*.dtsi rather than in per-board files, and the names are static.
>>>>
>>>> So I wonder if in U-Boot we really have to have IDs 0..n rather than
>>>> e.g. IDs 0, 2, 4 for the I2C ports (when some aren't used). Then, we
>>>> could just put the aliases in tegra*.dtsi, which makes life simpler when
>>>> creating board .dts files...
>>>
>>> Thanks for the background info. I'm focusing on getting T114 I2C in
>>> right now, so I can move on to MMC and USB, and what I have seems
>>> reasonable/conforms to what's already done in U-Boot.
>>
>> That doesn't necessarily make it correct.
>>
>>> I don't want to get into reworking everyone's dts/dtsi files right now
>>> to move aliases around, or find out which boards really have unused
>>> I2C ports (Dalmore uses 'em all, BTW, according to our I2C
>>> spreadsheet). If you want to send a set of cleanup/re-org patches for
>>> everybody's DTS files, or even just the Tegra boards, feel free. I'll
>>> be glad to review it/apply it to u-boot-tegra later.
>>
>> Sorry, but that's simply part of writing the .dts file for a board;
>> there is no way around that.
> 
> OK, how about this. For T114, I'll move the aliases to tegra114.dtsi,

I believe that makes sense.

But please do check that if you do disable one of the controllers, that
the other 4 I2C ports do all get registered and still work OK, so we've
tested that code-path. I'm not sure if we have tested the case where the
I2C channels don't have contiguous IDs yet, so I have no idea if it works.

> and use 400KHz for all I2C nodes except I2C1 (saw some internal code
> saying that it may not run well at that speed on the E1611 board).
> Since all 5 controllers have devices behind them, I think it's OK to
> leave all 5 in the aliases.

The downstream kernel only has I2C5 (IIRC) set to 400KHz - the other 4
are limited to 100KHz.