| Message ID | 20210924211139.3477-6-sergio.paracuellos@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: ralink: fix PCI IO resources | expand |
On Fri, Sep 24, 2021 at 11:11 PM Sergio Paracuellos <sergio.paracuellos@gmail.com> wrote: > > To make PCI IO work we need to properly virtually map IO cpu physical address > and set this virtual address as the address of the first PCI IO port which > is set using function 'set_io_port_base()'. > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> Acked-by: Arnd Bergmann <arnd@arndb.de> > +int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) > +{ > + size_t size = (res->end - res->start) + 1; > + unsigned long vaddr = (unsigned long)ioremap(phys_addr, size); > + > + set_io_port_base(vaddr); > + return 0; > +} It might be good to check that res->start is zero here, otherwise the io_port_base would be off. That could happen if you ever have more than one bridge. Arnd
Hi Arnd, On Sat, Sep 25, 2021 at 7:32 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Fri, Sep 24, 2021 at 11:11 PM Sergio Paracuellos > <sergio.paracuellos@gmail.com> wrote: > > > > To make PCI IO work we need to properly virtually map IO cpu physical address > > and set this virtual address as the address of the first PCI IO port which > > is set using function 'set_io_port_base()'. > > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> > > Acked-by: Arnd Bergmann <arnd@arndb.de> Thanks! > > > +int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) > > +{ > > + size_t size = (res->end - res->start) + 1; > > + unsigned long vaddr = (unsigned long)ioremap(phys_addr, size); > > + > > + set_io_port_base(vaddr); > > + return 0; > > +} > > It might be good to check that res->start is zero here, otherwise > the io_port_base would be off. That could happen if you ever have more > than one bridge. Do you mean something like the following? int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) { unsigned long vaddr; size_t size; if (res->start != 0) { // Should I WARN_ONCE or just show an error/warning message?? WARN_ONCE(1, "resource start must be zero\n"); return -ENODEV; } size = (res->end - res->start) + 1; vaddr = (unsigned long)ioremap(phys_addr, size); return 0; } Thanks, Sergio Paracuellos > > Arnd
On Sat, Sep 25, 2021 at 8:10 PM Sergio Paracuellos <sergio.paracuellos@gmail.com> wrote: > > It might be good to check that res->start is zero here, otherwise > > the io_port_base would be off. That could happen if you ever have more > > than one bridge. > > Do you mean something like the following? Yes, exactly. > int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) > { > unsigned long vaddr; > size_t size; > > if (res->start != 0) { > // Should I WARN_ONCE or just show an error/warning message?? > WARN_ONCE(1, "resource start must be zero\n"); > return -ENODEV; > } I don't care if it's WARN(), WARN_ONCE() or pr_warn(). If we ever see the message, the system is not working and the person who caused the problem will figure it out. Arnd
Hi Arnd, On Sat, Sep 25, 2021 at 9:34 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Sat, Sep 25, 2021 at 8:10 PM Sergio Paracuellos > <sergio.paracuellos@gmail.com> wrote: > > > It might be good to check that res->start is zero here, otherwise > > > the io_port_base would be off. That could happen if you ever have more > > > than one bridge. > > > > Do you mean something like the following? > > Yes, exactly. > > > int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) > > { > > unsigned long vaddr; > > size_t size; > > > > if (res->start != 0) { > > // Should I WARN_ONCE or just show an error/warning message?? > > WARN_ONCE(1, "resource start must be zero\n"); > > return -ENODEV; > > } > > I don't care if it's WARN(), WARN_ONCE() or pr_warn(). If we ever see the > message, the system is not working and the person who caused the problem > will figure it out. Pretty clear, thanks. I will collect you Acked-by's and make this change and send v3. Best regards, Sergio Paracuellos > > Arnd
diff --git a/arch/mips/include/asm/pci.h b/arch/mips/include/asm/pci.h index 9ffc8192adae..35270984a5f0 100644 --- a/arch/mips/include/asm/pci.h +++ b/arch/mips/include/asm/pci.h @@ -20,6 +20,8 @@ #include <linux/list.h> #include <linux/of.h> +#define pci_remap_iospace pci_remap_iospace + #ifdef CONFIG_PCI_DRIVERS_LEGACY /* diff --git a/arch/mips/pci/pci-generic.c b/arch/mips/pci/pci-generic.c index 95b00017886c..877ec9d6a614 100644 --- a/arch/mips/pci/pci-generic.c +++ b/arch/mips/pci/pci-generic.c @@ -46,3 +46,12 @@ void pcibios_fixup_bus(struct pci_bus *bus) { pci_read_bridge_bases(bus); } + +int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) +{ + size_t size = (res->end - res->start) + 1; + unsigned long vaddr = (unsigned long)ioremap(phys_addr, size); + + set_io_port_base(vaddr); + return 0; +}
To make PCI IO work we need to properly virtually map IO cpu physical address and set this virtual address as the address of the first PCI IO port which is set using function 'set_io_port_base()'. Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> --- arch/mips/include/asm/pci.h | 2 ++ arch/mips/pci/pci-generic.c | 9 +++++++++ 2 files changed, 11 insertions(+)