Am 13. September 2025 17:18:07 MESZ schrieb Tom Rini <tr...@konsulko.com>: >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.
I am fine with the current patch if you supply the follow up. > >> > + >> > +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. > It is unclear what the command does due to the incompleteness of this man-page. Is it copying the device-tree? Is it creating ATAGs? Is it adding ATAGs to a device-tree copy? >> >> > + => 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. All of these impact the spl command. Best regards Heinrich > >> 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.". >
