Re: [PATCH 11/11] configs: sunxi: Add support for Lichee Pi Nano

[Andre Przywara <andre.przywara@arm.com>]

On Tue,  4 Jan 2022 19:35:08 -0500
Jesse Taube <mr.bossman075@gmail.com> wrote:

Hi,

> From: Icenowy Zheng <icenowy@aosc.io>
> 
> The Lichee Pi Nano is a board based on the F1C100s.
> Add defconfigs for it.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> ---
>  configs/licheepi_nano_defconfig          | 13 ++++++++++++
>  configs/licheepi_nano_spiflash_defconfig | 25 ++++++++++++++++++++++++
>  2 files changed, 38 insertions(+)
>  create mode 100644 configs/licheepi_nano_defconfig
>  create mode 100644 configs/licheepi_nano_spiflash_defconfig
> 
> diff --git a/configs/licheepi_nano_defconfig b/configs/licheepi_nano_defconfig
> new file mode 100644
> index 0000000000..3a24870aaf
> --- /dev/null
> +++ b/configs/licheepi_nano_defconfig
> @@ -0,0 +1,13 @@
> +CONFIG_ARM=y
> +CONFIG_ARCH_SUNXI=y
> +CONFIG_MACH_SUNIV=y
> +CONFIG_SYS_TEXT_BASE=0x81700000

This belongs into boot/Kconfig, where we set the values for the other SoCs.
Please reorder them on the way, Kconfig goes with the first match, so we
should have a "default 0x4a000000 if ARCH_SUNXI" line at the end, and the
special snowflakes first. No need for && ARCH_SUNXI, MACH_SUNxxx is always
a subset of that.

> +CONFIG_SYS_LOAD_ADDR=0x81000000

This should be set in /Kconfig, as for the other SoCs.

> +CONFIG_SYS_MALLOC_LEN=0x120000

Same here, in /Kconfig, and reordering to simplify the expressions.

> +CONFIG_DRAM_CLK=156
> +CONFIG_SYS_DCACHE_OFF=y

Mmh, why is this? 

> +CONFIG_DRAM_ZQ=0
> +# CONFIG_VIDEO_SUNXI is not set
> +CONFIG_DEFAULT_DEVICE_TREE="suniv-f1c100s-licheepi-nano"
> +CONFIG_SPL=y
> +# CONFIG_SPL_DM_SERIAL is not set

That should not be needed?

> diff --git a/configs/licheepi_nano_spiflash_defconfig b/configs/licheepi_nano_spiflash_defconfig
> new file mode 100644
> index 0000000000..07b6a27dbe
> --- /dev/null
> +++ b/configs/licheepi_nano_spiflash_defconfig

I dislike the idea of providing separate defconfigs for those board
variants. We have other examples where a SPI flash chip is not always
populated, but that should go well with one defconfig. If there is no SPI
flash, the code will see that and bail out gracefully, no problems with
that.

So there should be one defconfig, including SPI flash support. I'd suggest
to drop that for now, as the current series does not cover this, AFAICT.
We can add this later, which should be simpler with my SPI fix series.

Cheers,
Andre

> @@ -0,0 +1,25 @@
> +CONFIG_ARM=y
> +CONFIG_ARCH_SUNXI=y
> +CONFIG_ENV_SIZE=0x8000
> +CONFIG_ENV_OFFSET=0xf8000
> +CONFIG_MACH_SUNIV=y
> +CONFIG_DRAM_CLK=156
> +CONFIG_DRAM_ZQ=0
> +# CONFIG_VIDEO_SUNXI is not set
> +CONFIG_DEFAULT_DEVICE_TREE="suniv-f1c100s-licheepi-nano"
> +CONFIG_SPL=y
> +# CONFIG_CMD_FLASH is not set
> +# CONFIG_CMD_FPGA is not set
> +CONFIG_CMD_SF=y
> +CONFIG_CMD_SPI=y
> +CONFIG_ENV_IS_IN_SPI_FLASH=y
> +CONFIG_DM_SPI_FLASH=y
> +CONFIG_SPI_FLASH=y
> +CONFIG_SPI_FLASH_BAR=y
> +CONFIG_SPI_FLASH_GIGADEVICE=y
> +CONFIG_SPI_FLASH_MACRONIX=y
> +CONFIG_SPI_FLASH_WINBOND=y
> +CONFIG_SPL_SPI_SUNXI=y
> +# CONFIG_SPL_DM_SERIAL is not set
> +CONFIG_DM_SPI=y
> +CONFIG_SUN6I_SPI=y