[v1] irqchip/apple-aic: Only handle PMC interrupt as FIQ when configured to fire FIQ

[PATCH] irqchip/apple-aic: Only handle PMC interrupt as FIQ when configured to fire FIQ

Posted by Nick Chan 1 year ago

The CPU PMU in Apple SoCs can be configured to fire its interrupt in one
of several ways, and since Apple A11 one of the method is FIQ. Only handle
the PMC interrupt as a FIQ when the CPU PMU has been configured to fire
FIQs.

Cc: stable@vger.kernel.org
Fixes: c7708816c944 ("irqchip/apple-aic: Wire PMU interrupts")
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 drivers/irqchip/irq-apple-aic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
index da5250f0155c..c3d435103d6d 100644
--- a/drivers/irqchip/irq-apple-aic.c
+++ b/drivers/irqchip/irq-apple-aic.c
@@ -577,7 +577,8 @@ static void __exception_irq_entry aic_handle_fiq(struct pt_regs *regs)
 						  AIC_FIQ_HWIRQ(AIC_TMR_EL02_VIRT));
 	}
 
-	if (read_sysreg_s(SYS_IMP_APL_PMCR0_EL1) & PMCR0_IACT) {
+	if (read_sysreg_s(SYS_IMP_APL_PMCR0_EL1) &
+	    (FIELD_PREP(PMCR0_IMODE, PMCR0_IMODE_FIQ) | PMCR0_IACT)) {
 		int irq;
 		if (cpumask_test_cpu(smp_processor_id(),
 				     &aic_irqc->fiq_aff[AIC_CPU_PMU_P]->aff))

base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
-- 
2.48.1

Re: [PATCH] irqchip/apple-aic: Only handle PMC interrupt as FIQ when configured to fire FIQ

Posted by Sven Peter 1 year ago

Hi,


On Fri, Jan 17, 2025, at 18:02, Nick Chan wrote:
> The CPU PMU in Apple SoCs can be configured to fire its interrupt in one
> of several ways, and since Apple A11 one of the method is FIQ. Only handle
> the PMC interrupt as a FIQ when the CPU PMU has been configured to fire
> FIQs.
>
> Cc: stable@vger.kernel.org
> Fixes: c7708816c944 ("irqchip/apple-aic: Wire PMU interrupts")
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
>  drivers/irqchip/irq-apple-aic.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-apple-aic.c 
> b/drivers/irqchip/irq-apple-aic.c
> index da5250f0155c..c3d435103d6d 100644
> --- a/drivers/irqchip/irq-apple-aic.c
> +++ b/drivers/irqchip/irq-apple-aic.c
> @@ -577,7 +577,8 @@ static void __exception_irq_entry 
> aic_handle_fiq(struct pt_regs *regs)
>  						  AIC_FIQ_HWIRQ(AIC_TMR_EL02_VIRT));
>  	}
> 
> -	if (read_sysreg_s(SYS_IMP_APL_PMCR0_EL1) & PMCR0_IACT) {
> +	if (read_sysreg_s(SYS_IMP_APL_PMCR0_EL1) &
> +	    (FIELD_PREP(PMCR0_IMODE, PMCR0_IMODE_FIQ) | PMCR0_IACT)) {

That's a somewhat unusual way to use FIELD_PREP and I'm not sure the
expression even does what you want. It's true when only PMCR0_IACT is set and
your commit description mentions that you only when to handle these when
FIQ have been configured. Am I missing something here?


Best,


Sven

Re: [PATCH] irqchip/apple-aic: Only handle PMC interrupt as FIQ when configured to fire FIQ

Posted by Nick Chan 1 year ago

Sven Peter 於 2025/1/18 夜晚8:24 寫道:
> Hi,
>
>
> On Fri, Jan 17, 2025, at 18:02, Nick Chan wrote:
>> The CPU PMU in Apple SoCs can be configured to fire its interrupt in one
>> of several ways, and since Apple A11 one of the method is FIQ. Only handle
>> the PMC interrupt as a FIQ when the CPU PMU has been configured to fire
>> FIQs.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: c7708816c944 ("irqchip/apple-aic: Wire PMU interrupts")
>> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
>> ---
>>  drivers/irqchip/irq-apple-aic.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-apple-aic.c 
>> b/drivers/irqchip/irq-apple-aic.c
>> index da5250f0155c..c3d435103d6d 100644
>> --- a/drivers/irqchip/irq-apple-aic.c
>> +++ b/drivers/irqchip/irq-apple-aic.c
>> @@ -577,7 +577,8 @@ static void __exception_irq_entry 
>> aic_handle_fiq(struct pt_regs *regs)
>>  						  AIC_FIQ_HWIRQ(AIC_TMR_EL02_VIRT));
>>  	}
>>
>> -	if (read_sysreg_s(SYS_IMP_APL_PMCR0_EL1) & PMCR0_IACT) {
>> +	if (read_sysreg_s(SYS_IMP_APL_PMCR0_EL1) &
>> +	    (FIELD_PREP(PMCR0_IMODE, PMCR0_IMODE_FIQ) | PMCR0_IACT)) {
> That's a somewhat unusual way to use FIELD_PREP and I'm not sure the
> expression even does what you want. It's true when only PMCR0_IACT is set and
> your commit description mentions that you only when to handle these when
> FIQ have been configured. Am I missing something here?
On a closer look the condition will evaluate to true when imode is between 7 and 4 inclusive and
PMCR0_IACT  is set. The intended behavior is to have it evaluate to true when imode is 4 and
PMCR0_IACT is set. Will send a v2.
>
>
> Best,
>
>
> Sven


Nick Chan