On Sat, Sep 13, 2025 at 02:32:26AM +0200, Heinrich Schuchardt wrote: > On 9/13/25 01:02, Tom Rini wrote: > > This converts the existing README.commands.spl file to rST as > > doc/usage/cmd/spl.rst and adds it to the command index. Minimal > > rewording is done to the previous contents, to make it fit better with > > the overall style of documentation now. > > > > Signed-off-by: Tom Rini <tr...@konsulko.com> > > --- > > Cc: Heinrich Schuchardt <xypron.g...@gmx.de> > > --- > > doc/README.commands.spl | 31 -------------------------- > > doc/usage/cmd/spl.rst | 48 +++++++++++++++++++++++++++++++++++++++++ > > doc/usage/index.rst | 1 + > > 3 files changed, 49 insertions(+), 31 deletions(-) > > delete mode 100644 doc/README.commands.spl > > create mode 100644 doc/usage/cmd/spl.rst > > > > diff --git a/doc/README.commands.spl b/doc/README.commands.spl > > deleted file mode 100644 > > index ecfd3ca9ee58..000000000000 > > --- a/doc/README.commands.spl > > +++ /dev/null > > @@ -1,31 +0,0 @@ > > -The spl command is used to export a boot parameter image to RAM. Later > > -it may implement more functions connected to the SPL. > > - > > -SUBCOMMAND EXPORT > > -To execute the command everything has to be in place as if bootm should be > > -used. (kernel image, initrd-image, fdt-image etc.) > > - > > -export has two subcommands: > > - atags: exports the ATAGS > > - fdt: exports the FDT > > - > > -Call is: > > -spl export <fdt|atags> [kernel_addr] [initrd_addr] [fdt_addr if fdt] > > - > > - > > -TYPICAL CALL > > - > > -on OMAP3: > > -nandecc hw > > -nand read 0x82000000 0x280000 0x400000 /* Read kernel image from NAND*/ > > -spl export atags /* export ATAGS */ > > -nand erase 0x680000 0x20000 /* erase - one page */ > > -nand write 0x80000100 0x680000 0x20000 /* write the image - one page */ > > - > > -call with FDT: > > -nandecc hw > > -nand read 0x82000000 0x280000 0x400000 /* Read kernel image from NAND*/ > > -tftpboot 0x80000100 devkit8000.dtb /* Read fdt */ > > -spl export fdt 0x82000000 - 0x80000100 /* export FDT */ > > -nand erase 0x680000 0x20000 /* erase - one page */ > > -nand write <adress shown by spl export> 0x680000 0x20000 > > diff --git a/doc/usage/cmd/spl.rst b/doc/usage/cmd/spl.rst > > new file mode 100644 > > index 000000000000..59471c65a623 > > --- /dev/null > > +++ b/doc/usage/cmd/spl.rst > > @@ -0,0 +1,48 @@ > > +.. SPDX-License-Identifier: GPL-2.0+: > > %s/GPL-2.0+/GPL-2.0-OR-LATER/ > > Cf. https://spdx.org/licenses/GPL-2.0-or-later.html
Yes, and it's not a hard requirement to use the new tag, checkpatch.pl doesn't even complain. I got this from the existing command I copied, FWIW. > > > + > > +.. index:: > > + single: spl (command) > > + > > +spl command > > +=========== > > + > > +Synopsis > > +-------- > > + > > +:: > > + > > + spl export <fdt|atags> [kernel_addr] [initrd_addr] [fdt_addr if fdt] > > Those brackets cannot be correct. I definitively cannot remove the > kernel_addr and provide an initrd_addr instead. Do you mean: > > spl export <fdt|atags> [kernel_addr [initrd_addr [fdt_addr if fdt]]] It's what the command itself says currently, however. I'd be happy to do a follow-up patch to fix both. > > + > > +Description > > +----------- > > + > > +The *spl* command is used to export a boot parameter image to RAM. Later > > In the current state the man-page is not very helpful. Please, move the more > extensive description from doc/develop/falcon.rst here. As I said in the commit message, it's a conversion with minimal changes. I'd be happy to do a follow-up however improving it (and making cmd/spl.c's help text also better/correct). > * I would not know what a "boot parameter image" might be. Please, explain > it for a novice user. > * Please, describe what each parameter is used for. > * Please, explain what atags refers to. I would assume you mean what is > described in > https://developer.arm.com/documentation/den0013/0400/Boot-Code/Booting-Linux/Kernel-parameters-using-ATAGs. > But don't expect a novice user to know. > * What does the command do if "fdt" is used? > > > +it may implement more functions connected to the SPL. > > + > > +spl export > > +~~~~~~~~~~ > > This heading makes no sense. I just copied doc/usage/cmd/acpi.rst for format as it's the first file. I'd be happy to look at a better example. > > +To execute the command everything has to be in place as if bootm should be > > Bootm can for instance boot a FIT image with an EFI binary. > I wouldn't expect this to work when booting in Falcon mode where EFI is not > available. > > This sentence should be replaced by a meaningful description. I don't understand what's unclear, sorry. And yes, Falcon mode and EFI aren't compatible, but that should be spelled out in doc/develop/falcon.rst and not here I think. > > > +used. (kernel image, initrd-image, fdt-image if used). The first argument > > +passed to the export subcommand must be either atags or fdt, which > > signifies if > > %s/signifies/indicates/ > > > +we are preparing an ATAGS or device tree for export. > > Isn't ATAGs the plural form of ATAG? The most consistent usage is "ATAGS" and not "ATAGs". > Why would you write 'an ATAGS' if there are multiple tags? > > Please, describe under which circumstances and with which values these > environment variables are filled and what they are used for. > > * fdtargsaddr > * fdtargslen These are covered in doc/develop/falcon.rst which should be linked from this document I think. > > + > > +Example > > +------- > > + > > +On an OMAP3 platform with ATAGS:: > > + > > + => nandecc hw > > + => nand read 0x82000000 0x280000 0x400000 /* Read kernel image > > from NAND*/ > > What format must the image have to be used for booting? A FIT image? > > > + => spl export atags /* export ATAGS */ > > Is spl export atags never used with kernel_addr, initrd_addr, fdt_addr? > > Does `spl export atags` without further arguments create any ATAGs? > Where would it take the values from? Working with ATAGS was indeed "fun". I'd honestly rather follow-up and remove ATAGS support from Falcon mode since it's 2025 and not 2012, but the point of this patch is to move doc/README.* content to rST so that we can then go "oof, that needs work" and try and improve it. > > + => nand erase 0x680000 0x20000 /* erase - one page */ > > + => nand write 0x80000100 0x680000 0x20000 /* write the image - > > one page */ > > What is written to nand here? > What relevance have these magic addresses? > How would a user know which address to use? More of the magic of ATAGS, which we can be glad aren't relevant anymore. It's always at that magic 0x100 offset. And the "on OMAP3" is the key to decoding the rest. > > +On an OMAP3 platform with device tree:: > > + > > + => nandecc hw > > + => nand read 0x82000000 0x280000 0x400000 /* Read kernel image > > from NAND*/ > > + => tftpboot 0x80000100 devkit8000.dtb /* Read fdt */ > > + => spl export fdt 0x82000000 - 0x80000100 /* export FDT */ > > Why would anybody use a device-tree from a TFTP server in conjunction with a > kernel from nand? It was the example at the time. But, because there's not a device tree on NAND, that's what spl export is creating. > > > + => nand erase 0x680000 0x20000 /* erase - one page */ > > + => nand write <adress shown by spl export> 0x680000 0x20000 > > The address seems to be gd->bd->bi_boot_params which is hard coded in board > files and not visible to the user. This is just gruesome. The address should > be exported to a variable to be usable in scripts. Preparing a board for falcon mode isn't a general task, you need to know what your platform is and where things are because you're about to make it a lot less flexible. > Which customizing is needed to use the spl command? These settings seem to > be relevant and should be described in sufficient detail: > > CONFIG_CMD_SPL > CONFIG_OF_LIBFDT > CONFIG_CMD_SPL_WRITE_SIZE > CONFIG_SUPPORT_PASSING_ATAGS > CONFIG_SYS_BOOT_RAMDISK_HIGH Some of these I suspect so, yes. > Which return values does the command provide? Only the standard ones. A quick grep on "on success" in doc/usage/cmd says we aren't consistent about this phrasing nor inclusion to start with. I think we should update the "General rules" section of doc/usage/cmdline.rst to state that "The return value $? is 0 (true) on success and 1 (false) on failure unless otherwise noted.". -- Tom
signature.asc
Description: PGP signature
