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

[Simon Glass]

Hi Stephen,

On Thu, Jan 12, 2012 at 4:10 PM, Stephen Warren <swarren at nvidia.com> wrote:
> 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.

OK, will do.

Regards,
Simon

>
> --
> nvpublic
>