Hi Simon,
Hi Tom,
On Fri, 2025-04-25 at 10:19 -0600, Tom Rini wrote:
> On Fri, Apr 25, 2025 at 09:03:17AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 25 Apr 2025 at 08:37, Tom Rini <tr...@konsulko.com> wrote:
> > >
> > > 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 <tr...@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.sch...@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.sch...@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.sch...@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).
> >
> > Let's see what the author says. I assume that U-Boot has its own
> > code
> > in RAUC so it might not be too bad.
>
> I mean, sure? But "should" is not "must" and I have zero desire to
> make supporting existing use cases harder. That will make migration
> to
> bootstd less attractive, not more. We don't want people to decide
> that
> can't upstream platform X or feature Y because they've already
> developed
> and deployed it using capitals or camel case in their environment.
I am not a core RAUC developer, but it should be theoretically possible
to change it to lower-case variable names. The problem is that this
creates incompatibilities with at least older versions of RAUC, which
only use upper-case. And that's something I honestly don't want to deal
with, as it makes the U-Boot and RAUC codebase bloated. After all,
changing the variable naming now wouldn't be very *robust* for the
update client. It is a key feature of such update clients to update
without complicated migration precedures and I don't want to break
that.
So yes, it would be nice to keep the naming in line with the existing
U-Boot convention, but no, I also see too many serious downsides that
would justify such a change.
Kind Regards,
Martin
