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

[Bin Meng]

Hi Simon,

On Tue, Dec 23, 2014 at 8:52 AM, Simon Glass <sjg at chromium.org> wrote:
> 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.

Yep, I missed this and looks like this should be enough for the
ns16550 compatible pci uart devices.

> 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.

OK, let me try it.

> 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,
Bin