[U-Boot] [PATCH 5/8] drivers: serial: Add ns16550 compatible pci uart driver
Bin Meng
bmeng.cn at gmail.com
Tue Aug 18 04:25:19 CEST 2015
Hi Simon,
On Tue, Aug 18, 2015 at 10:00 AM, Simon Glass <sjg at chromium.org> wrote:
> 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?
>
Agreed, but given fdtdec_get_addr() is broken (see previous
discussion), I decided to split that from the generic 16550 driver.
Although I see you revert that commit, I am not sure how
fdtdec_get_addr() will be changed in the future.
>>
>>>> + {}
>>>> +};
>>>> +
>>>> +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?
>
This is to have pci bus to filter the device creation. I see if I
define U_BOOT_PCI_DEVICE() and with a device node at the same time, it
will show twice in the 'dm tree' output. One with a driver name
'pci_serial' and the other one with 'serial'. Maybe I am using it
wrong?
>>>> +
>>>> + /* 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
More information about the U-Boot
mailing list