MSI support for Marvell EBU PCIe driver
Subsystems (6)
v2 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 2 follows the RFC version sent on March, 26th 2013. This is based on work done by Lior Amsalem <[email protected]>.
The patches do the following:
* Patch 1 comes from Thierry Redding, and adds a minimal msi_chip infrastructure.
This needs to be reviewed/acked by Bjorn Helgaas.
* Patch 2 extends the msi_chip infrastructure with a small registry, so that one driver can register an msi_chip, and another driver can find the msi_chip associated to a particular Device Tree node.
This needs to be reviewed/acked by Bjorn Helgaas.
* Patch 3 makes some not very interesting preparation in the Armada 370/XP IRQ controller driver.
* Patch 4 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 2.
* Patch 5 adjust the Armada 370/XP Device Tree to indicate that the MPIC is not only an interrupt-controller, but also an msi-controller.
* Patch 6 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 needs to be reviewed/acked by Bjorn Helgaas.
* Patch 7 tunes Kconfig to indicate that Armada 370/XP supports MSI.
* Patch 8 adjusts the Armada 370/XP Device Tree to add the msi-parent properties in the PCIe controller nodes.
This set of patches currently applies on top of the current "for-next" branch of Jason Cooper's repository.
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 (7):
PCI: Add registry of MSI chips
irqchip: armada-370-xp: properly request resources
irqchip: armada-370-xp: implement MSI support
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/mach-mvebu/Kconfig | 1 + drivers/irqchip/irq-armada-370-xp.c | 186 ++++++++++++++++++++++++++++++- drivers/pci/host/pci-mvebu.c | 21 ++++ drivers/pci/msi.c | 59 +++++++++- drivers/pci/probe.c | 1 + include/linux/msi.h | 22 ++++ include/linux/pci.h | 1 + 12 files changed, 288 insertions(+), 8 deletions(-)
Signature · 1 line
1.8.1.2
PATCH v2 1/8 PCI: Introduce new MSI chip infrastructure
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 | 34 +++++++++++++++++++++++++++++++--- drivers/pci/probe.c | 1 + include/linux/msi.h | 11 +++++++++++ include/linux/pci.h | 1 + 4 files changed, 44 insertions(+), 3 deletions(-)
drivers/pci/msi.c +31 -3
@@ -29,13 +29,41 @@ static int pci_msi_enable = 1;/* Arch hooks */ +int __weak arch_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 ; +} -#ifndef arch_msi_check_device -int arch_msi_check_device (struct pci_dev *dev,int nvec,int type) +void __weak arch_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); +} + +int __weak arch_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 ; } -#endif #ifndef arch_setup_msi_irqs# define arch_setup_msi_irqs default_setup_msi_irqs
drivers/pci/probe.c +1 -0
@@ -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
@@ -58,4 +58,15 @@ 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); +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
@@ -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
Bjorn Helgaas
maintainerPATCH v2 1/8 PCI: Introduce new MSI chip infrastructure
Quoted 39 lines · Thomas Petazzoni
From: Thierry Reding <[email protected]>
The new struct
msi_chipis 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()andarch_msi_check_device()) which check if a PCI device's bus has an attached MSI chip and forward the call appropriately.Signed-off-by Thierry Reding <[email protected]>Signed-off-by Thomas Petazzoni <[email protected]>--- drivers/pci/msi.c | 34 +++++++++++++++++++++++++++++++--- drivers/pci/probe.c | 1 + include/linux/msi.h | 11 +++++++++++ include/linux/pci.h | 1 + 4 files changed, 44 insertions(+), 3 deletions(-)drivers/pci/msi.c +11
-0@@ -29,13 +29,41 @@ static int pci_msi_enable = 1;/* Arch hooks */ +int __weak arch_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; +
drivers/pci/msi.c static int pci_msi_enable = 1; · line 50
Quoted 10 lines
drivers/pci/msi.c +7
-2+irq_set_chip_data (desc->irq, chip); +return err; + } + +return -EINVAL ; +} -#ifndef arch_msi_check_device -int arch_msi_check_device (struct pci_dev *dev,int nvec,int type) +void __weak arch_teardown_msi_irq (unsigned int irq)
Please make a separate patch before this one for the conversion from the "#define arch_msi_check_device" strategy to the weak function. I think it's a good idea to use a weak function rather than the #define, but we need to remove the #define from arch/powerpc/include/asm/pci.h at the same time.
I don't think these patches touch arch_setup_msi_irqs() or arch_teardown_msi_irqs(), but I'd like to do the same with them just so we consistently use the same strategy to solve the same problem.
Quoted 37 lines
drivers/pci/msi.c +13
-1{ +struct msi_chip *chip =irq_get_chip_data (irq); + +if (chip && chip->teardown_irq) + chip->teardown_irq (chip, irq); +} + +int __weak arch_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 ; } -#endif #ifndef arch_setup_msi_irqs# define arch_setup_msi_irqs default_setup_msi_irqsdrivers/pci/probe.c +1
-0@@ -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 +0
-0
include/linux/msi.h int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type); · line 62
Quoted 5 lines
include/linux/msi.h +2
-0@@ -58,4 +58,15 @@ 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); +struct msi_chip { +struct module *owner;
Can the MSI chip driver be a loadable module? Does it need to be?
Quoted 25 lines
include/linux/msi.h +9
-0+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@@ -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
PATCH v2 1/8 PCI: Introduce new MSI chip infrastructure
Dear Bjorn Helgaas,
Quoted 10 lines · Bjorn Helgaas
drivers/pci/msi.c static int pci_msi_enable = 1; · line 50
Quoted 3 lines
drivers/pci/msi.c +1
-2-#ifndef arch_msi_check_device -int arch_msi_check_device (struct pci_dev *dev,int nvec,int type) +void __weak arch_teardown_msi_irq (unsigned int irq)Please make a separate patch before this one for the conversion from the "#define
arch_msi_check_device" strategy to the weak function. I think it's a good idea to use a weak function rather than the #define, but we need to remove the #define fromarch/powerpc/include/asm/pci.hat the same time.
Quoted 3 lines
I don't think these patches touch
arch_setup_msi_irqs()orarch_teardown_msi_irqs(), but I'd like to do the same with them just so we consistently use the same strategy to solve the same problem.
Ok, I've tried to refactor all those MSI operations that can be overriden on a per-architecture basis, it will be part of v3 to be sent soon.
Quoted 4 lines
include/linux/msi.h int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type); · line 62
Quoted 2 lines
include/linux/msi.h +2
-0+struct msi_chip { +struct module *owner;Can the MSI chip driver be a loadable module? Does it need to be?
In the case of the Marvell SoC, the MSI logic is part of the IRQ controller driver, so it's very unlikely that it can be built as a module.
However, in the case of the Tegra PCIe hardware, the MSI logic is part of the PCIe hardware itself. And the PCIe driver could potentially be built as a module (even though some other implementations details in the support of PCI on ARM currently prevents this, it should be possible in theory). And since this code comes from Thierry Redding, who was writing it with the Tegra PCIe in mind, it makes sense to assume the MSI code could be built as a module.
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
PATCH v2 2/8 PCI: Add registry of MSI chips
In reply to Thomas PetazzoniThis 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/pci/msi.c | 25 +++++++++++++++++++++++++ include/linux/msi.h | 11 +++++++++++ 2 files changed, 36 insertions(+)
drivers/pci/msi.c +25 -0
@@ -20,6 +20,7 @@#include <linux/errno.h> #include <linux/io.h> #include <linux/slab.h> +#include <linux/of.h> #include "pci.h"
@@ -70,6 +71,30 @@ int __weak arch_msi_check_device(struct pci_dev *dev, int nvec, int type)# define HAVE_DEFAULT_MSI_SETUP_IRQS #endif +#ifdef CONFIG_OF +static LIST_HEAD (msi_chip_list); +static DEFINE_MUTEX (msi_chip_mutex); + +void 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 *msi_chip_find_by_of_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 ; +} +#endif +#ifdef HAVE_DEFAULT_MSI_SETUP_IRQS int default_setup_msi_irqs (struct pci_dev *dev,int nvec,int type) {
include/linux/msi.h +11 -0
@@ -61,6 +61,8 @@ int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);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);
@@ -69,4 +71,13 @@ struct msi_chip { int nvec, int type); }; +#if defined(CONFIG_PCI_MSI ) && defined(CONFIG_OF ) +void msi_chip_add (struct msi_chip *chip); +struct msi_chip *msi_chip_find_by_of_node (struct device_node *of_node); +#else +static inline void msi_chip_add (struct msi_chip *chip) {} +static inline struct msi_chip * +msi_chip_find_by_of_node (struct device_node *of_node) {return NULL ; } +#endif +#endif /* LINUX_MSI_H */
Signature · 1 line
1.8.1.2
PATCH v2 2/8 PCI: Add registry of MSI chips
On Thu, Jun 06, 2013 at 06:41:22PM +0200, Thomas Petazzoni wrote:
[...]
include/linux/msi.h int arch_msi_check_device(struct pci_dev* dev, int nvec, int type); · line 64
Quoted 9 lines
include/linux/msi.h +1
-0@@ -61,6 +61,8 @@ int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);struct msi_chip {struct module *owner;struct device *dev; +struct device_node *of_node;
Shouldn't we be reusing the dev.of_node field to look this information up? On Tegra we do register the MSI chip and set the dev member to the PCIe host bridge device. Can't the same thing be done for the Armada?
I assume that the MSI controller provider will be instantiated from the device tree and hence there will already be a struct platform_device and therefore also a struct device associated with it which should have the of_node field set to the correct value automatically.
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/20130612/674c4214/attachment.sig>
Bjorn Helgaas
maintainerPATCH v2 2/8 PCI: Add registry of MSI chips
In reply to Thomas PetazzoniQuoted 39 lines · Thomas Petazzoni
This commit adds a very basic registry of
msi_chipstructures, so that an IRQ controller driver can register anmsi_chip, and a PCIe host controller can find it, based on a 'structdevice_node'.Signed-off-by Thomas Petazzoni <[email protected]>--- drivers/pci/msi.c | 25 +++++++++++++++++++++++++ include/linux/msi.h | 11 +++++++++++ 2 files changed, 36 insertions(+)drivers/pci/msi.c +14
-0@@ -20,6 +20,7 @@#include <linux/errno.h> #include <linux/io.h> #include <linux/slab.h> +#include <linux/of.h> #include "pci.h" @@ -70,6 +71,30 @@ int __weak arch_msi_check_device(struct pci_dev *dev, int nvec, int type)# define HAVE_DEFAULT_MSI_SETUP_IRQS #endif +#ifdef CONFIG_OF +static LIST_HEAD (msi_chip_list); +static DEFINE_MUTEX (msi_chip_mutex); + +void 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 *msi_chip_find_by_of_node (struct device_node *of_node) +{
drivers/pci/msi.c int __weak arch_msi_check_device(struct pci_dev *dev, int nvec, int type) · line 96
Quoted 10 lines
drivers/pci/msi.c +10
-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" )) +return c; + } + + return NULL; +} +#endif
I don't really understand why the msi_chip_list lives here, since nothing in drivers/pci/msi.c uses it. Seems like maybe something that could be in the OF code somewhere?
Quoted 34 lines
drivers/pci/msi.c +1
-0+#ifdef HAVE_DEFAULT_MSI_SETUP_IRQS int default_setup_msi_irqs (struct pci_dev *dev,int nvec,int type) {include/linux/msi.h +11
-0@@ -61,6 +61,8 @@ int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);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);@@ -69,4 +71,13 @@ struct msi_chip { int nvec, int type); }; +#if defined(CONFIG_PCI_MSI ) && defined(CONFIG_OF ) +void msi_chip_add (struct msi_chip *chip); +struct msi_chip *msi_chip_find_by_of_node (struct device_node *of_node); +#else +static inline void msi_chip_add (struct msi_chip *chip) {} +static inline struct msi_chip * +msi_chip_find_by_of_node (struct device_node *of_node) {return NULL ; } +#endif +#endif /* LINUX_MSI_H */ Signature · 1 line
1.8.1.2
PATCH v2 2/8 PCI: Add registry of MSI chips
Dear Bjorn Helgaas,
Quoted 3 lines · Bjorn Helgaas
I don't really understand why the
msi_chip_listlives here, since nothing indrivers/pci/msi.cuses it. Seems like maybe something that could be in the OF code somewhere?
Ok, I'll move this to drivers/of/, we'll see what Grant says about this. Will be part of v3.
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
PATCH v2 3/8 irqchip: armada-370-xp: properly request resources
In reply to Thomas PetazzoniInstead 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
@@ -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
PATCH v2 4/8 irqchip: armada-370-xp: implement MSI support
In reply to Thomas PetazzoniThis 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 | 166 +++++++++++++++++++++++++++++++++++- 1 file changed, 165 insertions(+), 1 deletion(-)
drivers/irqchip/irq-armada-370-xp.c +165 -1
@@ -22,6 +22,8 @@#include <linux/of_address.h> #include <linux/of_irq.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 +53,22 @@#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 0xFFFF0000static 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 DECLARE_BITMAP(msi_used, PCI_MSI_DOORBELL_NR); +static DEFINE_MUTEX (msi_used_lock); +static phys_addr_t msi_doorbell_addr; +#endif /* * In SMP mode:
@@ -87,6 +99,126 @@ 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_alloc_msi (void ) +{ +int hwirq; + +mutex_lock (&msi_used_lock); + hwirq =find_first_zero_bit (&msi_used,PCI_MSI_DOORBELL_NR ); +if (hwirq >=PCI_MSI_DOORBELL_NR ) + hwirq = -ENOSPC ; +else +set_bit (hwirq, msi_used); +mutex_unlock (&msi_used_lock); + +return hwirq; +} + +static void armada_370_xp_free_msi (int hwirq) +{ +mutex_lock (&msi_used_lock); +if (!test_bit (hwirq, msi_used)) +pr_err ("trying to free unused MSI#%d\n" , hwirq); +else +clear_bit (hwirq, msi_used); +mutex_unlock (&msi_used_lock); +} + +static int armada_370_xp_setup_msi_irq (struct msi_chip *chip, +struct pci_dev *pdev, +struct msi_desc *desc) +{ +struct msi_msg msg; +int hwirq, irq; + + hwirq =armada_370_xp_alloc_msi (); +if (hwirq <0 ) +return hwirq; + + irq =irq_create_mapping (armada_370_xp_msi_domain, hwirq); +if (!irq) { +armada_370_xp_free_msi (hwirq); +return -EINVAL ; + } + +irq_set_msi_desc (irq, desc); + + msg.address_lo = msi_doorbell_addr; + msg.address_hi =0 ; + msg.data =0xf00 | (hwirq +16 ); + +write_msi_msg (irq, &msg); +return 0 ; +} + +static void armada_370_xp_teardown_msi_irq (struct msi_chip *chip, +unsigned int irq) +{ +struct irq_data *d =irq_get_irq_data (irq); +armada_370_xp_free_msi (d->hwirq); +} + +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; + +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 +346,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 +428,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 +454,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
PATCH v2 4/8 irqchip: armada-370-xp: implement MSI support
Quoted 85 lines · 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.
Signed-off-by Thomas Petazzoni <[email protected]>--- drivers/irqchip/irq-armada-370-xp.c | 166 +++++++++++++++++++++++++++++++++++- 1 file changed, 165 insertions(+), 1 deletion(-)drivers/irqchip/irq-armada-370-xp.c +44
-0@@ -22,6 +22,8 @@#include <linux/of_address.h> #include <linux/of_irq.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 +53,22 @@#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 0xFFFF0000static 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 DECLARE_BITMAP(msi_used, PCI_MSI_DOORBELL_NR); +static DEFINE_MUTEX (msi_used_lock); +static phys_addr_t msi_doorbell_addr; +#endif /* * In SMP mode: @@ -87,6 +99,126 @@ 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_alloc_msi (void ) +{ +int hwirq; + +mutex_lock (&msi_used_lock); + hwirq =find_first_zero_bit (&msi_used,PCI_MSI_DOORBELL_NR ); +if (hwirq >=PCI_MSI_DOORBELL_NR ) + hwirq = -ENOSPC ; +else +set_bit (hwirq, msi_used); +mutex_unlock (&msi_used_lock); + +return hwirq; +} + +static void armada_370_xp_free_msi (int hwirq) +{ +mutex_lock (&msi_used_lock); +if (!test_bit (hwirq, msi_used)) +pr_err ("trying to free unused MSI#%d\n" , hwirq); +else +clear_bit (hwirq, msi_used); +mutex_unlock (&msi_used_lock); +} + +static int armada_370_xp_setup_msi_irq (struct msi_chip *chip, +struct pci_dev *pdev, +struct msi_desc *desc) +{ +struct msi_msg msg; +int hwirq, irq;
drivers/irqchip/irq-armada-370-xp.c static void armada_370_xp_irq_unmask(struct irq_data *d) · line 143
Quoted 10 lines
drivers/irqchip/irq-armada-370-xp.c +10
-0+ + hwirq =armada_370_xp_alloc_msi (); +if (hwirq <0 ) +return hwirq; + + irq =irq_create_mapping (armada_370_xp_msi_domain, hwirq); +if (!irq) { +armada_370_xp_free_msi (hwirq); +return -EINVAL ; + }
This looks like something that the irq_domain should handle for you. It would be good to have a variant of irq_create_mapping() that keeps track of available hwirqs, allocates a free one, and maps it all with one function call. I can see that being useful by other MSI interrupt controllers and would eliminate needing to open-code it above.
take a look at the irqdomain/test branch on git://git.secretlab.ca/git/linux
I'm hoping to push that series into v3.11 and it simplifies the irq_domain code quite a bit. It would be easy to build an irq_create_mapping() variant on top of that. Take a look at irq_create_direct_mapping() for inspiration (although that function does it the other way around. It allocates a virq and uses that number for the hwirq number)
drivers/irqchip/irq-armada-370-xp.c static void armada_370_xp_irq_unmask(struct irq_data *d) · line 189
Quoted 6 lines
drivers/irqchip/irq-armada-370-xp.c +6
-0+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 );
devm_kzalloc() so it gets freed on failure or unbind; although for this device you may not care.
Quoted 103 lines
drivers/irqchip/irq-armada-370-xp.c +65
-1+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; + +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 +346,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 +428,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 +454,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 · 7 lines
1.8.1.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 2 lines
Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd.
PATCH v2 4/8 irqchip: armada-370-xp: implement MSI support
Dear Grant Likely,
Quoted 23 lines · Grant Likely
Quoted 6 lines
drivers/irqchip/irq-armada-370-xp.c +6
-0+static int armada_370_xp_setup_msi_irq (struct msi_chip *chip, +struct pci_dev *pdev, +struct msi_desc *desc) +{ +struct msi_msg msg; +int hwirq, irq;drivers/irqchip/irq-armada-370-xp.c static void armada_370_xp_irq_unmask(struct irq_data *d) · line 143
Quoted 10 lines
drivers/irqchip/irq-armada-370-xp.c +10
-0+ + hwirq =armada_370_xp_alloc_msi (); +if (hwirq <0 ) +return hwirq; + + irq =irq_create_mapping (armada_370_xp_msi_domain, hwirq); +if (!irq) { +armada_370_xp_free_msi (hwirq); +return -EINVAL ; + }This looks like something that the
irq_domainshould handle for you. It would be good to have a variant ofirq_create_mapping()that keeps track of available hwirqs, allocates a free one, and maps it all with one function call. I can see that being useful by other MSI interrupt controllers and would eliminate needing to open-code it above.
Quoted 8 lines
take a look at the irqdomain/test branch on
git://git.secretlab.ca/git/linuxI'm hoping to push that series into v3.11 and it simplifies the
irq_domaincode quite a bit. It would be easy to build anirq_create_mapping()variant on top of that. Take a look atirq_create_direct_mapping()for inspiration (although that function does it the other way around. It allocates a virq and uses that number for the hwirq number)
Ok, I've implemented something according to your idea. Will be part of the v3 of this series. For reference, the piece of code I've added in irqdomain.c looks like:
/**
+ * 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)
+{
+ unsigned int virq;
+ irq_hw_number_t hwirq;
+
+ pr_debug("irq_alloc_mapping(0x%p)\n", domain);
+
+ if (domain == NULL)
+ domain = irq_default_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;
+ }
+
+ virq = irq_alloc_desc_from(1, of_node_to_nid(domain->of_node));
+ if (virq <= 0) {
+ pr_debug("-> virq allocation failed\n");
+ return 0;
+ }
+
+ if (irq_domain_associate(domain, virq, hwirq)) {
+ irq_free_desc(virq);
+ return 0;
+ }
+
+ pr_debug("irq %lu on domain %s mapped to virtual irq %u\n",
+ hwirq, of_node_full_name(domain->of_node), virq);
+
+ *out_hwirq = hwirq;
+
+ return virq;
+}
+EXPORT_SYMBOL_GPL(irq_alloc_mapping);
+
+/**
Quoted 9 lines
drivers/irqchip/irq-armada-370-xp.c static void armada_370_xp_irq_unmask(struct irq_data *d) · line 189
Quoted 6 lines
drivers/irqchip/irq-armada-370-xp.c +6
-0+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 );
devm_kzalloc()so it gets freed on failure or unbind; although for this device you may not care.
Unfortunately, we don't have access to a platform_device by the time the IRQ controller driver is initialized. Thierry Redding suggested to use of_find_device_by_node(), but it returns NULL. I guess it's because this IRQ controller driver is initialized much before the platform_device structures are created.
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH v2 4/8 irqchip: armada-370-xp: implement MSI support
Quoted 51 lines · Thomas Petazzoni
Dear Grant Likely,
Quoted 23 lines · Grant Likely
Quoted 6 lines
drivers/irqchip/irq-armada-370-xp.c +6
-0+static int armada_370_xp_setup_msi_irq (struct msi_chip *chip, +struct pci_dev *pdev, +struct msi_desc *desc) +{ +struct msi_msg msg; +int hwirq, irq;drivers/irqchip/irq-armada-370-xp.c static void armada_370_xp_irq_unmask(struct irq_data *d) · line 143
Quoted 10 lines
drivers/irqchip/irq-armada-370-xp.c +10
-0+ + hwirq =armada_370_xp_alloc_msi (); +if (hwirq <0 ) +return hwirq; + + irq =irq_create_mapping (armada_370_xp_msi_domain, hwirq); +if (!irq) { +armada_370_xp_free_msi (hwirq); +return -EINVAL ; + }This looks like something that the
irq_domainshould handle for you. It would be good to have a variant ofirq_create_mapping()that keeps track of available hwirqs, allocates a free one, and maps it all with one function call. I can see that being useful by other MSI interrupt controllers and would eliminate needing to open-code it above.Quoted 8 lines
take a look at the irqdomain/test branch on
git://git.secretlab.ca/git/linuxI'm hoping to push that series into v3.11 and it simplifies the
irq_domaincode quite a bit. It would be easy to build anirq_create_mapping()variant on top of that. Take a look atirq_create_direct_mapping()for inspiration (although that function does it the other way around. It allocates a virq and uses that number for the hwirq number)Ok, I've implemented something according to your idea. Will be part of the v3 of this series. For reference, the piece of code I've added in
irqdomain.clooks like:/**
+ *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.
+ */
Review
Quoted 10 lines
(quoted diff) +10
-0+unsigned int irq_alloc_mapping(struct irq_domain *domain, + irq_hw_number_t *out_hwirq) +{ + unsigned int virq; + irq_hw_number_t hwirq; + + pr_debug("irq_alloc_mapping(0x%p)\n", domain); + + if (domain == NULL) + domain = irq_default_domain;
Drop the above 2 lines. You absolutely must know what irq_domain you want to operate on when calling this function. There is no situation where the default domain is what should be used.
Review
Quoted 4 lines
(quoted diff) +4
-0+ + for (hwirq = 0; hwirq < domain->hwirq_max; hwirq++) + if (!irq_find_mapping(domain, hwirq)) + break;
Ugh. This will be slow on domains with a high hwirq_max and low revmap_size since all the lookups will go out to the radix tree. Blech. Not much to do about it though at this point without implementing some kind of fast lookup path. To do it right would require iterating over the radix tree looking for a hole.
Quoted 6 lines
drivers/irqchip/irq-armada-370-xp.c +6
-0+ +if (hwirq == domain->hwirq_max) { +pr_debug ("-> no available hwirq found\n" ); +return 0 ; + } +
drivers/irqchip/irq-armada-370-xp.c static void armada_370_xp_irq_unmask(struct irq_data *d) · line 217
Quoted 10 lines
drivers/irqchip/irq-armada-370-xp.c +10
-0+ virq =irq_alloc_desc_from (1 ,of_node_to_nid (domain->of_node)); +if (virq <=0 ) { +pr_debug ("-> virq allocation failed\n" ); +return 0 ; + } + +if (irq_domain_associate (domain, virq, hwirq)) { +irq_free_desc (virq); +return 0 ; + }
Once a free hwirq has been found, it would be better to call irq_create_mapping() directly rather than open coding it.
Quoted 11 lines
(quoted diff) +11
-0+ + pr_debug("irq %lu on domain %s mapped to virtual irq %u\n", + hwirq, of_node_full_name(domain->of_node), virq); + + *out_hwirq = hwirq; + + return virq; +} +EXPORT_SYMBOL_GPL(irq_alloc_mapping); + +/**
PATCH v2 4/8 irqchip: armada-370-xp: implement MSI support
Dear Grant Likely,
Quoted 6 lines · Grant Likely
Review
Quoted 2 lines
(quoted diff) +2
-0+ if (domain == NULL) + domain = irq_default_domain;Drop the above 2 lines. You absolutely must know what
irq_domainyou want to operate on when calling this function. There is no situation where the default domain is what should be used.
Sure, makes sense.
Quoted 10 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ + for (hwirq = 0; hwirq < domain->hwirq_max; hwirq++) + if (!irq_find_mapping(domain, hwirq)) + break;Ugh. This will be slow on domains with a high
hwirq_maxand lowrevmap_sizesince all the lookups will go out to the radix tree. Blech. Not much to do about it though at this point without implementing some kind of fast lookup path. To do it right would require iterating over the radix tree looking for a hole.
So to conclude you would leave it as I proposed for now?
An option is to make irq_alloc_mapping() work only on linear domains, where hwirq_max == revmap_size, and return an error otherwise.
Quoted 2 lines
Once a free hwirq has been found, it would be better to call
irq_create_mapping()directly rather than open coding it.
Thanks, will do.
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH v2 4/8 irqchip: armada-370-xp: implement MSI support
In reply to Thomas PetazzoniOn Thu, Jun 06, 2013 at 06:41:24PM +0200, Thomas Petazzoni wrote:
[...]
drivers/irqchip/irq-armada-370-xp.c static int __init armada_370_xp_mpic_of_init(struct device_node *node, · line 458
Quoted 6 lines
drivers/irqchip/irq-armada-370-xp.c +2
-0@@ -292,6 +454,8 @@ static int __init armada_370_xp_mpic_of_init(struct device_node *node,#endif +armada_370_xp_msi_init (node); +
So I see that you don't have access to the original platform device here, but you could use of_find_device_by_node() to obtain it and pass that into armada_370_xp_msi_init() in order to set the msi_chip.dev field. Or you could do the lookup in armada_370_xp_msi_init() if you don't need it for anything else in armada_370_xp_mpic_of_init().
Doing the above will also allow you to use devm_kzalloc() as Grant suggested in his reply.
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/20130612/6af6e30d/attachment.sig>
PATCH v2 4/8 irqchip: armada-370-xp: implement MSI support
Dear Thierry Reding,
Quoted 6 lines · Thierry Reding
On Thu, Jun 06, 2013 at 06:41:24PM +0200, Thomas Petazzoni wrote:
[...]Quoted 4 lines
(quoted diff) +0
-0@@ -292,6 +454,8 @@ static int __init armada_370_xp_mpic_of_init(struct device_node *node, #endif
Quoted 8 lines
drivers/irqchip/irq-armada-370-xp.c static int __init armada_370_xp_mpic_of_init(struct device_node *node, · line 458
Quoted 2 lines
drivers/irqchip/irq-armada-370-xp.c +2
-0+armada_370_xp_msi_init (node); +So I see that you don't have access to the original platform device here, but you could use
of_find_device_by_node()to obtain it and pass that intoarmada_370_xp_msi_init()in order to set the msi_chip.dev field. Or you could do the lookup inarmada_370_xp_msi_init()if you don't need it for anything else inarmada_370_xp_mpic_of_init().
As I replied to Grant, of_find_device_by_node() returns NULL, I believe because the all IRQ controller driver initialization is done pretty early, before the of_platform_populate() call is made, so there is no platform_device associated with the IRQ controller node at the time the armada_370_xp_mpic_of_init() function is called.
Do you see another approach, especially in relation to your comment on PATCH 2/8 ?
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
PATCH v2 4/8 irqchip: armada-370-xp: implement MSI support
Quoted 18 lines · Thomas Petazzoni
Dear Thierry Reding,
Quoted 6 lines · Thierry Reding
On Thu, Jun 06, 2013 at 06:41:24PM +0200, Thomas Petazzoni wrote:
[...]Quoted 4 lines
(quoted diff) +0
-0@@ -292,6 +454,8 @@ static int __init armada_370_xp_mpic_of_init(struct device_node *node, #endifQuoted 8 lines
drivers/irqchip/irq-armada-370-xp.c static int __init armada_370_xp_mpic_of_init(struct device_node *node, · line 458
Quoted 2 lines
drivers/irqchip/irq-armada-370-xp.c +2
-0+armada_370_xp_msi_init (node); +So I see that you don't have access to the original platform device here, but you could use
of_find_device_by_node()to obtain it and pass that intoarmada_370_xp_msi_init()in order to set the msi_chip.dev field. Or you could do the lookup inarmada_370_xp_msi_init()if you don't need it for anything else inarmada_370_xp_mpic_of_init().
Quoted 8 lines
As I replied to Grant,
of_find_device_by_node()returns NULL, I believe because the all IRQ controller driver initialization is done pretty early, before theof_platform_populate()call is made, so there is noplatform_deviceassociated with the IRQ controller node at the time thearmada_370_xp_mpic_of_init()function is called.Do you see another approach, especially in relation to your comment on PATCH 2/8 ?
Hmmm, that's too bad. The only other possibility that I see is that you could associate the struct device at a later point when it becomes available, but looking at the irqchip driver it doesn't look like you get notification of that either. I suppose you could add a platform_driver to it and hook things up in its .probe() callback, but I'm not sure if that's in line with how the irqchip was designed. Adding Grant Likely and Thomas Gleixner on Cc, maybe they have better advice.
Looking at other irqchip drivers I find it a bit odd to see how they're structured, though. We've been preaching for years that drivers should be well-encapsulated and told everybody it was bad to use globals and they should be associating driver-specific data with each instance of a device. Then comes along irqchip and all of a sudden it's okay to use globals again. It feels a bit fragile.
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/20130618/51731f1c/attachment.sig>
PATCH v2 4/8 irqchip: armada-370-xp: implement MSI support
Dear Thierry Reding,
Quoted 9 lines · Thierry Reding
Quoted 8 lines
As I replied to Grant,
of_find_device_by_node()returns NULL, I believe because the all IRQ controller driver initialization is done pretty early, before theof_platform_populate()call is made, so there is noplatform_deviceassociated with the IRQ controller node at the time thearmada_370_xp_mpic_of_init()function is called.Do you see another approach, especially in relation to your comment on PATCH 2/8 ?
Quoted 7 lines
Hmmm, that's too bad. The only other possibility that I see is that you could associate the struct device at a later point when it becomes available, but looking at the irqchip driver it doesn't look like you get notification of that either. I suppose you could add a
platform_driverto it and hook things up in its .probe() callback, but I'm not sure if that's in line with how the irqchip was designed. Adding Grant Likely and Thomas Gleixner on Cc, maybe they have better advice.
If we do hook the MSI stuff in a ->probe() callback, then we'd have a dependency between the ->probe() of the PCIe driver and the ->probe() of the IRQ controller driver. In order for the PCIe ->probe() to succeed, it needs the MSI controller to be registered, which wouldn't appear until the ->probe() of the IRQ controller driver gets called. A typical case of platform device dependency where the two platform devices don't have a bus -> child dependency. Could be handled by a -EPROBE_DEFER trick, though.
Quoted 6 lines
Looking at other irqchip drivers I find it a bit odd to see how they're structured, though. We've been preaching for years that drivers should be well-encapsulated and told everybody it was bad to use globals and they should be associating driver-specific data with each instance of a device. Then comes along irqchip and all of a sudden it's okay to use globals again. It feels a bit fragile.
Yes, I've seen this as well, and I'm thinking of doing some improvements in this area if there's some interest. But I believe this is fairly separate from the specific discussion of this patch set.
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
PATCH v2 5/8 arm: mvebu: the MPIC now provides MSI controller features
In reply to Thomas PetazzoniAdds 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
@@ -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
PATCH v2 6/8 pci: mvebu: add support for MSI
In reply to Thomas PetazzoniThis 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 | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
drivers/pci/host/pci-mvebu.c +21 -0
@@ -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;
@@ -690,6 +692,8 @@ static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys)if (!bus)return NULL ; + bus->msi = pcie->msi; +pci_scan_child_bus (bus);return bus;
@@ -755,6 +759,21 @@ mvebu_pcie_map_registers(struct platform_device *pdev,return devm_request_and_ioremap (&pdev->dev, ®s); } +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 =msi_chip_find_by_of_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 +898,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
Bjorn Helgaas
maintainerPATCH v2 6/8 pci: mvebu: add support for MSI
Quoted 33 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_chipassociated to the PCIe controller thanks to the msi-parent DT property, and stores this reference in thepci_busstructure. This is enough to let the Linux PCI core use the functions ofmsi_chipto setup and teardown MSIs.Signed-off-by Thomas Petazzoni <[email protected]>--- drivers/pci/host/pci-mvebu.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)drivers/pci/host/pci-mvebu.c +2
-0@@ -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;
drivers/pci/host/pci-mvebu.c static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) · line 695
Quoted 4 lines
drivers/pci/host/pci-mvebu.c +1
-0@@ -690,6 +692,8 @@ static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys)if (!bus)return NULL ; + bus->msi = pcie->msi;
Ideally, arches could use pci_scan_root_bus(), which does the pci_scan_child_bus() itself, so I never like to add arch-specific code between pci_create_root_bus() and pci_scan_child_bus().
But you should be able to accomplish this by setting bus->msi in a pcibios_add_bus() hook. There is a corresponding pcibios_remove_bus() hook that might be useful for deallocating the msi_chip. I'm not too clear on how you manage the msi_chip lifetime -- I see the alloc in armada_370_xp_msi_init(), but I'm not sure whether you ever deallocate it or how you know when it would be safe to do so.
Quoted 37 lines
drivers/pci/host/pci-mvebu.c +17
-0pci_scan_child_bus (bus);return bus;@@ -755,6 +759,21 @@ mvebu_pcie_map_registers(struct platform_device *pdev,return devm_request_and_ioremap (&pdev->dev, ®s); } +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 =msi_chip_find_by_of_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 +898,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
PATCH v2 7/8 arm: mvebu: indicate that this platform supports MSI
In reply to Thomas PetazzoniNow 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
@@ -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
PATCH v2 8/8 arm: mvebu: link PCIe controllers to the MSI controller
In reply to Thomas PetazzoniThis 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
@@ -178,6 +178,7 @@ #address-cells = <3>; #size-cells = <2>; + msi-parent = <&mpic>; bus-range = <0x00 0xff>; reg = <0x40000 0x2000>, <0x80000 0x2000>;
arch/arm/boot/dts/armada-xp-mv78230.dtsi +1 -0
@@ -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
@@ -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
@@ -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
v2 cover MSI support for Marvell EBU PCIe driver
In reply to Thomas PetazzoniQuoted 2 lines · Thomas Petazzoni
This set of patches currently applies on top of the current "for-next" branch of Jason Cooper's repository.
Please be aware that for-next is *not* stable. I rebuild it every time I pull new stuff in. It's much more helpful to list only the branches you need to build/test.
In this case I imagine it's mvebu/pcie...
thx,
Jason.
v2 cover MSI support for Marvell EBU PCIe driver
Dear Jason Cooper,
Quoted 3 lines · Jason Cooper
Please be aware that for-next is *not* stable. I rebuild it every time I pull new stuff in. It's much more helpful to list only the branches you need to build/test.
Yes, I'm aware that for-next is not stable. Since the previous iteration of this patch series generated a lot of discussion, I was assuming there would be additional discussions on this new iteration, and that it would certainly not be the final version. So for ease of work, I've just based it on the branch that 'contains everything'.
For sure, if I get the proper ACKs and acceptance of the general idea, I'll go ahead and send a proper version based on the correct branches.
You suggested mvebu/pcie as a base, but it means I also need to merge mvebu/of_pci before that, and maybe also mvebu/pcie_bridge to have the latest changes in the PCIe driver, etc. Which is why I just went the easy/lazy way and did everything based on for-next :)
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
v2 cover MSI support for Marvell EBU PCIe driver
Quoted 8 lines · Thomas Petazzoni
Dear Jason Cooper,
Quoted 3 lines · Jason Cooper
Please be aware that for-next is *not* stable. I rebuild it every time I pull new stuff in. It's much more helpful to list only the branches you need to build/test.
Quoted 5 lines
Yes, I'm aware that for-next is not stable. Since the previous iteration of this patch series generated a lot of discussion, I was assuming there would be additional discussions on this new iteration, and that it would certainly not be the final version. So for ease of work, I've just based it on the branch that 'contains everything'.
Ahh, ok, I've done the same. Thanks for the clarification.
thx,
Jason.
v2 cover MSI support for Marvell EBU PCIe driver
In reply to Thomas PetazzoniThomas,
Quoted 59 lines · Thomas Petazzoni
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 2 follows the RFC version sent on March, 26th 2013. This is based on work done by Lior Amsalem <[email protected]>.
The patches do the following:
* Patch 1 comes from Thierry Redding, and adds a minimal
msi_chipinfrastructure.This needs to be reviewed/acked by Bjorn Helgaas.
* Patch 2 extends the
msi_chipinfrastructure with a small registry, so that one driver can register anmsi_chip, and another driver can find themsi_chipassociated to a particular Device Tree node.This needs to be reviewed/acked by Bjorn Helgaas.
* Patch 3 makes some not very interesting preparation in the Armada 370/XP IRQ controller driver.
* Patch 4 implements the MSI support in the Armada 370/XP IRQ controller driver. It registers an
msi_chipusing themsi_chip_add()function added in PATCH 2.* Patch 5 adjust the Armada 370/XP Device Tree to indicate that the MPIC is not only an interrupt-controller, but also an msi-controller.
* Patch 6 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 thepci_busstructure before the bus gets enumerated.This needs to be reviewed/acked by Bjorn Helgaas.
* Patch 7 tunes
Kconfigto indicate that Armada 370/XP supports MSI.* Patch 8 adjusts the Armada 370/XP Device Tree to add the msi-parent properties in the PCIe controller nodes.
This set of patches currently applies on top of the current "for-next" branch of Jason Cooper's repository.
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
Quoted 9 lines
Thomas Petazzoni (7):
PCI: Add registry of MSI chips
irqchip: armada-370-xp: properly request resources
irqchip: armada-370-xp: implement MSI support
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
Is there a reason why the following breakdown wouldn't work?
Quoted 5 lines
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 +
through mvebu/arm-soc
Quoted 1 line
arch/arm/mach-mvebu/Kconfig | 1 +
through mvebu/arm-soc after the other three have landed (v3.11-rc1)
Quoted 1 line
drivers/irqchip/irq-armada-370-xp.c | 186 ++++++++++++++++++++++++++++++-
through tglx
Quoted 5 lines
drivers/pci/host/pci-mvebu.c | 21 ++++ drivers/pci/msi.c | 59 +++++++++- drivers/pci/probe.c | 1 + include/linux/msi.h | 22 ++++ include/linux/pci.h | 1 +
through Bjorn
I think we should view the manner in which we brought in the initial mvebu-pcie series (all through arm-soc) as the exception, not the rule. I have no problem, and in fact, prefer to have them reviewed as a series, but if at *all* possible, the series should be structured so relevant maintainers can pick up the relevant patches into their trees.
thx,
Jason.
v2 cover MSI support for Marvell EBU PCIe driver
Dear Jason Cooper,
Quoted 13 lines · Jason Cooper
Quoted 12 lines
Thierry Reding (1):
PCI: Introduce new MSI chip infrastructureThomas Petazzoni (7):
PCI: Add registry of MSI chips
irqchip: armada-370-xp: properly request resources
irqchip: armada-370-xp: implement MSI support
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
Quoted 1 line
Is there a reason why the following breakdown wouldn't work?
No, it should work. And it's actually possible with how the patches are organized currently.
Quoted 7 lines
Quoted 5 lines
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 +through mvebu/arm-soc
Patches 5 and 8.
Quoted 4 lines
Quoted 2 lines
arch/arm/mach-mvebu/Kconfig | 1 +through mvebu/arm-soc after the other three have landed (v3.11-rc1)
Patch 7.
Quoted 3 lines
Quoted 1 line
drivers/irqchip/irq-armada-370-xp.c | 186 ++++++++++++++++++++++++++++++-through tglx
Patches 3 and 4.
Quoted 8 lines
Quoted 6 lines
drivers/pci/host/pci-mvebu.c | 21 ++++ drivers/pci/msi.c | 59 +++++++++- drivers/pci/probe.c | 1 + include/linux/msi.h | 22 ++++ include/linux/pci.h | 1 +through Bjorn
Patches 1, 2 and 6.
So as you can see, the patches are already broken down in a way that allows each maintainer to pick its part. But I admit I could probably reorder them in the following way:
1. PCI: Introduce new MSI chip infrastructure
2. PCI: Add registry of MSI chips
3. pci: mvebu: add support for MSI
4. irqchip: armada-370-xp: properly request resources
5. irqchip: armada-370-xp: implement MSI support
6. arm: mvebu: the MPIC now provides MSI controller features
7. arm: mvebu: link PCIe controllers to the MSI controller
8. arm: mvebu: indicate that this platform supports MSI
1-3 through Bjorn, 4-5 through tglx, 6-8 through you.
The only problem that I see is that 'irqchip: armada-370-xp: implement MSI support' (which goes through tglx) has a build dependency on 'PCI:
Add registry of MSI chips' (which goes through Bjorn). This is due to the IRQ controller using the msi_chip_add() function that is introduced earlier in the PCI core.
How would you solve that?
In any case, at this point I'd like first to have the ACK from Bjorn Helgaas and Arnd Bergmann on the general approach.
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
v2 cover MSI support for Marvell EBU PCIe driver
Quoted 16 lines · Thomas Petazzoni
Dear Jason Cooper,
Quoted 13 lines · Jason Cooper
Quoted 12 lines
Thierry Reding (1):
PCI: Introduce new MSI chip infrastructureThomas Petazzoni (7):
PCI: Add registry of MSI chips
irqchip: armada-370-xp: properly request resources
irqchip: armada-370-xp: implement MSI support
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
Quoted 4 lines
Quoted 1 line
Is there a reason why the following breakdown wouldn't work?
No, it should work. And it's actually possible with how the patches are organized currently.
Ok. good.
Quoted 47 lines
Quoted 7 lines
Quoted 5 lines
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 +through mvebu/arm-soc
Patches 5 and 8.
Quoted 4 lines
Quoted 2 lines
arch/arm/mach-mvebu/Kconfig | 1 +through mvebu/arm-soc after the other three have landed (v3.11-rc1)
Patch 7.
Quoted 3 lines
Quoted 1 line
drivers/irqchip/irq-armada-370-xp.c | 186 ++++++++++++++++++++++++++++++-through tglx
Patches 3 and 4.
Quoted 8 lines
Quoted 6 lines
drivers/pci/host/pci-mvebu.c | 21 ++++ drivers/pci/msi.c | 59 +++++++++- drivers/pci/probe.c | 1 + include/linux/msi.h | 22 ++++ include/linux/pci.h | 1 +through Bjorn
Patches 1, 2 and 6.
So as you can see, the patches are already broken down in a way that allows each maintainer to pick its part. But I admit I could probably reorder them in the following way:
1. PCI: Introduce new MSI chip infrastructure
2. PCI: Add registry of MSI chips
3. pci: mvebu: add support for MSI
4. irqchip: armada-370-xp: properly request resources
5. irqchip: armada-370-xp: implement MSI support
6. arm: mvebu: the MPIC now provides MSI controller features
7. arm: mvebu: link PCIe controllers to the MSI controller
8. arm: mvebu: indicate that this platform supports MSI
Quoted 7 lines
1-3 through Bjorn, 4-5 through tglx, 6-8 through you.
The only problem that I see is that 'irqchip: armada-370-xp: implement MSI support' (which goes through tglx) has a build dependency on 'PCI:
Add registry of MSI chips' (which goes through Bjorn). This is due to the IRQ controller using themsi_chip_add()function that is introduced earlier in the PCI core.
Yeah, that's what I was afraid of...
Quoted 1 line
How would you solve that?
Let me take a closer look at it later today or this weekend. I just wanted to make sure that we always defer to the appropriate maintainers first, and merge all through arm-soc as a last resort.
Quoted 2 lines
In any case, at this point I'd like first to have the ACK from Bjorn Helgaas and Arnd Bergmann on the general approach.
Of course.
thx,
Jason.
v2 cover MSI support for Marvell EBU PCIe driver
Dear Jason Cooper,
Quoted 8 lines · Jason Cooper
Quoted 7 lines
1-3 through Bjorn, 4-5 through tglx, 6-8 through you.
The only problem that I see is that 'irqchip: armada-370-xp: implement MSI support' (which goes through tglx) has a build dependency on 'PCI:
Add registry of MSI chips' (which goes through Bjorn). This is due to the IRQ controller using themsi_chip_add()function that is introduced earlier in the PCI core.
Quoted 7 lines
Yeah, that's what I was afraid of...
Quoted 1 line
How would you solve that?
Let me take a closer look at it later today or this weekend. I just wanted to make sure that we always defer to the appropriate maintainers first, and merge all through arm-soc as a last resort.
Now that I think of it, there is no problem in fact. Until you apply the patch 'arm: mvebu: indicate that this platform supports MSI', which modifies Kconfig to say that mvebu supports MSI, there is no way for the user to enable CONFIG_PCI_MSI.
And since in the IRQ controller driver the MSI code is inside a #ifdef CONFIG_PCI_MSI, this driver will build perfectly fine, as long as 'arm:
mvebu: indicate that this platform supports MSI' is not merged before 'PCI: Add registry of MSI chips'.
So we can perfectly let Bjorn take the three PCI patches, tglx take the two IRQ controller patches, and you take the ARM stuff. The only thing is that the ARM stuff should not surface until both the PCI patches and the IRQ controller patches have landed in mainline.
But well, before even thinking of merging all this, I'd like to hear from Bjorn about what he thinks of those patches.
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
v2 cover MSI support for Marvell EBU PCIe driver
In reply to Thomas PetazzoniBjorn,
Quoted 13 lines · Thomas Petazzoni
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 2 follows the RFC version sent on March, 26th 2013. This is based on work done by Lior Amsalem <[email protected]>.
The patches do the following:
* Patch 1 comes from Thierry Redding, and adds a minimal
msi_chipinfrastructure.
Quoted 7 lines
This needs to be reviewed/acked by Bjorn Helgaas.
* Patch 2 extends the
msi_chipinfrastructure with a small registry, so that one driver can register anmsi_chip, and another driver can find themsi_chipassociated to a particular Device Tree node.This needs to be reviewed/acked by Bjorn Helgaas.
Could you have a look at those two patches and let me know what you think?
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