[U-Boot] [PATCH 2/7] Tegra114: Add AVP (arm720t) files

[Stephen Warren]

On 01/17/2013 12:15 PM, Tom Warren wrote:
> Stephen,
> 
> On Wed, Jan 16, 2013 at 3:42 PM, Stephen Warren <swarren at wwwdotorg.org> 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.