On Mon, 2017-04-03 at 22:17 +0200, Wolfgang Denk wrote:
Dear Joakim,
Dear Wolfgang,
In message 1491221969.4177.81.camel@infinera.com you wrote:
I am looking at adding support for runtime sizing of CONFIG_ENV_ADDR as we need to replace out flash but we don't want to create a new u-boot binairy just for this simple change.
I doubt this will work for configurations that use embedded environment.
I see, I don't think so but will look closer.
While converting env_flash.c I noted the global variable env_t *env_ptr = (env_t *)CONFIG_ENV_ADDR; which cannot be runtime decided. Looking at users of this variable I only find one in pmc405de.c(not sure what that board is doing) and for what I can tell this variable is not correct for redundant env. either.
Did you look in the code only, or in all files?
code only, for the use of the env_ptr variable only as this is the variable I looking at.
Anyhow, I am faced wit two choices, either remove the env_ptr or convert it to a function call.
Probably neither will work for all use cases. You remember the good old times when we had parallel NOR flash with a few smaller sectors somewhere near the beginning or the end of the device? It was
Sure do, this is the reason I am having this problem. The new flashes do not have such smaller sectors, they are all uniform :(
pretty usual to use these small sectors for the environment, and it was the task of thelinker script to "wrap" the rest of the code around these reserved sectors. For this, the environment location must be known not only in the code, but also in the linker script.
After a brief look I think we are good. Let me explain, I am only making it possible to #define CONFIG_ENV_ADDR, CONFIG_ENV_SECT_SIZE etc. as non constant data by moving the assignment of flash_addr etc. to runtime, removing the static variable(less relocs to perform too :), I not forcing anyone to do so and only for env_flash.c
The only variable that I can't do away with it the: env_t *env_ptr = (env_t *)CONFIG_ENV_ADDR; as it is global. Nothing really uses this global variable(except pmc405de.c which is EPROM), not even the linker scripts below. They use #defines directly(CONFIG_ENV_OFFSET, CONFIG_SYS_FLASH_BASE etc. except for env_embedded.c which uses other variables)
As nothing really uses env_ptr and a variable isn't really a good interface(compare with the errno variable) I propose that the env_ptr variable in code is removed but lets start with removing it for env_flash.c first.
Without thorough checking , at least these files look suspicious to me:
arch/powerpc/cpu/mpc5xx/u-boot.lds: . = env_start; arch/powerpc/cpu/mpc5xx/u-boot.lds: .ppcenv : arch/powerpc/cpu/mpc5xx/u-boot.lds: common/env_embedded.o (.ppcenv) arch/powerpc/cpu/mpc5xxx/u-boot-customlayout.lds: . = DEFINED(env_offset) ? env_offset : .; arch/powerpc/cpu/mpc5xxx/u-boot-customlayout.lds: common/env_embedded.o (.ppcenv*) board/tqc/tqm8xx/u-boot.lds: . = DEFINED(env_offset) ? env_offset : .; board/tqc/tqm8xx/u-boot.lds: common/env_embedded.o (.ppcenv*) board/freescale/mx31ads/u-boot.lds: . = DEFINED(env_offset) ? env_offset : .; board/freescale/mx31ads/u-boot.lds: common/env_embedded.o(.text*)
Please have a look at these, and verify that the image layout does not change for these with any such changes.
Best regards,
Wolfgang Denk