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

[Thomas Fitzsimmons <fitzsim@fitzsim.org>]

Florian Fainelli <f.fainelli@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@fitzsim.org>
>> Cc: Stefan Roese <sr@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.

>> +
>> +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.

>> +
>> +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.

>> +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?

>> +/* Copied from stblinux, include/linux/brcmstb/brcmstb.h. */
>> +#define DEV_RD(x) (readl((x)))
>> +#define DEV_WR(x, y) do { writel((y), (x)); } while (0)
>> +#define DEV_UNSET(x, y) do { DEV_WR((x), DEV_RD(x) & ~(y)); } while (0)
>> +#define DEV_SET(x, y) do { DEV_WR((x), DEV_RD(x) | (y)); } while (0)
>> +
>> +#define DEV_WR_RB(x, y) do { DEV_WR((x), (y)); DEV_RD(x); } while (0)
>> +#define DEV_SET_RB(x, y) do { DEV_SET((x), (y)); DEV_RD(x); } while (0)
>> +#define DEV_UNSET_RB(x, y) do { DEV_UNSET((x), (y)); DEV_RD(x); } while (0)
>
> I would just flat out drop those macros and instead use standard
> accessors. Those happen to work just fine given Broadcom STB's GISB bus,
> but if you want portable drivers in u-boot, and you likely would want
> those, you should use more standard I/O accessors.

OK, I'll look into this.

Thanks for reviewing,
Thomas