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

2018-06-08 Thread Thomas Fitzsimmons 
Florian Fainelli  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 
>> Cc: Stefan Roese 
>> Cc: Tom Rini 
>> Cc: Florian Fainelli 
>> ---
>
> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2018-06-07 Thread 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 
> Cc: Stefan Roese 
> Cc: Tom Rini 
> Cc: Florian Fainelli 
> ---

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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2018-06-06 Thread Thomas Fitzsimmons 
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 
Cc: Stefan Roese 
Cc: Tom Rini 
Cc: Florian Fainelli 
---
Changes for v3:
   - Rebase to master
   - Add "ARM BROADCOM BCMSTB" entry to top-level MAINTAINERS
   - Fix SPDX formatting
   - In Kconfig use CPU_V7A, not CPU_V7, per acf15001...

 MAINTAINERS |  10 +
 arch/arm/Kconfig|  12 +
 arch/arm/Makefile   |   1 +
 arch/arm/mach-bcmstb/Kconfig|  64 
 arch/arm/mach-bcmstb/Makefile   |   8 +
 arch/arm/mach-bcmstb/include/mach/gpio.h|  11 +
 arch/arm/mach-bcmstb/include/mach/hardware.h|  11 +
 arch/arm/mach-bcmstb/include/mach/prior_stage.h |  30 ++
 arch/arm/mach-bcmstb/include/mach/sdhci.h   |  15 +
 arch/arm/mach-bcmstb/include/mach/timer.h   |  13 +
 arch/arm/mach-bcmstb/lowlevel_init.S|  21 ++
 board/broadcom/bcmstb/MAINTAINERS   |   6 +
 board/broadcom/bcmstb/Makefile  |   8 +
 board/broadcom/bcmstb/bcmstb.c  | 191 +++
 configs/bcm7445_defconfig   |  27 ++
 doc/README.bcm7xxx  | 147 
 drivers/mmc/Kconfig |  11 +
 drivers/mmc/Makefile|   1 +
 drivers/mmc/bcmstb_sdhci.c  |  67 
 drivers/spi/Kconfig |   7 +
 drivers/spi/Makefile|   1 +
 drivers/spi/bcmstb_spi.c| 439 
 drivers/spi/spi-uclass.c|   2 +-
 dts/Kconfig |   7 +
 include/configs/bcmstb.h| 188 ++
 include/fdtdec.h|   4 +
 lib/fdtdec.c|   4 +
 27 files changed, 1305 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-bcmstb/Kconfig
 create mode 100644 arch/arm/mach-bcmstb/Makefile
 create mode 100644 arch/arm/mach-bcmstb/include/mach/gpio.h
 create mode 100644 arch/arm/mach-bcmstb/include/mach/hardware.h
 create mode 100644 arch/arm/mach-bcmstb/include/mach/prior_stage.h
 create mode 100644 arch/arm/mach-bcmstb/include/mach/sdhci.h
 create mode 100644 arch/arm/mach-bcmstb/include/mach/timer.h
 create mode 100644 arch/arm/mach-bcmstb/lowlevel_init.S
 create mode 100644 board/broadcom/bcmstb/MAINTAINERS
 create mode 100644 board/broadcom/bcmstb/Makefile
 create mode 100644 board/broadcom/bcmstb/bcmstb.c
 create mode 100644 configs/bcm7445_defconfig
 create mode 100644 doc/README.bcm7xxx
 create mode 100644 drivers/mmc/bcmstb_sdhci.c
 create mode 100644 drivers/spi/bcmstb_spi.c
 create mode 100644 include/configs/bcmstb.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 642c448..58634fc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -107,6 +107,16 @@ F: drivers/video/bcm2835.c
 F: include/dm/platform_data/serial_bcm283x_mu.h
 F: drivers/pinctrl/broadcom/
 
+ARM BROADCOM BCMSTB
+M: Thomas Fitzsimmons 
+S: Maintained
+F: arch/arm/mach-bcmstb/
+F: board/broadcom/bcmstb/
+F: configs/bcm7445_defconfig
+F: doc/README.bcm7xxx
+F: drivers/mmc/bcmstb_sdhci.c
+F: drivers/spi/bcmstb_spi.c
+
 ARM FREESCALE IMX
 M: Stefano Babic 
 M: Fabio Estevam 
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index dde422b..fa2001b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -533,6 +533,16 @@ config TARGET_VEXPRESS_CA15_TC2
select CPU_V7_HAS_VIRT
select PL011_SERIAL
 
+config ARCH_BCMSTB
+   bool "Broadcom BCM7XXX family"
+   select CPU_V7A
+   select DM
+   select OF_CONTROL
+   select OF_PRIOR_STAGE
+   help
+ This enables support for Broadcom ARM-based set-top box
+ chipsets, including the 7445 family of chips.
+
 config TARGET_VEXPRESS_CA5X2
bool "Support vexpress_ca5x2"
select CPU_V7A
@@ -1297,6 +1307,8 @@ source "arch/arm/mach-at91/Kconfig"
 
 source "arch/arm/mach-bcm283x/Kconfig"
 
+source "arch/arm/mach-bcmstb/Kconfig"
+
 source "arch/arm/mach-davinci/Kconfig"
 
 source "arch/arm/mach-exynos/Kconfig"
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 680c6e8..03252fe 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -54,6 +54,7 @@ PLATFORM_CPPFLAGS += $(arch-y) $(tune-y)
 machine-$(CONFIG_ARCH_ASPEED)  += aspeed
 machine-$(CONFIG_ARCH_AT91)+= at91
 machine-$(CONFIG_ARCH_BCM283X) += bcm283x
+machine-$(CONFIG_ARCH_BCMSTB)  += bcmstb
 machine-$(CONFIG_ARCH_DAVINCI) += davinci
 machine-$(CONFIG_ARCH_EXYNOS)  += exynos
 machine-$(CONFIG_ARCH_HIGHBANK)+= highbank
diff --git