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].
Patches related with reverts are from this merge cycle so they are only
added to the staging git tree. So to have all stuff together I'd like to
get everybody Ack's to get all of this series through staging tree if
possible :).
Thanks in advance for your time.
Changes in v3:
- Collect Arnd's Acked-by for the patches.
- Be sure IO resource start address is zero and WARN_ONCE if it is not
on MIPS pci_remap_iospace() patch. Also make use of 'resource_size'
instead of do the logic explicitly again.
Changes in v2:
- re-do commit messages for PCI patch as Bjorn pointed out in previous series.
- Add Bjorn's Acked-by for PCI subsystem patch.
- Re-do commit message of MIPS 'pci_remap_iospace()' patch to align with changes
in the PCI patch (s/architecture dependent/architecture-specific/)
- Add Fixes-by tag for MIPS set PCI_IOBASE patch.
[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 pci_remap_iospace()
MIPS: implement architecture-specific '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 | 14 ++++++++++++++
drivers/pci/pci.c | 2 ++
drivers/staging/mt7621-pci/pci-mt7621.c | 4 +---
5 files changed, 22 insertions(+), 4 deletions(-)
--
2.25.1
This reverts commit 50fb34eca2944fd67493717c9fbda125336f1655.
Since IO_SPACE_LIMIT is not really being changed there is no
real need to adjust the ioport_resource end limit.
Acked-by: Arnd Bergmann <[email protected]>
Signed-off-by: Sergio Paracuellos <[email protected]>
---
drivers/staging/mt7621-pci/pci-mt7621.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c
index 86d9c3d122e2..6acfc94a16e7 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -526,8 +526,6 @@ static int mt7621_pci_probe(struct platform_device *pdev)
if (!dev->of_node)
return -ENODEV;
- ioport_resource.end = IO_SPACE_LIMIT;
-
bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
if (!bridge)
return -ENOMEM;
--
2.25.1
The value to adjust in the bridge register RALINK_PCI_IOBASE must take into
account the raw value from DT, not only the translated linux port number.
As long as io_offset is zero, the two are the same, but if you were to use
multiple host bridge in the system, or pick a different bus address in DT,
you can have a nonzero io_offset. At this means to take into account the
bus address which is used to calculate this offset, substracting it from
the IO resource start address.
Acked-by: Arnd Bergmann <[email protected]>
Signed-off-by: Sergio Paracuellos <[email protected]>
---
drivers/staging/mt7621-pci/pci-mt7621.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c
index 6acfc94a16e7..503cb1fca2e0 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -482,7 +482,7 @@ static int mt7621_pcie_enable_ports(struct pci_host_bridge *host)
/* Setup MEMWIN and IOWIN */
pcie_write(pcie, 0xffffffff, RALINK_PCI_MEMBASE);
- pcie_write(pcie, entry->res->start, RALINK_PCI_IOBASE);
+ pcie_write(pcie, entry->res->start - entry->offset, RALINK_PCI_IOBASE);
list_for_each_entry(port, &pcie->ports, list) {
if (port->enabled) {
--
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()'.
Acked-by: Arnd Bergmann <[email protected]>
Signed-off-by: Sergio Paracuellos <[email protected]>
---
arch/mips/include/asm/pci.h | 2 ++
arch/mips/pci/pci-generic.c | 14 ++++++++++++++
2 files changed, 16 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..18eb8a453a86 100644
--- a/arch/mips/pci/pci-generic.c
+++ b/arch/mips/pci/pci-generic.c
@@ -46,3 +46,17 @@ 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)
+{
+ unsigned long vaddr;
+
+ if (res->start != 0) {
+ WARN_ONCE(1, "resource start address is not zero\n");
+ return -ENODEV;
+ }
+
+ vaddr = (unsigned long)ioremap(phys_addr, resource_size(res));
+ set_io_port_base(vaddr);
+ return 0;
+}
--
2.25.1
This reverts commit 159697474db41732ef3b6c2e8d9395f09d1f659e.
There is no real need to increase IO_SPACE_LIMIT if PCI_IOBASE
is properly set to 'mips_io_port_base'. Hence revert this commit
first before doing anything else.
Signed-off-by: Sergio Paracuellos <[email protected]>
---
arch/mips/include/asm/mach-ralink/spaces.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/mips/include/asm/mach-ralink/spaces.h b/arch/mips/include/asm/mach-ralink/spaces.h
index 31a3525213cf..87d085c9ad61 100644
--- a/arch/mips/include/asm/mach-ralink/spaces.h
+++ b/arch/mips/include/asm/mach-ralink/spaces.h
@@ -2,7 +2,9 @@
#ifndef __ASM_MACH_RALINK_SPACES_H_
#define __ASM_MACH_RALINK_SPACES_H_
-#define IO_SPACE_LIMIT 0x1fffffff
+#define PCI_IOBASE _AC(0xa0000000, UL)
+#define PCI_IOSIZE SZ_16M
+#define IO_SPACE_LIMIT (PCI_IOSIZE - 1)
#include <asm/mach-generic/spaces.h>
#endif
--
2.25.1
On Sat, Sep 25, 2021 at 10:33 PM Sergio Paracuellos
<[email protected]> wrote:
>
> Patches related with reverts are from this merge cycle so they are only
> added to the staging git tree. So to have all stuff together I'd like to
> get everybody Ack's to get all of this series through staging tree if
> possible :).
>
> Thanks in advance for your time.
Looks all good to me now, just one general remark: Try to give a little more
time between respinning the entire series, otherwise you get the opposite
effect and reviewers start ignoring your emails after getting annoyed at the
number of emails. Once you are reasonably sure that no more comments are
coming in, or you have made substantial changes, it's time to resend the
series.
Arnd
Hi Arnd,
On Mon, Sep 27, 2021 at 9:51 AM Arnd Bergmann <[email protected]> wrote:
>
> On Sat, Sep 25, 2021 at 10:33 PM Sergio Paracuellos
> <[email protected]> wrote:
> >
> > Patches related with reverts are from this merge cycle so they are only
> > added to the staging git tree. So to have all stuff together I'd like to
> > get everybody Ack's to get all of this series through staging tree if
> > possible :).
> >
> > Thanks in advance for your time.
>
> Looks all good to me now, just one general remark: Try to give a little more
> time between respinning the entire series, otherwise you get the opposite
> effect and reviewers start ignoring your emails after getting annoyed at the
> number of emails. Once you are reasonably sure that no more comments are
> coming in, or you have made substantial changes, it's time to resend the
> series.
Thanks for reviewing this and also for the advice. Will take it into
account from now on.
>
> Arnd
Best regards,
Sergio Paracuellos
On Sat, Sep 25, 2021 at 10:32:19PM +0200, Sergio Paracuellos wrote:
> This reverts commit 159697474db41732ef3b6c2e8d9395f09d1f659e.
>
> There is no real need to increase IO_SPACE_LIMIT if PCI_IOBASE
> is properly set to 'mips_io_port_base'. Hence revert this commit
> first before doing anything else.
>
> Signed-off-by: Sergio Paracuellos <[email protected]>
> ---
> arch/mips/include/asm/mach-ralink/spaces.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/include/asm/mach-ralink/spaces.h b/arch/mips/include/asm/mach-ralink/spaces.h
> index 31a3525213cf..87d085c9ad61 100644
> --- a/arch/mips/include/asm/mach-ralink/spaces.h
> +++ b/arch/mips/include/asm/mach-ralink/spaces.h
> @@ -2,7 +2,9 @@
> #ifndef __ASM_MACH_RALINK_SPACES_H_
> #define __ASM_MACH_RALINK_SPACES_H_
>
> -#define IO_SPACE_LIMIT 0x1fffffff
> +#define PCI_IOBASE _AC(0xa0000000, UL)
> +#define PCI_IOSIZE SZ_16M
> +#define IO_SPACE_LIMIT (PCI_IOSIZE - 1)
>
> #include <asm/mach-generic/spaces.h>
> #endif
Acked-by: Thomas Bogendoerfer <[email protected]>
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
On Sat, Sep 25, 2021 at 10:32:23PM +0200, Sergio Paracuellos 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()'.
>
> Acked-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Sergio Paracuellos <[email protected]>
> ---
> arch/mips/include/asm/pci.h | 2 ++
> arch/mips/pci/pci-generic.c | 14 ++++++++++++++
> 2 files changed, 16 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..18eb8a453a86 100644
> --- a/arch/mips/pci/pci-generic.c
> +++ b/arch/mips/pci/pci-generic.c
> @@ -46,3 +46,17 @@ 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)
> +{
> + unsigned long vaddr;
> +
> + if (res->start != 0) {
> + WARN_ONCE(1, "resource start address is not zero\n");
> + return -ENODEV;
> + }
> +
> + vaddr = (unsigned long)ioremap(phys_addr, resource_size(res));
> + set_io_port_base(vaddr);
> + return 0;
> +}
Acked-by: Thomas Bogendoerfer <[email protected]>
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
Hi Greg,
On Sat, Sep 25, 2021 at 10:32 PM Sergio Paracuellos
<[email protected]> wrote:
>
> 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].
>
> Patches related with reverts are from this merge cycle so they are only
> added to the staging git tree. So to have all stuff together I'd like to
> get everybody Ack's to get all of this series through staging tree if
> possible :).
>
> Thanks in advance for your time.
>
> Changes in v3:
> - Collect Arnd's Acked-by for the patches.
> - Be sure IO resource start address is zero and WARN_ONCE if it is not
> on MIPS pci_remap_iospace() patch. Also make use of 'resource_size'
> instead of do the logic explicitly again.
I think nothing is missing to get this added through the staging tree.
Thanks in advance for your time.
Best regards,
Sergio Paracuellos
>
> Changes in v2:
> - re-do commit messages for PCI patch as Bjorn pointed out in previous series.
> - Add Bjorn's Acked-by for PCI subsystem patch.
> - Re-do commit message of MIPS 'pci_remap_iospace()' patch to align with changes
> in the PCI patch (s/architecture dependent/architecture-specific/)
> - Add Fixes-by tag for MIPS set PCI_IOBASE patch.
>
> [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 pci_remap_iospace()
> MIPS: implement architecture-specific '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 | 14 ++++++++++++++
> drivers/pci/pci.c | 2 ++
> drivers/staging/mt7621-pci/pci-mt7621.c | 4 +---
> 5 files changed, 22 insertions(+), 4 deletions(-)
>
> --
> 2.25.1
>
On Sun, Oct 03, 2021 at 06:21:21PM +0200, Sergio Paracuellos wrote:
> Hi Greg,
>
> On Sat, Sep 25, 2021 at 10:32 PM Sergio Paracuellos
> <[email protected]> wrote:
> >
> > 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].
> >
> > Patches related with reverts are from this merge cycle so they are only
> > added to the staging git tree. So to have all stuff together I'd like to
> > get everybody Ack's to get all of this series through staging tree if
> > possible :).
> >
> > Thanks in advance for your time.
> >
> > Changes in v3:
> > - Collect Arnd's Acked-by for the patches.
> > - Be sure IO resource start address is zero and WARN_ONCE if it is not
> > on MIPS pci_remap_iospace() patch. Also make use of 'resource_size'
> > instead of do the logic explicitly again.
>
> I think nothing is missing to get this added through the staging tree.
Great, thanks for sticking with this, will go queue it up now.
greg k-h
On Tue, Oct 5, 2021 at 12:36 PM Greg KH <[email protected]> wrote:
>
> On Sun, Oct 03, 2021 at 06:21:21PM +0200, Sergio Paracuellos wrote:
> > Hi Greg,
> >
> > On Sat, Sep 25, 2021 at 10:32 PM Sergio Paracuellos
> > <[email protected]> wrote:
> > >
> > > 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].
> > >
> > > Patches related with reverts are from this merge cycle so they are only
> > > added to the staging git tree. So to have all stuff together I'd like to
> > > get everybody Ack's to get all of this series through staging tree if
> > > possible :).
> > >
> > > Thanks in advance for your time.
> > >
> > > Changes in v3:
> > > - Collect Arnd's Acked-by for the patches.
> > > - Be sure IO resource start address is zero and WARN_ONCE if it is not
> > > on MIPS pci_remap_iospace() patch. Also make use of 'resource_size'
> > > instead of do the logic explicitly again.
> >
> > I think nothing is missing to get this added through the staging tree.
>
> Great, thanks for sticking with this, will go queue it up now.
Thanks!
Best regards,
Sergio Paracuellos
>
> greg k-h
On Sat, 2021-09-25 at 22:32 +0200, Sergio Paracuellos 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()'.
>
> Acked-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Sergio Paracuellos <[email protected]>
Hi,
the change is causing a WARNING on loongson64g-4core-ls7a:
[ 0.105781] loongson-pci 1a000000.pci: IO 0x0018020000..0x001803ffff ->
0x0000020000
[ 0.105792] loongson-pci 1a000000.pci: MEM 0x0040000000..0x007fffffff ->
0x0040000000
[ 0.105801] ------------[ cut here ]------------
[ 0.105804] WARNING: CPU: 0 PID: 1 at arch/mips/pci/pci-generic.c:55 pci_remap_iospace+0x80/0x88
[ 0.105815] resource start address is not zero
I'm not sure how to fix this one.
> ---
> arch/mips/include/asm/pci.h | 2 ++
> arch/mips/pci/pci-generic.c | 14 ++++++++++++++
> 2 files changed, 16 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..18eb8a453a86 100644
> --- a/arch/mips/pci/pci-generic.c
> +++ b/arch/mips/pci/pci-generic.c
> @@ -46,3 +46,17 @@ 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)
> +{
> + unsigned long vaddr;
> +
> + if (res->start != 0) {
> + WARN_ONCE(1, "resource start address is not zero\n");
> + return -ENODEV;
> + }
> +
> + vaddr = (unsigned long)ioremap(phys_addr, resource_size(res));
> + set_io_port_base(vaddr);
> + return 0;
> +}
--
Xi Ruoyao <[email protected]>
School of Aerospace Science and Technology, Xidian University
On 12/16/2021 07:44 PM, Xi Ruoyao wrote:
> On Sat, 2021-09-25 at 22:32 +0200, Sergio Paracuellos 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()'.
>>
>> Acked-by: Arnd Bergmann <[email protected]>
>> Signed-off-by: Sergio Paracuellos <[email protected]>
>
> Hi,
>
> the change is causing a WARNING on loongson64g-4core-ls7a:
>
> [ 0.105781] loongson-pci 1a000000.pci: IO 0x0018020000..0x001803ffff ->
> 0x0000020000
> [ 0.105792] loongson-pci 1a000000.pci: MEM 0x0040000000..0x007fffffff ->
> 0x0040000000
> [ 0.105801] ------------[ cut here ]------------
> [ 0.105804] WARNING: CPU: 0 PID: 1 at arch/mips/pci/pci-generic.c:55 pci_remap_iospace+0x80/0x88
> [ 0.105815] resource start address is not zero
>
> I'm not sure how to fix this one.
>
MIPS: Only define pci_remap_iospace() for Ralink
https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?h=mips-fixes&id=09d97da660ff77df20984496aa0abcd6b88819f2
On Thu, Dec 16, 2021 at 12:44 PM Xi Ruoyao <[email protected]> wrote:
>
> On Sat, 2021-09-25 at 22:32 +0200, Sergio Paracuellos 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()'.
> >
> > Acked-by: Arnd Bergmann <[email protected]>
> > Signed-off-by: Sergio Paracuellos <[email protected]>
>
> Hi,
>
> the change is causing a WARNING on loongson64g-4core-ls7a:
>
> [ 0.105781] loongson-pci 1a000000.pci: IO 0x0018020000..0x001803ffff ->
> 0x0000020000
> [ 0.105792] loongson-pci 1a000000.pci: MEM 0x0040000000..0x007fffffff ->
> 0x0040000000
> [ 0.105801] ------------[ cut here ]------------
> [ 0.105804] WARNING: CPU: 0 PID: 1 at arch/mips/pci/pci-generic.c:55 pci_remap_iospace+0x80/0x88
> [ 0.105815] resource start address is not zero
>
> I'm not sure how to fix this one.
It looks like this machine has two I/O spaces, one for ISA at 0x18000000/0x00000
and one for PCI at 0x18020000/0x20000, but the implementation assumes there
is only one. If you want to use pci_remap_iospace() on this platform,
it needs to
be extended to allow more than one such space.
Arnd
在2021年12月16日十二月 上午11:44，Xi Ruoyao写道：
> On Sat, 2021-09-25 at 22:32 +0200, Sergio Paracuellos 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()'.
>>
>> Acked-by: Arnd Bergmann <[email protected]>
>> Signed-off-by: Sergio Paracuellos <[email protected]>
>
> Hi,
>
> the change is causing a WARNING on loongson64g-4core-ls7a:
>
> [ 0.105781] loongson-pci 1a000000.pci: IO
> 0x0018020000..0x001803ffff ->
> 0x0000020000
> [ 0.105792] loongson-pci 1a000000.pci: MEM
> 0x0040000000..0x007fffffff ->
> 0x0040000000
> [ 0.105801] ------------[ cut here ]------------
> [ 0.105804] WARNING: CPU: 0 PID: 1 at arch/mips/pci/pci-generic.c:55
> pci_remap_iospace+0x80/0x88
> [ 0.105815] resource start address is not zero
>
> I'm not sure how to fix this one.
>
>> ---
>> arch/mips/include/asm/pci.h | 2 ++
>> arch/mips/pci/pci-generic.c | 14 ++++++++++++++
>> 2 files changed, 16 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..18eb8a453a86 100644
>> --- a/arch/mips/pci/pci-generic.c
>> +++ b/arch/mips/pci/pci-generic.c
>> @@ -46,3 +46,17 @@ 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)
>> +{
>> + unsigned long vaddr;
>> +
>> + if (res->start != 0) {
>> + WARN_ONCE(1, "resource start address is not zero\n");
>> + return -ENODEV;
>> + }
>> +
>> + vaddr = (unsigned long)ioremap(phys_addr, resource_size(res));
>> + set_io_port_base(vaddr);
>> + return 0;
>> +}
Hi all,
Another way could be keeping a linked list about PIO->PHYS mapping instead of using the single io_port_base variable.
Thanks.
>
> --
> Xi Ruoyao <[email protected]>
> School of Aerospace Science and Technology, Xidian University
--
- Jiaxun
On Thu, Dec 16, 2021 at 2:07 PM Jiaxun Yang <[email protected]> wrote:
> 在2021年12月16日十二月 上午11:44，Xi Ruoyao写道：
> Another way could be keeping a linked list about PIO->PHYS mapping instead of using the single io_port_base variable.
I think that would add a lot of complexity that isn't needed here. Not
sure if all MIPS CPUs
can do it, but the approach used on Arm is what fits in best with the
PCI drivers, these
reserve a virtual address range for the ports, and ioremap the
physical addresses into
the PIO range according to the mapping.
For the loongson case specifically, that's not even needed though, as
the two buses
have physically contiguous I/O port ranges, the code just needs to
detect this special
case.
Arnd
在 2021/12/16 13:50, Arnd Bergmann 写道:
> On Thu, Dec 16, 2021 at 2:07 PM Jiaxun Yang <[email protected]> wrote:
>> 在2021年12月16日十二月 上午11:44，Xi Ruoyao写道：
>> Another way could be keeping a linked list about PIO->PHYS mapping instead of using the single io_port_base variable.
> I think that would add a lot of complexity that isn't needed here. Not
> sure if all MIPS CPUs
> can do it, but the approach used on Arm is what fits in best with the
> PCI drivers, these
> reserve a virtual address range for the ports, and ioremap the
> physical addresses into
> the PIO range according to the mapping.
Yes, the Arm way was my previous approach when introducing PCI IO map
for Loongson.
It got refactored by this patch as TLB entries are expensive on MIPS,
also the size of IO range doesn't always fits a page.
>
> For the loongson case specifically, that's not even needed though, as
> the two buses
> have physically contiguous I/O port ranges, the code just needs to
> detect this special
> case.
We have MIPS Boston board (from imgtec) which has discontinuous IO
range.
Thanks.
>
> Arnd
- Jiaxun
On Thu, Dec 16, 2021 at 3:14 PM Jiaxun Yang <[email protected]> wrote:
> 在 2021/12/16 13:50, Arnd Bergmann 写道:
> > On Thu, Dec 16, 2021 at 2:07 PM Jiaxun Yang <[email protected]> wrote:
> >> 在2021年12月16日十二月 上午11:44，Xi Ruoyao写道：
> >> Another way could be keeping a linked list about PIO->PHYS mapping instead of using the single io_port_base variable.
> > I think that would add a lot of complexity that isn't needed here. Not
> > sure if all MIPS CPUs
> > can do it, but the approach used on Arm is what fits in best with the
> > PCI drivers, these
> > reserve a virtual address range for the ports, and ioremap the
> > physical addresses into
> > the PIO range according to the mapping.
>
> Yes, the Arm way was my previous approach when introducing PCI IO map
> for Loongson.
>
> It got refactored by this patch as TLB entries are expensive on MIPS,
> also the size of IO range doesn't always fits a page.
Are PIO accesses common enough that the TLB entry makes a difference?
I would imagine that on most systems with a PCI bus, there is not even
a single device that exposes an I/O resource, and even on those devices that
do, the kernel drivers tend to pick MMIO whenever both are available.
Arnd
在 2021/12/16 14:18, Arnd Bergmann 写道:
> On Thu, Dec 16, 2021 at 3:14 PM Jiaxun Yang <[email protected]> wrote:
>> 在 2021/12/16 13:50, Arnd Bergmann 写道:
>>> On Thu, Dec 16, 2021 at 2:07 PM Jiaxun Yang <[email protected]> wrote:
>>>> 在2021年12月16日十二月 上午11:44，Xi Ruoyao写道：
>>>> Another way could be keeping a linked list about PIO->PHYS mapping instead of using the single io_port_base variable.
>>> I think that would add a lot of complexity that isn't needed here. Not
>>> sure if all MIPS CPUs
>>> can do it, but the approach used on Arm is what fits in best with the
>>> PCI drivers, these
>>> reserve a virtual address range for the ports, and ioremap the
>>> physical addresses into
>>> the PIO range according to the mapping.
>> Yes, the Arm way was my previous approach when introducing PCI IO map
>> for Loongson.
>>
>> It got refactored by this patch as TLB entries are expensive on MIPS,
>> also the size of IO range doesn't always fits a page.
> Are PIO accesses common enough that the TLB entry makes a difference?
> I would imagine that on most systems with a PCI bus, there is not even
> a single device that exposes an I/O resource, and even on those devices that
> do, the kernel drivers tend to pick MMIO whenever both are available.
Actually that was claimed by the author of this patch :-)
I can understand the point. As he is working on a ramips system utlizes
1004Kec,
which has only 32 TLB entries, saving a entry can give considerable
improvement.
For Loongson as we have legacy i8042/i8259 which can only be accessed via
PIO, the access is very common.
For other systems I guess it's not that common.
Thanks.
>
> Arnd
- Jiaxun
On Thu, Dec 16, 2021 at 3:27 PM Jiaxun Yang <[email protected]> wrote:
> 在 2021/12/16 14:18, Arnd Bergmann 写道:
> >> It got refactored by this patch as TLB entries are expensive on MIPS,
> >> also the size of IO range doesn't always fits a page.
> > Are PIO accesses common enough that the TLB entry makes a difference?
> > I would imagine that on most systems with a PCI bus, there is not even
> > a single device that exposes an I/O resource, and even on those devices that
> > do, the kernel drivers tend to pick MMIO whenever both are available.
>
> Actually that was claimed by the author of this patch :-)
> I can understand the point. As he is working on a ramips system utlizes
> 1004Kec,
> which has only 32 TLB entries, saving a entry can give considerable
> improvement.
Ok
> For Loongson as we have legacy i8042/i8259 which can only be accessed via
> PIO, the access is very common.
Ah, right. It makes a lot of sense that anything based on ISA PC peripherals
would need it, regardless of the PCI support.
Arnd
在 2021/12/16 14:32, Arnd Bergmann 写道:
> On Thu, Dec 16, 2021 at 3:27 PM Jiaxun Yang <[email protected]> wrote:
>> 在 2021/12/16 14:18, Arnd Bergmann 写道:
<...>
>> Ah, right. It makes a lot of sense that anything based on ISA PC peripherals
>> would need it, regardless of the PCI support.
I'll draft a RFC patch with linked list approach later on.
For now Tiezhu's 09d97da660ff ("MIPS: Only define pci_remap_iospace()
for Ralink")
seems working.
Thanks.
>>
>> Arnd
- Jiaxun