[PATCH 03/17] x86: Allow coreboot serial driver to guess the UART

[Bin Meng]

Hi Simon,

On Wed, Apr 7, 2021 at 12:32 PM Simon Glass <sjg at chromium.org> wrote:
>
> At present this driver relies on coreboot to provide information about
> the console UART. However if coreboot is not compiled with the UART
> enabled, the information is left out. This configuration is quite
> common, e.g. with shipping x86-based Chrome OS Chromebooks.
>
> Add a way to determine the UART settings in this case, using a
> hard-coded list of PCI IDs.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
>  drivers/serial/serial_coreboot.c | 68 ++++++++++++++++++++++++++++----
>  include/pci_ids.h                |  1 +
>  2 files changed, 61 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c
> index de09c8681f5..4b4619432d8 100644
> --- a/drivers/serial/serial_coreboot.c
> +++ b/drivers/serial/serial_coreboot.c
> @@ -11,19 +11,71 @@
>  #include <serial.h>
>  #include <asm/cb_sysinfo.h>
>
> +static const struct pci_device_id ids[] = {
> +       { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) },
> +       {},
> +};
> +
> +/*
> + * 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)

This is really not a guess, but use a pre-configured platform data.
Also this only work for Apollo Lake board, and will break other boards
if they don't have cbinfo available.

Why not just simply put a serial node in the device tree and we are all done?

> +{
> +       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);
>         struct cb_serial *cb_info = lib_sysinfo.serial;
>
> -       plat->base = cb_info->baseaddr;
> -       plat->reg_shift = cb_info->regwidth == 4 ? 2 : 0;
> -       plat->reg_width = cb_info->regwidth;
> -       plat->clock = cb_info->input_hertz;
> -       plat->fcr = UART_FCR_DEFVAL;
> -       plat->flags = 0;
> -       if (cb_info->type == CB_SERIAL_TYPE_IO_MAPPED)
> -               plat->flags |= NS16550_FLAG_IO;
> +       if (cb_info) {
> +               plat->base = cb_info->baseaddr;
> +               plat->reg_shift = cb_info->regwidth == 4 ? 2 : 0;
> +               plat->reg_width = cb_info->regwidth;
> +               plat->clock = cb_info->input_hertz;
> +               plat->fcr = UART_FCR_DEFVAL;
> +               plat->flags = 0;
> +               if (cb_info->type == CB_SERIAL_TYPE_IO_MAPPED)
> +                       plat->flags |= NS16550_FLAG_IO;
> +       } else if (CONFIG_IS_ENABLED(PCI)) {
> +               int ret;
> +
> +               ret = guess_uart(plat);
> +               if (ret) {
> +                       /*
> +                        * Returning an error will cause U-Boot to complain that
> +                        * there is no UART, which may panic. So stay silent and
> +                        * pray that the video console will work.
> +                        */
> +                       log_debug("Cannot detect UART\n");
> +               }
> +       }
>
>         return 0;
>  }
> diff --git a/include/pci_ids.h b/include/pci_ids.h
> index 7ecedc7f04c..d91c1d08f1a 100644
> --- a/include/pci_ids.h
> +++ b/include/pci_ids.h
> @@ -2987,6 +2987,7 @@
>  #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_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
> --

Regards,
Bin