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

Simon Glass
2012-04-09T23:07:24Z

Hi Stephen,

On Mon, Apr 9, 2012 at 3:03 PM, Olof Johansson <olof@lixom.net> wrote:
> On Mon, Apr 9, 2012 at 2:59 PM, Stephen Warren <swarren@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@wwwdotorg.org> wrote:
>>>> On 04/05/2012 03:55 PM, Simon Glass wrote:
>>>>> From: Jimmy Zhang <jimmzhang@nvidia.com>
>>>>>
>>>>> Set Seaboard to optimal memory settings based on the SOC in use (T20 or T25).
>>>>>
>>>>> Signed-off-by: Simon Glass <sjg@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?

>
>
> -Olof

Regards,
Simon