Armada 375/38x perf support, and a bonus irqchip driver simplification
Subsystems (4)
cover Armada 375/38x perf support, and a bonus irqchip driver simplification
This patchset enables support for perf hardware events, by enabling the PMU interrupts in the irqchip driver.
While doing this, we noticed the driver could use some cleaning to simplify the overly complex implementation of the .map(), .unmask() and .mask() functions.
The first three patches are the result of this cleaning effort, while the rest of the series is the Perf support for Armada 375 and Armada 38x SoCs.
The series is based on v3.18-rc1.
Ezequiel Garcia (7):
irqchip: armada-370-xp: Simplify interrupt map, mask and unmask
irqchip: armada-370-xp: Initialize per cpu registers when CONFIG_SMP=N
irqchip: armada-370-xp: Introduce a is_percpu_irq() helper for
readability
irqchip: armada-370-xp: Enable Performance Counter interrupts
ARM: mvebu: Enable Performance Monitor Unit on Armada 375 SoC
ARM: mvebu: Enable Performance Monitor Unit on Armada 380/385 SoC
ARM: mvebu: Enable perf support in mvebu_v7_defconfig
arch/arm/boot/dts/armada-375.dtsi | 5 ++ arch/arm/boot/dts/armada-38x.dtsi | 5 ++ arch/arm/configs/mvebu_v7_defconfig | 1 + drivers/irqchip/irq-armada-370-xp.c | 95 ++++++++++++++++++------------------- 4 files changed, 58 insertions(+), 48 deletions(-)
Signature · 1 line
2.1.0
PATCH 1/7 irqchip: armada-370-xp: Simplify interrupt map, mask and unmask
The map, mask and unmask is unnecessarily complicated, with a different implementation for shared and per CPU interrupts. The current code does the following:
At probe time, all interrupts are disabled and masked on all CPUs.
Shared interrupts:
* When the interrupt is mapped(), it gets disabled and unmasked on the calling CPU.
* When the interrupt is unmasked(), masked(), it gets enabled and disabled.
Per CPU interrupts:
* When the interrupt is mapped, it gets masked on the calling CPU and enabled.
* When the interrupt is unmasked(), masked(), it gets unmasked and masked, on the calling CPU.
This commit simplifies this code, with a much simpler implementation, common to shared and per CPU interrupts.
* When the interrupt is mapped, it's enabled.
* When the interrupt is unmasked(), masked(), it gets unmasked and masked, on the calling CPU.
Tested on a Armada XP SoC with SMP and UP configurations, with chained and regular interrupts.
--- drivers/irqchip/irq-armada-370-xp.c | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-)
drivers/irqchip/irq-armada-370-xp.c +4 -23
@@ -73,33 +73,18 @@ static DEFINE_MUTEX(msi_used_lock);static phys_addr_t msi_doorbell_addr;#endif -/* -* In SMP mode: -* For shared global interrupts, mask/unmask global enable bit -* For CPU interrupts, mask/unmask the calling CPU's bit -*/ static void armada_370_xp_irq_mask (struct irq_data *d) {irq_hw_number_t hwirq =irqd_to_hwirq (d); -if (hwirq !=ARMADA_370_XP_TIMER0_PER_CPU_IRQ ) -writel (hwirq, main_int_base + -ARMADA_370_XP_INT_CLEAR_ENABLE_OFFS ); -else -writel (hwirq, per_cpu_int_base + -ARMADA_370_XP_INT_SET_MASK_OFFS ); +writel (hwirq, per_cpu_int_base +ARMADA_370_XP_INT_SET_MASK_OFFS ); }static void armada_370_xp_irq_unmask (struct irq_data *d) {irq_hw_number_t hwirq =irqd_to_hwirq (d); -if (hwirq !=ARMADA_370_XP_TIMER0_PER_CPU_IRQ ) -writel (hwirq, main_int_base + -ARMADA_370_XP_INT_SET_ENABLE_OFFS ); -else -writel (hwirq, per_cpu_int_base + -ARMADA_370_XP_INT_CLEAR_MASK_OFFS ); +writel (hwirq, per_cpu_int_base +ARMADA_370_XP_INT_CLEAR_MASK_OFFS ); }#ifdef CONFIG_PCI_MSI
@@ -282,12 +267,8 @@ static struct irq_chip armada_370_xp_irq_chip = {static int armada_370_xp_mpic_irq_map (struct irq_domain *h,unsigned int virq,irq_hw_number_t hw) { -armada_370_xp_irq_mask (irq_get_irq_data (virq)); -if (hw !=ARMADA_370_XP_TIMER0_PER_CPU_IRQ ) -writel (hw, per_cpu_int_base + -ARMADA_370_XP_INT_CLEAR_MASK_OFFS ); -else -writel (hw, main_int_base +ARMADA_370_XP_INT_SET_ENABLE_OFFS ); +writel (hw, main_int_base +ARMADA_370_XP_INT_SET_ENABLE_OFFS ); +irq_set_status_flags (virq,IRQ_LEVEL );if (hw ==ARMADA_370_XP_TIMER0_PER_CPU_IRQ ) {
Signature · 1 line
2.1.0
PATCH 1/7 irqchip: armada-370-xp: Simplify interrupt map, mask and unmask
Hi Ezequiel,
Quoted 22 lines · Ezequiel Garcia
The map, mask and unmask is unnecessarily complicated, with a different implementation for shared and per CPU interrupts. The current code does the following:
At probe time, all interrupts are disabled and masked on all CPUs.
Shared interrupts:
* When the interrupt is
mapped(), it gets disabled and unmasked on the calling CPU.* When the interrupt is
unmasked(),masked(), it gets enabled and disabled.Per CPU interrupts:
* When the interrupt is mapped, it gets masked on the calling CPU and enabled.
* When the interrupt is
unmasked(),masked(), it gets unmasked and masked, on the calling CPU.
Quoted 7 lines
This commit simplifies this code, with a much simpler implementation, common to shared and per CPU interrupts.
* When the interrupt is mapped, it's enabled.
* When the interrupt is
unmasked(),masked(), it gets unmasked and masked, on the calling CPU.
By doing this you change the behavior of the irqchip. Before this patch, masking a shared interrupt was masking it on all the CPUs in the same time whereas with this change it will mask the interrupt only on the calling CPU. It worth checking it is the expected behavior.
Moreover I wonder how it is supposed to work with the irq affinity. Let say that the irq was enabled on the CPU0, then there is irq_unmask call on CPU1, then the irq would be enabled on both CPUs. It will modify the irq affinity and moreover it will also lead to an invalidate state with the MPIC because we can't manage an interrupt on more than one CPU.
Quoted 1 line
From the point of the view of the
armada_370_xp_irqdriver there are potential bug, but
maybe the use case I described can't happen. Could you check it?
Thanks,
Gregory
Quoted 65 lines
Tested on a Armada XP SoC with SMP and UP configurations, with chained and regular interrupts.
Signed-off-by Ezequiel Garcia <[email protected]>--- drivers/irqchip/irq-armada-370-xp.c | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-)drivers/irqchip/irq-armada-370-xp.c +4
-23@@ -73,33 +73,18 @@ static DEFINE_MUTEX(msi_used_lock);static phys_addr_t msi_doorbell_addr;#endif -/* -* In SMP mode: -* For shared global interrupts, mask/unmask global enable bit -* For CPU interrupts, mask/unmask the calling CPU's bit -*/ static void armada_370_xp_irq_mask (struct irq_data *d) {irq_hw_number_t hwirq =irqd_to_hwirq (d); -if (hwirq !=ARMADA_370_XP_TIMER0_PER_CPU_IRQ ) -writel (hwirq, main_int_base + -ARMADA_370_XP_INT_CLEAR_ENABLE_OFFS ); -else -writel (hwirq, per_cpu_int_base + -ARMADA_370_XP_INT_SET_MASK_OFFS ); +writel (hwirq, per_cpu_int_base +ARMADA_370_XP_INT_SET_MASK_OFFS ); }static void armada_370_xp_irq_unmask (struct irq_data *d) {irq_hw_number_t hwirq =irqd_to_hwirq (d); -if (hwirq !=ARMADA_370_XP_TIMER0_PER_CPU_IRQ ) -writel (hwirq, main_int_base + -ARMADA_370_XP_INT_SET_ENABLE_OFFS ); -else -writel (hwirq, per_cpu_int_base + -ARMADA_370_XP_INT_CLEAR_MASK_OFFS ); +writel (hwirq, per_cpu_int_base +ARMADA_370_XP_INT_CLEAR_MASK_OFFS ); }#ifdef CONFIG_PCI_MSI @@ -282,12 +267,8 @@ static struct irq_chip armada_370_xp_irq_chip = {static int armada_370_xp_mpic_irq_map (struct irq_domain *h,unsigned int virq,irq_hw_number_t hw) { -armada_370_xp_irq_mask (irq_get_irq_data (virq)); -if (hw !=ARMADA_370_XP_TIMER0_PER_CPU_IRQ ) -writel (hw, per_cpu_int_base + -ARMADA_370_XP_INT_CLEAR_MASK_OFFS ); -else -writel (hw, main_int_base +ARMADA_370_XP_INT_SET_ENABLE_OFFS ); +writel (hw, main_int_base +ARMADA_370_XP_INT_SET_ENABLE_OFFS ); +irq_set_status_flags (virq,IRQ_LEVEL );if (hw ==ARMADA_370_XP_TIMER0_PER_CPU_IRQ ) {
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 1/7 irqchip: armada-370-xp: Simplify interrupt map, mask and unmask
Hi Gregory,
Quoted 28 lines · Gregory CLEMENT
Quoted 27 lines · Ezequiel Garcia
The map, mask and unmask is unnecessarily complicated, with a different implementation for shared and per CPU interrupts. The current code does the following:
At probe time, all interrupts are disabled and masked on all CPUs.
Shared interrupts:
* When the interrupt is
mapped(), it gets disabled and unmasked on the calling CPU.* When the interrupt is
unmasked(),masked(), it gets enabled and disabled.Per CPU interrupts:
* When the interrupt is mapped, it gets masked on the calling CPU and enabled.
* When the interrupt is
unmasked(),masked(), it gets unmasked and masked, on the calling CPU.This commit simplifies this code, with a much simpler implementation, common to shared and per CPU interrupts.
* When the interrupt is mapped, it's enabled.
Quoted 8 lines
Quoted 2 lines
* When the interrupt is
unmasked(),masked(), it gets unmasked and masked, on the calling CPU.By doing this you change the behavior of the irqchip. Before this patch, masking a shared interrupt was masking it on all the CPUs in the same time whereas with this change it will mask the interrupt only on the calling CPU. It worth checking it is the expected behavior.
The kernel will always mask/unmask on the appropriate CPU, so I don't see any issues with this.
Quoted 2 lines
Moreover I wonder how it is supposed to work with the irq affinity. Let say that the irq was enabled on the CPU0,
If you mean enabled as in irq_enable(irq_desc), then let's keep in mind that currently that calls .irq_unmask.
Quoted 8 lines
then there is
irq_unmaskcall on CPU1, then the irq would be enabled on both CPUs. It will modify the irq affinity and moreover it will also lead
to an invalidate state with the MPIC because we can't manage an interrupt on more than one CPU.From the point of the view of the
armada_370_xp_irqdriver there are potential bug, but maybe the use case I described can't happen. Could you check it?
In this irqchip driver, shared interrupts are handled via handle_level_irq(), which masks() and unmasks() the interrupt. In other words, it's always done in the same CPU. I guess this is the most frequent mask/unmask operation.
On the other side, if a driver calls enable_irq() on one CPU and then enable_irq() on the other CPU, that's exactly what will happen. Although the smp_affinity setting will prevent an interrupt from being dispatched to more than one CPU.
I really don't see an issue with this patch, but I think it needs as much discussion and as much testing as possible. This piece of code is rather critical, and is working just fine, so I'd like to have an excellent reason before changing any of it.
Signature · 11 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141104/c426c5f7/attachment.sig>
PATCH 1/7 irqchip: armada-370-xp: Simplify interrupt map, mask and unmask
Hi Ezequiel,
Quoted 67 lines · Ezequiel Garcia
Hi Gregory,
Quoted 28 lines · Gregory CLEMENT
Quoted 27 lines · Ezequiel Garcia
The map, mask and unmask is unnecessarily complicated, with a different implementation for shared and per CPU interrupts. The current code does the following:
At probe time, all interrupts are disabled and masked on all CPUs.
Shared interrupts:
* When the interrupt is
mapped(), it gets disabled and unmasked on the calling CPU.* When the interrupt is
unmasked(),masked(), it gets enabled and disabled.Per CPU interrupts:
* When the interrupt is mapped, it gets masked on the calling CPU and enabled.
* When the interrupt is
unmasked(),masked(), it gets unmasked and masked, on the calling CPU.This commit simplifies this code, with a much simpler implementation, common to shared and per CPU interrupts.
* When the interrupt is mapped, it's enabled.
Quoted 8 lines
Quoted 2 lines
* When the interrupt is
unmasked(),masked(), it gets unmasked and masked, on the calling CPU.By doing this you change the behavior of the irqchip. Before this patch, masking a shared interrupt was masking it on all the CPUs in the same time whereas with this change it will mask the interrupt only on the calling CPU. It worth checking it is the expected behavior.
The kernel will always mask/unmask on the appropriate CPU, so I don't see any issues with this.
Quoted 2 lines
Moreover I wonder how it is supposed to work with the irq affinity. Let say that the irq was enabled on the CPU0,
If you mean enabled as in
irq_enable(irq_desc), then let's keep in mind that currently that calls .irq_unmask.Quoted 8 lines
then there is
irq_unmaskcall on CPU1, then the irq would be enabled on both CPUs. It will modify the irq affinity and moreover it will also lead
to an invalidate state with the MPIC because we can't manage an interrupt on more than one CPU.From the point of the view of the
armada_370_xp_irqdriver there are potential bug, but maybe the use case I described can't happen. Could you check it?In this irqchip driver, shared interrupts are handled via
handle_level_irq(), whichmasks()andunmasks()the interrupt. In other words, it's always done in the same CPU. I guess this is the most frequent mask/unmask operation.On the other side, if a driver calls
enable_irq()on one CPU and thenenable_irq()on the other CPU, that's exactly what will happen. Although thesmp_affinitysetting will prevent an interrupt from being dispatched to more than one CPU.
Quoted 4 lines
I really don't see an issue with this patch, but I think it needs as much discussion and as much testing as possible. This piece of code is rather critical, and is working just fine, so I'd like to have an excellent reason before changing any of it.
We already talked about it on IRC but I write it agin o let the other people know our status on this subject.
If a driver call irq_enable() then this functions will call irq_enable() and as we didn't implement a .enbale() operation, it will call our only unmask() function.
So if the IRQ was unmasked on a CPU and a driver call an irq_enable() from an other CPU then we will end up with the IRQ enabled on 2 different CPUs. It is a problem for 2 reasons:
- our hardware don't handle a IRQ enable on more than one CPU
- it will modify the affinity at the hardware level because a new CPU will be able to receive an irq whereas we setup the affinity on only one CPU.
By only using the mask and unmask registers we will need to handle the affinity by software instead of using the hardware. I agree that the current code is not trivial, but adding more comment instead of removing this capability should be better.
Thanks,
Gregory
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 2/7 irqchip: armada-370-xp: Initialize per cpu registers when CONFIG_SMP=N
In reply to Ezequiel GarciaThe irqchip driver called armada_xp_mpic_smp_cpu_init() when CONFIG_SMP=Y to initialize some per cpu registers. The function is called on each CPU by calling it explicitly on the boot CPU and then using a CPU notifier for the non boot CPUs.
This commit removes the CONFIG_SMP constrain, so the per cpu registers are also initialized when CONFIG_SMP=N, which is the right thing to do.
--- drivers/irqchip/irq-armada-370-xp.c | 47 ++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 24 deletions(-)
drivers/irqchip/irq-armada-370-xp.c +23 -24
@@ -285,28 +285,6 @@ static int armada_370_xp_mpic_irq_map(struct irq_domain *h,return 0 ; } -#ifdef CONFIG_SMP -static void armada_mpic_send_doorbell (const struct cpumask *mask, -unsigned int irq) -{ -int cpu; -unsigned long map =0 ; - -/* Convert our logical CPU mask into a physical one. */ -for_each_cpu (cpu, mask) - map |=1 <<cpu_logical_map (cpu); - -/* -* Ensure that stores to Normal memory are visible to the -* other CPUs before issuing the IPI. -*/ -dsb (); - -/* submit softirq */ -writel ((map <<8 ) | irq, main_int_base + -ARMADA_370_XP_SW_TRIG_INT_OFFS ); -} -static void armada_xp_mpic_smp_cpu_init (void ) {u32 control;
@@ -329,6 +307,28 @@ static void armada_xp_mpic_smp_cpu_init(void)writel (0 , per_cpu_int_base +ARMADA_370_XP_INT_CLEAR_MASK_OFFS ); } +#ifdef CONFIG_SMP +static void armada_mpic_send_doorbell (const struct cpumask *mask, +unsigned int irq) +{ +int cpu; +unsigned long map =0 ; + +/* Convert our logical CPU mask into a physical one. */ +for_each_cpu (cpu, mask) + map |=1 <<cpu_logical_map (cpu); + +/* +* Ensure that stores to Normal memory are visible to the +* other CPUs before issuing the IPI. +*/ +dsb (); + +/* submit softirq */ +writel ((map <<8 ) | irq, main_int_base + +ARMADA_370_XP_SW_TRIG_INT_OFFS ); +} +static int armada_xp_mpic_secondary_init (struct notifier_block *nfb,unsigned long action,void *hcpu) {
@@ -492,9 +492,8 @@ static int __init armada_370_xp_mpic_of_init(struct device_node *node,BUG_ON (!armada_370_xp_mpic_domain); -#ifdef CONFIG_SMP +/* Setup for the boot CPU */ armada_xp_mpic_smp_cpu_init (); -#endif armada_370_xp_msi_init (node, main_int_res.start);
Signature · 1 line
2.1.0
PATCH 2/7 irqchip: armada-370-xp: Initialize per cpu registers when CONFIG_SMP=N
Hi Ezequiel,
Quoted 7 lines · Ezequiel Garcia
The irqchip driver called
armada_xp_mpic_smp_cpu_init()whenCONFIG_SMP=Yto initialize some per cpu registers. The function is called on each CPU by calling it explicitly on the boot CPU and then using a CPU notifier for the non boot CPUs.This commit removes the
CONFIG_SMPconstrain, so the per cpu registers are also initialized whenCONFIG_SMP=N, which is the right thing to do.
Sometime I find the diff output provided by git not really easy to read.
But about the change itself:
Thanks,
Gregory
Quoted 80 lines
Signed-off-by Ezequiel Garcia <[email protected]>--- drivers/irqchip/irq-armada-370-xp.c | 47 ++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 24 deletions(-)drivers/irqchip/irq-armada-370-xp.c +23
-24@@ -285,28 +285,6 @@ static int armada_370_xp_mpic_irq_map(struct irq_domain *h,return 0 ; } -#ifdef CONFIG_SMP -static void armada_mpic_send_doorbell (const struct cpumask *mask, -unsigned int irq) -{ -int cpu; -unsigned long map =0 ; - -/* Convert our logical CPU mask into a physical one. */ -for_each_cpu (cpu, mask) - map |=1 <<cpu_logical_map (cpu); - -/* -* Ensure that stores to Normal memory are visible to the -* other CPUs before issuing the IPI. -*/ -dsb (); - -/* submit softirq */ -writel ((map <<8 ) | irq, main_int_base + -ARMADA_370_XP_SW_TRIG_INT_OFFS ); -} -static void armada_xp_mpic_smp_cpu_init (void ) {u32 control;@@ -329,6 +307,28 @@ static void armada_xp_mpic_smp_cpu_init(void)writel (0 , per_cpu_int_base +ARMADA_370_XP_INT_CLEAR_MASK_OFFS ); } +#ifdef CONFIG_SMP +static void armada_mpic_send_doorbell (const struct cpumask *mask, +unsigned int irq) +{ +int cpu; +unsigned long map =0 ; + +/* Convert our logical CPU mask into a physical one. */ +for_each_cpu (cpu, mask) + map |=1 <<cpu_logical_map (cpu); + +/* +* Ensure that stores to Normal memory are visible to the +* other CPUs before issuing the IPI. +*/ +dsb (); + +/* submit softirq */ +writel ((map <<8 ) | irq, main_int_base + +ARMADA_370_XP_SW_TRIG_INT_OFFS ); +} +static int armada_xp_mpic_secondary_init (struct notifier_block *nfb,unsigned long action,void *hcpu) {@@ -492,9 +492,8 @@ static int __init armada_370_xp_mpic_of_init(struct device_node *node,BUG_ON (!armada_370_xp_mpic_domain); -#ifdef CONFIG_SMP +/* Setup for the boot CPU */ armada_xp_mpic_smp_cpu_init (); -#endif armada_370_xp_msi_init (node, main_int_res.start);
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 3/7 irqchip: armada-370-xp: Introduce a is_percpu_irq() helper for readability
In reply to Ezequiel GarciaThis commit introduces a helper function is_percpu_irq(), to be used when interrupts are mapped to decide which ones are set as per CPU.
This change will allow to extend the list of per cpu interrupts in a less intrusive fashion; also, it makes the code slightly more readable by keeping a list of the per CPU interrupts.
--- drivers/irqchip/irq-armada-370-xp.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
drivers/irqchip/irq-armada-370-xp.c +11 -1
@@ -73,6 +73,16 @@ static DEFINE_MUTEX(msi_used_lock);static phys_addr_t msi_doorbell_addr;#endif +static inline bool is_percpu_irq (irq_hw_number_t irq) +{ +switch (irq) { +case ARMADA_370_XP_TIMER0_PER_CPU_IRQ : +return true; +default : +return false; + } +} +static void armada_370_xp_irq_mask (struct irq_data *d) {irq_hw_number_t hwirq =irqd_to_hwirq (d);
@@ -271,7 +281,7 @@ static int armada_370_xp_mpic_irq_map(struct irq_domain *h,irq_set_status_flags (virq,IRQ_LEVEL ); -if (hw ==ARMADA_370_XP_TIMER0_PER_CPU_IRQ ) { +if (is_percpu_irq (hw)) {irq_set_percpu_devid (virq);irq_set_chip_and_handler (virq, &armada_370_xp_irq_chip, handle_percpu_devid_irq);
Signature · 1 line
2.1.0
PATCH 3/7 irqchip: armada-370-xp: Introduce a is_percpu_irq() helper for readability
Hi,
Quoted 6 lines · Ezequiel Garcia
This commit introduces a helper function
is_percpu_irq(), to be used when interrupts are mapped to decide which ones are set as per CPU.This change will allow to extend the list of per cpu interrupts in a less intrusive fashion; also, it makes the code slightly more readable by keeping a list of the per CPU interrupts.
I believe you can use the existing (and similarly named) irq_is_percpu for this.
Thanks,
Mark.
Quoted 44 lines
Signed-off-by Ezequiel Garcia <[email protected]>--- drivers/irqchip/irq-armada-370-xp.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)drivers/irqchip/irq-armada-370-xp.c +11
-1@@ -73,6 +73,16 @@ static DEFINE_MUTEX(msi_used_lock);static phys_addr_t msi_doorbell_addr;#endif +static inline bool is_percpu_irq (irq_hw_number_t irq) +{ +switch (irq) { +case ARMADA_370_XP_TIMER0_PER_CPU_IRQ : +return true; +default : +return false; + } +} +static void armada_370_xp_irq_mask (struct irq_data *d) {irq_hw_number_t hwirq =irqd_to_hwirq (d);@@ -271,7 +281,7 @@ static int armada_370_xp_mpic_irq_map(struct irq_domain *h,irq_set_status_flags (virq,IRQ_LEVEL ); -if (hw ==ARMADA_370_XP_TIMER0_PER_CPU_IRQ ) { +if (is_percpu_irq (hw)) {irq_set_percpu_devid (virq);irq_set_chip_and_handler (virq, &armada_370_xp_irq_chip, handle_percpu_devid_irq);Signature · 7 lines
2.1.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 3/7 irqchip: armada-370-xp: Introduce a is_percpu_irq() helper for readability
Quoted 6 lines · Mark Rutland
Hi,
Quoted 3 lines · Ezequiel Garcia
This commit introduces a helper function
is_percpu_irq(), to be used when interrupts are mapped to decide which ones are set as per CPU.
Quoted 6 lines
Quoted 3 lines
This change will allow to extend the list of per cpu interrupts in a less intrusive fashion; also, it makes the code slightly more readable by keeping a list of the per CPU interrupts.
I believe you can use the existing (and similarly named)
irq_is_percpufor this.
No, that won't work. The irq_is_percup will work only after you set the interrupt as per CPU, which is precisely the purpose of armada_370_xp_mpic_irq_map.
At some point, you need to decide tell the kernel which interrupts are per CPU, so we put that choice in an internal helper.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 4/7 irqchip: armada-370-xp: Enable Performance Counter interrupts
In reply to Ezequiel GarciaIn order to get the Performance Counter overflow events (i.e. hardware perf events), we need to unmask the interrupts in the "Coherency Fabric Local Cause" register, which is part of the percpu configuration of the interrupt controller.
--- drivers/irqchip/irq-armada-370-xp.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/irqchip/irq-armada-370-xp.c +9 -0
@@ -37,6 +37,8 @@/* Interrupt Controller Registers Map */ #define ARMADA_370_XP_INT_SET_MASK_OFFS (0x48)#define ARMADA_370_XP_INT_CLEAR_MASK_OFFS (0x4C) +#define ARMADA_370_XP_INT_FABRIC_MASK_OFFS (0x54) +#define ARMADA_370_XP_INT_CAUSE_PERF (cpu) (1 << cpu)#define ARMADA_370_XP_INT_CONTROL (0x00)#define ARMADA_370_XP_INT_SET_ENABLE_OFFS (0x30)
@@ -54,6 +56,7 @@#define ARMADA_370_XP_MAX_PER_CPU_IRQS (28)#define ARMADA_370_XP_TIMER0_PER_CPU_IRQ (5) +#define ARMADA_370_XP_FABRIC_IRQ (3)#define IPI_DOORBELL_START (0)#define IPI_DOORBELL_END (8)
@@ -77,6 +80,7 @@ static inline bool is_percpu_irq(irq_hw_number_t irq) {switch (irq) {case ARMADA_370_XP_TIMER0_PER_CPU_IRQ : +case ARMADA_370_XP_FABRIC_IRQ :return true;default :return false;
@@ -297,12 +301,17 @@ static int armada_370_xp_mpic_irq_map(struct irq_domain *h,static void armada_xp_mpic_smp_cpu_init (void ) { +unsigned long cpuid =cpu_logical_map (smp_processor_id ());u32 control;int nr_irqs, i; control =readl (main_int_base +ARMADA_370_XP_INT_CONTROL ); nr_irqs = (control >>2 ) &0x3ff ; +/* Enable Performance Counter Overflow interrupts */ +writel (ARMADA_370_XP_INT_CAUSE_PERF (cpuid), + per_cpu_int_base +ARMADA_370_XP_INT_FABRIC_MASK_OFFS ); +for (i =0 ; i < nr_irqs; i++)writel (i, per_cpu_int_base +ARMADA_370_XP_INT_SET_MASK_OFFS );
Signature · 1 line
2.1.0
PATCH 5/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 375 SoC
In reply to Ezequiel GarciaThe Armada 375 SoC has a Cortex-A9 CPU, and so the PMU is available to be used. This commit enables it in the devicetree.
--- arch/arm/boot/dts/armada-375.dtsi | 5 +++++ 1 file changed, 5 insertions(+)
arch/arm/boot/dts/armada-375.dtsi +5 -0
@@ -55,6 +55,11 @@ }; }; + pmu { + compatible = "arm,cortex-a9-pmu"; + interrupts-extended = <&mpic 3>; + }; + soc { compatible = "marvell,armada375-mbus", "marvell,armada370-mbus", "simple-bus"; #address-cells = <2>;
Signature · 1 line
2.1.0
PATCH 5/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 375 SoC
Quoted 13 lines · Ezequiel Garcia
The Armada 375 SoC has a Cortex-A9 CPU, and so the PMU is available to be used. This commit enables it in the devicetree.
Signed-off-by Ezequiel Garcia <[email protected]>--- arch/arm/boot/dts/armada-375.dtsi | 5 +++++ 1 file changed, 5 insertions(+)arch/arm/boot/dts/armada-375.dtsi +0
-0
arch/arm/boot/dts/armada-375.dtsi line 61
Quoted 7 lines
arch/arm/boot/dts/armada-375.dtsi +4
-0@@ -55,6 +55,11 @@ }; }; + pmu { + compatible = "arm,cortex-a9-pmu"; + interrupts-extended = <&mpic 3>; + };
Just to check - the interrupts from both CPUs are muxed into a single line into the interrupt controller?
This isn't gonig to work at the moment -- the perf code will associate this interrupt with CPU0 and you'll lose events on CPU1.
Hopefully there's a separate interrupt for CPU1?
Mark.
Quoted 13 lines
arch/arm/boot/dts/armada-375.dtsi +1
-0+ soc { compatible = "marvell,armada375-mbus", "marvell,armada370-mbus", "simple-bus"; #address-cells = <2>;Signature · 7 lines
2.1.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 5/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 375 SoC
Hi Mark,
Thanks for the reply. You made me research and test this in depth :)
Quoted 25 lines · Mark Rutland
Quoted 13 lines · Ezequiel Garcia
The Armada 375 SoC has a Cortex-A9 CPU, and so the PMU is available to be used. This commit enables it in the devicetree.
Signed-off-by Ezequiel Garcia <[email protected]>--- arch/arm/boot/dts/armada-375.dtsi | 5 +++++ 1 file changed, 5 insertions(+)arch/arm/boot/dts/armada-375.dtsi +0
-0arch/arm/boot/dts/armada-375.dtsi line 61
Quoted 7 lines
arch/arm/boot/dts/armada-375.dtsi +4
-0@@ -55,6 +55,11 @@ }; }; + pmu { + compatible = "arm,cortex-a9-pmu"; + interrupts-extended = <&mpic 3>; + };Just to check - the interrupts from both CPUs are muxed into a single line into the interrupt controller?
Quoted 5 lines
This isn't gonig to work at the moment -- the perf code will associate this interrupt with CPU0 and you'll lose events on CPU1.
Hopefully there's a separate interrupt for CPU1?
The <mpic 3> is a per CPU interrupt.
Actually, the interrupt contains more than just PMU events, it contains a summary of several CPU events: Perf counters for each CPU, Power management interrupts for each CPU, L2 cache interrupt, among others.
The interrupt cause can be read from a banked register called "CPU subsystem local cause". Conversely, each of these must be enabled from another (banked) register.
As far as I understand, this works (or should work) because the irqchip driver enables *just* the perf counter interrupt for the running CPU in the armada_xp_mpic_secondary_init(). Then, the ARM perf code requests the interrupt as per cpu and things just work, right?
However:
1) In the Armada 375 SoC case, the MPIC is chained and the code does not register the cpu notifier, so armada_xp_mpic_secondary_init is not called on CPU1.
2) Even when ensuring armada_xp_mpic_secondary_init is called on each CPU, and thus each perf counter interrupt is enabled, I can't see the PMU interrupt for CPU1, but just the one for the boot CPU.
I'll check with the hardware designer about this.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 5/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 375 SoC
Dear Ezequiel Garcia,
Quoted 5 lines · Ezequiel Garcia
The <mpic 3> is a per CPU interrupt.
Actually, the interrupt contains more than just PMU events, it contains a summary of several CPU events: Perf counters for each CPU, Power management interrupts for each CPU, L2 cache interrupt, among others.
This is kind of a side discussion but if this <mpic 3> interrupts does much more than PMU events, then we should implement a separate irqchip driver for this, to "demultiplex" the events notified by this interrupt.
Yes, today we are only using the PMU events from <mpic 3>, but what if tomorrow we need to be notified of other events in other drivers? We would be screwed, because we would have to change the DT
representation: the PMU would no longer take <&mpic 3> as the interrupt, but <&some_other_irq_controller XYZ> as the interrupt.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 5/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 375 SoC
Quoted 6 lines · Thomas Petazzoni
Dear Ezequiel Garcia,
Quoted 2 lines · Ezequiel Garcia
The <mpic 3> is a per CPU interrupt.
Quoted 8 lines
Quoted 3 lines
Actually, the interrupt contains more than just PMU events, it contains a summary of several CPU events: Perf counters for each CPU, Power management interrupts for each CPU, L2 cache interrupt, among others.
This is kind of a side discussion but if this <mpic 3> interrupts does much more than PMU events, then we should implement a separate irqchip driver for this, to "demultiplex" the events notified by this interrupt.
Oh, I didn't realize this was possible.
Quoted 6 lines
Yes, today we are only using the PMU events from <mpic 3>, but what if tomorrow we need to be notified of other events in other drivers? We would be screwed, because we would have to change the DT
representation: the PMU would no longer take <&mpic 3> as the interrupt, but <&some_other_irq_controllerXYZ> as the interrupt.
Yes, that sounds like a much better approach. I'll take a look and try to prepare something.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 5/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 375 SoC
Dear Ezequiel Garcia,
Quoted 8 lines · Ezequiel Garcia
Quoted 8 lines · Ezequiel Garcia
Quoted 6 lines
The <mpic 3> is a per CPU interrupt.
Actually, the interrupt contains more than just PMU events, it contains a summary of several CPU events: Perf counters for each CPU, Power management interrupts for each CPU, L2 cache interrupt, among others.
Quoted 5 lines
Quoted 3 lines
This is kind of a side discussion but if this <mpic 3> interrupts does much more than PMU events, then we should implement a separate irqchip driver for this, to "demultiplex" the events notified by this interrupt.
Oh, I didn't realize this was possible.
The only trick is that it can't be a separate DT node, because the registers that contains the mask/cause informations for the events notified by <mpic 3> belongs to the MPIC registers area.
Not sure how to handle that, maybe Mark will have some suggestions.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 5/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 375 SoC
Quoted 16 lines · Thomas Petazzoni
Dear Ezequiel Garcia,
Quoted 12 lines · Ezequiel Garcia
Quoted 11 lines · Ezequiel Garcia
Quoted 6 lines
The <mpic 3> is a per CPU interrupt.
Actually, the interrupt contains more than just PMU events, it contains a summary of several CPU events: Perf counters for each CPU, Power management interrupts for each CPU, L2 cache interrupt, among others.
This is kind of a side discussion but if this <mpic 3> interrupts does much more than PMU events, then we should implement a separate irqchip driver for this, to "demultiplex" the events notified by this interrupt.
Quoted 6 lines
Quoted 1 line
Oh, I didn't realize this was possible.
The only trick is that it can't be a separate DT node, because the registers that contains the mask/cause informations for the events notified by <mpic 3> belongs to the MPIC registers area.
Technically speaking, we can stop requesting the region in the mpic irqchip driver and share the registers, just as we do in other drivers.
Not the most elegant thing to do, though.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 5/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 375 SoC
In reply to Thomas PetazzoniQuoted 16 lines · Thomas Petazzoni
Dear Ezequiel Garcia,
Quoted 12 lines · Ezequiel Garcia
Quoted 11 lines · Ezequiel Garcia
Quoted 6 lines
The <mpic 3> is a per CPU interrupt.
Actually, the interrupt contains more than just PMU events, it contains a summary of several CPU events: Perf counters for each CPU, Power management interrupts for each CPU, L2 cache interrupt, among others.
This is kind of a side discussion but if this <mpic 3> interrupts does much more than PMU events, then we should implement a separate irqchip driver for this, to "demultiplex" the events notified by this interrupt.
Quoted 7 lines
Quoted 1 line
Oh, I didn't realize this was possible.
The only trick is that it can't be a separate DT node, because the registers that contains the mask/cause informations for the events notified by <mpic 3> belongs to the MPIC registers area.
Not sure how to handle that, maybe Mark will have some suggestions.
I'm not sure I follow why you would need a separate irqchip driver. Why can't this live in the existing mpic driver?
Thanks,
Mark.
PATCH 5/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 375 SoC
Quoted 19 lines · Mark Rutland
Quoted 18 lines · Thomas Petazzoni
Dear Ezequiel Garcia,
Quoted 13 lines · Ezequiel Garcia
Quoted 11 lines · Ezequiel Garcia
Quoted 6 lines
The <mpic 3> is a per CPU interrupt.
Actually, the interrupt contains more than just PMU events, it contains a summary of several CPU events: Perf counters for each CPU, Power management interrupts for each CPU, L2 cache interrupt, among others.
This is kind of a side discussion but if this <mpic 3> interrupts does much more than PMU events, then we should implement a separate irqchip driver for this, to "demultiplex" the events notified by this interrupt.
Oh, I didn't realize this was possible.
Quoted 9 lines
Quoted 5 lines
The only trick is that it can't be a separate DT node, because the registers that contains the mask/cause informations for the events notified by <mpic 3> belongs to the MPIC registers area.
Not sure how to handle that, maybe Mark will have some suggestions.
I'm not sure I follow why you would need a separate irqchip driver. Why can't this live in the existing mpic driver?
I've tried to add a demux interrupt controller for the CPU summary interrupts to be able to hook to the proper interrupt. This was doable without much pain [1].
However, due to the way the Performance counter overflow IRQ is exposed, it doesn't seem to meet perf's PMU irq handling requirement.
For per CPU interrupts, perf requests the interrupt and expects to get an interrupt on each CPU, with the counter overflow event for that CPU.
This is not the case for the CPU Summary interrupt. This *is* a per CPU interrupt, but there is a separate interrupt line for each CPU:
CPU summary per CPU interrupt 0, for the Perf counter on CPU0 CPU summary per CPU interrupt 1, for the Perf counter on CPU1
So, I thought about exposing the interrupt as a shared one and use interrupts 0 and 1, triggering interrupts on CPU0 with the counter overflow for CPU0 and CPU1. This doesn't work either, as the perf code expects to set the interrupt affinity to route each shared interrupt to the appropriate CPU.
I'm not sure how can I set the interrupt affinity for the demux controller, being a chained interrupt controller.
At this point, due to the SoC weirdness in exposing the PMU IRQ, I'm starting to think we will have to live with software events for this SoC, but I'd love to be proved wrong.
Signature · 11 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141031/0a53de45/attachment.sig>
PATCH 5/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 375 SoC
In reply to Ezequiel GarciaQuoted 30 lines · Ezequiel Garcia
Hi Mark,
Thanks for the reply. You made me research and test this in depth :)
Quoted 25 lines · Mark Rutland
Quoted 13 lines · Ezequiel Garcia
The Armada 375 SoC has a Cortex-A9 CPU, and so the PMU is available to be used. This commit enables it in the devicetree.
Signed-off-by Ezequiel Garcia <[email protected]>--- arch/arm/boot/dts/armada-375.dtsi | 5 +++++ 1 file changed, 5 insertions(+)arch/arm/boot/dts/armada-375.dtsi +0
-0arch/arm/boot/dts/armada-375.dtsi line 61
Quoted 7 lines
arch/arm/boot/dts/armada-375.dtsi +4
-0@@ -55,6 +55,11 @@ }; }; + pmu { + compatible = "arm,cortex-a9-pmu"; + interrupts-extended = <&mpic 3>; + };Just to check - the interrupts from both CPUs are muxed into a single line into the interrupt controller?
Quoted 7 lines
Quoted 5 lines
This isn't gonig to work at the moment -- the perf code will associate this interrupt with CPU0 and you'll lose events on CPU1.
Hopefully there's a separate interrupt for CPU1?
The <mpic 3> is a per CPU interrupt.
Ah, ok. I hadn't realised that was the case. That should be fine, then.
Quoted 7 lines
Actually, the interrupt contains more than just PMU events, it contains a summary of several CPU events: Perf counters for each CPU, Power management interrupts for each CPU, L2 cache interrupt, among others.
The interrupt cause can be read from a banked register called "CPU subsystem local cause". Conversely, each of these must be enabled from another (banked) register.
I think that as Thomas said in his reply that it would make sense to expose these as separate interrupts out of the mpic, given we have the capability to distinguish them at the mpic. That will prevent a lot of pain (e.g. percpu interrupts can't currently be shared), and would get rid of the need to hack armada_xp_mpic_secondary_init to enable a particular interrupt.
Quoted 4 lines
As far as I understand, this works (or should work) because the irqchip driver enables *just* the perf counter interrupt for the running CPU in the
armada_xp_mpic_secondary_init(). Then, the ARM perf code requests the interrupt as per cpu and things just work, right?
That would probably work currently, but I haven't thought about it in great detail.
It would be nicer if we enabled the perf counter interrupt only in response to an enable_percpu_irq though.
Quoted 11 lines
However:
1) In the Armada 375 SoC case, the MPIC is chained and the code does not register the cpu notifier, so
armada_xp_mpic_secondary_initis not called on CPU1.2) Even when ensuring
armada_xp_mpic_secondary_initis called on each CPU, and thus each perf counter interrupt is enabled, I can't see the PMU interrupt for CPU1, but just the one for the boot CPU.I'll check with the hardware designer about this.
Ok. It would be nice to get to the bottom of that. It doesn't seem like a great idea to add the PMU node with a potential dodgy interrupt -- you could get misleading output from perf in the case of overflows.
Thanks,
Mark.
PATCH 6/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 380/385 SoC
In reply to Ezequiel GarciaThe Armada 380 and 385 SoCs have a Cortex-A9 CPU, so the PMU is available to be used. This commit enables it in the devicetree.
--- arch/arm/boot/dts/armada-38x.dtsi | 5 +++++ 1 file changed, 5 insertions(+)
arch/arm/boot/dts/armada-38x.dtsi +5 -0
@@ -30,6 +30,11 @@ eth2 = ð2; }; + pmu { + compatible = "arm,cortex-a9-pmu"; + interrupts-extended = <&mpic 3>; + }; + soc { compatible = "marvell,armada380-mbus", "marvell,armada370-mbus", "simple-bus";
Signature · 1 line
2.1.0
PATCH 6/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 380/385 SoC
Quoted 13 lines · Ezequiel Garcia
The Armada 380 and 385 SoCs have a Cortex-A9 CPU, so the PMU is available to be used. This commit enables it in the devicetree.
Signed-off-by Ezequiel Garcia <[email protected]>--- arch/arm/boot/dts/armada-38x.dtsi | 5 +++++ 1 file changed, 5 insertions(+)arch/arm/boot/dts/armada-38x.dtsi +0
-0
arch/arm/boot/dts/armada-38x.dtsi line 36
Quoted 7 lines
arch/arm/boot/dts/armada-38x.dtsi +4
-0@@ -30,6 +30,11 @@ eth2 = ð2; }; + pmu { + compatible = "arm,cortex-a9-pmu"; + interrupts-extended = <&mpic 3>; + };
Is there not a separate interrupt for CPU1 on Armada 385?
Mark.
PATCH 6/7 ARM: mvebu: Enable Performance Monitor Unit on Armada 380/385 SoC
Quoted 17 lines · Mark Rutland
Quoted 16 lines · Ezequiel Garcia
The Armada 380 and 385 SoCs have a Cortex-A9 CPU, so the PMU is available to be used. This commit enables it in the devicetree.
Signed-off-by Ezequiel Garcia <[email protected]>--- arch/arm/boot/dts/armada-38x.dtsi | 5 +++++ 1 file changed, 5 insertions(+)arch/arm/boot/dts/armada-38x.dtsi +0
-0@@ -30,6 +30,11 @@ eth2 = ð2; };
Quoted 7 lines
arch/arm/boot/dts/armada-38x.dtsi line 36
Quoted 4 lines
arch/arm/boot/dts/armada-38x.dtsi +4
-0+ pmu { + compatible = "arm,cortex-a9-pmu"; + interrupts-extended = <&mpic 3>; + };Is there not a separate interrupt for CPU1 on Armada 385?
As I explained in my reply to PATCH 5/7, this is a per cpu interrupt.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 7/7 ARM: mvebu: Enable perf support in mvebu_v7_defconfig
In reply to Ezequiel GarciaNow that Armada 375/38x have support for the PMU, this commit enables perf events in the defconfig.
--- arch/arm/configs/mvebu_v7_defconfig | 1 + 1 file changed, 1 insertion(+)
arch/arm/configs/mvebu_v7_defconfig +1 -0
@@ -5,6 +5,7 @@ CONFIG_HIGH_RES_TIMERS=y CONFIG_LOG_BUF_SHIFT=14 CONFIG_BLK_DEV_INITRD=y CONFIG_EXPERT=y +CONFIG_PERF_EVENTS=y CONFIG_SLAB=y CONFIG_MODULES=y CONFIG_MODULE_UNLOAD=y
Signature · 1 line
2.1.0
PATCH 7/7 ARM: mvebu: Enable perf support in mvebu_v7_defconfig
Quoted 13 lines · Ezequiel Garcia
Now that Armada 375/38x have support for the PMU, this commit enables perf events in the defconfig.
Signed-off-by Ezequiel Garcia <[email protected]>--- arch/arm/configs/mvebu_v7_defconfig | 1 + 1 file changed, 1 insertion(+)arch/arm/configs/mvebu_v7_defconfig +0
-0
arch/arm/configs/mvebu_v7_defconfig CONFIG_HIGH_RES_TIMERS=y · line 8
Quoted 4 lines
arch/arm/configs/mvebu_v7_defconfig +1
-0@@ -5,6 +5,7 @@ CONFIG_HIGH_RES_TIMERS=y CONFIG_LOG_BUF_SHIFT=14 CONFIG_BLK_DEV_INITRD=y CONFIG_EXPERT=y +CONFIG_PERF_EVENTS=y
Isn't this provided by CONFIG_ARM already?
Surely you want CONFIG_HW_PERF_EVENTS?
Thanks,
Mark.
Quoted 12 lines
arch/arm/configs/mvebu_v7_defconfig +0
-0CONFIG_SLAB=y CONFIG_MODULES=y CONFIG_MODULE_UNLOAD=ySignature · 7 lines
2.1.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 7/7 ARM: mvebu: Enable perf support in mvebu_v7_defconfig
Quoted 9 lines · Mark Rutland
Quoted 8 lines · Ezequiel Garcia
Now that Armada 375/38x have support for the PMU, this commit enables perf events in the defconfig.
Signed-off-by Ezequiel Garcia <[email protected]>--- arch/arm/configs/mvebu_v7_defconfig | 1 + 1 file changed, 1 insertion(+)
Quoted 12 lines
arch/arm/configs/mvebu_v7_defconfig CONFIG_HIGH_RES_TIMERS=y · line 8
Quoted 9 lines
arch/arm/configs/mvebu_v7_defconfig +1
-0@@ -5,6 +5,7 @@ CONFIG_HIGH_RES_TIMERS=y CONFIG_LOG_BUF_SHIFT=14 CONFIG_BLK_DEV_INITRD=y CONFIG_EXPERT=y +CONFIG_PERF_EVENTS=yIsn't this provided by
CONFIG_ARMalready?
No, you need to select it explicitly, as the other platforms do.
Quoted 2 lines
Surely you want
CONFIG_HW_PERF_EVENTS?
Yes, but it defaults to 'yes' if PERF_EVENTS is enabled, so you don't need it in the defconfig.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 7/7 ARM: mvebu: Enable perf support in mvebu_v7_defconfig
[...]
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
+CONFIG_PERF_EVENTS=y
Isn't this provided by
CONFIG_ARMalready?No, you need to select it explicitly, as the other platforms do.
Sorry, I'd misread HAVE_PERF_EVENTS. Apologies for the noise.
Mark.
cover Armada 375/38x perf support, and a bonus irqchip driver simplification
In reply to Ezequiel GarciaQuoted 10 lines · Ezequiel Garcia
This patchset enables support for perf hardware events, by enabling the PMU interrupts in the irqchip driver.
While doing this, we noticed the driver could use some cleaning to simplify the overly complex implementation of the .map(), .unmask() and .mask() functions.
The first three patches are the result of this cleaning effort, while the rest of the series is the Perf support for Armada 375 and Armada 38x SoCs.
Quoted 8 lines
The series is based on v3.18-rc1.
Ezequiel Garcia (7):
irqchip: armada-370-xp: Simplify interrupt map, mask and unmask
irqchip: armada-370-xp: Initialize per cpu registers whenCONFIG_SMP=N
irqchip: armada-370-xp: Introduce ais_percpu_irq()helper for
readability
irqchip: armada-370-xp: Enable Performance Counter interrupts
Patches 1 to 4 tentatively applied to irqchip/mvebu. Things have been quiet, so let's get it in next for some more test coverage.
thx,
Jason.
Quoted 13 lines
ARM: mvebu: Enable Performance Monitor Unit on Armada 375 SoC
ARM: mvebu: Enable Performance Monitor Unit on Armada 380/385 SoC
ARM: mvebu: Enable perf support inmvebu_v7_defconfigarch/arm/boot/dts/armada-375.dtsi | 5 ++ arch/arm/boot/dts/armada-38x.dtsi | 5 ++ arch/arm/configs/mvebu_v7_defconfig | 1 + drivers/irqchip/irq-armada-370-xp.c | 95 ++++++++++++++++++------------------- 4 files changed, 58 insertions(+), 48 deletions(-)Signature · 1 line
2.1.0
cover Armada 375/38x perf support, and a bonus irqchip driver simplification
Jason, Ezequiel,
Quoted 20 lines · Jason Cooper
Quoted 10 lines · Ezequiel Garcia
This patchset enables support for perf hardware events, by enabling the PMU interrupts in the irqchip driver.
While doing this, we noticed the driver could use some cleaning to simplify the overly complex implementation of the .map(), .unmask() and .mask() functions.
The first three patches are the result of this cleaning effort, while the rest of the series is the Perf support for Armada 375 and Armada 38x SoCs.
Quoted 8 lines
The series is based on v3.18-rc1.
Ezequiel Garcia (7):
irqchip: armada-370-xp: Simplify interrupt map, mask and unmask
irqchip: armada-370-xp: Initialize per cpu registers whenCONFIG_SMP=N
irqchip: armada-370-xp: Introduce ais_percpu_irq()helper for
readability
irqchip: armada-370-xp: Enable Performance Counter interrupts
Quoted 2 lines
Patches 1 to 4 tentatively applied to irqchip/mvebu. Things have been quiet, so let's get it in next for some more test coverage.
Ezequiel, are these patches really the version we want to see merged? I think you're working on a different implementation that demultiplex the coherency fabric events interrupt.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
cover Armada 375/38x perf support, and a bonus irqchip driver simplification
Quoted 26 lines · Thomas Petazzoni
Jason, Ezequiel,
Quoted 22 lines · Jason Cooper
Quoted 10 lines · Ezequiel Garcia
This patchset enables support for perf hardware events, by enabling the PMU interrupts in the irqchip driver.
While doing this, we noticed the driver could use some cleaning to simplify the overly complex implementation of the .map(), .unmask() and .mask() functions.
The first three patches are the result of this cleaning effort, while the rest of the series is the Perf support for Armada 375 and Armada 38x SoCs.
Quoted 8 lines
The series is based on v3.18-rc1.
Ezequiel Garcia (7):
irqchip: armada-370-xp: Simplify interrupt map, mask and unmask
irqchip: armada-370-xp: Initialize per cpu registers whenCONFIG_SMP=N
irqchip: armada-370-xp: Introduce ais_percpu_irq()helper for
readability
irqchip: armada-370-xp: Enable Performance Counter interruptsPatches 1 to 4 tentatively applied to irqchip/mvebu. Things have been quiet, so let's get it in next for some more test coverage.
Jason, please consider *only* patch 1 for inclusion, as long as we have Gregory's Ack on it. I don't want to break the irqchip driver because of a clean-up! On the other side, if such weird handling is needed, we need find why and document it.
Quoted 4 lines
Ezequiel, are these patches really the version we want to see merged? I think you're working on a different implementation that demultiplex the coherency fabric events interrupt.
Indeed, I never managed to get Perf counter overflow events interrupt on both CPUs, so this is on hold for now (as I explained here
http://www.spinics.net/lists/arm-kernel/msg373929.html).
Let me fetch that and save you a click :)
"""
I've tried to add a demux interrupt controller for the CPU summary interrupts to be able to hook to the proper interrupt. This was doable without much pain [1].
However, due to the way the Performance counter overflow IRQ is exposed, it doesn't seem to meet perf's PMU irq handling requirement.
For per CPU interrupts, perf requests the interrupt and expects to get an interrupt on each CPU, with the counter overflow event for that CPU.
This is not the case for the CPU Summary interrupt. This *is* a per CPU interrupt, but there is a separate interrupt line for each CPU:
CPU summary per CPU interrupt 0, for the Perf counter on CPU0 CPU summary per CPU interrupt 1, for the Perf counter on CPU1
So, I thought about exposing the interrupt as a shared one and use interrupts 0 and 1, triggering interrupts on CPU0 with the counter overflow for CPU0 and CPU1. This doesn't work either, as the perf code expects to set the interrupt affinity to route each shared interrupt to the appropriate CPU.
I'm not sure how can I set the interrupt affinity for the demux controller, being a chained interrupt controller.
At this point, due to the SoC weirdness in exposing the PMU IRQ, I'm starting to think we will have to live with software events for this SoC, but I'd love to be proved wrong.
[1] http://sprunge.us/MfVN
"""
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
cover Armada 375/38x perf support, and a bonus irqchip driver simplification
Quoted 24 lines · Ezequiel Garcia
Quoted 23 lines · Thomas Petazzoni
Jason, Ezequiel,
Quoted 20 lines · Jason Cooper
Quoted 18 lines · Ezequiel Garcia
This patchset enables support for perf hardware events, by enabling the PMU interrupts in the irqchip driver.
While doing this, we noticed the driver could use some cleaning to simplify the overly complex implementation of the .map(), .unmask() and .mask() functions.
The first three patches are the result of this cleaning effort, while the rest of the series is the Perf support for Armada 375 and Armada 38x SoCs.
The series is based on v3.18-rc1.
Ezequiel Garcia (7):
irqchip: armada-370-xp: Simplify interrupt map, mask and unmask
irqchip: armada-370-xp: Initialize per cpu registers whenCONFIG_SMP=N
irqchip: armada-370-xp: Introduce ais_percpu_irq()helper for
readability
irqchip: armada-370-xp: Enable Performance Counter interrupts
Quoted 8 lines
Quoted 3 lines
Quoted 2 lines
Patches 1 to 4 tentatively applied to irqchip/mvebu. Things have been quiet, so let's get it in next for some more test coverage.
Jason, please consider *only* patch 1 for inclusion, as long as we have Gregory's Ack on it. I don't want to break the irqchip driver because of a clean-up! On the other side, if such weird handling is needed, we need find why and document it.
No sweat. I've now rolled irqchip/mvebu back to:
aaae00a7fa86 irqchip: armada-370-xp: Simplify interrupt map, mask and unmask
and an updated for-next is being build-tested and pushed hopefully before Stephen gets his cup of coffee. :)
thx,
Jason.
cover Armada 375/38x perf support, and a bonus irqchip driver simplification
Quoted 34 lines · Jason Cooper
Quoted 24 lines · Ezequiel Garcia
Quoted 23 lines · Thomas Petazzoni
Jason, Ezequiel,
Quoted 20 lines · Jason Cooper
Quoted 18 lines · Ezequiel Garcia
This patchset enables support for perf hardware events, by enabling the PMU interrupts in the irqchip driver.
While doing this, we noticed the driver could use some cleaning to simplify the overly complex implementation of the .map(), .unmask() and .mask() functions.
The first three patches are the result of this cleaning effort, while the rest of the series is the Perf support for Armada 375 and Armada 38x SoCs.
The series is based on v3.18-rc1.
Ezequiel Garcia (7):
irqchip: armada-370-xp: Simplify interrupt map, mask and unmask
irqchip: armada-370-xp: Initialize per cpu registers whenCONFIG_SMP=N
irqchip: armada-370-xp: Introduce ais_percpu_irq()helper for
readability
irqchip: armada-370-xp: Enable Performance Counter interruptsQuoted 8 lines
Quoted 3 lines
Quoted 2 lines
Patches 1 to 4 tentatively applied to irqchip/mvebu. Things have been quiet, so let's get it in next for some more test coverage.
Jason, please consider *only* patch 1 for inclusion, as long as we have Gregory's Ack on it. I don't want to break the irqchip driver because of a clean-up! On the other side, if such weird handling is needed, we need find why and document it.
Quoted 8 lines
No sweat. I've now rolled irqchip/mvebu back to:
aaae00a7fa86irqchip: armada-370-xp: Simplify interrupt map, mask and unmaskand an updated for-next is being build-tested and pushed hopefully before Stephen gets his cup of coffee. :)
Jason,
Please drop that one as well. Gregory and I have been discussing some more and we concluded that the change is wrong.
I'm working on a new series, but it'll take some time.
Thanks and sorry for the huge delay,
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com