Hi Tom,
On Fri, 25 Apr 2025 at 10:19, Tom Rini <tr...@konsulko.com> 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.
Yes, agreed.
Regards,
Simon
