* [PATCH 0/2] Support multiple MSI interrupts on FSL-MPIC
@ 2014-11-03 16:18 Johannes Thumshirn
2014-11-03 16:18 ` [PATCH 1/2] irqdomain: add support for creating a continous mapping Johannes Thumshirn
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Johannes Thumshirn @ 2014-11-03 16:18 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Scott Wood, Michael Ellerman
Cc: Johannes Thumshirn, linuxppc-dev
This series adds support for multiple MSI interrupts on FSL's MPIC. The patch
series was originally done by Sebastian Andrzej Siewior <bigeasy@linutronix.de>
and re-applied and tested by me.
I dodn't know whether it is OK to put my name on it or not. So if Sebastian has
a problem with it I'll of cause change it immediately. It was just convenient to
use my name in the git commit but I don't want to do any kind of copyright
infrigement.
Johannes Thumshirn (2):
irqdomain: add support for creating a continous mapping
powerpc: msi: fsl: add support for multiple MSI interrupts
arch/powerpc/kernel/msi.c | 4 --
arch/powerpc/platforms/cell/axon_msi.c | 3 ++
arch/powerpc/platforms/powernv/pci.c | 3 ++
arch/powerpc/platforms/pseries/msi.c | 3 ++
arch/powerpc/sysdev/fsl_msi.c | 22 ++++++---
arch/powerpc/sysdev/mpic_pasemi_msi.c | 3 ++
arch/powerpc/sysdev/mpic_u3msi.c | 3 ++
arch/powerpc/sysdev/ppc4xx_msi.c | 2 +
include/linux/irqdomain.h | 10 +++-
kernel/irq/irqdomain.c | 85 ++++++++++++++++++++++++++--------
10 files changed, 106 insertions(+), 32 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] irqdomain: add support for creating a continous mapping
2014-11-03 16:18 [PATCH 0/2] Support multiple MSI interrupts on FSL-MPIC Johannes Thumshirn
@ 2014-11-03 16:18 ` Johannes Thumshirn
2014-11-03 22:18 ` Scott Wood
2014-11-03 16:18 ` [PATCH 2/2] powerpc: msi: fsl: add support for multiple MSI interrupts Johannes Thumshirn
2014-11-03 17:51 ` [PATCH 0/2] Support multiple MSI interrupts on FSL-MPIC Scott Wood
2 siblings, 1 reply; 8+ messages in thread
From: Johannes Thumshirn @ 2014-11-03 16:18 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Scott Wood, Michael Ellerman
Cc: Johannes Thumshirn, linuxppc-dev
A MSI device may have multiple interrupts. That means that the
interrupts numbers should be continuos so that pdev->irq refers to the
first interrupt, pdev->irq + 1 to the second and so on.
This patch adds support for continuous allocation of virqs for a range
of hwirqs. The function is based on irq_create_mapping() but due to the
number argument there is very little in common now.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
include/linux/irqdomain.h | 10 ++++--
kernel/irq/irqdomain.c | 85 ++++++++++++++++++++++++++++++++++++-----------
2 files changed, 73 insertions(+), 22 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index b0f9d16..75662f3 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -175,8 +175,14 @@ extern void irq_domain_associate_many(struct irq_domain *domain,
extern void irq_domain_disassociate(struct irq_domain *domain,
unsigned int irq);
-extern unsigned int irq_create_mapping(struct irq_domain *host,
- irq_hw_number_t hwirq);
+extern unsigned int irq_create_mapping_block(struct irq_domain *host,
+ irq_hw_number_t hwirq, unsigned int num);
+static inline unsigned int irq_create_mapping(struct irq_domain *host,
+ irq_hw_number_t hwirq)
+{
+ return irq_create_mapping_block(host, hwirq, 1);
+}
+
extern void irq_dispose_mapping(unsigned int virq);
/**
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 6534ff6..fba488f 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -375,27 +375,61 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
}
EXPORT_SYMBOL_GPL(irq_create_direct_mapping);
+static int irq_check_continuous_mapping(struct irq_domain *domain,
+ irq_hw_number_t hwirq, unsigned int num)
+{
+ int virq;
+ int i;
+
+ virq = irq_find_mapping(domain, hwirq);
+
+ for (i = 1; i < num; i++) {
+ unsigned int next;
+
+ next = irq_find_mapping(domain, hwirq + i);
+ if (next == virq + i)
+ continue;
+
+ pr_err("irq: invalid partial mapping. First hwirq %lu maps to "
+ "%d and\n", hwirq, virq);
+ pr_err("irq: +%d hwirq (%lu) maps to %d but should be %d.\n",
+ i, hwirq + i, next, virq + i);
+ return -EINVAL;
+ }
+
+ pr_debug("-> existing mapping on virq %d\n", virq);
+ return virq;
+}
+
+
/**
- * irq_create_mapping() - Map a hardware interrupt into linux irq space
+ * irq_create_mapping_block() - Map multiple hardware interrupts
* @domain: domain owning this hardware interrupt or NULL for default domain
* @hwirq: hardware irq number in that domain space
+ * @num: number of interrupts
+ *
+ * Maps a hwirq to a newly allocated virq. If num is greater than 1 then num
+ * hwirqs (hwirq ... hwirq + num - 1) will be mapped and virq will be
+ * continuous.
+ * Returns the first linux virq number.
*
- * Only one mapping per hardware interrupt is permitted. Returns a linux
- * irq number.
* If the sense/trigger is to be specified, set_irq_type() should be called
* on the number returned from that call.
*/
-unsigned int irq_create_mapping(struct irq_domain *domain,
+unsigned int irq_create_mapping_block(struct irq_domain *domain,
irq_hw_number_t hwirq)
{
unsigned int hint;
int virq;
+ int node;
+ int i;
- pr_debug("irq_create_mapping(0x%p, 0x%lx)\n", domain, hwirq);
+ pr_debug("%s(0x%p, 0x%lx, %d)\n", __func__, domain, hwirq, num);
/* Look for default domain if nececssary */
- if (domain == NULL)
+ if (!domain && num == 1)
domain = irq_default_domain;
+
if (domain == NULL) {
WARN(1, "%s(, %lx) called with NULL domain\n", __func__, hwirq);
return 0;
@@ -403,35 +437,46 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
pr_debug("-> using domain @%p\n", domain);
/* Check if mapping already exists */
- virq = irq_find_mapping(domain, hwirq);
- if (virq) {
- pr_debug("-> existing mapping on virq %d\n", virq);
- return virq;
+ for (i = 0; i < num; i++) {
+ virq = irq_find_mapping(domain, hwirq + i);
+ if (virq != NO_IRQ) {
+ if (i == 0)
+ return irq_check_continuous_mapping(domain,
+ hwirq, num);
+ pr_err("irq: hwirq %ld has no mapping but hwirq %ld "
+ "maps to virq %d. This can't be a block\n",
+ hwirq, hwirq + i, virq);
+ return -EINVAL;
+ }
}
+ node = of_node_to_nid(domain->of_node);
+
/* Allocate a virtual interrupt number */
hint = hwirq % nr_irqs;
if (hint == 0)
hint++;
- virq = irq_alloc_desc_from(hint, of_node_to_nid(domain->of_node));
- if (virq <= 0)
- virq = irq_alloc_desc_from(1, of_node_to_nid(domain->of_node));
+ virq = irq_alloc_desc_from(hint, node);
+ if (virq <= 0 && hint != 1)
+ virq = irq_alloc_desc_from(1, 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;
+ irq_domain_associate_many(domain, virq, hwirq, num);
+ if (num == 1) {
+ pr_debug("irq %lu on domain %s mapped to virtual irq %u\n"
+ hwirq, of_node_full_name(domain->of_node), virq);
+ return virq;
}
- pr_debug("irq %lu on domain %s mapped to virtual irq %u\n",
- hwirq, of_node_full_name(domain->of_node), virq);
-
+ pr_debug("irqs %lu...%lu on domain %s mapped to virtual irqs %u..%u\n",
+ hwirq, hwirq + num - 1, of_node_full_name(domain->of_node),
+ virq, virq + num - 1);
return virq;
}
-EXPORT_SYMBOL_GPL(irq_create_mapping);
+EXPORT_SYMBOL_GPL(irq_create_mapping_block);
/**
* irq_create_strict_mappings() - Map a range of hw irqs to fixed linux irqs
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] powerpc: msi: fsl: add support for multiple MSI interrupts
2014-11-03 16:18 [PATCH 0/2] Support multiple MSI interrupts on FSL-MPIC Johannes Thumshirn
2014-11-03 16:18 ` [PATCH 1/2] irqdomain: add support for creating a continous mapping Johannes Thumshirn
@ 2014-11-03 16:18 ` Johannes Thumshirn
2014-11-03 17:51 ` [PATCH 0/2] Support multiple MSI interrupts on FSL-MPIC Scott Wood
2 siblings, 0 replies; 8+ messages in thread
From: Johannes Thumshirn @ 2014-11-03 16:18 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Scott Wood, Michael Ellerman
Cc: Johannes Thumshirn, linuxppc-dev
This patch pushes the check for nvec > 1 && MSI into the check function
of each MSI driver except for FSL's MSI where the functionality is
added.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
arch/powerpc/kernel/msi.c | 4 ----
arch/powerpc/platforms/cell/axon_msi.c | 3 +++
arch/powerpc/platforms/powernv/pci.c | 3 +++
arch/powerpc/platforms/pseries/msi.c | 3 +++
arch/powerpc/sysdev/fsl_msi.c | 22 ++++++++++++++++------
arch/powerpc/sysdev/mpic_pasemi_msi.c | 3 +++
arch/powerpc/sysdev/mpic_u3msi.c | 3 +++
arch/powerpc/sysdev/ppc4xx_msi.c | 2 ++
8 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/msi.c b/arch/powerpc/kernel/msi.c
index 71bd161..80ee2f4 100644
--- a/arch/powerpc/kernel/msi.c
+++ b/arch/powerpc/kernel/msi.c
@@ -20,10 +20,6 @@ int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
return -ENOSYS;
}
- /* PowerPC doesn't support multiple MSI yet */
- if (type == PCI_CAP_ID_MSI && nvec > 1)
- return 1;
-
return ppc_md.setup_msi_irqs(dev, nvec, type);
}
diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
index 862b327..537a70e 100644
--- a/arch/powerpc/platforms/cell/axon_msi.c
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -250,6 +250,9 @@ static int setup_msi_msg_address(struct pci_dev *dev, struct msi_msg *msg)
of_node_put(dn);
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
+
return 0;
}
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index b2187d0..de33ec0 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -63,6 +63,9 @@ static int pnv_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
if (pdn && pdn->force_32bit_msi && !phb->msi32_support)
return -ENODEV;
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
+
list_for_each_entry(entry, &pdev->msi_list, list) {
if (!entry->msi_attrib.is_64 && !phb->msi32_support) {
pr_warn("%s: Supports only 64-bit MSIs\n",
diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 8ab5add..544e924 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -383,6 +383,9 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
int nvec = nvec_in;
int use_32bit_msi_hack = 0;
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
+
if (type == PCI_CAP_ID_MSIX)
rc = check_req_msix(pdev, nvec);
else
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index de40b48..454e8b1 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -131,13 +131,19 @@ static void fsl_teardown_msi_irqs(struct pci_dev *pdev)
struct fsl_msi *msi_data;
list_for_each_entry(entry, &pdev->msi_list, list) {
+ int num;
+ int i;
+
if (entry->irq == NO_IRQ)
continue;
msi_data = irq_get_chip_data(entry->irq);
irq_set_msi_desc(entry->irq, NULL);
+ num = 1 << entry->msi_attrib.multiple;
msi_bitmap_free_hwirqs(&msi_data->bitmap,
- virq_to_hw(entry->irq), 1);
- irq_dispose_mapping(entry->irq);
+ virq_to_hw(entry->irq), num);
+
+ for (i = 0; i < num; i++)
+ irq_dispose_mapping(entry->irq + i);
}
return;
@@ -180,6 +186,7 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
struct msi_desc *entry;
struct msi_msg msg;
struct fsl_msi *msi_data;
+ int i;
if (type == PCI_CAP_ID_MSIX)
pr_debug("fslmsi: MSI-X untested, trying anyway.\n");
@@ -219,7 +226,8 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
if (phandle && (phandle != msi_data->phandle))
continue;
- hwirq = msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
+ hwirq = msi_bitmap_alloc_hwirqs(&msi_data->bitmap,
+ nvec);
if (hwirq >= 0)
break;
}
@@ -230,16 +238,18 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
goto out_free;
}
- virq = irq_create_mapping(msi_data->irqhost, hwirq);
+ virq = irq_create_mapping_block(msi_data->irqhost, hwirq, nvec);
if (virq == NO_IRQ) {
dev_err(&pdev->dev, "fail mapping hwirq %i\n", hwirq);
- msi_bitmap_free_hwirqs(&msi_data->bitmap, hwirq, 1);
+ msi_bitmap_free_hwirqs(&msi_data->bitmap, hwirq, nvec);
rc = -ENOSPC;
goto out_free;
}
+ entry->msi_attrib.multiple = get_count_order(nvec);
/* chip_data is msi_data via host->hostdata in host->map() */
- irq_set_msi_desc(virq, entry);
+ for (i = nvec - 1; i >= 0; i--)
+ irq_set_msi_desc(virq + i, entry);
fsl_compose_msi_msg(pdev, hwirq, &msg, msi_data);
write_msi_msg(virq, &msg);
diff --git a/arch/powerpc/sysdev/mpic_pasemi_msi.c b/arch/powerpc/sysdev/mpic_pasemi_msi.c
index 15dccd3..03bace0 100644
--- a/arch/powerpc/sysdev/mpic_pasemi_msi.c
+++ b/arch/powerpc/sysdev/mpic_pasemi_msi.c
@@ -139,6 +139,9 @@ static int pasemi_msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
write_msi_msg(virq, &msg);
}
+ if (typpe == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
+
return 0;
}
diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
index 623d7fb..5024629 100644
--- a/arch/powerpc/sysdev/mpic_u3msi.c
+++ b/arch/powerpc/sysdev/mpic_u3msi.c
@@ -133,6 +133,9 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
if (type == PCI_CAP_ID_MSIX)
pr_debug("u3msi: MSI-X untested, trying anyway.\n");
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
+
/* If we can't find a magic address then MSI ain't gonna work */
if (find_ht_magic_addr(pdev, 0) == 0 &&
find_u4_magic_addr(pdev, 0) == 0) {
diff --git a/arch/powerpc/sysdev/ppc4xx_msi.c b/arch/powerpc/sysdev/ppc4xx_msi.c
index 22b5200..8bbb228 100644
--- a/arch/powerpc/sysdev/ppc4xx_msi.c
+++ b/arch/powerpc/sysdev/ppc4xx_msi.c
@@ -89,6 +89,8 @@ static int ppc4xx_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
__func__, nvec, type);
if (type == PCI_CAP_ID_MSIX)
pr_debug("ppc4xx msi: MSI-X untested, trying anyway.\n");
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
msi_data->msi_virqs = kmalloc((msi_irqs) * sizeof(int), GFP_KERNEL);
if (!msi_data->msi_virqs)
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] Support multiple MSI interrupts on FSL-MPIC
2014-11-03 16:18 [PATCH 0/2] Support multiple MSI interrupts on FSL-MPIC Johannes Thumshirn
2014-11-03 16:18 ` [PATCH 1/2] irqdomain: add support for creating a continous mapping Johannes Thumshirn
2014-11-03 16:18 ` [PATCH 2/2] powerpc: msi: fsl: add support for multiple MSI interrupts Johannes Thumshirn
@ 2014-11-03 17:51 ` Scott Wood
2 siblings, 0 replies; 8+ messages in thread
From: Scott Wood @ 2014-11-03 17:51 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: Sebastian Andrzej Siewior, linuxppc-dev
On Mon, 2014-11-03 at 17:18 +0100, Johannes Thumshirn wrote:
> This series adds support for multiple MSI interrupts on FSL's MPIC. The patch
> series was originally done by Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> and re-applied and tested by me.
>
> I dodn't know whether it is OK to put my name on it or not. So if Sebastian has
> a problem with it I'll of cause change it immediately. It was just convenient to
> use my name in the git commit but I don't want to do any kind of copyright
> infrigement.
You should set the Author field to Sebastian, which will result in a
"From:" line at the top of the message body when sending patches via
e-mail.
-Scott
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] irqdomain: add support for creating a continous mapping
2014-11-03 16:18 ` [PATCH 1/2] irqdomain: add support for creating a continous mapping Johannes Thumshirn
@ 2014-11-03 22:18 ` Scott Wood
2014-11-04 8:35 ` Sebastian Andrzej Siewior
2014-11-05 16:55 ` Johannes Thumshirn
0 siblings, 2 replies; 8+ messages in thread
From: Scott Wood @ 2014-11-03 22:18 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: Sebastian Andrzej Siewior, linuxppc-dev
On Mon, 2014-11-03 at 17:18 +0100, Johannes Thumshirn wrote:
> A MSI device may have multiple interrupts. That means that the
> interrupts numbers should be continuos so that pdev->irq refers to the
> first interrupt, pdev->irq + 1 to the second and so on.
> This patch adds support for continuous allocation of virqs for a range
> of hwirqs. The function is based on irq_create_mapping() but due to the
> number argument there is very little in common now.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> ---
> include/linux/irqdomain.h | 10 ++++--
> kernel/irq/irqdomain.c | 85 ++++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 73 insertions(+), 22 deletions(-)
This is changing core kernel code and thus LKML should be CCed, as well
as Ben Herrenschmidt who is the maintainer of kernel/irq/irqdomain.c.
Also please respond to feedback in
http://patchwork.ozlabs.org/patch/322497/
Is it really necessary for the virqs to be contiguous? How is the
availability of multiple MSIs communicated to the driver? Is there an
example of a driver that currently uses multiple MSIs?
> /**
> - * irq_create_mapping() - Map a hardware interrupt into linux irq space
> + * irq_create_mapping_block() - Map multiple hardware interrupts
> * @domain: domain owning this hardware interrupt or NULL for default domain
> * @hwirq: hardware irq number in that domain space
> + * @num: number of interrupts
> + *
> + * Maps a hwirq to a newly allocated virq. If num is greater than 1 then num
> + * hwirqs (hwirq ... hwirq + num - 1) will be mapped and virq will be
> + * continuous.
> + * Returns the first linux virq number.
> *
> - * Only one mapping per hardware interrupt is permitted. Returns a linux
> - * irq number.
> * If the sense/trigger is to be specified, set_irq_type() should be called
> * on the number returned from that call.
> */
> -unsigned int irq_create_mapping(struct irq_domain *domain,
> +unsigned int irq_create_mapping_block(struct irq_domain *domain,
> irq_hw_number_t hwirq)
> {
Where is the num parameter? How does this even build?
> unsigned int hint;
> int virq;
> + int node;
> + int i;
>
> - pr_debug("irq_create_mapping(0x%p, 0x%lx)\n", domain, hwirq);
> + pr_debug("%s(0x%p, 0x%lx, %d)\n", __func__, domain, hwirq, num);
>
> /* Look for default domain if nececssary */
> - if (domain == NULL)
> + if (!domain && num == 1)
> domain = irq_default_domain;
> +
> if (domain == NULL) {
> WARN(1, "%s(, %lx) called with NULL domain\n", __func__, hwirq);
> return 0;
> @@ -403,35 +437,46 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
> pr_debug("-> using domain @%p\n", domain);
>
> /* Check if mapping already exists */
> - virq = irq_find_mapping(domain, hwirq);
> - if (virq) {
> - pr_debug("-> existing mapping on virq %d\n", virq);
> - return virq;
> + for (i = 0; i < num; i++) {
> + virq = irq_find_mapping(domain, hwirq + i);
> + if (virq != NO_IRQ) {
Please don't introduce new uses of NO_IRQ. irq_find_mapping() returns
zero on failure. Some architectures (e.g. ARM) define NO_IRQ as
something other than zero, which will cause this to break.
> + if (i == 0)
> + return irq_check_continuous_mapping(domain,
> + hwirq, num);
> + pr_err("irq: hwirq %ld has no mapping but hwirq %ld "
> + "maps to virq %d. This can't be a block\n",
> + hwirq, hwirq + i, virq);
> + return -EINVAL;
> + }
> }
Explain how you'd get into this error state, and how you'd avoid doing
so.
> + node = of_node_to_nid(domain->of_node);
> +
> /* Allocate a virtual interrupt number */
> hint = hwirq % nr_irqs;
> if (hint == 0)
> hint++;
> - virq = irq_alloc_desc_from(hint, of_node_to_nid(domain->of_node));
> - if (virq <= 0)
> - virq = irq_alloc_desc_from(1, of_node_to_nid(domain->of_node));
> + virq = irq_alloc_desc_from(hint, node);
> + if (virq <= 0 && hint != 1)
> + virq = irq_alloc_desc_from(1, node);
Factoring out node seems irrelevant to, and obscures, what you're doing
which is adding a chcek for hint. Why is a hint value of 1 special?
You're also still allocating only one virq, unlike in
http://patchwork.ozlabs.org/patch/322497/
-Scott
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] irqdomain: add support for creating a continous mapping
2014-11-03 22:18 ` Scott Wood
@ 2014-11-04 8:35 ` Sebastian Andrzej Siewior
2014-11-05 16:55 ` Johannes Thumshirn
1 sibling, 0 replies; 8+ messages in thread
From: Sebastian Andrzej Siewior @ 2014-11-04 8:35 UTC (permalink / raw)
To: Scott Wood, Johannes Thumshirn; +Cc: linuxppc-dev
On 11/03/2014 11:18 PM, Scott Wood wrote:
> Is it really necessary for the virqs to be contiguous? How is the
> availability of multiple MSIs communicated to the driver? Is there an
> example of a driver that currently uses multiple MSIs?
I used this in PCI and after enabling 2^x, I allocated an continuous
block starting at virq. There is no way of communicating this in any
other way. The first irq number is saved in pci_dev->irq and the
remaining have to follow.
Take a look at drivers/ata/ahci.c and you will see:
- pci_msi_vec_count() for number of irqs
- pci_enable_msi_block() to allocate the number of irqs
- pci_enable_msi() (no X)
- ahci_host_activate() does request_threaded_irq() for pdev->irq and
loops "number of ports" times which matches number of number of irqs
(or is less then).
Sebastian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] irqdomain: add support for creating a continous mapping
2014-11-03 22:18 ` Scott Wood
2014-11-04 8:35 ` Sebastian Andrzej Siewior
@ 2014-11-05 16:55 ` Johannes Thumshirn
1 sibling, 0 replies; 8+ messages in thread
From: Johannes Thumshirn @ 2014-11-05 16:55 UTC (permalink / raw)
To: Scott Wood; +Cc: Johannes Thumshirn, Sebastian Andrzej Siewior, linuxppc-dev
On Mon, Nov 03, 2014 at 04:18:51PM -0600, Scott Wood wrote:
> On Mon, 2014-11-03 at 17:18 +0100, Johannes Thumshirn wrote:
> > A MSI device may have multiple interrupts. That means that the
> > interrupts numbers should be continuos so that pdev->irq refers to the
> > first interrupt, pdev->irq + 1 to the second and so on.
> > This patch adds support for continuous allocation of virqs for a range
> > of hwirqs. The function is based on irq_create_mapping() but due to the
> > number argument there is very little in common now.
> >
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> > ---
> > include/linux/irqdomain.h | 10 ++++--
> > kernel/irq/irqdomain.c | 85 ++++++++++++++++++++++++++++++++++++-----------
> > 2 files changed, 73 insertions(+), 22 deletions(-)
>
> This is changing core kernel code and thus LKML should be CCed, as well
> as Ben Herrenschmidt who is the maintainer of kernel/irq/irqdomain.c.
>
> Also please respond to feedback in
> http://patchwork.ozlabs.org/patch/322497/
>
> Is it really necessary for the virqs to be contiguous? How is the
> availability of multiple MSIs communicated to the driver? Is there an
> example of a driver that currently uses multiple MSIs?
>
> > /**
> > - * irq_create_mapping() - Map a hardware interrupt into linux irq space
> > + * irq_create_mapping_block() - Map multiple hardware interrupts
> > * @domain: domain owning this hardware interrupt or NULL for default domain
> > * @hwirq: hardware irq number in that domain space
> > + * @num: number of interrupts
> > + *
> > + * Maps a hwirq to a newly allocated virq. If num is greater than 1 then num
> > + * hwirqs (hwirq ... hwirq + num - 1) will be mapped and virq will be
> > + * continuous.
> > + * Returns the first linux virq number.
> > *
> > - * Only one mapping per hardware interrupt is permitted. Returns a linux
> > - * irq number.
> > * If the sense/trigger is to be specified, set_irq_type() should be called
> > * on the number returned from that call.
> > */
> > -unsigned int irq_create_mapping(struct irq_domain *domain,
> > +unsigned int irq_create_mapping_block(struct irq_domain *domain,
> > irq_hw_number_t hwirq)
> > {
>
> Where is the num parameter? How does this even build?
F**k this was an error from porting the patch from 3.4.x to 3.18.
>
> > unsigned int hint;
> > int virq;
> > + int node;
> > + int i;
> >
> > - pr_debug("irq_create_mapping(0x%p, 0x%lx)\n", domain, hwirq);
> > + pr_debug("%s(0x%p, 0x%lx, %d)\n", __func__, domain, hwirq, num);
> >
> > /* Look for default domain if nececssary */
> > - if (domain == NULL)
> > + if (!domain && num == 1)
> > domain = irq_default_domain;
> > +
> > if (domain == NULL) {
> > WARN(1, "%s(, %lx) called with NULL domain\n", __func__, hwirq);
> > return 0;
> > @@ -403,35 +437,46 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
> > pr_debug("-> using domain @%p\n", domain);
> >
> > /* Check if mapping already exists */
> > - virq = irq_find_mapping(domain, hwirq);
> > - if (virq) {
> > - pr_debug("-> existing mapping on virq %d\n", virq);
> > - return virq;
> > + for (i = 0; i < num; i++) {
> > + virq = irq_find_mapping(domain, hwirq + i);
> > + if (virq != NO_IRQ) {
>
> Please don't introduce new uses of NO_IRQ. irq_find_mapping() returns
> zero on failure. Some architectures (e.g. ARM) define NO_IRQ as
> something other than zero, which will cause this to break.
OK
>
> > + if (i == 0)
> > + return irq_check_continuous_mapping(domain,
> > + hwirq, num);
> > + pr_err("irq: hwirq %ld has no mapping but hwirq %ld "
> > + "maps to virq %d. This can't be a block\n",
> > + hwirq, hwirq + i, virq);
> > + return -EINVAL;
> > + }
> > }
>
> Explain how you'd get into this error state, and how you'd avoid doing
> so.
According to Thomas Gleixner (http://patchwork.ozlabs.org/patch/322497/) the
whole loop is nonsense, so I'll have to rework it anyways.
>
> > + node = of_node_to_nid(domain->of_node);
> > +
> > /* Allocate a virtual interrupt number */
> > hint = hwirq % nr_irqs;
> > if (hint == 0)
> > hint++;
> > - virq = irq_alloc_desc_from(hint, of_node_to_nid(domain->of_node));
> > - if (virq <= 0)
> > - virq = irq_alloc_desc_from(1, of_node_to_nid(domain->of_node));
> > + virq = irq_alloc_desc_from(hint, node);
> > + if (virq <= 0 && hint != 1)
> > + virq = irq_alloc_desc_from(1, node);
>
> Factoring out node seems irrelevant to, and obscures, what you're doing
> which is adding a chcek for hint. Why is a hint value of 1 special?
>
OK
> You're also still allocating only one virq, unlike in
> http://patchwork.ozlabs.org/patch/322497/
>
> -Scott
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] powerpc: msi: fsl: add support for multiple MSI interrupts
2014-02-20 20:53 Sebastian Andrzej Siewior
@ 2014-02-20 20:53 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 8+ messages in thread
From: Sebastian Andrzej Siewior @ 2014-02-20 20:53 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Gavin Shan, Arnd Bergmann, Minghuan Lian, Alexey Kardashevskiy,
Alistair Popple, Sebastian Andrzej Siewior, Brian King,
Anton Blanchard, Scott Wood, linuxppc-dev
This patch pushes the check for nvec > 1 && MSI into the check function
of each MSI driver except for FSL's MSI where the functionality is
added.
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: Alistair Popple <alistair@popple.id.au>
Cc: Brian King <brking@linux.vnet.ibm.com>
Cc: Anton Blanchard <anton@samba.org>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Minghuan Lian <Minghuan.Lian@freescale.com>
Cc: Kumar Gala <galak@kernel.crashing.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
arch/powerpc/kernel/msi.c | 4 ----
arch/powerpc/platforms/cell/axon_msi.c | 3 +++
arch/powerpc/platforms/powernv/pci.c | 2 ++
arch/powerpc/platforms/pseries/msi.c | 3 +++
arch/powerpc/platforms/wsp/msi.c | 8 ++++++++
arch/powerpc/sysdev/fsl_msi.c | 29 +++++++++++++++++++++--------
arch/powerpc/sysdev/mpic_pasemi_msi.c | 3 ++-
arch/powerpc/sysdev/mpic_u3msi.c | 2 ++
arch/powerpc/sysdev/ppc4xx_msi.c | 2 ++
9 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/kernel/msi.c b/arch/powerpc/kernel/msi.c
index 8bbc12d..46b1470 100644
--- a/arch/powerpc/kernel/msi.c
+++ b/arch/powerpc/kernel/msi.c
@@ -20,10 +20,6 @@ int arch_msi_check_device(struct pci_dev* dev, int nvec, int type)
return -ENOSYS;
}
- /* PowerPC doesn't support multiple MSI yet */
- if (type == PCI_CAP_ID_MSI && nvec > 1)
- return 1;
-
if (ppc_md.msi_check_device) {
pr_debug("msi: Using platform check routine.\n");
return ppc_md.msi_check_device(dev, nvec, type);
diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
index 85825b5..6e592ed 100644
--- a/arch/powerpc/platforms/cell/axon_msi.c
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -204,6 +204,9 @@ static int axon_msi_check_device(struct pci_dev *dev, int nvec, int type)
if (!find_msi_translator(dev))
return -ENODEV;
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
+
return 0;
}
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 95633d7..1d08040 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -54,6 +54,8 @@ static int pnv_msi_check_device(struct pci_dev* pdev, int nvec, int type)
if (pdn && pdn->force_32bit_msi && !phb->msi32_support)
return -ENODEV;
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
return (phb && phb->msi_bmp.bitmap) ? 0 : -ENODEV;
}
diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 0c882e8..ad5e766 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -339,6 +339,9 @@ static int rtas_msi_check_device(struct pci_dev *pdev, int nvec, int type)
{
int quota, rc;
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
+
if (type == PCI_CAP_ID_MSIX)
rc = check_req_msix(pdev, nvec);
else
diff --git a/arch/powerpc/platforms/wsp/msi.c b/arch/powerpc/platforms/wsp/msi.c
index 380882f..0cabd46 100644
--- a/arch/powerpc/platforms/wsp/msi.c
+++ b/arch/powerpc/platforms/wsp/msi.c
@@ -21,6 +21,13 @@
#define MSI_ADDR_32 0xFFFF0000ul
#define MSI_ADDR_64 0x1000000000000000ul
+static int wsp_msi_check_device(struct pci_dev *dev, int nvec, int type)
+{
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
+ return 0;
+}
+
int wsp_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
{
struct pci_controller *phb;
@@ -98,5 +105,6 @@ void wsp_setup_phb_msi(struct pci_controller *phb)
out_be64(phb->cfg_data + PCIE_REG_IODA_DATA0, 1ull << 63);
ppc_md.setup_msi_irqs = wsp_setup_msi_irqs;
+ ppc_md.msi_check_device = wsp_msi_check_device;
ppc_md.teardown_msi_irqs = wsp_teardown_msi_irqs;
}
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index 77efbae..f07840f 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -123,13 +123,19 @@ static void fsl_teardown_msi_irqs(struct pci_dev *pdev)
struct fsl_msi *msi_data;
list_for_each_entry(entry, &pdev->msi_list, list) {
+ int num;
+ int i;
+
if (entry->irq == NO_IRQ)
continue;
msi_data = irq_get_chip_data(entry->irq);
irq_set_msi_desc(entry->irq, NULL);
+ num = 1 << entry->msi_attrib.multiple;
msi_bitmap_free_hwirqs(&msi_data->bitmap,
- virq_to_hw(entry->irq), 1);
- irq_dispose_mapping(entry->irq);
+ virq_to_hw(entry->irq), num);
+
+ for (i = 0; i < num; i++)
+ irq_dispose_mapping(entry->irq + i);
}
return;
@@ -172,6 +178,7 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
struct msi_desc *entry;
struct msi_msg msg;
struct fsl_msi *msi_data;
+ int i;
/*
* If the PCI node has an fsl,msi property, then we need to use it
@@ -207,7 +214,8 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
if (phandle && (phandle != msi_data->phandle))
continue;
- hwirq = msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
+ hwirq = msi_bitmap_alloc_hwirqs(&msi_data->bitmap,
+ nvec);
if (hwirq >= 0)
break;
}
@@ -218,17 +226,22 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
goto out_free;
}
- virq = irq_create_mapping(msi_data->irqhost, hwirq);
-
+ virq = irq_create_mapping_block(msi_data->irqhost, hwirq, nvec);
if (virq == NO_IRQ) {
dev_err(&pdev->dev, "fail mapping hwirq %i\n", hwirq);
- msi_bitmap_free_hwirqs(&msi_data->bitmap, hwirq, 1);
+ msi_bitmap_free_hwirqs(&msi_data->bitmap, hwirq, nvec);
rc = -ENOSPC;
goto out_free;
}
+ entry->msi_attrib.multiple = get_count_order(nvec);
/* chip_data is msi_data via host->hostdata in host->map() */
- irq_set_msi_desc(virq, entry);
-
+ for (i = nvec - 1; i >= 0; i--) {
+ /*
+ * write the virq mapping last so entry->irq will point
+ * to first virq
+ */
+ irq_set_msi_desc(virq + i, entry);
+ }
fsl_compose_msi_msg(pdev, hwirq, &msg, msi_data);
write_msi_msg(virq, &msg);
}
diff --git a/arch/powerpc/sysdev/mpic_pasemi_msi.c b/arch/powerpc/sysdev/mpic_pasemi_msi.c
index 38e6238..3d36f7e 100644
--- a/arch/powerpc/sysdev/mpic_pasemi_msi.c
+++ b/arch/powerpc/sysdev/mpic_pasemi_msi.c
@@ -67,7 +67,8 @@ static int pasemi_msi_check_device(struct pci_dev *pdev, int nvec, int type)
{
if (type == PCI_CAP_ID_MSIX)
pr_debug("pasemi_msi: MSI-X untested, trying anyway\n");
-
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
return 0;
}
diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
index 9a7aa0e..f6f86ac 100644
--- a/arch/powerpc/sysdev/mpic_u3msi.c
+++ b/arch/powerpc/sysdev/mpic_u3msi.c
@@ -109,6 +109,8 @@ static int u3msi_msi_check_device(struct pci_dev *pdev, int nvec, int type)
{
if (type == PCI_CAP_ID_MSIX)
pr_debug("u3msi: MSI-X untested, trying anyway.\n");
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
/* If we can't find a magic address then MSI ain't gonna work */
if (find_ht_magic_addr(pdev, 0) == 0 &&
diff --git a/arch/powerpc/sysdev/ppc4xx_msi.c b/arch/powerpc/sysdev/ppc4xx_msi.c
index 43948da..af1efeb 100644
--- a/arch/powerpc/sysdev/ppc4xx_msi.c
+++ b/arch/powerpc/sysdev/ppc4xx_msi.c
@@ -140,6 +140,8 @@ static int ppc4xx_msi_check_device(struct pci_dev *pdev, int nvec, int type)
__func__, nvec, type);
if (type == PCI_CAP_ID_MSIX)
pr_debug("ppc4xx msi: MSI-X untested, trying anyway.\n");
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
return 0;
}
--
1.9.0.rc3
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-11-05 16:56 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-03 16:18 [PATCH 0/2] Support multiple MSI interrupts on FSL-MPIC Johannes Thumshirn
2014-11-03 16:18 ` [PATCH 1/2] irqdomain: add support for creating a continous mapping Johannes Thumshirn
2014-11-03 22:18 ` Scott Wood
2014-11-04 8:35 ` Sebastian Andrzej Siewior
2014-11-05 16:55 ` Johannes Thumshirn
2014-11-03 16:18 ` [PATCH 2/2] powerpc: msi: fsl: add support for multiple MSI interrupts Johannes Thumshirn
2014-11-03 17:51 ` [PATCH 0/2] Support multiple MSI interrupts on FSL-MPIC Scott Wood
-- strict thread matches above, loose matches on Subject: below --
2014-02-20 20:53 Sebastian Andrzej Siewior
2014-02-20 20:53 ` [PATCH 2/2] powerpc: msi: fsl: add support for multiple MSI interrupts Sebastian Andrzej Siewior
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).