[U-Boot] [PATCH 4/5] x86: Support PCI UART in the x86_serial driver

[Bin Meng]

Hi Simon,

On Sat, Dec 20, 2014 at 1:00 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 19 December 2014 at 19:43, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Hi Simon,
>>
>> On Sat, Dec 20, 2014 at 5:52 AM, Simon Glass <sjg at chromium.org> wrote:
>>> Hi BIn,
>>>
>>> On 19 December 2014 at 00:19, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>> Newer x86 Platform Controller Hub chipset starts to integrate NS16550
>>>> compatible PCI UART devices. The board configuration file needs to
>>>> supply the PCI UART vendor ID and device ID via CONFIG_PCI_UART_DEV
>>>> if we want to use the PCI UART as the U-Boot serial console.
>>>>
>>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>>> ---
>>>>
>>>>  drivers/serial/serial_x86.c | 30 ++++++++++++++++++++++++++++++
>>>>  1 file changed, 30 insertions(+)
>>>>
>>>> diff --git a/drivers/serial/serial_x86.c b/drivers/serial/serial_x86.c
>>>> index e81e035..7ddd596 100644
>>>> --- a/drivers/serial/serial_x86.c
>>>> +++ b/drivers/serial/serial_x86.c
>>>> @@ -8,6 +8,17 @@
>>>>  #include <dm.h>
>>>>  #include <ns16550.h>
>>>>  #include <serial.h>
>>>> +#include <asm/pci.h>
>>>> +#include <errno.h>
>>>> +
>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>> +
>>>> +#ifdef CONFIG_PCI_UART
>>>> +static struct pci_device_id uart_supported[] = {
>>>> +       CONFIG_PCI_UART_DEV,
>>>> +       { }
>>>> +};
>>>> +#endif
>>>>
>>>>  static const struct udevice_id x86_serial_ids[] = {
>>>>         { .compatible = "x86-uart" },
>>>> @@ -17,6 +28,9 @@ static const struct udevice_id x86_serial_ids[] = {
>>>>  static int x86_serial_ofdata_to_platdata(struct udevice *dev)
>>>>  {
>>>>         struct ns16550_platdata *plat = dev_get_platdata(dev);
>>>> +#ifdef CONFIG_PCI_UART
>>>> +       pci_dev_t devbusfn;
>>>> +#endif
>>>>         int ret;
>>>>
>>>>         ret = ns16550_serial_ofdata_to_platdata(dev);
>>>> @@ -24,6 +38,22 @@ static int x86_serial_ofdata_to_platdata(struct udevice *dev)
>>>>                 return ret;
>>>>         plat->clock = 1843200;
>>>>
>>>> +#ifdef CONFIG_PCI_UART
>>>> +       /*
>>>> +        * Newer x86 Platform Controller Hub chipset starts to integrate
>>>> +        * NS16550 compatible PCI UART devices. The board configuration
>>>> +        * file needs to supply the PCI UART vendor ID and device ID via
>>>> +        * CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot
>>>> +        * serial console.
>>>> +        */
>>>> +       devbusfn = pci_early_find_devices(uart_supported, 0);
>>>> +       if (devbusfn == -1)
>>>> +               return -ENODEV;
>>>
>>> I'm not sure why we need this. Is it because we don't know the device
>>> address of the UART?
>>>
>>
>> Yes, the UART device is not on the host bridge (bus 0). It is on the
>> PCH which is connected to the one of the downstream PCIe port of the
>> host bridge. Which PCIe port is used is determined by the board
>> designer. So that on my Crown Bay board the UART is on b.d.f 2.10.1
>> may become b.d.f 3.10.1 on some other queensbay platform board.
>> Actually the pci_find_devices() is a standard way to locate the
>> device's b.d.f, as you see in many PCI ethernet drivers in U-Boot. You
>> have no way to figure out the device will be inserted to which PCI
>> slot on the board.
>
> OK I see, thanks. Is it possible to find the serial port by class
> rather than having to specify every vendor/ID? If not, is there a
> binding that allows us to add device tree nodes for the serial ports
> (even with just vendor/device, not full PCI bus address) and be able
> to specify which is the console that way? It would be good to avoid a
> new CONFIG if we can. It feels odd to override an existing device tree
> node.
>

Unforuntately not like USB OHCI/EHCI, we cannot use class to determine
it is a serial port device. Currently there is no device tree bindings
for PCI device drivers. Maybe this can be done when driver model
supports PCI?

Regards,
Bin