PCI: aardvark: misc improvements
Subsystems (3)
Thomas Petazzoni
maintainercover PCI: aardvark: misc improvements
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(-)
Signature · 1 line
2.14.4
Thomas Petazzoni
maintainerPATCH 1/2 PCI: aardvark: Size bridges before resources allocation
From: Zachary Zhang <[email protected]>
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.
--- drivers/pci/controller/pci-aardvark.c | 1 + 1 file changed, 1 insertion(+)
drivers/pci/controller/pci-aardvark.c +1 -0
@@ -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)
Signature · 1 line
2.14.4
PATCH 1/2 PCI: aardvark: Size bridges before resources allocation
Quoted 16 lines · Thomas Petazzoni
From: Zachary Zhang <[email protected]>
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.
Quoted 6 lines
Signed-off-by Zachary Zhang <[email protected]>[Thomas: edit commit log.]Signed-off-by Thomas Petazzoni <[email protected]>--- 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
Quoted 16 lines
drivers/pci/controller/pci-aardvark.c +1
-0@@ -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)Signature · 1 line
2.14.4
Thomas Petazzoni
maintainerPATCH 1/2 PCI: aardvark: Size bridges before resources allocation
Hello Lorenzo,
Thanks for your feedback!
Quoted 4 lines · Lorenzo Pieralisi
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:
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
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
PATCH 1/2 PCI: aardvark: Size bridges before resources allocation
Quoted 18 lines · Thomas Petazzoni
Hello Lorenzo,
Thanks for your feedback!
Quoted 4 lines · Lorenzo Pieralisi
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.
Quoted 8 lines
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 <[email protected]>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
Quoted 7 lines
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
Thomas Petazzoni
maintainerPATCH 1/2 PCI: aardvark: Size bridges before resources allocation
Hello,
Quoted 10 lines · Lorenzo Pieralisi
Quoted 10 lines
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:
Quoted 8 lines
Quoted 5 lines
Fixes 8c39d710363c1 ("PCI: aardvark: Add Aardvark PCI host controller driver")Cc <[email protected]>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
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
Lorenzo Pieralisi
AppliedPATCH 1/2 PCI: aardvark: Size bridges before resources allocation
Quoted 20 lines · Thomas Petazzoni
Hello,
Quoted 16 lines · Lorenzo Pieralisi
Quoted 15 lines
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 <[email protected]>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.
Quoted 6 lines
Quoted 2 lines
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
Thomas Petazzoni
maintainerPATCH 2/2 PCI: aardvark: Convert to use pci_host_probe()
In reply to Thomas PetazzoniNow 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.
--- drivers/pci/controller/pci-aardvark.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)
drivers/pci/controller/pci-aardvark.c +1 -11
@@ -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 ; }
Signature · 1 line
2.14.4