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/hs... [1] Signed-off-by: Conor Dooley conor.dooley@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.griffin@microchip.com CC: Padmarao Begari padmarao.begari@microchip.com CC: Cyril Jean cyril.jean@microchip.com CC: Tom Rini trini@konsulko.com CC: Conor Dooley conor.dooley@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