[U-Boot] [PATCH] sunxi: add spi flash bootcmd

[Andre Przywara]

On Mon, 7 Jan 2019 19:47:26 +0200
Oskari Lemmelä <oskari at lemmela.net> wrote:

> On 7.1.2019 15.54, Andre Przywara wrote:
> > On Mon, 7 Jan 2019 14:16:23 +0100
> > Maxime Ripard <maxime.ripard at bootlin.com> wrote:
> >  
> >> On Sat, Jan 05, 2019 at 08:03:46PM +0200, Oskari Lemmela wrote:  
> >>> Enable SPI bootcmd if CONFIG_DM_SPI_FLASH is defined.
> >>>
> >>> Signed-off-by: Oskari Lemmela <oskari at lemmela.net>
> >>> ---
> >>>  include/configs/sunxi-common.h | 16 ++++++++++++++++
> >>>  1 file changed, 16 insertions(+)
> >>>
> >>> diff --git a/include/configs/sunxi-common.h
> >>> b/include/configs/sunxi-common.h index 9819d9980c..b2443ef678
> >>> 100644 --- a/include/configs/sunxi-common.h
> >>> +++ b/include/configs/sunxi-common.h
> >>> @@ -401,6 +401,21 @@ extern int soft_i2c_gpio_scl;
> >>>  #define BOOT_TARGET_DEVICES_USB(func)
> >>>  #endif
> >>>  
> >>> +#ifdef CONFIG_DM_SPI_FLASH
> >>> +#define BOOT_TARGET_DEVICES_SPI(func) func(SPI, spi, 0)
> >>> +#define BOOTENV_DEV_SPI(devtypeu, devtypel, instance) \    
> >> This belongs more in include/config_distro_bootcmd.h, there's
> >> nothing really sunxi specific here.  
> True. I'll try to rework it that way.

I don't think it belongs there. As the name says, this is for distros
to find their kernel, using all kind of standards: boot partitions,
magic filenames, config files, etc.
Loading from some made up address from SPI flash doesn't belong there.

> >>> +	"image_addr=0x100000\0" \
> >>> +	"image_size=0x700000\0" \    
> >> image_addr and image_size usually come from Kconfig, instead of
> >> being hardcoded  
> > Actually this whole approach looks very specific to me, if I
> > understand this correctly, it loads 7 MB from 1MB of the SPI flash
> > and treats this as a kernel?
> > This looks very arbitrary to me, requires at least 8MB of SPI flash
> > (many boards come with just 2MB) and will never hold any normal
> > arm64 kernel (which are in the range of 16MB, typically).
> > So this might fit some specific embedded project, but doesn't look
> > like something we want in the default environment. If there is a
> > use case for it, people can always either set up their specific
> > environment (and store it in FAT), or change the default
> > environment just for their build.
> >
> > Cheers,
> > Andre.  
> Making image_addr and image_size configurable via Kconfig would allow
> this be generic approach.

It would still not be generic, but I am fine with this if the whole
thing is protected by Kconfig: So it only probes SPI flash if there was
a flash load address specified at compile time. And by default there
should be none, as I still consider this a special use case.
In an ideal world the SPI flash would be owned by firmware, so we could
put EFI applications (EFI shell, grub) on there, for instance.

Can you also add some documentation on this feature somewhere?

> Goal is not load kernel image directly as it would be too big for most
> of SPI
> flash devices. Approach is load FIT image some where else and then let
> u-boot
> uncompress and boot it.
> 
> Most likely 2MB flash device is not enough even when lzma compressing
> kernel.
>  
> >>> +	"bootcmd_" #devtypel #instance "=" \
> >>> +		"if sf probe " #instance "; then " \
> >>> +			"sf read ${ramdisk_addr_r} ${image_addr}
> >>> ${image_size}; " \    
> >> s/ramdisk_addr_r/kernel_addr_r/  
> Reason explained earlier boot process would be loading FIT image to
> ramdisk_addr_r. Then u-boot will decompress kernel, dtb and ramdisk to
> memory
> and boot kernel.
> 
> Or could it be better not to use ramdisk_addr_r and replace it with
> some other configurable memory address?

I guess you need to, otherwise extracting the ramdisk would overwrite
the image, wouldn't it?

Does bootm detect the ramdisk in a FIT image and does the right thing
automatically?

Cheers,
Andre.

> Example boot log:
> 
> => sf probe 0            
> SF: Detected w25q128bv with page size 256 Bytes, erase size 4 KiB,
> total 16 MiB
> => sf read ${ramdisk_addr_r} ${image_addr} ${image_size}  
> device 0 offset 0x100000, size 0x700000
> SF: 7340032 bytes @ 0x100000 Read: OK
> => bootm ${ramdisk_addr_r}  
> ## Loading kernel from FIT Image at 4fe00000 ...
>    Using 'config at 1' configuration
>    Trying 'kernel at 1' kernel subimage
>      Description:  ARM64 Linux-4.19.13
>      Type:         Kernel Image
>      Compression:  lzma compressed
>      Data Start:   0x4fe000e8
>      Data Size:    2812412 Bytes = 2.7 MiB
>      Architecture: AArch64
>      OS:           Linux
>      Load Address: 0x40080000
>      Entry Point:  0x40080000
>      Hash algo:    crc32
>      Hash value:   27f9179d
>      Hash algo:    sha1
>      Hash value:   f5e9b16f6958cca20013f06074ad85141ccd3cc6
>    Verifying Hash Integrity ... crc32+ sha1+ OK
> ## Loading fdt from FIT Image at 4fe00000 ...
>    Using 'config at 1' configuration
>    Trying 'fdt at 1' fdt subimage
>      Description:  ARM64 sun50i-a64-pine64-lts device tree blob
>      Type:         Flat Device Tree
>      Compression:  uncompressed
>      Data Start:   0x500aec2c
>      Data Size:    13158 Bytes = 12.8 KiB
>      Architecture: AArch64
>      Hash algo:    crc32
>      Hash value:   3d2574b7
>      Hash algo:    sha1
>      Hash value:   c55ea000ca7925a505a5de69425b49d8092ceb5e
>    Verifying Hash Integrity ... crc32+ sha1+ OK
>    Booting using the fdt blob at 0x500aec2c
> EHCI failed to shut down host controller.
> EHCI failed to shut down host controller.
>    Uncompressing Kernel Image ... OK
>    Loading Device Tree to 0000000049ff9000, end 0000000049fff365 ...
> OK
> 
> Starting kernel ...
> 
> [    0.000000] Booting Linux on physical CPU 0x0
> >>
> >> Thanks!
> >> Maxime
> >>  
> 
> Thanks!
> Oskari