[PATCH v3 10/10] env: ubi: add support to create environment volume if it does not exist

[Weijie Gao]

On Tue, 2026-04-28 at 08:11 -0600, Simon Glass wrote:
> Hi Weijie,
> 
> On 2026-04-27T07:09:11, Weijie Gao <weijie.gao at mediatek.com> wrote:
> > env: ubi: add support to create environment volume if it does not
> > exist
> > 
> > Add an option to allow environment volume being auto created if not
> > exist.
> > 
> > Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
> > 
> > env/Kconfig | 11 +++++++++++
> >  env/ubi.c   | 35 ++++++++++++++++++++++++++++++++++-
> >  2 files changed, 45 insertions(+), 1 deletion(-)
> > diff --git a/env/Kconfig b/env/Kconfig
> > @@ -699,6 +699,17 @@ config ENV_UBI_VOLUME_REDUND
> > +config ENV_UBI_VOLUME_CREATE
> > +     bool "Create UBI volume if not exist"
> > +     depends on ENV_IS_IN_UBI
> > +     help
> > +       This option is useful if u-boot will be booted from a fresh
> > device
> > +       where environment volume hasn't been created in the UBI
> > partition.
> > +       This is a common case where factory UBI image contains only
> > volumes
> > +       with valid data.
> > +       By enabling this option, environment volume(s) will be
> > created before
> > +       loading if not exist.
> 
> Please use 'U-Boot' rather than 'u-boot' and tidy the grammar (e.g.
> 'if it does not exist', 'where the environment volume has not been
> created', 'any missing environment volumes will be created before
> loading'). Also mention that with CONFIG_ENV_REDUNDANT both volumes
> are created.

OK.

> 
> > diff --git a/env/ubi.c b/env/ubi.c
> > @@ -134,6 +151,15 @@ static int env_ubi_load(void)
> > +     if (IS_ENABLED(CONFIG_ENV_UBI_VOLUME_CREATE)) {
> > +             create1_fail =
> > env_ubi_volume_create(CONFIG_ENV_UBI_VOLUME);
> > +             create2_fail =
> > env_ubi_volume_create(CONFIG_ENV_UBI_VOLUME_REDUND);
> > +             if (create1_fail && create2_fail) {
> > +                     env_set_default(NULL, 0);
> > +                     return -ENODEV;
> > +             }
> > +     }
> 
> Just to clarify the intent: if only one creation fails, this falls
> through and the subsequent ubi_volume_read() prints 'Unable to read
> redundant env from ...' which is confusing on a fresh device. Clearer
> to fail (or print a dedicated message) as soon as either creation
> fails - what do you think?

There's one case:
One volume exists and contains valid env data, and another one is
missing.
If we still failed to create the missing one (perhaps due to no space
left in UBI), we can continue to read the valid volume.

So I think it's better to add a check before reading:
if (!create1_fail) {
	read1_fail = ubi_volume_read(...);
} else {
	read1_fail = create1_fail;
}

> 
> > diff --git a/env/ubi.c b/env/ubi.c
> > @@ -105,12 +105,29 @@ static int env_ubi_save(void)
> > +     ret = ubi_create_vol(volume, CONFIG_ENV_SIZE, true,
> > UBI_VOL_NUM_AUTO,
> > +                          false);
> 
> The dynamic flag is hard-coded to true. Sensible default, but please
> mention the choice in the commit message or Kconfig help.

Ah. I remind that this is the reason I added __weak in the first patch.
One can override this function to create static volume.
I'll add another Kconfig to allow user to choose the volume type.

> 
> The commit message is quite terse - normally we like to see a
> motivation (fresh device with a factory UBI image lacking the env
> volume) - also note that for CONFIG_ENV_REDUNDANT both volumes are
> created.

OK. I'll modify it.

> 
> Regards,
> Simon