| Message ID | 20230221201925.9644-58-pali@kernel.org |
|---|---|
| State | Accepted |
| Commit | 056808a4bbcc611d8cdedd937d9e1177b441716a |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | arm: mvebu: Various fixes | expand |
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 ? 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'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). 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 >
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 > >
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 > > >
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;
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. 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(-)