[U-Boot] [PATCH v2 2/3] sunxi: Pine64-LTS: enable environment in spi flash

[André Przywara]

On Fri, 2 Aug 2019 10:47:10 +0300
Oskari Lemmelä <oskari at lemmela.net> wrote:

Hi Oskari,

> On 2.8.2019 3.15, André Przywara wrote:
> > On 01/08/2019 20:50, Oskari Lemmela wrote:
> >
> > Hi,
> >  
> >> enable config options for spi flash device and
> >> environment in spi flash.  
> > First I am not sure if defining this unconditionally to put the
> > environment in SPI flash is a good idea. People might expect to be able
> > to (continue to) use an environment from the SD card if they just booted
> > from one.
> > Hence my efforts to allow to use the environment from the respective
> > boot media:
> > https://lists.denx.de/pipermail/u-boot/2019-June/372025.html  
> This patch was just for testing purposes.

Please mark patches accordingly if they are not meant for being merged.
Although I am not sure why you posted it then in the first place?

> That is true current code picks
> SD card environment if it is able to load it and both (MMC and SPI) are
> enabled.

Well, it does that, but uses the optional(!) eMMC device for that.
If there is none, it will proceed with SPI
Another reason to rework the env device selection and make more
decisions at runtime.

Also it will overwrite a U-Boot copy on the SPI flash in the process.

> I read through that discussion around your environment patches.
> For fail-safe purposes it might be good idea to have at least Kconfig
> option to
> allow load working environment from different media if primary one fails. 

Yeah, that was my plan, just didn't come around to it yet.

> > Secondly, this uses CONFIG_ENV_OFFSET to place the environment in SPI
> > flash. For historical reasons this is 0x88000 for sunxi (old raw SD card
> > offset), so 544KB. Which lets a saveenv overwrite the last part of
> > U-Boot, if you have put it there.
> >
> > So we should define this to something higher, I suggest just below 1MB,
> > so 0xe0000.
> > Actually I wonder why the default size is 128KB, when the typical
> > environments are so much smaller. For my experiments I used 64KB at
> > 0xf0000, which gives U-Boot more room to grow in the future.  
> That is true. Might be that default offset is fine for 32-bit systems which
> doesn't require ATF. I have used 0xE0000 in my defconfigs.
> So better defaults could be
> 
>         default 0x88000 if (ARCH_SUNXI && !MACH_SUN50I)
>         default 0xE0000 if ARCH_ZYNQ || (ARCH_SUNXI && MACH_SUN50I)

Something like this, though it should be (ARCH_SUNXI && ARM64), to also
cover the H5, H6 and follow-up accidents from Allwinner.
Amd it's a bit weird that this value is shared with the raw MMC offset,
as SPI flash and MMC are quite different in terms of usage.
So I think we should split these two at some point.

Cheers,
Andre.

> Or what you think?
> 
> Oskari
> >
> > Cheers,
> > Andre.
> >  
> >> Signed-off-by: Oskari Lemmela <oskari at lemmela.net>
> >> ---
> >>  configs/pine64-lts_defconfig | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/configs/pine64-lts_defconfig b/configs/pine64-lts_defconfig
> >> index 0da6b70ea8..34c38cf1fd 100644
> >> --- a/configs/pine64-lts_defconfig
> >> +++ b/configs/pine64-lts_defconfig
> >> @@ -18,3 +18,10 @@ CONFIG_SUN8I_EMAC=y
> >>  CONFIG_USB_EHCI_HCD=y
> >>  CONFIG_USB_OHCI_HCD=y
> >>  CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE=y
> >> +CONFIG_DM_SPI=y
> >> +CONFIG_DM_SPI_FLASH=y
> >> +CONFIG_ENV_IS_IN_SPI_FLASH=y
> >> +CONFIG_SPI=y
> >> +CONFIG_SPI_FLASH=y
> >> +CONFIG_SPI_FLASH_WINBOND=y
> >> +CONFIG_SPI_SUNXI=y
> >>  
>