[PATCH 1/2] bootstd: Add implementation for bootmeth rauc

[Simon Glass]

Hi Martin,

On Thu, 24 Apr 2025 at 06:43, Martin Schwan <M.Schwan at phytec.de> wrote:
>
> Hi Simon,
>
> thanks for the review and sorry for my delayed reply.
>
> On Mon, 2025-04-14 at 05:32 -0600, Simon Glass wrote:
> > Hi Martin,
> >
> > On Wed, 29 Jan 2025 at 07:25, Martin Schwan <m.schwan at phytec.de>
> > wrote:
> > >
> > > Add a bootmeth driver which supports booting A/B system with RAUC
> > > as
> > > their update client.
> > >
> > > Signed-off-by: Martin Schwan <m.schwan at phytec.de>
> > > ---
> > >  boot/Kconfig         |  11 ++
> > >  boot/Makefile        |   1 +
> > >  boot/bootmeth_rauc.c | 408
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 420 insertions(+)
> >
> > This looks fine to me. Just some nits below.
> >

[..]

> > > +
> > > +       partitions = env_get("rauc_partitions");
> > > +       if (!partitions)
> > > +               return log_msg_ret("env", -ENOENT);
> > > +       partitions_list = str_to_list(partitions);
> > > +
> > > +       slots = env_get("rauc_slots");
> > > +       if (!slots)
> > > +               return log_msg_ret("env", -ENOENT);
> > > +       slots_list = str_to_list(slots);
> > > +
> > > +       boot_order = env_get("BOOT_ORDER");
> >
> > Environment variables should be in lower case.
>
> Unfortunately, we are bound to the naming RAUC uses for these
> environment variables, which is upper case. See
> https://rauc.readthedocs.io/en/latest/integration.html#set-up-u-boot-boot-script-for-rauc

Can't you change it, since you are introducing new support here?

If not, then you could perhaps set the env variables to lowercase when
you find uppercase variables? I suppose that would require updating
the client to do the same thing too?

It would be very confusing for a user to have upper-case variables for
some things.

>
> >
> > > +       if (!boot_order) {
> > > +               if (env_set("BOOT_ORDER", slots))
> > > +                       return log_msg_ret("env", -EPERM);
> > > +       }
> > > +       priv->boot_order = strdup(boot_order);
> > > +
> > > +       default_tries = env_get_ulong("rauc_slot_default_tries",
> > > 10, 3);
> > > +       for (i = 0; slots_list[i] != NULL; i++) {
> > > +               sprintf(boot_left, "BOOT_%s_LEFT", slots_list[i]);
> >
> > same here

[..]

Regards,
Simon