Hi Connor,
On Thu, 27 Jun 2024 at 10:38, Conor Dooley conor.dooley@microchip.com wrote:
On Thu, Jun 27, 2024 at 09:36:49AM +0100, Simon Glass wrote:
On Tue, 25 Jun 2024 at 15:34, Tom Rini trini@konsulko.com wrote:
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
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. */
I'm adding in Simon and Ilias as this touches on one of those frequent topics about how device trees can/should be passed along to us.
The only thing I can think of is implementing bloblist in the a1 (?) firmware, then passing the DT in that.
a1 is the register that is used on riscv to pass the dtb, I think the corresponding thing on arm64 is x0.
Re-reading the firware handoff spec, it's difficult to see what benefits it actually provides us when we only ever have a single dtb which the firmware does not interact with/use. We are super space constrained in the firmware even carving out 4.5 KiB for a devicetree blob is a stretch and requires disabling other features and ripping out anything in the DT not required for U-Boot to load the OS. Even the ~1 KiB mentioned in bloblist.h for handling a bloblist would be a challenge.
I think the only way a bloblist could work is if it was created at build time and linked into the firmware, since the on-disk format seems pretty minimal. Is there tooling for generating a bloblist at build time that I could use to check whether or not a bloblist is viable? I'd also have to investigate how that would interact with OpenSBI, since it's integrated into the firmware and involved with loading U-Boot.
There is not such a tool, but it would be easy enough to write. If you think that would help, I could give it a try.
The confusing thing for me is that the 'firmware' is very constrained...but are you saying that U-Boot is not? Why pass U-Boot the DT? Perhaps you could explain the boot sequence so I can understand it better?
It seems that you still need to be able to turn that on and off in U-Boot though. So far we have not agreed the mechanism to do that, I have the same problem, with a pending patch here[1]
It seems your patch is trying to do some runtime determination of whether to examine the bloblist or not, but the ?existing? build-time check for BLOBLIST being enabled would work equally well/poorly as the OF_BOARD check the code I am adding. I'm not even really sure what runtime option could be used here here to check if the passed dtb/bloblist was to be used. U-Boot only runs here as supervisor mode U-Boot proper and always has a more complete devicetree. Whether to use the one passed to U-Boot just depends on what the person with the board wants to do - which, given this is an FPGA, could be vary significantly.
OK...so does this platform use SPL?
Regards, Simon