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

[Simon Glass]

HI Bin,

On 19 December 2014 at 22:36, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> 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?

Sorry for the delay.

If you look here you can see a class called ''16550 compatable serial
controller'

http://www.acm.uiuc.edu/sigops/roll_your_own/7.c.1.html

If that means what it says it might be good enough.

I looked through the bindings and it is possible to use PCI with device tree.

http://www.o3one.org/hwdocs/openfirmware/pci_supplement_2_1.pdf

So one option would be to actually add your serial ports for this
board to the device tree. Then you could use the stdout property to
specify which is used for the serial port, and everything should just
work.

Let me know what you think about that idea. With this patch I worry
that we will end up adding a big long list of PCI IDs plus not have
control of the console UART.

I'm not opposed to this patch if it is the best way, I just want to
make sure we have explored other options.

Regards,
Simon