[PATCH v3 0/4] ARM/ARM64: PCI: PCI_PROBE_ONLY clean-up

[PATCH v3 0/4] ARM/ARM64: PCI: PCI_PROBE_ONLY clean-up

[Lorenzo Pieralisi]

Current arm/arm64 code prevents enabling resources in the respective
pcibios_enable_device() callbacks if the PCI_PROBE_ONLY flag is set,
in that on those platforms the resources tree was not validated properly
since PCI resources are not currently claimed on arm/arm64 PCI_PROBE_ONLY
systems.

This is a temporary kludge, in that PCI_PROBE_ONLY flag is used to
describe systems with fixed resources, that can nonetheless be enabled
through the standard pci_enable_resources() call present in PCI core code,
(ie via the generic pcibios_enable_device() call).

To remove the PCI_PROBE_ONLY flag in arm/arm64 pcibios_enable_device()
callbacks, the PCI host controllers that can be used with
PCI_PROBE_ONLY configurations must make sure that devices resources are
validated and inserted in the kernel resource tree even on PCI_PROBE_ONLY
systems so that the generic pcibios_enable_device() generic code, while
enabling resources (pci_enable_resources()), does not find dangling
resources pointers (ie missing parent pointers) that are omens of
an incomplete resource tree, causing failures in resources enablement.

PCI core code provides interfaces to assign/reassign/reallocate PCI
bus resources but it is currently lacking an interface to claim
resources for a specific bus. Arches implement resources claiming through
ad-hoc code built on top of pci_claim_resource() API, but that code
cannot be leveraged on architectures like arm/arm64 that rely on the
generic PCI infrastructure to carry out resources claiming/assignment.

Therefore, to clean up the arm/arm64 resources enablement on
PCI_PROBE_ONLY systems this patchset implements four patches:

PATCH 1: Create PCI core code infrastructure to claim bus resources
PATCH 2: Leverage the infrastructure in PATCH 1 to claim resources in
         the PCI generic host controller on PCI_PROBE_ONLY systems

PATCH 3: Remove the PCI_PROBE_ONLY kludge from the arm64 PCI back-end to
         complete the clean-up and leverage the PCI generic
         pcibios_enable_device() implementation

PATCH 4: Remove the PCI_PROBE_ONLY kludge from the arm PCI back-end to
         complete the clean-up and leverage the PCI generic
         pcibios_enable_device() implementation

Tested on arm/arm64 systems with kvmtool and PCI host generic.

v2 -> v3

- Split patch 3 in two patches so that arm and arm64 are handled
  in different patches
- Improved commits logs, clarified resource parent assignment
- Rebased against v4.7-rc2

v2: https://patchwork.ozlabs.org/patch/590556/

v1 -> v2

- Rewrote patch 1 to recursively claim resources for the whole PCI bus
  hierarchy
- Updated commits logs/tags
- Rebased against v4.5-rc6

v1: https://patchwork.ozlabs.org/patch/545669/

Lorenzo Pieralisi (4):
  PCI: add generic code to claim bus resources
  PCI: host-generic: claim bus resources on PCI_PROBE_ONLY set-ups
  ARM64/PCI: remove arch specific pcibios_enable_device()
  ARM/PCI: remove arch specific pcibios_enable_device()

 arch/arm/kernel/bios32.c           | 12 -------
 arch/arm64/kernel/pci.c            | 13 --------
 drivers/pci/host/pci-host-common.c | 27 +++++++++++++--
 drivers/pci/setup-bus.c            | 68 ++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h                |  1 +
 5 files changed, 94 insertions(+), 27 deletions(-)

-- 
2.6.4


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

[PATCH v3 0/4] ARM/ARM64: PCI: PCI_PROBE_ONLY clean-up

[Lorenzo Pieralisi]

Current arm/arm64 code prevents enabling resources in the respective
pcibios_enable_device() callbacks if the PCI_PROBE_ONLY flag is set,
in that on those platforms the resources tree was not validated properly
since PCI resources are not currently claimed on arm/arm64 PCI_PROBE_ONLY
systems.

This is a temporary kludge, in that PCI_PROBE_ONLY flag is used to
describe systems with fixed resources, that can nonetheless be enabled
through the standard pci_enable_resources() call present in PCI core code,
(ie via the generic pcibios_enable_device() call).

To remove the PCI_PROBE_ONLY flag in arm/arm64 pcibios_enable_device()
callbacks, the PCI host controllers that can be used with
PCI_PROBE_ONLY configurations must make sure that devices resources are
validated and inserted in the kernel resource tree even on PCI_PROBE_ONLY
systems so that the generic pcibios_enable_device() generic code, while
enabling resources (pci_enable_resources()), does not find dangling
resources pointers (ie missing parent pointers) that are omens of
an incomplete resource tree, causing failures in resources enablement.

PCI core code provides interfaces to assign/reassign/reallocate PCI
bus resources but it is currently lacking an interface to claim
resources for a specific bus. Arches implement resources claiming through
ad-hoc code built on top of pci_claim_resource() API, but that code
cannot be leveraged on architectures like arm/arm64 that rely on the
generic PCI infrastructure to carry out resources claiming/assignment.

Therefore, to clean up the arm/arm64 resources enablement on
PCI_PROBE_ONLY systems this patchset implements four patches:

PATCH 1: Create PCI core code infrastructure to claim bus resources
PATCH 2: Leverage the infrastructure in PATCH 1 to claim resources in
         the PCI generic host controller on PCI_PROBE_ONLY systems

PATCH 3: Remove the PCI_PROBE_ONLY kludge from the arm64 PCI back-end to
         complete the clean-up and leverage the PCI generic
         pcibios_enable_device() implementation

PATCH 4: Remove the PCI_PROBE_ONLY kludge from the arm PCI back-end to
         complete the clean-up and leverage the PCI generic
         pcibios_enable_device() implementation

Tested on arm/arm64 systems with kvmtool and PCI host generic.

v2 -> v3

- Split patch 3 in two patches so that arm and arm64 are handled
  in different patches
- Improved commits logs, clarified resource parent assignment
- Rebased against v4.7-rc2

v2: https://patchwork.ozlabs.org/patch/590556/

v1 -> v2

- Rewrote patch 1 to recursively claim resources for the whole PCI bus
  hierarchy
- Updated commits logs/tags
- Rebased against v4.5-rc6

v1: https://patchwork.ozlabs.org/patch/545669/

Lorenzo Pieralisi (4):
  PCI: add generic code to claim bus resources
  PCI: host-generic: claim bus resources on PCI_PROBE_ONLY set-ups
  ARM64/PCI: remove arch specific pcibios_enable_device()
  ARM/PCI: remove arch specific pcibios_enable_device()

 arch/arm/kernel/bios32.c           | 12 -------
 arch/arm64/kernel/pci.c            | 13 --------
 drivers/pci/host/pci-host-common.c | 27 +++++++++++++--
 drivers/pci/setup-bus.c            | 68 ++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h                |  1 +
 5 files changed, 94 insertions(+), 27 deletions(-)

-- 
2.6.4

[PATCH v3 1/4] PCI: add generic code to claim bus resources

[Lorenzo Pieralisi]

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.

Based on the x86/ia64 resources claiming arch implementations, implement
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(+)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 55641a3..1d1a2c9 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1423,6 +1423,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 b67e4df..cea1a66 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1114,6 +1114,7 @@ int pci_set_vpd_size(struct pci_dev *dev, size_t len);
 /* 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.6.4


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

[PATCH v3 1/4] PCI: add generic code to claim bus resources

[Lorenzo Pieralisi]

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.

Based on the x86/ia64 resources claiming arch implementations, implement
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(+)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 55641a3..1d1a2c9 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1423,6 +1423,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 b67e4df..cea1a66 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1114,6 +1114,7 @@ int pci_set_vpd_size(struct pci_dev *dev, size_t len);
 /* 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.6.4

[PATCH v3 2/4] PCI: host-generic: claim bus resources on PCI_PROBE_ONLY set-ups

[Lorenzo Pieralisi]

The PCI host generic driver does not reassign bus resources on systems
that require the BARs set-up to be immutable (ie PCI_PROBE_ONLY) since
that would trigger system failures. Nonetheless, PCI bus resources
allocated to PCI bridge and devices must be claimed in order to be
validated and inserted in the kernel resource tree, but the current
driver omits the resources claiming and relies on arch specific kludges
to prevent probing failure (ie preventing resources enablement on
PCI_PROBE_ONLY systems).

Add code to the PCI host generic driver that correctly claims bus
resources upon probe on systems that are required to prevent
reassignment after bus enumeration, so that the allocated resources
can be enabled successfully upon PCI device drivers probing, without
resorting to arch back-ends workarounds.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: David Daney <david.daney@cavium.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pci-host-common.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
index 8cba7ab..1684882 100644
--- a/drivers/pci/host/pci-host-common.c
+++ b/drivers/pci/host/pci-host-common.c
@@ -154,8 +154,31 @@ int pci_host_common_probe(struct platform_device *pdev,
 	}
 
 	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
-
-	if (!pci_has_flag(PCI_PROBE_ONLY)) {
+	/*
+	 * For PCI bridges and PCI devices to be fully set-up, their
+	 * resources must be initialized and inserted in the kernel
+	 * resource tree, which implicitly means that the resources
+	 * are correctly inserted in the kernel resource tree and are
+	 * assigned a parent pointer to create the resource hierarchy.
+	 *
+	 * On PCI_PROBE_ONLY systems, the pci_bus_claim_resource()
+	 * API claims the resources as set-up by firmware, which takes
+	 * care of inserting them in the resource tree and setting their
+	 * parent pointers accordingly.
+	 *
+	 * On !PCI_PROBE_ONLY systems, the firmware set-up is discarded,
+	 * and resource assignment and insertion in the resource tree is
+	 * carried out through the PCI resources API:
+	 *
+	 * (ie pci_bus_size_bridges() and pci_bus_assign_resources())
+	 *
+	 * that sizes the bridges and assigns and inserts resources in the
+	 * kernel resource tree (which also implies setting up their parent
+	 * pointers), completing the resources set-up.
+	 */
+	if (pci_has_flag(PCI_PROBE_ONLY)) {
+		pci_bus_claim_resources(bus);
+	} else {
 		pci_bus_size_bridges(bus);
 		pci_bus_assign_resources(bus);
 
-- 
2.6.4


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

[PATCH v3 2/4] PCI: host-generic: claim bus resources on PCI_PROBE_ONLY set-ups

[Lorenzo Pieralisi]

The PCI host generic driver does not reassign bus resources on systems
that require the BARs set-up to be immutable (ie PCI_PROBE_ONLY) since
that would trigger system failures. Nonetheless, PCI bus resources
allocated to PCI bridge and devices must be claimed in order to be
validated and inserted in the kernel resource tree, but the current
driver omits the resources claiming and relies on arch specific kludges
to prevent probing failure (ie preventing resources enablement on
PCI_PROBE_ONLY systems).

Add code to the PCI host generic driver that correctly claims bus
resources upon probe on systems that are required to prevent
reassignment after bus enumeration, so that the allocated resources
can be enabled successfully upon PCI device drivers probing, without
resorting to arch back-ends workarounds.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: David Daney <david.daney@cavium.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pci-host-common.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
index 8cba7ab..1684882 100644
--- a/drivers/pci/host/pci-host-common.c
+++ b/drivers/pci/host/pci-host-common.c
@@ -154,8 +154,31 @@ int pci_host_common_probe(struct platform_device *pdev,
 	}
 
 	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
-
-	if (!pci_has_flag(PCI_PROBE_ONLY)) {
+	/*
+	 * For PCI bridges and PCI devices to be fully set-up, their
+	 * resources must be initialized and inserted in the kernel
+	 * resource tree, which implicitly means that the resources
+	 * are correctly inserted in the kernel resource tree and are
+	 * assigned a parent pointer to create the resource hierarchy.
+	 *
+	 * On PCI_PROBE_ONLY systems, the pci_bus_claim_resource()
+	 * API claims the resources as set-up by firmware, which takes
+	 * care of inserting them in the resource tree and setting their
+	 * parent pointers accordingly.
+	 *
+	 * On !PCI_PROBE_ONLY systems, the firmware set-up is discarded,
+	 * and resource assignment and insertion in the resource tree is
+	 * carried out through the PCI resources API:
+	 *
+	 * (ie pci_bus_size_bridges() and pci_bus_assign_resources())
+	 *
+	 * that sizes the bridges and assigns and inserts resources in the
+	 * kernel resource tree (which also implies setting up their parent
+	 * pointers), completing the resources set-up.
+	 */
+	if (pci_has_flag(PCI_PROBE_ONLY)) {
+		pci_bus_claim_resources(bus);
+	} else {
 		pci_bus_size_bridges(bus);
 		pci_bus_assign_resources(bus);
 
-- 
2.6.4

[PATCH v3 3/4] ARM64/PCI: remove arch specific pcibios_enable_device()

[Lorenzo Pieralisi]

The arm64 pcibios_enable_device() implementation exists solely
to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
on those systems the PCI resources are currently not claimed (ie
inserted in the PCI resource tree - which means their parent
pointer is not correctly set-up) therefore they can not be enabled
since this would trigger PCI set-ups failures.

By introducing resources claiming in the PCI host controllers set-ups
that have PCI_PROBE_ONLY as a probe option, there is no need for arch
specific pcibios_enable_device() implementations anymore in that the
kernel can rely on the generic pcibios_enable_device() implementation
without resorting to arch specific code to work around the missing
resources claiming enumeration step.

On !PCI_PROBE_ONLY PCI bus set-ups, resources are always assigned
in the PCI host controllers drivers; since the PCI resource assignment
API takes care of inserting the assigned resources in the resource tree,
the resources parent pointer is correctly set-up, which means that this
patch leaves behaviour unchanged for all arm64 PCI set-ups that do not
set the PCI_PROBE_ONLY flag.

Remove the pcibios_enable_device() function from the arm64 arch
back-end so that the kernel now uses its generic implementation.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/kernel/pci.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 3c4e308..39cfa03 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -36,19 +36,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 	return res->start;
 }
 
-/**
- * pcibios_enable_device - Enable I/O and memory.
- * @dev: PCI device to be enabled
- * @mask: bitmask of BARs to enable
- */
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	if (pci_has_flag(PCI_PROBE_ONLY))
-		return 0;
-
-	return pci_enable_resources(dev, mask);
-}
-
 /*
  * Try to assign the IRQ number from DT when adding a new device
  */
-- 
2.6.4


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

[PATCH v3 3/4] ARM64/PCI: remove arch specific pcibios_enable_device()

[Lorenzo Pieralisi]

The arm64 pcibios_enable_device() implementation exists solely
to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
on those systems the PCI resources are currently not claimed (ie
inserted in the PCI resource tree - which means their parent
pointer is not correctly set-up) therefore they can not be enabled
since this would trigger PCI set-ups failures.

By introducing resources claiming in the PCI host controllers set-ups
that have PCI_PROBE_ONLY as a probe option, there is no need for arch
specific pcibios_enable_device() implementations anymore in that the
kernel can rely on the generic pcibios_enable_device() implementation
without resorting to arch specific code to work around the missing
resources claiming enumeration step.

On !PCI_PROBE_ONLY PCI bus set-ups, resources are always assigned
in the PCI host controllers drivers; since the PCI resource assignment
API takes care of inserting the assigned resources in the resource tree,
the resources parent pointer is correctly set-up, which means that this
patch leaves behaviour unchanged for all arm64 PCI set-ups that do not
set the PCI_PROBE_ONLY flag.

Remove the pcibios_enable_device() function from the arm64 arch
back-end so that the kernel now uses its generic implementation.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/kernel/pci.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 3c4e308..39cfa03 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -36,19 +36,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 	return res->start;
 }
 
-/**
- * pcibios_enable_device - Enable I/O and memory.
- * @dev: PCI device to be enabled
- * @mask: bitmask of BARs to enable
- */
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	if (pci_has_flag(PCI_PROBE_ONLY))
-		return 0;
-
-	return pci_enable_resources(dev, mask);
-}
-
 /*
  * Try to assign the IRQ number from DT when adding a new device
  */
-- 
2.6.4

[PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Lorenzo Pieralisi]

The arm pcibios_enable_device() implementation exists solely
to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
on those systems the PCI resources are currently not claimed (ie
inserted in the PCI resource tree - which means their parent
pointer is not correctly set-up) therefore they can not be enabled
since this would trigger PCI set-ups failures.

After removing the pci=firmware command line option in:

commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
command line parameter handling")

(that was used to set the PCI_PROBE_ONLY flag through the command line)
and by introducing resources claiming in the PCI host controllers
set-ups that have PCI_PROBE_ONLY as a probe option, there is no need for
arch specific pcibios_enable_device() implementations anymore in that
the kernel can rely on the generic pcibios_enable_device()
implementation without resorting to arch specific code to work around
the missing resources claiming enumeration step.

On !PCI_PROBE_ONLY PCI bus set-ups, resources are always assigned
either in pcibios initialization code or PCI host controllers drivers;
since the PCI resource assignment API takes care of inserting the
assigned resources in the resource tree, the resources parent pointers
are correctly set-up, which means that this patch leaves behaviour
unchanged for all arm PCI set-ups that do not set the PCI_PROBE_ONLY
flag.

Remove the pcibios_enable_device() function from the arm arch back-end
so that the kernel now uses its generic implementation.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <linux@arm.linux.org.uk>
---
 arch/arm/kernel/bios32.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 05e61a2..488545f 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -590,18 +590,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 	return start;
 }
 
-/**
- * pcibios_enable_device - Enable I/O and memory.
- * @dev: PCI device to be enabled
- */
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	if (pci_has_flag(PCI_PROBE_ONLY))
-		return 0;
-
-	return pci_enable_resources(dev, mask);
-}
-
 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
 			enum pci_mmap_state mmap_state, int write_combine)
 {
-- 
2.6.4


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

[PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Lorenzo Pieralisi]

The arm pcibios_enable_device() implementation exists solely
to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
on those systems the PCI resources are currently not claimed (ie
inserted in the PCI resource tree - which means their parent
pointer is not correctly set-up) therefore they can not be enabled
since this would trigger PCI set-ups failures.

After removing the pci=firmware command line option in:

commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
command line parameter handling")

(that was used to set the PCI_PROBE_ONLY flag through the command line)
and by introducing resources claiming in the PCI host controllers
set-ups that have PCI_PROBE_ONLY as a probe option, there is no need for
arch specific pcibios_enable_device() implementations anymore in that
the kernel can rely on the generic pcibios_enable_device()
implementation without resorting to arch specific code to work around
the missing resources claiming enumeration step.

On !PCI_PROBE_ONLY PCI bus set-ups, resources are always assigned
either in pcibios initialization code or PCI host controllers drivers;
since the PCI resource assignment API takes care of inserting the
assigned resources in the resource tree, the resources parent pointers
are correctly set-up, which means that this patch leaves behaviour
unchanged for all arm PCI set-ups that do not set the PCI_PROBE_ONLY
flag.

Remove the pcibios_enable_device() function from the arm arch back-end
so that the kernel now uses its generic implementation.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <linux@arm.linux.org.uk>
---
 arch/arm/kernel/bios32.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 05e61a2..488545f 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -590,18 +590,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 	return start;
 }
 
-/**
- * pcibios_enable_device - Enable I/O and memory.
- * @dev: PCI device to be enabled
- */
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	if (pci_has_flag(PCI_PROBE_ONLY))
-		return 0;
-
-	return pci_enable_resources(dev, mask);
-}
-
 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
 			enum pci_mmap_state mmap_state, int write_combine)
 {
-- 
2.6.4

Re: [PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Bjorn Helgaas]

On Wed, Jun 08, 2016 at 12:04:50PM +0100, Lorenzo Pieralisi wrote:
> The arm pcibios_enable_device() implementation exists solely
> to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
> on those systems the PCI resources are currently not claimed (ie
> inserted in the PCI resource tree - which means their parent
> pointer is not correctly set-up) therefore they can not be enabled
> since this would trigger PCI set-ups failures.
> 
> After removing the pci=firmware command line option in:
> 
> commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
> command line parameter handling")
> 
> (that was used to set the PCI_PROBE_ONLY flag through the command line)
> and by introducing resources claiming in the PCI host controllers
> set-ups that have PCI_PROBE_ONLY as a probe option, there is no need for
> arch specific pcibios_enable_device() implementations anymore in that
> the kernel can rely on the generic pcibios_enable_device()
> implementation without resorting to arch specific code to work around
> the missing resources claiming enumeration step.
> 
> On !PCI_PROBE_ONLY PCI bus set-ups, resources are always assigned
> either in pcibios initialization code or PCI host controllers drivers;
> since the PCI resource assignment API takes care of inserting the
> assigned resources in the resource tree, the resources parent pointers
> are correctly set-up, which means that this patch leaves behaviour
> unchanged for all arm PCI set-ups that do not set the PCI_PROBE_ONLY
> flag.
> 
> Remove the pcibios_enable_device() function from the arm arch back-end
> so that the kernel now uses its generic implementation.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Acked-by: Will Deacon <will.deacon@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> ---
>  arch/arm/kernel/bios32.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index 05e61a2..488545f 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -590,18 +590,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  	return start;
>  }
>  
> -/**
> - * pcibios_enable_device - Enable I/O and memory.
> - * @dev: PCI device to be enabled
> - */
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> -{
> -	if (pci_has_flag(PCI_PROBE_ONLY))
> -		return 0;
> -
> -	return pci_enable_resources(dev, mask);
> -}

This looks great.

What about the PCI_PROBE_ONLY test in pci_common_init_dev()?  Don't we
need to either remove that test (if it's impossible to get there with
PCI_PROBE_ONLY set), or add a pci_bus_claim_resources() call as we did
in pci_host_common_probe()?

I think it's unlikely that we'd get to pci_common_init_dev() with
PCI_PROBE_ONLY set:

  - the only way to set PCI_PROBE_ONLY on ARM is to call
    of_pci_check_probe_only(),

  - the only ARM caller of of_pci_check_probe_only() is
    pci_host_common_probe(),

  - pci_host_common_probe() doesn't call pci_common_init_dev().

But I guess it's possible to imagine a platform with both a generic
PCI bridge and a MVEBU, R-Car, or Tegra bridge.  Then
pci_host_common_probe() could set PCI_PROBE_ONLY, and we'd claim
resources under the generic bridge via the previous patch, but still
not claim those under the MVEBU bridge.  Then enabling the MVEBU
devices would fail.

I know this is a ridiculous scenario, but the code looks inconsistent
as it is.

>  int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
>  			enum pci_mmap_state mmap_state, int write_combine)
>  {
> -- 
> 2.6.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Bjorn Helgaas]

On Wed, Jun 08, 2016 at 12:04:50PM +0100, Lorenzo Pieralisi wrote:
> The arm pcibios_enable_device() implementation exists solely
> to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
> on those systems the PCI resources are currently not claimed (ie
> inserted in the PCI resource tree - which means their parent
> pointer is not correctly set-up) therefore they can not be enabled
> since this would trigger PCI set-ups failures.
> 
> After removing the pci=firmware command line option in:
> 
> commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
> command line parameter handling")
> 
> (that was used to set the PCI_PROBE_ONLY flag through the command line)
> and by introducing resources claiming in the PCI host controllers
> set-ups that have PCI_PROBE_ONLY as a probe option, there is no need for
> arch specific pcibios_enable_device() implementations anymore in that
> the kernel can rely on the generic pcibios_enable_device()
> implementation without resorting to arch specific code to work around
> the missing resources claiming enumeration step.
> 
> On !PCI_PROBE_ONLY PCI bus set-ups, resources are always assigned
> either in pcibios initialization code or PCI host controllers drivers;
> since the PCI resource assignment API takes care of inserting the
> assigned resources in the resource tree, the resources parent pointers
> are correctly set-up, which means that this patch leaves behaviour
> unchanged for all arm PCI set-ups that do not set the PCI_PROBE_ONLY
> flag.
> 
> Remove the pcibios_enable_device() function from the arm arch back-end
> so that the kernel now uses its generic implementation.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Acked-by: Will Deacon <will.deacon@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> ---
>  arch/arm/kernel/bios32.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index 05e61a2..488545f 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -590,18 +590,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  	return start;
>  }
>  
> -/**
> - * pcibios_enable_device - Enable I/O and memory.
> - * @dev: PCI device to be enabled
> - */
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> -{
> -	if (pci_has_flag(PCI_PROBE_ONLY))
> -		return 0;
> -
> -	return pci_enable_resources(dev, mask);
> -}

This looks great.

What about the PCI_PROBE_ONLY test in pci_common_init_dev()?  Don't we
need to either remove that test (if it's impossible to get there with
PCI_PROBE_ONLY set), or add a pci_bus_claim_resources() call as we did
in pci_host_common_probe()?

I think it's unlikely that we'd get to pci_common_init_dev() with
PCI_PROBE_ONLY set:

  - the only way to set PCI_PROBE_ONLY on ARM is to call
    of_pci_check_probe_only(),

  - the only ARM caller of of_pci_check_probe_only() is
    pci_host_common_probe(),

  - pci_host_common_probe() doesn't call pci_common_init_dev().

But I guess it's possible to imagine a platform with both a generic
PCI bridge and a MVEBU, R-Car, or Tegra bridge.  Then
pci_host_common_probe() could set PCI_PROBE_ONLY, and we'd claim
resources under the generic bridge via the previous patch, but still
not claim those under the MVEBU bridge.  Then enabling the MVEBU
devices would fail.

I know this is a ridiculous scenario, but the code looks inconsistent
as it is.

>  int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
>  			enum pci_mmap_state mmap_state, int write_combine)
>  {
> -- 
> 2.6.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Lorenzo Pieralisi]

On Wed, Jun 22, 2016 at 05:43:58PM -0500, Bjorn Helgaas wrote:
> On Wed, Jun 08, 2016 at 12:04:50PM +0100, Lorenzo Pieralisi wrote:
> > The arm pcibios_enable_device() implementation exists solely
> > to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
> > on those systems the PCI resources are currently not claimed (ie
> > inserted in the PCI resource tree - which means their parent
> > pointer is not correctly set-up) therefore they can not be enabled
> > since this would trigger PCI set-ups failures.
> > 
> > After removing the pci=firmware command line option in:
> > 
> > commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
> > command line parameter handling")
> > 
> > (that was used to set the PCI_PROBE_ONLY flag through the command line)
> > and by introducing resources claiming in the PCI host controllers
> > set-ups that have PCI_PROBE_ONLY as a probe option, there is no need for
> > arch specific pcibios_enable_device() implementations anymore in that
> > the kernel can rely on the generic pcibios_enable_device()
> > implementation without resorting to arch specific code to work around
> > the missing resources claiming enumeration step.
> > 
> > On !PCI_PROBE_ONLY PCI bus set-ups, resources are always assigned
> > either in pcibios initialization code or PCI host controllers drivers;
> > since the PCI resource assignment API takes care of inserting the
> > assigned resources in the resource tree, the resources parent pointers
> > are correctly set-up, which means that this patch leaves behaviour
> > unchanged for all arm PCI set-ups that do not set the PCI_PROBE_ONLY
> > flag.
> > 
> > Remove the pcibios_enable_device() function from the arm arch back-end
> > so that the kernel now uses its generic implementation.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Acked-by: Will Deacon <will.deacon@arm.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Russell King <linux@arm.linux.org.uk>
> > ---
> >  arch/arm/kernel/bios32.c | 12 ------------
> >  1 file changed, 12 deletions(-)
> > 
> > diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> > index 05e61a2..488545f 100644
> > --- a/arch/arm/kernel/bios32.c
> > +++ b/arch/arm/kernel/bios32.c
> > @@ -590,18 +590,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> >  	return start;
> >  }
> >  
> > -/**
> > - * pcibios_enable_device - Enable I/O and memory.
> > - * @dev: PCI device to be enabled
> > - */
> > -int pcibios_enable_device(struct pci_dev *dev, int mask)
> > -{
> > -	if (pci_has_flag(PCI_PROBE_ONLY))
> > -		return 0;
> > -
> > -	return pci_enable_resources(dev, mask);
> > -}
> 
> This looks great.
> 
> What about the PCI_PROBE_ONLY test in pci_common_init_dev()?  Don't we
> need to either remove that test (if it's impossible to get there with
> PCI_PROBE_ONLY set), or add a pci_bus_claim_resources() call as we did
> in pci_host_common_probe()?

Yes, you are right, I went for the second option given what you
say below and sent you and Russell an additional patch that
should be added to this series.

> I think it's unlikely that we'd get to pci_common_init_dev() with
> PCI_PROBE_ONLY set:
> 
>   - the only way to set PCI_PROBE_ONLY on ARM is to call
>     of_pci_check_probe_only(),
> 
>   - the only ARM caller of of_pci_check_probe_only() is
>     pci_host_common_probe(),
> 
>   - pci_host_common_probe() doesn't call pci_common_init_dev().
> 
> But I guess it's possible to imagine a platform with both a generic
> PCI bridge and a MVEBU, R-Car, or Tegra bridge.  Then
> pci_host_common_probe() could set PCI_PROBE_ONLY, and we'd claim
> resources under the generic bridge via the previous patch, but still
> not claim those under the MVEBU bridge.  Then enabling the MVEBU
> devices would fail.
> 
> I know this is a ridiculous scenario, but the code looks inconsistent
> as it is.

Well, I do not think that's an *existing* scenario, but this does
not mean that the code is correct, so you are right and it has to
be fixed, please let me know if the patch I sent is fine.

I really have to remove PCI_PROBE_ONLY entirely from ARM/ARM64
kernels.

Thanks !
Lorenzo

> 
> >  int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
> >  			enum pci_mmap_state mmap_state, int write_combine)
> >  {
> > -- 
> > 2.6.4
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Lorenzo Pieralisi]

On Wed, Jun 22, 2016 at 05:43:58PM -0500, Bjorn Helgaas wrote:
> On Wed, Jun 08, 2016 at 12:04:50PM +0100, Lorenzo Pieralisi wrote:
> > The arm pcibios_enable_device() implementation exists solely
> > to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
> > on those systems the PCI resources are currently not claimed (ie
> > inserted in the PCI resource tree - which means their parent
> > pointer is not correctly set-up) therefore they can not be enabled
> > since this would trigger PCI set-ups failures.
> > 
> > After removing the pci=firmware command line option in:
> > 
> > commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
> > command line parameter handling")
> > 
> > (that was used to set the PCI_PROBE_ONLY flag through the command line)
> > and by introducing resources claiming in the PCI host controllers
> > set-ups that have PCI_PROBE_ONLY as a probe option, there is no need for
> > arch specific pcibios_enable_device() implementations anymore in that
> > the kernel can rely on the generic pcibios_enable_device()
> > implementation without resorting to arch specific code to work around
> > the missing resources claiming enumeration step.
> > 
> > On !PCI_PROBE_ONLY PCI bus set-ups, resources are always assigned
> > either in pcibios initialization code or PCI host controllers drivers;
> > since the PCI resource assignment API takes care of inserting the
> > assigned resources in the resource tree, the resources parent pointers
> > are correctly set-up, which means that this patch leaves behaviour
> > unchanged for all arm PCI set-ups that do not set the PCI_PROBE_ONLY
> > flag.
> > 
> > Remove the pcibios_enable_device() function from the arm arch back-end
> > so that the kernel now uses its generic implementation.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Acked-by: Will Deacon <will.deacon@arm.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Russell King <linux@arm.linux.org.uk>
> > ---
> >  arch/arm/kernel/bios32.c | 12 ------------
> >  1 file changed, 12 deletions(-)
> > 
> > diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> > index 05e61a2..488545f 100644
> > --- a/arch/arm/kernel/bios32.c
> > +++ b/arch/arm/kernel/bios32.c
> > @@ -590,18 +590,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> >  	return start;
> >  }
> >  
> > -/**
> > - * pcibios_enable_device - Enable I/O and memory.
> > - * @dev: PCI device to be enabled
> > - */
> > -int pcibios_enable_device(struct pci_dev *dev, int mask)
> > -{
> > -	if (pci_has_flag(PCI_PROBE_ONLY))
> > -		return 0;
> > -
> > -	return pci_enable_resources(dev, mask);
> > -}
> 
> This looks great.
> 
> What about the PCI_PROBE_ONLY test in pci_common_init_dev()?  Don't we
> need to either remove that test (if it's impossible to get there with
> PCI_PROBE_ONLY set), or add a pci_bus_claim_resources() call as we did
> in pci_host_common_probe()?

Yes, you are right, I went for the second option given what you
say below and sent you and Russell an additional patch that
should be added to this series.

> I think it's unlikely that we'd get to pci_common_init_dev() with
> PCI_PROBE_ONLY set:
> 
>   - the only way to set PCI_PROBE_ONLY on ARM is to call
>     of_pci_check_probe_only(),
> 
>   - the only ARM caller of of_pci_check_probe_only() is
>     pci_host_common_probe(),
> 
>   - pci_host_common_probe() doesn't call pci_common_init_dev().
> 
> But I guess it's possible to imagine a platform with both a generic
> PCI bridge and a MVEBU, R-Car, or Tegra bridge.  Then
> pci_host_common_probe() could set PCI_PROBE_ONLY, and we'd claim
> resources under the generic bridge via the previous patch, but still
> not claim those under the MVEBU bridge.  Then enabling the MVEBU
> devices would fail.
> 
> I know this is a ridiculous scenario, but the code looks inconsistent
> as it is.

Well, I do not think that's an *existing* scenario, but this does
not mean that the code is correct, so you are right and it has to
be fixed, please let me know if the patch I sent is fine.

I really have to remove PCI_PROBE_ONLY entirely from ARM/ARM64
kernels.

Thanks !
Lorenzo

> 
> >  int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
> >  			enum pci_mmap_state mmap_state, int write_combine)
> >  {
> > -- 
> > 2.6.4
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Bjorn Helgaas]

[+cc Guan]

On Wed, Jun 08, 2016 at 12:04:50PM +0100, Lorenzo Pieralisi wrote:
> The arm pcibios_enable_device() implementation exists solely
> to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
> on those systems the PCI resources are currently not claimed (ie
> inserted in the PCI resource tree - which means their parent
> pointer is not correctly set-up) therefore they can not be enabled
> since this would trigger PCI set-ups failures.
> 
> After removing the pci=firmware command line option in:
> 
> commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
> command line parameter handling")

903589ca7165 removed "pci=firmware" from ARM and from
Documentation/kernel-parameters.txt (where it was documented as
ARM-specific).

I think unicore32 copied that code from arm, so unicore32 is one of
the few remaining arches that can set PCI_PROBE_ONLY.  I suspect this
was just copied from arm and may not be necessary on unicore32.

Guan, can you confirm that PCI_PROBE_ONLY is necessary on unicore32,
or that it can be removed?

> (that was used to set the PCI_PROBE_ONLY flag through the command line)
> and by introducing resources claiming in the PCI host controllers
> set-ups that have PCI_PROBE_ONLY as a probe option, there is no need for
> arch specific pcibios_enable_device() implementations anymore in that
> the kernel can rely on the generic pcibios_enable_device()
> implementation without resorting to arch specific code to work around
> the missing resources claiming enumeration step.
> 
> On !PCI_PROBE_ONLY PCI bus set-ups, resources are always assigned
> either in pcibios initialization code or PCI host controllers drivers;
> since the PCI resource assignment API takes care of inserting the
> assigned resources in the resource tree, the resources parent pointers
> are correctly set-up, which means that this patch leaves behaviour
> unchanged for all arm PCI set-ups that do not set the PCI_PROBE_ONLY
> flag.
> 
> Remove the pcibios_enable_device() function from the arm arch back-end
> so that the kernel now uses its generic implementation.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Acked-by: Will Deacon <will.deacon@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> ---
>  arch/arm/kernel/bios32.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index 05e61a2..488545f 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -590,18 +590,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  	return start;
>  }
>  
> -/**
> - * pcibios_enable_device - Enable I/O and memory.
> - * @dev: PCI device to be enabled
> - */
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> -{
> -	if (pci_has_flag(PCI_PROBE_ONLY))
> -		return 0;
> -
> -	return pci_enable_resources(dev, mask);
> -}
> -
>  int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
>  			enum pci_mmap_state mmap_state, int write_combine)
>  {
> -- 
> 2.6.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Bjorn Helgaas]

[+cc Guan]

On Wed, Jun 08, 2016 at 12:04:50PM +0100, Lorenzo Pieralisi wrote:
> The arm pcibios_enable_device() implementation exists solely
> to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
> on those systems the PCI resources are currently not claimed (ie
> inserted in the PCI resource tree - which means their parent
> pointer is not correctly set-up) therefore they can not be enabled
> since this would trigger PCI set-ups failures.
> 
> After removing the pci=firmware command line option in:
> 
> commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
> command line parameter handling")

903589ca7165 removed "pci=firmware" from ARM and from
Documentation/kernel-parameters.txt (where it was documented as
ARM-specific).

I think unicore32 copied that code from arm, so unicore32 is one of
the few remaining arches that can set PCI_PROBE_ONLY.  I suspect this
was just copied from arm and may not be necessary on unicore32.

Guan, can you confirm that PCI_PROBE_ONLY is necessary on unicore32,
or that it can be removed?

> (that was used to set the PCI_PROBE_ONLY flag through the command line)
> and by introducing resources claiming in the PCI host controllers
> set-ups that have PCI_PROBE_ONLY as a probe option, there is no need for
> arch specific pcibios_enable_device() implementations anymore in that
> the kernel can rely on the generic pcibios_enable_device()
> implementation without resorting to arch specific code to work around
> the missing resources claiming enumeration step.
> 
> On !PCI_PROBE_ONLY PCI bus set-ups, resources are always assigned
> either in pcibios initialization code or PCI host controllers drivers;
> since the PCI resource assignment API takes care of inserting the
> assigned resources in the resource tree, the resources parent pointers
> are correctly set-up, which means that this patch leaves behaviour
> unchanged for all arm PCI set-ups that do not set the PCI_PROBE_ONLY
> flag.
> 
> Remove the pcibios_enable_device() function from the arm arch back-end
> so that the kernel now uses its generic implementation.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Acked-by: Will Deacon <will.deacon@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> ---
>  arch/arm/kernel/bios32.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index 05e61a2..488545f 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -590,18 +590,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  	return start;
>  }
>  
> -/**
> - * pcibios_enable_device - Enable I/O and memory.
> - * @dev: PCI device to be enabled
> - */
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> -{
> -	if (pci_has_flag(PCI_PROBE_ONLY))
> -		return 0;
> -
> -	return pci_enable_resources(dev, mask);
> -}
> -
>  int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
>  			enum pci_mmap_state mmap_state, int write_combine)
>  {
> -- 
> 2.6.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Xuetao Guan]

> [+cc Guan]
>
> On Wed, Jun 08, 2016 at 12:04:50PM +0100, Lorenzo Pieralisi wrote:
>> The arm pcibios_enable_device() implementation exists solely
>> to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
>> on those systems the PCI resources are currently not claimed (ie
>> inserted in the PCI resource tree - which means their parent
>> pointer is not correctly set-up) therefore they can not be enabled
>> since this would trigger PCI set-ups failures.
>>
>> After removing the pci=firmware command line option in:
>>
>> commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
>> command line parameter handling")
>
> 903589ca7165 removed "pci=firmware" from ARM and from
> Documentation/kernel-parameters.txt (where it was documented as
> ARM-specific).
>
> I think unicore32 copied that code from arm, so unicore32 is one of
> the few remaining arches that can set PCI_PROBE_ONLY.  I suspect this
> was just copied from arm and may not be necessary on unicore32.
>
> Guan, can you confirm that PCI_PROBE_ONLY is necessary on unicore32,
> or that it can be removed?
>

Yes, this code was copied from arm, and it's not necessary.
It can be removed safely.

Guan Xuetao


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

[PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Xuetao Guan]

> [+cc Guan]
>
> On Wed, Jun 08, 2016 at 12:04:50PM +0100, Lorenzo Pieralisi wrote:
>> The arm pcibios_enable_device() implementation exists solely
>> to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
>> on those systems the PCI resources are currently not claimed (ie
>> inserted in the PCI resource tree - which means their parent
>> pointer is not correctly set-up) therefore they can not be enabled
>> since this would trigger PCI set-ups failures.
>>
>> After removing the pci=firmware command line option in:
>>
>> commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
>> command line parameter handling")
>
> 903589ca7165 removed "pci=firmware" from ARM and from
> Documentation/kernel-parameters.txt (where it was documented as
> ARM-specific).
>
> I think unicore32 copied that code from arm, so unicore32 is one of
> the few remaining arches that can set PCI_PROBE_ONLY.  I suspect this
> was just copied from arm and may not be necessary on unicore32.
>
> Guan, can you confirm that PCI_PROBE_ONLY is necessary on unicore32,
> or that it can be removed?
>

Yes, this code was copied from arm, and it's not necessary.
It can be removed safely.

Guan Xuetao

Re: [PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Bjorn Helgaas]

On Thu, Jun 23, 2016 at 06:39:03PM +0800, Xuetao Guan wrote:
> > [+cc Guan]
> >
> > On Wed, Jun 08, 2016 at 12:04:50PM +0100, Lorenzo Pieralisi wrote:
> >> The arm pcibios_enable_device() implementation exists solely
> >> to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
> >> on those systems the PCI resources are currently not claimed (ie
> >> inserted in the PCI resource tree - which means their parent
> >> pointer is not correctly set-up) therefore they can not be enabled
> >> since this would trigger PCI set-ups failures.
> >>
> >> After removing the pci=firmware command line option in:
> >>
> >> commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
> >> command line parameter handling")
> >
> > 903589ca7165 removed "pci=firmware" from ARM and from
> > Documentation/kernel-parameters.txt (where it was documented as
> > ARM-specific).
> >
> > I think unicore32 copied that code from arm, so unicore32 is one of
> > the few remaining arches that can set PCI_PROBE_ONLY.  I suspect this
> > was just copied from arm and may not be necessary on unicore32.
> >
> > Guan, can you confirm that PCI_PROBE_ONLY is necessary on unicore32,
> > or that it can be removed?
> >
> 
> Yes, this code was copied from arm, and it's not necessary.
> It can be removed safely.

Great, thank you!  I'll add the patch below to my pci/resource branch for
v4.8.  Let me know if you see any issues with it.


commit b9b8a53e24649e48c0a8bceb8d9d8fe1cfee018c
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Thu Jun 23 11:33:24 2016 -0500

    unicore32/PCI: Remove pci=firmware command line parameter handling
    
    Remove support for the "pci=firmware" command line parameter, which was
    way to keep the kernel from changing any PCI BAR assignments.  This was
    copied from ARM, but is not actually needed on unicore32.
    
    The corresponding ARM support was removed by 903589ca7165 ("ARM: 8554/1:
    kernel: pci: remove pci=firmware command line parameter handling").
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
index d45fa5f..62137d1 100644
--- a/arch/unicore32/kernel/pci.c
+++ b/arch/unicore32/kernel/pci.c
@@ -265,10 +265,8 @@ static int __init pci_common_init(void)
 
 	pci_fixup_irqs(pci_common_swizzle, pci_puv3_map_irq);
 
-	if (!pci_has_flag(PCI_PROBE_ONLY)) {
-		pci_bus_size_bridges(puv3_bus);
-		pci_bus_assign_resources(puv3_bus);
-	}
+	pci_bus_size_bridges(puv3_bus);
+	pci_bus_assign_resources(puv3_bus);
 	pci_bus_add_devices(puv3_bus);
 	return 0;
 }
@@ -279,9 +277,6 @@ char * __init pcibios_setup(char *str)
 	if (!strcmp(str, "debug")) {
 		debug_pci = 1;
 		return NULL;
-	} else if (!strcmp(str, "firmware")) {
-		pci_add_flags(PCI_PROBE_ONLY);
-		return NULL;
 	}
 	return str;
 }

[PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Bjorn Helgaas]

On Thu, Jun 23, 2016 at 06:39:03PM +0800, Xuetao Guan wrote:
> > [+cc Guan]
> >
> > On Wed, Jun 08, 2016 at 12:04:50PM +0100, Lorenzo Pieralisi wrote:
> >> The arm pcibios_enable_device() implementation exists solely
> >> to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
> >> on those systems the PCI resources are currently not claimed (ie
> >> inserted in the PCI resource tree - which means their parent
> >> pointer is not correctly set-up) therefore they can not be enabled
> >> since this would trigger PCI set-ups failures.
> >>
> >> After removing the pci=firmware command line option in:
> >>
> >> commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
> >> command line parameter handling")
> >
> > 903589ca7165 removed "pci=firmware" from ARM and from
> > Documentation/kernel-parameters.txt (where it was documented as
> > ARM-specific).
> >
> > I think unicore32 copied that code from arm, so unicore32 is one of
> > the few remaining arches that can set PCI_PROBE_ONLY.  I suspect this
> > was just copied from arm and may not be necessary on unicore32.
> >
> > Guan, can you confirm that PCI_PROBE_ONLY is necessary on unicore32,
> > or that it can be removed?
> >
> 
> Yes, this code was copied from arm, and it's not necessary.
> It can be removed safely.

Great, thank you!  I'll add the patch below to my pci/resource branch for
v4.8.  Let me know if you see any issues with it.


commit b9b8a53e24649e48c0a8bceb8d9d8fe1cfee018c
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Thu Jun 23 11:33:24 2016 -0500

    unicore32/PCI: Remove pci=firmware command line parameter handling
    
    Remove support for the "pci=firmware" command line parameter, which was
    way to keep the kernel from changing any PCI BAR assignments.  This was
    copied from ARM, but is not actually needed on unicore32.
    
    The corresponding ARM support was removed by 903589ca7165 ("ARM: 8554/1:
    kernel: pci: remove pci=firmware command line parameter handling").
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
index d45fa5f..62137d1 100644
--- a/arch/unicore32/kernel/pci.c
+++ b/arch/unicore32/kernel/pci.c
@@ -265,10 +265,8 @@ static int __init pci_common_init(void)
 
 	pci_fixup_irqs(pci_common_swizzle, pci_puv3_map_irq);
 
-	if (!pci_has_flag(PCI_PROBE_ONLY)) {
-		pci_bus_size_bridges(puv3_bus);
-		pci_bus_assign_resources(puv3_bus);
-	}
+	pci_bus_size_bridges(puv3_bus);
+	pci_bus_assign_resources(puv3_bus);
 	pci_bus_add_devices(puv3_bus);
 	return 0;
 }
@@ -279,9 +277,6 @@ char * __init pcibios_setup(char *str)
 	if (!strcmp(str, "debug")) {
 		debug_pci = 1;
 		return NULL;
-	} else if (!strcmp(str, "firmware")) {
-		pci_add_flags(PCI_PROBE_ONLY);
-		return NULL;
 	}
 	return str;
 }

Re: [PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Xuetao Guan]

> On Thu, Jun 23, 2016 at 06:39:03PM +0800, Xuetao Guan wrote:
>> > [+cc Guan]
>> >
>> > On Wed, Jun 08, 2016 at 12:04:50PM +0100, Lorenzo Pieralisi wrote:
>> >> The arm pcibios_enable_device() implementation exists solely
>> >> to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
>> >> on those systems the PCI resources are currently not claimed (ie
>> >> inserted in the PCI resource tree - which means their parent
>> >> pointer is not correctly set-up) therefore they can not be enabled
>> >> since this would trigger PCI set-ups failures.
>> >>
>> >> After removing the pci=firmware command line option in:
>> >>
>> >> commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
>> >> command line parameter handling")
>> >
>> > 903589ca7165 removed "pci=firmware" from ARM and from
>> > Documentation/kernel-parameters.txt (where it was documented as
>> > ARM-specific).
>> >
>> > I think unicore32 copied that code from arm, so unicore32 is one of
>> > the few remaining arches that can set PCI_PROBE_ONLY.  I suspect this
>> > was just copied from arm and may not be necessary on unicore32.
>> >
>> > Guan, can you confirm that PCI_PROBE_ONLY is necessary on unicore32,
>> > or that it can be removed?
>> >
>>
>> Yes, this code was copied from arm, and it's not necessary.
>> It can be removed safely.
>
> Great, thank you!  I'll add the patch below to my pci/resource branch for
> v4.8.  Let me know if you see any issues with it.
>
>
> commit b9b8a53e24649e48c0a8bceb8d9d8fe1cfee018c
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date:   Thu Jun 23 11:33:24 2016 -0500
>
>     unicore32/PCI: Remove pci=firmware command line parameter handling
>
>     Remove support for the "pci=firmware" command line parameter, which
> was
>     way to keep the kernel from changing any PCI BAR assignments.  This
> was
>     copied from ARM, but is not actually needed on unicore32.
>
>     The corresponding ARM support was removed by 903589ca7165 ("ARM:
> 8554/1:
>     kernel: pci: remove pci=firmware command line parameter handling").
>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Sorry for late.

Please add my ack,
Acked-by: GUAN Xuetao <gxt@mprc.pku.edu.cn>

>
> diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
> index d45fa5f..62137d1 100644
> --- a/arch/unicore32/kernel/pci.c
> +++ b/arch/unicore32/kernel/pci.c
> @@ -265,10 +265,8 @@ static int __init pci_common_init(void)
>
>  	pci_fixup_irqs(pci_common_swizzle, pci_puv3_map_irq);
>
> -	if (!pci_has_flag(PCI_PROBE_ONLY)) {
> -		pci_bus_size_bridges(puv3_bus);
> -		pci_bus_assign_resources(puv3_bus);
> -	}
> +	pci_bus_size_bridges(puv3_bus);
> +	pci_bus_assign_resources(puv3_bus);
>  	pci_bus_add_devices(puv3_bus);
>  	return 0;
>  }
> @@ -279,9 +277,6 @@ char * __init pcibios_setup(char *str)
>  	if (!strcmp(str, "debug")) {
>  		debug_pci = 1;
>  		return NULL;
> -	} else if (!strcmp(str, "firmware")) {
> -		pci_add_flags(PCI_PROBE_ONLY);
> -		return NULL;
>  	}
>  	return str;
>  }
>

[PATCH v3 4/4] ARM/PCI: remove arch specific pcibios_enable_device()

[Xuetao Guan]

> On Thu, Jun 23, 2016 at 06:39:03PM +0800, Xuetao Guan wrote:
>> > [+cc Guan]
>> >
>> > On Wed, Jun 08, 2016 at 12:04:50PM +0100, Lorenzo Pieralisi wrote:
>> >> The arm pcibios_enable_device() implementation exists solely
>> >> to prevent enabling PCI resources on PCI_PROBE_ONLY systems, since
>> >> on those systems the PCI resources are currently not claimed (ie
>> >> inserted in the PCI resource tree - which means their parent
>> >> pointer is not correctly set-up) therefore they can not be enabled
>> >> since this would trigger PCI set-ups failures.
>> >>
>> >> After removing the pci=firmware command line option in:
>> >>
>> >> commit 903589ca7165 ("ARM: 8554/1: kernel: pci: remove pci=firmware
>> >> command line parameter handling")
>> >
>> > 903589ca7165 removed "pci=firmware" from ARM and from
>> > Documentation/kernel-parameters.txt (where it was documented as
>> > ARM-specific).
>> >
>> > I think unicore32 copied that code from arm, so unicore32 is one of
>> > the few remaining arches that can set PCI_PROBE_ONLY.  I suspect this
>> > was just copied from arm and may not be necessary on unicore32.
>> >
>> > Guan, can you confirm that PCI_PROBE_ONLY is necessary on unicore32,
>> > or that it can be removed?
>> >
>>
>> Yes, this code was copied from arm, and it's not necessary.
>> It can be removed safely.
>
> Great, thank you!  I'll add the patch below to my pci/resource branch for
> v4.8.  Let me know if you see any issues with it.
>
>
> commit b9b8a53e24649e48c0a8bceb8d9d8fe1cfee018c
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date:   Thu Jun 23 11:33:24 2016 -0500
>
>     unicore32/PCI: Remove pci=firmware command line parameter handling
>
>     Remove support for the "pci=firmware" command line parameter, which
> was
>     way to keep the kernel from changing any PCI BAR assignments.  This
> was
>     copied from ARM, but is not actually needed on unicore32.
>
>     The corresponding ARM support was removed by 903589ca7165 ("ARM:
> 8554/1:
>     kernel: pci: remove pci=firmware command line parameter handling").
>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Sorry for late.

Please add my ack,
Acked-by: GUAN Xuetao <gxt@mprc.pku.edu.cn>

>
> diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
> index d45fa5f..62137d1 100644
> --- a/arch/unicore32/kernel/pci.c
> +++ b/arch/unicore32/kernel/pci.c
> @@ -265,10 +265,8 @@ static int __init pci_common_init(void)
>
>  	pci_fixup_irqs(pci_common_swizzle, pci_puv3_map_irq);
>
> -	if (!pci_has_flag(PCI_PROBE_ONLY)) {
> -		pci_bus_size_bridges(puv3_bus);
> -		pci_bus_assign_resources(puv3_bus);
> -	}
> +	pci_bus_size_bridges(puv3_bus);
> +	pci_bus_assign_resources(puv3_bus);
>  	pci_bus_add_devices(puv3_bus);
>  	return 0;
>  }
> @@ -279,9 +277,6 @@ char * __init pcibios_setup(char *str)
>  	if (!strcmp(str, "debug")) {
>  		debug_pci = 1;
>  		return NULL;
> -	} else if (!strcmp(str, "firmware")) {
> -		pci_add_flags(PCI_PROBE_ONLY);
> -		return NULL;
>  	}
>  	return str;
>  }
>

Re: [PATCH v3 0/4] ARM/ARM64: PCI: PCI_PROBE_ONLY clean-up

[Bjorn Helgaas]

On Wed, Jun 08, 2016 at 12:04:46PM +0100, Lorenzo Pieralisi wrote:
> Current arm/arm64 code prevents enabling resources in the respective
> pcibios_enable_device() callbacks if the PCI_PROBE_ONLY flag is set,
> in that on those platforms the resources tree was not validated properly
> since PCI resources are not currently claimed on arm/arm64 PCI_PROBE_ONLY
> systems.
> 
> This is a temporary kludge, in that PCI_PROBE_ONLY flag is used to
> describe systems with fixed resources, that can nonetheless be enabled
> through the standard pci_enable_resources() call present in PCI core code,
> (ie via the generic pcibios_enable_device() call).
> 
> To remove the PCI_PROBE_ONLY flag in arm/arm64 pcibios_enable_device()
> callbacks, the PCI host controllers that can be used with
> PCI_PROBE_ONLY configurations must make sure that devices resources are
> validated and inserted in the kernel resource tree even on PCI_PROBE_ONLY
> systems so that the generic pcibios_enable_device() generic code, while
> enabling resources (pci_enable_resources()), does not find dangling
> resources pointers (ie missing parent pointers) that are omens of
> an incomplete resource tree, causing failures in resources enablement.
> 
> PCI core code provides interfaces to assign/reassign/reallocate PCI
> bus resources but it is currently lacking an interface to claim
> resources for a specific bus. Arches implement resources claiming through
> ad-hoc code built on top of pci_claim_resource() API, but that code
> cannot be leveraged on architectures like arm/arm64 that rely on the
> generic PCI infrastructure to carry out resources claiming/assignment.
> 
> Therefore, to clean up the arm/arm64 resources enablement on
> PCI_PROBE_ONLY systems this patchset implements four patches:
> 
> PATCH 1: Create PCI core code infrastructure to claim bus resources
> PATCH 2: Leverage the infrastructure in PATCH 1 to claim resources in
>          the PCI generic host controller on PCI_PROBE_ONLY systems
> 
> PATCH 3: Remove the PCI_PROBE_ONLY kludge from the arm64 PCI back-end to
>          complete the clean-up and leverage the PCI generic
>          pcibios_enable_device() implementation
> 
> PATCH 4: Remove the PCI_PROBE_ONLY kludge from the arm PCI back-end to
>          complete the clean-up and leverage the PCI generic
>          pcibios_enable_device() implementation
> 
> Tested on arm/arm64 systems with kvmtool and PCI host generic.
> 
> v2 -> v3
> 
> - Split patch 3 in two patches so that arm and arm64 are handled
>   in different patches
> - Improved commits logs, clarified resource parent assignment
> - Rebased against v4.7-rc2
> 
> v2: https://patchwork.ozlabs.org/patch/590556/
> 
> v1 -> v2
> 
> - Rewrote patch 1 to recursively claim resources for the whole PCI bus
>   hierarchy
> - Updated commits logs/tags
> - Rebased against v4.5-rc6
> 
> v1: https://patchwork.ozlabs.org/patch/545669/
> 
> Lorenzo Pieralisi (4):
>   PCI: add generic code to claim bus resources
>   PCI: host-generic: claim bus resources on PCI_PROBE_ONLY set-ups
>   ARM64/PCI: remove arch specific pcibios_enable_device()
>   ARM/PCI: remove arch specific pcibios_enable_device()
> 
>  arch/arm/kernel/bios32.c           | 12 -------
>  arch/arm64/kernel/pci.c            | 13 --------
>  drivers/pci/host/pci-host-common.c | 27 +++++++++++++--
>  drivers/pci/setup-bus.c            | 68 ++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h                |  1 +
>  5 files changed, 94 insertions(+), 27 deletions(-)

I applied these to pci/resource for v4.8, thanks, Lorenzo!

It's awesome to get rid of that PCI_PROBE_ONLY hack.

We might still tweak this a little based on my question about
pci_common_init_dev(), so don't build things on top of this yet (good
advice in general for my branches: I often rebase and remerge them, so
it's best to base things on my "master" branch unless you actually
depend on something that's not in there.)

[PATCH v3 0/4] ARM/ARM64: PCI: PCI_PROBE_ONLY clean-up

[Bjorn Helgaas]

On Wed, Jun 08, 2016 at 12:04:46PM +0100, Lorenzo Pieralisi wrote:
> Current arm/arm64 code prevents enabling resources in the respective
> pcibios_enable_device() callbacks if the PCI_PROBE_ONLY flag is set,
> in that on those platforms the resources tree was not validated properly
> since PCI resources are not currently claimed on arm/arm64 PCI_PROBE_ONLY
> systems.
> 
> This is a temporary kludge, in that PCI_PROBE_ONLY flag is used to
> describe systems with fixed resources, that can nonetheless be enabled
> through the standard pci_enable_resources() call present in PCI core code,
> (ie via the generic pcibios_enable_device() call).
> 
> To remove the PCI_PROBE_ONLY flag in arm/arm64 pcibios_enable_device()
> callbacks, the PCI host controllers that can be used with
> PCI_PROBE_ONLY configurations must make sure that devices resources are
> validated and inserted in the kernel resource tree even on PCI_PROBE_ONLY
> systems so that the generic pcibios_enable_device() generic code, while
> enabling resources (pci_enable_resources()), does not find dangling
> resources pointers (ie missing parent pointers) that are omens of
> an incomplete resource tree, causing failures in resources enablement.
> 
> PCI core code provides interfaces to assign/reassign/reallocate PCI
> bus resources but it is currently lacking an interface to claim
> resources for a specific bus. Arches implement resources claiming through
> ad-hoc code built on top of pci_claim_resource() API, but that code
> cannot be leveraged on architectures like arm/arm64 that rely on the
> generic PCI infrastructure to carry out resources claiming/assignment.
> 
> Therefore, to clean up the arm/arm64 resources enablement on
> PCI_PROBE_ONLY systems this patchset implements four patches:
> 
> PATCH 1: Create PCI core code infrastructure to claim bus resources
> PATCH 2: Leverage the infrastructure in PATCH 1 to claim resources in
>          the PCI generic host controller on PCI_PROBE_ONLY systems
> 
> PATCH 3: Remove the PCI_PROBE_ONLY kludge from the arm64 PCI back-end to
>          complete the clean-up and leverage the PCI generic
>          pcibios_enable_device() implementation
> 
> PATCH 4: Remove the PCI_PROBE_ONLY kludge from the arm PCI back-end to
>          complete the clean-up and leverage the PCI generic
>          pcibios_enable_device() implementation
> 
> Tested on arm/arm64 systems with kvmtool and PCI host generic.
> 
> v2 -> v3
> 
> - Split patch 3 in two patches so that arm and arm64 are handled
>   in different patches
> - Improved commits logs, clarified resource parent assignment
> - Rebased against v4.7-rc2
> 
> v2: https://patchwork.ozlabs.org/patch/590556/
> 
> v1 -> v2
> 
> - Rewrote patch 1 to recursively claim resources for the whole PCI bus
>   hierarchy
> - Updated commits logs/tags
> - Rebased against v4.5-rc6
> 
> v1: https://patchwork.ozlabs.org/patch/545669/
> 
> Lorenzo Pieralisi (4):
>   PCI: add generic code to claim bus resources
>   PCI: host-generic: claim bus resources on PCI_PROBE_ONLY set-ups
>   ARM64/PCI: remove arch specific pcibios_enable_device()
>   ARM/PCI: remove arch specific pcibios_enable_device()
> 
>  arch/arm/kernel/bios32.c           | 12 -------
>  arch/arm64/kernel/pci.c            | 13 --------
>  drivers/pci/host/pci-host-common.c | 27 +++++++++++++--
>  drivers/pci/setup-bus.c            | 68 ++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h                |  1 +
>  5 files changed, 94 insertions(+), 27 deletions(-)

I applied these to pci/resource for v4.8, thanks, Lorenzo!

It's awesome to get rid of that PCI_PROBE_ONLY hack.

We might still tweak this a little based on my question about
pci_common_init_dev(), so don't build things on top of this yet (good
advice in general for my branches: I often rebase and remerge them, so
it's best to base things on my "master" branch unless you actually
depend on something that's not in there.)