This patch adds required definitions to allow for PCI buses on OpenRISC.
This is being in the QEMU virt platform.
OpenRISC does not have IO ports so this defines PCI IO to be allowed in
any range. Keeping PIO_RESERVED defined as 0 allows OpenRISC to use
MMIO for all IO.
Also, since commit 66bcd06099bb ("parport_pc: Also enable driver for PCI
systems") all platforms that support PCI also need to support parallel
port. We add a generic header to support parallel port drivers.
Signed-off-by: Stafford Horne <[email protected]>
---
Since v1:
- Revert definition of IO_SPACE_LIMIT
arch/openrisc/Kconfig | 7 ++++---
arch/openrisc/include/asm/Kbuild | 1 +
arch/openrisc/include/asm/io.h | 2 +-
arch/openrisc/include/asm/pci.h | 36 ++++++++++++++++++++++++++++++++
4 files changed, 42 insertions(+), 4 deletions(-)
create mode 100644 arch/openrisc/include/asm/pci.h
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index e814df4c483c..327241988819 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -21,7 +21,9 @@ config OPENRISC
select GENERIC_IRQ_PROBE
select GENERIC_IRQ_SHOW
select GENERIC_IOMAP
+ select GENERIC_PCI_IOMAP
select GENERIC_CPU_DEVICES
+ select HAVE_PCI
select HAVE_UID16
select GENERIC_ATOMIC64
select GENERIC_CLOCKEVENTS_BROADCAST
@@ -32,6 +34,8 @@ config OPENRISC
select CPU_NO_EFFICIENT_FFS if !OPENRISC_HAVE_INST_FF1
select ARCH_USE_QUEUED_RWLOCKS
select OMPIC if SMP
+ select PCI_DOMAINS_GENERIC if PCI
+ select PCI_MSI if PCI
select ARCH_WANT_FRAME_POINTERS
select GENERIC_IRQ_MULTI_HANDLER
select MMU_GATHER_NO_RANGE if MMU
@@ -46,9 +50,6 @@ config MMU
config GENERIC_HWEIGHT
def_bool y
-config NO_IOPORT_MAP
- def_bool y
-
# For now, use generic checksum functions
#These can be reimplemented in assembly later if so inclined
config GENERIC_CSUM
diff --git a/arch/openrisc/include/asm/Kbuild b/arch/openrisc/include/asm/Kbuild
index 3386b9c1c073..c8c99b554ca4 100644
--- a/arch/openrisc/include/asm/Kbuild
+++ b/arch/openrisc/include/asm/Kbuild
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
generic-y += extable.h
generic-y += kvm_para.h
+generic-y += parport.h
generic-y += spinlock_types.h
generic-y += spinlock.h
generic-y += qrwlock_types.h
diff --git a/arch/openrisc/include/asm/io.h b/arch/openrisc/include/asm/io.h
index c298061c70a7..625ac6ad1205 100644
--- a/arch/openrisc/include/asm/io.h
+++ b/arch/openrisc/include/asm/io.h
@@ -17,7 +17,7 @@
#include <linux/types.h>
/*
- * PCI: can we really do 0 here if we have no port IO?
+ * PCI: We do not use IO ports in OpenRISC
*/
#define IO_SPACE_LIMIT 0
diff --git a/arch/openrisc/include/asm/pci.h b/arch/openrisc/include/asm/pci.h
new file mode 100644
index 000000000000..e0865d2f3f42
--- /dev/null
+++ b/arch/openrisc/include/asm/pci.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __ASM_OPENRISC_PCI_H
+#define __ASM_OPENRISC_PCI_H
+
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/dma-mapping.h>
+
+#include <asm/io.h>
+
+#define PCIBIOS_MIN_IO 0
+#define PCIBIOS_MIN_MEM 0
+
+/* OpenRISC bootloaders do not initialize PCI bus */
+#define pcibios_assign_all_busses() 1
+
+#define ARCH_GENERIC_PCI_MMAP_RESOURCE 1
+
+extern int isa_dma_bridge_buggy;
+
+#ifdef CONFIG_PCI
+static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
+{
+ /* no legacy IRQs on or1k */
+ return -ENODEV;
+}
+
+static inline int pci_proc_domain(struct pci_bus *bus)
+{
+ /* always show the domain in /proc */
+ return 1;
+}
+#endif /* CONFIG_PCI */
+
+#endif /* __ASM_OPENRISC_PCI_H */
--
2.36.1
On Thu, Jul 14, 2022 at 6:27 AM Stafford Horne <[email protected]> wrote:
>
> This patch adds required definitions to allow for PCI buses on OpenRISC.
> This is being in the QEMU virt platform.
>
> OpenRISC does not have IO ports so this defines PCI IO to be allowed in
> any range. Keeping PIO_RESERVED defined as 0 allows OpenRISC to use
> MMIO for all IO.
Ok, this looks better.
> Also, since commit 66bcd06099bb ("parport_pc: Also enable driver for PCI
> systems") all platforms that support PCI also need to support parallel
> port. We add a generic header to support parallel port drivers.
The parport_pc driver is actually one of the things that doesn't work without
I/O ports, so at least the description here is misleading. We should really
have Kconfig logic to enforce this, but that is a separate topic.
> diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> index e814df4c483c..327241988819 100644
> --- a/arch/openrisc/Kconfig
> +++ b/arch/openrisc/Kconfig
> @@ -21,7 +21,9 @@ config OPENRISC
> select GENERIC_IRQ_PROBE
> select GENERIC_IRQ_SHOW
> select GENERIC_IOMAP
> + select GENERIC_PCI_IOMAP
> select GENERIC_CPU_DEVICES
> @@ -46,9 +50,6 @@ config MMU
> config GENERIC_HWEIGHT
> def_bool y
>
> -config NO_IOPORT_MAP
> - def_bool y
> -
GENERIC_IOMAP makes no sense without PIO, and I think you also
need to keep the NO_IOPORT_MAP. I think you still want
GENERIC_PCI_IOMAP, which in the absence of the other two
should turn just return an __iomem pointer for memory resource
and NULL for i/o resources.
> +static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
> +{
> + /* no legacy IRQs on or1k */
> + return -ENODEV;
> +}
The comment seems misleading, as "legacy IRQs" normally refers to
non-MSI interrupts, which you do support. It's only the legacy IDE
interrupts that are not supported.
I see that the asm-generic/pci.h file is now completely useless,
as it only has a single function left in it, and this one is wrong
on most architectures -- it only works when you have PC-style
interrupt numbers. Out of the five architectures that include
asm-generic/pci.h (m68k, s390, sparc, x86, xtensa), I would
expect only x86 to use this version, and maybe a few sparc
machines.
Can I ask you to move out the existing asm-generic/pci.h
code into those architectures, and add a new file in its place
that you can use as-is on openrisc? Ideally we should
be able to also remove most of the contents of asm/pci.h
on arm64 and riscv. If you have conflicting settings, the normal
way to handle them in asm-generic headers is like
#ifndef PCIBIOS_MIN_IO
#define PCIBIOS_MIN_IO 0
#endif
#ifndef pcibios_assign_all_busses
#define pcibios_assign_all_busses() 1
#endif
Arnd
On Thu, Jul 14, 2022 at 09:56:44AM +0200, Arnd Bergmann wrote:
> On Thu, Jul 14, 2022 at 6:27 AM Stafford Horne <[email protected]> wrote:
> >
> > This patch adds required definitions to allow for PCI buses on OpenRISC.
> > This is being in the QEMU virt platform.
> >
> > OpenRISC does not have IO ports so this defines PCI IO to be allowed in
> > any range. Keeping PIO_RESERVED defined as 0 allows OpenRISC to use
> > MMIO for all IO.
>
> Ok, this looks better.
>
> > Also, since commit 66bcd06099bb ("parport_pc: Also enable driver for PCI
> > systems") all platforms that support PCI also need to support parallel
> > port. We add a generic header to support parallel port drivers.
>
> The parport_pc driver is actually one of the things that doesn't work without
> I/O ports, so at least the description here is misleading. We should really
> have Kconfig logic to enforce this, but that is a separate topic.
OK, let me try to fix up this comment in v3.
> > diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> > index e814df4c483c..327241988819 100644
> > --- a/arch/openrisc/Kconfig
> > +++ b/arch/openrisc/Kconfig
> > @@ -21,7 +21,9 @@ config OPENRISC
> > select GENERIC_IRQ_PROBE
> > select GENERIC_IRQ_SHOW
> > select GENERIC_IOMAP
> > + select GENERIC_PCI_IOMAP
> > select GENERIC_CPU_DEVICES
>
> > @@ -46,9 +50,6 @@ config MMU
> > config GENERIC_HWEIGHT
> > def_bool y
> >
> > -config NO_IOPORT_MAP
> > - def_bool y
> > -
I tried the below patch on top of this but I get failures, as the __pci_ioport_map
uses ioport_map.
lib/pci_iomap.c: In function 'pci_iomap_range':
CC drivers/i2c/i2c-core-base.o
./include/asm-generic/pci_iomap.h:29:41: error: implicit declaration of function 'ioport_map'; did you mean 'ioremap'? [-Werror=implicit-function-declaration]
29 | #define __pci_ioport_map(dev, port, nr) ioport_map((port), (nr))
| ^~~~~~~~~~
lib/pci_iomap.c:44:24: note: in expansion of macro '__pci_ioport_map'
44 | return __pci_ioport_map(dev, start, len);
| ^~~~~~~~~~~~~~~~
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 327241988819..c7f282f60f64 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -20,7 +20,6 @@ config OPENRISC
select GENERIC_IRQ_CHIP
select GENERIC_IRQ_PROBE
select GENERIC_IRQ_SHOW
- select GENERIC_IOMAP
select GENERIC_PCI_IOMAP
select GENERIC_CPU_DEVICES
select HAVE_PCI
@@ -50,6 +49,9 @@ config MMU
config GENERIC_HWEIGHT
def_bool y
+config NO_IOPORT_MAP
+ def_bool y
+
# For now, use generic checksum functions
#These can be reimplemented in assembly later if so inclined
config GENERIC_CSUM
> GENERIC_IOMAP makes no sense without PIO, and I think you also
> need to keep the NO_IOPORT_MAP. I think you still want
> GENERIC_PCI_IOMAP, which in the absence of the other two
> should turn just return an __iomem pointer for memory resource
> and NULL for i/o resources.
OK.
If we keep NO_IOPORT_MAP, it causes HAS_IOPORT_MAP to be false and it removes
the definition of ioport_map which still seems to be needed at link time. Maybe
thats an issue though.
> > +static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
> > +{
> > + /* no legacy IRQs on or1k */
> > + return -ENODEV;
> > +}
>
> The comment seems misleading, as "legacy IRQs" normally refers to
> non-MSI interrupts, which you do support. It's only the legacy IDE
> interrupts that are not supported.
OK, I might have copied this from another architecture. I will check if those
comments should be updated too.
> I see that the asm-generic/pci.h file is now completely useless,
> as it only has a single function left in it, and this one is wrong
> on most architectures -- it only works when you have PC-style
> interrupt numbers. Out of the five architectures that include
> asm-generic/pci.h (m68k, s390, sparc, x86, xtensa), I would
> expect only x86 to use this version, and maybe a few sparc
> machines.
>
> Can I ask you to move out the existing asm-generic/pci.h
> code into those architectures, and add a new file in its place
> that you can use as-is on openrisc? Ideally we should
> be able to also remove most of the contents of asm/pci.h
> on arm64 and riscv. If you have conflicting settings, the normal
> way to handle them in asm-generic headers is like
Yeah, that sounds like a good plan,
> #ifndef PCIBIOS_MIN_IO
> #define PCIBIOS_MIN_IO 0
> #endif
>
> #ifndef pcibios_assign_all_busses
> #define pcibios_assign_all_busses() 1
> #endif
OK.
-Stafford
On Thu, Jul 14, 2022 at 2:41 PM Stafford Horne <[email protected]> wrote:
> On Thu, Jul 14, 2022 at 09:56:44AM +0200, Arnd Bergmann wrote:
> > > diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> > > index e814df4c483c..327241988819 100644
> > > --- a/arch/openrisc/Kconfig
> > > +++ b/arch/openrisc/Kconfig
> > > @@ -21,7 +21,9 @@ config OPENRISC
> > > select GENERIC_IRQ_PROBE
> > > select GENERIC_IRQ_SHOW
> > > select GENERIC_IOMAP
> > > + select GENERIC_PCI_IOMAP
> > > select GENERIC_CPU_DEVICES
> >
> > > @@ -46,9 +50,6 @@ config MMU
> > > config GENERIC_HWEIGHT
> > > def_bool y
> > >
> > > -config NO_IOPORT_MAP
> > > - def_bool y
> > > -
>
> I tried the below patch on top of this but I get failures, as the __pci_ioport_map
> uses ioport_map.
>
> lib/pci_iomap.c: In function 'pci_iomap_range':
> CC drivers/i2c/i2c-core-base.o
> ./include/asm-generic/pci_iomap.h:29:41: error: implicit declaration of function 'ioport_map'; did you mean 'ioremap'? [-Werror=implicit-function-declaration]
> 29 | #define __pci_ioport_map(dev, port, nr) ioport_map((port), (nr))
> | ^~~~~~~~~~
> lib/pci_iomap.c:44:24: note: in expansion of macro '__pci_ioport_map'
> 44 | return __pci_ioport_map(dev, start, len);
> | ^~~~~~~~~~~~~~~~
>
Ah, I see. So setting NO_IOPORT_MAP without GENERIC_PCI_IOMAP
probably just works, but then you'd also build all the driver that use
ioport_map() when they cannot work.
Maybe add another
#define __pci_ioport_map(dev, port, nr) NULL
case to include/asm-generic/pci_iomap.h, or add an #ifdef to lib
pci_iomap_range() to not call it in this case.
> > GENERIC_IOMAP makes no sense without PIO, and I think you also
> > need to keep the NO_IOPORT_MAP. I think you still want
> > GENERIC_PCI_IOMAP, which in the absence of the other two
> > should turn just return an __iomem pointer for memory resource
> > and NULL for i/o resources.
>
> OK.
>
> If we keep NO_IOPORT_MAP, it causes HAS_IOPORT_MAP to be false and it removes
> the definition of ioport_map which still seems to be needed at link time. Maybe
> thats an issue though.
This is the intention of CONFIG_NO_IOPORT_MAP, it's meant to be set
on architectures that have no way of defining ioport_map() in a sensible
way.
Arnd