[PATCH RFC 00/15] Support SPI NAND booting on the T113

[Sam Edwards]

On Wed, Apr 10, 2024 at 10:26 PM John Watts <contact at jookia.org> wrote:
>
> This series is my current working and tested setup for booting from
> SPI NAND chips on the Allwinner T113.
>
> I have included the following patches from others. I may have modified
> them to work with the latest mainline:
>
> https://lore.kernel.org/all/20221014030520.3067228-1-uwu@icenowy.me/
> https://lore.kernel.org/all/20231111133432.755363-2-bigunclemax@gmail.com/
>
> Hopefully this can get the ball rolling on how to properly implement
> SPI NAND support in mainline U-Boot.
>
> Signed-off-by: John Watts <contact at jookia.org>

Hi John,

It doesn't look like I was sent the whole series (only 00 and 01), but
I was able to find it on Patchwork and sift through it. A few general
comments follow:

The introduction of `SUNXI_BOOTED_FROM_SPINAND` is the right call,
since the newer sunxis use this for SPI-NAND, and
`SUNXI_BOOTED_FROM_SPI` for SPI-NOR. The older sunxis, however, will
use `SUNXI_BOOTED_FROM_SPI` for "it's either SPI-NAND or SPI-NOR, have
fun figuring out which." While the rationale in 09/15 ("Instead of
trying to boot from SPI NAND then SPI NOR in series, select one based
on the current boot device.") is solid, we still need some code
(perhaps in/called from `sunxi_get_boot_device`?) to handle the
`SUNXI_BOOTED_FROM_SPI` case by performing flash type detection --
disabled for those sunxis known to use `SUNXI_BOOTED_FROM_SPINAND`, of
course.

06/15 ("sunxi: enable support for SPI NAND booting on SUNIV") should
be dropped from the series. You are updating `suniv_get_boot_source`
when introducing `SUNXI_BOOTED_FROM_SPINAND` anyway, so you can just
hook up `SUNIV_BOOTED_FROM_NAND` at that time (and a heads-up: I think
you got it wired backwards in this version of the series).

On a more fundamental note: I am hesitant about the overall approach
of having NAND reading code in `arch/arm/mach-sunxi/spl_spi_sunxi.c`.
NANDs are more complex than NORs (requiring e.g. bad block handling)
and U-Boot generally keeps the SPL load methods in
`common/spl/spl_*.c`. It's good that the UBI-on-SPI-NAND method is
hooked up there, but the "U-Boot image follows the (good) blocks of
the SPL in NAND" method should probably live in the same directory.

Here's what I'd like to explore: can we introduce a
`common/spl/spl_spinand.c` and update the `drivers/mtd/nand/spi/*.c`
code to support running in SPL? This way
`arch/arm/mach-sunxi/spl_spi_sunxi.c` must only provide the SPL-mode
implementation of SPI -- that is, the actual sunxi-specific bit. Also,
updating the `drivers/mtd/nand/spi/*.c` drivers for SPL-friendliness
will mean dropping those `SPL_SPINAND_{PAGE,BLOCK}_SIZE` configuration
options: the tables in the drivers will be the ones providing those
after NAND autodetection.

Thoughts?

Cheers,
Sam

> ---
> Icenowy Zheng (5):
>       sunxi: SPL SPI: extract code for doing SPI transfer
>       sunxi: SPL SPI: add support for read command with 2 byte address
>       sunxi: SPL SPI: allow multiple boot attempt
>       sunxi: SPL SPI: add initial support for booting from SPI NAND
>       sunxi: enable support for SPI NAND booting on SUNIV
>
> John Watts (9):
>       sunxi: Separate boot device and boot position
>       spl: Add BOOT_DEVICE_SPINAND option
>       sunxi: Implement BOOT_DEVICE_SPINAND in SPL
>       spl: Add SPL_SPINAND configuration options
>       sunxi: Use SPL_SPINAND for configuration
>       nand: Add spinand_ helper functions
>       sunxi: Implement spinand_ helpers
>       spl: Support SPI NAND boot in UBI
>       spl: Support loading FIT images in UBI
>
> Maksim Kiselev (1):
>       sunxi: SPL SPI: Add SPI boot support for the Allwinner R528/T113 SoCs
>
>  arch/arm/include/asm/arch-sunxi/spl.h |   3 +-
>  arch/arm/include/asm/spl.h            |   1 +
>  arch/arm/mach-sunxi/Kconfig           |   2 +-
>  arch/arm/mach-sunxi/board.c           |  31 +--
>  arch/arm/mach-sunxi/spl_spi_sunxi.c   | 348 +++++++++++++++++++++++++---------
>  arch/mips/include/asm/spl.h           |   1 +
>  arch/riscv/include/asm/spl.h          |   1 +
>  arch/sandbox/include/asm/spl.h        |   1 +
>  common/spl/Kconfig                    |  21 ++
>  common/spl/spl_ubi.c                  |  49 ++++-
>  include/nand.h                        |   3 +
>  11 files changed, 354 insertions(+), 107 deletions(-)
> ---
> base-commit: 777c28460947371ada40868dc994dfe8537d7115
> change-id: 20240411-spinand-eb7d8319813b
>
> Best regards,
> --
> John Watts <contact at jookia.org>
>