
Florian Fainelli f.fainelli@gmail.com writes:
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@fitzsim.org Cc: Stefan Roese sr@denx.de Cc: Tom Rini trini@konsulko.com Cc: Florian Fainelli f.fainelli@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.
This worked via the default get_ticks implementation in lib/time.c, which tracks rollovers and converts to a 64-bit value. But I agree it's better to use the high timer register, so that (among other reasons) get_ticks reflects total uptime including time spent in BOLT. I overrode get_ticks in v4 of the patch to use the high and low timer registers.
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.
Makes sense, moved to a 7445-specific header in v4.
+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.
In v4 I adjusted the comment and added an entry to the README to document the expectation.
Thanks, Thomas