| Message ID | 20230327171546.v3.16.I00095fb6a52abfda4447281cc13a5c5b76902dda@changeid |
|---|---|
| State | Superseded |
| Delegated to: | Bin Meng |
| Headers | show |
| Series | x86: Various minor enhancements for coreboot | expand |
Hi Simon, On Mon, Mar 27, 2023 at 12:17 PM Simon Glass <sjg@chromium.org> wrote: > > When coreboot does not pass a UART in its sysinfo struct, there is no > easy way to find it out. Add a way to specify known UARTs so we can > find them without needing help from coreboot. > > Since coreboot does not actually init the serial device when serial is > disabled, it is not possible to make it add this information to the > sysinfo table. > > Also, we cannot use the class information, since we don't know which > UART is being used. For example, with Alder Lake there are two: > > 00.16.00 0x8086 0x51e0 Simple comm. controller 0x80 > 00.1e.00 0x8086 0x51a8 Simple comm. controller 0x80 > > In our case the second one is the right one, but thre is no way to > distinguish it from the first one without using the device ID. > > If we have Adler Lake hardware which uses a different UART, we could > perhaps look at the ACPI tables, or the machine information passed in > the SMBIOS tables. > > This was discussed previously before: [1] > > [1] https://patchwork.ozlabs.org/project/uboot/patch/20210407163159.3.I967ea8c85e009f870c7aa944372d32c990f1b14a@changeid/ > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > (no changes since v2) > > Changes in v2: > - Move this patch to last in the series, so it can be dropped if needed Since now we can parse the DBG2 ACPI table to get the UART info, I'd rather not apply this patch. > > arch/x86/dts/coreboot.dts | 4 ++++ > drivers/serial/serial_coreboot.c | 41 ++++++++++++++++++++++++++++++++ > include/pci_ids.h | 3 +++ > 3 files changed, 48 insertions(+) > Regards, Bin
diff --git a/arch/x86/dts/coreboot.dts b/arch/x86/dts/coreboot.dts index f9ff5346a79b..1abd3fddd15c 100644 --- a/arch/x86/dts/coreboot.dts +++ b/arch/x86/dts/coreboot.dts @@ -14,6 +14,8 @@ /include/ "rtc.dtsi" #include "tsc_timer.dtsi" +#include <dt-bindings/pci/pci.h> +#include <pci_ids.h> / { model = "coreboot x86 payload"; @@ -34,6 +36,8 @@ pci { compatible = "pci-x86"; bootph-all; + u-boot,pci-pre-reloc = < + PCI_VENDEV(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ADP_P_UART0) >; }; serial: serial { diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c index 23066e4d0543..bb3a5362dfaf 100644 --- a/drivers/serial/serial_coreboot.c +++ b/drivers/serial/serial_coreboot.c @@ -17,6 +17,12 @@ DECLARE_GLOBAL_DATA_PTR; +static const struct pci_device_id ids[] = { + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ADP_P_UART0) }, + {}, +}; + static int read_dbg2(struct ns16550_plat *plat) { struct acpi_table_header *tab; @@ -94,6 +100,39 @@ static int read_dbg2(struct ns16550_plat *plat) return 0; } +/* + * Coreboot only sets up the UART if it uses it and doesn't bother to put the + * details in sysinfo if it doesn't. Try to guess in that case, using devices + * we know about + * + * @plat: Platform data to fill in + * @return 0 if found, -ve if no UART was found + */ +static int guess_uart(struct ns16550_plat *plat) +{ + struct udevice *bus, *dev; + ulong addr; + int index; + int ret; + + ret = uclass_first_device_err(UCLASS_PCI, &bus); + if (ret) + return ret; + index = 0; + ret = pci_bus_find_devices(bus, ids, &index, &dev); + if (ret) + return ret; + addr = dm_pci_read_bar32(dev, 0); + plat->base = addr; + plat->reg_shift = 2; + plat->reg_width = 4; + plat->clock = 1843200; + plat->fcr = UART_FCR_DEFVAL; + plat->flags = 0; + + return 0; +} + static int coreboot_of_to_plat(struct udevice *dev) { struct ns16550_plat *plat = dev_get_plat(dev); @@ -113,6 +152,8 @@ static int coreboot_of_to_plat(struct udevice *dev) } else if (IS_ENABLED(CONFIG_COREBOOT_SERIAL_FROM_DBG2)) { ret = read_dbg2(plat); } + if (ret && CONFIG_IS_ENABLED(PCI)) + ret = guess_uart(plat); if (ret) { /* diff --git a/include/pci_ids.h b/include/pci_ids.h index 88b0a6404585..5ae1b9b7fb6e 100644 --- a/include/pci_ids.h +++ b/include/pci_ids.h @@ -2992,6 +2992,9 @@ #define PCI_DEVICE_ID_INTEL_UNC_R3QPI1 0x3c45 #define PCI_DEVICE_ID_INTEL_JAKETOWN_UBOX 0x3ce0 #define PCI_DEVICE_ID_INTEL_IOAT_SNB 0x402f +#define PCI_DEVICE_ID_INTEL_ADP_P_UART0 0x51a8 +#define PCI_DEVICE_ID_INTEL_ADP_P_UART1 0x51a9 +#define PCI_DEVICE_ID_INTEL_APL_UART2 0x5ac0 #define PCI_DEVICE_ID_INTEL_5100_16 0x65f0 #define PCI_DEVICE_ID_INTEL_5100_19 0x65f3 #define PCI_DEVICE_ID_INTEL_5100_21 0x65f5
When coreboot does not pass a UART in its sysinfo struct, there is no easy way to find it out. Add a way to specify known UARTs so we can find them without needing help from coreboot. Since coreboot does not actually init the serial device when serial is disabled, it is not possible to make it add this information to the sysinfo table. Also, we cannot use the class information, since we don't know which UART is being used. For example, with Alder Lake there are two: 00.16.00 0x8086 0x51e0 Simple comm. controller 0x80 00.1e.00 0x8086 0x51a8 Simple comm. controller 0x80 In our case the second one is the right one, but thre is no way to distinguish it from the first one without using the device ID. If we have Adler Lake hardware which uses a different UART, we could perhaps look at the ACPI tables, or the machine information passed in the SMBIOS tables. This was discussed previously before: [1] [1] https://patchwork.ozlabs.org/project/uboot/patch/20210407163159.3.I967ea8c85e009f870c7aa944372d32c990f1b14a@changeid/ Signed-off-by: Simon Glass <sjg@chromium.org> --- (no changes since v2) Changes in v2: - Move this patch to last in the series, so it can be dropped if needed arch/x86/dts/coreboot.dts | 4 ++++ drivers/serial/serial_coreboot.c | 41 ++++++++++++++++++++++++++++++++ include/pci_ids.h | 3 +++ 3 files changed, 48 insertions(+)