[PATCH v2 0/3] rockchip: spl: Cache boot source id for later use

[PATCH v2 0/3] rockchip: spl: Cache boot source id for later use

[Jonas Karlman]

This series fixes an issue where /chosen/u-boot,spl-boot-device prop
cannot be resolved to the SPI flash node path in spl_perform_fixups() if
the affected device was booted from SPI flash.

Changes in v2:
- Add patch that sync boot source id enum with bootrom
- Add patch that adds missing SPI flash node path for SoCs with DTs
  having a jedec,spi-nor compatible flash@0 node.
- Only cache boot source id that can be resolved to a node path
- Make use of synced BROM_BOOTSOURCE_UNKNOWN enum value
- Update commit message
- Add fixes tag
- Collect r-b tag

Jason Zhu (1):
  rockchip: bootrom: Sync bootsource id enum from bootrom

Jonas Karlman (2):
  rockchip: spl: Cache boot source id for later use
  rockchip: spl: Add SPI NOR flash to boot_devices array

 arch/arm/include/asm/arch-rockchip/bootrom.h |  3 +++
 arch/arm/mach-rockchip/px30/px30.c           |  1 +
 arch/arm/mach-rockchip/rk3288/rk3288.c       |  1 +
 arch/arm/mach-rockchip/rk3308/rk3308.c       |  1 +
 arch/arm/mach-rockchip/rk3368/rk3368.c       |  1 +
 arch/arm/mach-rockchip/rv1126/rv1126.c       |  1 +
 arch/arm/mach-rockchip/spl.c                 | 14 +++++++++++---
 7 files changed, 19 insertions(+), 3 deletions(-)

-- 
2.43.2

[PATCH v2 1/3] rockchip: bootrom: Sync bootsource id enum from bootrom

[Jonas Karlman]

From: Jason Zhu <jason.zhu@rock-chips.com>

Add more bootsource id:
1. BROM_BOOTSOURCE_UNKNOWN
2. BROM_BOOTSOURCE_I2C
3. BROM_BOOTSOURCE_SPI

Signed-off-by: Jason Zhu <jason.zhu@rock-chips.com>
[jonas@kwiboo.se: Update commit message]
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- New patch
---
 arch/arm/include/asm/arch-rockchip/bootrom.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/include/asm/arch-rockchip/bootrom.h b/arch/arm/include/asm/arch-rockchip/bootrom.h
index 4276a0f6811a..ecf3b4e7428c 100644
--- a/arch/arm/include/asm/arch-rockchip/bootrom.h
+++ b/arch/arm/include/asm/arch-rockchip/bootrom.h
@@ -43,11 +43,14 @@ void back_to_bootrom(enum rockchip_bootrom_cmd brom_cmd);
  * Boot-device identifiers as used by the BROM
  */
 enum {
+	BROM_BOOTSOURCE_UNKNOWN = 0,
 	BROM_BOOTSOURCE_NAND = 1,
 	BROM_BOOTSOURCE_EMMC = 2,
 	BROM_BOOTSOURCE_SPINOR = 3,
 	BROM_BOOTSOURCE_SPINAND = 4,
 	BROM_BOOTSOURCE_SD = 5,
+	BROM_BOOTSOURCE_I2C = 8,
+	BROM_BOOTSOURCE_SPI = 9,
 	BROM_BOOTSOURCE_USB = 10,
 	BROM_LAST_BOOTSOURCE = BROM_BOOTSOURCE_USB
 };
-- 
2.43.2

[PATCH v2 2/3] rockchip: spl: Cache boot source id for later use

[Jonas Karlman]

Rockchip BROM writes a boot source id at CFG_IRAM_BASE + 0x10, this id
indicates from what storage media TPL/SPL was loaded from.

SPL uses this id to determine what device "same-as-spl" represent when
determining from where FIT should be loaded. This works as long as the
boot_devices array contain a matching id <-> node path entry.

However, SPL typically load a small part of TF-A into SRAM and on RK3399
this overwrites the CFG_IRAM_BASE + 0x10 addr used for boot source id.

For affected devices the u-boot,spl-boot-device would not be set when
booting from SPI flash and the flash@0 node was not explicitly listed
in the u-boot,spl-boot-order prop.

Here boot source id is 3 before FIT images is loaded, and 0 after:

  U-Boot SPL 2024.04-rc4 (Mar 15 2024 - 17:26:19 +0000)
  board_spl_was_booted_from: brom_bootdevice_id 3 maps to '/spi@ff1d0000/flash@0'
  Trying to boot from SPI
  ## Checking hash(es) for config config-1 ... OK
  ## Checking hash(es) for Image atf-1 ... sha256+ OK
  ## Checking hash(es) for Image u-boot ... sha256+ OK
  ## Checking hash(es) for Image fdt-1 ... sha256+ OK
  ## Checking hash(es) for Image atf-2 ... sha256+ OK
  ## Checking hash(es) for Image atf-3 ... sha256+ OK
  board_spl_was_booted_from: failed to resolve brom_bootdevice_id 0
  spl_decode_boot_device: could not find udevice for /mmc@fe330000
  spl_decode_boot_device: could not find udevice for /mmc@fe320000
  spl_perform_fixups: could not map boot_device to ofpath: -19

Use a static brom_bootsource_id_cache to save the boot source id after
an initial read from SRAM to fix this, this allow spl_perform_fixups()
to resolve correct boot source path for "same-as-spl" after SPL have
loaded TF-A related FIT images into memory.

With this the spl-boot-device prop can correctly be resolved to the
SPI flash node in the control FDT:

  => fdt addr ${fdtcontroladdr}
  Working FDT set to f1ee6710
  => fdt list /chosen
  chosen {
      u-boot,spl-boot-device = "/spi@ff1d0000/flash@0";
      stdout-path = "serial2:1500000n8";
      u-boot,spl-boot-order = "same-as-spl", "/mmc@fe330000", "/mmc@fe320000";
  };

Fixes: d57e16c7e712 ("rockchip: find U-boot proper boot device by inverting the logic that sets it")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
v2:
- Only cache boot source id that can be resolved to a node path
- Make use of the new BROM_BOOTSOURCE_UNKNOWN enum value
- Update commit message
- Add fixes tag
- Collect r-b tag
---
 arch/arm/mach-rockchip/spl.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-rockchip/spl.c b/arch/arm/mach-rockchip/spl.c
index 1586a093fc37..3ce7e792b5a2 100644
--- a/arch/arm/mach-rockchip/spl.c
+++ b/arch/arm/mach-rockchip/spl.c
@@ -32,18 +32,26 @@ __weak const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
 
 const char *board_spl_was_booted_from(void)
 {
-	u32  bootdevice_brom_id = readl(BROM_BOOTSOURCE_ID_ADDR);
+	static u32 brom_bootsource_id_cache = BROM_BOOTSOURCE_UNKNOWN;
+	u32 bootdevice_brom_id;
 	const char *bootdevice_ofpath = NULL;
 
+	if (brom_bootsource_id_cache != BROM_BOOTSOURCE_UNKNOWN)
+		bootdevice_brom_id = brom_bootsource_id_cache;
+	else
+		bootdevice_brom_id = readl(BROM_BOOTSOURCE_ID_ADDR);
+
 	if (bootdevice_brom_id < ARRAY_SIZE(boot_devices))
 		bootdevice_ofpath = boot_devices[bootdevice_brom_id];
 
-	if (bootdevice_ofpath)
+	if (bootdevice_ofpath) {
+		brom_bootsource_id_cache = bootdevice_brom_id;
 		debug("%s: brom_bootdevice_id %x maps to '%s'\n",
 		      __func__, bootdevice_brom_id, bootdevice_ofpath);
-	else
+	} else {
 		debug("%s: failed to resolve brom_bootdevice_id %x\n",
 		      __func__, bootdevice_brom_id);
+	}
 
 	return bootdevice_ofpath;
 }
-- 
2.43.2

[PATCH v2 3/3] rockchip: spl: Add SPI NOR flash to boot_devices array

[Jonas Karlman]

Add missing boot source id <-> device tree node path mapping for SPI NOR
flash on PX30, RK3288, RK3308, RK3368 and RV1126.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- New patch
---
 arch/arm/mach-rockchip/px30/px30.c     | 1 +
 arch/arm/mach-rockchip/rk3288/rk3288.c | 1 +
 arch/arm/mach-rockchip/rk3308/rk3308.c | 1 +
 arch/arm/mach-rockchip/rk3368/rk3368.c | 1 +
 arch/arm/mach-rockchip/rv1126/rv1126.c | 1 +
 5 files changed, 5 insertions(+)

diff --git a/arch/arm/mach-rockchip/px30/px30.c b/arch/arm/mach-rockchip/px30/px30.c
index b4f655fa4b3e..2ec3289d75b7 100644
--- a/arch/arm/mach-rockchip/px30/px30.c
+++ b/arch/arm/mach-rockchip/px30/px30.c
@@ -19,6 +19,7 @@
 
 const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
 	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff390000",
+	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff3a0000/flash@0",
 	[BROM_BOOTSOURCE_SD] = "/mmc@ff370000",
 };
 
diff --git a/arch/arm/mach-rockchip/rk3288/rk3288.c b/arch/arm/mach-rockchip/rk3288/rk3288.c
index c77c56c1dab5..70cf50029121 100644
--- a/arch/arm/mach-rockchip/rk3288/rk3288.c
+++ b/arch/arm/mach-rockchip/rk3288/rk3288.c
@@ -28,6 +28,7 @@ DECLARE_GLOBAL_DATA_PTR;
 
 const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
 	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff0f0000",
+	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff130000/flash@0",
 	[BROM_BOOTSOURCE_SD] = "/mmc@ff0c0000",
 };
 
diff --git a/arch/arm/mach-rockchip/rk3308/rk3308.c b/arch/arm/mach-rockchip/rk3308/rk3308.c
index 27a748327e31..b3ffabc5449a 100644
--- a/arch/arm/mach-rockchip/rk3308/rk3308.c
+++ b/arch/arm/mach-rockchip/rk3308/rk3308.c
@@ -141,6 +141,7 @@ enum {
 
 const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
 	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff490000",
+	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff4c0000/flash@0",
 	[BROM_BOOTSOURCE_SD] = "/mmc@ff480000",
 };
 
diff --git a/arch/arm/mach-rockchip/rk3368/rk3368.c b/arch/arm/mach-rockchip/rk3368/rk3368.c
index 651ba1090206..8f5ca1dfa7c9 100644
--- a/arch/arm/mach-rockchip/rk3368/rk3368.c
+++ b/arch/arm/mach-rockchip/rk3368/rk3368.c
@@ -55,6 +55,7 @@ struct mm_region *mem_map = rk3368_mem_map;
 
 const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
 	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff0f0000",
+	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff120000/flash@0",
 	[BROM_BOOTSOURCE_SD] = "/mmc@ff0c0000",
 };
 
diff --git a/arch/arm/mach-rockchip/rv1126/rv1126.c b/arch/arm/mach-rockchip/rv1126/rv1126.c
index 8589c46f10aa..40eb9eb7b196 100644
--- a/arch/arm/mach-rockchip/rv1126/rv1126.c
+++ b/arch/arm/mach-rockchip/rv1126/rv1126.c
@@ -15,6 +15,7 @@
 
 const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
 	[BROM_BOOTSOURCE_EMMC] = "/mmc@ffc50000",
+	[BROM_BOOTSOURCE_SPINOR] = "/spi@ffc90000/flash@0",
 	[BROM_BOOTSOURCE_SD] = "/mmc@ffc60000",
 };
 
-- 
2.43.2

Re: [PATCH v2 1/3] rockchip: bootrom: Sync bootsource id enum from bootrom

[Dragan Simic]

On 2024-03-22 21:50, Jonas Karlman wrote:
> From: Jason Zhu <jason.zhu@rock-chips.com>
> 
> Add more bootsource id:
> 1. BROM_BOOTSOURCE_UNKNOWN
> 2. BROM_BOOTSOURCE_I2C
> 3. BROM_BOOTSOURCE_SPI
> 
> Signed-off-by: Jason Zhu <jason.zhu@rock-chips.com>
> [jonas@kwiboo.se: Update commit message]
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

Looking good to me.

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

> ---
> v2:
> - New patch
> ---
>  arch/arm/include/asm/arch-rockchip/bootrom.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/include/asm/arch-rockchip/bootrom.h
> b/arch/arm/include/asm/arch-rockchip/bootrom.h
> index 4276a0f6811a..ecf3b4e7428c 100644
> --- a/arch/arm/include/asm/arch-rockchip/bootrom.h
> +++ b/arch/arm/include/asm/arch-rockchip/bootrom.h
> @@ -43,11 +43,14 @@ void back_to_bootrom(enum rockchip_bootrom_cmd 
> brom_cmd);
>   * Boot-device identifiers as used by the BROM
>   */
>  enum {
> +	BROM_BOOTSOURCE_UNKNOWN = 0,
>  	BROM_BOOTSOURCE_NAND = 1,
>  	BROM_BOOTSOURCE_EMMC = 2,
>  	BROM_BOOTSOURCE_SPINOR = 3,
>  	BROM_BOOTSOURCE_SPINAND = 4,
>  	BROM_BOOTSOURCE_SD = 5,
> +	BROM_BOOTSOURCE_I2C = 8,
> +	BROM_BOOTSOURCE_SPI = 9,
>  	BROM_BOOTSOURCE_USB = 10,
>  	BROM_LAST_BOOTSOURCE = BROM_BOOTSOURCE_USB
>  };

Re: [PATCH v2 3/3] rockchip: spl: Add SPI NOR flash to boot_devices array

[Dragan Simic]

Hello Jonas,

Please see my comments below.

On 2024-03-22 21:50, Jonas Karlman wrote:
> Add missing boot source id <-> device tree node path mapping for SPI 
> NOR
> flash on PX30, RK3288, RK3308, RK3368 and RV1126.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> v2:
> - New patch
> ---
>  arch/arm/mach-rockchip/px30/px30.c     | 1 +
>  arch/arm/mach-rockchip/rk3288/rk3288.c | 1 +
>  arch/arm/mach-rockchip/rk3308/rk3308.c | 1 +
>  arch/arm/mach-rockchip/rk3368/rk3368.c | 1 +
>  arch/arm/mach-rockchip/rv1126/rv1126.c | 1 +
>  5 files changed, 5 insertions(+)
> 
> diff --git a/arch/arm/mach-rockchip/px30/px30.c
> b/arch/arm/mach-rockchip/px30/px30.c
> index b4f655fa4b3e..2ec3289d75b7 100644
> --- a/arch/arm/mach-rockchip/px30/px30.c
> +++ b/arch/arm/mach-rockchip/px30/px30.c
> @@ -19,6 +19,7 @@
> 
>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff390000",
> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff3a0000/flash@0",

AFAICT, no PX30-based board contains an SPI chip, but making this
future-proof perhaps shouldn't hurt.

>  	[BROM_BOOTSOURCE_SD] = "/mmc@ff370000",
>  };
> 
> diff --git a/arch/arm/mach-rockchip/rk3288/rk3288.c
> b/arch/arm/mach-rockchip/rk3288/rk3288.c
> index c77c56c1dab5..70cf50029121 100644
> --- a/arch/arm/mach-rockchip/rk3288/rk3288.c
> +++ b/arch/arm/mach-rockchip/rk3288/rk3288.c
> @@ -28,6 +28,7 @@ DECLARE_GLOBAL_DATA_PTR;
> 
>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff0f0000",
> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff130000/flash@0",

Using spi2 for a flash chip applies to some RK3288-based boards, but 
AFAICT
not to all of them.  For example, arch/arm/dts/rk3288-evb.dtsi uses spi0
for a flash chip, which in turn translates to /spi@ff110000/flash@0.  
The
same also applies to arch/arm/dts/rk3288-firefly.dtsi, for example.

>  	[BROM_BOOTSOURCE_SD] = "/mmc@ff0c0000",
>  };
> 
> diff --git a/arch/arm/mach-rockchip/rk3308/rk3308.c
> b/arch/arm/mach-rockchip/rk3308/rk3308.c
> index 27a748327e31..b3ffabc5449a 100644
> --- a/arch/arm/mach-rockchip/rk3308/rk3308.c
> +++ b/arch/arm/mach-rockchip/rk3308/rk3308.c
> @@ -141,6 +141,7 @@ enum {
> 
>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff490000",
> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff4c0000/flash@0",

AFAICT, no RK3308-based board contains an SPI chip, and I'm not really
sure where does /spi@ff4c0000/flash@0 come from.

>  	[BROM_BOOTSOURCE_SD] = "/mmc@ff480000",
>  };
> 
> diff --git a/arch/arm/mach-rockchip/rk3368/rk3368.c
> b/arch/arm/mach-rockchip/rk3368/rk3368.c
> index 651ba1090206..8f5ca1dfa7c9 100644
> --- a/arch/arm/mach-rockchip/rk3368/rk3368.c
> +++ b/arch/arm/mach-rockchip/rk3368/rk3368.c
> @@ -55,6 +55,7 @@ struct mm_region *mem_map = rk3368_mem_map;
> 
>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff0f0000",
> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff120000/flash@0",

This is fine.

>  	[BROM_BOOTSOURCE_SD] = "/mmc@ff0c0000",
>  };
> 
> diff --git a/arch/arm/mach-rockchip/rv1126/rv1126.c
> b/arch/arm/mach-rockchip/rv1126/rv1126.c
> index 8589c46f10aa..40eb9eb7b196 100644
> --- a/arch/arm/mach-rockchip/rv1126/rv1126.c
> +++ b/arch/arm/mach-rockchip/rv1126/rv1126.c
> @@ -15,6 +15,7 @@
> 
>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ffc50000",
> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ffc90000/flash@0",

This is also fine.

>  	[BROM_BOOTSOURCE_SD] = "/mmc@ffc60000",
>  };

Re: [PATCH v2 2/3] rockchip: spl: Cache boot source id for later use

[Dragan Simic]

On 2024-03-22 21:50, Jonas Karlman wrote:
> Rockchip BROM writes a boot source id at CFG_IRAM_BASE + 0x10, this id
> indicates from what storage media TPL/SPL was loaded from.
> 
> SPL uses this id to determine what device "same-as-spl" represent when
> determining from where FIT should be loaded. This works as long as the
> boot_devices array contain a matching id <-> node path entry.
> 
> However, SPL typically load a small part of TF-A into SRAM and on 
> RK3399
> this overwrites the CFG_IRAM_BASE + 0x10 addr used for boot source id.
> 
> For affected devices the u-boot,spl-boot-device would not be set when
> booting from SPI flash and the flash@0 node was not explicitly listed
> in the u-boot,spl-boot-order prop.
> 
> Here boot source id is 3 before FIT images is loaded, and 0 after:
> 
>   U-Boot SPL 2024.04-rc4 (Mar 15 2024 - 17:26:19 +0000)
>   board_spl_was_booted_from: brom_bootdevice_id 3 maps to
> '/spi@ff1d0000/flash@0'
>   Trying to boot from SPI
>   ## Checking hash(es) for config config-1 ... OK
>   ## Checking hash(es) for Image atf-1 ... sha256+ OK
>   ## Checking hash(es) for Image u-boot ... sha256+ OK
>   ## Checking hash(es) for Image fdt-1 ... sha256+ OK
>   ## Checking hash(es) for Image atf-2 ... sha256+ OK
>   ## Checking hash(es) for Image atf-3 ... sha256+ OK
>   board_spl_was_booted_from: failed to resolve brom_bootdevice_id 0
>   spl_decode_boot_device: could not find udevice for /mmc@fe330000
>   spl_decode_boot_device: could not find udevice for /mmc@fe320000
>   spl_perform_fixups: could not map boot_device to ofpath: -19
> 
> Use a static brom_bootsource_id_cache to save the boot source id after
> an initial read from SRAM to fix this, this allow spl_perform_fixups()
> to resolve correct boot source path for "same-as-spl" after SPL have
> loaded TF-A related FIT images into memory.
> 
> With this the spl-boot-device prop can correctly be resolved to the
> SPI flash node in the control FDT:
> 
>   => fdt addr ${fdtcontroladdr}
>   Working FDT set to f1ee6710
>   => fdt list /chosen
>   chosen {
>       u-boot,spl-boot-device = "/spi@ff1d0000/flash@0";
>       stdout-path = "serial2:1500000n8";
>       u-boot,spl-boot-order = "same-as-spl", "/mmc@fe330000", 
> "/mmc@fe320000";
>   };
> 
> Fixes: d57e16c7e712 ("rockchip: find U-boot proper boot device by
> inverting the logic that sets it")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Looking good to me!  Thanks for taking the time to implement this
version, as a more robust approach.

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

> ---
> v2:
> - Only cache boot source id that can be resolved to a node path
> - Make use of the new BROM_BOOTSOURCE_UNKNOWN enum value
> - Update commit message
> - Add fixes tag
> - Collect r-b tag
> ---
>  arch/arm/mach-rockchip/spl.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/spl.c 
> b/arch/arm/mach-rockchip/spl.c
> index 1586a093fc37..3ce7e792b5a2 100644
> --- a/arch/arm/mach-rockchip/spl.c
> +++ b/arch/arm/mach-rockchip/spl.c
> @@ -32,18 +32,26 @@ __weak const char * const
> boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
> 
>  const char *board_spl_was_booted_from(void)
>  {
> -	u32  bootdevice_brom_id = readl(BROM_BOOTSOURCE_ID_ADDR);
> +	static u32 brom_bootsource_id_cache = BROM_BOOTSOURCE_UNKNOWN;
> +	u32 bootdevice_brom_id;
>  	const char *bootdevice_ofpath = NULL;
> 
> +	if (brom_bootsource_id_cache != BROM_BOOTSOURCE_UNKNOWN)
> +		bootdevice_brom_id = brom_bootsource_id_cache;
> +	else
> +		bootdevice_brom_id = readl(BROM_BOOTSOURCE_ID_ADDR);
> +
>  	if (bootdevice_brom_id < ARRAY_SIZE(boot_devices))
>  		bootdevice_ofpath = boot_devices[bootdevice_brom_id];
> 
> -	if (bootdevice_ofpath)
> +	if (bootdevice_ofpath) {
> +		brom_bootsource_id_cache = bootdevice_brom_id;
>  		debug("%s: brom_bootdevice_id %x maps to '%s'\n",
>  		      __func__, bootdevice_brom_id, bootdevice_ofpath);
> -	else
> +	} else {
>  		debug("%s: failed to resolve brom_bootdevice_id %x\n",
>  		      __func__, bootdevice_brom_id);
> +	}
> 
>  	return bootdevice_ofpath;
>  }

Re: [PATCH v2 3/3] rockchip: spl: Add SPI NOR flash to boot_devices array

[Jonas Karlman]

Hi Dragan,

On 2024-03-24 17:52, Dragan Simic wrote:
> Hello Jonas,
> 
> Please see my comments below.
> 
> On 2024-03-22 21:50, Jonas Karlman wrote:
>> Add missing boot source id <-> device tree node path mapping for SPI 
>> NOR
>> flash on PX30, RK3288, RK3308, RK3368 and RV1126.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> v2:
>> - New patch
>> ---
>>  arch/arm/mach-rockchip/px30/px30.c     | 1 +
>>  arch/arm/mach-rockchip/rk3288/rk3288.c | 1 +
>>  arch/arm/mach-rockchip/rk3308/rk3308.c | 1 +
>>  arch/arm/mach-rockchip/rk3368/rk3368.c | 1 +
>>  arch/arm/mach-rockchip/rv1126/rv1126.c | 1 +
>>  5 files changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/mach-rockchip/px30/px30.c
>> b/arch/arm/mach-rockchip/px30/px30.c
>> index b4f655fa4b3e..2ec3289d75b7 100644
>> --- a/arch/arm/mach-rockchip/px30/px30.c
>> +++ b/arch/arm/mach-rockchip/px30/px30.c
>> @@ -19,6 +19,7 @@
>>
>>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff390000",
>> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff3a0000/flash@0",
> 
> AFAICT, no PX30-based board contains an SPI chip, but making this
> future-proof perhaps shouldn't hurt.

The rk3326-odroid-go2 variant of PX30 has a jedec,spi-nor node.

> 
>>  	[BROM_BOOTSOURCE_SD] = "/mmc@ff370000",
>>  };
>>
>> diff --git a/arch/arm/mach-rockchip/rk3288/rk3288.c
>> b/arch/arm/mach-rockchip/rk3288/rk3288.c
>> index c77c56c1dab5..70cf50029121 100644
>> --- a/arch/arm/mach-rockchip/rk3288/rk3288.c
>> +++ b/arch/arm/mach-rockchip/rk3288/rk3288.c
>> @@ -28,6 +28,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>
>>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff0f0000",
>> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff130000/flash@0",
> 
> Using spi2 for a flash chip applies to some RK3288-based boards, but 
> AFAICT
> not to all of them.  For example, arch/arm/dts/rk3288-evb.dtsi uses spi0
> for a flash chip, which in turn translates to /spi@ff110000/flash@0.  
> The
> same also applies to arch/arm/dts/rk3288-firefly.dtsi, for example.

The RK3288 TRM explicitly mention that it can boot from SPI2 CS0.

Also I can only find jedec,spi-nor reference in rk3288-phycore-som.dtsi
and rk3288-veyron.dtsi, nothing in rk3288-evb.dtsi or rk3288-firefly.dtsi.

> 
>>  	[BROM_BOOTSOURCE_SD] = "/mmc@ff0c0000",
>>  };
>>
>> diff --git a/arch/arm/mach-rockchip/rk3308/rk3308.c
>> b/arch/arm/mach-rockchip/rk3308/rk3308.c
>> index 27a748327e31..b3ffabc5449a 100644
>> --- a/arch/arm/mach-rockchip/rk3308/rk3308.c
>> +++ b/arch/arm/mach-rockchip/rk3308/rk3308.c
>> @@ -141,6 +141,7 @@ enum {
>>
>>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff490000",
>> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff4c0000/flash@0",
> 
> AFAICT, no RK3308-based board contains an SPI chip, and I'm not really
> sure where does /spi@ff4c0000/flash@0 come from.

RK3308 TRM mention that it can boot from the SFC. And yes there is no
upstream board that currently have a jedec,spi-nor node, it however
have the sfc (spi@ff4c0000) node so this adds correct node path.

Regards,
Jonas

> 
>>  	[BROM_BOOTSOURCE_SD] = "/mmc@ff480000",
>>  };
>>
>> diff --git a/arch/arm/mach-rockchip/rk3368/rk3368.c
>> b/arch/arm/mach-rockchip/rk3368/rk3368.c
>> index 651ba1090206..8f5ca1dfa7c9 100644
>> --- a/arch/arm/mach-rockchip/rk3368/rk3368.c
>> +++ b/arch/arm/mach-rockchip/rk3368/rk3368.c
>> @@ -55,6 +55,7 @@ struct mm_region *mem_map = rk3368_mem_map;
>>
>>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff0f0000",
>> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff120000/flash@0",
> 
> This is fine.
> 
>>  	[BROM_BOOTSOURCE_SD] = "/mmc@ff0c0000",
>>  };
>>
>> diff --git a/arch/arm/mach-rockchip/rv1126/rv1126.c
>> b/arch/arm/mach-rockchip/rv1126/rv1126.c
>> index 8589c46f10aa..40eb9eb7b196 100644
>> --- a/arch/arm/mach-rockchip/rv1126/rv1126.c
>> +++ b/arch/arm/mach-rockchip/rv1126/rv1126.c
>> @@ -15,6 +15,7 @@
>>
>>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ffc50000",
>> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ffc90000/flash@0",
> 
> This is also fine.
> 
>>  	[BROM_BOOTSOURCE_SD] = "/mmc@ffc60000",
>>  };

Re: [PATCH v2 3/3] rockchip: spl: Add SPI NOR flash to boot_devices array

[Dragan Simic]

On 2024-03-24 18:28, Jonas Karlman wrote:
> On 2024-03-24 17:52, Dragan Simic wrote:
>> Please see my comments below.
>> 
>> On 2024-03-22 21:50, Jonas Karlman wrote:
>>> Add missing boot source id <-> device tree node path mapping for SPI
>>> NOR
>>> flash on PX30, RK3288, RK3308, RK3368 and RV1126.
>>> 
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>> ---
>>> v2:
>>> - New patch
>>> ---
>>>  arch/arm/mach-rockchip/px30/px30.c     | 1 +
>>>  arch/arm/mach-rockchip/rk3288/rk3288.c | 1 +
>>>  arch/arm/mach-rockchip/rk3308/rk3308.c | 1 +
>>>  arch/arm/mach-rockchip/rk3368/rk3368.c | 1 +
>>>  arch/arm/mach-rockchip/rv1126/rv1126.c | 1 +
>>>  5 files changed, 5 insertions(+)
>>> 
>>> diff --git a/arch/arm/mach-rockchip/px30/px30.c
>>> b/arch/arm/mach-rockchip/px30/px30.c
>>> index b4f655fa4b3e..2ec3289d75b7 100644
>>> --- a/arch/arm/mach-rockchip/px30/px30.c
>>> +++ b/arch/arm/mach-rockchip/px30/px30.c
>>> @@ -19,6 +19,7 @@
>>> 
>>>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff390000",
>>> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff3a0000/flash@0",
>> 
>> AFAICT, no PX30-based board contains an SPI chip, but making this
>> future-proof perhaps shouldn't hurt.
> 
> The rk3326-odroid-go2 variant of PX30 has a jedec,spi-nor node.

Ah, I see, I missed that.  Thanks for the clarification.

>>>  	[BROM_BOOTSOURCE_SD] = "/mmc@ff370000",
>>>  };
>>> 
>>> diff --git a/arch/arm/mach-rockchip/rk3288/rk3288.c
>>> b/arch/arm/mach-rockchip/rk3288/rk3288.c
>>> index c77c56c1dab5..70cf50029121 100644
>>> --- a/arch/arm/mach-rockchip/rk3288/rk3288.c
>>> +++ b/arch/arm/mach-rockchip/rk3288/rk3288.c
>>> @@ -28,6 +28,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>> 
>>>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff0f0000",
>>> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff130000/flash@0",
>> 
>> Using spi2 for a flash chip applies to some RK3288-based boards, but
>> AFAICT not to all of them.  For example, arch/arm/dts/rk3288-evb.dtsi
>> uses spi0 for a flash chip, which in turn translates to
>> /spi@ff110000/flash@0.  The same also applies to
>> arch/arm/dts/rk3288-firefly.dtsi, for example.
> 
> The RK3288 TRM explicitly mention that it can boot from SPI2 CS0.
> 
> Also I can only find jedec,spi-nor reference in rk3288-phycore-som.dtsi
> and rk3288-veyron.dtsi, nothing in rk3288-evb.dtsi or 
> rk3288-firefly.dtsi.

Oh, I see now, you're right.  I've checked the Firefly-RK3288 schematic
and it just exposes the spi0 interface on one of its expansion headers.

>>>  	[BROM_BOOTSOURCE_SD] = "/mmc@ff0c0000",
>>>  };
>>> 
>>> diff --git a/arch/arm/mach-rockchip/rk3308/rk3308.c
>>> b/arch/arm/mach-rockchip/rk3308/rk3308.c
>>> index 27a748327e31..b3ffabc5449a 100644
>>> --- a/arch/arm/mach-rockchip/rk3308/rk3308.c
>>> +++ b/arch/arm/mach-rockchip/rk3308/rk3308.c
>>> @@ -141,6 +141,7 @@ enum {
>>> 
>>>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff490000",
>>> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff4c0000/flash@0",
>> 
>> AFAICT, no RK3308-based board contains an SPI chip, and I'm not really
>> sure where does /spi@ff4c0000/flash@0 come from.
> 
> RK3308 TRM mention that it can boot from the SFC. And yes there is no
> upstream board that currently have a jedec,spi-nor node, it however
> have the sfc (spi@ff4c0000) node so this adds correct node path.

Hmm, I see no sfc node in any of the arch/arm/dts/rk3308* files?
Maybe I'm missing something, but there's also no "ff4c0000" string
in the entire arch/arm/dts directory.

Re: [PATCH v2 3/3] rockchip: spl: Add SPI NOR flash to boot_devices array

[Jonas Karlman]

On 2024-03-24 21:58, Dragan Simic wrote:
> On 2024-03-24 18:28, Jonas Karlman wrote:
>> On 2024-03-24 17:52, Dragan Simic wrote:
>>> Please see my comments below.
>>>
>>> On 2024-03-22 21:50, Jonas Karlman wrote:
>>>> Add missing boot source id <-> device tree node path mapping for SPI
>>>> NOR
>>>> flash on PX30, RK3288, RK3308, RK3368 and RV1126.
>>>>
>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>> ---
>>>> v2:
>>>> - New patch
>>>> ---
>>>>  arch/arm/mach-rockchip/px30/px30.c     | 1 +
>>>>  arch/arm/mach-rockchip/rk3288/rk3288.c | 1 +
>>>>  arch/arm/mach-rockchip/rk3308/rk3308.c | 1 +
>>>>  arch/arm/mach-rockchip/rk3368/rk3368.c | 1 +
>>>>  arch/arm/mach-rockchip/rv1126/rv1126.c | 1 +
>>>>  5 files changed, 5 insertions(+)
>>>>
>>>> diff --git a/arch/arm/mach-rockchip/px30/px30.c
>>>> b/arch/arm/mach-rockchip/px30/px30.c
>>>> index b4f655fa4b3e..2ec3289d75b7 100644
>>>> --- a/arch/arm/mach-rockchip/px30/px30.c
>>>> +++ b/arch/arm/mach-rockchip/px30/px30.c
>>>> @@ -19,6 +19,7 @@
>>>>
>>>>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>>>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff390000",
>>>> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff3a0000/flash@0",
>>>
>>> AFAICT, no PX30-based board contains an SPI chip, but making this
>>> future-proof perhaps shouldn't hurt.
>>
>> The rk3326-odroid-go2 variant of PX30 has a jedec,spi-nor node.
> 
> Ah, I see, I missed that.  Thanks for the clarification.
> 
>>>>  	[BROM_BOOTSOURCE_SD] = "/mmc@ff370000",
>>>>  };
>>>>
>>>> diff --git a/arch/arm/mach-rockchip/rk3288/rk3288.c
>>>> b/arch/arm/mach-rockchip/rk3288/rk3288.c
>>>> index c77c56c1dab5..70cf50029121 100644
>>>> --- a/arch/arm/mach-rockchip/rk3288/rk3288.c
>>>> +++ b/arch/arm/mach-rockchip/rk3288/rk3288.c
>>>> @@ -28,6 +28,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>>>
>>>>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>>>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff0f0000",
>>>> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff130000/flash@0",
>>>
>>> Using spi2 for a flash chip applies to some RK3288-based boards, but
>>> AFAICT not to all of them.  For example, arch/arm/dts/rk3288-evb.dtsi
>>> uses spi0 for a flash chip, which in turn translates to
>>> /spi@ff110000/flash@0.  The same also applies to
>>> arch/arm/dts/rk3288-firefly.dtsi, for example.
>>
>> The RK3288 TRM explicitly mention that it can boot from SPI2 CS0.
>>
>> Also I can only find jedec,spi-nor reference in rk3288-phycore-som.dtsi
>> and rk3288-veyron.dtsi, nothing in rk3288-evb.dtsi or 
>> rk3288-firefly.dtsi.
> 
> Oh, I see now, you're right.  I've checked the Firefly-RK3288 schematic
> and it just exposes the spi0 interface on one of its expansion headers.
> 
>>>>  	[BROM_BOOTSOURCE_SD] = "/mmc@ff0c0000",
>>>>  };
>>>>
>>>> diff --git a/arch/arm/mach-rockchip/rk3308/rk3308.c
>>>> b/arch/arm/mach-rockchip/rk3308/rk3308.c
>>>> index 27a748327e31..b3ffabc5449a 100644
>>>> --- a/arch/arm/mach-rockchip/rk3308/rk3308.c
>>>> +++ b/arch/arm/mach-rockchip/rk3308/rk3308.c
>>>> @@ -141,6 +141,7 @@ enum {
>>>>
>>>>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>>>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff490000",
>>>> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff4c0000/flash@0",
>>>
>>> AFAICT, no RK3308-based board contains an SPI chip, and I'm not really
>>> sure where does /spi@ff4c0000/flash@0 come from.
>>
>> RK3308 TRM mention that it can boot from the SFC. And yes there is no
>> upstream board that currently have a jedec,spi-nor node, it however
>> have the sfc (spi@ff4c0000) node so this adds correct node path.
> 
> Hmm, I see no sfc node in any of the arch/arm/dts/rk3308* files?
> Maybe I'm missing something, but there's also no "ff4c0000" string
> in the entire arch/arm/dts directory.

The rk3308 DTs in U-Boot is very outdated so the sfc node cannot be
found in current master branch.

The sfc node is part of linux upstream DT, should also match in
dts/upstream in next branch and after the rk3308 DT sync series [1].

rockchip: rk3308: Sync DT with linux v6.8 and update defconfigs:
[1] https://patchwork.ozlabs.org/cover/1912830/

Regards,
Jonas

Re: [PATCH v2 3/3] rockchip: spl: Add SPI NOR flash to boot_devices array

[Dragan Simic]

On 2024-03-24 22:23, Jonas Karlman wrote:
> On 2024-03-24 21:58, Dragan Simic wrote:
>> On 2024-03-24 18:28, Jonas Karlman wrote:
>>> On 2024-03-24 17:52, Dragan Simic wrote:
>>>> On 2024-03-22 21:50, Jonas Karlman wrote:
>>>>> diff --git a/arch/arm/mach-rockchip/rk3308/rk3308.c
>>>>> b/arch/arm/mach-rockchip/rk3308/rk3308.c
>>>>> index 27a748327e31..b3ffabc5449a 100644
>>>>> --- a/arch/arm/mach-rockchip/rk3308/rk3308.c
>>>>> +++ b/arch/arm/mach-rockchip/rk3308/rk3308.c
>>>>> @@ -141,6 +141,7 @@ enum {
>>>>> 
>>>>>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>>>>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@ff490000",
>>>>> +	[BROM_BOOTSOURCE_SPINOR] = "/spi@ff4c0000/flash@0",
>>>> 
>>>> AFAICT, no RK3308-based board contains an SPI chip, and I'm not 
>>>> really
>>>> sure where does /spi@ff4c0000/flash@0 come from.
>>> 
>>> RK3308 TRM mention that it can boot from the SFC. And yes there is no
>>> upstream board that currently have a jedec,spi-nor node, it however
>>> have the sfc (spi@ff4c0000) node so this adds correct node path.
>> 
>> Hmm, I see no sfc node in any of the arch/arm/dts/rk3308* files?
>> Maybe I'm missing something, but there's also no "ff4c0000" string
>> in the entire arch/arm/dts directory.
> 
> The rk3308 DTs in U-Boot is very outdated so the sfc node cannot be
> found in current master branch.
> 
> The sfc node is part of linux upstream DT, should also match in
> dts/upstream in next branch and after the rk3308 DT sync series [1].
> 
> rockchip: rk3308: Sync DT with linux v6.8 and update defconfigs:
> [1] https://patchwork.ozlabs.org/cover/1912830/

All good, thanks for explaining it further.  Please add

Reviewed-by: Dragan Simic <dsimic@manjaro.org>