[PATCH 0/2] PCI: aardvark: misc improvements

[PATCH 0/2] PCI: aardvark: misc improvements

[Thomas Petazzoni]

Hello,

This small set of two patches make sure the pci-aardvark driver calls
pci_bus_size_bridges() and then converts it to use pci_host_probe().

Thanks,

Thomas

Thomas Petazzoni (1):
  PCI: aardvark: Convert to use pci_host_probe()

Zachary Zhang (1):
  PCI: aardvark: Size bridges before resources allocation

 drivers/pci/controller/pci-aardvark.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

-- 
2.14.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] PCI: aardvark: Size bridges before resources allocation

[Thomas Petazzoni]

From: Zachary Zhang <zhangzg@marvell.com>

The PCIE I/O and MEM resource allocation mechanism is that root bus
goes through the following steps:

1. Check PCI bridges' range and computes I/O and Mem base/limits.

2. Sort all subordinate devices I/O and MEM resource requirements and
   allocate the resources and writes/updates subordinate devices'
   requirements to PCI bridges I/O and Mem MEM/limits registers.

Currently, PCI Aardvark driver only handles the second step and lacks
the first step, so there is an I/O and MEM resource allocation failure
when using a PCI switch. This commit fixes that by sizing bridges
before doing the resource allocation.

Signed-off-by: Zachary Zhang <zhangzg@marvell.com>
[Thomas: edit commit log.]
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 drivers/pci/controller/pci-aardvark.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 486c41721c89..8e92231214e3 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -1065,6 +1065,7 @@ static int advk_pcie_probe(struct platform_device *pdev)
 
 	bus = bridge->bus;
 
+	pci_bus_size_bridges(bus);
 	pci_bus_assign_resources(bus);
 
 	list_for_each_entry(child, &bus->children, node)
-- 
2.14.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] PCI: aardvark: Size bridges before resources allocation

[Lorenzo Pieralisi]

On Fri, Jun 29, 2018 at 11:16:19AM +0200, Thomas Petazzoni wrote:
> From: Zachary Zhang <zhangzg@marvell.com>
> 
> The PCIE I/O and MEM resource allocation mechanism is that root bus
> goes through the following steps:
> 
> 1. Check PCI bridges' range and computes I/O and Mem base/limits.
> 
> 2. Sort all subordinate devices I/O and MEM resource requirements and
>    allocate the resources and writes/updates subordinate devices'
>    requirements to PCI bridges I/O and Mem MEM/limits registers.
> 
> Currently, PCI Aardvark driver only handles the second step and lacks
> the first step, so there is an I/O and MEM resource allocation failure
> when using a PCI switch. This commit fixes that by sizing bridges
> before doing the resource allocation.
> 
> Signed-off-by: Zachary Zhang <zhangzg@marvell.com>
> [Thomas: edit commit log.]
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  drivers/pci/controller/pci-aardvark.c | 1 +
>  1 file changed, 1 insertion(+)

Hi Thomas,

I am queueing these two patches but this one seems a serious bug,
I reckon we should send it to stable kernels (and would be grateful
if you provide me with a Fixes tag and a kernel log to add to the
commit log).

Thanks,
Lorenzo

> 
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index 486c41721c89..8e92231214e3 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -1065,6 +1065,7 @@ static int advk_pcie_probe(struct platform_device *pdev)
>  
>  	bus = bridge->bus;
>  
> +	pci_bus_size_bridges(bus);
>  	pci_bus_assign_resources(bus);
>  
>  	list_for_each_entry(child, &bus->children, node)
> -- 
> 2.14.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] PCI: aardvark: Size bridges before resources allocation

[Thomas Petazzoni]

Hello Lorenzo,

Thanks for your feedback!

On Thu, 5 Jul 2018 15:15:22 +0100, Lorenzo Pieralisi wrote:

> I am queueing these two patches but this one seems a serious bug,
> I reckon we should send it to stable kernels (and would be grateful
> if you provide me with a Fixes tag and a kernel log to add to the
> commit log).

It is a serious bug, but it is not a regression. I.e whatever is fixed
by this patch never worked. So if you take a conservative view on
pushing patches to stable and only want to fix regressions, this one
doesn't qualify. However, if you take a more open approach and say you
want to fix obvious bugs/issues even in older stable kernels, then
indeed this patch qualifies.

If you are within the kernel developers with this more open approach to
sending patches to stable, then you can add:

Fixes: 8c39d710363c1 ("PCI: aardvark: Add Aardvark PCI host controller driver")
Cc: <stable@vger.kernel.org>

when applying the patch. Unless of course you want me to resend the
patch series with this added. Just let met know what you prefer.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] PCI: aardvark: Size bridges before resources allocation

[Lorenzo Pieralisi]

On Thu, Jul 05, 2018 at 04:19:09PM +0200, Thomas Petazzoni wrote:
> Hello Lorenzo,
> 
> Thanks for your feedback!
> 
> On Thu, 5 Jul 2018 15:15:22 +0100, Lorenzo Pieralisi wrote:
> 
> > I am queueing these two patches but this one seems a serious bug,
> > I reckon we should send it to stable kernels (and would be grateful
> > if you provide me with a Fixes tag and a kernel log to add to the
> > commit log).
> 
> It is a serious bug, but it is not a regression. I.e whatever is fixed
> by this patch never worked. So if you take a conservative view on
> pushing patches to stable and only want to fix regressions, this one
> doesn't qualify. However, if you take a more open approach and say you
> want to fix obvious bugs/issues even in older stable kernels, then
> indeed this patch qualifies.
> 
> If you are within the kernel developers with this more open approach to
> sending patches to stable, then you can add:
> 
> Fixes: 8c39d710363c1 ("PCI: aardvark: Add Aardvark PCI host controller driver")
> Cc: <stable@vger.kernel.org>
> 
> when applying the patch. Unless of course you want me to resend the
> patch series with this added. Just let met know what you prefer.

It is up to you, I think it qualifies for stable kernels so I would
send it but it is your choice, please let me know.

Thanks,
Lorenzo

> Thanks!
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] PCI: aardvark: Size bridges before resources allocation

[Thomas Petazzoni]

Hello,

On Fri, 6 Jul 2018 12:01:30 +0100, Lorenzo Pieralisi wrote:

> > It is a serious bug, but it is not a regression. I.e whatever is fixed
> > by this patch never worked. So if you take a conservative view on
> > pushing patches to stable and only want to fix regressions, this one
> > doesn't qualify. However, if you take a more open approach and say you
> > want to fix obvious bugs/issues even in older stable kernels, then
> > indeed this patch qualifies.
> > 
> > If you are within the kernel developers with this more open approach to
> > sending patches to stable, then you can add:
> > 
> > Fixes: 8c39d710363c1 ("PCI: aardvark: Add Aardvark PCI host controller driver")
> > Cc: <stable@vger.kernel.org>
> > 
> > when applying the patch. Unless of course you want me to resend the
> > patch series with this added. Just let met know what you prefer.  
> 
> It is up to you, I think it qualifies for stable kernels so I would
> send it but it is your choice, please let me know.

It's OK for me to have it in stable kernels. The fix is pretty obvious,
so you can add the Fixes/Cc tags when applying. Unless of course you
want me to resend an updated version.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] PCI: aardvark: Size bridges before resources allocation

[Lorenzo Pieralisi]

On Fri, Jul 06, 2018 at 01:42:45PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri, 6 Jul 2018 12:01:30 +0100, Lorenzo Pieralisi wrote:
> 
> > > It is a serious bug, but it is not a regression. I.e whatever is fixed
> > > by this patch never worked. So if you take a conservative view on
> > > pushing patches to stable and only want to fix regressions, this one
> > > doesn't qualify. However, if you take a more open approach and say you
> > > want to fix obvious bugs/issues even in older stable kernels, then
> > > indeed this patch qualifies.
> > > 
> > > If you are within the kernel developers with this more open approach to
> > > sending patches to stable, then you can add:
> > > 
> > > Fixes: 8c39d710363c1 ("PCI: aardvark: Add Aardvark PCI host controller driver")
> > > Cc: <stable@vger.kernel.org>
> > > 
> > > when applying the patch. Unless of course you want me to resend the
> > > patch series with this added. Just let met know what you prefer.  
> > 
> > It is up to you, I think it qualifies for stable kernels so I would
> > send it but it is your choice, please let me know.
> 
> It's OK for me to have it in stable kernels. The fix is pretty obvious,
> so you can add the Fixes/Cc tags when applying. Unless of course you
> want me to resend an updated version.

Applied to pci/aardvark for v4.19, thanks.

Lorenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] PCI: aardvark: Convert to use pci_host_probe()

[Thomas Petazzoni]

Now that the pci_bus_size_bridges() call has been added to
advk_pcie_probe(), the end of the function is exactly an open-coded
version of pci_host_probe(). So instead of duplicating this code, use
pci_host_probe() directly.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 drivers/pci/controller/pci-aardvark.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 8e92231214e3..cea79e5ccd5d 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -1000,7 +1000,6 @@ static int advk_pcie_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct advk_pcie *pcie;
 	struct resource *res;
-	struct pci_bus *bus, *child;
 	struct pci_host_bridge *bridge;
 	int ret, irq;
 
@@ -1056,22 +1055,13 @@ static int advk_pcie_probe(struct platform_device *pdev)
 	bridge->map_irq = of_irq_parse_and_map_pci;
 	bridge->swizzle_irq = pci_common_swizzle;
 
-	ret = pci_scan_root_bus_bridge(bridge);
+	ret = pci_host_probe(bridge);
 	if (ret < 0) {
 		advk_pcie_remove_msi_irq_domain(pcie);
 		advk_pcie_remove_irq_domain(pcie);
 		return ret;
 	}
 
-	bus = bridge->bus;
-
-	pci_bus_size_bridges(bus);
-	pci_bus_assign_resources(bus);
-
-	list_for_each_entry(child, &bus->children, node)
-		pcie_bus_configure_settings(child);
-
-	pci_bus_add_devices(bus);
 	return 0;
 }
 
-- 
2.14.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel