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

Tony Dinh
2023-02-25T22:37:46Z

Hi Pali,

On Sat, Feb 25, 2023 at 1:13 PM Pali Rohár <pali@kernel.org> wrote:
>
> 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?

I've double checked that in u-boot and Linux to make sure both use
64KB env size and 4K sector, and at the same location! I was guessing
that the block protection is causing a problem, so I also had a patch
for the flash chip mx25l3205d to unprotect all blocks too (currently
u-boot spi-nor does not do that for the Macronix flash). But that
still doesn't help :) I think I might try moving the envs to a
different location to see if it helps.

Thanks,
Tony

>
> > 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
> > >