Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1427600 > unrolled thread

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

Started byGeert Uytterhoeven <geert@linux-m68k.org>
First post2016-06-21 12:50 +0200
Last post2016-06-21 18:50 +0200
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v1 19/25] PCI: rcar Gen2: Request host bridge window resources Geert Uytterhoeven <geert@linux-m68k.org> - 2016-06-21 12:50 +0200
    Re: [PATCH v1 19/25] PCI: rcar Gen2: Request host bridge window  resources Bjorn Helgaas <helgaas@kernel.org> - 2016-06-21 16:40 +0200
      Re: [PATCH v1 19/25] PCI: rcar Gen2: Request host bridge window  resources Valentine Barshak <valentine.barshak@cogentembedded.com> - 2016-06-21 17:50 +0200
        Re: [PATCH v1 19/25] PCI: rcar Gen2: Request host bridge window  resources Bjorn Helgaas <helgaas@kernel.org> - 2016-06-21 18:50 +0200

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

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2016-06-21 12:50 +0200
SubjectRe: [PATCH v1 19/25] PCI: rcar Gen2: Request host bridge window resources
Message-ID<rMrpM-4GP-15@gated-at.bofh.it>
Hi Bjorn,

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]
-pci_bus 0000:00: No busn resource found for root bus, will use [bus 00-ff]
-pci 0000:00:00.0: [1033:0000] type 00 class 0x060000
-pci 0000:00:00.0: reg 0x10: [mem 0xee090800-0xee090bff]
-pci 0000:00:00.0: reg 0x14: [mem 0x40000000-0x7fffffff pref]
-pci 0000:00:01.0: [1033:0035] type 00 class 0x0c0310
-pci 0000:00:01.0: reg 0x10: [mem 0x00000000-0x00000fff]
-pci 0000:00:01.0: supports D1 D2
-pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot
-pci 0000:00:02.0: [1033:00e0] type 00 class 0x0c0320
-pci 0000:00:02.0: reg 0x10: [mem 0x00000000-0x000000ff]
-pci 0000:00:02.0: supports D1 D2
-pci 0000:00:02.0: PME# supported from D0 D1 D2 D3hot
-PCI: bus0: Fast back to back transfers disabled
-pci_bus 0000:00: busn_res: [bus 00-ff] end is updated to 00
-pci 0000:00:01.0: BAR 0: assigned [mem 0xee080000-0xee080fff]
-pci 0000:00:02.0: BAR 0: assigned [mem 0xee081000-0xee0810ff]
+pci-rcar-gen2 ee090000.pci: resource collision: [io
0xee080000-0xee0810ff] conflicts with PCI IO [io  0x0000-0xfffff]

and:

 pci-rcar-gen2 ee0d0000.pci: PCI: bus0 revision 11
-pci-rcar-gen2 ee0d0000.pci: PCI host bridge to bus 0001:01
-pci_bus 0001:01: root bus resource [io  0xee0c0000-0xee0c10ff]
-pci_bus 0001:01: root bus resource [mem 0xee0c0000-0xee0c10ff]
-pci_bus 0001:01: No busn resource found for root bus, will use [bus 01-ff]
-pci 0001:01:00.0: [1033:0000] type 00 class 0x060000
-pci 0001:01:00.0: reg 0x10: [mem 0xee0d0800-0xee0d0bff]
-pci 0001:01:00.0: reg 0x14: [mem 0x40000000-0x7fffffff pref]
-pci 0001:01:01.0: [1033:0035] type 00 class 0x0c0310
-pci 0001:01:01.0: reg 0x10: [mem 0x00000000-0x00000fff]
-pci 0001:01:01.0: supports D1 D2
-pci 0001:01:01.0: PME# supported from D0 D1 D2 D3hot
-pci 0001:01:02.0: [1033:00e0] type 00 class 0x0c0320
-pci 0001:01:02.0: reg 0x10: [mem 0x00000000-0x000000ff]
-pci 0001:01:02.0: supports D1 D2
-pci 0001:01:02.0: PME# supported from D0 D1 D2 D3hot
-PCI: bus1: Fast back to back transfers disabled
-pci_bus 0001:01: busn_res: [bus 01-ff] end is updated to 01
-pci 0001:01:01.0: BAR 0: assigned [mem 0xee0c0000-0xee0c0fff]
-pci 0001:01:02.0: BAR 0: assigned [mem 0xee0c1000-0xee0c10ff]
+pci-rcar-gen2 ee0d0000.pci: resource collision: [io
0xee0c0000-0xee0c10ff] conflicts with PCI IO [io  0x0000-0xfffff]

# cat /proc/iomem
30000000-37ffffff : /pcie@fe000000
38000000-3fffffff : /pcie@fe000000
40000000-6fffffff : System RAM
  40008000-40a1c15b : Kernel code
  40e00000-40e9ea97 : Kernel data
e6060000-e606024f : /pfc@e6060000
e60b0000-e60b0424 : /i2c@e60b0000
e6150000-e6150fff : /clock-controller@e6150000
e61f0000-e61f0013 : /thermal@e61f0000
e61f0100-e61f0137 : /thermal@e61f0000
e6530000-e653003f : /i2c@e6530000
e6590000-e65900ff : /usb@e6590000
e6590100-e65901ff : /usb-phy@e6590100
e65a0000-e65a00ff : /dma-controller@e65a0000
e65b0000-e65b00ff : /dma-controller@e65b0000
e6700000-e671ffff : /dma-controller@e6700000
e6720000-e673ffff : /dma-controller@e6720000
e6b10000-e6b1002b : /spi@e6b10000
e6e20000-e6e20063 : /spi@e6e20000
e6e60000-e6e6003f : e6e60000.serial
e6e68000-e6e6803f : e6e68000.serial
e6ef1000-e6ef1fff : /video@e6ef1000
ec500000-ec500fff : scu
ec540000-ec540fff : ssiu
ec541000-ec54127f : ssi
ec5a0000-ec5a00ff : adg
ec700000-ec70ffff : /dma-controller@ec700000
ec720000-ec72ffff : /dma-controller@ec720000
ec740000-ec7401ff : audmapp
ee090000-ee090bff : /pci@ee090000
ee0d0000-ee0d0bff : /pci@ee0d0000
ee300000-ee301fff : /sata@ee300000
ee700000-ee7003ff : /ethernet@ee700000
fe000000-fe07ffff : /pcie@fe000000
fe200000-fe3fffff : /pcie@fe000000
fe928000-fe92ffff : /vsp1@fe928000
fe930000-fe937fff : /vsp1@fe930000
fe938000-fe93ffff : /vsp1@fe938000
fe980000-fe9902ff : /jpeg-codec@fe980000
feb00000-feb3ffff : du
feb90000-feb9001b : lvds.0
# cat /proc/ioports
00000000-000fffff : /pcie@fe000000
#

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [next] | [standalone]

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

FromBjorn Helgaas <helgaas@kernel.org>
Date2016-06-21 16:40 +0200
SubjectRe: [PATCH v1 19/25] PCI: rcar Gen2: Request host bridge window resources
Message-ID<rMv0m-71Y-29@gated-at.bofh.it>
In reply to#1427600
[+cc Valentine]

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?

Can you try the patch below (apply it before the 1bd019707b7c patch
that broke things)?


commit b64dc28f5f2b3afe47ee4a42fb79db84ec4227f8
Author: Bjorn Helgaas <bhelgaas@google.com>
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@google.com>

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);

[toc] | [prev] | [next] | [standalone]

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

FromValentine Barshak <valentine.barshak@cogentembedded.com>
Date2016-06-21 17:50 +0200
SubjectRe: [PATCH v1 19/25] PCI: rcar Gen2: Request host bridge window resources
Message-ID<rMw66-7Hw-31@gated-at.bofh.it>
In reply to#1427829
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]

> 
> Can you try the patch below (apply it before the 1bd019707b7c patch
> that broke things)?
> 
> 
> commit b64dc28f5f2b3afe47ee4a42fb79db84ec4227f8
> Author: Bjorn Helgaas <bhelgaas@google.com>
> 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@google.com>
> 
> 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.

[toc] | [prev] | [next] | [standalone]

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

FromBjorn Helgaas <helgaas@kernel.org>
Date2016-06-21 18:50 +0200
SubjectRe: [PATCH v1 19/25] PCI: rcar Gen2: Request host bridge window resources
Message-ID<rMx29-8iu-27@gated-at.bofh.it>
In reply to#1427897
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>

diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
index 0070e85..2d88af5 100644
--- a/arch/arm/include/asm/mach/pci.h
+++ b/arch/arm/include/asm/mach/pci.h
@@ -22,6 +22,7 @@ struct hw_pci {
 	struct msi_controller *msi_ctrl;
 	struct pci_ops	*ops;
 	int		nr_controllers;
+	unsigned int	io_optional:1;
 	void		**private_data;
 	int		(*setup)(int nr, struct pci_sys_data *);
 	struct pci_bus *(*scan)(int nr, struct pci_sys_data *);
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 05e61a2..65f12ef 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -410,7 +410,8 @@ static int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 	return irq;
 }
 
-static int pcibios_init_resources(int busnr, struct pci_sys_data *sys)
+static int pcibios_init_resource(int busnr, struct pci_sys_data *sys,
+				 int io_optional)
 {
 	int ret;
 	struct resource_entry *window;
@@ -420,6 +421,14 @@ static int pcibios_init_resources(int busnr, struct pci_sys_data *sys)
 			 &iomem_resource, sys->mem_offset);
 	}
 
+	/*
+	 * If a platform says I/O port support is optional, we don't add
+	 * the default I/O space.  The platform is responsible for adding
+	 * any I/O space it needs.
+	 */
+	if (io_optional)
+		return 0;
+
 	resource_list_for_each_entry(window, &sys->resources)
 		if (resource_type(window->res) == IORESOURCE_IO)
 			return 0;
@@ -466,7 +475,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		if (ret > 0) {
 			struct pci_host_bridge *host_bridge;
 
-			ret = pcibios_init_resources(nr, sys);
+			ret = pcibios_init_resource(nr, sys, hw->io_optional);
 			if (ret)  {
 				kfree(sys);
 				break;

commit eeb4d6cdf8960c484c5a6eb9b310145b75a59ec1
Author: Bjorn Helgaas <bhelgaas@google.com>
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 and set struct hw_pci.io_optional so
    the ARM PCI code doesn't add a default one for us.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/host/pci-rcar-gen2.c b/drivers/pci/host/pci-rcar-gen2.c
index 9980a4b..ddf7765 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);
@@ -421,6 +411,7 @@ static int rcar_pci_probe(struct platform_device *pdev)
 	hw_private[0] = priv;
 	memset(&hw, 0, sizeof(hw));
 	hw.nr_controllers = ARRAY_SIZE(hw_private);
+	hw.io_optional = 1;
 	hw.private_data = hw_private;
 	hw.map_irq = rcar_pci_map_irq;
 	hw.ops = &rcar_pci_ops;

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web