[PATCH v4 3/4] boot: Add more debugging to iter_incr()

[Simon Glass]

Hi Tom,

On Thu, 9 Oct 2025 at 18:30, Tom Rini <trini at konsulko.com> wrote:
>
> On Thu, Oct 09, 2025 at 03:29:54AM -0600, Simon Glass wrote:
>
> > This function is the core of the bootstd iteration. Add some debugging
> > for the decisions it makes along the way, to make it easier to track
> > what is going on.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >  boot/bootflow.c | 23 +++++++++++++++++++----
> >  1 file changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/boot/bootflow.c b/boot/bootflow.c
> > index 78df09f369d..de69e27bec7 100644
> > --- a/boot/bootflow.c
> > +++ b/boot/bootflow.c
> > @@ -193,8 +193,10 @@ static int iter_incr(struct bootflow_iter *iter)
> >       log_debug("entry: err=%d\n", iter->err);
> >       global = iter->doing_global;
> >
> > -     if (iter->err == BF_NO_MORE_DEVICES)
> > +     if (iter->err == BF_NO_MORE_DEVICES) {
> > +             log_debug("-> err: no more devices1\n");
> >               return BF_NO_MORE_DEVICES;
> > +     }
>
> Thinking more about what I said in the previous iteration about git
> blame history, ones like this should be log_msg_ret (the history on
> "when did the test for == BF_NO_MORE_DEVICES come from is unchanged, but
> now you can have debug statements when enabled).

Yes we can add that as well, but I still want to have the log_debug()
as this doesn't require enabling CONFIG_LOG_ERROR_RETURN. That feature
produces a lot of output even in normal operation since it shows
errors dealt with by higher level code. It is really only designed to
find the source of a particular error when you are stuck.

>
> [snip]
> > @@ -228,11 +234,15 @@ static int iter_incr(struct bootflow_iter *iter)
> >
> >       if (iter->err != BF_NO_MORE_PARTS) {
> >               /* ...select next partition  */
> > -             if (++iter->part <= iter->max_part)
> > +             if (++iter->part <= iter->max_part) {
> > +                     log_debug("-> next partition %d max %d\n", iter->part,
> > +                               iter->max_part);
> >                       return 0;
> > +             }
>
> Shouldn't this be a debug message instead in the caller?

I am trying to log_debug() every exit from this function...so you can
see the entry and then which path it took.

>
> [snip]
> > @@ -326,8 +336,13 @@ static int iter_incr(struct bootflow_iter *iter)
> >       }
> >
> >       /* if there are no more bootdevs, give up */
> > -     if (ret)
> > +     if (ret) {
> > +             log_debug("-> no more bootdevs\n");
> >               return log_msg_ret("incr", BF_NO_MORE_DEVICES);
> > +     }
>
> Then do we actually need both a log_debug and a log_msg_ret?

Please see above.

Regards,