Re: [PATCH v3] PCI: of: Avoid pci_remap_iospace() when PCI_IOBASE not defined

From: Arnd Bergmann <arnd@arndb.de>
To: Sergio Paracuellos <sergio.paracuellos@gmail.com>
Cc: linux-pci <linux-pci@vger.kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-staging@lists.linux.dev,
	gregkh <gregkh@linuxfoundation.org>,
	Liviu Dudau <Liviu.Dudau@arm.com>, Rob Herring <robh@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH v3] PCI: of: Avoid pci_remap_iospace() when PCI_IOBASE not defined
Date: Wed, 22 Sep 2021 17:47:20 +0200	[thread overview]
Message-ID: <CAK8P3a2WPOYS7ra_epyZ_bBBpPK8+AgEynK0pKOUZ6ajubcHew@mail.gmail.com> (raw)
In-Reply-To: <20210922042041.16326-1-sergio.paracuellos@gmail.com>

On Wed, Sep 22, 2021 at 6:23 AM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:

> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index d84381ce82b5..7d7aab1d1d64 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -564,12 +564,14 @@ static int pci_parse_request_of_pci_ranges(struct device *dev,
>
>                 switch (resource_type(res)) {
>                 case IORESOURCE_IO:
> +#ifdef PCI_IOBASE
>                         err = devm_pci_remap_iospace(dev, res, iobase);
>                         if (err) {
>                                 dev_warn(dev, "error %d: failed to map resource %pR\n",
>                                          err, res);
>                                 resource_list_destroy_entry(win);
>                         }
> +#endif
>                         break;

I wonder if we should have a different symbol controlling this than PCI_IOBASE,
because we are somewhat overloading the semantics here. There are a couple
of ways that I/O space can be handled

a) inb()/outb() are custom instructions, as on x86, PCI_IOBASE is not defined
b) there is no I/O space, as on s390, PCI_IOBASE is not defined
c) PCI_IOBASE points to a virtual address used for dynamic mapping of I/O
    space, as on ARM
d) PCI_IOBASE is NULL, and the port number corresponds to the virtual
   address (some older architectures)

I'm not completely sure where your platform fits in here, it sounds like you
address them using a machine specific physical address as the base in
inb() plus the port number as an offset, is that correct?

       Arnd