[PATCH v2] board: mpfs_icicle: implement board_fdt_blob_setup()/board_fit_config_name_match()

[PATCH v2] board: mpfs_icicle: implement board_fdt_blob_setup()/board_fit_config_name_match()

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The firmware on the Icicle is capable of providing a devicetree in a1 to
U-Boot, but until now the devicetree has been packaged in a "payload" [1]
alongside U-Boot (or other bootloaders/RTOSes) and appended to the image.
The address of this appended devicetree is placed in a1 by the firmware.
This meant that the mechanism used by OF_SEPARATE to locate the
devicetree at the end of the image would pick up the one provided by the
firmware when u-boot-nodtb.bin was in the payload and U-Boot's devicetree
when u-boot.bin was.

The firmware is now going to be capable of providing a minimal devicetree
(quite cut down due to severe space constraints), but this devicetree is
linked into the firmware that runs out of the L2 rather than at the end
of the U-Boot image.

Implement board_fdt_blob_setup() so that this devicetree can be
optionally used, and the devicetree provided in the "payload" can be
used without relying on "happening" to implement the same strategy as
OF_SEPARATE expects in combination with u-boot-nodtb.bin.
Unlike other RISC-V boards, the firmware provided devicetree is only
used when OF_BOARD is set, so that the almost certainly more complete
devicetree in U-Boot will be used unless explicitly requested otherwise.

Implement board_fit_config_name_match(), so that, using the firmware
provided cut-down/minimal dtb, U-Boot can select one of several
devicetrees when MULTI_DTB_FIT is enabled.

Enabling both MULTI_DTB_FIT and OF_BOARD will lead to a conflict
between the two options, with the latter taking priority due to
board_fdt_blob_setup() being executed before board_fit_config_name_match(),
which causes gd->fdt_blob to be overwritten with a pointer to the
minimal devicetree rather than the location of the fit image containing
the multiple dtbs. Let MULTI_DTB_FIT take priority in this case, by
explicitly blocking the override when MULTI_DTB_FIT is enabled.

Link: https://github.com/polarfire-soc/hart-software-services/blob/master/tools/hss-payload-generator/README.md [1]
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---

v2:
- implement MULTI_DTB_FIT too

v1 was a year ago here:
https://lore.kernel.org/u-boot/20240625090806.1787287-2-conor.dooley@microchip.com/

There was some discussion on that thread, but ultimately I didn't find
what was mentioned to be worth implementing. Heinrich suggested to me, I
guess on IRC or something since it's not in that thread, implementing
MULTI_DTB_FIT, which I have done. A lot of the reason for the patch when
I wrote it was speculative, but we have some actual users for OF_BOARD
now - hence the v2 after about a year's gap.

CC: Ivan Griffin <ivan.griffin@microchip.com>
CC: Cyril Jean <cyril.jean@microchip.com>
CC: Tom Rini <trini@konsulko.com>
CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
CC: Simon Glass <sjg@chromium.org>
CC: Jamie.Gibbons@microchip.com
CC: u-boot@lists.denx.de
---
 board/microchip/mpfs_icicle/mpfs_icicle.c | 63 +++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/board/microchip/mpfs_icicle/mpfs_icicle.c b/board/microchip/mpfs_icicle/mpfs_icicle.c
index 4d7d843dfa3..6b6984eae3f 100644
--- a/board/microchip/mpfs_icicle/mpfs_icicle.c
+++ b/board/microchip/mpfs_icicle/mpfs_icicle.c
@@ -9,6 +9,7 @@
 #include <init.h>
 #include <asm/global_data.h>
 #include <asm/io.h>
+#include <asm/sections.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -50,6 +51,68 @@ static void read_device_serial_number(u8 *response, u8 response_size)
 		response_buf[idx] = readb(MPFS_SYS_SERVICE_MAILBOX + idx);
 }
 
+#if defined(CONFIG_MULTI_DTB_FIT)
+int board_fit_config_name_match(const char *name)
+{
+	const void *fdt;
+	int list_len;
+
+	/*
+	 * If there's not a HSS provided dtb, there's no point re-selecting
+	 * since we'd just end up re-selecting the same dtb again.
+	 */
+	if (!gd->arch.firmware_fdt_addr)
+		return -EINVAL;
+
+	fdt = (void *)gd->arch.firmware_fdt_addr;
+
+	list_len = fdt_stringlist_count(fdt, 0, "compatible");
+	if (list_len < 1)
+		return -EINVAL;
+
+	for (int i = 0; i < list_len; i++) {
+		int len, match;
+		const char *compat;
+		char *devendored;
+
+		compat = fdt_stringlist_get(fdt, 0, "compatible", i, &len);
+		if (!compat)
+			return -EINVAL;
+
+		strtok((char *)compat, ",");
+
+		devendored = strtok(NULL, ",");
+		if (!devendored)
+			return -EINVAL;
+
+		match = strcmp(devendored, name);
+		if (!match)
+			return 0;
+	}
+
+	return -EINVAL;
+}
+#endif
+
+int board_fdt_blob_setup(void **fdtp)
+{
+	fdtp = (void *)_end;
+
+	/*
+	 * The devicetree provided by the previous stage is very minimal due to
+	 * severe space constraints. The firmware performs no fixups etc.
+	 * U-Boot, if providing a devicetree, almost certainly has a better
+	 * more complete one than the firmware so that provided by the firmware
+	 * is ignored for OF_SEPARATE.
+	 */
+	if (IS_ENABLED(CONFIG_OF_BOARD) && !IS_ENABLED(CONFIG_MULTI_DTB_FIT)) {
+		if (gd->arch.firmware_fdt_addr)
+			fdtp = (void *)(uintptr_t)gd->arch.firmware_fdt_addr;
+	}
+
+	return 0;
+}
+
 int board_init(void)
 {
 	/* For now nothing to do here. */
-- 
2.45.2

Re: [PATCH v2] board: mpfs_icicle: implement board_fdt_blob_setup()/board_fit_config_name_match()

[Leo Liang]

On Mon, Jun 23, 2025 at 12:51:36PM +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The firmware on the Icicle is capable of providing a devicetree in a1 to
> U-Boot, but until now the devicetree has been packaged in a "payload" [1]
> alongside U-Boot (or other bootloaders/RTOSes) and appended to the image.
> The address of this appended devicetree is placed in a1 by the firmware.
> This meant that the mechanism used by OF_SEPARATE to locate the
> devicetree at the end of the image would pick up the one provided by the
> firmware when u-boot-nodtb.bin was in the payload and U-Boot's devicetree
> when u-boot.bin was.
> 
> The firmware is now going to be capable of providing a minimal devicetree
> (quite cut down due to severe space constraints), but this devicetree is
> linked into the firmware that runs out of the L2 rather than at the end
> of the U-Boot image.
> 
> Implement board_fdt_blob_setup() so that this devicetree can be
> optionally used, and the devicetree provided in the "payload" can be
> used without relying on "happening" to implement the same strategy as
> OF_SEPARATE expects in combination with u-boot-nodtb.bin.
> Unlike other RISC-V boards, the firmware provided devicetree is only
> used when OF_BOARD is set, so that the almost certainly more complete
> devicetree in U-Boot will be used unless explicitly requested otherwise.
> 
> Implement board_fit_config_name_match(), so that, using the firmware
> provided cut-down/minimal dtb, U-Boot can select one of several
> devicetrees when MULTI_DTB_FIT is enabled.
> 
> Enabling both MULTI_DTB_FIT and OF_BOARD will lead to a conflict
> between the two options, with the latter taking priority due to
> board_fdt_blob_setup() being executed before board_fit_config_name_match(),
> which causes gd->fdt_blob to be overwritten with a pointer to the
> minimal devicetree rather than the location of the fit image containing
> the multiple dtbs. Let MULTI_DTB_FIT take priority in this case, by
> explicitly blocking the override when MULTI_DTB_FIT is enabled.
> 
> Link: https://github.com/polarfire-soc/hart-software-services/blob/master/tools/hss-payload-generator/README.md [1]
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> 
> v2:
> - implement MULTI_DTB_FIT too
> 
> v1 was a year ago here:
> https://lore.kernel.org/u-boot/20240625090806.1787287-2-conor.dooley@microchip.com/
> 
> There was some discussion on that thread, but ultimately I didn't find
> what was mentioned to be worth implementing. Heinrich suggested to me, I
> guess on IRC or something since it's not in that thread, implementing
> MULTI_DTB_FIT, which I have done. A lot of the reason for the patch when
> I wrote it was speculative, but we have some actual users for OF_BOARD
> now - hence the v2 after about a year's gap.
> 
> CC: Ivan Griffin <ivan.griffin@microchip.com>
> CC: Cyril Jean <cyril.jean@microchip.com>
> CC: Tom Rini <trini@konsulko.com>
> CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: Simon Glass <sjg@chromium.org>
> CC: Jamie.Gibbons@microchip.com
> CC: u-boot@lists.denx.de
> ---
>  board/microchip/mpfs_icicle/mpfs_icicle.c | 63 +++++++++++++++++++++++
>  1 file changed, 63 insertions(+)

Acked-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Re: [PATCH v2] board: mpfs_icicle: implement board_fdt_blob_setup()/board_fit_config_name_match()

[Conor Dooley]

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

On Thu, Jul 03, 2025 at 06:09:53PM +0800, Leo Liang wrote:
> On Mon, Jun 23, 2025 at 12:51:36PM +0100, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > 
> > The firmware on the Icicle is capable of providing a devicetree in a1 to
> > U-Boot, but until now the devicetree has been packaged in a "payload" [1]
> > alongside U-Boot (or other bootloaders/RTOSes) and appended to the image.
> > The address of this appended devicetree is placed in a1 by the firmware.
> > This meant that the mechanism used by OF_SEPARATE to locate the
> > devicetree at the end of the image would pick up the one provided by the
> > firmware when u-boot-nodtb.bin was in the payload and U-Boot's devicetree
> > when u-boot.bin was.
> > 
> > The firmware is now going to be capable of providing a minimal devicetree
> > (quite cut down due to severe space constraints), but this devicetree is
> > linked into the firmware that runs out of the L2 rather than at the end
> > of the U-Boot image.
> > 
> > Implement board_fdt_blob_setup() so that this devicetree can be
> > optionally used, and the devicetree provided in the "payload" can be
> > used without relying on "happening" to implement the same strategy as
> > OF_SEPARATE expects in combination with u-boot-nodtb.bin.
> > Unlike other RISC-V boards, the firmware provided devicetree is only
> > used when OF_BOARD is set, so that the almost certainly more complete
> > devicetree in U-Boot will be used unless explicitly requested otherwise.
> > 
> > Implement board_fit_config_name_match(), so that, using the firmware
> > provided cut-down/minimal dtb, U-Boot can select one of several
> > devicetrees when MULTI_DTB_FIT is enabled.
> > 
> > Enabling both MULTI_DTB_FIT and OF_BOARD will lead to a conflict
> > between the two options, with the latter taking priority due to
> > board_fdt_blob_setup() being executed before board_fit_config_name_match(),
> > which causes gd->fdt_blob to be overwritten with a pointer to the
> > minimal devicetree rather than the location of the fit image containing
> > the multiple dtbs. Let MULTI_DTB_FIT take priority in this case, by
> > explicitly blocking the override when MULTI_DTB_FIT is enabled.
> > 
> > Link: https://github.com/polarfire-soc/hart-software-services/blob/master/tools/hss-payload-generator/README.md [1]
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > 
> > v2:
> > - implement MULTI_DTB_FIT too
> > 
> > v1 was a year ago here:
> > https://lore.kernel.org/u-boot/20240625090806.1787287-2-conor.dooley@microchip.com/
> > 
> > There was some discussion on that thread, but ultimately I didn't find
> > what was mentioned to be worth implementing. Heinrich suggested to me, I
> > guess on IRC or something since it's not in that thread, implementing
> > MULTI_DTB_FIT, which I have done. A lot of the reason for the patch when
> > I wrote it was speculative, but we have some actual users for OF_BOARD
> > now - hence the v2 after about a year's gap.
> > 
> > CC: Ivan Griffin <ivan.griffin@microchip.com>
> > CC: Cyril Jean <cyril.jean@microchip.com>
> > CC: Tom Rini <trini@konsulko.com>
> > CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > CC: Simon Glass <sjg@chromium.org>
> > CC: Jamie.Gibbons@microchip.com
> > CC: u-boot@lists.denx.de
> > ---
> >  board/microchip/mpfs_icicle/mpfs_icicle.c | 63 +++++++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> 
> Acked-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Hmm, I made a mistake in the MULTI_DTB_FIT portion of this, and the
strtok() is causing problems (I forgot about the modification in place,
and the testcases I had used didn't catch it). Have you sent this in a
PR yet? If you have, I'll send a fix otherwise a v3.

Cheers,
Conor.

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

Re: [PATCH v2] board: mpfs_icicle: implement board_fdt_blob_setup()/board_fit_config_name_match()

[Leo Liang]

Hi Conor,

On Fri, Jul 04, 2025 at 11:40:21AM +0100, Conor Dooley wrote:
> [EXTERNAL MAIL]

> Date: Fri, 4 Jul 2025 11:40:21 +0100
> From: Conor Dooley <conor.dooley@microchip.com>
> To: Leo Liang <ycliang@andestech.com>
> CC: Conor Dooley <conor@kernel.org>, u-boot@lists.denx.de, Ivan Griffin
>  <ivan.griffin@microchip.com>, Cyril Jean <cyril.jean@microchip.com>, Tom
>  Rini <trini@konsulko.com>, Ilias Apalodimas <ilias.apalodimas@linaro.org>,
>  Simon Glass <sjg@chromium.org>, Jamie.Gibbons@microchip.com
> Subject: Re: [PATCH v2] board: mpfs_icicle: implement
>  board_fdt_blob_setup()/board_fit_config_name_match()
> 
> On Thu, Jul 03, 2025 at 06:09:53PM +0800, Leo Liang wrote:
> > On Mon, Jun 23, 2025 at 12:51:36PM +0100, Conor Dooley wrote:
> > > From: Conor Dooley <conor.dooley@microchip.com>
> > > 
> > > The firmware on the Icicle is capable of providing a devicetree in a1 to
> > > U-Boot, but until now the devicetree has been packaged in a "payload" [1]
> > > alongside U-Boot (or other bootloaders/RTOSes) and appended to the image.
> > > The address of this appended devicetree is placed in a1 by the firmware.
> > > This meant that the mechanism used by OF_SEPARATE to locate the
> > > devicetree at the end of the image would pick up the one provided by the
> > > firmware when u-boot-nodtb.bin was in the payload and U-Boot's devicetree
> > > when u-boot.bin was.
> > > 
> > > The firmware is now going to be capable of providing a minimal devicetree
> > > (quite cut down due to severe space constraints), but this devicetree is
> > > linked into the firmware that runs out of the L2 rather than at the end
> > > of the U-Boot image.
> > > 
> > > Implement board_fdt_blob_setup() so that this devicetree can be
> > > optionally used, and the devicetree provided in the "payload" can be
> > > used without relying on "happening" to implement the same strategy as
> > > OF_SEPARATE expects in combination with u-boot-nodtb.bin.
> > > Unlike other RISC-V boards, the firmware provided devicetree is only
> > > used when OF_BOARD is set, so that the almost certainly more complete
> > > devicetree in U-Boot will be used unless explicitly requested otherwise.
> > > 
> > > Implement board_fit_config_name_match(), so that, using the firmware
> > > provided cut-down/minimal dtb, U-Boot can select one of several
> > > devicetrees when MULTI_DTB_FIT is enabled.
> > > 
> > > Enabling both MULTI_DTB_FIT and OF_BOARD will lead to a conflict
> > > between the two options, with the latter taking priority due to
> > > board_fdt_blob_setup() being executed before board_fit_config_name_match(),
> > > which causes gd->fdt_blob to be overwritten with a pointer to the
> > > minimal devicetree rather than the location of the fit image containing
> > > the multiple dtbs. Let MULTI_DTB_FIT take priority in this case, by
> > > explicitly blocking the override when MULTI_DTB_FIT is enabled.
> > > 
> > > Link: https://github.com/polarfire-soc/hart-software-services/blob/master/tools/hss-payload-generator/README.md [1]
> > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > ---
> > > 
> > > v2:
> > > - implement MULTI_DTB_FIT too
> > > 
> > > v1 was a year ago here:
> > > https://lore.kernel.org/u-boot/20240625090806.1787287-2-conor.dooley@microchip.com/
> > > 
> > > There was some discussion on that thread, but ultimately I didn't find
> > > what was mentioned to be worth implementing. Heinrich suggested to me, I
> > > guess on IRC or something since it's not in that thread, implementing
> > > MULTI_DTB_FIT, which I have done. A lot of the reason for the patch when
> > > I wrote it was speculative, but we have some actual users for OF_BOARD
> > > now - hence the v2 after about a year's gap.
> > > 
> > > CC: Ivan Griffin <ivan.griffin@microchip.com>
> > > CC: Cyril Jean <cyril.jean@microchip.com>
> > > CC: Tom Rini <trini@konsulko.com>
> > > CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > CC: Simon Glass <sjg@chromium.org>
> > > CC: Jamie.Gibbons@microchip.com
> > > CC: u-boot@lists.denx.de
> > > ---
> > >  board/microchip/mpfs_icicle/mpfs_icicle.c | 63 +++++++++++++++++++++++
> > >  1 file changed, 63 insertions(+)
> > 
> > Acked-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> 
> Hmm, I made a mistake in the MULTI_DTB_FIT portion of this, and the
> strtok() is causing problems (I forgot about the modification in place,
> and the testcases I had used didn't catch it). Have you sent this in a
> PR yet? If you have, I'll send a fix otherwise a v3.

The PR has been sent and merged.
Could you send a fix for this?

Best regards,
Leo

> 
> Cheers,
> Conor.

Re: [PATCH v2] board: mpfs_icicle: implement board_fdt_blob_setup()/board_fit_config_name_match()

[Conor Dooley]

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

On Fri, Jul 04, 2025 at 07:01:22PM +0800, Leo Liang wrote:
> 
> The PR has been sent and merged.
> Could you send a fix for this?


Yup, it may be Monday before I send it though.

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