Hi Connor, On Thu, 27 Jun 2024 at 10:38, Conor Dooley <conor.doo...@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 <tr...@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/hss-payload-generator/README.md > > > > [1] > > > > Signed-off-by: Conor Dooley <conor.doo...@microchip.com> > > > > --- > > > > 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. */ > > > > > > 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
