[PATCH 0/2] dm: core: Add support for filling script_offset_f/script_size_f from DT

[PATCH 0/2] dm: core: Add support for filling script_offset_f/script_size_f from DT

[Michal Simek]

Hi,

The commit https://github.com/devicetree-org/dt-schema/pull/105 added
support for reading memory location where bootscript should be loaded.
This is follow up series on reading information from DT where boot script
is placed in flash memory.

Patches are based on
https://lore.kernel.org/r/b37aec1460d93727a9065766aa5f50fa356ae5aa.1690873880.git.michal.simek@amd.com
and
https://lore.kernel.org/r/b77352af92102dfad2e12a49a7680f5f64f8ee31.1690873880.git.michal.simek@amd.com

I didn't send pull request to dt-schema yet because I want to check logic
around it and DT variable names bootscr-flash-offset and bootscr-flash-size
are correct.

Also there should be follow up patch to remove CONFIG_BOOT_SCRIPT_OFFSET
Kconfig symbol and remove variable script_size_f setup from config file.

Also keep in mind that this is pretty much just 1:1 conversion between DT
and variable. Upstream code is using it for QSPI only and we are also using
in for NAND devices too. But the same approach could be used for DISKs,
EEPROMs or SD RAW. That's why I pretty much think that DT description
should be done a little bit differently.
For example:
bootscr-location = <&nvmem offset size>;

Thanks,
Michal


Michal Simek (2):
  dm: core: ofnode: Add ofnode_read_bootscript_flash()
  xilinx: board: Add support to pick bootscr flash offset/size from DT

 arch/sandbox/dts/test.dts   |  2 ++
 board/xilinx/common/board.c | 13 +++++++++++--
 drivers/core/ofnode.c       | 34 ++++++++++++++++++++++++++++++++++
 include/dm/ofnode.h         | 27 +++++++++++++++++++++++++++
 test/dm/ofnode.c            |  9 +++++++--
 5 files changed, 81 insertions(+), 4 deletions(-)

-- 
2.36.1

[PATCH 1/2] dm: core: ofnode: Add ofnode_read_bootscript_flash()

[Michal Simek]

ofnode_read_bootscript_flash() reads bootscript address from
/options/u-boot DT node. bootscr-flash-offset and bootscr-flash-size
properties are read and values are filled. When bootscr-flash-size is not
defined, bootscr-flash-offset property is unusable that's why cleaned.
Both of these properties should be defined to function properly.

Also add test to cover this new function.

Signed-off-by: Michal Simek <michal.simek@amd.com>
---

 arch/sandbox/dts/test.dts |  2 ++
 drivers/core/ofnode.c     | 34 ++++++++++++++++++++++++++++++++++
 include/dm/ofnode.h       | 27 +++++++++++++++++++++++++++
 test/dm/ofnode.c          |  9 +++++++--
 4 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 94b00b80429b..e308ee06c4d9 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -83,6 +83,8 @@
 		u-boot {
 			compatible = "u-boot,config";
 			bootscr-ram-offset = /bits/ 64 <0x12345678>;
+			bootscr-flash-offset = /bits/ 64 <0>;
+			bootscr-flash-size = /bits/ 64 <0x2000>;
 		};
 	};
 
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index d94f0300c30a..023aad06ddc3 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -1588,6 +1588,40 @@ int ofnode_read_bootscript_address(u64 *bootscr_address, u64 *bootscr_offset)
 	return 0;
 }
 
+int ofnode_read_bootscript_flash(u64 *bootscr_flash_offset,
+				 u64 *bootscr_flash_size)
+{
+	int ret;
+	ofnode uboot;
+
+	*bootscr_flash_offset = 0;
+	*bootscr_flash_size = 0;
+
+	uboot = ofnode_path("/options/u-boot");
+	if (!ofnode_valid(uboot)) {
+		printf("%s: Missing /u-boot node\n", __func__);
+		return -EINVAL;
+	}
+
+	ret = ofnode_read_u64(uboot, "bootscr-flash-offset",
+			      bootscr_flash_offset);
+	if (ret)
+		return -EINVAL;
+
+	ret = ofnode_read_u64(uboot, "bootscr-flash-size",
+			      bootscr_flash_size);
+	if (ret)
+		return -EINVAL;
+
+	if (!bootscr_flash_size) {
+		debug("bootscr-flash-size is zero. Ignoring properties!\n");
+		*bootscr_flash_offset = 0;
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 ofnode ofnode_get_phy_node(ofnode node)
 {
 	/* DT node properties that reference a PHY node */
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index a297236112ed..4500ba6acdd1 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -1520,6 +1520,27 @@ const char *ofnode_conf_read_str(const char *prop_name);
  */
 int ofnode_read_bootscript_address(u64 *bootscr_address, u64 *bootscr_offset);
 
+/**
+ * ofnode_read_bootscript_flash() - Read bootscr-flash-offset/size
+ *
+ * @bootscr_flash_offset: pointer to 64bit offset where bootscr-flash-offset
+ * property value is stored
+ * @bootscr_flash_size: pointer to 64bit size where bootscr-flash-size property
+ * value is stored
+ *
+ * This reads a bootscr-flash-offset and bootscr-flash-size properties from
+ * the /options/u-boot/ node of the devicetree. bootscr-flash-offset holds
+ * the offset of the boot script file from start of flash. bootscr-flash-size
+ * holds the boot script size in flash. When bootscr-flash-size is not defined,
+ * bootscr-flash-offset property is cleaned.
+ *
+ * This only works with the control FDT.
+ *
+ * Return: 0 if OK, -EINVAL if property is not found or incorrect.
+ */
+int ofnode_read_bootscript_flash(u64 *bootscr_flash_offset,
+				 u64 *bootscr_flash_size);
+
 #else /* CONFIG_DM */
 static inline bool ofnode_conf_read_bool(const char *prop_name)
 {
@@ -1541,6 +1562,12 @@ int ofnode_read_bootscript_address(u64 *bootscr_address, u64 *bootscr_offset)
 	return -EINVAL;
 }
 
+int ofnode_read_bootscript_flash(u64 *bootscr_flash_offset,
+				 u64 *bootscr_flash_size)
+{
+	return -EINVAL;
+}
+
 #endif /* CONFIG_DM */
 
 /**
diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c
index a4e04f23784f..d84b738ed7c4 100644
--- a/test/dm/ofnode.c
+++ b/test/dm/ofnode.c
@@ -585,14 +585,19 @@ DM_TEST(dm_test_ofnode_conf, 0);
 
 static int dm_test_ofnode_options(struct unit_test_state *uts)
 {
-	u64 bootscr_address;
-	u64 bootscr_offset;
+	u64 bootscr_address, bootscr_offset;
+	u64 bootscr_flash_offset, bootscr_flash_size;
 
 	ut_assertok(ofnode_read_bootscript_address(&bootscr_address,
 						   &bootscr_offset));
 	ut_asserteq_64(0, bootscr_address);
 	ut_asserteq_64(0x12345678, bootscr_offset);
 
+	ut_assertok(ofnode_read_bootscript_flash(&bootscr_flash_offset,
+						 &bootscr_flash_size));
+	ut_asserteq_64(0, bootscr_flash_offset);
+	ut_asserteq_64(0x2000, bootscr_flash_size);
+
 	return 0;
 }
 DM_TEST(dm_test_ofnode_options, 0);
-- 
2.36.1

[PATCH 2/2] xilinx: board: Add support to pick bootscr flash offset/size from DT

[Michal Simek]

Location of bootscript in flash can be specified via /options/u-boot DT
node by using bootscr-flash-offset and bootscr-flash-size properties.
Values should be saved to script_offset_f and script_size_f variables.
Variables are described in doc/develop/bootstd.rst as:
script_offset_f
    SPI flash offset from which to load the U-Boot script, e.g. 0xffe000

script_size_f
    Size of the script to load, e.g. 0x2000

Both of them are used by sf_get_bootflow() in drivers/mtd/spi/sf_bootdev.c
to identify bootscript location inside flash.

Signed-off-by: Michal Simek <michal.simek@amd.com>
---

 board/xilinx/common/board.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 61dd998198e9..5e208a2250f6 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -411,6 +411,7 @@ int board_late_init_xilinx(void)
 	int i, id, macid = 0;
 	struct xilinx_board_description *desc;
 	phys_size_t bootm_size = gd->ram_top - gd->ram_base;
+	u64 bootscr_flash_offset, bootscr_flash_size;
 
 	if (!IS_ENABLED(CONFIG_MICROBLAZE)) {
 		ulong scriptaddr;
@@ -435,11 +436,19 @@ int board_late_init_xilinx(void)
 		}
 	}
 
+	if (!ofnode_read_bootscript_flash(&bootscr_flash_offset,
+					  &bootscr_flash_size)) {
+		ret |= env_set_hex("script_offset_f", bootscr_flash_offset);
+		ret |= env_set_hex("script_size_f", bootscr_flash_size);
+	} else {
+		printf("!!! Please define bootscr-flash-offset via DT !!!\n");
+		ret |= env_set_hex("script_offset_f",
+				   CONFIG_BOOT_SCRIPT_OFFSET);
+	}
+
 	if (IS_ENABLED(CONFIG_ARCH_ZYNQ) || IS_ENABLED(CONFIG_MICROBLAZE))
 		bootm_size = min(bootm_size, (phys_size_t)(SZ_512M + SZ_256M));
 
-	ret |= env_set_hex("script_offset_f", CONFIG_BOOT_SCRIPT_OFFSET);
-
 	ret |= env_set_addr("bootm_low", (void *)gd->ram_base);
 	ret |= env_set_addr("bootm_size", (void *)bootm_size);
 
-- 
2.36.1

Re: [PATCH 1/2] dm: core: ofnode: Add ofnode_read_bootscript_flash()

[Simon Glass]

On Thu, 3 Aug 2023 at 07:36, Michal Simek <michal.simek@amd.com> wrote:
>
> ofnode_read_bootscript_flash() reads bootscript address from
> /options/u-boot DT node. bootscr-flash-offset and bootscr-flash-size
> properties are read and values are filled. When bootscr-flash-size is not
> defined, bootscr-flash-offset property is unusable that's why cleaned.
> Both of these properties should be defined to function properly.
>
> Also add test to cover this new function.
>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
>
>  arch/sandbox/dts/test.dts |  2 ++
>  drivers/core/ofnode.c     | 34 ++++++++++++++++++++++++++++++++++
>  include/dm/ofnode.h       | 27 +++++++++++++++++++++++++++
>  test/dm/ofnode.c          |  9 +++++++--
>  4 files changed, 70 insertions(+), 2 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 0/2] dm: core: Add support for filling script_offset_f/script_size_f from DT

[Michal Simek]

On 8/3/23 15:36, Michal Simek wrote:
> Hi,
> 
> The commit https://github.com/devicetree-org/dt-schema/pull/105 added
> support for reading memory location where bootscript should be loaded.
> This is follow up series on reading information from DT where boot script
> is placed in flash memory.
> 
> Patches are based on
> https://lore.kernel.org/r/b37aec1460d93727a9065766aa5f50fa356ae5aa.1690873880.git.michal.simek@amd.com
> and
> https://lore.kernel.org/r/b77352af92102dfad2e12a49a7680f5f64f8ee31.1690873880.git.michal.simek@amd.com
> 
> I didn't send pull request to dt-schema yet because I want to check logic
> around it and DT variable names bootscr-flash-offset and bootscr-flash-size
> are correct.
> 
> Also there should be follow up patch to remove CONFIG_BOOT_SCRIPT_OFFSET
> Kconfig symbol and remove variable script_size_f setup from config file.
> 
> Also keep in mind that this is pretty much just 1:1 conversion between DT
> and variable. Upstream code is using it for QSPI only and we are also using
> in for NAND devices too. But the same approach could be used for DISKs,
> EEPROMs or SD RAW. That's why I pretty much think that DT description
> should be done a little bit differently.
> For example:
> bootscr-location = <&nvmem offset size>;
> 
> Thanks,
> Michal
> 
> 
> Michal Simek (2):
>    dm: core: ofnode: Add ofnode_read_bootscript_flash()
>    xilinx: board: Add support to pick bootscr flash offset/size from DT
> 
>   arch/sandbox/dts/test.dts   |  2 ++
>   board/xilinx/common/board.c | 13 +++++++++++--
>   drivers/core/ofnode.c       | 34 ++++++++++++++++++++++++++++++++++
>   include/dm/ofnode.h         | 27 +++++++++++++++++++++++++++
>   test/dm/ofnode.c            |  9 +++++++--
>   5 files changed, 81 insertions(+), 4 deletions(-)
> 

Applied.
M