Skip to contentSkip to conversation

MSI support for Marvell EBU PCIe driver

PATCH Superseded Thomas Petazzoni 3 participants 29 messages Jun 19, 2013 – Jun 25, 2013

Subsystems (13)

v3 of 9 Superseded Latest is v9 →

lore
Share & tools
lore
#0 lore raw

v3 cover MSI support for Marvell EBU PCIe driver

Hello,

This set of patches introduces Message Signaled Interrupt support in the Marvell EBU PCIe driver. It has been successfully tested on the Armada XP GP platform with an Intel e1000e PCIe network card that supports MSI.

This version 3 follows:
* PATCH version 2 sent on June, 6th 2013
* RFC version 1 sent on March, 26th 2013

Changes since v2:

* Add an IRQ domain function that allows to allocate dynamically a free hwirq number from an IRQ domain, and create a virq mapping to it. It was suggested by Grant Likely in order to remove the hwirq allocation code from the IRQ driver itself. (PATCH 1)

* Separate the use of weak functions from the introduction of the msi_chip infrastructure, and use weak functions for all architecture-specific MSI hooks. Suggested by Bjorn Helgaas (PATCH 2).

* Move the msi_chip registry to drivers/of. Suggested by Bjorn Helgaas (PATCH 4).

* Use pcibios_add_bus() and pcibios_remove_bus() to connect the PCI busses to their msi_chip, as suggested by Bjorn Helgaas. Requires some ARM PCI core changes (PATCH 7), and then changes to the PCI driver itself (PATCH 9).

It would be great if some of those patches were taken for 3.11. For example, PATCH 1, PATCH 2, PATCH 5 and PATCH 7 are completely independent from each other.

The patches do the following:

* Patch 1 adds an IRQ domain function that allows to allocate dynamically a free hwirq number from an IRQ domain, and create a virq mapping to it. It was suggested by Grant Likely in order to remove the hwirq allocation code from the IRQ driver itself.

This patch needs the Ack of Grant Likely.

* Patch 2 reworks how the architecture-specific MSI functions can be overriden by architecture-specific code, by using weak functions. It was suggested by Bjorn Helgaas.

This patch needs the Ack from Bjorn Helgaas.

* Patch 3 adds a minimal msi_chip infrastructure, that allows a pci_bus to be connected to a msi_chip, and that provides default implementations of the architecture-specific MSI functions to use msi_chip.

This patch needs the Ack from Bjorn Helgaas.

* Patch 4 adds a small registry of msi_chip <-> of_node in the OF code. It was originally located in drivers/pci/msi.c, but Bjorn Helgaas suggested that it probably belongs to drivers/of/.

This patch needs the Ack from Grant Likely or Rob Herring, and probably Bjorn Helgaas as well.

* Patch 5 makes some not very interesting preparation in the Armada 370/XP IRQ controller driver.

This patch needs the Ack from Thomas Gleixner.

* Patch 6 implements the MSI support in the Armada 370/XP IRQ controller driver. It registers an msi_chip using the msi_chip_add() function added in PATCH 3.

This patch needs the Ack from Thomas Gleixner.

* Patch 7 extends the ARM PCI core to expose ->add_bus() and ->remove_bus() hooks to PCI drivers. This was suggested by Bjorn Helgaas to allow the PCI driver to connected the PCI busses with the corresponding MSI chip.

This patch needs the Ack from Russell King.

* Patch 8 adjust the Armada 370/XP Device Tree to indicate that the MPIC is not only an interrupt-controller, but also an msi-controller.

This patch needs the Ack from Jason Cooper, Gregory Clement and/or Andrew Lunn, the Marvell Maintainers.

* Patch 9 adds MSI support in the Marvell PCIe host controller driver. The work to do here is minimal: get a reference to the msi-parent controller thanks to msi_chip_find_by_of_node(), and link it to the pci_bus structure before the bus gets enumerated.

This patch needs the Ack from Bjorn Helgaas.

* Patch 10 tunes Kconfig to indicate that Armada 370/XP supports MSI.

This patch needs the Ack from Jason Cooper, Gregory Clement and/or Andrew Lunn, the Marvell Maintainers.

* Patch 8 adjusts the Armada 370/XP Device Tree to add the msi-parent properties in the PCIe controller nodes.

This patch needs the Ack from Jason Cooper, Gregory Clement and/or Andrew Lunn, the Marvell Maintainers.

This set of patches currently applies on top of the current "for-next" branch of Jason Cooper's repository + the irqdomain/test branch of Grant Likely's tree.

The Device Tree binding documentation updates will be added once the general approach is agreed on.

Thanks,

Thomas

Thierry Reding (1):
PCI: Introduce new MSI chip infrastructure

Thomas Petazzoni (10):
irqdomain: add irq_alloc_mapping() function
pci: use weak functions for MSI arch-specific functions
of: pci: add registry of MSI chips
irqchip: armada-370-xp: properly request resources
irqchip: armada-370-xp: implement MSI support
arm: pci: add ->add_bus() and ->remove_bus() hooks to hw_pci
arm: mvebu: the MPIC now provides MSI controller features
pci: mvebu: add support for MSI
arm: mvebu: indicate that this platform supports MSI
arm: mvebu: link PCIe controllers to the MSI controller

 arch/arm/boot/dts/armada-370-xp.dtsi     |   1 +
 arch/arm/boot/dts/armada-370.dtsi        |   1 +
 arch/arm/boot/dts/armada-xp-mv78230.dtsi |   1 +
 arch/arm/boot/dts/armada-xp-mv78260.dtsi |   1 +
 arch/arm/boot/dts/armada-xp-mv78460.dtsi |   1 +
 arch/arm/include/asm/mach/pci.h          |   4 +
 arch/arm/kernel/bios32.c                 |  16 ++++
 arch/arm/mach-mvebu/Kconfig              |   1 +
 arch/mips/include/asm/pci.h              |   5 -
 arch/powerpc/include/asm/pci.h           |   5 -
 arch/s390/include/asm/pci.h              |   4 -
 arch/x86/include/asm/pci.h               |  28 ------
 arch/x86/kernel/x86_init.c               |  21 +++++
 drivers/irqchip/irq-armada-370-xp.c      | 155 ++++++++++++++++++++++++++++++-
 drivers/of/of_pci.c                      |  22 +++++
 drivers/pci/host/pci-mvebu.c             |  26 ++++++
 drivers/pci/msi.c                        |  80 +++++++++++-----
 drivers/pci/probe.c                      |   1 +
 include/linux/irqdomain.h                |   2 +
 include/linux/msi.h                      |  28 +++++-
 include/linux/of_pci.h                   |   4 +
 include/linux/pci.h                      |   1 +
 kernel/irq/irqdomain.c                   |  32 +++++++
 23 files changed, 371 insertions(+), 69 deletions(-)
Signature · 1 line
1.8.1.2
#1 lore raw

PATCH v3 1/11 irqdomain: add irq_alloc_mapping() function

This commit extends the irqdomain subsystem with an irq_alloc_mapping() function which allows to let the irqdomain code find an available hwirq number in the range [ 0 ; domain size ] for the given domain, and create a virq mapping for it.

---
 include/linux/irqdomain.h |  2 ++
 kernel/irq/irqdomain.c    | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)
include/linux/irqdomain.h +2 -0
index 02f7658..5f9c3ff 100644
@@ -195,6 +195,8 @@ static inline unsigned int irq_linear_revmap(struct irq_domain *domain,
 extern unsigned int irq_find_mapping(struct irq_domain *host,
 				     irq_hw_number_t hwirq);
 extern unsigned int irq_create_direct_mapping(struct irq_domain *host);
+extern unsigned int irq_alloc_mapping(struct irq_domain *host,
+				      irq_hw_number_t *hwirq);
 extern int irq_create_strict_mappings(struct irq_domain *domain,
 				      unsigned int irq_base,
 				      irq_hw_number_t hwirq_base, int count);
kernel/irq/irqdomain.c +32 -0
index 836a0f7..be82d2e 100644
@@ -375,6 +375,38 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
 EXPORT_SYMBOL_GPL(irq_create_direct_mapping);
 
 /**
+ * irq_alloc_mapping() - Allocate an irq for mapping
+ * @domain: domain to allocate the irq for or NULL for default domain
+ * @hwirq:  reference to the returned hwirq
+ *
+ * This routine are used for irq controllers which can choose the
+ * hardware interrupt number from a range [ 0 ; domain size ], such as
+ * is often the case with PCI MSI controllers. The function will
+ * returned the allocated hwirq number in the hwirq pointer, and the
+ * corresponding virq number as the return value.
+ */
+unsigned int irq_alloc_mapping(struct irq_domain *domain,
+			       irq_hw_number_t *out_hwirq)
+{
+	irq_hw_number_t hwirq;
+
+	pr_debug("irq_alloc_mapping(0x%p)\n", domain);
+
+	for (hwirq = 0; hwirq < domain->hwirq_max; hwirq++)
+		if (!irq_find_mapping(domain, hwirq))
+			break;
+
+	if (hwirq == domain->hwirq_max) {
+		pr_debug("-> no available hwirq found\n");
+		return 0;
+	}
+
+	*out_hwirq = hwirq;
+	return irq_create_mapping(domain, hwirq);
+}
+EXPORT_SYMBOL_GPL(irq_alloc_mapping);
+
+/**
  * irq_create_mapping() - Map a hardware interrupt into linux irq space
  * @domain: domain owning this hardware interrupt or NULL for default domain
  * @hwirq: hardware irq number in that domain space
Signature · 1 line
1.8.1.2
#2 lore raw

PATCH v3 1/11 irqdomain: add irq_alloc_mapping() function

Grant,

Quoted 6 lines · Thomas Petazzoni

This commit extends the irqdomain subsystem with an irq_alloc_mapping() function which allows to let the irqdomain code find an available hwirq number in the range [ 0 ; domain size ] for the given domain, and create a virq mapping for it.

Any comments? Any chance that you could take this for 3.11?

Thanks!

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#3 lore raw

PATCH v3 2/11 pci: use weak functions for MSI arch-specific functions

In reply to Thomas Petazzoni

Until now, the MSI architecture-specific functions could be overloaded using a fairly complex set of #define and compile-time conditionals. In order to prepare for the introduction of the msi_chip infrastructure, it is desirable to switch all those functions to use the 'weak' mechanism. This commit converts all the architecture that were overidding those MSI functions to use the new strategy.

The new strategy consists of defining two functions for each hook:

* One function is called default_<something>() and is not weak. It contains the default implementation of the hook.

* One function is called arch_<something>() and is weak. The default implementation (in drivers/pci/msi.c) simply calls the corresponding default_<something>() function.

The idea is that architecture-specific code can re-implement its own, non-weak, version of arch_<something>(). And to achieve this, it may wish to call back into the default implementation provided by the PCI core, which is possible thanks to the default_<something>() function.

This is needed because the x86 Xen implementation of teardown_msi_irqs() needs to do some work, and then call the default implementation of this hook.

---
 arch/mips/include/asm/pci.h    |  5 ----
 arch/powerpc/include/asm/pci.h |  5 ----
 arch/s390/include/asm/pci.h    |  4 ---
 arch/x86/include/asm/pci.h     | 28 --------------------
 arch/x86/kernel/x86_init.c     | 21 +++++++++++++++
 drivers/pci/msi.c              | 58 +++++++++++++++++++++++++++---------------
 include/linux/msi.h            | 15 ++++++++++-
 7 files changed, 72 insertions(+), 64 deletions(-)
arch/mips/include/asm/pci.h +0 -5
index b8e24fd..031f4c1 100644
@@ -137,11 +137,6 @@ static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
 	return channel ? 15 : 14;
 }
 
-#ifdef CONFIG_CPU_CAVIUM_OCTEON
-/* MSI arch hook for OCTEON */
-#define arch_setup_msi_irqs arch_setup_msi_irqs
-#endif
-
 extern char * (*pcibios_plat_setup)(char *str);
 
 #ifdef CONFIG_OF
arch/powerpc/include/asm/pci.h +0 -5
index 6653f27..95145a1 100644
@@ -113,11 +113,6 @@ extern int pci_domain_nr(struct pci_bus *bus);
 /* Decide whether to display the domain number in /proc */
 extern int pci_proc_domain(struct pci_bus *bus);
 
-/* MSI arch hooks */
-#define arch_setup_msi_irqs arch_setup_msi_irqs
-#define arch_teardown_msi_irqs arch_teardown_msi_irqs
-#define arch_msi_check_device arch_msi_check_device
-
 struct vm_area_struct;
 /* Map a range of PCI memory or I/O space for a device into user space */
 int pci_mmap_page_range(struct pci_dev *pdev, struct vm_area_struct *vma,
arch/s390/include/asm/pci.h +0 -4
index 6c18012..8641e8d 100644
@@ -21,10 +21,6 @@ void pci_iounmap(struct pci_dev *, void __iomem *);
 int pci_domain_nr(struct pci_bus *);
 int pci_proc_domain(struct pci_bus *);
 
-/* MSI arch hooks */
-#define arch_setup_msi_irqs	arch_setup_msi_irqs
-#define arch_teardown_msi_irqs	arch_teardown_msi_irqs
-
 #define ZPCI_BUS_NR			0	/* default bus number */
 #define ZPCI_DEVFN			0	/* default device number */
 
arch/x86/include/asm/pci.h +0 -28
index d9e9e6c..8c61de0 100644
@@ -100,29 +100,6 @@ static inline void early_quirks(void) { }
 extern void pci_iommu_alloc(void);
 
 #ifdef CONFIG_PCI_MSI
-/* MSI arch specific hooks */
-static inline int x86_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
-{
-	return x86_msi.setup_msi_irqs(dev, nvec, type);
-}
-
-static inline void x86_teardown_msi_irqs(struct pci_dev *dev)
-{
-	x86_msi.teardown_msi_irqs(dev);
-}
-
-static inline void x86_teardown_msi_irq(unsigned int irq)
-{
-	x86_msi.teardown_msi_irq(irq);
-}
-static inline void x86_restore_msi_irqs(struct pci_dev *dev, int irq)
-{
-	x86_msi.restore_msi_irqs(dev, irq);
-}
-#define arch_setup_msi_irqs x86_setup_msi_irqs
-#define arch_teardown_msi_irqs x86_teardown_msi_irqs
-#define arch_teardown_msi_irq x86_teardown_msi_irq
-#define arch_restore_msi_irqs x86_restore_msi_irqs
 /* implemented in arch/x86/kernel/apic/io_apic. */
 struct msi_desc;
 int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
@@ -130,11 +107,6 @@ void native_teardown_msi_irq(unsigned int irq);
 void native_restore_msi_irqs(struct pci_dev *dev, int irq);
 int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc,
 		  unsigned int irq_base, unsigned int irq_offset);
-/* default to the implementation in drivers/lib/msi.c */
-#define HAVE_DEFAULT_MSI_TEARDOWN_IRQS
-#define HAVE_DEFAULT_MSI_RESTORE_IRQS
-void default_teardown_msi_irqs(struct pci_dev *dev);
-void default_restore_msi_irqs(struct pci_dev *dev, int irq);
 #else
 #define native_setup_msi_irqs		NULL
 #define native_teardown_msi_irq		NULL
arch/x86/kernel/x86_init.c +21 -0
index 45a14db..a2b189c 100644
@@ -116,6 +116,27 @@ struct x86_msi_ops x86_msi = {
 	.setup_hpet_msi		= default_setup_hpet_msi,
 };
 
+/* MSI arch specific hooks */
+int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+	return x86_msi.setup_msi_irqs(dev, nvec, type);
+}
+
+void arch_teardown_msi_irqs(struct pci_dev *dev)
+{
+	x86_msi.teardown_msi_irqs(dev);
+}
+
+void arch_teardown_msi_irq(unsigned int irq)
+{
+	x86_msi.teardown_msi_irq(irq);
+}
+
+void arch_restore_msi_irqs(struct pci_dev *dev, int irq)
+{
+	x86_msi.restore_msi_irqs(dev, irq);
+}
+
 struct x86_io_apic_ops x86_io_apic_ops = {
 	.init			= native_io_apic_init_mappings,
 	.read			= native_io_apic_read,
drivers/pci/msi.c +37 -21

91 lines not on this page · raw message

include/linux/msi.h +14 -1
index 20c2d6d..e792a3f 100644
@@ -50,12 +50,25 @@ struct msi_desc {
 };
 
 /*
- * The arch hook for setup up msi irqs
+ * The arch hooks to setup up msi irqs. Those functions are
+ * implemented as weak symbols so that they /can/ be overriden by
+ * architecture specific code if needed. In turn, such architecture
+ * specific code may be interested in calling back into the default
+ * implementation of those operations, in which case the non-weak
+ * default_*() variants can be used.
  */
 int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc);
 void arch_teardown_msi_irq(unsigned int irq);
 int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
 void arch_teardown_msi_irqs(struct pci_dev *dev);
 int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);
+void arch_restore_msi_irqs(struct pci_dev *dev, int irq);
+
+int default_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc);
+void default_teardown_msi_irq(unsigned int irq);
+int default_msi_check_device(struct pci_dev *dev, int nvec, int type);
+int default_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
+void default_teardown_msi_irqs(struct pci_dev *dev);
+void default_restore_msi_irqs(struct pci_dev *dev, int irq);
 
 #endif /* LINUX_MSI_H */
Signature · 1 line
1.8.1.2
#4 lore raw

PATCH v3 2/11 pci: use weak functions for MSI arch-specific functions

On Wed, Jun 19, 2013 at 06:56:10PM +0200, Thomas Petazzoni wrote:
[...]

Quoted 24 lines
drivers/pci/msi.c +14 -2
index 2c10752..4bc0c8f 100644
@@ -30,19 +30,35 @@ static int pci_msi_enable = 1;
 
 /* Arch hooks */
 
-#ifndef arch_msi_check_device
-int arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
+int default_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
+{
+	return -EINVAL;
+}
+
+int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
+{
+	return default_setup_msi_irq(dev, desc);
+}
+
+void default_teardown_msi_irq(unsigned int irq)
+{
+}
+

drivers/pci/msi.c static int pci_msi_enable = 1; · line 38

Quoted 10 lines
drivers/pci/msi.c +6 -1
+void __weak arch_teardown_msi_irq(unsigned int irq)
+{
+	return default_teardown_msi_irq(irq);
+}
+
+int default_msi_check_device(struct pci_dev *dev, int nvec, int type)
 {
 	return 0;
 }
-#endif

I don't think keeping the default_*() for these three is necessary, given that they don't do anything and therefore no architecture is likely to call them when overriding.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130620/d0f26728/attachment.sig>

#5 lore raw

PATCH v3 2/11 pci: use weak functions for MSI arch-specific functions

Dear Thierry Reding,

Quoted 30 lines · Thierry Reding
Quoted 30 lines
drivers/pci/msi.c +20 -2
index 2c10752..4bc0c8f 100644
@@ -30,19 +30,35 @@ static int pci_msi_enable = 1;
 
 /* Arch hooks */
 
-#ifndef arch_msi_check_device
-int arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
+int default_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
+{
+	return -EINVAL;
+}
+
+int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
+{
+	return default_setup_msi_irq(dev, desc);
+}
+
+void default_teardown_msi_irq(unsigned int irq)
+{
+}
+
+void __weak arch_teardown_msi_irq(unsigned int irq)
+{
+	return default_teardown_msi_irq(irq);
+}
+
+int default_msi_check_device(struct pci_dev *dev, int nvec, int type)
Quoted 8 lines

drivers/pci/msi.c static int pci_msi_enable = 1; · line 38

Quoted 4 lines
drivers/pci/msi.c +0 -1
 {
 	return 0;
 }
-#endif

I don't think keeping the default_*() for these three is necessary, given that they don't do anything and therefore no architecture is likely to call them when overriding.

Ok. I was just keeping them for the sake of consistency with the other calls that do have a default behavior, but if it's considered not useful, I'll get rid of them in v4.

Thanks for your comments!

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
maintainer
#6 lore raw

PATCH v3 2/11 pci: use weak functions for MSI arch-specific functions

In reply to Thierry Reding
Quoted 32 lines · Thierry Reding

On Wed, Jun 19, 2013 at 06:56:10PM +0200, Thomas Petazzoni wrote:
[...]

Quoted 30 lines
drivers/pci/msi.c +20 -2
index 2c10752..4bc0c8f 100644
@@ -30,19 +30,35 @@ static int pci_msi_enable = 1;
 
 /* Arch hooks */
 
-#ifndef arch_msi_check_device
-int arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
+int default_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
+{
+     return -EINVAL;
+}
+
+int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
+{
+     return default_setup_msi_irq(dev, desc);
+}
+
+void default_teardown_msi_irq(unsigned int irq)
+{
+}
+
+void __weak arch_teardown_msi_irq(unsigned int irq)
+{
+     return default_teardown_msi_irq(irq);
+}
+
+int default_msi_check_device(struct pci_dev *dev, int nvec, int type)
Quoted 8 lines

drivers/pci/msi.c static int pci_msi_enable = 1; · line 38

Quoted 4 lines
drivers/pci/msi.c +0 -1
 {
      return 0;
 }
-#endif

I don't think keeping the default_*() for these three is necessary, given that they don't do anything and therefore no architecture is likely to call them when overriding.

I agree; the whole point of __weak is to provide a "default" implementation, so I hope you can just remove any empty default_*() functions and fold the others into the arch_*() functions directly.

If there actually are callbacks from arch-specific strong functions back to the stuff in the default_*() functions, that suggests that we should refactor and rename that bit of functionality.

Bjorn

#7 lore raw

PATCH v3 2/11 pci: use weak functions for MSI arch-specific functions

Dear Bjorn Helgaas,

Quoted 7 lines · Bjorn Helgaas

I agree; the whole point of __weak is to provide a "default" implementation, so I hope you can just remove any empty default_*() functions and fold the others into the arch_*() functions directly.

If there actually are callbacks from arch-specific strong functions back to the stuff in the default_*() functions, that suggests that we should refactor and rename that bit of functionality.

There is one such case, as highlighted in the commit log of this patch:

"""
This is needed because the x86 Xen implementation of teardown_msi_irqs() needs to do some work, and then call the default implementation of this hook.
"""

Any suggestion on how to solve this particular case?

Thanks,

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
maintainer
#8 lore raw

PATCH v3 2/11 pci: use weak functions for MSI arch-specific functions

Quoted 14 lines · Thomas Petazzoni

Dear Bjorn Helgaas,

Quoted 7 lines · Bjorn Helgaas

I agree; the whole point of __weak is to provide a "default" implementation, so I hope you can just remove any empty default_*() functions and fold the others into the arch_*() functions directly.

If there actually are callbacks from arch-specific strong functions back to the stuff in the default_*() functions, that suggests that we should refactor and rename that bit of functionality.

There is one such case, as highlighted in the commit log of this patch:

Quoted 7 lines

"""
This is needed because the x86 Xen implementation of teardown_msi_irqs() needs to do some work, and then call the default implementation of this hook.
"""

Any suggestion on how to solve this particular case?

Nope. Just leave it as-is for that case and consolidate what you can.

Bjorn

#9 lore raw

PATCH v3 2/11 pci: use weak functions for MSI arch-specific functions

Dear Bjorn Helgaas,

Quoted 3 lines · Bjorn Helgaas
Quoted 1 line

Any suggestion on how to solve this particular case?

Nope. Just leave it as-is for that case and consolidate what you can.

So I keep the weak arch_*() function and the strong default_*() for this particular hook, and for all the other hooks, I only keep a weak arch_*() function. Is this what you suggest for now?

Best regards,

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
maintainer
#10 lore raw

PATCH v3 2/11 pci: use weak functions for MSI arch-specific functions

Quoted 11 lines · Thomas Petazzoni

Dear Bjorn Helgaas,

Quoted 3 lines · Bjorn Helgaas
Quoted 1 line

Any suggestion on how to solve this particular case?

Nope. Just leave it as-is for that case and consolidate what you can.

So I keep the weak arch_*() function and the strong default_*() for this particular hook, and for all the other hooks, I only keep a weak arch_*() function. Is this what you suggest for now?

Yep, that sounds good.

Bjorn

#11 lore raw

PATCH v3 3/11 PCI: Introduce new MSI chip infrastructure

In reply to Thomas Petazzoni

From: Thierry Reding <[email protected]>

The new struct msi_chip is used to associated an MSI controller with a PCI bus. It is automatically handed down from the root to its children during bus enumeration.

This patch provides default (weak) implementations for the architecture- specific MSI functions (arch_setup_msi_irq(), arch_teardown_msi_irq() and arch_msi_check_device()) which check if a PCI device's bus has an attached MSI chip and forward the call appropriately.

---
 drivers/pci/msi.c   | 22 ++++++++++++++++++++++
 drivers/pci/probe.c |  1 +
 include/linux/msi.h | 11 +++++++++++
 include/linux/pci.h |  1 +
 4 files changed, 35 insertions(+)
drivers/pci/msi.c +22 -0
index 4bc0c8f..dc357e9 100644
@@ -32,6 +32,19 @@ static int pci_msi_enable = 1;
 
 int default_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
 {
+	struct msi_chip *chip = dev->bus->msi;
+
+	if (chip && chip->setup_irq) {
+		int err;
+
+		err = chip->setup_irq(chip, dev, desc);
+		if (err < 0)
+			return err;
+
+		irq_set_chip_data(desc->irq, chip);
+		return err;
+	}
+
 	return -EINVAL;
 }
 
@@ -42,6 +55,10 @@ int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
 
 void default_teardown_msi_irq(unsigned int irq)
 {
+	struct msi_chip *chip = irq_get_chip_data(irq);
+
+	if (chip && chip->teardown_irq)
+		chip->teardown_irq(chip, irq);
 }
 
 void __weak arch_teardown_msi_irq(unsigned int irq)
@@ -51,6 +68,11 @@ void __weak arch_teardown_msi_irq(unsigned int irq)
 
 int default_msi_check_device(struct pci_dev *dev, int nvec, int type)
 {
+	struct msi_chip *chip = dev->bus->msi;
+
+	if (chip && chip->check_device)
+		return chip->check_device(chip, dev, nvec, type);
+
 	return 0;
 }
 
drivers/pci/probe.c +1 -0
index 70f10fa..c8591e4 100644
@@ -634,6 +634,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 
 	child->parent = parent;
 	child->ops = parent->ops;
+	child->msi = parent->msi;
 	child->sysdata = parent->sysdata;
 	child->bus_flags = parent->bus_flags;
 
include/linux/msi.h +11 -0
index e792a3f..e3a137d 100644
@@ -71,4 +71,15 @@ int default_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
 void default_teardown_msi_irqs(struct pci_dev *dev);
 void default_restore_msi_irqs(struct pci_dev *dev, int irq);
 
+struct msi_chip {
+	struct module *owner;
+	struct device *dev;
+
+	int (*setup_irq)(struct msi_chip *chip, struct pci_dev *dev,
+			 struct msi_desc *desc);
+	void (*teardown_irq)(struct msi_chip *chip, unsigned int irq);
+	int (*check_device)(struct msi_chip *chip, struct pci_dev *dev,
+			    int nvec, int type);
+};
+
 #endif /* LINUX_MSI_H */
include/linux/pci.h +1 -0
index 3a24e4f..7ffc012 100644
@@ -432,6 +432,7 @@ struct pci_bus {
 	struct resource busn_res;	/* bus numbers routed to this bus */
 
 	struct pci_ops	*ops;		/* configuration access functions */
+	struct msi_chip	*msi;		/* MSI controller */
 	void		*sysdata;	/* hook for sys-specific extension */
 	struct proc_dir_entry *procdir;	/* directory entry in /proc/bus/pci */
 
Signature · 1 line
1.8.1.2
#12 lore raw

PATCH v3 4/11 of: pci: add registry of MSI chips

In reply to Thomas Petazzoni

This commit adds a very basic registry of msi_chip structures, so that an IRQ controller driver can register an msi_chip, and a PCIe host controller can find it, based on a 'struct device_node'.

---
 drivers/of/of_pci.c    | 22 ++++++++++++++++++++++
 include/linux/msi.h    |  2 ++
 include/linux/of_pci.h |  4 ++++
 3 files changed, 28 insertions(+)
drivers/of/of_pci.c +22 -0
index 42c687a..ca12db8 100644
@@ -89,3 +89,25 @@ int of_pci_parse_bus_range(struct device_node *node, struct resource *res)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_pci_parse_bus_range);
+
+static LIST_HEAD(msi_chip_list);
+static DEFINE_MUTEX(msi_chip_mutex);
+
+void of_msi_chip_add(struct msi_chip *chip)
+{
+	mutex_lock(&msi_chip_mutex);
+	list_add(&chip->link, &msi_chip_list);
+	mutex_unlock(&msi_chip_mutex);
+}
+
+struct msi_chip *of_msi_chip_find_by_node(struct device_node *of_node)
+{
+	struct msi_chip *c;
+	list_for_each_entry(c, &msi_chip_list, link) {
+		if (c->of_node == of_node &&
+		    of_property_read_bool(c->of_node, "msi-controller"))
+			return c;
+	}
+
+	return NULL;
+}
include/linux/msi.h +2 -0
index e3a137d..8b930c3 100644
@@ -74,6 +74,8 @@ void default_restore_msi_irqs(struct pci_dev *dev, int irq);
 struct msi_chip {
 	struct module *owner;
 	struct device *dev;
+	struct device_node *of_node;
+	struct list_head link;
 
 	int (*setup_irq)(struct msi_chip *chip, struct pci_dev *dev,
 			 struct msi_desc *desc);
include/linux/of_pci.h +4 -0
index 7a04826..6e88189 100644
@@ -2,6 +2,7 @@
 #define __OF_PCI_H
 
 #include <linux/pci.h>
+#include <linux/msi.h>
 
 struct pci_dev;
 struct of_irq;
@@ -13,4 +14,7 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
 int of_pci_get_devfn(struct device_node *np);
 int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
 
+void of_msi_chip_add(struct msi_chip *chip);
+struct msi_chip *of_msi_chip_find_by_node(struct device_node *of_node);
+
 #endif
Signature · 1 line
1.8.1.2
#13 lore raw

PATCH v3 4/11 of: pci: add registry of MSI chips

Grant,

Quoted 10 lines · Thomas Petazzoni

This commit adds a very basic registry of msi_chip structures, so that an IRQ controller driver can register an msi_chip, and a PCIe host controller can find it, based on a 'struct device_node'.

---
 drivers/of/of_pci.c    | 22 ++++++++++++++++++++++
 include/linux/msi.h    |  2 ++
 include/linux/of_pci.h |  4 ++++
 3 files changed, 28 insertions(+)

Any comments about this idea? It is quite fundamental for this patch, which requires a way of connecting PCI busses (handled by a PCI driver) and a msi_chip (handled by an IRQ controller driver).

Thanks,

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#14 lore raw

PATCH v3 4/11 of: pci: add registry of MSI chips

In reply to Thomas Petazzoni
Quoted 16 lines · Thomas Petazzoni

This commit adds a very basic registry of msi_chip structures, so that an IRQ controller driver can register an msi_chip, and a PCIe host controller can find it, based on a 'struct device_node'.

---
 drivers/of/of_pci.c    | 22 ++++++++++++++++++++++
 include/linux/msi.h    |  2 ++
 include/linux/of_pci.h |  4 ++++
 3 files changed, 28 insertions(+)
drivers/of/of_pci.c +0 -0
index 42c687a..ca12db8 100644

drivers/of/of_pci.c int of_pci_parse_bus_range(struct device_node *node, struct resource *res) · line 94

Quoted 6 lines
drivers/of/of_pci.c +3 -0
@@ -89,3 +89,25 @@ int of_pci_parse_bus_range(struct device_node *node, struct resource *res)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_pci_parse_bus_range);
+
+static LIST_HEAD(msi_chip_list);
+static DEFINE_MUTEX(msi_chip_mutex);

Should all of this code perhaps be conditionalized by an #ifdef PCI_MSI?

drivers/of/of_pci.c int of_pci_parse_bus_range(struct device_node *node, struct resource *res) · line 101

Quoted 7 lines
drivers/of/of_pci.c +7 -0
+
+void of_msi_chip_add(struct msi_chip *chip)
+{
+	mutex_lock(&msi_chip_mutex);
+	list_add(&chip->link, &msi_chip_list);
+	mutex_unlock(&msi_chip_mutex);
+}

Since eventually we may want to use these functions from modules, maybe they should be exported?

Also isn't this missing an of_msi_chip_remove() counterpart?

drivers/of/of_pci.c int of_pci_parse_bus_range(struct device_node *node, struct resource *res) · line 103

Quoted 1 line
drivers/of/of_pci.c +1 -0
+struct msi_chip *of_msi_chip_find_by_node(struct device_node *of_node)

Most other functions of this type are named of_find_*_by_node(), so for consistency it'd be better to call this of_find_msi_chip_by_node().

drivers/of/of_pci.c int of_pci_parse_bus_range(struct device_node *node, struct resource *res) · line 108

Quoted 5 lines
drivers/of/of_pci.c +5 -0
+{
+	struct msi_chip *c;
+	list_for_each_entry(c, &msi_chip_list, link) {
+		if (c->of_node == of_node &&
+		    of_property_read_bool(c->of_node, "msi-controller"))

Perhaps it would be more efficient to put this check within the of_msi_chip_add() function?

include/linux/msi.h void default_restore_msi_irqs(struct pci_dev *dev, int irq); · line 77

Quoted 9 lines
include/linux/msi.h +1 -0
index e3a137d..8b930c3 100644
@@ -74,6 +74,8 @@ void default_restore_msi_irqs(struct pci_dev *dev, int irq);
 struct msi_chip {
 	struct module *owner;
 	struct device *dev;
+	struct device_node *of_node;

I know there's no easier way to do this, but I think we need to find a way to clean this up at some point. There's no point in storing the OF node in two places.

include/linux/msi.h void default_restore_msi_irqs(struct pci_dev *dev, int irq); · line 78

Quoted 1 line
include/linux/msi.h +1 -0
+	struct list_head link;

I'd prefer this to be named "list" for consistency.

Quoted 13 lines
include/linux/of_pci.h +1 -0
index 7a04826..6e88189 100644
@@ -2,6 +2,7 @@
 #define __OF_PCI_H
 
 #include <linux/pci.h>
+#include <linux/msi.h>
 
 struct pci_dev;
 struct of_irq;

include/linux/of_pci.h struct device_node *of_pci_find_child_device(struct device_node *parent, · line 18

Quoted 5 lines
include/linux/of_pci.h +2 -0
@@ -13,4 +14,7 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
 int of_pci_get_devfn(struct device_node *np);
 int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
 
+void of_msi_chip_add(struct msi_chip *chip);
+struct msi_chip *of_msi_chip_find_by_node(struct device_node *of_node);

Since these will be exported and conditionalized on PCI_MSI as suggested above, maybe they should get a dummy implementation if (!OF || !PCI_MSI) so that drivers don't have to conditionally call them?

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130621/7f5bbf19/attachment.sig>

#15 lore raw

PATCH v3 5/11 irqchip: armada-370-xp: properly request resources

In reply to Thomas Petazzoni

Instead of using of_iomap(), we now use of_address_to_resource(), request_mem_region() and ioremap(). This allows the corresponding I/O regions to be properly requested and visible in /proc/iomem.

The main motivation for this change is that the introduction of the MSI support requires us to get the physical address of the main interrupt controller registers, so we will need the corresponding 'struct resource' anyway.

We also take this opportunity to change a panic() to BUG_ON(), in order to be consistent with the rest of the driver.

---
 drivers/irqchip/irq-armada-370-xp.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
drivers/irqchip/irq-armada-370-xp.c +16 -4
index bb328a3..26adc74 100644
@@ -248,12 +248,25 @@ armada_370_xp_handle_irq(struct pt_regs *regs)
 static int __init armada_370_xp_mpic_of_init(struct device_node *node,
 					     struct device_node *parent)
 {
+	struct resource main_int_res, per_cpu_int_res;
 	u32 control;
 
-	main_int_base = of_iomap(node, 0);
-	per_cpu_int_base = of_iomap(node, 1);
+	BUG_ON(of_address_to_resource(node, 0, &main_int_res));
+	BUG_ON(of_address_to_resource(node, 1, &per_cpu_int_res));
 
+	BUG_ON(!request_mem_region(main_int_res.start,
+				   resource_size(&main_int_res),
+				   node->full_name));
+	BUG_ON(!request_mem_region(per_cpu_int_res.start,
+				   resource_size(&per_cpu_int_res),
+				   node->full_name));
+
+	main_int_base = ioremap(main_int_res.start,
+				resource_size(&main_int_res));
 	BUG_ON(!main_int_base);
+
+	per_cpu_int_base = ioremap(per_cpu_int_res.start,
+				   resource_size(&per_cpu_int_res));
 	BUG_ON(!per_cpu_int_base);
 
 	control = readl(main_int_base + ARMADA_370_XP_INT_CONTROL);
@@ -262,8 +275,7 @@ static int __init armada_370_xp_mpic_of_init(struct device_node *node,
 		irq_domain_add_linear(node, (control >> 2) & 0x3ff,
 				&armada_370_xp_mpic_irq_ops, NULL);
 
-	if (!armada_370_xp_mpic_domain)
-		panic("Unable to add Armada_370_Xp MPIC irq domain (DT)\n");
+	BUG_ON(!armada_370_xp_mpic_domain);
 
 	irq_set_default_host(armada_370_xp_mpic_domain);
 
Signature · 1 line
1.8.1.2
#16 lore raw

PATCH v3 5/11 irqchip: armada-370-xp: properly request resources

Thomas Gleixner,

Quoted 9 lines · Thomas Petazzoni

Instead of using of_iomap(), we now use of_address_to_resource(), request_mem_region() and ioremap(). This allows the corresponding I/O regions to be properly requested and visible in /proc/iomem.

The main motivation for this change is that the introduction of the MSI support requires us to get the physical address of the main interrupt controller registers, so we will need the corresponding 'struct resource' anyway.

Quoted 7 lines

We also take this opportunity to change a panic() to BUG_ON(), in order to be consistent with the rest of the driver.

---
 drivers/irqchip/irq-armada-370-xp.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Any chance you could take this one for 3.11 ? It's independent of the rest of the patch series, and it would reduce the amount of code remaining to merge for the entire MSI feature.

Thanks,

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#17 lore raw

PATCH v3 6/11 irqchip: armada-370-xp: implement MSI support

In reply to Thomas Petazzoni

This commit introduces the support for the MSI interrupts in the armada-370-xp interrupt controller driver. It registers an MSI chip to the MSI chip registry, which will be used by the Marvell PCIe host controller driver.

The MSI interrupts use the 16 high doorbells, and are therefore notified using IRQ1 of the main interrupt controller.

---
 drivers/irqchip/irq-armada-370-xp.c | 135 +++++++++++++++++++++++++++++++++++-
 1 file changed, 134 insertions(+), 1 deletion(-)
drivers/irqchip/irq-armada-370-xp.c +134 -1
index 26adc74..9e73547 100644
@@ -21,7 +21,10 @@
 #include <linux/io.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/of_pci.h>
 #include <linux/irqdomain.h>
+#include <linux/slab.h>
+#include <linux/msi.h>
 #include <asm/mach/arch.h>
 #include <asm/exception.h>
 #include <asm/smp_plat.h>
@@ -51,12 +54,20 @@
 #define IPI_DOORBELL_START                      (0)
 #define IPI_DOORBELL_END                        (8)
 #define IPI_DOORBELL_MASK                       0xFF
+#define PCI_MSI_DOORBELL_START                  (16)
+#define PCI_MSI_DOORBELL_NR                     (16)
+#define PCI_MSI_DOORBELL_END                    (32)
+#define PCI_MSI_DOORBELL_MASK                   0xFFFF0000
 
 static DEFINE_RAW_SPINLOCK(irq_controller_lock);
 
 static void __iomem *per_cpu_int_base;
 static void __iomem *main_int_base;
 static struct irq_domain *armada_370_xp_mpic_domain;
+#ifdef CONFIG_PCI_MSI
+static struct irq_domain *armada_370_xp_msi_domain;
+static phys_addr_t msi_doorbell_addr;
+#endif
 
 /*
  * In SMP mode:
@@ -87,6 +98,96 @@ static void armada_370_xp_irq_unmask(struct irq_data *d)
 				ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
 }
 
+#ifdef CONFIG_PCI_MSI
+
+static int armada_370_xp_setup_msi_irq(struct msi_chip *chip,
+				       struct pci_dev *pdev,
+				       struct msi_desc *desc)
+{
+	struct msi_msg msg;
+	irq_hw_number_t hwirq;
+	int virq;
+
+	virq = irq_alloc_mapping(armada_370_xp_msi_domain, &hwirq);
+	if (!virq)
+		return -EINVAL;
+
+	irq_set_msi_desc(virq, desc);
+
+	msg.address_lo = msi_doorbell_addr;
+	msg.address_hi = 0;
+	msg.data = 0xf00 | (hwirq + 16);
+
+	write_msi_msg(virq, &msg);
+	return 0;
+}
+
+static void armada_370_xp_teardown_msi_irq(struct msi_chip *chip,
+					   unsigned int irq)
+{
+	irq_dispose_mapping(irq);
+}
+
+static struct irq_chip armada_370_xp_msi_irq_chip = {
+	.name = "armada_370_xp_msi_irq",
+	.irq_enable = unmask_msi_irq,
+	.irq_disable = mask_msi_irq,
+	.irq_mask = mask_msi_irq,
+	.irq_unmask = unmask_msi_irq,
+};
+
+static int armada_370_xp_msi_map(struct irq_domain *domain, unsigned int virq,
+				 irq_hw_number_t hw)
+{
+	irq_set_chip_and_handler(virq, &armada_370_xp_msi_irq_chip,
+				 handle_simple_irq);
+	set_irq_flags(virq, IRQF_VALID);
+
+	return 0;
+}
+
+static const struct irq_domain_ops armada_370_xp_msi_irq_ops = {
+	.map = armada_370_xp_msi_map,
+};
+
+static int armada_370_xp_msi_init(struct device_node *node)
+{
+	struct msi_chip *msi_chip;
+	u32 reg;
+
+	msi_chip = kzalloc(sizeof(*msi_chip), GFP_KERNEL);
+	if (!msi_chip)
+		return -ENOMEM;
+
+	armada_370_xp_msi_domain =
+		irq_domain_add_linear(NULL, PCI_MSI_DOORBELL_NR,
+				      &armada_370_xp_msi_irq_ops, NULL);
+	if (!armada_370_xp_msi_domain) {
+		kfree(msi_chip);
+		return -ENOMEM;
+	}
+
+	msi_chip->of_node = node;
+	msi_chip->setup_irq = armada_370_xp_setup_msi_irq;
+	msi_chip->teardown_irq = armada_370_xp_teardown_msi_irq;
+
+	of_msi_chip_add(msi_chip);
+
+	reg = readl(per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_MSK_OFFS)
+		| PCI_MSI_DOORBELL_MASK;
+
+	writel(reg, per_cpu_int_base +
+	       ARMADA_370_XP_IN_DRBEL_MSK_OFFS);
+
+	/* Unmask IPI interrupt */
+	writel(1, per_cpu_int_base + ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
+
+	return 0;
+}
+#else
+static inline int armada_370_xp_msi_init(struct device_node *node) { return 0; }
+#endif
+
 #ifdef CONFIG_SMP
 static int armada_xp_set_affinity(struct irq_data *d,
 				  const struct cpumask *mask_val, bool force)
@@ -214,12 +315,39 @@ armada_370_xp_handle_irq(struct pt_regs *regs)
 		if (irqnr > 1022)
 			break;
 
-		if (irqnr > 0) {
+		if (irqnr > 1) {
 			irqnr =	irq_find_mapping(armada_370_xp_mpic_domain,
 					irqnr);
 			handle_IRQ(irqnr, regs);
 			continue;
 		}
+
+#ifdef CONFIG_PCI_MSI
+		/* MSI handling */
+		if (irqnr == 1) {
+			u32 msimask, msinr;
+
+			msimask = readl_relaxed(per_cpu_int_base +
+						ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS)
+				& PCI_MSI_DOORBELL_MASK;
+
+			writel(~PCI_MSI_DOORBELL_MASK, per_cpu_int_base +
+			       ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS);
+
+			for (msinr = PCI_MSI_DOORBELL_START;
+			     msinr < PCI_MSI_DOORBELL_END; msinr++) {
+				int irq;
+
+				if (!(msimask & BIT(msinr)))
+					continue;
+
+				irq = irq_find_mapping(armada_370_xp_msi_domain,
+						       msinr - 16);
+				handle_IRQ(irq, regs);
+			}
+		}
+#endif
+
 #ifdef CONFIG_SMP
 		/* IPI Handling */
 		if (irqnr == 0) {
@@ -269,6 +397,9 @@ static int __init armada_370_xp_mpic_of_init(struct device_node *node,
 				   resource_size(&per_cpu_int_res));
 	BUG_ON(!per_cpu_int_base);
 
+	msi_doorbell_addr = main_int_res.start +
+		ARMADA_370_XP_SW_TRIG_INT_OFFS;
+
 	control = readl(main_int_base + ARMADA_370_XP_INT_CONTROL);
 
 	armada_370_xp_mpic_domain =
@@ -292,6 +423,8 @@ static int __init armada_370_xp_mpic_of_init(struct device_node *node,
 
 #endif
 
+	armada_370_xp_msi_init(node);
+
 	set_handle_irq(armada_370_xp_handle_irq);
 
 	return 0;
Signature · 1 line
1.8.1.2
#18 lore raw

PATCH v3 7/11 arm: pci: add ->add_bus() and ->remove_bus() hooks to hw_pci

In reply to Thomas Petazzoni

Some PCI drivers may need to adjust the pci_bus structure after it has been allocated by the Linux PCI core. The PCI core allows architectures to implement the pcibios_add_bus() and pcibios_remove_bus() for this purpose. This commit therefore extends the hw_pci and pci_sys_data structures of the ARM PCI core to allow PCI drivers to register ->add_bus() and ->remove_bus() in hw_pci, which will get called when a bus is added or removed from the system.

This will be used for example by the Marvell PCIe driver to connect a particular PCI bus with its corresponding MSI chip to handle Message Signaled Interrupts.

---
 arch/arm/include/asm/mach/pci.h |  4 ++++
 arch/arm/kernel/bios32.c        | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)
arch/arm/include/asm/mach/pci.h +4 -0
index 7d2c3c8..b71cc9d 100644
@@ -35,6 +35,8 @@ struct hw_pci {
 					  resource_size_t start,
 					  resource_size_t size,
 					  resource_size_t align);
+	void            (*add_bus)(struct pci_bus *bus);
+	void            (*remove_bus)(struct pci_bus *bus);
 };
 
 /*
@@ -62,6 +64,8 @@ struct pci_sys_data {
 					  resource_size_t start,
 					  resource_size_t size,
 					  resource_size_t align);
+	void            (*add_bus)(struct pci_bus *bus);
+	void            (*remove_bus)(struct pci_bus *bus);
 	void		*private_data;	/* platform controller private data	*/
 };
 
arch/arm/kernel/bios32.c +16 -0
index b2ed73c..0a441f8 100644
@@ -363,6 +363,20 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 }
 EXPORT_SYMBOL(pcibios_fixup_bus);
 
+void pcibios_add_bus(struct pci_bus *bus)
+{
+	struct pci_sys_data *sys = bus->sysdata;
+	if (sys->add_bus)
+		sys->add_bus(bus);
+}
+
+void pcibios_remove_bus(struct pci_bus *bus)
+{
+	struct pci_sys_data *sys = bus->sysdata;
+	if (sys->remove_bus)
+		sys->remove_bus(bus);
+}
+
 /*
  * Swizzle the device pin each time we cross a bridge.  If a platform does
  * not provide a swizzle function, we perform the standard PCI swizzling.
@@ -463,6 +477,8 @@ static void pcibios_init_hw(struct hw_pci *hw, struct list_head *head)
 		sys->swizzle = hw->swizzle;
 		sys->map_irq = hw->map_irq;
 		sys->align_resource = hw->align_resource;
+		sys->add_bus        = hw->add_bus;
+		sys->remove_bus     = hw->remove_bus;
 		INIT_LIST_HEAD(&sys->resources);
 
 		if (hw->private_data)
Signature · 1 line
1.8.1.2
#19 lore raw

PATCH v3 7/11 arm: pci: add ->add_bus() and ->remove_bus() hooks to hw_pci

Russell,

Quoted 12 lines · Thomas Petazzoni

Some PCI drivers may need to adjust the pci_bus structure after it has been allocated by the Linux PCI core. The PCI core allows architectures to implement the pcibios_add_bus() and pcibios_remove_bus() for this purpose. This commit therefore extends the hw_pci and pci_sys_data structures of the ARM PCI core to allow PCI drivers to register ->add_bus() and ->remove_bus() in hw_pci, which will get called when a bus is added or removed from the system.

This will be used for example by the Marvell PCIe driver to connect a particular PCI bus with its corresponding MSI chip to handle Message Signaled Interrupts.

Quoted 5 lines
---
 arch/arm/include/asm/mach/pci.h |  4 ++++
 arch/arm/kernel/bios32.c        | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

Do you have any comments ? Any chance to get this merged for 3.11 ? It is independent of the previous patches of the patch series, so it can be merged independently.

Thanks!

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#20 lore raw

PATCH v3 7/11 arm: pci: add ->add_bus() and ->remove_bus() hooks to hw_pci

In reply to Thomas Petazzoni
Quoted 12 lines · Thomas Petazzoni

Some PCI drivers may need to adjust the pci_bus structure after it has been allocated by the Linux PCI core. The PCI core allows architectures to implement the pcibios_add_bus() and pcibios_remove_bus() for this purpose. This commit therefore extends the hw_pci and pci_sys_data structures of the ARM PCI core to allow PCI drivers to register ->add_bus() and ->remove_bus() in hw_pci, which will get called when a bus is added or removed from the system.

This will be used for example by the Marvell PCIe driver to connect a particular PCI bus with its corresponding MSI chip to handle Message Signaled Interrupts.

Quoted 5 lines
---
 arch/arm/include/asm/mach/pci.h |  4 ++++
 arch/arm/kernel/bios32.c        | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130621/19a95846/attachment-0001.sig>

#21 lore raw

PATCH v3 8/11 arm: mvebu: the MPIC now provides MSI controller features

In reply to Thomas Petazzoni

Adds the 'msi-controller' property to the main interrupt controller Device Tree node, to indicate that it can now behave as a MSI controller.

---
 arch/arm/boot/dts/armada-370-xp.dtsi | 1 +
 1 file changed, 1 insertion(+)
arch/arm/boot/dts/armada-370-xp.dtsi +1 -0
index bb6224c..9020478 100644
@@ -52,6 +52,7 @@
 				#interrupt-cells = <1>;
 				#size-cells = <1>;
 				interrupt-controller;
+				msi-controller;
 			};
 
 			coherency-fabric at 20200 {
Signature · 1 line
1.8.1.2
#22 lore raw

PATCH v3 8/11 arm: mvebu: the MPIC now provides MSI controller features

Quoted 8 lines · Thomas Petazzoni

Adds the 'msi-controller' property to the main interrupt controller Device Tree node, to indicate that it can now behave as a MSI controller.

---
 arch/arm/boot/dts/armada-370-xp.dtsi | 1 +
 1 file changed, 1 insertion(+)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130621/daacaf43/attachment.sig>

#23 lore raw

PATCH v3 9/11 pci: mvebu: add support for MSI

In reply to Thomas Petazzoni

This commit adds support for Message Signaled Interrupts in the Marvell PCIe host controller. The work is very simple: it simply gets a reference to the msi_chip associated to the PCIe controller thanks to the msi-parent DT property, and stores this reference in the pci_bus structure. This is enough to let the Linux PCI core use the functions of msi_chip to setup and teardown MSIs.

---
 drivers/pci/host/pci-mvebu.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
drivers/pci/host/pci-mvebu.c +26 -0
index 13a633b..89638d1 100644
@@ -11,6 +11,7 @@
 #include <linux/clk.h>
 #include <linux/module.h>
 #include <linux/mbus.h>
+#include <linux/msi.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
 #include <linux/of_address.h>
@@ -107,6 +108,7 @@ struct mvebu_pcie_port;
 struct mvebu_pcie {
 	struct platform_device *pdev;
 	struct mvebu_pcie_port *ports;
+	struct msi_chip *msi;
 	struct resource io;
 	struct resource realio;
 	struct resource mem;
@@ -695,6 +697,12 @@ static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	return bus;
 }
 
+void mvebu_pcie_add_bus(struct pci_bus *bus)
+{
+	struct mvebu_pcie *pcie = sys_to_pcie(bus->sysdata);
+	bus->msi = pcie->msi;
+}
+
 resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev,
 					  const struct resource *res,
 					  resource_size_t start,
@@ -731,6 +739,7 @@ static void __init mvebu_pcie_enable(struct mvebu_pcie *pcie)
 	hw.map_irq        = mvebu_pcie_map_irq;
 	hw.ops            = &mvebu_pcie_ops;
 	hw.align_resource = mvebu_pcie_align_resource;
+	hw.add_bus        = mvebu_pcie_add_bus;
 
 	pci_common_init(&hw);
 }
@@ -755,6 +764,21 @@ mvebu_pcie_map_registers(struct platform_device *pdev,
 	return devm_request_and_ioremap(&pdev->dev, &regs);
 }
 
+static void __init mvebu_pcie_msi_enable(struct mvebu_pcie *pcie)
+{
+	struct device_node *msi_node;
+
+	msi_node = of_parse_phandle(pcie->pdev->dev.of_node,
+				    "msi-parent", 0);
+	if (!msi_node)
+		return;
+
+	pcie->msi = of_msi_chip_find_by_node(msi_node);
+
+	if (pcie->msi)
+		pcie->msi->dev = &pcie->pdev->dev;
+}
+
 static int __init mvebu_pcie_probe(struct platform_device *pdev)
 {
 	struct mvebu_pcie *pcie;
@@ -879,6 +903,8 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
 		i++;
 	}
 
+	mvebu_pcie_msi_enable(pcie);
+
 	mvebu_pcie_enable(pcie);
 
 	return 0;
Signature · 1 line
1.8.1.2
#24 lore raw

PATCH v3 9/11 pci: mvebu: add support for MSI

Quoted 11 lines · Thomas Petazzoni

This commit adds support for Message Signaled Interrupts in the Marvell PCIe host controller. The work is very simple: it simply gets a reference to the msi_chip associated to the PCIe controller thanks to the msi-parent DT property, and stores this reference in the pci_bus structure. This is enough to let the Linux PCI core use the functions of msi_chip to setup and teardown MSIs.

---
 drivers/pci/host/pci-mvebu.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130621/c5330e02/attachment.sig>

maintainer
#25 lore raw

PATCH v3 9/11 pci: mvebu: add support for MSI

Quoted 8 lines · Thierry Reding
Quoted 7 lines · Thomas Petazzoni

This commit adds support for Message Signaled Interrupts in the Marvell PCIe host controller. The work is very simple: it simply gets a reference to the msi_chip associated to the PCIe controller thanks to the msi-parent DT property, and stores this reference in the pci_bus structure. This is enough to let the Linux PCI core use the functions of msi_chip to setup and teardown MSIs.

Quoted 6 lines
Quoted 4 lines
---
 drivers/pci/host/pci-mvebu.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

I assume you'll merge this through arm-soc or some other non-PCI tree.

Bjorn

#26 lore raw

PATCH v3 10/11 arm: mvebu: indicate that this platform supports MSI

In reply to Thomas Petazzoni

Now that all the pieces are in place, we can adjust the mach-mvebu/Kconfig file to indicate that this platform supports MSIs.

---
 arch/arm/mach-mvebu/Kconfig | 1 +
 1 file changed, 1 insertion(+)
arch/arm/mach-mvebu/Kconfig +1 -0
index 9eb63d7..36951b1 100644
@@ -15,6 +15,7 @@ config ARCH_MVEBU
 	select ARCH_REQUIRE_GPIOLIB
 	select MIGHT_HAVE_PCI
 	select PCI_QUIRKS if PCI
+	select ARCH_SUPPORTS_MSI if PCI
 
 if ARCH_MVEBU
 
Signature · 1 line
1.8.1.2
#27 lore raw

PATCH v3 10/11 arm: mvebu: indicate that this platform supports MSI

Quoted 13 lines · Thomas Petazzoni

Now that all the pieces are in place, we can adjust the mach-mvebu/Kconfig file to indicate that this platform supports MSIs.

---
 arch/arm/mach-mvebu/Kconfig | 1 +
 1 file changed, 1 insertion(+)
arch/arm/mach-mvebu/Kconfig +0 -0
index 9eb63d7..36951b1 100644

arch/arm/mach-mvebu/Kconfig config ARCH_MVEBU · line 18

Quoted 4 lines
arch/arm/mach-mvebu/Kconfig +1 -0
@@ -15,6 +15,7 @@ config ARCH_MVEBU
 	select ARCH_REQUIRE_GPIOLIB
 	select MIGHT_HAVE_PCI
 	select PCI_QUIRKS if PCI
+	select ARCH_SUPPORTS_MSI if PCI

I think we can get rid of ARCH_SUPPORTS_MSI now. I actually wrote a patch once to do that but haven't posted it yet. The reason is that the new weak functions do the right thing if MSI isn't supported by some architecture or platform.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130621/4f32694f/attachment-0001.sig>

#28 lore raw

PATCH v3 11/11 arm: mvebu: link PCIe controllers to the MSI controller

In reply to Thomas Petazzoni

This commit adjusts the Armada 370 and Armada XP PCIe controllers Device Tree informations to reference their MSI controller. In the case of this platform, the MSI controller is implemented by the MPIC.

---
 arch/arm/boot/dts/armada-370.dtsi        | 1 +
 arch/arm/boot/dts/armada-xp-mv78230.dtsi | 1 +
 arch/arm/boot/dts/armada-xp-mv78260.dtsi | 1 +
 arch/arm/boot/dts/armada-xp-mv78460.dtsi | 1 +
 4 files changed, 4 insertions(+)
arch/arm/boot/dts/armada-370.dtsi +1 -0
index fa3dfc6..a60018d 100644
@@ -178,6 +178,7 @@
 				#address-cells = <3>;
 				#size-cells = <2>;
 
+				msi-parent = <&mpic>;
 				bus-range = <0x00 0xff>;
 
 				ranges = <0x82000000 0 0x40000 0x40000 0 0x00002000   /* Port 0.0 registers */
arch/arm/boot/dts/armada-xp-mv78230.dtsi +1 -0
index f8eaa38..4a9a305 100644
@@ -91,6 +91,7 @@
 #address-cells = <3>;
 #size-cells = <2>;
 
+				msi-parent = <&mpic>;
 				bus-range = <0x00 0xff>;
 
 				ranges = <0x82000000 0 0x40000 0x40000 0 0x00002000   /* Port 0.0 registers */
arch/arm/boot/dts/armada-xp-mv78260.dtsi +1 -0
index f4029f0..bc63783 100644
@@ -111,6 +111,7 @@
 				#address-cells = <3>;
 				#size-cells = <2>;
 
+				msi-parent = <&mpic>;
 				bus-range = <0x00 0xff>;
 
 				ranges = <0x82000000 0 0x40000 0x40000 0 0x00002000   /* Port 0.0 registers */
arch/arm/boot/dts/armada-xp-mv78460.dtsi +1 -0
index c7b1f4d..77db2bc 100644
@@ -127,6 +127,7 @@
 				#address-cells = <3>;
 				#size-cells = <2>;
 
+				msi-parent = <&mpic>;
 				bus-range = <0x00 0xff>;
 
 				ranges = <0x82000000 0 0x40000 0x40000 0 0x00002000   /* Port 0.0 registers */
Signature · 1 line
1.8.1.2