Re: [PATCH RFC u-boot-mvebu 57/59] arm: mvebu: Define env_sf_get_env_addr() also for Proper U-Boot

["Pali Rohár" <pali@kernel.org>]

On Friday 24 February 2023 19:58:37 Tony Dinh wrote:
> Hi Pali,
> 
> On Tue, Feb 21, 2023 at 12:22 PM Pali Rohár <pali@kernel.org> wrote:
> >
> > Proper U-Boot moves SPI0 CS0 Flash mapping from 0xD4000000 to 0xF4000000
> > and change its size from 64 MB to 8 MB. Definitions are already in
> > MBUS_SPI_BASE/MBUS_SPI_SIZE macros. So define these macros also for SPL
> > build, use them in env_sf_get_env_addr() function and move this function
> > from spl.c to cpu.c to be available in Proper U-Boot too.
> 
> Interesting! So would it  affect the board that has a 4MB SPI flash ?

This change just allows to use read-only SPI0 CS0 Flash mapping in the
env code in proper u-boot. If there is a board code which requires
access to env from proper u-boot before relocation happens then this
change allows it. But only if env is stored in the first 8MB of SPI0
flash.

Note that after relocation u-boot uses full-feature SPI driver for
accessing flash memory, by coping its content to RAM during read.

So any access to env after relocation should not be affected by this
change.

IIRC only Omnia board for now uses this feature (for configuring
serdes based on env settings) and for now only in SPL.

> I know there is a bug somewhere, because I need to set both
> CONFIG_ENV_OFFSET and CONFIG_ENV_ADDR. One or the other is not enough.

I am not sure for what is _ADDR needed (I am only on mobile for now, so
I cannot check it)

> I'm debugging an error that seemingly has something to do with the SPI
> envs area: how u-boot set envs and recalculate checksum, and Linux
> fw_setenv() seems to not be doing the same thing. This is on a 4MB SPI
> mx25l3205d flash (Thecus N2350 board).

Maybe you have set wrong env size or wrong flash erase block size?

> It might also have something to do with a SPI flash being set to some
> protected  blocks (in the Status Register upon boot).
> 
> Thanks,
> Tony
> 
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  arch/arm/mach-mvebu/cpu.c              |  9 +++++++++
> >  arch/arm/mach-mvebu/include/mach/cpu.h |  5 +++++
> >  arch/arm/mach-mvebu/spl.c              | 13 -------------
> >  3 files changed, 14 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c
> > index c5089a91c747..97154aaa2a7e 100644
> > --- a/arch/arm/mach-mvebu/cpu.c
> > +++ b/arch/arm/mach-mvebu/cpu.c
> > @@ -35,6 +35,15 @@ static const struct mbus_win windows[] = {
> >  #endif
> >  };
> >
> > +/* SPI0 CS0 Flash of size MBUS_SPI_SIZE is mapped to address MBUS_SPI_BASE */
> > +#if CONFIG_ENV_SPI_BUS == 0 && CONFIG_ENV_SPI_CS == 0 && \
> > +    CONFIG_ENV_OFFSET + CONFIG_ENV_SIZE <= MBUS_SPI_SIZE
> > +void *env_sf_get_env_addr(void)
> > +{
> > +       return (void *)MBUS_SPI_BASE + CONFIG_ENV_OFFSET;
> > +}
> > +#endif
> > +
> >  void lowlevel_init(void)
> >  {
> >         /*
> > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
> > index c17c2440f1b1..906a8737a401 100644
> > --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> > @@ -71,8 +71,13 @@ enum cpu_attrib {
> >  #define MBUS_PCI_MEM_SIZE      ((MBUS_PCI_MAX_PORTS * 128) << 20)
> >  #define MBUS_PCI_IO_BASE       0xF1100000
> >  #define MBUS_PCI_IO_SIZE       ((MBUS_PCI_MAX_PORTS * 64) << 10)
> > +#ifdef CONFIG_SPL_BUILD
> > +#define MBUS_SPI_BASE          0xD4000000
> > +#define MBUS_SPI_SIZE          (64 << 20)
> > +#else
> >  #define MBUS_SPI_BASE          0xF4000000
> >  #define MBUS_SPI_SIZE          (8 << 20)
> > +#endif
> >  #define MBUS_DFX_BASE          0xF6000000
> >  #define MBUS_DFX_SIZE          (1 << 20)
> >  #define MBUS_BOOTROM_BASE      0xF8000000
> > diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> > index 02528e025d8c..6b8c72a71dab 100644
> > --- a/arch/arm/mach-mvebu/spl.c
> > +++ b/arch/arm/mach-mvebu/spl.c
> > @@ -308,19 +308,6 @@ int board_return_to_bootrom(struct spl_image_info *spl_image,
> >         hang();
> >  }
> >
> > -/*
> > - * SPI0 CS0 Flash is mapped to address range 0xD4000000 - 0xD7FFFFFF by BootROM.
> > - * Proper U-Boot removes this direct mapping. So it is available only in SPL.
> > - */
> > -#if defined(CONFIG_SPL_ENV_IS_IN_SPI_FLASH) && \
> > -    CONFIG_ENV_SPI_BUS == 0 && CONFIG_ENV_SPI_CS == 0 && \
> > -    CONFIG_ENV_OFFSET + CONFIG_ENV_SIZE <= 64*1024*1024
> > -void *env_sf_get_env_addr(void)
> > -{
> > -       return (void *)0xD4000000 + CONFIG_ENV_OFFSET;
> > -}
> > -#endif
> > -
> >  void board_init_f(ulong dummy)
> >  {
> >         int ret;
> > --
> > 2.20.1
> >