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

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3122 bytes --]

On Fri, Apr 25, 2025 at 08:35:52AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 24 Apr 2025 at 12:49, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Apr 24, 2025 at 12:31:38PM -0600, Simon Glass wrote:
> > > Hi Martin,
> > >
> > > On Thu, 24 Apr 2025 at 06:43, Martin Schwan <M.Schwan@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@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@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.
> >
> > Simon, this is an unreasonable request. This is how RAUC has been
> > working and deployed for years and years. It would be a nightmare trying
> > to handle migration of existing deployments for an aesthetic change.
> 
> Yes, I figured. Perhaps it could support lower case in future? This is
> the first I have seen on RAUC in U-Boot, although I do see this file
> which has both lower and upper case:
> 
> include/env/phytec/rauc.env

I don't think there's any need to complicate RAUC support here (nor
in the future Mender nor swupdate nor any other long standing OTA
mechanism that might be more easily integrated via bootstd than
environment patching).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]