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

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Simon, Svyatoslav,

On Thu, Dec 28, 2023 at 21:52, Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> чт, 28 груд. 2023 р. о 21:48 Simon Glass <sjg@chromium.org> пише:
>>
>> On Thu, Dec 28, 2023 at 6:02 PM Svyatoslav Ryhel <clamor95@gmail.com> wrote:
>> >
>> > From: Ion Agorria <ion@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@agorria.com>
>> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>> > Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> > ---
>> >  common/console.c | 3 +++
>> >  1 file changed, 3 insertions(+)
>>
>> Reviewed-by: Simon Glass <sjg@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().

>
> Best Regards,
> Svyatoslav R.
>
>> >
>> > diff --git a/common/console.c b/common/console.c
>> > index 6f2089caa0..e6d7ebe935 100644
>> > --- a/common/console.c
>> > +++ b/common/console.c
>> > @@ -821,6 +821,9 @@ int console_record_init(void)
>> >         ret = membuff_new((struct membuff *)&gd->console_in,
>> >                           CONFIG_CONSOLE_RECORD_IN_SIZE);
>> >
>> > +       /* Start recording from the beginning */
>> > +       gd->flags |= GD_FLG_RECORD;
>> > +
>> >         return ret;
>> >  }
>> >
>> > --
>> > 2.40.1
>> >
>>
>> Regards,
>> Simon