Skip to contentSkip to conversation

PCI: generic: remove dependency on hw_pci

PATCH New Jayachandran C 6 participants 14 messages Aug 4, 2015 – Aug 14, 2015 PCI

lore
Share & tools
lore

Jayachandran C

author
#0 lore raw

PATCH v4 1/3 PCI: generic: remove dependency on hw_pci

The current code in pci-host-generic.c uses pci_common_init_dev() from arm architecture to do some part of the PCI setup, and this prevents it from being used with arm64 architecture.

The part of pci_common_init_dev() that is really needed by pci-host-generic.c is very limited and can be done in the same file without using hw_pci API of ARM. 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]>
Acked-by Lorenzo Pieralisi <[email protected]>
Tested-by Lorenzo Pieralisi <[email protected]>
Tested-by Pavel Fedin <[email protected]>

---
Here's v4 of the patchset.
v3-v4
- Changes for comments by Lorenzo Pieralisi/Will Deacon
- add PCI_REASSIGN_ALL_BUS flag if not probe only
- split setup-irq.o into another patch
- minor clean up of commit messages

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..53703e3 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 | PCI_REASSIGN_ALL_BUS);
+
+	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 · 1 line
1.9.1
author
#1 lore raw

PATCH v4 2/3 PCI: Add setup-irq.o when compiling for arm64

ARM64 requires setup-irq.o to provide pci_fixup_irqs() implementation. We are adding this now to support the pci-host-generic host controller, but we enable it for ARM64 PCI so that other host controllers can use this as well.

Signed-off-by Jayachandran C <[email protected]>
---
 drivers/pci/Makefile | 1 +
 1 file changed, 1 insertion(+)
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
Signature · 1 line
1.9.1
author
#2 lore raw

PATCH v4 3/3 PCI: generic: add arm64 support

In reply to Jayachandran C

Make pci-host-generic driver (kernel option PCI_HOST_GENERIC) available on arm64.

Signed-off-by Jayachandran C <[email protected]>
---
 drivers/pci/host/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
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
maintainer
#3 lore raw

PATCH v4 1/3 PCI: generic: remove dependency on hw_pci

In reply to Jayachandran C
Quoted 11 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from arm architecture to do some part of the PCI setup, and this prevents it from being used with arm64 architecture.

The part of pci_common_init_dev() that is really needed by pci-host-generic.c is very limited and can be done in the same file without using hw_pci API of ARM. 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.

Quoted 4 lines
Signed-off-by Jayachandran C <[email protected]>
Acked-by Lorenzo Pieralisi <[email protected]>
Tested-by Lorenzo Pieralisi <[email protected]>
Tested-by Pavel Fedin <[email protected]>

I applied all three of these to my pci/enumeration branch for v4.3.

I made a few changes, the most important being to call pcie_bus_configure_settings() before pci_bus_add_devices(), because drivers may claim devices in pci_bus_add_devices(), and the PCI core shouldn't touch devices after drivers claim them.

I dropped the acks and tested-by because of that change, but I'll happily add them back if Lorenzo and Pavel confirm. The patches are here if you want to review/test them:

https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/enumeration

I'm sure the other patches on that branch will need some tweaks, too, so I don't have any problem with updating these.

Bjorn

#4 lore raw

PATCH v4 1/3 PCI: generic: remove dependency on hw_pci

Quoted 24 lines · Bjorn Helgaas
Quoted 11 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from arm architecture to do some part of the PCI setup, and this prevents it from being used with arm64 architecture.

The part of pci_common_init_dev() that is really needed by pci-host-generic.c is very limited and can be done in the same file without using hw_pci API of ARM. 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.

Quoted 4 lines
Signed-off-by Jayachandran C <[email protected]>
Acked-by Lorenzo Pieralisi <[email protected]>
Tested-by Lorenzo Pieralisi <[email protected]>
Tested-by Pavel Fedin <[email protected]>

I applied all three of these to my pci/enumeration branch for v4.3.

I made a few changes, the most important being to call pcie_bus_configure_settings() before pci_bus_add_devices(), because drivers may claim devices in pci_bus_add_devices(), and the PCI core shouldn't touch devices after drivers claim them.

Quoted 8 lines

I dropped the acks and tested-by because of that change, but I'll happily add them back if Lorenzo and Pavel confirm. The patches are here if you want to review/test them:

https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/enumeration

I'm sure the other patches on that branch will need some tweaks, too, so I don't have any problem with updating these.

Excellent, thanks Bjorn! Marc and I will take these for a spin in conjunction with tip/irq/core and the arm64 queue.

That said, I just tried to build this and it fails to compile the x-gene PCI host driver. Fixup below.

Will

--->8

drivers/pci/host/pci-xgene.c +1 -1
index 514f41b86c49..57ac0c7108a8 100644
@@ -506,7 +506,7 @@ static int xgene_pcie_msi_enable(struct xgene_pcie_port *port)
 {
 	struct device_node *msi_node;
 
-	msi_node = of_parse_phandle(port->dev.of_node, "msi-parent", 0);
+	msi_node = of_parse_phandle(port->dev->of_node, "msi-parent", 0);
 	if (!msi_node)
 		return -ENODEV;
#5 lore raw

PATCH v4 1/3 PCI: generic: remove dependency on hw_pci

Quoted 2 lines · Will Deacon

That said, I just tried to build this and it fails to compile the x-gene PCI host driver. Fixup below.

Wait, there's more!

Will

--->8

drivers/pci/host/pci-xgene.c +2 -2
index 514f41b86c49..8ab3bd098424 100644
@@ -506,7 +506,7 @@ static int xgene_pcie_msi_enable(struct xgene_pcie_port *port)
 {
 	struct device_node *msi_node;
 
-	msi_node = of_parse_phandle(port->dev.of_node, "msi-parent", 0);
+	msi_node = of_parse_phandle(port->dev->of_node, "msi-parent", 0);
 	if (!msi_node)
 		return -ENODEV;
 

@@ -515,7 +515,7 @@ static int xgene_pcie_msi_enable(struct xgene_pcie_port *port)
 		return -ENODEV;
 
 	of_node_put(msi_node);
-	port->msi->dev = &port->dev;
+	port->msi->dev = port->dev;
 	return 0;
 }
maintainer
#6 lore raw

PATCH v4 1/3 PCI: generic: remove dependency on hw_pci

Quoted 5 lines · Will Deacon
Quoted 2 lines · Will Deacon

That said, I just tried to build this and it fails to compile the x-gene PCI host driver. Fixup below.

Wait, there's more!

Thanks! I fixed these two and updated pci/enumeration. But the 0-day build also found this:

commit: cc44fbb49d995ad2348d530300abf7877105094f [4/13] ARM/PCI, designware, xilinx: Use pci_scan_root_bus_msi()
drivers/built-in.o: In function `dw_msi_setup_irq':

Quoted 1 line
Quoted 1 line

:(.text+0x2a3c8): undefined reference to `pci_write_msi_msg'

I don't see the problem there yet, and I'm going on vacation until Monday, so I won't be able to do anything with this until then.

Bjorn

Quoted 23 lines
drivers/pci/host/pci-xgene.c +2 -2
index 514f41b86c49..8ab3bd098424 100644
@@ -506,7 +506,7 @@ static int xgene_pcie_msi_enable(struct xgene_pcie_port *port)
 {
        struct device_node *msi_node;
 
-       msi_node = of_parse_phandle(port->dev.of_node, "msi-parent", 0);
+       msi_node = of_parse_phandle(port->dev->of_node, "msi-parent", 0);
        if (!msi_node)
                return -ENODEV;
 

@@ -515,7 +515,7 @@ static int xgene_pcie_msi_enable(struct xgene_pcie_port *port)
                return -ENODEV;
 
        of_node_put(msi_node);
-       port->msi->dev = &port->dev;
+       port->msi->dev = port->dev;
        return 0;
 }
#7 lore raw

PATCH v4 1/3 PCI: generic: remove dependency on hw_pci

In reply to Will Deacon

Hi Bjorn,

Quoted 25 lines · Will Deacon
Quoted 24 lines · Bjorn Helgaas
Quoted 24 lines · Jayachandran C
Quoted 11 lines
Quoted 11 lines

The current code in pci-host-generic.c uses pci_common_init_dev() from arm architecture to do some part of the PCI setup, and this prevents it from being used with arm64 architecture.

The part of pci_common_init_dev() that is really needed by pci-host-generic.c is very limited and can be done in the same file without using hw_pci API of ARM. 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.

Quoted 4 lines
Quoted 4 lines
Signed-off-by Jayachandran C<[email protected]>
Acked-by Lorenzo Pieralisi<[email protected]>
Tested-by Lorenzo Pieralisi<[email protected]>
Tested-by Pavel Fedin<[email protected]>

I applied all three of these to my pci/enumeration branch for v4.3.

I made a few changes, the most important being to call pcie_bus_configure_settings() before pci_bus_add_devices(), because drivers may claim devices in pci_bus_add_devices(), and the PCI core shouldn't touch devices after drivers claim them.

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

I dropped the acks and tested-by because of that change, but I'll happily add them back if Lorenzo and Pavel confirm. The patches are here if you want to review/test them:

https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/enumeration

I'm sure the other patches on that branch will need some tweaks, too, so I don't have any problem with updating these.

I have also tested on AMD Seattle (w/ non-PROBE_ONLY mode).

Tested-by Suravee Suthikulpanit <[email protected]>
#8 lore raw

PATCH v4 1/3 PCI: generic: remove dependency on hw_pci

In reply to Bjorn Helgaas

Hello!

Quoted 2 lines

I dropped the acks and tested-by because of that change, but I'll happily add them back if Lorenzo and Pavel confirm.

Confirm.

Tested-by Pavel Fedin <[email protected]>

I also added MSI support. Can i post it as patch on top of your branch?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

#9 lore raw

PATCH v4 1/3 PCI: generic: remove dependency on hw_pci

Quoted 1 line · Pavel Fedin

Hello!

Hi Pavel,

Quoted 7 lines
Quoted 2 lines

I dropped the acks and tested-by because of that change, but I'll happily add them back if Lorenzo and Pavel confirm.

Confirm.

Tested-by Pavel Fedin <[email protected]>

I also added MSI support. Can i post it as patch on top of your branch?

There shouldn't be any need with the changes queued in tip/irq/core. Can you give the following branch a go, please (it doesn't look like Bjorn's stuff is in linux-next, unfortunately)?

git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git devel

It's a temporary testing branch (subject to rebasing), but it should be indicative of what's queued for 4.3.

Will

#10 lore raw

PATCH v4 1/3 PCI: generic: remove dependency on hw_pci

Hello!

Quoted 7 lines
Quoted 1 line

I also added MSI support. Can i post it as patch on top of your branch?

There shouldn't be any need with the changes queued in tip/irq/core. Can you give the following branch a go, please (it doesn't look like Bjorn's stuff is in linux-next, unfortunately)?

git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git devel

I have checked, they already implemented generic handling for "msi-parent" property in the device
tree, and MSI works out of the box.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

maintainer
#11 lore raw

PATCH v4 1/3 PCI: generic: remove dependency on hw_pci

In reply to Will Deacon
Quoted 4 lines · Will Deacon
Quoted 3 lines · Pavel Fedin
Quoted 2 lines

I dropped the acks and tested-by because of that change, but I'll happily add them back if Lorenzo and Pavel confirm.

Quoted 8 lines
Quoted 4 lines

Confirm.

Tested-by Pavel Fedin <[email protected]>

I also added MSI support. Can i post it as patch on top of your branch?

There shouldn't be any need with the changes queued in tip/irq/core. Can you give the following branch a go, please (it doesn't look like Bjorn's stuff is in linux-next, unfortunately)?

We still have a build issue (undefined reference to pci_write_msi_msg in imx_v6_v7_defconfig) to resolve before putting it in linux-next. Hopefully we can resolve that today.

#12 lore raw

PATCH v4 1/3 PCI: generic: remove dependency on hw_pci

Hi Bjorn,

Quoted 10 lines · Bjorn Helgaas
Quoted 9 lines · Will Deacon
Quoted 7 lines · Pavel Fedin
Quoted 2 lines

I dropped the acks and tested-by because of that change, but I'll happily add them back if Lorenzo and Pavel confirm.

Confirm.

Tested-by Pavel Fedin <[email protected]>

I also added MSI support. Can i post it as patch on top of your branch?

Quoted 7 lines
Quoted 3 lines

There shouldn't be any need with the changes queued in tip/irq/core. Can you give the following branch a go, please (it doesn't look like Bjorn's stuff is in linux-next, unfortunately)?

We still have a build issue (undefined reference to pci_write_msi_msg in imx_v6_v7_defconfig) to resolve before putting it in linux-next. Hopefully we can resolve that today.

I have sent you a fix for that, please let me know if that's ok with you or you want me to rework it.

Thanks,
Lorenzo

#13 lore raw

PATCH v4 1/3 PCI: generic: remove dependency on hw_pci

In reply to Bjorn Helgaas
Quoted 24 lines · Bjorn Helgaas
Quoted 11 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from arm architecture to do some part of the PCI setup, and this prevents it from being used with arm64 architecture.

The part of pci_common_init_dev() that is really needed by pci-host-generic.c is very limited and can be done in the same file without using hw_pci API of ARM. 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.

Quoted 4 lines
Signed-off-by Jayachandran C <[email protected]>
Acked-by Lorenzo Pieralisi <[email protected]>
Tested-by Lorenzo Pieralisi <[email protected]>
Tested-by Pavel Fedin <[email protected]>

I applied all three of these to my pci/enumeration branch for v4.3.

I made a few changes, the most important being to call pcie_bus_configure_settings() before pci_bus_add_devices(), because drivers may claim devices in pci_bus_add_devices(), and the PCI core shouldn't touch devices after drivers claim them.

Quoted 6 lines

I dropped the acks and tested-by because of that change, but I'll happily add them back if Lorenzo and Pavel confirm. The patches are here if you want to review/test them:

https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/enumeration

I am happy with your changes so you can certainly add my acked-by/tested-by tags back, with thanks.

Thank you !
Lorenzo

Quoted 2 lines

I'm sure the other patches on that branch will need some tweaks, too, so I don't have any problem with updating these.