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

[Simon Glass]

Hi Stephen,

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?

Regards,
Simon

>
> --
> nvpublic
>