[U-Boot] [PATCH 5/8] drivers: serial: Add ns16550 compatible pci uart driver
Simon Glass
sjg at chromium.org
Tue Aug 18 04:00:31 CEST 2015
Hi Bin,
On 17 August 2015 at 18:20, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Tue, Aug 18, 2015 at 6:14 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Bin,
>>
>> On 15 August 2015 at 01:07, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> This adds a new driver to support National Semiconductor 16550
>>> compatible UART device with PCI interface. The initial support
>>> only adds device IDs for Intel Topcliff chipset UART devices.
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>> ---
>>>
>>> drivers/serial/Kconfig | 9 ++++++
>>> drivers/serial/Makefile | 1 +
>>> drivers/serial/serial_pci.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 85 insertions(+)
>>> create mode 100644 drivers/serial/serial_pci.c
>>>
>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>>> index fd126a8..f2eccdd 100644
>>> --- a/drivers/serial/Kconfig
>>> +++ b/drivers/serial/Kconfig
>>> @@ -128,3 +128,12 @@ config X86_SERIAL
>>> enabled in the device tree with the correct input clock frequency
>>> provided (default 1843200). Enable this to obtain serial console
>>> output.
>>> +
>>> +config PCI_SERIAL
>>> + bool "Support for 16550 serial port on PCI bus"
>>> + depends on DM_PCI
>>> + default n
>>> + help
>>> + This is the UART driver for ns16550 compatible chipset with PCI
>>> + interface. This can be enabled in the device tree with the correct
>>> + properties provided. If unsure, say N.
>>> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
>>> index 1d1f036..a7e2cd2 100644
>>> --- a/drivers/serial/Makefile
>>> +++ b/drivers/serial/Makefile
>>> @@ -45,6 +45,7 @@ obj-$(CONFIG_TEGRA_SERIAL) += serial_tegra.o
>>> obj-$(CONFIG_UNIPHIER_SERIAL) += serial_uniphier.o
>>> obj-$(CONFIG_OMAP_SERIAL) += serial_omap.o
>>> obj-$(CONFIG_X86_SERIAL) += serial_x86.o
>>> +obj-$(CONFIG_PCI_SERIAL) += serial_pci.o
>>> obj-$(CONFIG_STM32_SERIAL) += serial_stm32.o
>>>
>>> ifndef CONFIG_SPL_BUILD
>>> diff --git a/drivers/serial/serial_pci.c b/drivers/serial/serial_pci.c
>>> new file mode 100644
>>> index 0000000..bc87c9a
>>> --- /dev/null
>>> +++ b/drivers/serial/serial_pci.c
>>> @@ -0,0 +1,75 @@
>>> +/*
>>> + * Copyright (C) 2015, Bin Meng <bmeng.cn at gmail.com>
>>> + *
>>> + * This driver aims to support National Semiconductor 16550 compatible
>>> + * UART device with PCI interface.
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <dm.h>
>>> +#include <fdtdec.h>
>>> +#include <ns16550.h>
>>> +#include <serial.h>
>>> +
>>> +DECLARE_GLOBAL_DATA_PTR;
>>> +
>>> +static struct pci_device_id supported[] = {
>>> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_TCF_UART_0) },
>>> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_TCF_UART_1) },
>>> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_TCF_UART_2) },
>>> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_TCF_UART_3) },
>>
>> Ick, did I miss the discussion on this? I really want us to keep this
>> stuff in the device tree.
>>
>
> I don't think you missed any discussion. But I think you might misread
> this code? Do you mean there is no need to declare what devices are
> supported by this driver using U_BOOT_PCI_DEVICE?
OK but we have gone from a generic driver that worked with anything to
one where you have to enable it for every ID on every board. How is
that an improvement?
>
>>> + {}
>>> +};
>>> +
>>> +static const struct udevice_id pci_serial_ids[] = {
>>> + { .compatible = "pci-uart" },
>>> + { }
>>> +};
>>> +
>>> +static int pci_serial_ofdata_to_platdata(struct udevice *dev)
>>> +{
>>> + struct ns16550_platdata *plat = dev_get_platdata(dev);
>>> + struct fdt_pci_addr pci_addr;
>>> + u32 bar;
>>> + int ret;
If we have a device tree node, why do we need the U_BOOT_PCI_DEVICE() stuff?
>>> +
>>> + /* we prefer to use a memory-mapped register */
>>> + ret = fdtdec_get_pci_addr(gd->fdt_blob, dev->of_offset,
>>> + FDT_PCI_SPACE_MEM32, "reg", &pci_addr);
>>> + if (ret) {
>>> + /* try if there is any i/o-mapped register */
>>> + ret = fdtdec_get_pci_addr(gd->fdt_blob, dev->of_offset,
>>> + FDT_PCI_SPACE_IO, "reg", &pci_addr);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + ret = fdtdec_get_pci_bar32(gd->fdt_blob, dev->of_offset,
>>> + &pci_addr, &bar);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + plat->base = bar;
>>> + plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>>> + "reg-shift", 1);
>>> + plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>>> + "clock-frequency", 1843200);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +U_BOOT_DRIVER(serial_pci) = {
>>> + .name = "serial_pci",
>>> + .id = UCLASS_SERIAL,
>>> + .of_match = pci_serial_ids,
>>> + .ofdata_to_platdata = pci_serial_ofdata_to_platdata,
>>> + .platdata_auto_alloc_size = sizeof(struct ns16550_platdata),
>>> + .priv_auto_alloc_size = sizeof(struct NS16550),
>>> + .probe = ns16550_serial_probe,
>>> + .ops = &ns16550_serial_ops,
>>> + .flags = DM_FLAG_PRE_RELOC,
>>> +};
>>> +
>>> +U_BOOT_PCI_DEVICE(serial_pci, supported);
>>> --
>
> Regards,
> Bin
Regards,
Simon
More information about the U-Boot
mailing list