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

[Jesse Taube <mr.bossman075@gmail.com>]

On 1/26/22 09:13, Andre Przywara wrote:
> 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 subet of that.
hmm this is new to me, i will change it.
>> +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?
doesn't boot when `enable_caches` gets called in mach-sunxi/board.c.
> 
>> +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?
Yup DM_SPL isnt set, ill remove it.
>> 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.
Yes i already fixed.
> 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
>