On Tue, Jun 25, 2024 at 10:08:06AM +0100, Conor Dooley wrote: > 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. > > Link: > https://github.com/polarfire-soc/hart-software-services/blob/master/tools/hss-payload-generator/README.md > [1] > Signed-off-by: Conor Dooley <conor.doo...@microchip.com>
Off-list it was suggested to me to use MULTI_DTB_FIT in addition what
what I've done here, to allow U-Boot to actually make use of the
information in the firmware provided dtb to select a more complete dtb
for the OS (or for itself). I whipped up the following quickly just to
test that it works, but was super lazy about it as you can see:
diff --git a/board/microchip/mpfs_icicle/mpfs_icicle.c
b/board/microchip/mpfs_icicle/mpfs_icicle.c
index ade150bec98..76f37a7199c 100644
--- a/board/microchip/mpfs_icicle/mpfs_icicle.c
+++ b/board/microchip/mpfs_icicle/mpfs_icicle.c
@@ -52,6 +52,31 @@ static void read_device_serial_number(u8 *response, u8
response_size)
response_buf[idx] = readb(MPFS_SYS_SERVICE_MAILBOX + idx);
}
+#ifdef CONFIG_MULTI_DTB_FIT
+int board_fit_config_name_match(const char *name)
+{
+
+ char compat[256] = "microchip,";
+ size_t max = 256 - strlen("microchip,");
+ int ret;
+
+ /*
+ * 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;
+
+ strncat(compat, name, max);
+ ret = fdt_node_check_compatible((void *)gd->arch.firmware_fdt_addr, 0,
compat);
+ if (ret)
+ return -EINVAL;
+
+ debug("found a match for compat: %s\n", compat);
+ return 0;
+}
+#endif
+
void *board_fdt_blob_setup(int *err)
{
*err = 0;
I'd rather invert the logic so that we compare the name of the config
with the compatible strings sans vendor prefix - what's here would work
for any of the boards were we are the vendor but not for the
beaglev-fire. Granted that board is not supported in U-Boot right now,
but I'll probably accompany a v2 of this that with an OF_UPSTREAM
conversion for the PolarFire SoC boards.
However, I don't really understand how I could make my implementation of
board_fdt_blob_setup() play nicely. It appears that if I enable OF_BOARD
then then the multi dtb stuff never kicks in (since we've updated the
fdt pointer to that of the firmware provided dtb). Given that
setup_multi_dtb_fit() is called almost immediately after
board_fdt_blob_setup() in fdtdec_setup(), it seems like dropping
board_fdt_blob_setup() entirely might be a better approach?
I think that'll have the same behaviours we want w.r.t. firmware config
options etc - except U-Boot will have to be built with functional
devicetrees for all boards it wants to support. Maybe that's not a
problem since it'd still be a single build for all boards. Maybe
Heinrich has a better opinion there than I do..
Thanks,
Conor.
> ---
> CC: Ivan Griffin <ivan.grif...@microchip.com>
> CC: Padmarao Begari <padmarao.beg...@microchip.com>
> CC: Cyril Jean <cyril.j...@microchip.com>
> CC: Tom Rini <tr...@konsulko.com>
> CC: Conor Dooley <conor.doo...@microchip.com>
> CC: u-boot@lists.denx.de
> ---
> board/microchip/mpfs_icicle/mpfs_icicle.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/board/microchip/mpfs_icicle/mpfs_icicle.c
> b/board/microchip/mpfs_icicle/mpfs_icicle.c
> index 4d7d843dfa3..2c1f7175f0e 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,24 @@ static void read_device_serial_number(u8 *response, u8
> response_size)
> response_buf[idx] = readb(MPFS_SYS_SERVICE_MAILBOX + idx);
> }
>
> +void *board_fdt_blob_setup(int *err)
> +{
> + *err = 0;
> + /*
> + * 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)) {
> + if (gd->arch.firmware_fdt_addr)
> + return (ulong *)(uintptr_t)gd->arch.firmware_fdt_addr;
> + }
> +
> + return (ulong *)_end;
> +}
> +
> int board_init(void)
> {
> /* For now nothing to do here. */
> --
> 2.43.2
>
signature.asc
Description: PGP signature
