Hi Desapogu, On 2026-07-15T13:40:49, Desapogu Jayaramudu <[email protected]> wrote: > acpi: fix no display when serial console is disabled in coreboot config > > From: Desapogu Jayaramudu <[email protected]> > > When the serial console is disabled in coreboot configuration, > no display output is observed during boot. > > coreboot can expose serial console/debug-port information either through > coreboot tables(lb_serial) in src/lib/coreboot_table.c or through > ACPI DBG2 tables(RSDT/XSDT) during write_acpi_tables().When coreboot generates > a DBG2 table,it is publishing UART/debug-port information (IO or MMIO) through > ACPI for the operating system.By default coreboot assigned subtype > (ACPI_DBG2_PORT_SERIAL_16550 = 0x12)for both IO/MEM in src/acpi/acpi.c. > > U-Boot as a coreboot payload consumes serial data either from coreboot table > or > ACPI DBG2.When serial console is disabled in coreboot config , the coreboot > table > contains no serial data.So U-boot fallback and check the ACPI DBG2 table for > UART > serial console using read_dbg2() to find port type and subtype.Currently > u-boot > reads subtype as 0x12.But u-boot in include/acpi/acpi_table.h subtype defined > as > 0x0000 this value suppots only port I/O as per microsoft DBG2. For AMD > platforms > [...] > > drivers/serial/serial_coreboot.c | 2 +- > include/acpi/acpi_table.h | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-)
> acpi: fix no display when serial console is disabled in coreboot config The change is mostly in drivers/serial, so please use a 'serial: coreboot:' prefix. The subject is also over the 60-character limit - something like 'serial: coreboot: Accept DBG2 subtype 0x12' would do. > From: Desapogu Jayaramudu <[email protected]> The commit author is 'jayaram <[email protected]>', which looks like a local machine account. Please can you set git user.name and user.email to match your Signed-off-by, so this From: line is not needed. > U-Boot as a coreboot payload consumes serial data either from coreboot table > or > ACPI DBG2.When serial console is disabled in coreboot config , the coreboot > table Please can you tidy up the commit message: add a space after full stops and commas ('DBG2.When', 'config ,'), use 'U-Boot' consistently and wrap lines at 72 columns. > diff --git a/include/acpi/acpi_table.h b/include/acpi/acpi_table.h > @@ -668,6 +668,7 @@ struct __packed acpi_dmar { > #define ACPI_DBG2_16550_COMPATIBLE 0x0000 > +#define ACPI_DBG2_16550_GAS 0x0012 > #define ACPI_DBG2_16550_SUBSET 0x0001 Please keep this list sorted by value (after ACPI_DBG2_BCM2835) and use a tab for alignment like the other entries. The DBG2 spec calls 0x0012 '16550-compatible with parameters defined in Generic Address Structure', so perhaps ACPI_DBG2_16550_WITH_GAS, with a short comment giving the spec description. > diff --git a/drivers/serial/serial_coreboot.c > b/drivers/serial/serial_coreboot.c > @@ -59,7 +59,7 @@ static int read_dbg2(struct ns16550_plat *plat) > - if (dbg->port_subtype != ACPI_DBG2_16550_COMPATIBLE) { > + if (dbg->port_subtype != ACPI_DBG2_16550_COMPATIBLE && > dbg->port_subtype != ACPI_DBG2_16550_GAS) { Just to check the semantics: for subtype 0x12 the access parameters are supposed to come from the GAS, yet the code below still hard-codes plat->reg_width = 4 (with the comment that coreboot sets bit_width to 0) and derives reg_shift from access_size with a coreboot-specific adjustment. Does coreboot fill in the GAS the same way for 0x12 on your AMD platforms? If the GAS values are valid for this subtype, it would be better to honour them (bit_width, access_size) rather than relying on the workaround. What do you think? This could be a follow-up patch, if needed. Also please wrap this line to stay within 80 columns, e.g. if (dbg->port_subtype != ACPI_DBG2_16550_COMPATIBLE && dbg->port_subtype != ACPI_DBG2_16550_WITH_GAS) { Regards, Simon
