MIPs ralink need a special tratement regarding the way it handles PCI IO
resources. On MIPS I/O ports are memory mapped, so we access them using normal
load/store instructions. MIPS 'plat_mem_setup()' function does a call to
'set_io_port_base(KSEG1)'. There, variable 'mips_io_port_base'
is set then using this address which is a virtual address to which all
ports are being mapped. Ralink I/O space has a mapping of bus address
equal to the window into the mmio space, with an offset of IO start range
cpu address. This means that to have this working we need:
- linux port numbers in the range 0-0xffff.
- pci port numbers in the range 0-0xffff.
- io_offset being zero.
These means at the end to have bus address 0 mapped to IO range cpu address.
We need a way of properly set 'mips_io_port_base' with a virtually mapped
value of the IO cpu address.
This series do the following approach:
1) Revert two bad commit from a previous attempt of make this work [0].
2) Set PCI_IOBASE to mips 'mips_io_port_base'.
3) Allow architecture dependent 'pci_remap_iospace'.
4) Implement 'pci_remap_iospace' for MIPS.
5) Be sure IOBASE address for IO window is set with correct value.
More context about this series appoach in this mail thread [1].
Thanks in advance for your time.
[0]: https://www.spinics.net/lists/kernel/msg4051474.html
[1]: https://lkml.org/lkml/2021/9/22/6
Sergio Paracuellos (6):
Revert "MIPS: ralink: don't define PC_IOBASE but increase
IO_SPACE_LIMIT"
Revert "staging: mt7621-pci: set end limit for 'ioport_resource'"
MIPS: ralink: set PCI_IOBASE to 'mips_io_port_base'
PCI: allow architecture specific implementation of pci_remap_iospace()
MIPS: implement architecture dependent 'pci_remap_iospace()'
staging: mt7621-pci: properly adjust base address for the IO window
arch/mips/include/asm/mach-ralink/spaces.h | 4 +++-
arch/mips/include/asm/pci.h | 2 ++
arch/mips/pci/pci-generic.c | 9 +++++++++
drivers/pci/pci.c | 2 ++
drivers/staging/mt7621-pci/pci-mt7621.c | 4 +---
5 files changed, 17 insertions(+), 4 deletions(-)
--
2.25.1
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 <[email protected]>
---
arch/mips/include/asm/pci.h | 2 ++
arch/mips/pci/pci-generic.c | 9 +++++++++
2 files changed, 11 insertions(+)
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;
+}
--
2.25.1
On Fri, Sep 24, 2021 at 11:11 PM Sergio Paracuellos
<[email protected]> 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 <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>
> +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 <[email protected]> wrote:
>
> On Fri, Sep 24, 2021 at 11:11 PM Sergio Paracuellos
> <[email protected]> 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 <[email protected]>
>
> Acked-by: Arnd Bergmann <[email protected]>
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
<[email protected]> 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 <[email protected]> wrote:
>
> On Sat, Sep 25, 2021 at 8:10 PM Sergio Paracuellos
> <[email protected]> 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