archive mirror
 help / color / mirror / Atom feed
From: Arnd Bergmann <>
To: Stafford Horne <>
Cc: LKML <>,
	Openrisc <>,
	Jonas Bonn <>,
	Stefan Kristiansson <>,
	Palmer Dabbelt <>,
	Peter Zijlstra <>,
	Arnd Bergmann <>
Subject: Re: [PATCH v2 1/2] openrisc: Add pci bus support
Date: Thu, 14 Jul 2022 09:56:44 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Thu, Jul 14, 2022 at 6:27 AM Stafford Horne <> 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
>         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

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

#define PCIBIOS_MIN_IO 0

#ifndef pcibios_assign_all_busses
#define pcibios_assign_all_busses() 1


  reply	other threads:[~2022-07-14  7:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-14  4:27 [PATCH v2 0/2] OpenRISC support for virt platform with PCI Stafford Horne
2022-07-14  4:27 ` [PATCH v2 1/2] openrisc: Add pci bus support Stafford Horne
2022-07-14  7:56   ` Arnd Bergmann [this message]
2022-07-14 12:41     ` Stafford Horne
2022-07-14 13:14       ` Arnd Bergmann
2022-07-14  4:27 ` [PATCH v2 2/2] openrisc: Add virt defconfig Stafford Horne

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='' \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).