[U-Boot] [PATCH 0/5] dragonboard 410c patches

Rob Clark (5): Makefile: also build fdt for snapdragon db410c: use fdt passed from lk db410c: add reserved-memory node to dts db410c: on aarch64 the fdtfile is in per-vendor subdirectory db410c: config updates
Makefile | 6 ++++ arch/arm/Kconfig | 2 +- arch/arm/dts/dragonboard410c.dts | 7 ++++- board/qualcomm/dragonboard410c/Makefile | 1 + board/qualcomm/dragonboard410c/dragonboard410c.c | 8 ++++++ board/qualcomm/dragonboard410c/lowlevel_init.S | 36 ++++++++++++++++++++++++ configs/dragonboard410c_defconfig | 8 +++++- include/configs/dragonboard410c.h | 2 +- 8 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 board/qualcomm/dragonboard410c/lowlevel_init.S

Snapdragon is a bit of a funny case, where we want to build the fdt to pass to lk, which loads us, but otherwise want to treat it as OF_BOARD, since lk will patch the fdt (for example, to insert simple-framebuffer node).
Signed-off-by: Rob Clark robdclark@gmail.com --- Makefile | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Makefile b/Makefile index 452596485d..0749cdfc5d 100644 --- a/Makefile +++ b/Makefile @@ -780,6 +780,12 @@ ALL-$(CONFIG_SPL_FRAMEWORK) += u-boot.img endif ALL-$(CONFIG_TPL) += tpl/u-boot-tpl.bin ALL-$(CONFIG_OF_SEPARATE) += u-boot.dtb +# Snapdragon devices, where lk/aboot loads u-boot is kind of a special +# case, because lk loads the fdt which is embedded (along with u-boot) +# in a boot.img. But it also does some fdt fixups. So generally we +# want to treat it like CONFIG_OF_BOARD=y, except that we also want +# to build the dtb's +ALL-$(CONFIG_ARCH_SNAPDRAGON) += u-boot.dtb ifeq ($(CONFIG_SPL_FRAMEWORK),y) ALL-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img endif

On Fri, Jul 21, 2017 at 03:10:10PM -0400, Rob Clark wrote:
Snapdragon is a bit of a funny case, where we want to build the fdt to pass to lk, which loads us, but otherwise want to treat it as OF_BOARD, since lk will patch the fdt (for example, to insert simple-framebuffer node).
Signed-off-by: Rob Clark robdclark@gmail.com
Makefile | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Makefile b/Makefile index 452596485d..0749cdfc5d 100644 --- a/Makefile +++ b/Makefile @@ -780,6 +780,12 @@ ALL-$(CONFIG_SPL_FRAMEWORK) += u-boot.img endif ALL-$(CONFIG_TPL) += tpl/u-boot-tpl.bin ALL-$(CONFIG_OF_SEPARATE) += u-boot.dtb +# Snapdragon devices, where lk/aboot loads u-boot is kind of a special +# case, because lk loads the fdt which is embedded (along with u-boot) +# in a boot.img. But it also does some fdt fixups. So generally we +# want to treat it like CONFIG_OF_BOARD=y, except that we also want +# to build the dtb's +ALL-$(CONFIG_ARCH_SNAPDRAGON) += u-boot.dtb ifeq ($(CONFIG_SPL_FRAMEWORK),y) ALL-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img endif
We really can't just select OF_SEPARATE here? Because the code in lib/fdtdec.c is wrong, as it stands, yes? If so, could we shuffle that code right to allow for the case of "we get our DT modified by firmware that loads us" ? That doesn't soune like a super uncommon case moving forward.

On Fri, Jul 21, 2017 at 6:18 PM, Tom Rini trini@konsulko.com wrote:
On Fri, Jul 21, 2017 at 03:10:10PM -0400, Rob Clark wrote:
Snapdragon is a bit of a funny case, where we want to build the fdt to pass to lk, which loads us, but otherwise want to treat it as OF_BOARD, since lk will patch the fdt (for example, to insert simple-framebuffer node).
Signed-off-by: Rob Clark robdclark@gmail.com
Makefile | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Makefile b/Makefile index 452596485d..0749cdfc5d 100644 --- a/Makefile +++ b/Makefile @@ -780,6 +780,12 @@ ALL-$(CONFIG_SPL_FRAMEWORK) += u-boot.img endif ALL-$(CONFIG_TPL) += tpl/u-boot-tpl.bin ALL-$(CONFIG_OF_SEPARATE) += u-boot.dtb +# Snapdragon devices, where lk/aboot loads u-boot is kind of a special +# case, because lk loads the fdt which is embedded (along with u-boot) +# in a boot.img. But it also does some fdt fixups. So generally we +# want to treat it like CONFIG_OF_BOARD=y, except that we also want +# to build the dtb's +ALL-$(CONFIG_ARCH_SNAPDRAGON) += u-boot.dtb ifeq ($(CONFIG_SPL_FRAMEWORK),y) ALL-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img endif
We really can't just select OF_SEPARATE here? Because the code in lib/fdtdec.c is wrong, as it stands, yes? If so, could we shuffle that code right to allow for the case of "we get our DT modified by firmware that loads us" ? That doesn't soune like a super uncommon case moving forward.
from a quick look, I think the only reason we need OF_BOARD is the call to board_fdt_blob_setup() in fdtdec..
I guess we could make that a weak sym, and call it unconditionally.. in that case I think OF_SEPARATE would work. If you think that is a cleaner solution, I can give that a try.
BR, -R

On Fri, Jul 21, 2017 at 06:57:58PM -0400, Rob Clark wrote:
On Fri, Jul 21, 2017 at 6:18 PM, Tom Rini trini@konsulko.com wrote:
On Fri, Jul 21, 2017 at 03:10:10PM -0400, Rob Clark wrote:
Snapdragon is a bit of a funny case, where we want to build the fdt to pass to lk, which loads us, but otherwise want to treat it as OF_BOARD, since lk will patch the fdt (for example, to insert simple-framebuffer node).
Signed-off-by: Rob Clark robdclark@gmail.com
Makefile | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Makefile b/Makefile index 452596485d..0749cdfc5d 100644 --- a/Makefile +++ b/Makefile @@ -780,6 +780,12 @@ ALL-$(CONFIG_SPL_FRAMEWORK) += u-boot.img endif ALL-$(CONFIG_TPL) += tpl/u-boot-tpl.bin ALL-$(CONFIG_OF_SEPARATE) += u-boot.dtb +# Snapdragon devices, where lk/aboot loads u-boot is kind of a special +# case, because lk loads the fdt which is embedded (along with u-boot) +# in a boot.img. But it also does some fdt fixups. So generally we +# want to treat it like CONFIG_OF_BOARD=y, except that we also want +# to build the dtb's +ALL-$(CONFIG_ARCH_SNAPDRAGON) += u-boot.dtb ifeq ($(CONFIG_SPL_FRAMEWORK),y) ALL-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img endif
We really can't just select OF_SEPARATE here? Because the code in lib/fdtdec.c is wrong, as it stands, yes? If so, could we shuffle that code right to allow for the case of "we get our DT modified by firmware that loads us" ? That doesn't soune like a super uncommon case moving forward.
from a quick look, I think the only reason we need OF_BOARD is the call to board_fdt_blob_setup() in fdtdec..
I guess we could make that a weak sym, and call it unconditionally.. in that case I think OF_SEPARATE would work. If you think that is a cleaner solution, I can give that a try.
I think so. I really don't want to have changes in the Makefiles to special case logic like this if we can avoid it, without making the code ugly too. Thanks!

lk patches the fdt to set some device's MAC addresses and more importantly to patch in the simple-framebuffer node that we want u-boot to see.
Signed-off-by: Rob Clark robdclark@gmail.com --- board/qualcomm/dragonboard410c/Makefile | 1 + board/qualcomm/dragonboard410c/dragonboard410c.c | 8 ++++++ board/qualcomm/dragonboard410c/lowlevel_init.S | 36 ++++++++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 board/qualcomm/dragonboard410c/lowlevel_init.S
diff --git a/board/qualcomm/dragonboard410c/Makefile b/board/qualcomm/dragonboard410c/Makefile index cd678088fa..5082383be4 100644 --- a/board/qualcomm/dragonboard410c/Makefile +++ b/board/qualcomm/dragonboard410c/Makefile @@ -5,4 +5,5 @@ #
obj-y := dragonboard410c.o +obj-y += lowlevel_init.o extra-y += head.o diff --git a/board/qualcomm/dragonboard410c/dragonboard410c.c b/board/qualcomm/dragonboard410c/dragonboard410c.c index 37d0b85e0e..1fa4dc1b15 100644 --- a/board/qualcomm/dragonboard410c/dragonboard410c.c +++ b/board/qualcomm/dragonboard410c/dragonboard410c.c @@ -27,6 +27,14 @@ int dram_init_banksize(void) return 0; }
+extern unsigned long fw_dtb_pointer; + +void *board_fdt_blob_setup(void) +{ + if (fdt_magic(fw_dtb_pointer) != FDT_MAGIC) + return NULL; + return (void *)fw_dtb_pointer; +}
int board_prepare_usb(enum usb_init_type type) { diff --git a/board/qualcomm/dragonboard410c/lowlevel_init.S b/board/qualcomm/dragonboard410c/lowlevel_init.S new file mode 100644 index 0000000000..cdbd8e14db --- /dev/null +++ b/board/qualcomm/dragonboard410c/lowlevel_init.S @@ -0,0 +1,36 @@ +/* + * (C) Copyright 2016 + * Cédric Schieli cschieli@gmail.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <config.h> + +.align 8 +.global fw_dtb_pointer +fw_dtb_pointer: +#ifdef CONFIG_ARM64 + .dword 0x0 +#else + .word 0x0 +#endif + +/* + * Routine: save_boot_params (called after reset from start.S) + * Description: save ATAG/FDT address provided by the firmware at boot time + */ + +.global save_boot_params +save_boot_params: + + /* The firmware provided ATAG/FDT address can be found in r2/x0 */ +#ifdef CONFIG_ARM64 + adr x8, fw_dtb_pointer + str x0, [x8] +#else + str r2, fw_dtb_pointer +#endif + + /* Returns */ + b save_boot_params_ret

If lk lights up display and populates simple-framebuffer node, it will also setup a reserved-memory node (needed by simplefb on linux). But it isn't clever enough to cope when the reserved-memory node is not present.
Signed-off-by: Rob Clark robdclark@gmail.com --- arch/arm/dts/dragonboard410c.dts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/dragonboard410c.dts b/arch/arm/dts/dragonboard410c.dts index 7746622dda..c239d9d997 100644 --- a/arch/arm/dts/dragonboard410c.dts +++ b/arch/arm/dts/dragonboard410c.dts @@ -23,11 +23,16 @@ reg = <0 0x80000000 0 0x3da00000>; };
+ reserved-memory { + #address-cells = <2>; + #size-cells = <2>; + ranges; + }; + chosen { stdout-path = "/soc/serial@78b0000"; };
- soc { #address-cells = <0x1>; #size-cells = <0x1>;

Signed-off-by: Rob Clark robdclark@gmail.com --- include/configs/dragonboard410c.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/configs/dragonboard410c.h b/include/configs/dragonboard410c.h index 11c842d952..8e742d82ce 100644 --- a/include/configs/dragonboard410c.h +++ b/include/configs/dragonboard410c.h @@ -104,7 +104,7 @@ REFLASH(dragonboard/u-boot.img, 8)\ "initrd_high=0xffffffffffffffff\0" \ "linux_image=Image\0" \ "kernel_addr_r=0x81000000\0"\ - "fdtfile=apq8016-sbc.dtb\0" \ + "fdtfile=qcom/apq8016-sbc.dtb\0" \ "fdt_addr_r=0x83000000\0"\ "ramdisk_addr_r=0x84000000\0"\ "scriptaddr=0x90000000\0"\

Signed-off-by: Rob Clark robdclark@gmail.com --- arch/arm/Kconfig | 2 +- configs/dragonboard410c_defconfig | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index f7b44392ac..db6a325ee1 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -633,7 +633,7 @@ config ARCH_SNAPDRAGON select DM_SERIAL select SPMI select OF_CONTROL - select OF_SEPARATE + select OF_BOARD
config ARCH_SOCFPGA bool "Altera SOCFPGA family" diff --git a/configs/dragonboard410c_defconfig b/configs/dragonboard410c_defconfig index d992c2adda..45a121822f 100644 --- a/configs/dragonboard410c_defconfig +++ b/configs/dragonboard410c_defconfig @@ -37,4 +37,10 @@ CONFIG_USB_EHCI_MSM=y CONFIG_USB_ULPI_VIEWPORT=y CONFIG_USB_ULPI=y CONFIG_USB_STORAGE=y -CONFIG_OF_LIBFDT_OVERLAY=y +CONFIG_DM_VIDEO=y +# CONFIG_VIDEO_BPP8 is not set +CONFIG_VIDEO_BPP16=y +CONFIG_VIDEO_BPP32=y +CONFIG_CONSOLE_NORMAL=y +CONFIG_VIDEO_SIMPLE=y +CONFIG_OF_BOARD=y

On Fri, Jul 21, 2017 at 03:10:14PM -0400, Rob Clark wrote:
Signed-off-by: Rob Clark robdclark@gmail.com
arch/arm/Kconfig | 2 +- configs/dragonboard410c_defconfig | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index f7b44392ac..db6a325ee1 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -633,7 +633,7 @@ config ARCH_SNAPDRAGON select DM_SERIAL select SPMI select OF_CONTROL
- select OF_SEPARATE
- select OF_BOARD
config ARCH_SOCFPGA bool "Altera SOCFPGA family" diff --git a/configs/dragonboard410c_defconfig b/configs/dragonboard410c_defconfig index d992c2adda..45a121822f 100644 --- a/configs/dragonboard410c_defconfig +++ b/configs/dragonboard410c_defconfig @@ -37,4 +37,10 @@ CONFIG_USB_EHCI_MSM=y CONFIG_USB_ULPI_VIEWPORT=y CONFIG_USB_ULPI=y CONFIG_USB_STORAGE=y -CONFIG_OF_LIBFDT_OVERLAY=y +CONFIG_DM_VIDEO=y +# CONFIG_VIDEO_BPP8 is not set +CONFIG_VIDEO_BPP16=y +CONFIG_VIDEO_BPP32=y +CONFIG_CONSOLE_NORMAL=y +CONFIG_VIDEO_SIMPLE=y +CONFIG_OF_BOARD=y
This doesn't look like you updated it with savedefconfig, did you? Also, we don't want to nuke the overlay support I assume. Thanks!

On Fri, Jul 21, 2017 at 6:20 PM, Tom Rini trini@konsulko.com wrote:
On Fri, Jul 21, 2017 at 03:10:14PM -0400, Rob Clark wrote:
Signed-off-by: Rob Clark robdclark@gmail.com
arch/arm/Kconfig | 2 +- configs/dragonboard410c_defconfig | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index f7b44392ac..db6a325ee1 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -633,7 +633,7 @@ config ARCH_SNAPDRAGON select DM_SERIAL select SPMI select OF_CONTROL
select OF_SEPARATE
select OF_BOARD
config ARCH_SOCFPGA bool "Altera SOCFPGA family" diff --git a/configs/dragonboard410c_defconfig b/configs/dragonboard410c_defconfig index d992c2adda..45a121822f 100644 --- a/configs/dragonboard410c_defconfig +++ b/configs/dragonboard410c_defconfig @@ -37,4 +37,10 @@ CONFIG_USB_EHCI_MSM=y CONFIG_USB_ULPI_VIEWPORT=y CONFIG_USB_ULPI=y CONFIG_USB_STORAGE=y -CONFIG_OF_LIBFDT_OVERLAY=y +CONFIG_DM_VIDEO=y +# CONFIG_VIDEO_BPP8 is not set +CONFIG_VIDEO_BPP16=y +CONFIG_VIDEO_BPP32=y +CONFIG_CONSOLE_NORMAL=y +CONFIG_VIDEO_SIMPLE=y +CONFIG_OF_BOARD=y
This doesn't look like you updated it with savedefconfig, did you?
No, probably not.. tbh this is the first time I've done a defconfig update (u-boot or kernel), board level stuff is a bit outside what I normally work on (ie. mesa and gpu/drm), so pls forgive cluelessness in this regard..
Also, we don't want to nuke the overlay support I assume. Thanks!
probably not.. although it is still an open question, I think, from distro standpoint how to eventually pass the right fdt to kernel. As it stands, the stage before u-boot patches the fdt bundled with u-boot.img with useful things like wifi/bt mac addresses. But the fdt in u-boot.img (at least if built from u-boot tree) is highly insufficient to enabled all of the upstream drivers. And requiring the end user to flash a new u-boot.img w/ bundled fdt as new features (where there is not yet any clue how dt bindings should look, like bus-scaling), isn't super friendly.
We might eventually want some optional board hook to let board specific code patch an fdt loaded from OS media by u-boot (or grub?) based on fields in fdt passed from previous stage to u-boot. Maybe overlays are a way to do that.. idk, I still need to look into how they work..
BR, -R

On Fri, Jul 21, 2017 at 07:08:23PM -0400, Rob Clark wrote:
On Fri, Jul 21, 2017 at 6:20 PM, Tom Rini trini@konsulko.com wrote:
On Fri, Jul 21, 2017 at 03:10:14PM -0400, Rob Clark wrote:
Signed-off-by: Rob Clark robdclark@gmail.com
arch/arm/Kconfig | 2 +- configs/dragonboard410c_defconfig | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index f7b44392ac..db6a325ee1 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -633,7 +633,7 @@ config ARCH_SNAPDRAGON select DM_SERIAL select SPMI select OF_CONTROL
select OF_SEPARATE
select OF_BOARD
config ARCH_SOCFPGA bool "Altera SOCFPGA family" diff --git a/configs/dragonboard410c_defconfig b/configs/dragonboard410c_defconfig index d992c2adda..45a121822f 100644 --- a/configs/dragonboard410c_defconfig +++ b/configs/dragonboard410c_defconfig @@ -37,4 +37,10 @@ CONFIG_USB_EHCI_MSM=y CONFIG_USB_ULPI_VIEWPORT=y CONFIG_USB_ULPI=y CONFIG_USB_STORAGE=y -CONFIG_OF_LIBFDT_OVERLAY=y +CONFIG_DM_VIDEO=y +# CONFIG_VIDEO_BPP8 is not set +CONFIG_VIDEO_BPP16=y +CONFIG_VIDEO_BPP32=y +CONFIG_CONSOLE_NORMAL=y +CONFIG_VIDEO_SIMPLE=y +CONFIG_OF_BOARD=y
This doesn't look like you updated it with savedefconfig, did you?
No, probably not.. tbh this is the first time I've done a defconfig update (u-boot or kernel), board level stuff is a bit outside what I normally work on (ie. mesa and gpu/drm), so pls forgive cluelessness in this regard..
Ah, yeah. It's the handy-dandy way to keep things in sync and sometimes even avoid patch conflicts with others.
Also, we don't want to nuke the overlay support I assume. Thanks!
probably not.. although it is still an open question, I think, from distro standpoint how to eventually pass the right fdt to kernel. As it stands, the stage before u-boot patches the fdt bundled with u-boot.img with useful things like wifi/bt mac addresses. But the fdt in u-boot.img (at least if built from u-boot tree) is highly insufficient to enabled all of the upstream drivers. And requiring the end user to flash a new u-boot.img w/ bundled fdt as new features (where there is not yet any clue how dt bindings should look, like bus-scaling), isn't super friendly.
We might eventually want some optional board hook to let board specific code patch an fdt loaded from OS media by u-boot (or grub?) based on fields in fdt passed from previous stage to u-boot. Maybe overlays are a way to do that.. idk, I still need to look into how they work..
Yeah, overlays are the standard DT way to deal with things like add-on boards for 96boards/CHIP/beaglebone/etc, along with similar but non-dev board cases. We want to keep the 'overlay' option enabled for Dragonboard I think :)
participants (2)
-
Rob Clark
-
Tom Rini