From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Tue, 05 May 2015 10:10:05 -0600 Subject: [U-Boot] [PATCH 21/24] tegra124: Implement spl_was_boot_source() In-Reply-To: References: <1430760687-28505-2-git-send-email-sjg@chromium.org> <1430760687-28505-22-git-send-email-sjg@chromium.org> <5548E79D.7080804@wwwdotorg.org> Message-ID: <5548EB5D.7090105@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 05/05/2015 10:02 AM, Simon Glass wrote: > Hi Stephen, > > On 5 May 2015 at 09:54, Stephen Warren wrote: >> On 05/04/2015 11:31 AM, Simon Glass wrote: >>> >>> Add an implementation of this function for Tegra. >> >> >>> diff --git a/arch/arm/mach-tegra/board.c b/arch/arm/mach-tegra/board.c >> >> >>> +#ifndef CONFIG_SPL_BUILD >>> +void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3) >>> +{ >>> + from_spl = r0 != SPL_RUNNING_FROM_UBOOT; >>> + save_boot_params_ret(); >>> +} >>> +#endif >> >> >> (Using terminology from: >> https://patchwork.ozlabs.org/patch/467771/ >> arm: spl: Enable detecting when U-Boot is started from SPL >> ) >> >> That doesn't look right. Surely (at least on Tegra), if the r/o U-Boot >> chain-loads to the r/w U-Boot, then the chain-loaded U-Boot has no SPL and >> is just the main CPU build of U-Boot. >> >> Hence, "SPL_RUNNING_FROM_UBOOT" seems incorrectly named, since the r/o >> U-Boot doesn't chain to SPL but rather to U-Boot. > > What name do you suggest? I was trying to add a prefix indicating that > it relates to non-SPL start-up of U-Boot. Well, that name specifically states that it's SPL that's running, whereas the exact opposite is true. Perhaps UBOOT_CHAIN_LOADED_FROM_UBOOT? >> This approach sounds a little brittle; what happens if r0 just happens to >> have that value. Won't the code get confused? > > Yes, but SPL does not set that value in r0, and we have control over this. > >> Why does U-Boot care whether it's been chain-loaded? Shouldn't it always >> behave identically in all cases, so it's independent of what caused it to >> run? > > In the case of read-only U-Boot it must find the read-write one to > jump to. In the case of read-write U-Boot it must boot a kernel. Surely that should be taken care of by placing the correct boot scripts into the U-Boot environment, rather than hard-coding specific boot behaviour into the U-Boot binary? This feature seems really use-case-specific; I wonder if it's useful/generic-enough to upstream even?