[U-Boot] [PATCH v3 1/1] board: arm: Add support for Broadcom BCM7445

[Florian Fainelli]

On 06/06/2018 11:35 AM, Thomas Fitzsimmons wrote:
> Add support for loading U-Boot on the Broadcom 7445 SoC.  This port
> assumes Broadcom's BOLT bootloader is acting as the second stage
> bootloader, and U-Boot is acting as the third stage bootloader, loaded
> as an ELF program by BOLT.
> 
> Signed-off-by: Thomas Fitzsimmons <fitzsim at fitzsim.org>
> Cc: Stefan Roese <sr at denx.de>
> Cc: Tom Rini <trini at konsulko.com>
> Cc: Florian Fainelli <f.fainelli at gmail.com>
> ---

Looks good, still some minor comments about the choice of representation
for physical addresses of peripherals, see below.

> +config BCMSTB_TIMER_LOW
> +	hex "Address of BCMSTB timer low register"
> +	default 0xf0412008

This looks very simplistic here since the CPU system control timer is a
64-bit timer.

I am really not a big fan of all of those configurable addresses which
are a) fixed given a specific SoC family (7445, 7439 etc.) and b) are
error prone because we let an user change those without necessarily
knowing what is the implication. I really think sticking those constants
into a header file would be much more appropriate.

> +void enable_caches(void)
> +{
> +	/*
> +	 * Nothing required here, since the prior stage bootloader has
> +	 * enabled I-cache and D-cache already.  Implementing this
> +	 * function silences the warning in the default function.
> +	 */

This heavily depends on how you load your binary from BOLT, so you must
be careful about this statement here.
-- 
Florian