PCI: generic: remove dependency on hw_pci

Enable pci-host-generic driver available in arm64. This needs drivers/pci/setup-irq.o as well.

Signed-off-by Jayachandran C <[email protected]>

---
 drivers/pci/Makefile     | 1 +
 drivers/pci/host/Kconfig | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

drivers/pci/Makefile +1 -0

index 73e4af4..be3f631 100644
@@ -33,6 +33,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
 #
 obj-$(CONFIG_ALPHA) += setup-irq.o
 obj-$(CONFIG_ARM) += setup-irq.o
+obj-$(CONFIG_ARM64) += setup-irq.o
 obj-$(CONFIG_UNICORE32) += setup-irq.o
 obj-$(CONFIG_SUPERH) += setup-irq.o
 obj-$(CONFIG_MIPS) += setup-irq.o

drivers/pci/host/Kconfig +1 -1

index c132bdd..675c2d1 100644
@@ -53,7 +53,7 @@ config PCI_RCAR_GEN2_PCIE
 
 config PCI_HOST_GENERIC
 	bool "Generic PCI host controller"
-	depends on ARM && OF
+	depends on (ARM || ARM64) && OF
 	help
 	  Say Y here if you want to support a simple generic PCI host
 	  controller, such as the one emulated by kvmtool.

Signature · 1 line

1.9.1

The current code in pci-host-generic.c uses pci_common_init_dev() from ARM platform to do some of the PCI initializations, and this prevents it from being used in ARM64.

The initialization done by pci_common_init_dev() that is needed by pci-host-generic.c is really limited, and can be done easily in the same file without using hw_pci API. The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this can be handled by setting up gen_pci to have a pci_sys_data variable as the first element.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v3 of the patchset.

v2-v3
- rebase to 4.2-rc
- fix PCI_PROBE_ONLY check before calling pcie configure
- added a comment above sysdata
- updated the commit message

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

Notes:
- passing a zeroed out pci_sys_data for ARM looks ok, but I haven't tested it on ARM.
- tested it on ARM64 fast model
- Any information on how this can be tested on arm is welcome.
- There is only one ifdef, and that can be removed when arm64 gets a sysdata, or when arm loses its sysdata.

 drivers/pci/host/pci-host-generic.c | 55 ++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 19 deletions(-)

drivers/pci/host/pci-host-generic.c +36 -19

index ba46e58..e9d9c80 100644
@@ -38,7 +38,15 @@ struct gen_pci_cfg_windows {
 	const struct gen_pci_cfg_bus_ops	*ops;
 };
 
+/*
+ * ARM needs platform specific pci_sys_data as the sysdata for PCI.
+ * We add the sys as the first field below to handle this. sys will
+ * set to 0, so that the pci functions in do the right thing.
+ */
 struct gen_pci {
+#ifdef CONFIG_ARM
+	struct pci_sys_data			sys;
+#endif
 	struct pci_host_bridge			host;
 	struct gen_pci_cfg_windows		cfg;
 	struct list_head			resources;

@@ -48,8 +56,7 @@ static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
 					     unsigned int devfn,
 					     int where)
 {
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = bus->sysdata;
 	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
 
 	return pci->cfg.win[idx] + ((devfn << 8) | where);

@@ -64,8 +71,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus,
 					      unsigned int devfn,
 					      int where)
 {
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = bus->sysdata;
 	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
 
 	return pci->cfg.win[idx] + ((devfn << 12) | where);

@@ -198,13 +204,6 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
 	return 0;
 }
 
-static int gen_pci_setup(int nr, struct pci_sys_data *sys)
-{
-	struct gen_pci *pci = sys->private_data;
-	list_splice_init(&pci->resources, &sys->resources);
-	return 1;
-}
-
 static int gen_pci_probe(struct platform_device *pdev)
 {
 	int err;

@@ -214,13 +213,7 @@ static int gen_pci_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
-	struct hw_pci hw = {
-		.nr_controllers	= 1,
-		.private_data	= (void **)&pci,
-		.setup		= gen_pci_setup,
-		.map_irq	= of_irq_parse_and_map_pci,
-		.ops		= &gen_pci_ops,
-	};
+	struct pci_bus *bus;
 
 	if (!pci)
 		return -ENOMEM;

@@ -258,7 +251,31 @@ static int gen_pci_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	pci_common_init_dev(dev, &hw);
+	/* do not reassign resource if probe only */
+	if (!pci_has_flag(PCI_PROBE_ONLY))
+		pci_add_flags(PCI_REASSIGN_ALL_RSRC);
+
+	bus = pci_scan_root_bus(dev, 0, &gen_pci_ops, pci, &pci->resources);
+	if (!bus) {
+		dev_err(dev, "Scanning rootbus failed");
+		return -ENODEV;
+	}
+
+	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+
+	if (!pci_has_flag(PCI_PROBE_ONLY)) {
+		pci_bus_size_bridges(bus);
+		pci_bus_assign_resources(bus);
+	}
+	pci_bus_add_devices(bus);
+
+	/* Configure PCI Express settings */
+	if (!pci_has_flag(PCI_PROBE_ONLY)) {
+		struct pci_bus *child;
+
+		list_for_each_entry(child, &bus->children, node)
+			pcie_bus_configure_settings(child);
+	}
 	return 0;
 }
 

Signature · 6 lines

1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 10 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from ARM platform to do some of the PCI initializations, and this prevents it from being used in ARM64.

The initialization done by pci_common_init_dev() that is needed by pci-host-generic.c is really limited, and can be done easily in the same file without using hw_pci API. The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this can be handled by setting up gen_pci to have a pci_sys_data variable as the first element.

Ok, I still do not like leaving pci_sys_data there, and to remove it there is only one snag (+ one patch to prevent resources enablement on ARM64 PROBE_ONLY systems to have a consolidated ARM/ARM64 host generic), which consists in removing the align_resource pointer from pci_sys_data. A proposed solution (maybe not ideal, we can consider it a temporary step to make progress) here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/359273.html

Comments very appreciated on the patch above, I do not have major concerns with this patch other than the pci_sys_data hack, because that's what it is.

Quoted 22 lines · Lorenzo Pieralisi

Quoted 10 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from ARM platform to do some of the PCI initializations, and this prevents it from being used in ARM64.

The initialization done by pci_common_init_dev() that is needed by pci-host-generic.c is really limited, and can be done easily in the same file without using hw_pci API. The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this can be handled by setting up gen_pci to have a pci_sys_data variable as the first element.

Ok, I still do not like leaving pci_sys_data there, and to remove it there is only one snag (+ one patch to prevent resources enablement on ARM64 PROBE_ONLY systems to have a consolidated ARM/ARM64 host generic), which consists in removing the align_resource pointer from pci_sys_data. A proposed solution (maybe not ideal, we can consider it a temporary step to make progress) here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/359273.html

Quoted 3 lines

Comments very appreciated on the patch above, I do not have major concerns with this patch other than the pci_sys_data hack, because that's what it is.

In may opinion, we have to leave ->sysdata on ARM pointing to a valid pci_sys_data. Leaving it pointing to random stuff and hoping that it will not be used is pretty bad - so I don't see why such a patch would make it upstream.

The second issue I have is adding dependencies between patches that enable basic functionality on arm64 with arm infrastructure cleanup patches. Given the number of kernel releases the cleanups take [and factoring in the amount of mess arm has :)], we will end up waiting a long time for arm64.

BTW, I am not sure why there is a pushback on wrapping a structure in another, container_of() is the linux way of doing things :)

The current code in pci-host-generic.c uses pci_common_init_dev() from ARM platform to do some of the PCI initializations, and this prevents it from being used in ARM64.

The initialization done by pci_common_init_dev() that is needed by pci-host-generic.c is really limited, and can be done easily in the same file without using hw_pci API. The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this can be handled by setting up gen_pci to have a pci_sys_data variable as the first element.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v3 of the patchset.

v2-v3
- rebase to 4.2-rc
- fix PCI_PROBE_ONLY check before calling pcie configure
- added a comment above sysdata
- updated the commit message

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

Notes:
- passing a zeroed out pci_sys_data for ARM looks ok, but I haven't tested it on ARM.
- tested it on ARM64 fast model
- Any information on how this can be tested on arm is welcome.
- There is only one ifdef, and that can be removed when arm64 gets a sysdata, or when arm loses its sysdata.

 drivers/pci/host/pci-host-generic.c | 55 ++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 19 deletions(-)

drivers/pci/host/pci-host-generic.c +11 -18

index ba46e58..e9d9c80 100644
@@ -38,7 +38,15 @@ struct gen_pci_cfg_windows {
 	const struct gen_pci_cfg_bus_ops	*ops;
 };
 
+/*
+ * ARM needs platform specific pci_sys_data as the sysdata for PCI.
+ * We add the sys as the first field below to handle this. sys will
+ * set to 0, so that the pci functions in do the right thing.
+ */
 struct gen_pci {
+#ifdef CONFIG_ARM
+	struct pci_sys_data			sys;
+#endif
 	struct pci_host_bridge			host;
 	struct gen_pci_cfg_windows		cfg;
 	struct list_head			resources;

@@ -48,8 +56,7 @@ static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
 					     unsigned int devfn,
 					     int where)
 {
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = bus->sysdata;
 	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
 
 	return pci->cfg.win[idx] + ((devfn << 8) | where);

@@ -64,8 +71,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus,
 					      unsigned int devfn,
 					      int where)
 {
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = bus->sysdata;
 	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
 
 	return pci->cfg.win[idx] + ((devfn << 12) | where);

@@ -198,13 +204,6 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
 	return 0;
 }
 
-static int gen_pci_setup(int nr, struct pci_sys_data *sys)
-{
-	struct gen_pci *pci = sys->private_data;
-	list_splice_init(&pci->resources, &sys->resources);
-	return 1;
-}
-
 static int gen_pci_probe(struct platform_device *pdev)
 {
 	int err;

@@ -214,13 +213,7 @@ static int gen_pci_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
-	struct hw_pci hw = {
-		.nr_controllers	= 1,
-		.private_data	= (void **)&pci,
-		.setup		= gen_pci_setup,
-		.map_irq	= of_irq_parse_and_map_pci,
-		.ops		= &gen_pci_ops,
-	};
+	struct pci_bus *bus;
 
 	if (!pci)
 		return -ENOMEM;

drivers/pci/host/pci-host-generic.c +22 -0

+
+	bus = pci_scan_root_bus(dev, 0, &gen_pci_ops, pci, &pci->resources);
+	if (!bus) {
+		dev_err(dev, "Scanning rootbus failed");
+		return -ENODEV;
+	}
+
+	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+
+	if (!pci_has_flag(PCI_PROBE_ONLY)) {
+		pci_bus_size_bridges(bus);
+		pci_bus_assign_resources(bus);
+	}
+	pci_bus_add_devices(bus);
+
+	/* Configure PCI Express settings */
+	if (!pci_has_flag(PCI_PROBE_ONLY)) {
+		struct pci_bus *child;
+
+		list_for_each_entry(child, &bus->children, node)
+			pcie_bus_configure_settings(child);
+	}
 	return 0;
 }
 

Signature · 6 lines

1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 10 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from ARM platform to do some of the PCI initializations, and this prevents it from being used in ARM64.

The initialization done by pci_common_init_dev() that is needed by pci-host-generic.c is really limited, and can be done easily in the same file without using hw_pci API. The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this can be handled by setting up gen_pci to have a pci_sys_data variable as the first element.

For the sake of enabling it on ARM64, I am okay with getting this merged, keeping in mind that as I already said pci_sys_data has to go, it is a hack and I do not want it to spread to ALL PCI host drivers that have to work on both ARM/ARM64.

Comments and tags below.

Quoted 100 lines

Signed-off-by Jayachandran C <[email protected]>

---
Here's v3 of the patchset.

v2-v3
- rebase to 4.2-rc
- fix PCI_PROBE_ONLY check before calling pcie configure
- added a comment above sysdata
- updated the commit message

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

Notes:
- passing a zeroed out pci_sys_data for ARM looks ok, but I haven't tested it on ARM.
- tested it on ARM64 fast model
- Any information on how this can be tested on arm is welcome.
- There is only one ifdef, and that can be removed when arm64 gets a sysdata, or when arm loses its sysdata.

 drivers/pci/host/pci-host-generic.c | 55 ++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 19 deletions(-)

drivers/pci/host/pci-host-generic.c +11 -18

index ba46e58..e9d9c80 100644
@@ -38,7 +38,15 @@ struct gen_pci_cfg_windows {
 	const struct gen_pci_cfg_bus_ops	*ops;
 };
 
+/*
+ * ARM needs platform specific pci_sys_data as the sysdata for PCI.
+ * We add the sys as the first field below to handle this. sys will
+ * set to 0, so that the pci functions in do the right thing.
+ */
 struct gen_pci {
+#ifdef CONFIG_ARM
+	struct pci_sys_data			sys;
+#endif
 	struct pci_host_bridge			host;
 	struct gen_pci_cfg_windows		cfg;
 	struct list_head			resources;

@@ -48,8 +56,7 @@ static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
 					     unsigned int devfn,
 					     int where)
 {
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = bus->sysdata;
 	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
 
 	return pci->cfg.win[idx] + ((devfn << 8) | where);

@@ -64,8 +71,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus,
 					      unsigned int devfn,
 					      int where)
 {
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = bus->sysdata;
 	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
 
 	return pci->cfg.win[idx] + ((devfn << 12) | where);

@@ -198,13 +204,6 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
 	return 0;
 }
 
-static int gen_pci_setup(int nr, struct pci_sys_data *sys)
-{
-	struct gen_pci *pci = sys->private_data;
-	list_splice_init(&pci->resources, &sys->resources);
-	return 1;
-}
-
 static int gen_pci_probe(struct platform_device *pdev)
 {
 	int err;

@@ -214,13 +213,7 @@ static int gen_pci_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
-	struct hw_pci hw = {
-		.nr_controllers	= 1,
-		.private_data	= (void **)&pci,
-		.setup		= gen_pci_setup,
-		.map_irq	= of_irq_parse_and_map_pci,
-		.ops		= &gen_pci_ops,
-	};
+	struct pci_bus *bus;
 
 	if (!pci)
 		return -ENOMEM;

@@ -258,7 +251,31 @@ static int gen_pci_probe(struct platform_device *pdev)
 		return err;
 	}
 

drivers/pci/host/pci-host-generic.c static int gen_pci_probe(struct platform_device *pdev) · line 256

Quoted 4 lines

drivers/pci/host/pci-host-generic.c +3 -1

-	pci_common_init_dev(dev, &hw);
+	/* do not reassign resource if probe only */
+	if (!pci_has_flag(PCI_PROBE_ONLY))
+		pci_add_flags(PCI_REASSIGN_ALL_RSRC);

You should add PCI_REASSIGN_ALL_BUS here. As I said in patch 2 comment,

please enable setup-irq.c in an explicit commit, re-sequence the series and fold the Kconfig ARM64 enablement in this patch.

Quoted 119 lines · Lorenzo Pieralisi

Quoted 10 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from ARM platform to do some of the PCI initializations, and this prevents it from being used in ARM64.

The initialization done by pci_common_init_dev() that is needed by pci-host-generic.c is really limited, and can be done easily in the same file without using hw_pci API. The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this can be handled by setting up gen_pci to have a pci_sys_data variable as the first element.

For the sake of enabling it on ARM64, I am okay with getting this merged, keeping in mind that as I already said pci_sys_data has to go, it is a hack and I do not want it to spread to ALL PCI host drivers that have to work on both ARM/ARM64.

Comments and tags below.

Quoted 100 lines

Signed-off-by Jayachandran C <[email protected]>

---
Here's v3 of the patchset.

v2-v3
- rebase to 4.2-rc
- fix PCI_PROBE_ONLY check before calling pcie configure
- added a comment above sysdata
- updated the commit message

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

Notes:
- passing a zeroed out pci_sys_data for ARM looks ok, but I haven't tested it on ARM.
- tested it on ARM64 fast model
- Any information on how this can be tested on arm is welcome.
- There is only one ifdef, and that can be removed when arm64 gets a sysdata, or when arm loses its sysdata.

 drivers/pci/host/pci-host-generic.c | 55 ++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 19 deletions(-)

drivers/pci/host/pci-host-generic.c +11 -18

index ba46e58..e9d9c80 100644
@@ -38,7 +38,15 @@ struct gen_pci_cfg_windows {
 	const struct gen_pci_cfg_bus_ops	*ops;
 };
 
+/*
+ * ARM needs platform specific pci_sys_data as the sysdata for PCI.
+ * We add the sys as the first field below to handle this. sys will
+ * set to 0, so that the pci functions in do the right thing.
+ */
 struct gen_pci {
+#ifdef CONFIG_ARM
+	struct pci_sys_data			sys;
+#endif
 	struct pci_host_bridge			host;
 	struct gen_pci_cfg_windows		cfg;
 	struct list_head			resources;

@@ -48,8 +56,7 @@ static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
 					     unsigned int devfn,
 					     int where)
 {
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = bus->sysdata;
 	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
 
 	return pci->cfg.win[idx] + ((devfn << 8) | where);

@@ -64,8 +71,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus,
 					      unsigned int devfn,
 					      int where)
 {
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = bus->sysdata;
 	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
 
 	return pci->cfg.win[idx] + ((devfn << 12) | where);

@@ -198,13 +204,6 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
 	return 0;
 }
 
-static int gen_pci_setup(int nr, struct pci_sys_data *sys)
-{
-	struct gen_pci *pci = sys->private_data;
-	list_splice_init(&pci->resources, &sys->resources);
-	return 1;
-}
-
 static int gen_pci_probe(struct platform_device *pdev)
 {
 	int err;

@@ -214,13 +213,7 @@ static int gen_pci_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
-	struct hw_pci hw = {
-		.nr_controllers	= 1,
-		.private_data	= (void **)&pci,
-		.setup		= gen_pci_setup,
-		.map_irq	= of_irq_parse_and_map_pci,
-		.ops		= &gen_pci_ops,
-	};
+	struct pci_bus *bus;
 
 	if (!pci)
 		return -ENOMEM;

@@ -258,7 +251,31 @@ static int gen_pci_probe(struct platform_device *pdev)
 		return err;
 	}
 

Quoted 6 lines

drivers/pci/host/pci-host-generic.c static int gen_pci_probe(struct platform_device *pdev) · line 256

Quoted 4 lines

drivers/pci/host/pci-host-generic.c +3 -1

-	pci_common_init_dev(dev, &hw);
+	/* do not reassign resource if probe only */
+	if (!pci_has_flag(PCI_PROBE_ONLY))
+		pci_add_flags(PCI_REASSIGN_ALL_RSRC);

You should add PCI_REASSIGN_ALL_BUS here. As I said in patch 2 comment,

The idea is to do exactly what pci_common_init_dev does on ARM. There is not need to change behavior on ARM and deal with the fallout in this patchset.

Quoted 2 lines

please enable setup-irq.c in an explicit commit, re-sequence the series and fold the Kconfig ARM64 enablement in this patch.

I would think that the Makefile and Kconfig changes should be togethter as the arm64 enablement patch. The previous patch is to update the gen_pci driver, and it really makes sense in that order. I am not sure why you suggest this change.

Quoted 9 lines · Jayachandran C.

Quoted 7 lines · Lorenzo Pieralisi

drivers/pci/host/pci-host-generic.c static int gen_pci_probe(struct platform_device *pdev) · line 256

Quoted 4 lines · Jayachandran C

drivers/pci/host/pci-host-generic.c +3 -1

-	pci_common_init_dev(dev, &hw);
+	/* do not reassign resource if probe only */
+	if (!pci_has_flag(PCI_PROBE_ONLY))
+		pci_add_flags(PCI_REASSIGN_ALL_RSRC);

You should add PCI_REASSIGN_ALL_BUS here. As I said in patch 2 comment,

Quoted 3 lines

The idea is to do exactly what pci_common_init_dev does on ARM. There is not need to change behavior on ARM and deal with the fallout in this patchset.

You are enabling this driver on ARM64, and on ARM64 that flag does make a difference, it does not on ARM as far as I can tell, but you do not seem to care.

Actually Rob asked you to put together a function since most of this code is replicated across ARM drivers eg:

drivers/pci/host/pci-versatile.c

Leave the code as it is and I will consolidate the code later.

Quoted 7 lines

Quoted 2 lines

please enable setup-irq.c in an explicit commit, re-sequence the series and fold the Kconfig ARM64 enablement in this patch.

I would think that the Makefile and Kconfig changes should be togethter as the arm64 enablement patch. The previous patch is to update the gen_pci driver, and it really makes sense in that order. I am not sure why you suggest this change.

Because this is not the only driver that requires compiling drivers/pci/setup-irq.c on ARM64, I thought it made more sense to do it in a separate patch to make that change standalone.

If for any reason we have to revert your patch that enables PCI host generic on ARM64 (and we remove with it the compilation of setup-irq.c) we would break other ARM64 PCI hosts that require it, right ?

I leave the decision to Bjorn since I am away next week, I am fine with the code as it is so please go ahead.

Hi Jayachandran, Bjorn,

Quoted 37 lines · Lorenzo Pieralisi

Quoted 9 lines · Jayachandran C.

Quoted 7 lines · Lorenzo Pieralisi

drivers/pci/host/pci-host-generic.c static int gen_pci_probe(struct platform_device *pdev) · line 256

Quoted 4 lines · Jayachandran C

drivers/pci/host/pci-host-generic.c +3 -1

-	pci_common_init_dev(dev, &hw);
+	/* do not reassign resource if probe only */
+	if (!pci_has_flag(PCI_PROBE_ONLY))
+		pci_add_flags(PCI_REASSIGN_ALL_RSRC);

You should add PCI_REASSIGN_ALL_BUS here. As I said in patch 2 comment,

Quoted 3 lines

The idea is to do exactly what pci_common_init_dev does on ARM. There is not need to change behavior on ARM and deal with the fallout in this patchset.

You are enabling this driver on ARM64, and on ARM64 that flag does make a difference, it does not on ARM as far as I can tell, but you do not seem to care.

Actually Rob asked you to put together a function since most of this code is replicated across ARM drivers eg:

drivers/pci/host/pci-versatile.c

Leave the code as it is and I will consolidate the code later.

Quoted 7 lines

Quoted 2 lines

please enable setup-irq.c in an explicit commit, re-sequence the series and fold the Kconfig ARM64 enablement in this patch.

I would think that the Makefile and Kconfig changes should be togethter as the arm64 enablement patch. The previous patch is to update the gen_pci driver, and it really makes sense in that order. I am not sure why you suggest this change.

Because this is not the only driver that requires compiling drivers/pci/setup-irq.c on ARM64, I thought it made more sense to do it in a separate patch to make that change standalone.

Quoted 7 lines

If for any reason we have to revert your patch that enables PCI host generic on ARM64 (and we remove with it the compilation of setup-irq.c) we would break other ARM64 PCI hosts that require it, right ?

I leave the decision to Bjorn since I am away next week, I am fine with the code as it is so please go ahead.

In the interest of getting something merged, can I suggest you (Jayachandran) repost the patches with Lorenzo's suggestions to reorder the series and split up the setup-irq compilation into a separate patch?

You can also add his ack to this patch.

It's a pity that the code isn't quite how we'd like it to end up, but this at least enables people for 4.3 and I know that Lorenzo is working on tidying up the loose ends as additional patches (but he's on holiday this week). That includes removal of the PROBE_ONLY bodge that I've got queued in arch/arm64/kernel/pci.c.

Bjorn, does that sound ok to you? I'm keen to have something working, as I'm starting to get beaten up about the lack of generic PCI on arm64.

The current code in pci-host-generic.c uses pci_common_init_dev() from ARM platform to do some of the PCI initializations, and this prevents it from being used in ARM64.

The initialization done by pci_common_init_dev() that is needed by pci-host-generic.c is really limited, and can be done easily in the same file without using hw_pci API. The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this can be handled by setting up gen_pci to have a pci_sys_data variable as the first element.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v3 of the patchset.

v2-v3
- rebase to 4.2-rc
- fix PCI_PROBE_ONLY check before calling pcie configure
- added a comment above sysdata
- updated the commit message

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

Notes:
- passing a zeroed out pci_sys_data for ARM looks ok, but I haven't tested it on ARM.

- tested it on ARM64 fast model
- Any information on how this can be tested on arm is welcome.
- There is only one ifdef, and that can be removed when arm64 gets a sysdata, or when arm loses its sysdata.

drivers/pci/host/pci-host-generic.c +15 -1

@@ -258,7 +251,31 @@ static int gen_pci_probe(struct platform_device *pdev)
                return err;
        }
 
-       pci_common_init_dev(dev, &hw);
+       /* do not reassign resource if probe only */
+       if (!pci_has_flag(PCI_PROBE_ONLY))
+               pci_add_flags(PCI_REASSIGN_ALL_RSRC);
+
+       bus = pci_scan_root_bus(dev, 0, &gen_pci_ops, pci, &pci->resources);
+       if (!bus) {
+               dev_err(dev, "Scanning rootbus failed");
+               return -ENODEV;
+       }
+
+       pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+
+       if (!pci_has_flag(PCI_PROBE_ONLY)) {
+               pci_bus_size_bridges(bus);
+               pci_bus_assign_resources(bus);

Notes:
- passing a zeroed out pci_sys_data for ARM looks ok, but I haven't tested it on ARM.
- tested it on ARM64 fast model
- Any information on how this can be tested on arm is welcome.