[U-Boot] Remove global variable env_t *env_ptr ?

[Joakim Tjernlund <Joakim.Tjernlund@infinera.com>]

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
>