diff mbox

[v2,1/3] drivers: pci: add generic code to claim bus resources

Message ID 1456843449-19393-2-git-send-email-lorenzo.pieralisi@arm.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Lorenzo Pieralisi March 1, 2016, 2:44 p.m. UTC
From: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>

PCI core code contains a set of functions, eg:

pci_assign_unassigned_bus_resources()

that allow to assign the PCI resources for a given bus after
enumeration.

On systems where the PCI BARs are immutable (ie they must not and can
not be assigned), PCI bus resources should still be inserted in the PCI
resources tree (even if they are not (re)-assigned), but there is
no generic PCI kernel function for that purpose. Currently the PCI bus
resources insertion in the resources tree is implemented in an arch
specific fashion through arch specific callbacks that rely on the PCI
resources claiming API (for bridges and devices) which resulted in arches
implementations that contain duplicated code to claim resources for a
given PCI bus hierarchy.

This patch, based on the x86/ia64 resources claiming arch implementations,
implements a set of functions in core PCI code that provides a PCI core
interface for resources claiming for a given PCI bus hierarchy, paving
the way for further resource claiming consolidation across architectures.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/setup-bus.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h     |  1 +
 2 files changed, 69 insertions(+)

Comments

Lorenzo Pieralisi April 26, 2016, 12:47 p.m. UTC | #1
[Replying to self]

On Tue, Mar 01, 2016 at 02:44:07PM +0000, Lorenzo Pieralisi wrote:
> From: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>
> 
> PCI core code contains a set of functions, eg:
> 
> pci_assign_unassigned_bus_resources()
> 
> that allow to assign the PCI resources for a given bus after
> enumeration.
> 
> On systems where the PCI BARs are immutable (ie they must not and can
> not be assigned), PCI bus resources should still be inserted in the PCI
> resources tree (even if they are not (re)-assigned), but there is
> no generic PCI kernel function for that purpose. Currently the PCI bus
> resources insertion in the resources tree is implemented in an arch
> specific fashion through arch specific callbacks that rely on the PCI
> resources claiming API (for bridges and devices) which resulted in arches
> implementations that contain duplicated code to claim resources for a
> given PCI bus hierarchy.
> 
> This patch, based on the x86/ia64 resources claiming arch implementations,
> implements a set of functions in core PCI code that provides a PCI core
> interface for resources claiming for a given PCI bus hierarchy, paving
> the way for further resource claiming consolidation across architectures.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/setup-bus.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h     |  1 +
>  2 files changed, 69 insertions(+)

I have v3 ready, which does not change this patch, however I have
some questions below before asking to merge it.

> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 7796d0a..95f0906 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1424,6 +1424,74 @@ void pci_bus_assign_resources(const struct pci_bus *bus)
>  }
>  EXPORT_SYMBOL(pci_bus_assign_resources);
>  
> +static void pci_claim_device_resources(struct pci_dev *dev)
> +{
> +	int i;
> +
> +	for (i = 0; i < PCI_BRIDGE_RESOURCES; i++) {
> +		struct resource *r = &dev->resource[i];
> +
> +		if (!r->flags || r->parent)
> +			continue;
> +
> +		pci_claim_resource(dev, i);

pci_claim_resource() (and in particular pci_find_parent_resource()) fails
if resource.start == 0x0. Is this intentional ? The resource.start == 0x0
check is used in other bits of the kernel to detect an "invalid"
resource, I am not sure I understand its implications correctly.

> +	}
> +}
> +
> +static void pci_claim_bridge_resources(struct pci_dev *dev)
> +{
> +	int i;
> +
> +	for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) {
> +		struct resource *r = &dev->resource[i];
> +
> +		if (!r->flags || r->parent)
> +			continue;
> +
> +		pci_claim_bridge_resource(dev, i);
> +	}
> +}
> +
> +static void pci_bus_allocate_dev_resources(struct pci_bus *b)
> +{
> +	struct pci_dev *dev;
> +	struct pci_bus *child;
> +
> +	list_for_each_entry(dev, &b->devices, bus_list) {
> +		pci_claim_device_resources(dev);
> +
> +		child = dev->subordinate;
> +		if (child)
> +			pci_bus_allocate_dev_resources(child);
> +	}
> +}
> +
> +static void pci_bus_allocate_resources(struct pci_bus *b)
> +{
> +	struct pci_bus *child;
> +
> +	/*
> +	 * Carry out a depth-first search on the PCI bus
> +	 * tree to allocate bridge apertures. Read the
> +	 * programmed bridge bases and recursively claim
> +	 * the respective bridge resources.
> +	 */
> +	if (b->self) {

Is it *always* correct to check b->self to detect a device backing
the PCI bridge ? All PCI bridges should have their ->self pointer
set-up correctly, I wanted to countercheck anyway.

Comments welcome, thanks.

Lorenzo

> +		pci_read_bridge_bases(b);
> +		pci_claim_bridge_resources(b->self);
> +	}
> +
> +	list_for_each_entry(child, &b->children, node)
> +		pci_bus_allocate_resources(child);
> +}
> +
> +void pci_bus_claim_resources(struct pci_bus *b)
> +{
> +	pci_bus_allocate_resources(b);
> +	pci_bus_allocate_dev_resources(b);
> +}
> +EXPORT_SYMBOL(pci_bus_claim_resources);
> +
>  static void __pci_bridge_assign_resources(const struct pci_dev *bridge,
>  					  struct list_head *add_head,
>  					  struct list_head *fail_head)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 2771625..e66eba7 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1102,6 +1102,7 @@ ssize_t pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void
>  /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
>  resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx);
>  void pci_bus_assign_resources(const struct pci_bus *bus);
> +void pci_bus_claim_resources(struct pci_bus *bus);
>  void pci_bus_size_bridges(struct pci_bus *bus);
>  int pci_claim_resource(struct pci_dev *, int);
>  int pci_claim_bridge_resource(struct pci_dev *bridge, int i);
> -- 
> 2.5.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 7796d0a..95f0906 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1424,6 +1424,74 @@  void pci_bus_assign_resources(const struct pci_bus *bus)
 }
 EXPORT_SYMBOL(pci_bus_assign_resources);
 
+static void pci_claim_device_resources(struct pci_dev *dev)
+{
+	int i;
+
+	for (i = 0; i < PCI_BRIDGE_RESOURCES; i++) {
+		struct resource *r = &dev->resource[i];
+
+		if (!r->flags || r->parent)
+			continue;
+
+		pci_claim_resource(dev, i);
+	}
+}
+
+static void pci_claim_bridge_resources(struct pci_dev *dev)
+{
+	int i;
+
+	for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) {
+		struct resource *r = &dev->resource[i];
+
+		if (!r->flags || r->parent)
+			continue;
+
+		pci_claim_bridge_resource(dev, i);
+	}
+}
+
+static void pci_bus_allocate_dev_resources(struct pci_bus *b)
+{
+	struct pci_dev *dev;
+	struct pci_bus *child;
+
+	list_for_each_entry(dev, &b->devices, bus_list) {
+		pci_claim_device_resources(dev);
+
+		child = dev->subordinate;
+		if (child)
+			pci_bus_allocate_dev_resources(child);
+	}
+}
+
+static void pci_bus_allocate_resources(struct pci_bus *b)
+{
+	struct pci_bus *child;
+
+	/*
+	 * Carry out a depth-first search on the PCI bus
+	 * tree to allocate bridge apertures. Read the
+	 * programmed bridge bases and recursively claim
+	 * the respective bridge resources.
+	 */
+	if (b->self) {
+		pci_read_bridge_bases(b);
+		pci_claim_bridge_resources(b->self);
+	}
+
+	list_for_each_entry(child, &b->children, node)
+		pci_bus_allocate_resources(child);
+}
+
+void pci_bus_claim_resources(struct pci_bus *b)
+{
+	pci_bus_allocate_resources(b);
+	pci_bus_allocate_dev_resources(b);
+}
+EXPORT_SYMBOL(pci_bus_claim_resources);
+
 static void __pci_bridge_assign_resources(const struct pci_dev *bridge,
 					  struct list_head *add_head,
 					  struct list_head *fail_head)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2771625..e66eba7 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1102,6 +1102,7 @@  ssize_t pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void
 /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
 resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx);
 void pci_bus_assign_resources(const struct pci_bus *bus);
+void pci_bus_claim_resources(struct pci_bus *bus);
 void pci_bus_size_bridges(struct pci_bus *bus);
 int pci_claim_resource(struct pci_dev *, int);
 int pci_claim_bridge_resource(struct pci_dev *bridge, int i);