Hi Stephen, On Thu, Jan 12, 2012 at 4:10 PM, Stephen Warren <swar...@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 <swar...@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 <swar...@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 <swar...@nvidia.com> >> >> >> wrote: >> >> >> > On 12/26/2011 12:33 PM, Simon Glass wrote: >> >> >> >> From: Jimmy Zhang <jimmzh...@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 > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot