[U-Boot] [PATCH v4 19/23] tegra: Add EMC settings for Seaboard

[Stephen Warren]

On 04/09/2012 05:07 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Mon, Apr 9, 2012 at 3:03 PM, Olof Johansson <olof at lixom.net> wrote:
>> On Mon, Apr 9, 2012 at 2:59 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>> On 04/09/2012 03:40 PM, Simon Glass wrote:
>>>> +Olof
>>>>
>>>> Hi Stephen,
>>>>
>>>> On Mon, Apr 9, 2012 at 2:27 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>>> On 04/05/2012 03:55 PM, Simon Glass wrote:
>>>>>> From: Jimmy Zhang <jimmzhang at nvidia.com>
>>>>>>
>>>>>> Set Seaboard to optimal memory settings based on the SOC in use (T20 or T25).
>>>>>>
>>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>>>> ---
>>>>>> Changes in v2:
>>>>>> - Move EMC tables to device tree
>>>>>> - Removed check for nominal voltage (not needed as it is done just before)
>>>>>>
>>>>>> Changes in v3:
>>>>>> - Add better error reporting when EMC setup fails
>>>>>>
>>>>>> Changes in v4:
>>>>>> - Remove support for T20 memory timings
>>>>>
>>>>>> diff --git a/board/nvidia/common/emc.c b/board/nvidia/common/emc.c
>>>>>
>>>>>> +/* This rate is hard-coded for now, until fdt provides them */
>>>>>> +#define EMC_SDRAM_RATE_T25   (380000 * 2 * 1000)
>>>>>> +
>>>>>> +int board_emc_init(void)
>>>>>> +{
>>>>>> +     unsigned rate;
>>>>>> +
>>>>>> +     switch (tegra_get_chip_type()) {
>>>>>> +     default:
>>>>>> +     case TEGRA_SOC_T20:
>>>>>> +             debug("%s: EMC timings not supported for T20 Seaboard\n",
>>>>>> +                   __func__);
>>>>>
>>>>> This isn't Seaboard-specific code, so the string shouldn't say
>>>>> "Seaboard" there.
>>>>>
>>>>> Why not support Tegra20? Many/all of the other Tegra boards U-Boot
>>>>> supports are Tegra20 not Tegra25.
>>>>>
>>>>> Presumably this code doesn't blow up if the EMC tables aren't in the
>>>>> .dts file; the code should use the tables if they're present, otherwise
>>>>> be a no-op.
>>>>
>>>> I don't mind, we can either go with v3 (with T20) or v4 (without).
>>>> Both sets of patches are on the list and the removable of T20 support
>>>> is the only change in v4. Please can you discuss this with Olof?
>>>
>>> IIRC, Olof objected to the incorrect Seaboard .dts file (which contained
>>> two unrelated sets of EMC tables for different board variants), not the
>>> ability for the EMC driver itself to function on either Tegra20 or Tegra25.
>>
>> Correct. I objected to the one device tree describing 50% inaccurate
>> contents without a documented way to tell the accurate from inaccurate
>> (unlike the case with bootid straps).
>>
>> What we do on the kernel side is that if the existing programming,
>> i.e. what's setup by BCT/u-boot is not matching the clocks in the
>> table, then we report it but continue with the existing settings. If
>> someone passes in bogus (matching) data in both BCT and the device
>> tree then all bets are off.
> 
> OK, that's different from what I understood (remove support for T20
> Seaboard as was apparently done in the kernel). So I think this means
> that this patch should revert back to the original version, right?

In the kernel, the Seaboard .dts file only supports Tegra25 since the
EMC tables we put into that .dts file are for Tegra25. There is no
Seaboard-with-Tegra20 .dts file.

I don't believe the EMC driver has any code that cares about
Tegra20-vs-Tegra25, so should work just fine for either.

In practice, we don't have any .dts files for boards with Tegra20 that
contain EMC tables though.