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

Thu May 10 17:43:00 UTC 2018

On 05/10/2018 06:04 AM, Thomas Fitzsimmons wrote:
> Florian Fainelli <f.fainelli at gmail.com> writes:
> 
>> On 05/06/2018 04:09 AM, Thomas Fitzsimmons wrote:
>>> Add support for loading U-Boot on the Broadcom 7445D0 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>
>>
>>>
>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>> index 9bd70f4..b2df30a 100644
>>> --- a/arch/arm/Kconfig
>>> +++ b/arch/arm/Kconfig
>>> @@ -498,6 +498,17 @@ config TARGET_VEXPRESS_CA15_TC2
>>>  	select CPU_V7_HAS_VIRT
>>>  	select PL011_SERIAL
>>>  
>>> +config TARGET_BCM7445D0
>>> +	bool "Broadcom 7445D0 TSBL"
>>> +	select CPU_V7
>>> +	select SUPPORT_SPL
>>> +	help
>>> +	  Support for the Broadcom 7445D0 SoC.  This port assumes Bolt
>>
>> BOLT
>>
>>> +	  is acting as the second stage bootloader, and U-Boot is
>>> +	  acting as the third stage bootloader (TSBL), loaded by Bolt.
>>
>> again BOLT
> 
> Oops, will fix in a v2 patch.
> 
>>> +	  This port may work on other BCM7xxx boards with
>>> +	  configuration changes.
>>
>> There are other revisions than D0, so I would just name this
>> TARGET_BCM7445. You would likely want to create a TARGET_BRCMSTB general
>> menu which would encompass all ARMv7-A based SoCs from the Broadcom
>> STB/CM division, and then have per-chip Kconfig options (similar to what
>> the older <= 3.14 STB Linux kernels did).
> 
> OK, will try this in v2.
> 
>>> +
>>> +config BCMSTB_ACCOMMODATE_STBLINUX
>>> +	bool ""
>>> +	default y
>>> +	help
>>> +	  This prevents U-Boot from adding memory reservations for the
>>> +          lengths of initramfs and DTB.  Without skipping these,
>>> +          stblinux's "contiguous memory allocator" (CMA) Linux driver
>>> +          (cma_driver) will allocate memory ranges smaller than what
>>> +          are actually available, because it only checks reservation
>>> +          sizes.  It doesn't check if the reserved range overlaps the
>>> +          range it allocates.  stblinux also tries to move the DTB to
>>> +          a lower memory location early in the Linux boot.  If the FIT
>>> +          image specifies a load address for the initramfs then
>>> +          sometimes the DTB is moved into the range where the
>>> +          initramfs image is loaded.  Defining this will mean that
>>> +          FIT-provided initramfs load addresses are ignored.
>>
>> What STB Linux kernel did you observe this with? I am afraid this is
>> still true about the ranges vs. allocation even in newer kernels, but
>> that is kind of intented to keep the logic KISS (because it's already
>> too complicated IMHO).
> 
> I investigated the allocation discrepancy and wrote the workaround while
> we were still using stblinux-3.14.  Since then we've updated to
> stblinux-4.1 and I've left the workaround enabled, but I haven't
> investigated its interactions with the newer bmem mechanism.  I should
> probably revisit this though, with stblinux-4.1 and stblinux-4.9, just
> to make sure this macro is still useful.

Sounds good, let me know if there is something that does not seem quite
right, we could fix it.

> 
>>> +
>>> +config BCMSTB_SDHCI
>>> +	bool ""
>>> +	default y
>>> +
>>> +config BCMSTB_SDHCI_BASE
>>> +	hex ""
>>> +	default 0xf03e0200
>>> +
>>> +config BCMSTB_SPI_BASE
>>> +	hex ""
>>> +	default 0xf03e3400
>>
>> Why don't you get those from the Device Tree blob that BOLT passes?
> 
> During development I did implement that for SDHCI_BASE, so it is
> possible.  But I ended up #ifdef'ing it out and hard-coding the address
> in production, to keep the runtime logic simpler.  Doing DTB traversal
> in code adds complexity but it may achieve portability to different
> BCM7xxx SoCs without further code changes, which would be nice.

Given what I see with Broadcom STB customers, I don't think there is a
strong push for anything other than BOLT, or another proprietary TSBL,
so with that, I am not sure about whether there would be an use case for
e.g; a single u-boot binary that would run on more than a flavor of a
given SoC. I would be inclined to put the different base register
addresses into a header file and use those constants in the respective
drivers (or even better, in the part of code that does register those).

> 
>>> +
>>> +config CMD_FDT_MAX_DUMP
>>> +	int ""
>>> +	default 256
>>> +
>>> +config GENERIC_MMC
>>> +	bool ""
>>> +	default y
>>> +
>>> +config MMC_SDMA
>>> +	bool ""
>>> +	default y
>>> +
>>> +config SDHCI
>>> +	bool ""
>>> +	default y
>>> +
>>> +config SYS_BCMSTB_SPI_WAIT
>>> +	int ""
>>> +	default 10
>>> +
>>> +config SYS_FDT_SAVE_ADDRESS
>>> +	hex ""
>>> +	default 0x1f00000
>>> +
>>> +config SYS_NO_FLASH
>>> +	bool ""
>>> +	default y
>>> +
>>> +config TIMER_FREQUENCY_REGISTER_ADDRESS
>>> +	hex ""
>>> +	default 0xf0412020
>>> +
>>> +config TIMER_LOW_REGISTER_ADDRESS
>>> +	hex ""
>>> +	default 0xf0412008
>>
>> All of these physical address ares not going to change given a
>> 7445-based design, so why not hard code them in a header file unless you
>> are keen on taking them from the passed Device Tree blob from BOLT?
> 
> Agreed, a header file or BOLT DTB lookup for these values would be
> better.  I think for a v2 of the patch, I'll move these to a header
> file.  If I get to adding another BCM7xxx board (I'm looking at BCM7260
> right now) then I can revisit BOLT DTB lookups in the context of
> portability between the two SoCs.

Sounds good.

> 
>>> +int dram_init_banksize(void)
>>> +{
>>> +	bd_t *bd = gd->bd;
>>> +
>>> +	bd->bi_dram[0].start = 0x00000000;
>>> +	bd->bi_dram[0].size  = 0x40000000;
>>> +	bd->bi_dram[1].start = 0x40000000;
>>> +	bd->bi_dram[1].size  = 0x40000000;
>>> +	bd->bi_dram[2].start = 0x80000000;
>>> +	bd->bi_dram[2].size  = 0x40000000;
>>
>> This may be true for your system if you have 3x1GB populated, but 7445
>> supports additional extension regions, so this must be configurable if
>> you want to make this flexible enough for other people to use it.
> 
> OK, I'm not familiar with extension regions.  Would it be enough to size
> and initialize bi_dram based on different CONFIG_NR_DRAM_BANKS values?

Humm, sure that would be a good step forward. This is something that
might be best extracted from the Device Tree provided by BOLT because
there is a standard memory node's "reg" property being provided.

A possible challenge with u-boot's bi_dram structure is that the
extension regions are way above the 4GB PA boundary. As long as you
don't need to access those in u-boot, you should be fine with a
"standard setup" (short page table descriptors etc.).
-- 
Florian