gic : check if there are pending interrupts
PATCH 1/1 gic : check if there are pending interrupts
The following patch checks if there are pending interrupts on the gic.
This function is needed for example for the ux500 cpuidle driver. When the A9 cores and the gic are decoupled from the PRCMU, the idle routine has to check if an interrupt is pending on the gic before entering in retention mode.
--- arch/arm/common/gic.c | 37 +++++++++++++++++++++++++++++++++++ arch/arm/include/asm/hardware/gic.h | 2 +- 2 files changed, 38 insertions(+), 1 deletions(-)
arch/arm/common/gic.c +37 -0
@@ -750,6 +750,43 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) }#endif +/* +* gic_pending_irq - checks if there are pending interrupts on the gic +* +* Disabling an interrupt only disables the forwarding of the +* interrupt to any CPU interface. It does not prevent the interrupt +* from changing state, for example becoming pending, or active and +* pending if it is already active. For this reason, we have to check +* the interrupt is pending *and* is enabled. +* +* Returns true if there are pending and enabled interrupts, false +* otherwise. +*/ +bool gic_pending_irq (unsigned int gic_nr) +{ +u32 pr;/* Pending register */ +u32 er;/* Enable register */ +void __iomem *dist_base; +int gic_irqs; +int i; + +BUG_ON (gic_nr >=MAX_GIC_NR ); + + gic_irqs = gic_data[gic_nr].gic_irqs; + dist_base =gic_data_dist_base (&gic_data[gic_nr]); + +for (i =0 ; i <DIV_ROUND_UP (gic_irqs,32 ); i++) { + + pr =readl_relaxed (dist_base +GIC_DIST_PENDING_SET + i *4 ); + er =readl_relaxed (dist_base +GIC_DIST_ENABLE_SET + i *4 ); + +if (pr & er) +return true; + } + +return false; +} +#ifdef CONFIG_OF static int gic_cnt__initdata =0 ;
arch/arm/include/asm/hardware/gic.h +1 -1
@@ -45,7 +45,7 @@ void gic_secondary_init(unsigned int);void gic_handle_irq (struct pt_regs *regs);void gic_cascade_irq (unsigned int gic_nr,unsigned int irq);void gic_raise_softirq (const struct cpumask *mask,unsigned int irq); - +bool gic_pending_irq (unsigned int gic_nr);static inline void gic_init (unsigned int nr,int start,void __iomem *dist ,void __iomem *cpu) {
Signature · 1 line
1.7.5.4
PATCH 1/1 gic : check if there are pending interrupts
Quoted 6 lines · Daniel Lezcano
The following patch checks if there are pending interrupts on the gic.
This function is needed for example for the ux500 cpuidle driver. When the A9 cores and the gic are decoupled from the PRCMU, the idle routine has to check if an interrupt is pending on the gic before entering in retention mode.
That sounds racy. Soon as you check an interrupt can assert.
Quoted 18 lines
Signed-off-by Daniel Lezcano <[email protected]>--- arch/arm/common/gic.c | 37 +++++++++++++++++++++++++++++++++++ arch/arm/include/asm/hardware/gic.h | 2 +- 2 files changed, 38 insertions(+), 1 deletions(-)arch/arm/common/gic.c +3
-0@@ -750,6 +750,43 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) }#endif +/* +* gic_pending_irq - checks if there are pending interrupts on the gic +*
arch/arm/common/gic.c void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) · line 765
Quoted 10 lines
arch/arm/common/gic.c +10
-0+* Disabling an interrupt only disables the forwarding of the +* interrupt to any CPU interface. It does not prevent the interrupt +* from changing state, for example becoming pending, or active and +* pending if it is already active. For this reason, we have to check +* the interrupt is pending *and* is enabled. +* +* Returns true if there are pending and enabled interrupts, false +* otherwise. +*/ +bool gic_pending_irq (unsigned int gic_nr)
Seems like this should be solved with a generic interface and not gic specific.
Would we ever need this for a secondary gic (gic_nr != 0)?
arch/arm/common/gic.c void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) · line 778
Quoted 13 lines
arch/arm/common/gic.c +13
-0+{ +u32 pr;/* Pending register */ +u32 er;/* Enable register */ +void __iomem *dist_base; +int gic_irqs; +int i; + +BUG_ON (gic_nr >=MAX_GIC_NR ); + + gic_irqs = gic_data[gic_nr].gic_irqs; + dist_base =gic_data_dist_base (&gic_data[gic_nr]); + +for (i =0 ; i <DIV_ROUND_UP (gic_irqs,32 ); i++) {
Wouldn't you want to skip PPIs if the CPU interface is disabled?
arch/arm/common/gic.c void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) · line 782
Quoted 4 lines
arch/arm/common/gic.c +4
-0+ + pr =readl_relaxed (dist_base +GIC_DIST_PENDING_SET + i *4 ); + er =readl_relaxed (dist_base +GIC_DIST_ENABLE_SET + i *4 ); +
What if an interrupt is pending, but routed to a core which is not getting put into low power state?
Rob
Quoted 23 lines
arch/arm/common/gic.c +7
-0+if (pr & er) +return true; + } + +return false; +} +#ifdef CONFIG_OF static int gic_cnt__initdata =0 ;arch/arm/include/asm/hardware/gic.h +1
-1@@ -45,7 +45,7 @@ void gic_secondary_init(unsigned int);void gic_handle_irq (struct pt_regs *regs);void gic_cascade_irq (unsigned int gic_nr,unsigned int irq);void gic_raise_softirq (const struct cpumask *mask,unsigned int irq); - +bool gic_pending_irq (unsigned int gic_nr);static inline void gic_init (unsigned int nr,int start,void __iomem *dist ,void __iomem *cpu) {
PATCH 1/1 gic : check if there are pending interrupts
Quoted 9 lines · Rob Herring
Quoted 6 lines · Daniel Lezcano
The following patch checks if there are pending interrupts on the gic.
This function is needed for example for the ux500 cpuidle driver. When the A9 cores and the gic are decoupled from the PRCMU, the idle routine has to check if an interrupt is pending on the gic before entering in retention mode.
That sounds racy. Soon as you check an interrupt can assert.
Yes it sounds racy but not for the ux500 where the gic can be decoupled and the irq are catched by the PRCMU to wake up the core A9.
Quoted 32 lines
Quoted 17 lines
Signed-off-by Daniel Lezcano<[email protected]>--- arch/arm/common/gic.c | 37 +++++++++++++++++++++++++++++++++++ arch/arm/include/asm/hardware/gic.h | 2 +- 2 files changed, 38 insertions(+), 1 deletions(-)arch/arm/common/gic.c +3
-0@@ -750,6 +750,43 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) }#endif +/* +* gic_pending_irq - checks if there are pending interrupts on the gic +* arch/arm/common/gic.c void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) · line 765
Quoted 10 lines
arch/arm/common/gic.c +10
-0+* Disabling an interrupt only disables the forwarding of the +* interrupt to any CPU interface. It does not prevent the interrupt +* from changing state, for example becoming pending, or active and +* pending if it is already active. For this reason, we have to check +* the interrupt is pending *and* is enabled. +* +* Returns true if there are pending and enabled interrupts, false +* otherwise. +*/ +bool gic_pending_irq (unsigned int gic_nr)Seems like this should be solved with a generic interface and not gic specific.
Quoted 1 line
Would we ever need this for a secondary gic (
gic_nr!= 0)?
I don't think so for this specific routine which will be used by the ux500 for the moment.
Quoted 16 lines
arch/arm/common/gic.c void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
Quoted 13 lines
arch/arm/common/gic.c +13
-0+{ +u32 pr;/* Pending register */ +u32 er;/* Enable register */ +void __iomem *dist_base; +int gic_irqs; +int i; + +BUG_ON (gic_nr>=MAX_GIC_NR ); + + gic_irqs = gic_data[gic_nr].gic_irqs; + dist_base =gic_data_dist_base (&gic_data[gic_nr]); + +for (i =0 ; i<DIV_ROUND_UP (gic_irqs,32 ); i++) {Wouldn't you want to skip PPIs if the CPU interface is disabled?
Quoted 7 lines
arch/arm/common/gic.c void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) · line 782
Quoted 4 lines
arch/arm/common/gic.c +4
-0+ + pr =readl_relaxed (dist_base +GIC_DIST_PENDING_SET + i *4 ); + er =readl_relaxed (dist_base +GIC_DIST_ENABLE_SET + i *4 ); +What if an interrupt is pending, but routed to a core which is not getting put into low power state?
Hmm, right. That does not happen with the ux500 because the gic is decoupled from the cores. Checking if some irq are pending on the gic makes sense only on this case which is very specific to the ux500 SoC. I am wondering if that would not make sense to move this routine to the mfd's prcmu.c file, no ?
-- Daniel
Signature · 5 lines
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
PATCH 1/1 gic : check if there are pending interrupts
In reply to Daniel LezcanoQuoted 6 lines · Daniel Lezcano
The following patch checks if there are pending interrupts on the gic.
This function is needed for example for the ux500 cpuidle driver. When the A9 cores and the gic are decoupled from the PRCMU, the idle routine has to check if an interrupt is pending on the gic before entering in retention mode.
So what happens if an interrupt becomes pending after you've checked it but before you've gone into retention mode? This basically sounds like one big racy idea to me.
Hardware which allows you to go into retention mode with interrupts pending which should prevent it (or at least cause it to re-awake) just sounds like a massive fail to me.
PATCH 1/1 gic : check if there are pending interrupts
Quoted 11 lines · Russell King - ARM Linux
Quoted 6 lines · Daniel Lezcano
The following patch checks if there are pending interrupts on the gic.
This function is needed for example for the ux500 cpuidle driver. When the A9 cores and the gic are decoupled from the PRCMU, the idle routine has to check if an interrupt is pending on the gic before entering in retention mode.
So what happens if an interrupt becomes pending after you've checked it but before you've gone into retention mode? This basically sounds like one big racy idea to me.
The retention mode for the ux500 SoC is the following:
1 - the gic is decoupled : no interrupts will occur and wake up the second core if it is in WFI
2 - the gic settings are copied to the PRCMU which is in charge to wake up the cores and recouple the gic. We have to check here if an interrupts occurred between 1 and 2 in order to abort the retention mode and recouple the gic.
3 - we go to retention mode where the PRCMU waits for both cores to be in WFI, then if an interrupt is pending (the interrupt occurred between 2 -3), or occurs, it recouples the gic and wakes the cores.
Here is the implementation to illustrate this.
static inline int ux500_enter_arm_retention(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
{
int this_cpu = smp_processor_id();
bool recouple = false;
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &this_cpu);
if (atomic_inc_return(&master) == num_online_cpus()) {
/* With this lock, we prevent the other cpu to exit and enter
* this function again and become the master */
if (!spin_trylock(&master_lock))
goto wfi;
WARN_ONCE(prcmu_gic_decouple(), "Failed to decouple gic from prcmu");
/* If an abort occurs, we will have to recouple the gic
* manually */
recouple = true;
/* At this state, as the gic is decoupled, if the other
* cpu is in WFI, we have the guarantee it won't be wake
* up, so we can safely go to retention */
if (!ux500_pm_other_cpu_wfi(this_cpu))
goto out;
/* The prcmu will be in charge of watching the interrupts
* and wake up the cpus */
ux500_pm_prcmu_copy_gic_settings();
/* Check in the meantime an interrupt did
* not occur on the gic ... */
if (gic_pending_irq(0))
goto out;
/* ... and the prcmu */
if (ux500_pm_prcmu_pending_interrupt())
goto out;
/* Go to the retention state, the prcmu will wait for the
* cpu to go WFI and this is what happens after exiting this
* 'master' critical section */
if (prcmu_set_power_state(PRCMU_AP_IDLE, true, true))
goto out;
/* When we switch to retention, the prcmu is in charge
* of recoupling the gic automatically */
recouple = false;
spin_unlock(&master_lock);
}
wfi:
cpu_do_idle();
out:
atomic_dec(&master);
if (recouple) {
WARN_ONCE(prcmu_gic_recouple(), "Failed to recouple gic to prcmu");
spin_unlock(&master_lock);
}
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &this_cpu);
return index;
}The function to check if there are pending interrupts on the gic is needed for the cpuidle driver, as well as a function to copy the gic settings to the PRCMU. Maybe this is too SoC specific and the function could moved to the ux500 with some information exported from gic.c, no ?
Quoted 3 lines
Hardware which allows you to go into retention mode with interrupts pending which should prevent it (or at least cause it to re-awake) just sounds like a massive fail to me.
Well, I don't have an opinion on this :)
Thanks
-- Daniel
Signature · 5 lines
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog