From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Thu, 17 Jan 2013 15:42:30 -0700 Subject: [U-Boot] [PATCH 2/7] Tegra114: Add AVP (arm720t) files In-Reply-To: References: <1358370848-29469-1-git-send-email-twarren@nvidia.com> <1358370848-29469-3-git-send-email-twarren@nvidia.com> <50F72CBA.9050800@wwwdotorg.org> Message-ID: <50F87E56.1000705@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 01/17/2013 12:15 PM, Tom Warren wrote: > Stephen, > > On Wed, Jan 16, 2013 at 3:42 PM, Stephen Warren wrote: >> On 01/16/2013 02:14 PM, Tom Warren wrote: >>> This provides SPL support for T114 boards - AVP early init, plus >>> CPU (A15) init/jump to main U-Boot. >>> diff --git a/arch/arm/cpu/arm720t/tegra-common/cpu.h b/arch/arm/cpu/arm720t/tegra-common/cpu.h >> >>> +#ifndef TRUE >>> +#define TRUE 1 >>> +#endif >>> +#ifndef FALSE >>> +#define FALSE 0 >>> +#endif >> >> Surely those are in a standard header somewhere; we shouldn't create yet >> another duplicate of them. > > Couldn't find 'em on a quick search (grep define.TRUE) except in > places like scsi.h and ext4_common.h and fpga.h. If you have a > standard header that you know of, let me know. Hmmm. Further investigation shows it is indeed once of those standard things that isn't actually defined anywhere standard:-( >>> diff --git a/arch/arm/cpu/arm720t/tegra114/cpu.c b/arch/arm/cpu/arm720t/tegra114/cpu.c >> >>> +static int is_partition_powered(u32 mask) >> >>> + reg = readl(&pmc->pmc_pwrgate_status); >>> + if ((reg & mask) == mask) >>> + return TRUE; >>> + >>> + return FALSE; >> >> The last 4 lines are just "return reg & mask;" or "return (reg & mask) >> == mask;". Same in the next function. > > I'm porting internal bootloader bringup code here, and trying to avoid > unnecessary changes, but I can modify these to remove the TRUE/FALSE > in V2. I don't think our downstream code is relevant for upstreaming; changes sent upstream should be clean/minimal/standalone. In fact, I find upstreaming a good time to explicitly remove/clean-up all the cruft that has accumulated downstream, which wasn't always thought through thoroughly. >>> +void powerup_cpus(void) >>> +{ >>> + debug("powerup_cpus entry\n"); >>> + >>> + /* Are we booting to the fast cluster? */ >>> + if (get_cluster_id() == 0) { >> >> Why would we ever boot on anything other than the fast cluster? I would >> assume that the kernel assume it will always get booted on the fast >> cluster, which would then imply that U-Boot had to boot on or switch to >> the fast cluster. > > Again, this is from internal NV bootloader code that I know works. I > don't know the circumstances where we might be booted on the LP > cluster, but I figured if the internal bootloader code thought it > worth checking, so should I. If you have unimpeachable evidence to > the contrary, I can optimize this out. I think it's more that if we don't have concrete evidence that the code is needed, we shouldn't bloat usptream with cruft.