On 09/08/16 05:25, John Stultz wrote:
...
> So actually no. We usually call irqd_set_trigger_type() but something
> still doesn't work.
>
> Interestingly, just adding irq_set_irq_type(virq, type); to the top of
> that block (leaving the rest of the code) also works.
Interesting. By saving the trigger type during the mapping, we defer
setting the interrupt type to when the interrupt is requested. So this
would imply that the interrupt is not being setup as expected when its
requested.
>> What is odd, is that the above sequence is only executed if a irq
>> mapping exists and so really, AFAICT this should not happen. Ie. the irq
>> descriptor should have been allocated for the mapping to exist. We
>> should probably warn if this happens.
>>
>> Without reverting the above, can you add a print to show the
>> domain->name, hwirq and virq information if !irq_data? That will confirm
>> the domain for us.
>
> So I put some printk info in (in either case since I'm never seeing
> the !irq_data case happen):
>
> [ 1.514217] JDB: virq: 93 hwirq: 74 domain name: msmgpio
> [ 1.838342] JDB: virq: 25 hwirq: 6 domain name: msmgpio
>
> Which is odd, looking at:
>
> shell@flo:/ $ cat /proc/interrupts
> CPU0 CPU1 CPU2 CPU3
> 16: 1159 1138 1332 1574 GIC-0 18 Edge
> gp_timer
> 25: 0 0 0 0 msmgpio 6 Edge
> ekth3500
> 111: 6 0 0 0 GIC-0 51 Edge
> qcom_rpm_ack
> 112: 0 0 0 0 GIC-0 53 Edge
> qcom_rpm_err
> 113: 0 0 0 0 GIC-0 54 Edge
> qcom_rpm_wakeup
> 114: 48 0 0 0 GIC-0 132 Edge
> msm_otg, ci_hdrc_msm
> 115: 796 0 0 0 GIC-0 130 Level
> bam_dma
> 116: 0 0 0 0 GIC-0 128 Level
> bam_dma
> 117: 0 0 0 0 GIC-0 127 Level
> bam_dma
> 118: 2627 0 0 0 GIC-0 136 Level
> mmci-pl18x (cmd)
> 119: 54 0 0 0 GIC-0 226 Level
> i2c_qup
> 120: 21 0 0 0 GIC-0 183 Level
> i2c_qup
> 122: 0 0 0 0 GIC-0 189 Level
> i2c_qup
> 123: 202 0 0 0 GIC-0 190 Level
> msm_serial0
> 124: 0 0 0 0 GIC-0 70 Edge smsm
> 125: 0 0 0 0 GIC-0 121 Edge smsm
> 126: 0 0 0 0 GIC-0 236 Edge smsm
> 127: 0 0 0 0 GIC-0 169 Edge smsm
> 131: 0 0 0 0 pm8xxx 195 Edge
> Volume Up
> 165: 0 0 0 0 pm8xxx 229 Edge
> Volume Down
> 184: 0 0 0 0 pm8xxx 39 Edge
> pm8xxx_rtc_alarm
> 185: 0 0 0 0 pm8xxx 50 Edge
> pmic8xxx_pwrkey_release
> 186: 0 0 0 0 pm8xxx 51 Edge
> pmic8xxx_pwrkey_press
> IPI0: 0 1 1 1 CPU wakeup interrupts
> IPI1: 0 0 0 0 Timer broadcast interrupts
> IPI2: 944 539 1015 529 Rescheduling interrupts
> IPI3: 1 4 6 4 Function call interrupts
> IPI4: 0 0 0 0 CPU stop interrupts
> IPI5: 0 0 0 0 IRQ work interrupts
> IPI6: 0 0 0 0 completion interrupts
> Err: 0
>
> Since 25 maps to the ekth3500 (touch panel, which is still working
> fine), but 93/74 doesn't seem to map to anything, and the problematic
> irqs are the volume keys 195/229 and power keys 50/51.
So looking at the DT source, I believe that hwirq 74 (virq 93) is the
problem. This is the parent interrupt from the pm8xxx to the apq8064
if it is not requested then the type is not set. It seems that for
parent interrupts these are not typically requested, but enabled when
an irqchip is chained.
To confirm and for testing purposes I am curious if this works ...
if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
irq_data = irq_get_irq_data(virq);
if (!irq_data)
return 0;
- irqd_set_trigger_type(irq_data, type);
+ if (hwirq == 74)
+ irq_set_irq_type(virq, type);
+ else
+ irqd_set_trigger_type(irq_data, type);
return virq;
}
If that works, then does the following also work (without the above) ...
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index b4c1bc7c9ca2..e111b72e3162 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -824,6 +824,7 @@ __irq_do_set_handler(struct irq_desc *desc,
irq_flow_handler_t handle,
irq_settings_set_norequest(desc);
irq_settings_set_nothread(desc);
desc->action = &chained_action;
+ __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));
irq_startup(desc, true);
}
}
It looks like there is a path for parent interrupts where the type
is not getting set. If the above works then we can discuss with Thomas
and Marc on the correct fix.
Cheers
Jon
--
nvpublic
