Hi Bin, On Tue, 19 Sept 2023 at 02:47, Bin Meng <bmeng...@gmail.com> wrote: > > Hi Simon, > > On Mon, Sep 11, 2023 at 3:13 AM Simon Glass <s...@chromium.org> wrote: > > > > This driver is not actually built since a Kconfig was never created for > > it. > > > > Add a Kconfig (which is already implied by COREBOOT) and update the > > implementation to avoid using unnecessary memory. Drop the #ifdef at the > > top since we can rely on Kconfig to get that right. > > > > To enable it (in addition to serial and video), use: > > > > setenv stdout serial,vidconsole,cbmem > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > --- > > > > Changes in v4: > > - Add some comments to help understand the overflow mechanism > > > > Changes in v2: > > - Update to support the new overflow mechanism > > > > drivers/misc/Kconfig | 8 +++++++ > > drivers/misc/cbmem_console.c | 43 ++++++++++++++++++++++-------------- > > 2 files changed, 34 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > > index a6e3f62ecb09..c930e4a361bf 100644 > > --- a/drivers/misc/Kconfig > > +++ b/drivers/misc/Kconfig > > @@ -122,6 +122,14 @@ config VEXPRESS_CONFIG > > configuration bus on the Arm Versatile Express boards via > > a sysreg driver. > > > > +config CBMEM_CONSOLE > > + bool "Write console output to coreboot cbmem" > > + depends on X86 > > + help > > + Enables console output to the cbmem console, which is a memory > > + region set up by coreboot to hold a record of all console output. > > + Enable this only if booting from coreboot. > > + > > config CMD_CROS_EC > > bool "Enable crosec command" > > depends on CROS_EC > > diff --git a/drivers/misc/cbmem_console.c b/drivers/misc/cbmem_console.c > > index 8bbe33d414da..3cbe9fb1a46a 100644 > > --- a/drivers/misc/cbmem_console.c > > +++ b/drivers/misc/cbmem_console.c > > @@ -5,27 +5,37 @@ > > > > #include <common.h> > > #include <console.h> > > -#ifndef CONFIG_SYS_COREBOOT > > -#error This driver requires coreboot > > -#endif > > - > > #include <asm/cb_sysinfo.h> > > > > -struct cbmem_console { > > - u32 buffer_size; > > - u32 buffer_cursor; > > - u8 buffer_body[0]; > > -} __attribute__ ((__packed__)); > > - > > -static struct cbmem_console *cbmem_console_p; > > - > > void cbmemc_putc(struct stdio_dev *dev, char data) > > { > > - int cursor; > > + const struct sysinfo_t *sysinfo = cb_get_sysinfo(); > > + struct cbmem_console *cons; > > + uint pos, flags; > > + > > + if (!sysinfo) > > + return; > > + cons = sysinfo->cbmem_cons; > > + if (!cons) > > + return; > > + > > + pos = cons->cursor & CBMC_CURSOR_MASK; > > + > > + /* preserve the overflow flag if present */ > > + flags = cons->cursor & ~CBMC_CURSOR_MASK; > > + > > + cons->body[pos++] = data; > > + > > + /* > > + * Deal with overflow - the flag may be cleared by another program > > which > > + * reads the buffer out, e.g. Linux > > + */ > > U-Boot is not memory resident, so this does not sound like a correct > overflow mechanism to me.
I am not sure what you mean. This logic is used in coreboot and some payloads, so I want to do the same in U-Boot. It basically let's U-Boot handle an overflow properly by setting the overflow flag. A later program (e.g. Linux) can then tell that that an overflow occurred. > > > + if (pos >= cons->size) { > > + pos = 0; > > + flags |= CBMC_OVERFLOW; > > + } > > > > - cursor = cbmem_console_p->buffer_cursor++; > > - if (cursor < cbmem_console_p->buffer_size) > > - cbmem_console_p->buffer_body[cursor] = data; > > + cons->cursor = flags | pos; > > } > > > > void cbmemc_puts(struct stdio_dev *dev, const char *str) > > @@ -40,7 +50,6 @@ int cbmemc_init(void) > > { > > int rc; > > struct stdio_dev cons_dev; > > - cbmem_console_p = lib_sysinfo.cbmem_cons; > > > > memset(&cons_dev, 0, sizeof(cons_dev)); > > > Regards, Simon