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

[Simon Glass]

Hi,

On Tue, Apr 10, 2012 at 12:56 AM, Marc Dietrich <marvin24 at gmx.de> wrote:
> Am Montag, 9. April 2012, 21:51:32 schrieb 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.
>
> well, that may change in the future. In fact, I have a EMC table here for
> paz00, but only for 333 MHz. I like to submit them once I got 166 MHz values.

OK, I am going to put the T20 code back in and send an updated patch.

Regards,
Simon

>
> Marc
>
>
>
>
>