[U-Boot] [PATCH v2 5/7] serial: ns16550: Support ns16550 compatible pci uart devices

[Bin Meng]

Hi Simon,

On Sun, Dec 28, 2014 at 9:55 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 27 December 2014 at 05:10, Bin Meng <bmeng.cn at gmail.com> wrote:
>> There are many pci uart devices which are ns16550 compatible. We can
>> describe them in the board dts file and use it as the U-Boot serial
>> console as specified in the chosen node 'stdout-path' property.
>>
>> Those pci uart devices can have their register be memory mapped, or
>
> memory-mapped
>
>> i/o mapped. The driver will try to use memory mapped register if the
>
> i/o-mapped
>
> s/memory mapped/the memory-mapped/
>
>> reg property in the node has an entry to describe the memory mapped
>
> memory-mapped
>
>> register, otherwise i/o mapped register will be used.
>
> i/o-mapped
>
>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>
>> ---
>>
>> Changes in v2:
>> - New patch to support ns16550 compatible pci uart devices
>>
>>  drivers/serial/ns16550.c | 33 ++++++++++++++++++++++++++++++++-
>>  1 file changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
>> index af5beba..2001fac 100644
>> --- a/drivers/serial/ns16550.c
>> +++ b/drivers/serial/ns16550.c
>> @@ -289,10 +289,41 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
>>         struct ns16550_platdata *plat = dev->platdata;
>>         fdt_addr_t addr;
>>
>> +       /* try plb device first */
>
> What is plb?

I mean Processor Local Bus, a concept I believe is popular in the
embedded powerpc world, but I realized it might not be that popuar to
other architectures. Maybe I could just remove it.

>>         addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
>> -       if (addr == FDT_ADDR_T_NONE)
>> +       if (addr == FDT_ADDR_T_NONE) {
>> +#ifdef CONFIG_PCI
>> +               /* then try pci device */
>> +               struct fdt_pci_addr pci_addr;
>> +               u32 bar;
>> +               int ret;
>> +
>> +               /* we prefer to use memory mapped register */
>
> a memory-mapped
>
>> +               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 io 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;
>> +
>> +               addr = bar;
>> +               goto cont;
>> +#endif
>>                 return -EINVAL;
>> +       }
>
> Instead of the above 4 lines, move the #ifdef CONFIG_PCI up one line, then here:
>
>    }
> if (addr == FDT_ADDR_T_NONE)
>    return -EINVAL;
>
> This avoids the goto.

Yep, this is better. Will fix it.

>>
>> +cont:
>>         plat->base = addr;
>>         plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>>                                          "reg-shift", 1);
>> --
>> 1.8.2.1
>>

Regards,
Bin