mmp: irq: Don't clear unused interrupt enable bits
Subsystems (3)
PATCH 1/1 mmp: irq: Don't clear unused interrupt enable bits
When enabling/masking interrupts, the existing MMP2 code clears a mask of 0x7f in the interrupt enable register.
The lower 5 bits here are not directly used by Linux:
0:3 is interrupt priority
4 determines whether the interrupt gets delivered to the Security Processor
In the OLPC case, a special firmware is running on the SP, and we do not want to mask it from receiving the interrupts it has already unmasked.
Refine the mask to only deal with the bits that are of specific interest to Linux running on the main CPU.
--- arch/arm/mach-mmp/irq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/mach-mmp/irq.c +1 -1
@@ -190,7 +190,7 @@ static struct mmp_intc_conf mmp_conf = {static struct mmp_intc_conf mmp2_conf = { .conf_enable =0x20 , .conf_disable =0x0 , - .conf_mask =0x7f , + .conf_mask =0x60 , };/* MMP (ARMv5) */
Signature · 1 line
1.8.1.4
PATCH 1/1 mmp: irq: Don't clear unused interrupt enable bits
Quoted 7 lines · Daniel Drake
When enabling/masking interrupts, the existing MMP2 code clears a mask of
0x7fin the interrupt enable register.
The lower 5 bits here are not directly used by Linux:
0:3 is interrupt priority
4 determines whether the interrupt gets delivered to the Security Processor
Quoted 5 lines
In the OLPC case, a special firmware is running on the SP, and we do not want to mask it from receiving the interrupts it has already unmasked.
Refine the mask to only deal with the bits that are of specific interest to Linux running on the main CPU.
Is there any way to check this at run-time or at least by boot parameters? Decide whether the interrupt gets delivered to SP seems to be a choice that the main CPU should be doing, as well as the interrupt priority.
And this is really all about bit 4 right?
Quoted 23 lines
Signed-off-by Daniel Drake <[email protected]>--- arch/arm/mach-mmp/irq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm/mach-mmp/irq.c +1
-2@@ -190,7 +190,7 @@ static struct mmp_intc_conf mmp_conf = {static struct mmp_intc_conf mmp2_conf = { .conf_enable =0x20 , .conf_disable =0x0 , - .conf_mask =0x7f , + .conf_mask =0x60 , };/* MMP (ARMv5) */ --1.8.1.4
PATCH 1/1 mmp: irq: Don't clear unused interrupt enable bits
Quoted 1 line · Eric Miao
Is there any way to check this at run-time or at least by boot parameters?
The only thing that occurs to me would be checking for the device tree node that corresponds to our input serio. But doing that from platform code would be horrible, and it would also be nasty doing it from the input driver.
Quoted 2 lines
Decide whether the interrupt gets delivered to SP seems to be a choice that the main CPU should be doing, as well as the interrupt priority.
Incidentally the main CPU does make these choices - just before Linux has booted.
While Linux has no interaction with the SP (it is not even aware of it), I would say not stomping on its configuration is an OK policy to have.
Quoted 1 line
And this is really all about bit 4 right?
Yes, bit 4 is the crucial one, it is required for the OLPC inbuilt keyboard/touchpad to work.
However on numerous occasions during interactions with Marvell they have stressed the importance of setting interrupt priorities on interrupts used for wakeups. We don't think it makes a difference for the issues that we have seen, but we do not ignore the request. As that cannot be done in Linux, we do it in the firmware, and it seems like while Linux does not work with interrupt priorities it should not overwite any earlier setup.
Daniel
PATCH 1/1 mmp: irq: Don't clear unused interrupt enable bits
Quoted 7 lines · Daniel Drake
Quoted 1 line · Eric Miao
Is there any way to check this at run-time or at least by boot parameters?
The only thing that occurs to me would be checking for the device tree node that corresponds to our input serio. But doing that from platform code would be horrible, and it would also be nasty doing it from the input driver.
I think that should be OK and clean, e.g.
if (of_property_read_bool(np, "irq-route-to-sp")) {
....
}I'm fine with leaving these bits out as your patch does, the only concern is some other code may have a chance to depend on this, and may benefit from this boot configuration flexibility as well.
Quoted 16 lines
Quoted 3 lines
Decide whether the interrupt gets delivered to SP seems to be a choice that the main CPU should be doing, as well as the interrupt priority.
Incidentally the main CPU does make these choices - just before Linux has booted.
While Linux has no interaction with the SP (it is not even aware of it), I would say not stomping on its configuration is an OK policy to have.
Quoted 1 line
And this is really all about bit 4 right?
Yes, bit 4 is the crucial one, it is required for the OLPC inbuilt keyboard/touchpad to work.
Quoted 7 lines
However on numerous occasions during interactions with Marvell they have stressed the importance of setting interrupt priorities on interrupts used for wakeups. We don't think it makes a difference for the issues that we have seen, but we do not ignore the request. As that cannot be done in Linux, we do it in the firmware, and it seems like while Linux does not work with interrupt priorities it should not overwite any earlier setup.
Haojian & Mingliang, could you help check this is the case, and that no known driver (including those for upstream) is depending on this behavior?
Quoted 2 lines
Daniel
PATCH 1/1 mmp: irq: Don't clear unused interrupt enable bits
Quoted 3 lines · Eric Miao
I'm fine with leaving these bits out as your patch does, the only concern is some other code may have a chance to depend on this, and may benefit from this boot configuration flexibility as well.
I would prefer to leave them out for simplicity of the driver. However, I'm happy to weigh in if/when any future need comes up where Linux needs to be directly involved with SP setup.
Daniel
PATCH 1/1 mmp: irq: Don't clear unused interrupt enable bits
In reply to Eric MiaoAdd Chao to comment.
-----Original Message-----
From: Eric Miao [mailto:eric.y.miao at gmail.com]
Sent: 2013?6?29? 12:55
To: Daniel Drake
Cc: Haojian Zhuang; linux-arm-kernel; paul fox; Mingliang Hu
Subject: Re: [PATCH] mmp: irq: Don't clear unused interrupt enable bits
Quoted 7 lines · Daniel Drake
Quoted 1 line · Eric Miao
Is there any way to check this at run-time or at least by boot parameters?
The only thing that occurs to me would be checking for the device tree node that corresponds to our input serio. But doing that from platform code would be horrible, and it would also be nasty doing it from the input driver.
I think that should be OK and clean, e.g.
if (of_property_read_bool(np, "irq-route-to-sp")) {
....
}I'm fine with leaving these bits out as your patch does, the only concern is some other code may have a chance to depend on this, and may benefit from this boot configuration flexibility as well.
Quoted 16 lines
Quoted 3 lines
Decide whether the interrupt gets delivered to SP seems to be a choice that the main CPU should be doing, as well as the interrupt priority.
Incidentally the main CPU does make these choices - just before Linux has booted.
While Linux has no interaction with the SP (it is not even aware of it), I would say not stomping on its configuration is an OK policy to have.
Quoted 1 line
And this is really all about bit 4 right?
Yes, bit 4 is the crucial one, it is required for the OLPC inbuilt keyboard/touchpad to work.
Quoted 7 lines
However on numerous occasions during interactions with Marvell they have stressed the importance of setting interrupt priorities on interrupts used for wakeups. We don't think it makes a difference for the issues that we have seen, but we do not ignore the request. As that cannot be done in Linux, we do it in the firmware, and it seems like while Linux does not work with interrupt priorities it should not overwite any earlier setup.
Haojian & Mingliang, could you help check this is the case, and that no known driver (including those for upstream) is depending on this behavior?
Quoted 2 lines
Daniel