[PATCH v6 4/6] common: console: record console from the beginning

Simon Glass sjg at chromium.org
Tue Jan 2 15:06:31 CET 2024


Hi Mattijs,

On Tue, Jan 2, 2024 at 2:52 AM Mattijs Korpershoek
<mkorpershoek at baylibre.com> wrote:
>
> Hi Simon, Svyatoslav,
>
> On Thu, Dec 28, 2023 at 21:52, Svyatoslav Ryhel <clamor95 at gmail.com> wrote:
>
> > чт, 28 груд. 2023 р. о 21:48 Simon Glass <sjg at chromium.org> пише:
> >>
> >> On Thu, Dec 28, 2023 at 6:02 PM Svyatoslav Ryhel <clamor95 at gmail.com> wrote:
> >> >
> >> > From: Ion Agorria <ion at agorria.com>
> >> >
> >> > Set flag to enable console record on console_record_init
> >> > and not only on console_record_reset_enable. This fixes
> >> > missing start of U-Boot log for fastboot oem console
> >> > command.
> >> >
> >> > Signed-off-by: Ion Agorria <ion at agorria.com>
> >> > Signed-off-by: Svyatoslav Ryhel <clamor95 at gmail.com>
> >> > Reviewed-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
> >> > ---
> >> >  common/console.c | 3 +++
> >> >  1 file changed, 3 insertions(+)
> >>
> >> Reviewed-by: Simon Glass <sjg at chromium.org>
> >>
> >> OK, I can see the use of this...but I wonder if we can now get rid of
> >> the same line of code from console_record_reset_enable() ?
> >>
> >
> > Interesting question but let's leave it to a dedicated patch :)
>
> I've looked a little more into to this, and I'm not so sure we can get
> rid of the gd->flags |= GD_FLG_RECORD; in console_record_reset_enable().
>
> Removing the flag seems to break quite some tests in
> test/py/tests/test_ut.py.
>
> The breakage can be explained that various unit tests clear the
> GD_FLG_RECORD with:
>
>         gd->flags &= ~GD_FLG_RECORD;
>
> Therefore, I would suggest we keep the flag in
> console_record_reset_enable().

>From my look all of those are not needed in tests, i.e. are bugs. If
you are able to do a patch to remove those lines, it would avoid the
confusion.

Also, by setting UT_TESTF_CONSOLE_REC for a test, console recording is
set up automatically, so the console_record_reset_enable() is not
needed at the start of the test.

Regards,
Simon


More information about the U-Boot mailing list