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

[Marc Dietrich]

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.

Marc