diff mbox series

[v2,6/9] spacemit: k1: Add multiple device tree support

Message ID 20260520-b4-k1-spl-pinctrl-spinor-v2-6-8d25db98ac8e@riscstar.com
State New
Delegated to: Andes
Headers show
Series riscv: spacemit: k1: add pinctrl/GPIO and SPI NOR support | expand

Commit Message

Guodong Xu May 20, 2026, 10:45 a.m. UTC
Enable multiple DTB support in the FIT image for the Spacemit K1 SoC,
allowing a single U-Boot binary to support different board variants.

The SPL reads the board type from EEPROM and selects the corresponding
device tree at runtime via board_fit_config_name_match(), ensuring the
correct hardware description is passed to U-Boot proper.

Signed-off-by: Guodong Xu <guodong@riscstar.com>

---
v2:
- Reworked.  Drop the v1 approach (new local k1-muse-pi-pro.dts,
  enlarge SYS_MALLOC_F_LEN, MMODE switch).
- Use binman --fit-multi-config so u-boot.itb packs multiple board
  DTs from the upstream tree.
---
 board/spacemit/k1/spl.c       | 42 ++++++++++++++++++++++++++++++++++--------
 configs/spacemit_k1_defconfig |  2 ++
 2 files changed, 36 insertions(+), 8 deletions(-)

Comments

Aurelien Jarno May 24, 2026, 9:25 p.m. UTC | #1
Hi,

On 2026-05-20 06:45, Guodong Xu wrote:
> Enable multiple DTB support in the FIT image for the Spacemit K1 SoC,
> allowing a single U-Boot binary to support different board variants.
> 
> The SPL reads the board type from EEPROM and selects the corresponding
> device tree at runtime via board_fit_config_name_match(), ensuring the
> correct hardware description is passed to U-Boot proper.
> 
> Signed-off-by: Guodong Xu <guodong@riscstar.com>
> 
> ---
> v2:
> - Reworked.  Drop the v1 approach (new local k1-muse-pi-pro.dts,
>   enlarge SYS_MALLOC_F_LEN, MMODE switch).
> - Use binman --fit-multi-config so u-boot.itb packs multiple board
>   DTs from the upstream tree.
> ---
>  board/spacemit/k1/spl.c       | 42 ++++++++++++++++++++++++++++++++++--------
>  configs/spacemit_k1_defconfig |  2 ++
>  2 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/board/spacemit/k1/spl.c b/board/spacemit/k1/spl.c
> index da4169fbc8c..d749e21a2d5 100644
> --- a/board/spacemit/k1/spl.c
> +++ b/board/spacemit/k1/spl.c
> @@ -13,6 +13,7 @@
>  #include <dm/device.h>
>  #include <dm/uclass.h>
>  #include <i2c.h>
> +#include <linux/ctype.h>
>  #include <linux/delay.h>
>  #include <log.h>
>  #include <power/regulator.h>
> @@ -55,6 +56,8 @@ struct ddr_cfg {
>  binman_sym_declare(ulong, ddr_fw, image_pos);
>  binman_sym_declare(ulong, ddr_fw, size);
>  
> +char product_name[I2C_BUF_SIZE] = "k1";
> +
>  static void i2c_early_init(void)
>  {
>  	struct udevice *bus;
> @@ -322,14 +325,8 @@ void nor_early_init(void)
>  	udelay(10);
>  }
>  
> -void *board_spl_fit_buffer_addr(ulong fit_size, int sectors, int bl_len)
> -{
> -	return (void *)CONFIG_SPL_LOAD_FIT_ADDRESS;
> -}
> -
>  void board_init_f(ulong dummy)
>  {
> -	u8 i2c_buf[I2C_BUF_SIZE] = { 0 };
>  	int ret;
>  
>  	ret = spl_early_init();
> @@ -344,11 +341,11 @@ void board_init_f(ulong dummy)
>  	preloader_console_init();
>  
>  	i2c_early_init();
> -	ret = read_product_name(i2c_buf, I2C_BUF_SIZE);
> +	ret = read_product_name(product_name, I2C_BUF_SIZE);
>  	if (ret)
>  		log_info("Fail to detect board:%d\n", ret);
>  	else
> -		log_info("Get board name:%s\n", (char *)i2c_buf);
> +		log_info("Get board name:%s\n", product_name);
>  	pmic_init();
>  
>  	ddr_early_init();
> @@ -363,3 +360,32 @@ u32 spl_boot_device(void)
>  void spl_board_init(void)
>  {
>  }
> +
> +int board_fit_config_name_match(const char *name)
> +{
> +	char fdt_name[I2C_BUF_SIZE];
> +	int i;
> +
> +	memset(fdt_name, 0, I2C_BUF_SIZE);
> +	if (!strncmp(product_name, "k1-x_", 5)) {
> +		snprintf(fdt_name, I2C_BUF_SIZE, "%s-%s", "k1",
> +			 &product_name[5]);
> +	}

I wonder if any normalisation would be better in board_init_f() as
board_fit_config_name_match() is basically called for each possible 
board. Same for the tolower() loop below.

> +	if (fdt_name[0] == '\0') {
> +		/* set default board name */
> +		sprintf(fdt_name, "k1-musepi-pro");
> +	}

I am not sure k1-musepi-pro is a good default. The experience is that 
the Banana-Pi F3 (known as k1-x_deb1 in the vendor code) doesn't  have a 
product name defined in the TLV EEPROM and thus should probably be the 
default.

> +	for (i = 0; i < I2C_BUF_SIZE; i++) {
> +		if (fdt_name[i] == '\0')
> +			break;
> +		fdt_name[i] = tolower(fdt_name[i]);
> +	}
> +	if (!strcmp(name, fdt_name))
> +		return 0;

I am not sure that all the vendor product names will match the linux dtb 
names, so maybe just using a table to do the mapping would be better.  
Anyway, as long as the musepi-pro is the only one supported that should 
be fine.

Regards
Aurelien
Guodong Xu May 26, 2026, 1:24 p.m. UTC | #2
Hi, Aurelien

Thanks for the review.

On Mon, May 25, 2026 at 5:26 AM Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> Hi,
>
> On 2026-05-20 06:45, Guodong Xu wrote:
> > Enable multiple DTB support in the FIT image for the Spacemit K1 SoC,
> > allowing a single U-Boot binary to support different board variants.
> >
> > @@ -363,3 +360,32 @@ u32 spl_boot_device(void)
> >  void spl_board_init(void)
> >  {
> >  }
> > +
> > +int board_fit_config_name_match(const char *name)
> > +{
> > +     char fdt_name[I2C_BUF_SIZE];
> > +     int i;
> > +
> > +     memset(fdt_name, 0, I2C_BUF_SIZE);
> > +     if (!strncmp(product_name, "k1-x_", 5)) {
> > +             snprintf(fdt_name, I2C_BUF_SIZE, "%s-%s", "k1",
> > +                      &product_name[5]);
> > +     }
>
> I wonder if any normalisation would be better in board_init_f() as
> board_fit_config_name_match() is basically called for each possible
> board. Same for the tolower() loop below.

Agree, will move both into board_init_f() in v3.

>
> > +     if (fdt_name[0] == '\0') {
> > +             /* set default board name */
> > +             sprintf(fdt_name, "k1-musepi-pro");
> > +     }
>
> I am not sure k1-musepi-pro is a good default. The experience is that
> the Banana-Pi F3 (known as k1-x_deb1 in the vendor code) doesn't  have a
> product name defined in the TLV EEPROM and thus should probably be the
> default.

Thank you for this observation. Agree.

Will change the default to spacemit/k1-bananapi-f3 in v3 so a
BPI-F3 with no TLV product name falls through to the correct DTB.

>
> > +     for (i = 0; i < I2C_BUF_SIZE; i++) {
> > +             if (fdt_name[i] == '\0')
> > +                     break;
> > +             fdt_name[i] = tolower(fdt_name[i]);
> > +     }
> > +     if (!strcmp(name, fdt_name))
> > +             return 0;
>
> I am not sure that all the vendor product names will match the linux dtb
> names, so maybe just using a table to do the mapping would be better.
> Anyway, as long as the musepi-pro is the only one supported that should
> be fine.

Agree. A table will be more flexible and easy-to-expand. In v3 I will use
an explicit per-board mapping:
  k1-x_MUSE-Pi-Pro -> spacemit/k1-musepi-pro
  k1-x_deb1 -> spacemit/k1-bananapi-f3
  k1-x_milkv-jupiter -> spacemit/k1-milkv-jupiter

, falling back to spacemit/k1-bananapi-f3 by default.

With these, tolower() will not be necessary.

BR,
Guodong / docularxu

> Regards
> Aurelien
>
> --
> Aurelien Jarno                          GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net                     http://aurel32.net
Simon Glass May 26, 2026, 1:36 p.m. UTC | #3
Hi Guodong,

On Tue, 19 May 2026 at 16:49, Guodong Xu <guodong@riscstar.com> wrote:
>
> Enable multiple DTB support in the FIT image for the Spacemit K1 SoC,
> allowing a single U-Boot binary to support different board variants.
>
> The SPL reads the board type from EEPROM and selects the corresponding
> device tree at runtime via board_fit_config_name_match(), ensuring the
> correct hardware description is passed to U-Boot proper.
>
> Signed-off-by: Guodong Xu <guodong@riscstar.com>
>
> ---
> v2:
> - Reworked.  Drop the v1 approach (new local k1-muse-pi-pro.dts,
>   enlarge SYS_MALLOC_F_LEN, MMODE switch).
> - Use binman --fit-multi-config so u-boot.itb packs multiple board
>   DTs from the upstream tree.
> ---
>  board/spacemit/k1/spl.c       | 42 ++++++++++++++++++++++++++++++++++--------
>  configs/spacemit_k1_defconfig |  2 ++
>  2 files changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/board/spacemit/k1/spl.c b/board/spacemit/k1/spl.c
> index da4169fbc8c..d749e21a2d5 100644
> --- a/board/spacemit/k1/spl.c
> +++ b/board/spacemit/k1/spl.c
> @@ -13,6 +13,7 @@
>  #include <dm/device.h>
>  #include <dm/uclass.h>
>  #include <i2c.h>
> +#include <linux/ctype.h>
>  #include <linux/delay.h>
>  #include <log.h>
>  #include <power/regulator.h>
> @@ -55,6 +56,8 @@ struct ddr_cfg {
>  binman_sym_declare(ulong, ddr_fw, image_pos);
>  binman_sym_declare(ulong, ddr_fw, size);
>
> +char product_name[I2C_BUF_SIZE] = "k1";
> +
>  static void i2c_early_init(void)
>  {
>         struct udevice *bus;
> @@ -322,14 +325,8 @@ void nor_early_init(void)
>         udelay(10);
>  }
>
> -void *board_spl_fit_buffer_addr(ulong fit_size, int sectors, int bl_len)
> -{
> -       return (void *)CONFIG_SPL_LOAD_FIT_ADDRESS;
> -}
> -
>  void board_init_f(ulong dummy)
>  {
> -       u8 i2c_buf[I2C_BUF_SIZE] = { 0 };
>         int ret;
>
>         ret = spl_early_init();
> @@ -344,11 +341,11 @@ void board_init_f(ulong dummy)
>         preloader_console_init();
>
>         i2c_early_init();
> -       ret = read_product_name(i2c_buf, I2C_BUF_SIZE);
> +       ret = read_product_name(product_name, I2C_BUF_SIZE);
>         if (ret)
>                 log_info("Fail to detect board:%d\n", ret);
>         else
> -               log_info("Get board name:%s\n", (char *)i2c_buf);
> +               log_info("Get board name:%s\n", product_name);
>         pmic_init();

This is quite hacky. Firstly you should be able to delay selecting the
DT until board_init_r() - i.e. once SPL is running fully. Why is it
needed so early?

Then, you should be able to use a syscon driver to obtain the
information, using driver model. Do you have lots of size constraints
which prevent driver model in SPL?

>
>         ddr_early_init();
> @@ -363,3 +360,32 @@ u32 spl_boot_device(void)
>  void spl_board_init(void)
>  {
>  }
> +
> +int board_fit_config_name_match(const char *name)
> +{
> +       char fdt_name[I2C_BUF_SIZE];
> +       int i;
> +
> +       memset(fdt_name, 0, I2C_BUF_SIZE);
> +       if (!strncmp(product_name, "k1-x_", 5)) {
> +               snprintf(fdt_name, I2C_BUF_SIZE, "%s-%s", "k1",
> +                        &product_name[5]);
> +       }
> +       if (fdt_name[0] == '\0') {

if (!*fdt_name) {

> +               /* set default board name */
> +               sprintf(fdt_name, "k1-musepi-pro");
> +       }
> +       for (i = 0; i < I2C_BUF_SIZE; i++) {
> +               if (fdt_name[i] == '\0')

same

how about adding a helper function in lib/ to lower-case and return a string?

> +                       break;
> +               fdt_name[i] = tolower(fdt_name[i]);
> +       }
> +       if (!strcmp(name, fdt_name))
> +               return 0;

blank line before final return

> +       return -ENOENT;
> +}
> +
> +void *board_spl_fit_buffer_addr(ulong fit_size, int sectors, int bl_len)
> +{
> +       return (void *)CONFIG_SPL_LOAD_FIT_ADDRESS;
> +}
> diff --git a/configs/spacemit_k1_defconfig b/configs/spacemit_k1_defconfig
> index 64c724c62a5..465788cc2e5 100644
> --- a/configs/spacemit_k1_defconfig
> +++ b/configs/spacemit_k1_defconfig
> @@ -5,6 +5,7 @@ CONFIG_NR_DRAM_BANKS=2
>  CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>  CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x1000000
>  CONFIG_DEFAULT_DEVICE_TREE="spacemit/k1-musepi-pro"
> +CONFIG_OF_LIST="spacemit/k1-bananapi-f3 spacemit/k1-musepi-pro"
>  CONFIG_SPL_SYS_MALLOC_F_LEN=0x4000
>  CONFIG_SPL_STACK=0xc083fb00
>  CONFIG_SPL_TEXT_BASE=0xc0801000
> @@ -27,6 +28,7 @@ CONFIG_SPL_RISCV_MMODE=y
>  # CONFIG_SPL_SMP is not set
>  CONFIG_STACK_SIZE=0x100000
>  CONFIG_FIT=y
> +CONFIG_MULTI_DTB_FIT=y
>  CONFIG_SPL_HAS_LOAD_FIT_ADDRESS=y
>  CONFIG_SPL_LOAD_FIT_ADDRESS=0x08000000
>  CONFIG_SUPPORT_RAW_INITRD=y
>
> --
> 2.43.0
>

Regards,
Simon
Guodong Xu June 4, 2026, 2:21 p.m. UTC | #4
Hi Aurelien, Simon

Thanks both for the review.

I'm moving on from riscstar, so I won't be sending v3 of this series
myself. My colleague Raymond Mao (CC'd) will take it over and address
your comments.

Thanks again for taking the time to look at it.

Best regards,
Guodong Xu
diff mbox series

Patch

diff --git a/board/spacemit/k1/spl.c b/board/spacemit/k1/spl.c
index da4169fbc8c..d749e21a2d5 100644
--- a/board/spacemit/k1/spl.c
+++ b/board/spacemit/k1/spl.c
@@ -13,6 +13,7 @@ 
 #include <dm/device.h>
 #include <dm/uclass.h>
 #include <i2c.h>
+#include <linux/ctype.h>
 #include <linux/delay.h>
 #include <log.h>
 #include <power/regulator.h>
@@ -55,6 +56,8 @@  struct ddr_cfg {
 binman_sym_declare(ulong, ddr_fw, image_pos);
 binman_sym_declare(ulong, ddr_fw, size);
 
+char product_name[I2C_BUF_SIZE] = "k1";
+
 static void i2c_early_init(void)
 {
 	struct udevice *bus;
@@ -322,14 +325,8 @@  void nor_early_init(void)
 	udelay(10);
 }
 
-void *board_spl_fit_buffer_addr(ulong fit_size, int sectors, int bl_len)
-{
-	return (void *)CONFIG_SPL_LOAD_FIT_ADDRESS;
-}
-
 void board_init_f(ulong dummy)
 {
-	u8 i2c_buf[I2C_BUF_SIZE] = { 0 };
 	int ret;
 
 	ret = spl_early_init();
@@ -344,11 +341,11 @@  void board_init_f(ulong dummy)
 	preloader_console_init();
 
 	i2c_early_init();
-	ret = read_product_name(i2c_buf, I2C_BUF_SIZE);
+	ret = read_product_name(product_name, I2C_BUF_SIZE);
 	if (ret)
 		log_info("Fail to detect board:%d\n", ret);
 	else
-		log_info("Get board name:%s\n", (char *)i2c_buf);
+		log_info("Get board name:%s\n", product_name);
 	pmic_init();
 
 	ddr_early_init();
@@ -363,3 +360,32 @@  u32 spl_boot_device(void)
 void spl_board_init(void)
 {
 }
+
+int board_fit_config_name_match(const char *name)
+{
+	char fdt_name[I2C_BUF_SIZE];
+	int i;
+
+	memset(fdt_name, 0, I2C_BUF_SIZE);
+	if (!strncmp(product_name, "k1-x_", 5)) {
+		snprintf(fdt_name, I2C_BUF_SIZE, "%s-%s", "k1",
+			 &product_name[5]);
+	}
+	if (fdt_name[0] == '\0') {
+		/* set default board name */
+		sprintf(fdt_name, "k1-musepi-pro");
+	}
+	for (i = 0; i < I2C_BUF_SIZE; i++) {
+		if (fdt_name[i] == '\0')
+			break;
+		fdt_name[i] = tolower(fdt_name[i]);
+	}
+	if (!strcmp(name, fdt_name))
+		return 0;
+	return -ENOENT;
+}
+
+void *board_spl_fit_buffer_addr(ulong fit_size, int sectors, int bl_len)
+{
+	return (void *)CONFIG_SPL_LOAD_FIT_ADDRESS;
+}
diff --git a/configs/spacemit_k1_defconfig b/configs/spacemit_k1_defconfig
index 64c724c62a5..465788cc2e5 100644
--- a/configs/spacemit_k1_defconfig
+++ b/configs/spacemit_k1_defconfig
@@ -5,6 +5,7 @@  CONFIG_NR_DRAM_BANKS=2
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
 CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x1000000
 CONFIG_DEFAULT_DEVICE_TREE="spacemit/k1-musepi-pro"
+CONFIG_OF_LIST="spacemit/k1-bananapi-f3 spacemit/k1-musepi-pro"
 CONFIG_SPL_SYS_MALLOC_F_LEN=0x4000
 CONFIG_SPL_STACK=0xc083fb00
 CONFIG_SPL_TEXT_BASE=0xc0801000
@@ -27,6 +28,7 @@  CONFIG_SPL_RISCV_MMODE=y
 # CONFIG_SPL_SMP is not set
 CONFIG_STACK_SIZE=0x100000
 CONFIG_FIT=y
+CONFIG_MULTI_DTB_FIT=y
 CONFIG_SPL_HAS_LOAD_FIT_ADDRESS=y
 CONFIG_SPL_LOAD_FIT_ADDRESS=0x08000000
 CONFIG_SUPPORT_RAW_INITRD=y