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

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(+)

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
>

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

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

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

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