[PATCH 00/10] xilinx: Add support for DTB reselection
Hi,
this series add support for board or board+cc runtime DT selection. EEPROM memory is read and based on that decoded if this is legacy/fru based format and proper DTB is used. There is a need to have all DTBs 64bit aligned. If you don't have it you will end up in exception. But one patch in this series is trying to detect it and panic before you reach it to let you know what's wrong.
Enforcing mkimage/dtb alignment is done based on CONFIG_PHYS_64BIT and affects all 64bit systems but it is also not wrong for them to be properly aligned.
Thanks, Michal
Michal Simek (10): xilinx: fru: Replace spaces with \0 in detected name xilinx: Use variable for passing board_name xilinx: common: Change board_info[] handling xilinx: common: Free allocated structure xilinx: Add support for generic board detection xilinx: zynqmp: Check that DT is 64bit aligned Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems arm64: dts: Make sure that all DTBs are 64bit aligned for 64bit systems xilinx: zynqmp: Generate different u-boot.itb for MULTI_DTB_FIT xilinx: common: Enabling generic function for DT reselection
Makefile | 7 ++ arch/arm/dts/Makefile | 4 + arch/arm/dts/zynqmp-sm-k26-revA.dts | 3 + arch/arm/mach-zynqmp/mkimage_fit_atf.sh | 47 +++++++ board/xilinx/common/board.c | 160 +++++++++++++++++++----- board/xilinx/zynqmp/zynqmp.c | 3 + 6 files changed, 194 insertions(+), 30 deletions(-)
FRU spec expected \0 for unused symbols but unfortunately a lot of boards are using spaces instead of \0. That's why after saving it to desc->name name is checked again and all spaces are converted to \0. This will ensure that names can be used for string manipulations like concatenation.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
board/xilinx/common/board.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c index 90c87bab5cff..1458b31c21a9 100644 --- a/board/xilinx/common/board.c +++ b/board/xilinx/common/board.c @@ -168,7 +168,7 @@ static bool xilinx_detect_legacy(u8 *buffer) static int xilinx_read_eeprom_fru(struct udevice *dev, char *name, struct xilinx_board_description *desc) { - int ret, eeprom_size; + int i, ret, eeprom_size; u8 *fru_content;
/* FIXME this is shortcut - if eeprom type is wrong it will fail */ @@ -207,6 +207,10 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char *name, sizeof(desc->manufacturer)); strncpy(desc->name, (char *)fru_data.brd.product_name, sizeof(desc->name)); + for (i = 0; i < sizeof(desc->name); i++) { + if (desc->name[i] == ' ') + desc->name[i] = '\0'; + } strncpy(desc->revision, (char *)fru_data.brd.rev, sizeof(desc->revision)); strncpy(desc->serial, (char *)fru_data.brd.serial_number,
Use variable which points to DEVICE_TREE by default. The reason for this change is to enable DTB_RESELECT and MULTI_DTB_FIT where board detection can be used for change DTB at run time. That's why there must be reference in board_fit_config_name_match() via variable instead of hardcoding it which is sufficient for that use case.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
board/xilinx/common/board.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c index 1458b31c21a9..db9705c1b7e4 100644 --- a/board/xilinx/common/board.c +++ b/board/xilinx/common/board.c @@ -436,11 +436,13 @@ int board_late_init_xilinx(void) } #endif
+static char *board_name = DEVICE_TREE; + int __maybe_unused board_fit_config_name_match(const char *name) { - debug("%s: Check %s, default %s\n", __func__, name, DEVICE_TREE); + debug("%s: Check %s, default %s\n", __func__, name, board_name);
- if (!strcmp(name, DEVICE_TREE)) + if (!strcmp(name, board_name)) return 0;
return -1;
Origin code was allocating only pointers to struct xilinx_board_description and there was separate allocation for structure self and freeing in case of failure. The code is directly allocating space for all structures by one calloc to simlify logic.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
board/xilinx/common/board.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c index db9705c1b7e4..44c8aa5eefbb 100644 --- a/board/xilinx/common/board.c +++ b/board/xilinx/common/board.c @@ -68,7 +68,7 @@ struct xilinx_board_description { };
static int highest_id = -1; -static struct xilinx_board_description **board_info; +static struct xilinx_board_description *board_info;
#define XILINX_I2C_DETECTION_BITS sizeof(struct fru_common_hdr)
@@ -280,7 +280,7 @@ static int xilinx_read_eeprom_single(char *name,
__maybe_unused int xilinx_read_eeprom(void) { - int id, ret; + int id; char name_buf[8]; /* 8 bytes should be enough for nvmem+number */ struct xilinx_board_description *desc;
@@ -289,7 +289,7 @@ __maybe_unused int xilinx_read_eeprom(void) if (highest_id < 0) return -EINVAL;
- board_info = calloc(1, sizeof(desc) * highest_id); + board_info = calloc(1, sizeof(*desc) * (highest_id + 1)); if (!board_info) return -ENOMEM;
@@ -300,21 +300,10 @@ __maybe_unused int xilinx_read_eeprom(void) snprintf(name_buf, sizeof(name_buf), "nvmem%d", id);
/* Alloc structure */ - desc = board_info[id]; - if (!desc) { - desc = calloc(1, sizeof(*desc)); - if (!desc) - return -ENOMEM; - - board_info[id] = desc; - } + desc = &board_info[id];
/* Ignoring return value for supporting multiple chips */ - ret = xilinx_read_eeprom_single(name_buf, desc); - if (ret) { - free(desc); - board_info[id] = NULL; - } + xilinx_read_eeprom_single(name_buf, desc); }
/* @@ -400,7 +389,7 @@ int board_late_init_xilinx(void) ret |= env_set_addr("bootm_size", (void *)bootm_size);
for (id = 0; id <= highest_id; id++) { - desc = board_info[id]; + desc = &board_info[id]; if (desc && desc->header == EEPROM_HEADER_MAGIC) { if (desc->manufacturer[0]) ret |= env_set_by_index("manufacturer", id,
There is no need to keep fru_content around. Free this space.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
board/xilinx/common/board.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c index 44c8aa5eefbb..2aecb14d8e27 100644 --- a/board/xilinx/common/board.c +++ b/board/xilinx/common/board.c @@ -185,8 +185,7 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char *name, eeprom_size); if (ret) { debug("%s: I2C EEPROM read failed\n", __func__); - free(fru_content); - return ret; + goto end; }
printf("Xilinx I2C FRU format at %s:\n", name); @@ -194,12 +193,13 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char *name, ret = fru_display(0); if (ret) { printf("FRU format decoding failed.\n"); - return ret; + goto end; }
if (desc->header == EEPROM_HEADER_MAGIC) { debug("Information already filled\n"); - return -EINVAL; + ret = -EINVAL; + goto end; }
/* It is clear that FRU was captured and structures were filled */ @@ -217,7 +217,9 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char *name, sizeof(desc->serial)); desc->header = EEPROM_HEADER_MAGIC;
- return 0; +end: + free(fru_content); + return ret; }
static bool xilinx_detect_fru(u8 *buffer)
Add support for changing DT at run time. It is done via board_detection() which returns platform_id and platform_version which can be used via board_name_decode() to compose board_local_name string which corresponds with DT which is should be used.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
board/xilinx/common/board.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c index 2aecb14d8e27..92874272893a 100644 --- a/board/xilinx/common/board.c +++ b/board/xilinx/common/board.c @@ -463,3 +463,34 @@ int print_cpuinfo(void) return 0; } #endif + +#if CONFIG_IS_ENABLED(DTB_RESELECT) +char * __maybe_unused __weak board_name_decode(void) +{ + return NULL; +} + +bool __maybe_unused __weak board_detection(void) +{ + return false; +} + +int embedded_dtb_select(void) +{ + if (board_detection()) { + char *board_local_name; + + board_local_name = board_name_decode(); + if (board_local_name) { + board_name = board_local_name; + debug("Detected name: %s\n", board_name); + + /* Time to change DTB on fly */ + /* Both ways should work here */ + /* fdtdec_resetup(&rescan); */ + fdtdec_setup(); + } + } + return 0; +} +#endif
DT needs to be 64bit aligned. If it is not fdt64_to_cpu will fail when try to read information about reserved memory. The system ends in exception without any clue what's going it. That's why detect not aligned DT and panic to show where the issue is coming from.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
board/xilinx/zynqmp/zynqmp.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index e43177ea4e48..ea15e62eb21e 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -470,6 +470,9 @@ ulong board_get_usable_ram_top(ulong total_size) phys_addr_t reg; struct lmb lmb;
+ if (!IS_ALIGNED((ulong)gd->fdt_blob, 0x8)) + panic("Not 64bit aligned DT location: %p\n", gd->fdt_blob); + /* found enough not-reserved memory to relocated U-Boot */ lmb_init(&lmb); lmb_add(&lmb, gd->ram_base, gd->ram_size);
Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT image placed and aligned only by 32bits (4bytes). For 64bit systems there is 64bit (8bytes) alignment required. That's why make sure that fit-dtb.blob and u-boot.itb as our primary target images for Xilinx ZynqMP are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to identify 64bit systems (including 32bit systems with PAE).
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
Makefile | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/Makefile b/Makefile index 269e353a28ad..1bbe95595efe 100644 --- a/Makefile +++ b/Makefile @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) -T firmware -C none -O u-boot \ -a 0 -e 0 -E \ $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) -d /dev/null
+ifeq ($(CONFIG_PHYS_64BIT),y) +MKIMAGEFLAGS_fit-dtb.blob += -B 0x8 +endif + ifneq ($(EXT_DTB),) u-boot-fit-dtb.bin: u-boot-nodtb.bin $(EXT_DTB) $(call if_changed,cat) @@ -1431,6 +1435,9 @@ MKIMAGEFLAGS_u-boot.itb = else MKIMAGEFLAGS_u-boot.itb = -E endif +ifeq ($(CONFIG_PHYS_64BIT),y) +MKIMAGEFLAGS_u-boot.itb += -B 0x8 +endif
ifdef U_BOOT_ITS u-boot.itb: u-boot-nodtb.bin \
On 8/19/21 12:19 PM, Michal Simek wrote:
Hi,
Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT image placed and aligned only by 32bits (4bytes). For 64bit systems there is 64bit (8bytes) alignment required. That's why make sure that fit-dtb.blob and u-boot.itb as our primary target images for Xilinx ZynqMP are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to identify 64bit systems (including 32bit systems with PAE).
Signed-off-by: Michal Simek michal.simek@xilinx.com
Makefile | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/Makefile b/Makefile index 269e353a28ad..1bbe95595efe 100644 --- a/Makefile +++ b/Makefile @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) -T firmware -C none -O u-boot \ -a 0 -e 0 -E \ $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) -d /dev/null
+ifeq ($(CONFIG_PHYS_64BIT),y)
Why is this restricted to 64-bit "systems"? The DT spec[1] clearly states that some DT parts (/memreserved/ block, for instance), must be 64-bit aligned, which means the whole blobs needs to be 64-bit aligned. Granted this probably does not cause real issues on 32-bit systems, but is violating the spec anyway. So I'd say we add the alignment requirement unconditionally.
Cheers, Andre
[1] https://github.com/devicetree-org/devicetree-specification/releases/download...
+MKIMAGEFLAGS_fit-dtb.blob += -B 0x8 +endif
- ifneq ($(EXT_DTB),) u-boot-fit-dtb.bin: u-boot-nodtb.bin $(EXT_DTB) $(call if_changed,cat)
@@ -1431,6 +1435,9 @@ MKIMAGEFLAGS_u-boot.itb = else MKIMAGEFLAGS_u-boot.itb = -E endif +ifeq ($(CONFIG_PHYS_64BIT),y) +MKIMAGEFLAGS_u-boot.itb += -B 0x8 +endif
ifdef U_BOOT_ITS u-boot.itb: u-boot-nodtb.bin \
Hi Andre,
On 8/19/21 5:56 PM, Andre Przywara wrote:
On 8/19/21 12:19 PM, Michal Simek wrote:
Hi,
Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT image placed and aligned only by 32bits (4bytes). For 64bit systems there is 64bit (8bytes) alignment required. That's why make sure that fit-dtb.blob and u-boot.itb as our primary target images for Xilinx ZynqMP are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to identify 64bit systems (including 32bit systems with PAE).
Signed-off-by: Michal Simek michal.simek@xilinx.com
Makefile | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/Makefile b/Makefile index 269e353a28ad..1bbe95595efe 100644 --- a/Makefile +++ b/Makefile @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) -T firmware -C none -O u-boot \ -a 0 -e 0 -E \ $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) -d /dev/null +ifeq ($(CONFIG_PHYS_64BIT),y)
Why is this restricted to 64-bit "systems"? The DT spec[1] clearly states that some DT parts (/memreserved/ block, for instance), must be 64-bit aligned, which means the whole blobs needs to be 64-bit aligned. Granted this probably does not cause real issues on 32-bit systems, but is violating the spec anyway. So I'd say we add the alignment requirement unconditionally.
That's even better for me and we need to make sure that dtbs itself are aligned and also dtbs inside FIT image are aligned too.
Cheers, Michal
On Thu, Aug 19, 2021 at 06:01:39PM +0200, Michal Simek wrote:
Hi Andre,
On 8/19/21 5:56 PM, Andre Przywara wrote:
On 8/19/21 12:19 PM, Michal Simek wrote:
Hi,
Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT image placed and aligned only by 32bits (4bytes). For 64bit systems there is 64bit (8bytes) alignment required. That's why make sure that fit-dtb.blob and u-boot.itb as our primary target images for Xilinx ZynqMP are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to identify 64bit systems (including 32bit systems with PAE).
Signed-off-by: Michal Simek michal.simek@xilinx.com
Makefile | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/Makefile b/Makefile index 269e353a28ad..1bbe95595efe 100644 --- a/Makefile +++ b/Makefile @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) -T firmware -C none -O u-boot \ -a 0 -e 0 -E \ $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) -d /dev/null +ifeq ($(CONFIG_PHYS_64BIT),y)
Why is this restricted to 64-bit "systems"? The DT spec[1] clearly states that some DT parts (/memreserved/ block, for instance), must be 64-bit aligned, which means the whole blobs needs to be 64-bit aligned. Granted this probably does not cause real issues on 32-bit systems, but is violating the spec anyway. So I'd say we add the alignment requirement unconditionally.
That's even better for me and we need to make sure that dtbs itself are aligned and also dtbs inside FIT image are aligned too.
Right, all dtbs need to be 8 byte aligned to start with. Enforcing this is what will unblock moving to a newer libfdt where that's checked for up-front now.
On 8/19/21 6:18 PM, Tom Rini wrote:
On Thu, Aug 19, 2021 at 06:01:39PM +0200, Michal Simek wrote:
Hi Andre,
On 8/19/21 5:56 PM, Andre Przywara wrote:
On 8/19/21 12:19 PM, Michal Simek wrote:
Hi,
Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT image placed and aligned only by 32bits (4bytes). For 64bit systems there is 64bit (8bytes) alignment required. That's why make sure that fit-dtb.blob and u-boot.itb as our primary target images for Xilinx ZynqMP are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to identify 64bit systems (including 32bit systems with PAE).
Signed-off-by: Michal Simek michal.simek@xilinx.com
Makefile | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/Makefile b/Makefile index 269e353a28ad..1bbe95595efe 100644 --- a/Makefile +++ b/Makefile @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) -T firmware -C none -O u-boot \ -a 0 -e 0 -E \ $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) -d /dev/null +ifeq ($(CONFIG_PHYS_64BIT),y)
Why is this restricted to 64-bit "systems"? The DT spec[1] clearly states that some DT parts (/memreserved/ block, for instance), must be 64-bit aligned, which means the whole blobs needs to be 64-bit aligned. Granted this probably does not cause real issues on 32-bit systems, but is violating the spec anyway. So I'd say we add the alignment requirement unconditionally.
That's even better for me and we need to make sure that dtbs itself are aligned and also dtbs inside FIT image are aligned too.
Right, all dtbs need to be 8 byte aligned to start with. Enforcing this is what will unblock moving to a newer libfdt where that's checked for up-front now.
As is in the second thread. Does it make sense to also align the end? I did that in 8/10 patch. The problem with these alignments is that you also need to align the start of FIT image. Maybe would the best to copy fdt to different aligned location.
Thanks, Michal
On Thu, Aug 19, 2021 at 06:31:25PM +0200, Michal Simek wrote:
On 8/19/21 6:18 PM, Tom Rini wrote:
On Thu, Aug 19, 2021 at 06:01:39PM +0200, Michal Simek wrote:
Hi Andre,
On 8/19/21 5:56 PM, Andre Przywara wrote:
On 8/19/21 12:19 PM, Michal Simek wrote:
Hi,
Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT image placed and aligned only by 32bits (4bytes). For 64bit systems there is 64bit (8bytes) alignment required. That's why make sure that fit-dtb.blob and u-boot.itb as our primary target images for Xilinx ZynqMP are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to identify 64bit systems (including 32bit systems with PAE).
Signed-off-by: Michal Simek michal.simek@xilinx.com
Makefile | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/Makefile b/Makefile index 269e353a28ad..1bbe95595efe 100644 --- a/Makefile +++ b/Makefile @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) -T firmware -C none -O u-boot \ -a 0 -e 0 -E \ $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) -d /dev/null +ifeq ($(CONFIG_PHYS_64BIT),y)
Why is this restricted to 64-bit "systems"? The DT spec[1] clearly states that some DT parts (/memreserved/ block, for instance), must be 64-bit aligned, which means the whole blobs needs to be 64-bit aligned. Granted this probably does not cause real issues on 32-bit systems, but is violating the spec anyway. So I'd say we add the alignment requirement unconditionally.
That's even better for me and we need to make sure that dtbs itself are aligned and also dtbs inside FIT image are aligned too.
Right, all dtbs need to be 8 byte aligned to start with. Enforcing this is what will unblock moving to a newer libfdt where that's checked for up-front now.
As is in the second thread. Does it make sense to also align the end? I did that in 8/10 patch. The problem with these alignments is that you also need to align the start of FIT image. Maybe would the best to copy fdt to different aligned location.
Right, so a good question. I have suggested before that we stop assuming that we (U-Boot SPL, etc) can use the device tree in-place and instead move it somewhere aligned. I'm not sure off-hand what ends up being best, someone would need to investigate a bit.
DTBs for 64bit systems should be also 64bit aligned.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
arch/arm/dts/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 537c96bf5b35..8d4fc333ea7a 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -1,5 +1,9 @@ # SPDX-License-Identifier: GPL-2.0+
+ifdef CONFIG_PHYS_64BIT +DTC_FLAGS += -a 0x8 +endif + dtb-$(CONFIG_TARGET_SMARTWEB) += at91sam9260-smartweb.dtb dtb-$(CONFIG_TARGET_TAURUS) += at91sam9g20-taurus.dtb dtb-$(CONFIG_TARGET_CORVUS) += at91sam9g45-corvus.dtb
On 8/19/21 12:19 PM, Michal Simek wrote:
Hi Michal,
DTBs for 64bit systems should be also 64bit aligned.
What does "align" mean here, exactly? This is about generating .dtb *files*, right? dtc makes sure that the internal structures are properly aligned, so what else should be aligned here?
Signed-off-by: Michal Simek michal.simek@xilinx.com
arch/arm/dts/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 537c96bf5b35..8d4fc333ea7a 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -1,5 +1,9 @@ # SPDX-License-Identifier: GPL-2.0+
+ifdef CONFIG_PHYS_64BIT +DTC_FLAGS += -a 0x8
By looking into the dtc source this looks like to make sure the *size* of the DTB is 8-byte aligned, is that the intention here, or even useful? If it is, it should apply unconditionally, not only to 64-bit systems.
Cheers, Andre
+endif
- dtb-$(CONFIG_TARGET_SMARTWEB) += at91sam9260-smartweb.dtb dtb-$(CONFIG_TARGET_TAURUS) += at91sam9g20-taurus.dtb dtb-$(CONFIG_TARGET_CORVUS) += at91sam9g45-corvus.dtb
Hi Andre,
On 8/19/21 6:10 PM, Andre Przywara wrote:
On 8/19/21 12:19 PM, Michal Simek wrote:
Hi Michal,
DTBs for 64bit systems should be also 64bit aligned.
What does "align" mean here, exactly? This is about generating .dtb *files*, right? dtc makes sure that the internal structures are properly aligned, so what else should be aligned here?
Signed-off-by: Michal Simek michal.simek@xilinx.com
arch/arm/dts/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 537c96bf5b35..8d4fc333ea7a 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -1,5 +1,9 @@ # SPDX-License-Identifier: GPL-2.0+ +ifdef CONFIG_PHYS_64BIT +DTC_FLAGS += -a 0x8
By looking into the dtc source this looks like to make sure the *size* of the DTB is 8-byte aligned, is that the intention here, or even useful? If it is, it should apply unconditionally, not only to 64-bit systems.
The question is correct. I did it mostly for being safe that DTBs start and end is 64bit aligned. I didn't hit any issue with not aligned end and maybe it is not useful to do it. It was just more convenient for me to work with also 64bit aligned image size.
Thanks, Michal
When MULTI_DTB_FIT is enabled fit-dtb.blob fit image is created which contain all DTBs listed by CONFIG_OF_LIST. And with DTB_RELESELECT there is a need to handle it as one file with DTBs in it not as separate DTBs in u-boot.its/itb. That's why extend mkimage_fit_atf.sh to generate u-boot.itb correctly.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
arch/arm/mach-zynqmp/mkimage_fit_atf.sh | 47 +++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
diff --git a/arch/arm/mach-zynqmp/mkimage_fit_atf.sh b/arch/arm/mach-zynqmp/mkimage_fit_atf.sh index 592be7f67066..37106909f1ee 100755 --- a/arch/arm/mach-zynqmp/mkimage_fit_atf.sh +++ b/arch/arm/mach-zynqmp/mkimage_fit_atf.sh @@ -111,6 +111,51 @@ cat << __TEE __TEE fi
+MULTI_DTB=`awk '/CONFIG_MULTI_DTB_FIT / { print $3 }' include/generated/autoconf.h` + +if [ $MULTI_DTB -eq 1 ]; then + cat << __FDT_IMAGE_EOF + fdt_1 { + description = "Multi DTB fit image"; + data = /incbin/("fit-dtb.blob"); + type = "flat_dt"; + arch = "arm64"; + compression = "none"; + $DTB_LOAD + hash { + algo = "md5"; + }; + }; + }; + configurations { + default = "config_1"; +__FDT_IMAGE_EOF + +if [ ! -f $BL31 ]; then +cat << __CONF_SECTION1_EOF + config_1 { + description = "Multi DTB without TF-A"; + firmware = "uboot"; + loadables = "fdt_1"; + }; +__CONF_SECTION1_EOF +else +cat << __CONF_SECTION1_EOF + config_1 { + description = "Multi DTB with TF-A"; + firmware = "atf"; + loadables = "uboot", "fdt_1"; + }; +__CONF_SECTION1_EOF +fi + +cat << __ITS_EOF + }; +}; +__ITS_EOF + +else + DEFAULT=1 cnt=1 for dtname in $DT @@ -181,3 +226,5 @@ cat << __ITS_EOF }; }; __ITS_EOF + +fi
U-Boot support board detection at run time and based on it change DT. This feature is implemented for SOM Kria platforms which contain two eeproms which contain information about SOM module and CC (Carrier card). Full U-Boot starts with minimal DT file defined by CONFIG_DEFAULT_DEVICE_TREE which is available in multi DTB fit image. It is using default setup of board_name variable initializaed to DEVICE_TREE which corresponds to CONFIG_DEFAULT_DEVICE_TREE option.
When DTB_RESELECT is enabled board_detection() is called. Keep it your mind that this code is called before relocation. board_detection() is calling xilinx_read_eeprom() which fills board_info (xilinx_board_description) structure which are parsed in board_name_decode(). Based on DT configuration and amount of nvmemX aliases name of the board is composed by concatenating CONFIG_SYS_BOARD "-" <board_name> "-rev" <board_revision> "-" <cc_name> "-rev" <cc_revision>.
If CC is not present or more are available it keeps going.
When board name is composed and returned from board_name_decode() it is assigned to board_name variable which is used by board_fit_config_name_match() which is called via fdtdec_setup() when it goes over config options in multi dtb FIT image.
From practical point of view multi DTB image is key point here which has to
contain configs for detected combinations. Unfortunately as of now they have to be full DTBs and DTBOs are not supported.
That's why configuration like: config_X { description = "zynqmp-board-cc"; fdt = "board", "cc"; };
needs to be squashed together with: fdtoverlay -o zynqmp-board-cc -i arch/arm/dts/zynqmp-board.dtb \ arch/arm/dts/zynqmp-cc.dtbo
and only one dtb is in fit: config_X { description = "zynqmp-board-cc"; fdt = "board-cc"; };
For creating multi DTBs fit image use mkimage -E, e.g.: mkimage -E -f all.its all.dtb
When DTB_RESELECT is enabled xilinx_read_eeprom() is called before relocation and it uses calloc for getting a buffer. Because this is dynamic memory it is not relocated that's why xilinx_read_eeprom() is called again as the part of board_init(). This second read with calloc buffer placed in proper position board_late_init_xilinx() can setup u-boot variables as before.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
arch/arm/dts/zynqmp-sm-k26-revA.dts | 3 ++ board/xilinx/common/board.c | 84 ++++++++++++++++++++++++++--- 2 files changed, 81 insertions(+), 6 deletions(-)
diff --git a/arch/arm/dts/zynqmp-sm-k26-revA.dts b/arch/arm/dts/zynqmp-sm-k26-revA.dts index e274100a9bc5..5f55df28f331 100644 --- a/arch/arm/dts/zynqmp-sm-k26-revA.dts +++ b/arch/arm/dts/zynqmp-sm-k26-revA.dts @@ -204,17 +204,20 @@
&i2c1 { status = "okay"; + u-boot,dm-pre-reloc; clock-frequency = <400000>; scl-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>; sda-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
eeprom: eeprom@50 { /* u46 - also at address 0x58 */ + u-boot,dm-pre-reloc; compatible = "st,24c64", "atmel,24c64"; /* st m24c64 */ reg = <0x50>; /* WP pin EE_WP_EN connected to slg7x644092@68 */ };
eeprom_cc: eeprom@51 { /* required by spec - also at address 0x59 */ + u-boot,dm-pre-reloc; compatible = "st,24c64", "atmel,24c64"; /* st m24c64 */ reg = <0x51>; }; diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c index 92874272893a..979df44306af 100644 --- a/board/xilinx/common/board.c +++ b/board/xilinx/common/board.c @@ -19,6 +19,7 @@ #include <net.h> #include <generated/dt.h> #include <soc.h> +#include <linux/ctype.h>
#include "fru.h"
@@ -188,12 +189,14 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char *name, goto end; }
- printf("Xilinx I2C FRU format at %s:\n", name); - fru_capture((unsigned long)fru_content); - ret = fru_display(0); - if (ret) { - printf("FRU format decoding failed.\n"); - goto end; + if (gd->flags & GD_FLG_RELOC || (_DEBUG && CONFIG_IS_ENABLED(DTB_RESELECT))) { + printf("Xilinx I2C FRU format at %s:\n", name); + fru_capture((unsigned long)fru_content); + ret = fru_display(0); + if (ret) { + printf("FRU format decoding failed.\n"); + goto end; + } }
if (desc->header == EEPROM_HEADER_MAGIC) { @@ -465,13 +468,82 @@ int print_cpuinfo(void) #endif
#if CONFIG_IS_ENABLED(DTB_RESELECT) +#define MAX_NAME_LENGTH 50 + char * __maybe_unused __weak board_name_decode(void) { + char *board_local_name; + struct xilinx_board_description *desc; + int i, id; + + board_local_name = calloc(1, MAX_NAME_LENGTH); + if (!board_info) + return NULL; + + for (id = 0; id <= highest_id; id++) { + desc = &board_info[id]; + + /* No board description */ + if (!desc) + goto error; + + /* Board is not detected */ + if (desc->header != EEPROM_HEADER_MAGIC) + continue; + + /* The first string should be soc name */ + if (!id) + strcat(board_local_name, CONFIG_SYS_BOARD); + + /* + * For two purpose here: + * soc_name- eg: zynqmp- + * and between base board and CC eg: ..revA-sck... + */ + strcat(board_local_name, "-"); + + if (desc->name[0]) { + /* For DT composition name needs to be lowercase */ + for (i = 0; i < sizeof(desc->name); i++) + desc->name[i] = tolower(desc->name[i]); + + strcat(board_local_name, desc->name); + } + if (desc->revision[0]) { + strcat(board_local_name, "-rev"); + + /* And revision needs to be uppercase */ + for (i = 0; i < sizeof(desc->revision); i++) + desc->revision[i] = toupper(desc->revision[i]); + + strcat(board_local_name, desc->revision); + } + } + + /* + * Longer strings will end up with buffer overflow and potential + * attacks that's why check it + */ + if (strlen(board_local_name) >= MAX_NAME_LENGTH) + panic("Board name can't be determined\n"); + + if (strlen(board_local_name)) + return board_local_name; + +error: + free(board_local_name); return NULL; }
bool __maybe_unused __weak board_detection(void) { + if (CONFIG_IS_ENABLED(DM_I2C) && CONFIG_IS_ENABLED(I2C_EEPROM)) { + int ret; + + ret = xilinx_read_eeprom(); + return !ret ? true : false; + } + return false; }
participants (3)
-
Andre Przywara -
Michal Simek -
Tom Rini