[U-Boot] [PATCH 13/14] tegra: Add EMC settings for Seaboard, Harmony

[Stephen Warren]

Simon Glass wrote at Thursday, January 12, 2012 5:06 PM:
> On Thu, Jan 12, 2012 at 4:01 PM, Stephen Warren <swarren at nvidia.com> wrote:
> > Simon Glass wrote at Thursday, January 12, 2012 4:55 PM:
> >> On Thu, Jan 12, 2012 at 3:42 PM, Stephen Warren <swarren at nvidia.com> wrote:
> >> > Simon Glass wrote at Thursday, January 12, 2012 4:05 PM:
> >> >> On Tue, Jan 10, 2012 at 10:46 AM, Stephen Warren <swarren at nvidia.com> wrote:
> >> >> > On 12/26/2011 12:33 PM, Simon Glass wrote:
> >> >> >> From: Jimmy Zhang <jimmzhang at nvidia.com>
> >> >> >>
> >> >> >> Set Seaboard and Harmony to optimal memory settings based on the SOC
> >> >> >> in use (T20 or T25).
> >> > ...
> >> >> >> +int board_emc_init(void)
> >> >> >> +{
> >> >> >> +       int     i;
> >> >> >> +       DECLARE_GLOBAL_DATA_PTR;
> >> >> >> +
> >> >> >> +#ifdef CONFIG_TEGRA_PMU
> >> >> >> +       /* if voltage has not been set properly, return */
> >> >> >> +       if (!pmu_is_voltage_nominal())
> >> >> >> +               return -1;
> >> >> >> +#endif
> >> >> >
> >> >> > Why/when would the PMU voltage not be nominal?
> >> >>
> >> >> On boot, it starts up lower and we raise it to nominal so we can run
> >> >> at full speed.
> >> >>
> >> >> > Can't we error out the compile if the options that cause the PMU voltage
> >> >> > to be initialized to nominal are not set, instead of detecting this at
> >> >> > runtime?
> >> >>
> >> >> I don't think so, since we can't know in U-Boot what the start-up voltages are.
> >> >
> >> > So how does the system get to the nominal state? And if board_emc_init()
> >> > is called when the system isn't in the nominal state, does it somehow get
> >> > called again later once it is, so that the EMC initialization doesn't fail
> >> > the error-check quoted above?
> >>
> >> We call board_emc_init() after pmu_set_nominal().
> >>
> >> >
> >> > In other words, presumably U-Boot explicitly programs the PMU into the
> >> > nominal stage at some point. Shouldn't we defer calling board_emc_init()
> >> > until after that time, thus making that error-check redundant?
> >>
> >> Yes, but if you look at the patch above, that's what we do:
> >>
> >>  #ifdef CONFIG_TEGRA_PMU
> >>        pmu_set_nominal();
> >> +
> >> +       board_emc_init();
> >>  #endif
> >>  #endif
> >
> > OK, so in practice,
> >
> > /* if voltage has not been set properly, return */
> > if (!pmu_is_voltage_nominal())
> >
> > ... will never fire. My original point was that if so, why is that check
> > needed? I suppose it's a reasonable safety net though - that's the
> > reason?
> 
> OK I see. It certainly shouldn't - it is a check that everything is
> well since this code is in a different file and it is possible that
> someone may get this wrong. If they do then the system may continue
> but die later in interesting ways. Still, the user has other equally
> complex things to worry about.
> 
> I'm happy to remove this particularly as this might become example
> code for other boards - what do you think?

The check itself is probably fine for the reasons you state. However,
I'd suggest adjusting the comment to something more like:

This code should only be called once the PMU is operating at nominal
voltage. Hence, this test should never fail. However, this prevents
unpredictable failures from occurring later if this pre-condition is
not met.

-- 
nvpublic