Some multi-core ARM chips designate a unique IRQ number for each core for
private peripheral interrupts (PPIs). Others designate a common IRQ number
for all cores. In the latter case, requesting/freeing private peripheral
interrupts currently unmasks/masks the interrupt for only the
executing core, respectively.
With this change, request_irq will unmask a PPI on all cores so a separate
call to enable_irq on the other cores is not required. Likewise, free_irq
will mask a PPI on the other cores. Also, shutdown is implemented instead
of disable to allow for lazy IRQ disabling.
Signed-off-by: Stephen Caudle <[email protected]>
---
arch/arm/Kconfig | 5 +++
arch/arm/common/gic.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 91 insertions(+), 0 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 30ddd06..7f11e31 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1153,6 +1153,11 @@ config SMP
If you don't know what to do here, say N.
+config IRQ_PER_CPU
+ bool
+ depends on SMP
+ default n
+
config HAVE_ARM_SCU
bool
depends on SMP
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 886daaf..937a33a 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -44,12 +44,25 @@ struct gic_chip_data {
unsigned int wakeup_irqs[32];
unsigned int enabled_irqs[32];
#endif
+#ifdef CONFIG_IRQ_PER_CPU
+ struct call_single_data ppi_data[NR_CPUS];
+#endif
};
#ifndef MAX_GIC_NR
#define MAX_GIC_NR 1
#endif
+#ifdef CONFIG_IRQ_PER_CPU
+#ifndef GIC_PPI_FIRST
+#define GIC_PPI_FIRST 16
+#endif
+
+#ifndef GIC_PPI_LAST
+#define GIC_PPI_LAST 31
+#endif
+#endif
+
static struct gic_chip_data gic_data[MAX_GIC_NR];
static inline void __iomem *gic_dist_base(unsigned int irq)
@@ -272,6 +285,75 @@ static int gic_set_type(unsigned int irq, unsigned int type)
return 0;
}
+#ifdef CONFIG_IRQ_PER_CPU
+static inline void gic_smp_call_function(struct call_single_data *data)
+{
+ int cpu;
+
+ /* Make sure data is visible */
+ smp_mb();
+
+ /*
+ * Since this function is called with interrupts disabled,
+ * smp_call_function can't be used here because it warns (even
+ * if wait = 0) when interrupts are disabled.
+ *
+ * __smp_call_function_single doesn't warn when interrupts are
+ * disabled and not waiting, so use it instead.
+ */
+ for_each_online_cpu(cpu)
+ if (cpu != smp_processor_id())
+ __smp_call_function_single(cpu, data, 0);
+}
+
+static void gic_mask_ppi(void *info)
+{
+ struct irq_desc *desc = info;
+ gic_mask_irq(desc->irq);
+}
+
+static void gic_unmask_ppi(void *info)
+{
+ struct irq_desc *desc = info;
+ gic_unmask_irq(desc->irq);
+}
+
+static void gic_enable_irq(unsigned int irq)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+ struct gic_chip_data *gic_data = get_irq_chip_data(irq);
+ int cpu = smp_processor_id();
+
+ if (irq >= GIC_PPI_FIRST && irq <= GIC_PPI_LAST) {
+ gic_data->ppi_data[cpu].func = gic_unmask_ppi;
+ gic_data->ppi_data[cpu].info = desc;
+
+ /* Unmask PPIs on all cores during enable. */
+ gic_smp_call_function(&gic_data->ppi_data[cpu]);
+ }
+
+ desc->chip->unmask(irq);
+ desc->status &= ~IRQ_MASKED;
+}
+
+static void gic_shutdown_irq(unsigned int irq)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+ struct gic_chip_data *gic_data = get_irq_chip_data(irq);
+ int cpu = smp_processor_id();
+
+ if (irq >= GIC_PPI_FIRST && irq <= GIC_PPI_LAST) {
+ gic_data->ppi_data[cpu].func = gic_mask_ppi;
+ gic_data->ppi_data[cpu].info = desc;
+
+ /* Mask PPIs on all cores during disable. */
+ gic_smp_call_function(&gic_data->ppi_data[cpu]);
+ }
+
+ desc->chip->mask(irq);
+ desc->status |= IRQ_MASKED;
+}
+#endif
static struct irq_chip gic_chip = {
.name = "GIC",
@@ -283,6 +365,10 @@ static struct irq_chip gic_chip = {
#endif
.set_type = gic_set_type,
.set_wake = gic_set_wake,
+#ifdef CONFIG_IRQ_PER_CPU
+ .enable = gic_enable_irq,
+ .shutdown = gic_shutdown_irq,
+#endif
};
void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
--
1.7.3.2
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum
Russell,
Can you pull in this patch or let me know if you want something changed
with it?
Thanks,
Stephen
On 11/03/2010 05:46 PM, Stephen Caudle wrote:
> Some multi-core ARM chips designate a unique IRQ number for each core for
> private peripheral interrupts (PPIs). Others designate a common IRQ number
> for all cores. In the latter case, requesting/freeing private peripheral
> interrupts currently unmasks/masks the interrupt for only the
> executing core, respectively.
>
> With this change, request_irq will unmask a PPI on all cores so a separate
> call to enable_irq on the other cores is not required. Likewise, free_irq
> will mask a PPI on the other cores. Also, shutdown is implemented instead
> of disable to allow for lazy IRQ disabling.
>
> Signed-off-by: Stephen Caudle<[email protected]>
> ---
> arch/arm/Kconfig | 5 +++
> arch/arm/common/gic.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 91 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 30ddd06..7f11e31 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1153,6 +1153,11 @@ config SMP
>
> If you don't know what to do here, say N.
>
> +config IRQ_PER_CPU
> + bool
> + depends on SMP
> + default n
> +
> config HAVE_ARM_SCU
> bool
> depends on SMP
> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> index 886daaf..937a33a 100644
> --- a/arch/arm/common/gic.c
> +++ b/arch/arm/common/gic.c
> @@ -44,12 +44,25 @@ struct gic_chip_data {
> unsigned int wakeup_irqs[32];
> unsigned int enabled_irqs[32];
> #endif
> +#ifdef CONFIG_IRQ_PER_CPU
> + struct call_single_data ppi_data[NR_CPUS];
> +#endif
> };
>
> #ifndef MAX_GIC_NR
> #define MAX_GIC_NR 1
> #endif
>
> +#ifdef CONFIG_IRQ_PER_CPU
> +#ifndef GIC_PPI_FIRST
> +#define GIC_PPI_FIRST 16
> +#endif
> +
> +#ifndef GIC_PPI_LAST
> +#define GIC_PPI_LAST 31
> +#endif
> +#endif
> +
> static struct gic_chip_data gic_data[MAX_GIC_NR];
>
> static inline void __iomem *gic_dist_base(unsigned int irq)
> @@ -272,6 +285,75 @@ static int gic_set_type(unsigned int irq, unsigned int type)
> return 0;
> }
>
> +#ifdef CONFIG_IRQ_PER_CPU
> +static inline void gic_smp_call_function(struct call_single_data *data)
> +{
> + int cpu;
> +
> + /* Make sure data is visible */
> + smp_mb();
> +
> + /*
> + * Since this function is called with interrupts disabled,
> + * smp_call_function can't be used here because it warns (even
> + * if wait = 0) when interrupts are disabled.
> + *
> + * __smp_call_function_single doesn't warn when interrupts are
> + * disabled and not waiting, so use it instead.
> + */
> + for_each_online_cpu(cpu)
> + if (cpu != smp_processor_id())
> + __smp_call_function_single(cpu, data, 0);
> +}
> +
> +static void gic_mask_ppi(void *info)
> +{
> + struct irq_desc *desc = info;
> + gic_mask_irq(desc->irq);
> +}
> +
> +static void gic_unmask_ppi(void *info)
> +{
> + struct irq_desc *desc = info;
> + gic_unmask_irq(desc->irq);
> +}
> +
> +static void gic_enable_irq(unsigned int irq)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> + struct gic_chip_data *gic_data = get_irq_chip_data(irq);
> + int cpu = smp_processor_id();
> +
> + if (irq>= GIC_PPI_FIRST&& irq<= GIC_PPI_LAST) {
> + gic_data->ppi_data[cpu].func = gic_unmask_ppi;
> + gic_data->ppi_data[cpu].info = desc;
> +
> + /* Unmask PPIs on all cores during enable. */
> + gic_smp_call_function(&gic_data->ppi_data[cpu]);
> + }
> +
> + desc->chip->unmask(irq);
> + desc->status&= ~IRQ_MASKED;
> +}
> +
> +static void gic_shutdown_irq(unsigned int irq)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> + struct gic_chip_data *gic_data = get_irq_chip_data(irq);
> + int cpu = smp_processor_id();
> +
> + if (irq>= GIC_PPI_FIRST&& irq<= GIC_PPI_LAST) {
> + gic_data->ppi_data[cpu].func = gic_mask_ppi;
> + gic_data->ppi_data[cpu].info = desc;
> +
> + /* Mask PPIs on all cores during disable. */
> + gic_smp_call_function(&gic_data->ppi_data[cpu]);
> + }
> +
> + desc->chip->mask(irq);
> + desc->status |= IRQ_MASKED;
> +}
> +#endif
>
> static struct irq_chip gic_chip = {
> .name = "GIC",
> @@ -283,6 +365,10 @@ static struct irq_chip gic_chip = {
> #endif
> .set_type = gic_set_type,
> .set_wake = gic_set_wake,
> +#ifdef CONFIG_IRQ_PER_CPU
> + .enable = gic_enable_irq,
> + .shutdown = gic_shutdown_irq,
> +#endif
> };
>
> void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
On Wed, Nov 03, 2010 at 05:46:02PM -0400, Stephen Caudle wrote:
> Some multi-core ARM chips designate a unique IRQ number for each core for
> private peripheral interrupts (PPIs). Others designate a common IRQ number
> for all cores. In the latter case, requesting/freeing private peripheral
> interrupts currently unmasks/masks the interrupt for only the
> executing core, respectively.
>
> With this change, request_irq will unmask a PPI on all cores so a separate
> call to enable_irq on the other cores is not required. Likewise, free_irq
> will mask a PPI on the other cores. Also, shutdown is implemented instead
> of disable to allow for lazy IRQ disabling.
Sorry, missed this.
If it's a private peripheral, it can only be accessed from its associated
CPU. What that means is you don't want to enable the interrupt on other
CPUs as the peripheral may not be present or initialized on that CPU.
So I'm nervous about this change - architecturally it feels like the
wrong thing to do to take the PPI interrupts through the generic IRQ
infrastructure.
On 11/30/2010 01:07 PM, Russell King - ARM Linux wrote:
> Sorry, missed this.
>
> If it's a private peripheral, it can only be accessed from its associated
> CPU. What that means is you don't want to enable the interrupt on other
> CPUs as the peripheral may not be present or initialized on that CPU.
Understood. But the alternative is to require all code that requests a
PPI to have to enable the IRQ on the other cores. This seems
unreasonable to me.
> So I'm nervous about this change - architecturally it feels like the
> wrong thing to do to take the PPI interrupts through the generic IRQ
> infrastructure.
What do suggest as an alternative to this solution? Creating separate
IRQ numbers for each core (per PPI) doesn't seem to scale well as the
number of cores increase.
~Stephen
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
On Wed, Dec 01, 2010 at 11:36:10AM -0500, Stephen Caudle wrote:
> On 11/30/2010 01:07 PM, Russell King - ARM Linux wrote:
>> Sorry, missed this.
>>
>> If it's a private peripheral, it can only be accessed from its associated
>> CPU. What that means is you don't want to enable the interrupt on other
>> CPUs as the peripheral may not be present or initialized on that CPU.
>
> Understood. But the alternative is to require all code that requests a
> PPI to have to enable the IRQ on the other cores. This seems
> unreasonable to me.
It is also unreasonable to have one core enabling the PPI on other
cores where the hardware behind the interrupt may not have been
initialized yet. If it is a private interrupt for a private peripheral,
then only the associated CPU should be enabling that interrupt.
I guess this is something which genirq can't cope with, in which case
either genirq needs to be modified to cope with private CPU interrupts,
which are controlled individually by their associated CPU, or we need a
private interface to support this.
On 12/01/2010 12:14 PM, Russell King - ARM Linux wrote:
> On Wed, Dec 01, 2010 at 11:36:10AM -0500, Stephen Caudle wrote:
>> On 11/30/2010 01:07 PM, Russell King - ARM Linux wrote:
>>> Sorry, missed this.
>>>
>>> If it's a private peripheral, it can only be accessed from its associated
>>> CPU. What that means is you don't want to enable the interrupt on other
>>> CPUs as the peripheral may not be present or initialized on that CPU.
>>
>> Understood. But the alternative is to require all code that requests a
>> PPI to have to enable the IRQ on the other cores. This seems
>> unreasonable to me.
>
> It is also unreasonable to have one core enabling the PPI on other
> cores where the hardware behind the interrupt may not have been
> initialized yet. If it is a private interrupt for a private peripheral,
> then only the associated CPU should be enabling that interrupt.
>
> I guess this is something which genirq can't cope with, in which case
> either genirq needs to be modified to cope with private CPU interrupts,
> which are controlled individually by their associated CPU, or we need a
> private interface to support this.
I see your point. Our immediate need for this is to support a
performance monitor interrupt that happens to be a PPI. It is used by
perf events (and subsequently, oprofile).
Since PPIs are so machine-specific, I started looking into patching
perf_events.c by adding a machine specific function to handle the PMU
IRQ request. For mach-msm, we would call request_irq like normal, but
also unmask the performance monitor interrupt on the other cores. The
downside to this is that a machine specific implementation would be
needed anytime a PPI is requested, not just in perf_events.c.
Then, I saw Thomas' email regarding our local timer PPI:
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-December/033840.html.
Russell, before I submit another patch, I would like to know if you
prefer a more generic approach like Thomas suggests, or a
machine-specific approach like I have described?
Thanks,
Stephen
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
On 12/09/2010 11:24 AM, Stephen Caudle wrote:
>> It is also unreasonable to have one core enabling the PPI on other
>> cores where the hardware behind the interrupt may not have been
>> initialized yet. If it is a private interrupt for a private peripheral,
>> then only the associated CPU should be enabling that interrupt.
>>
>> I guess this is something which genirq can't cope with, in which case
>> either genirq needs to be modified to cope with private CPU interrupts,
>> which are controlled individually by their associated CPU, or we need a
>> private interface to support this.
>
> I see your point. Our immediate need for this is to support a
> performance monitor interrupt that happens to be a PPI. It is used by
> perf events (and subsequently, oprofile).
>
> Since PPIs are so machine-specific, I started looking into patching
> perf_events.c by adding a machine specific function to handle the PMU
> IRQ request. For mach-msm, we would call request_irq like normal, but
> also unmask the performance monitor interrupt on the other cores. The
> downside to this is that a machine specific implementation would be
> needed anytime a PPI is requested, not just in perf_events.c.
>
> Then, I saw Thomas' email regarding our local timer PPI:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-December/033840.html.
>
> Russell, before I submit another patch, I would like to know if you
> prefer a more generic approach like Thomas suggests, or a
> machine-specific approach like I have described?
Russell, what are your thoughts on this?
Thanks,
Stephen
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
On Thu, Dec 16, 2010 at 09:54:23AM -0500, Stephen Caudle wrote:
> On 12/09/2010 11:24 AM, Stephen Caudle wrote:
> >> It is also unreasonable to have one core enabling the PPI on other
> >> cores where the hardware behind the interrupt may not have been
> >> initialized yet. If it is a private interrupt for a private peripheral,
> >> then only the associated CPU should be enabling that interrupt.
> >>
> >> I guess this is something which genirq can't cope with, in which case
> >> either genirq needs to be modified to cope with private CPU interrupts,
> >> which are controlled individually by their associated CPU, or we need a
> >> private interface to support this.
> >
> > I see your point. Our immediate need for this is to support a
> > performance monitor interrupt that happens to be a PPI. It is used by
> > perf events (and subsequently, oprofile).
> >
> > Since PPIs are so machine-specific, I started looking into patching
> > perf_events.c by adding a machine specific function to handle the PMU
> > IRQ request. For mach-msm, we would call request_irq like normal, but
> > also unmask the performance monitor interrupt on the other cores. The
> > downside to this is that a machine specific implementation would be
> > needed anytime a PPI is requested, not just in perf_events.c.
> >
> > Then, I saw Thomas' email regarding our local timer PPI:
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2010-December/033840.html.
> >
> > Russell, before I submit another patch, I would like to know if you
> > prefer a more generic approach like Thomas suggests, or a
> > machine-specific approach like I have described?
>
> Russell, what are your thoughts on this?
I've not changed my thoughts on this. PPIs should not be handled by
genirq - it just doesn't make sense for them to be.
On 12/16/2010 10:03 AM, Russell King - ARM Linux wrote:
> I've not changed my thoughts on this. PPIs should not be handled by
> genirq - it just doesn't make sense for them to be.
Understood. I will work on a patch to perf events to support the MSM
performance monitor PPI on SMP.
~Stephen
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.