Re: [PATCH v4] serial: coreboot: Accept DBG2 subtype 0x12

Hi Jayaramudu,

(I see this in patchwork but not the mailing list, so am, trying to do
a manual reply)

On 2026-07-28T05:02:11, Desapogu Jayaramudu <[email protected]> wrote:
> serial: coreboot: Accept DBG2 subtype 0x12
>
> U-Boot running as a coreboot payload obtains serial console
> information either from the coreboot table or from the ACPI DBG2
> table.
>
> When serial console support is disabled in coreboot, the coreboot
> table does not contain serial information. In this case U-Boot falls
> back to parsing the DBG2 table.
>
> Some AMD platforms or any other platforms that uses memory-mapped I/O
> advertise the debug UART using DBG2 subtype 0x0012 ("16550-compatible
> with parameters defined in Generic Address Structure"). U-Boot
> currently accepts only subtype 0x0000, causing DBG2 UART detection to
> fail and leaving no console output.
>
> Accept subtype 0x0012 as well and keep the existing handling logic.
>
> Signed-off-by: Desapogu Jayaramudu <[email protected]>
>
> drivers/serial/serial_coreboot.c | 3 ++-
>  include/acpi/acpi_table.h        | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <[email protected]>

Some nits / thoughts below:

> diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c
> @@ -59,7 +59,8 @@ static int read_dbg2(struct ns16550_plat *plat)
> +     if (dbg->port_subtype != ACPI_DBG2_16550_COMPATIBLE &&
> +         dbg->port_subtype != ACPI_DBG2_16550_WITH_GAS) {

Please remove the trailing whitespace after the '{'

Also subtype 0x12 means the register parameters come from the Generic
Address Structure, but the code just below still hardcodes:

    plat->reg_width = 4; /* coreboot sets bit_width to 0 */

That comment only holds for the 0x0000 case. For a genuine 0x12 entry
the GAS bit_width is meaningful, so forcing 4 may give the wrong width
on platforms that advertise something else. Just to check - do your
AMD platforms populate bit_width in the GAS, and if so should
reg_width be derived from it, perhaps as a follow-up? What do you
think?

Regards,
Simon