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

[Valentine Barshak]

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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > ---
> > >  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]

> 
> Can you try the patch below (apply it before the 1bd019707b7c patch
> that broke things)?
> 
> 
> commit b64dc28f5f2b3afe47ee4a42fb79db84ec4227f8
> Author: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Date:   Tue Jun 21 09:19:34 2016 -0500
> 
>     PCI: rcar: Drop gen2 dummy I/O port region
>     
>     Previously we added a dummy I/O port region even though the R-Car
>     controller doesn't support PCI port I/O.  This resulted in bogus root bus
>     resources like this:
>     
>       pci_bus 0000:00: root bus resource [io  0xee080000-0xee0810ff]
>       pci_bus 0000:00: root bus resource [mem 0xee080000-0xee0810ff]
>     
>     Drop the unused dummy I/O port region.
>     
>     Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> 
> diff --git a/drivers/pci/host/pci-rcar-gen2.c b/drivers/pci/host/pci-rcar-gen2.c
> index 9980a4b..53ae619 100644
> --- a/drivers/pci/host/pci-rcar-gen2.c
> +++ b/drivers/pci/host/pci-rcar-gen2.c
> @@ -97,7 +97,6 @@
>  struct rcar_pci_priv {
>  	struct device *dev;
>  	void __iomem *reg;
> -	struct resource io_res;
>  	struct resource mem_res;
>  	struct resource *cfg_res;
>  	unsigned busnr;
> @@ -273,7 +272,6 @@ static int rcar_pci_setup(int nr, struct pci_sys_data *sys)
>  		rcar_pci_setup_errirq(priv);
>  
>  	/* Add PCI resources */
> -	pci_add_resource(&sys->resources, &priv->io_res);
>  	pci_add_resource(&sys->resources, &priv->mem_res);
>  
>  	/* Setup bus number based on platform device id / of bus-range */
> @@ -371,14 +369,6 @@ static int rcar_pci_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	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;
> -
>  	priv->cfg_res = cfg_res;
>  
>  	priv->irq = platform_get_irq(pdev, 0);

-- 
Thanks,
Val.