[U-Boot] [PATCH 00/13] System Firmware Loader for TI K3 family SoCs

[dannenberg at ti.com <dannenberg@ti.com>]

Hi TF,
thanks for chiming in. Comments inlined...

On Wed, May 08, 2019 at 04:31:35AM +0000, Chee, Tien Fong wrote:
> On Tue, 2019-05-07 at 22:00 +0200, Simon Goldschmidt wrote:
> > 
> > On 07.05.19 19:25, Andreas Dannenberg wrote:
> > > 
> [...]
> > > 
> > > While I also have a working solution based on the existing FS
> > > loader
> > > framework this has its own challenges, namely by its very nature
> > > only
> > > addressing a subset of our use cases (no eMMC/SD RAW boot support
> > > for
> > > example), 
> 
> IMO, it's actually not that hard to enhance RAW support, i think
> minimal changes are required. I have attached the patches about an
> example of loading RAW from QSPI that i have done locally last few week
> ago.

As your patches show, no it's not hard, it's more or less taking pieces
from the SPL loader framework and refactoring them into the FS loader,
creating a good and universal solution usable across SPL and U-Boot in
environments that are not tightly constrained in terms of memory.

What I was going after is finding a way to load from different media
"pre-relocation" SPL (board_init_f), with almost no memory available,
where I have to agonize over every single KB available.

> > > being heavier on resource usage (needing to use ENV to pass
> > > parameters),
> 
> ENV is optional, you can use DTS.

Is it? I had to update the FS loader framework when I experimented with
it, please see attached patch. I had refactored it such that I can pass
in all relevant data via platform data for the intial boot mode I was
going after, so that I can dynamically configure it on the fly from
early SPL board_init_f() based on boot media / boot mode, etc.

> For example loading FPGA bitstream from QSPI RAW:
> 
> /* DTS */
> / {
> +	aliases {
> +		spi0 = &qspi;
> +	};
> +
> +	fs_loader0: fs-loader {
> +		u-boot,dm-pre-reloc;
> +		compatible = "u-boot,fs-loader";
> +		sfconfig = <0 0 100000000 3>;
> +	};
> +};
> +
> +&fpga_mgr {
> +	u-boot,dm-pre-reloc;
> +	firmware-loader = <&fs_loader0>;
> +	altr,bitstream = "300000";
> +};

The above hard-codes and duplicates information that is already known to
U-Boot (CONFIG_SF_DEFAULT_*), and will do more of the same as this is
being extended. How does one keep this consistent?

And how does this scale to support like 5 different boot modes using a
single DTB? I guess one  could populate 5 nodes, and then pick one based
on boot mode during SPL execution, by extending the probe function
accordingly.

> 
> > > and not addressing the need to probe the boot peripheral.
> 
> You can add more different probing method in function called
> "fs_loader_probe". Current fs_loader supports block(sdmmc, emmc, etc...) probing, and with
> the patches attached support QSPI probing.
> 
> Another idea come to mind, we can use fs_loader for loading FIT boot
> image into RAM, and boot from RAM with existing SPL loader framework,
> but i'm not sure this implementation fit to your use case?

Unfortunately for what I'm working on there is no space for this. It
would be nice to be able to load our "System Firmware" alongside the
next stage of U-Boot in a single FIT, and then just extact that firmware
image similar to what CONFIG_SPL_FPGA_SUPPORT does in spl_fit.c. 
However I must load SYSFW and the U-Boot next stage as two steps
(while bringing up DDR in-between....).

As I tried to explain in my earlier email to Simon, I almost see the
minimally-intrusive extensions I did to the SPL loader framework via
"spl: Make image loader infrastructure more universal" and the FS loader
framework to be elements that are orthogonal and can both exist. Maybe
FS loader can partially build upon the SPL loader foundation as well?

--
Andreas Dannenberg
Texas Instruments Inc


> 
> > > This particular framework works well for use cases requiring to
> > > load
> > > firmware from FS-based media once DDR is up and U-Boot is in a more
> > > "initialized" state but it is not a one-fits all solution for very
> > > early use in SPL board_init_f() accross different boot modes.
> > And would it be an option to improve the loader (maybe dropping the
> > "fs" 
> > from its name)? I think it's an "fs" loader because its idea has
> > been 
> > copied from Linux. I think in U-Boot, it's more common to have things
> > at 
> > a raw offset instead of a file system. Just thinking...
> 
> Current fs_loader only support filesystem, and i agree that it made
> sense to remove the "fs" once it supports the RAW offset as well.
> 
> Thanks.
> 
> Regards,
> TF
> 
> > 
> > And the current state of that fs_loader is like it is because it
> > fits 
> > its single user (socfpga stratix 10), I think.
> > 
> > Anyway, even if you do need yet another loader, would it make sense
> > to 
> > create a common file instead of adding this in your arch/mach?
> > 
> > Regards,
> > Simon
> > 
> > > 
> > > 
> > > Andreas Dannenberg (10):
> > >    mmc: k3_arasan: Allow driver to probe without PDs specified
> > >    spl: Allow skipping clearing BSS during relocation
> > >    spl: Make image loader infrastructure more universal
> > >    arm: K3: Introduce System Firmware loader framework
> > >    armV7R: K3: am654: Allow using SPL BSS pre-relocation
> > >    armv7R: K3: am654: Use full malloc implementation in SPL
> > >    armV7R: K3: am654: Load SYSFW binary and config from boot media
> > >    configs: am65x_evm_r5: All sysfw to be loaded via MMC
> > >    configs: am65x_hs_evm_r5: All sysfw to be loaded via MMC
> > >    configs: am65x_hs_evm: Add Support for eMMC boot
> > > 
> > > Faiz Abbas (2):
> > >    configs: am65x_evm: Add Support for eMMC boot
> > >    am65x: README: Add eMMC layout and flash instructions
> > > 
> > > Lokesh Vutla (1):
> > >    armv7R: dts: k3: am654: Update mmc nodes for loading sysfw
> > > 
> > >   arch/arm/dts/k3-am654-r5-base-board.dts      |  18 ++
> > >   arch/arm/lib/crt0.S                          |   3 +
> > >   arch/arm/mach-k3/Kconfig                     |  40 +++
> > >   arch/arm/mach-k3/Makefile                    |   1 +
> > >   arch/arm/mach-k3/am6_init.c                  |  34 ++-
> > >   arch/arm/mach-k3/include/mach/sysfw-loader.h |  12 +
> > >   arch/arm/mach-k3/sysfw-loader.c              | 263
> > > +++++++++++++++++++
> > >   board/ti/am65x/Kconfig                       |   1 +
> > >   board/ti/am65x/README                        |  52 ++++
> > >   common/spl/Kconfig                           |  13 +
> > >   common/spl/spl_fit.c                         |  14 +
> > >   common/spl/spl_mmc.c                         |  76 ++++--
> > >   configs/am65x_evm_a53_defconfig              |   2 +
> > >   configs/am65x_evm_r5_defconfig               |   7 +-
> > >   configs/am65x_hs_evm_a53_defconfig           |   2 +
> > >   configs/am65x_hs_evm_r5_defconfig            |   7 +-
> > >   drivers/mmc/k3_arsan_sdhci.c                 |  16 +-
> > >   include/configs/am65x_evm.h                  |  30 ++-
> > >   include/spl.h                                |  26 ++
> > >   19 files changed, 577 insertions(+), 40 deletions(-)
> > >   create mode 100644 arch/arm/mach-k3/include/mach/sysfw-loader.h
> > >   create mode 100644 arch/arm/mach-k3/sysfw-loader.c
> > > 



-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-misc-fs_loader-Allow-initializing-blkdev-using-platf.patch
Type: text/x-diff
Size: 2922 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190508/1e439629/attachment.patch>