Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] PCI: mvebu: Fix duplicate resource requests
@ 2020-10-23 14:52 Rob Herring
  2020-10-26 16:42 ` Jan Kundrát
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Rob Herring @ 2020-10-23 14:52 UTC (permalink / raw)
  To: vtolkm, Thomas Petazzoni, Jason Cooper
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, Russell King, linux-pci,
	linux-arm-kernel

With commit 669cbc708122 ("PCI: Move DT resource setup into
devm_pci_alloc_host_bridge()"), the DT 'ranges' is parsed and populated
into resources when the host bridge is allocated. The resources are
requested as well, but that happens a 2nd time for the mvebu driver in
mvebu_pcie_parse_request_resources(). We should only be requesting the
additional resources added in mvebu_pcie_parse_request_resources().
These are not added by default because the use custom properties rather
than standard DT address translation.

Also, the bus ranges was also populated by default, so we can remove
it from mvebu_pci_host_probe().

Fixes: 669cbc708122 ("PCI: Move DT resource setup into devm_pci_alloc_host_bridge()")
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=209729
Reported-by: vtolkm@googlemail.com
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-pci@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
v2:
 - Fix copy-n-paste error with ioport_resource
---
 drivers/pci/controller/pci-mvebu.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
index c39978b750ec..653c0b3d2912 100644
--- a/drivers/pci/controller/pci-mvebu.c
+++ b/drivers/pci/controller/pci-mvebu.c
@@ -960,25 +960,16 @@ static void mvebu_pcie_powerdown(struct mvebu_pcie_port *port)
 }
 
 /*
- * We can't use devm_of_pci_get_host_bridge_resources() because we
- * need to parse our special DT properties encoding the MEM and IO
- * apertures.
+ * devm_of_pci_get_host_bridge_resources() only sets up translateable resources,
+ * so we need extra resource setup parsing our special DT properties encoding
+ * the MEM and IO apertures.
  */
 static int mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
 {
 	struct device *dev = &pcie->pdev->dev;
-	struct device_node *np = dev->of_node;
 	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
 	int ret;
 
-	/* Get the bus range */
-	ret = of_pci_parse_bus_range(np, &pcie->busn);
-	if (ret) {
-		dev_err(dev, "failed to parse bus-range property: %d\n", ret);
-		return ret;
-	}
-	pci_add_resource(&bridge->windows, &pcie->busn);
-
 	/* Get the PCIe memory aperture */
 	mvebu_mbus_get_pcie_mem_aperture(&pcie->mem);
 	if (resource_size(&pcie->mem) == 0) {
@@ -988,6 +979,9 @@ static int mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
 
 	pcie->mem.name = "PCI MEM";
 	pci_add_resource(&bridge->windows, &pcie->mem);
+	ret = devm_request_resource(dev, &iomem_resource, &pcie->mem);
+	if (ret)
+		return ret;
 
 	/* Get the PCIe IO aperture */
 	mvebu_mbus_get_pcie_io_aperture(&pcie->io);
@@ -1001,9 +995,12 @@ static int mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
 		pcie->realio.name = "PCI I/O";
 
 		pci_add_resource(&bridge->windows, &pcie->realio);
+		ret = devm_request_resource(dev, &ioport_resource, &pcie->realio);
+		if (ret)
+			return ret;
 	}
 
-	return devm_request_pci_bus_resources(dev, &bridge->windows);
+	return 0;
 }
 
 /*
-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] PCI: mvebu: Fix duplicate resource requests
  2020-10-23 14:52 [PATCH v2] PCI: mvebu: Fix duplicate resource requests Rob Herring
@ 2020-10-26 16:42 ` Jan Kundrát
  2020-11-03 16:29 ` Lorenzo Pieralisi
  2020-11-03 19:32 ` Bjorn Helgaas
  2 siblings, 0 replies; 4+ messages in thread
From: Jan Kundrát @ 2020-10-26 16:42 UTC (permalink / raw)
  To: Rob Herring
  Cc: vtolkm, Thomas Petazzoni, Jason Cooper, Lorenzo Pieralisi,
	Bjorn Helgaas, Russell King, linux-pci, linux-arm-kernel

On pátek 23. října 2020 16:52:52 CEST, Rob Herring wrote:
> With commit 669cbc708122 ("PCI: Move DT resource setup into
> devm_pci_alloc_host_bridge()"), the DT 'ranges' is parsed and populated
> into resources when the host bridge is allocated. The resources are
> requested as well, but that happens a 2nd time for the mvebu driver in
> mvebu_pcie_parse_request_resources(). We should only be requesting the
> additional resources added in mvebu_pcie_parse_request_resources().
> These are not added by default because the use custom properties rather
> than standard DT address translation.
>
> Also, the bus ranges was also populated by default, so we can remove
> it from mvebu_pci_host_probe().
>
> Fixes: 669cbc708122 ("PCI: Move DT resource setup into 
> devm_pci_alloc_host_bridge()")
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=209729
> Reported-by: vtolkm@googlemail.com
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>

Without this patch, 5.9.1 won't find the eMMC card on Clearfog Base for me, 
so:

Tested-by: Jan Kundrát <jan.kundrat@cesnet.cz>

Thanks for the fix,
Jan

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] PCI: mvebu: Fix duplicate resource requests
  2020-10-23 14:52 [PATCH v2] PCI: mvebu: Fix duplicate resource requests Rob Herring
  2020-10-26 16:42 ` Jan Kundrát
@ 2020-11-03 16:29 ` Lorenzo Pieralisi
  2020-11-03 19:32 ` Bjorn Helgaas
  2 siblings, 0 replies; 4+ messages in thread
From: Lorenzo Pieralisi @ 2020-11-03 16:29 UTC (permalink / raw)
  To: Rob Herring, Bjorn Helgaas
  Cc: vtolkm, Thomas Petazzoni, Jason Cooper, Russell King, linux-pci,
	linux-arm-kernel

On Fri, Oct 23, 2020 at 09:52:52AM -0500, Rob Herring wrote:
> With commit 669cbc708122 ("PCI: Move DT resource setup into
> devm_pci_alloc_host_bridge()"), the DT 'ranges' is parsed and populated
> into resources when the host bridge is allocated. The resources are
> requested as well, but that happens a 2nd time for the mvebu driver in
> mvebu_pcie_parse_request_resources(). We should only be requesting the
> additional resources added in mvebu_pcie_parse_request_resources().
> These are not added by default because the use custom properties rather
> than standard DT address translation.
> 
> Also, the bus ranges was also populated by default, so we can remove
> it from mvebu_pci_host_probe().
> 
> Fixes: 669cbc708122 ("PCI: Move DT resource setup into devm_pci_alloc_host_bridge()")
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=209729
> Reported-by: vtolkm@googlemail.com
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> v2:
>  - Fix copy-n-paste error with ioport_resource
> ---
>  drivers/pci/controller/pci-mvebu.c | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)

Hi Bjorn, strictly speaking, this bug was introduced in v5.9 - it would
be good to fix it in one of the upcoming -rc*:

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
> index c39978b750ec..653c0b3d2912 100644
> --- a/drivers/pci/controller/pci-mvebu.c
> +++ b/drivers/pci/controller/pci-mvebu.c
> @@ -960,25 +960,16 @@ static void mvebu_pcie_powerdown(struct mvebu_pcie_port *port)
>  }
>  
>  /*
> - * We can't use devm_of_pci_get_host_bridge_resources() because we
> - * need to parse our special DT properties encoding the MEM and IO
> - * apertures.
> + * devm_of_pci_get_host_bridge_resources() only sets up translateable resources,
> + * so we need extra resource setup parsing our special DT properties encoding
> + * the MEM and IO apertures.
>   */
>  static int mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
>  {
>  	struct device *dev = &pcie->pdev->dev;
> -	struct device_node *np = dev->of_node;
>  	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
>  	int ret;
>  
> -	/* Get the bus range */
> -	ret = of_pci_parse_bus_range(np, &pcie->busn);
> -	if (ret) {
> -		dev_err(dev, "failed to parse bus-range property: %d\n", ret);
> -		return ret;
> -	}
> -	pci_add_resource(&bridge->windows, &pcie->busn);
> -
>  	/* Get the PCIe memory aperture */
>  	mvebu_mbus_get_pcie_mem_aperture(&pcie->mem);
>  	if (resource_size(&pcie->mem) == 0) {
> @@ -988,6 +979,9 @@ static int mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
>  
>  	pcie->mem.name = "PCI MEM";
>  	pci_add_resource(&bridge->windows, &pcie->mem);
> +	ret = devm_request_resource(dev, &iomem_resource, &pcie->mem);
> +	if (ret)
> +		return ret;
>  
>  	/* Get the PCIe IO aperture */
>  	mvebu_mbus_get_pcie_io_aperture(&pcie->io);
> @@ -1001,9 +995,12 @@ static int mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
>  		pcie->realio.name = "PCI I/O";
>  
>  		pci_add_resource(&bridge->windows, &pcie->realio);
> +		ret = devm_request_resource(dev, &ioport_resource, &pcie->realio);
> +		if (ret)
> +			return ret;
>  	}
>  
> -	return devm_request_pci_bus_resources(dev, &bridge->windows);
> +	return 0;
>  }
>  
>  /*
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] PCI: mvebu: Fix duplicate resource requests
  2020-10-23 14:52 [PATCH v2] PCI: mvebu: Fix duplicate resource requests Rob Herring
  2020-10-26 16:42 ` Jan Kundrát
  2020-11-03 16:29 ` Lorenzo Pieralisi
@ 2020-11-03 19:32 ` Bjorn Helgaas
  2 siblings, 0 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2020-11-03 19:32 UTC (permalink / raw)
  To: Rob Herring
  Cc: vtolkm, Thomas Petazzoni, Jason Cooper, Bjorn Helgaas, linux-pci,
	Lorenzo Pieralisi, Russell King, linux-arm-kernel

On Fri, Oct 23, 2020 at 09:52:52AM -0500, Rob Herring wrote:
> With commit 669cbc708122 ("PCI: Move DT resource setup into
> devm_pci_alloc_host_bridge()"), the DT 'ranges' is parsed and populated
> into resources when the host bridge is allocated. The resources are
> requested as well, but that happens a 2nd time for the mvebu driver in
> mvebu_pcie_parse_request_resources(). We should only be requesting the
> additional resources added in mvebu_pcie_parse_request_resources().
> These are not added by default because the use custom properties rather
> than standard DT address translation.
> 
> Also, the bus ranges was also populated by default, so we can remove
> it from mvebu_pci_host_probe().
> 
> Fixes: 669cbc708122 ("PCI: Move DT resource setup into devm_pci_alloc_host_bridge()")
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=209729
> Reported-by: vtolkm@googlemail.com
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>

Applied with Jan's tested-by and Lorenzo's ack to for-linus for
v5.10, thanks!

> ---
> v2:
>  - Fix copy-n-paste error with ioport_resource
> ---
>  drivers/pci/controller/pci-mvebu.c | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
> index c39978b750ec..653c0b3d2912 100644
> --- a/drivers/pci/controller/pci-mvebu.c
> +++ b/drivers/pci/controller/pci-mvebu.c
> @@ -960,25 +960,16 @@ static void mvebu_pcie_powerdown(struct mvebu_pcie_port *port)
>  }
>  
>  /*
> - * We can't use devm_of_pci_get_host_bridge_resources() because we
> - * need to parse our special DT properties encoding the MEM and IO
> - * apertures.
> + * devm_of_pci_get_host_bridge_resources() only sets up translateable resources,
> + * so we need extra resource setup parsing our special DT properties encoding
> + * the MEM and IO apertures.
>   */
>  static int mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
>  {
>  	struct device *dev = &pcie->pdev->dev;
> -	struct device_node *np = dev->of_node;
>  	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
>  	int ret;
>  
> -	/* Get the bus range */
> -	ret = of_pci_parse_bus_range(np, &pcie->busn);
> -	if (ret) {
> -		dev_err(dev, "failed to parse bus-range property: %d\n", ret);
> -		return ret;
> -	}
> -	pci_add_resource(&bridge->windows, &pcie->busn);
> -
>  	/* Get the PCIe memory aperture */
>  	mvebu_mbus_get_pcie_mem_aperture(&pcie->mem);
>  	if (resource_size(&pcie->mem) == 0) {
> @@ -988,6 +979,9 @@ static int mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
>  
>  	pcie->mem.name = "PCI MEM";
>  	pci_add_resource(&bridge->windows, &pcie->mem);
> +	ret = devm_request_resource(dev, &iomem_resource, &pcie->mem);
> +	if (ret)
> +		return ret;
>  
>  	/* Get the PCIe IO aperture */
>  	mvebu_mbus_get_pcie_io_aperture(&pcie->io);
> @@ -1001,9 +995,12 @@ static int mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
>  		pcie->realio.name = "PCI I/O";
>  
>  		pci_add_resource(&bridge->windows, &pcie->realio);
> +		ret = devm_request_resource(dev, &ioport_resource, &pcie->realio);
> +		if (ret)
> +			return ret;
>  	}
>  
> -	return devm_request_pci_bus_resources(dev, &bridge->windows);
> +	return 0;
>  }
>  
>  /*
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-23 14:52 [PATCH v2] PCI: mvebu: Fix duplicate resource requests Rob Herring
2020-10-26 16:42 ` Jan Kundrát
2020-11-03 16:29 ` Lorenzo Pieralisi
2020-11-03 19:32 ` Bjorn Helgaas

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org
	public-inbox-index linux-pci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git