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

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

•  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).

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

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

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

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

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