[PATCH 00/10] xilinx: Add support for DTB reselection

[PATCH 00/10] xilinx: Add support for DTB reselection

[Michal Simek]

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(-)

-- 
2.32.0

[PATCH 01/10] xilinx: fru: Replace spaces with \0 in detected name

[Michal Simek]

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,
-- 
2.32.0

[PATCH 02/10] xilinx: Use variable for passing board_name

[Michal Simek]

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;
-- 
2.32.0

[PATCH 03/10] xilinx: common: Change board_info[] handling

[Michal Simek]

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,
-- 
2.32.0

[PATCH 04/10] xilinx: common: Free allocated structure

[Michal Simek]

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)
-- 
2.32.0

[PATCH 05/10] xilinx: Add support for generic board detection

[Michal Simek]

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
-- 
2.32.0

[PATCH 06/10] xilinx: zynqmp: Check that DT is 64bit aligned

[Michal Simek]

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);
-- 
2.32.0

[PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems

[Michal Simek]

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 \
-- 
2.32.0

[PATCH 08/10] arm64: dts: Make sure that all DTBs are 64bit aligned for 64bit systems

[Michal Simek]

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
-- 
2.32.0

[PATCH 09/10] xilinx: zynqmp: Generate different u-boot.itb for MULTI_DTB_FIT

[Michal Simek]

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
-- 
2.32.0

[PATCH 10/10] xilinx: common: Enabling generic function for DT reselection

[Michal Simek]

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;
 }
 
-- 
2.32.0

Re: [PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems

[Andre Przywara]

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/v0.3/devicetree-specification-v0.3.pdf

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

Re: [PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems

[Michal Simek]

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

Re: [PATCH 08/10] arm64: dts: Make sure that all DTBs are 64bit aligned for 64bit systems

[Andre Przywara]

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
> 

Re: [PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems

[Michal Simek]

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

Re: [PATCH 08/10] arm64: dts: Make sure that all DTBs are 64bit aligned for 64bit systems

[Michal Simek]

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

Re: [PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2684 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 00/10] xilinx: Add support for DTB reselection

[Michal Simek]

po 23. 8. 2021 v 9:37 odesílatel Michal Simek <michal.simek@xilinx.com> napsal:
>
> 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
>
> Changes in v2:
> - Update subject and commit message
> - Enable 64bit alignment for all - reported by Andre
> - Update subject and commit message
> - Enable it for all (not just 64bit systems)
> - Update if condition for MULTI_DTB
> - call fru_capture in non DEBUG mode
> - Show detected name as standard message to see what has been chosen
>
> 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
>   arm64: dts: Make sure that all DTBs are 64bit aligned
>   xilinx: zynqmp: Generate different u-boot.itb for MULTI_DTB_FIT
>   xilinx: common: Enabling generic function for DT reselection
>
>  Makefile                                |   3 +
>  arch/arm/dts/Makefile                   |   2 +-
>  arch/arm/dts/zynqmp-sm-k26-revA.dts     |   3 +
>  arch/arm/mach-zynqmp/mkimage_fit_atf.sh |  47 +++++++
>  board/xilinx/common/board.c             | 158 +++++++++++++++++++-----
>  board/xilinx/zynqmp/zynqmp.c            |   3 +
>  6 files changed, 186 insertions(+), 30 deletions(-)
>
> --
> 2.33.0
>

This should have v2 in subject but it has changes in v2.
Simon: Can we extend patman to capture this configuration where you
have changes for newer revision then revision of the whole series?

Anyway applied to my tree.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs

[PATCH 00/10] xilinx: Add support for DTB reselection

[Michal Simek]

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

Changes in v2:
- Update subject and commit message
- Enable 64bit alignment for all - reported by Andre
- Update subject and commit message
- Enable it for all (not just 64bit systems)
- Update if condition for MULTI_DTB
- call fru_capture in non DEBUG mode
- Show detected name as standard message to see what has been chosen

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
  arm64: dts: Make sure that all DTBs are 64bit aligned
  xilinx: zynqmp: Generate different u-boot.itb for MULTI_DTB_FIT
  xilinx: common: Enabling generic function for DT reselection

 Makefile                                |   3 +
 arch/arm/dts/Makefile                   |   2 +-
 arch/arm/dts/zynqmp-sm-k26-revA.dts     |   3 +
 arch/arm/mach-zynqmp/mkimage_fit_atf.sh |  47 +++++++
 board/xilinx/common/board.c             | 158 +++++++++++++++++++-----
 board/xilinx/zynqmp/zynqmp.c            |   3 +
 6 files changed, 186 insertions(+), 30 deletions(-)

-- 
2.33.0