[PATCH v2 05/17] boot: Move showing of bootflows out of the command

[Tom Rini]

On Wed, Oct 08, 2025 at 06:50:41AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 7 Oct 2025 at 09:39, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Tue, Oct 07, 2025 at 09:14:29AM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Tue, 7 Oct 2025 at 07:50, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Tue, Oct 07, 2025 at 05:13:55AM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Mon, 6 Oct 2025 at 17:45, Tom Rini <trini at konsulko.com> wrote:
> > > > > >
> > > > > > On Mon, Oct 06, 2025 at 05:30:23PM -0600, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Thu, 2 Oct 2025 at 14:15, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, Oct 01, 2025 at 03:26:30PM -0600, Simon Glass wrote:
> > > > > > > >
> > > > > > > > > It is helpful in tests to be able to show the bootflow that is being
> > > > > > > > > examined. Move show_bootflow() into boot/ and rename it.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > Changes in v2:
> > > > > > > > > - Add a log_err() for an invalid state
> > > > > > > > >
> > > > > > > > >  boot/bootflow.c    | 57 ++++++++++++++++++++++++++++++++++++++
> > > > > > > > >  cmd/bootflow.c     | 68 ++--------------------------------------------
> > > > > > > > >  include/bootflow.h |  9 ++++++
> > > > > > > > >  3 files changed, 69 insertions(+), 65 deletions(-)
> > > > > > > > [snip]
> > > > > > > > > +     case BOOTFLOWST_COUNT:
> > > > > > > > > +             log_err("Unexpected boot value of bootflow error %d",
> > > > > > > > > +                      bflow->state);
> > > > > > > >
> > > > > > > > A small thing, checkpatch.pl catches that this isn't aligned with the '('
> > > > > > > > here as it should be.
> > > > > > >
> > > > > > > OK. I'm unsure whether I really want this line anyway, since it
> > > > > > > increases code size.
> > > > > > >
> > > > > > > >
> > > > > > > > A larger thing, and please correct me if I'm wrong, but on reading the
> > > > > > > > whole set of changes, this move + rename just means we're putting more
> > > > > > > > info in the test output, and nothing else?
> > > > > > >
> > > > > > > It will also appear if you have CONFIG BOOTSTD_FULL and use 'bootflow
> > > > > > > list' or 'bootflow scan -l'.
> > > > > >
> > > > > > But that should be the case before this patch as well, yes?
> > > > >
> > > > > Yes, that's right. This is just moving the code into a place where it
> > > > > can be used from tests.
> > > >
> > > > But it's not being used from tests, with this series.
> > >
> > > Please see this one: 'boot: Add a new test for global bootmeths'
> >
> > Yes, it calls the function, but doesn't seem to do anything with that,
> > especially considering what the test does before these patches.
> 
> Well, it prints out the information so you can see it when running the
> test. Other tests show what is going on so it seemed sensible to do
> the same here.

The flipside is that since all of the test is working testing the values
of the struct it's easy to debug any failure as we'll be told field X
expected value X but found Y.

> I'm happy to remove this patch if you like.

Yes, please drop this patch, thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20251008/5b1444bb/attachment.sig>