[PATCH] env: Leave invalid env for nowhere location

[Marek Vasut]

On 6/7/21 9:54 AM, Kunihiko Hayashi wrote:

Hi,

[...]

>>>> I would expect that after relocation, if all you have is env_nowhere
>>>> driver, the env_nowhere_init() is called again from the first for() 
>>>> loop
>>>> of env_init() [1], which would set gd->env_valid to ENV_INVALID [1], 
>>>> and
>>>> that for() loop would exit with ret = -ENOENT [2], so then the last 
>>>> part
>>>> of env_init() would check for ret == -ENOENT and update gd->env_addr to
>>>> relocated default_environment [3].
>>>>
>>>> 324  int env_init(void)
>>>> 325  {
>>>> 326    struct env_driver *drv;
>>>> 327    int ret = -ENOENT;
>>>> 328    int prio;
>>>> 329
>>>> 330    for (prio = 0; (drv = env_driver_lookup(ENVOP_INIT, prio)); 
>>>> prio++) {
>>>>            /* Part [1] */
>>>> 331      if (!drv->init || !(ret = drv->init()))
>>>> 332        env_set_inited(drv->location);
>>>> 333      if (ret == -ENOENT)
>>>> 334        env_set_inited(drv->location);
>>>> 335
>>>> 336      debug("%s: Environment %s init done (ret=%d)\n", __func__,
>>>> 337            drv->name, ret);
>>>> 338
>>>>            /* Part [2] */
>>>> 339      if (gd->env_valid == ENV_INVALID)
>>>> 340        ret = -ENOENT;
>>>> 341    }
>>>> 342
>>>> 343    if (!prio)
>>>> 344      return -ENODEV;
>>>> 345
>>>>          /* Part [3] */
>>>> 346    if (ret == -ENOENT) {
>>>>            /* This should be relocated default_environment address */
>>>> 347      gd->env_addr = (ulong)&default_environment[0];
>>>> 348      gd->env_valid = ENV_VALID;
>>>> 349
>>>> 350      return 0;
>>>> 351    }
>>>> 352
>>>> 353    return ret;
>>>> 354  }
>>>>
>>>> Or am I missing something obvious ?
>>>
>>> These are called before relocation, and update gd->env_addr to 
>>> non-relocated
>>> default_environment by [3].
>>>
>>> After that, gd->env_addr is relocated in initr_reloc_global_data()
>>> if CONFIG_SYS_RELOC_GD_ENV_ADDR is defined.
>>>
>>> | #ifdef CONFIG_SYS_RELOC_GD_ENV_ADDR
>>> |     /*
>>> |      * Relocate the early env_addr pointer unless we know it is not 
>>> inside
>>> |      * the binary. Some systems need this and for the rest, it 
>>> doesn't hurt.
>>> |      */
>>> |     gd->env_addr += gd->reloc_off;
>>> | #endif
>>>
>>
>> Shouldn't the post-relocation env update happen in env_relocate() ?
> 
> Usually env_relocate() calls env_load() that uses relocated gd->env_addr.
> It's no problem.
> 
> If CONFIG_SYS_TEXT_BASE is zero, gd->reloc_off becomes illegal.
> CONFIG_SYS_RELOC_GD_ENV_ADDR should be disabled in that case.

But then, if CONFIG_SYS_TEXT_BASE is zero, the env shouldn't be 
relocated or how should it behave ?