[v1,19/25] PCI: rcar Gen2: Request host bridge window resources

On Tue, Jun 21, 2016 at 06:41:00PM +0300, Valentine Barshak wrote:
> On Tue, Jun 21, 2016 at 09:26:23AM -0500, Bjorn Helgaas wrote:
> > [+cc Valentine]
> > 
> 
> Hi Bjorn,
> 
> > Hi Geert,
> > 
> > Thanks a lot for testing this, and sorry for the breakage.
> > 
> > On Tue, Jun 21, 2016 at 12:41:31PM +0200, Geert Uytterhoeven wrote:
> > > On Tue, Jun 7, 2016 at 1:07 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > > > Request host bridge window resources so they appear in ioport_resource and
> > > > iomem_resource and are reflected in /proc/ioports and /proc/iomem.
> > > >
> > > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > > > ---
> > > >  drivers/pci/host/pci-rcar-gen2.c |    4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/host/pci-rcar-gen2.c b/drivers/pci/host/pci-rcar-gen2.c
> > > > index 9980a4b..617a6b2 100644
> > > > --- a/drivers/pci/host/pci-rcar-gen2.c
> > > > +++ b/drivers/pci/host/pci-rcar-gen2.c
> > > > @@ -194,6 +194,7 @@ static int rcar_pci_setup(int nr, struct pci_sys_data *sys)
> > > >         struct rcar_pci_priv *priv = sys->private_data;
> > > >         void __iomem *reg = priv->reg;
> > > >         u32 val;
> > > > +       int ret;
> > > >
> > > >         pm_runtime_enable(priv->dev);
> > > >         pm_runtime_get_sync(priv->dev);
> > > > @@ -275,6 +276,9 @@ static int rcar_pci_setup(int nr, struct pci_sys_data *sys)
> > > >         /* Add PCI resources */
> > > >         pci_add_resource(&sys->resources, &priv->io_res);
> > > >         pci_add_resource(&sys->resources, &priv->mem_res);
> > > > +       ret = devm_request_pci_bus_resources(priv->dev, &sys->resources);
> > > > +       if (ret < 0)
> > > > +               return ret;
> > > >
> > > >         /* Setup bus number based on platform device id / of bus-range */
> > > >         sys->busnr = priv->busnr;
> > > 
> > > This patch (commit 1bd019707b7c9249d34c5d348f1ef75eb4d83e89 in pci/next)
> > > broke PCI on r8a7791/koelsch. Dmesg differences are:
> > > 
> > >  pci-rcar-gen2 ee090000.pci: PCI: bus0 revision 11
> > > -pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
> > > -pci_bus 0000:00: root bus resource [io  0xee080000-0xee0810ff]
> > > -pci_bus 0000:00: root bus resource [mem 0xee080000-0xee0810ff]
> > 
> > This is probably a result of this code in drivers/pci/host/pci-rcar-gen2.c:
> > 
> >         /*
> >          * The controller does not support/use port I/O,
> >          * so setup a dummy port I/O region here.
> >          */
> >         priv->io_res.start = priv->mem_res.start;
> >         priv->io_res.end = priv->mem_res.end;
> >         priv->io_res.flags = IORESOURCE_IO;
> > 
> > We try to avoid adding dummy regions like this, but maybe we missed
> > this one.  I haven't found any email discussion about it yet, so I
> > don't know what the reason for this one is.  Valentine, do you
> > remember?
> 
> I do not, but I think I've found something in my mailbox.
> Below is the quote from the original conversation with:
> (Re: [PATCH] pci: Add R-Car Gen2 internal PCI support)
> 
> [quote starts]
>  >>+    priv->mem_res = *mem_res;
>  >>+    /*
>  >>+     * The controller does not support/use port I/O,
>  >>+     * so setup a dummy port I/O region here.
>  >>+     */
>  >>+    priv->io_res.start = priv->mem_res.start;
>  >>+    priv->io_res.end = priv->mem_res.end;
>  >>+    priv->io_res.flags = IORESOURCE_IO;
>  >>
>  > I don't understand this.  There's no requirement (at least as far as the
>  > PCI core is concerned) to supply an I/O aperture at all, and I think it
>  > would be better if you didn't.
>  >
>  > Oh, I see ... maybe pcibios_init_resources() forces you to have an
>  > I/O resource to avoid having it give you a default one?  And I
>  > suppose that since you have several host bridges, these dummy I/O
>  > regions have to be distinct.  Ugh.  Well, I guess this is something
>  > you'd have to fix here or in the ARM code, it's up to you what to do.
> 
>  Exactly. This is to avoid assigning default I/O resources.
> [quote ends]

Oh, right, now I remember.  Thanks for digging that out.

I propose the patches below to remove the requirement for having an
I/O space.  Any chance one of you could test them?

commit 8387e687f72747b994c54e29e31861f0db07eb0c
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Tue Jun 21 10:54:29 2016 -0500

    ARM: Make PCI I/O space optional

    For callers of pci_common_init_dev(), we previously always required a PCI
    I/O port resource.  If the caller's ->setup() function had added an I/O
    resource, we used that; otherwise, we added a default 64K I/O port space
    for it.

    There are PCI host bridges that do not support I/O port space, and we
    should not add fictitious spaces for them.

    If a caller sets struct hw_pci.io_optional, assume it is responsible for
    adding any I/O port resource it desires, and do not add any default I/O
    port space.

    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

[v1,19/25] PCI: rcar Gen2: Request host bridge window resources

Commit Message

Comments

Patch