| Message ID | 1419682210-21181-6-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Bin, On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@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@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? > 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. > > +cont: > plat->base = addr; > plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset, > "reg-shift", 1); > -- > 1.8.2.1 > Regards, Simon
Hi Simon, On Sun, Dec 28, 2014 at 9:55 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@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@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
Hi Bin, On 28 December 2014 at 23:15, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Sun, Dec 28, 2014 at 9:55 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@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@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. Seems fine, but might want to write it out in full. > >>> 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, Simon
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 */ 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 */ + 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; + } +cont: plat->base = addr; plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "reg-shift", 1);
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 i/o mapped. The driver will try to use memory mapped register if the reg property in the node has an entry to describe the memory mapped register, otherwise i/o mapped register will be used. Signed-off-by: Bin Meng <bmeng.cn@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(-)