Support IRQ_NOAUTOEN flag in set_irq_chained_handler()
PATCH 1/1 Support IRQ_NOAUTOEN flag in set_irq_chained_handler()
From: Esben Haabendal <[email protected]>
Handle IRQ_NOAUTOEN in __set_irq_handler() (ie. for set_irq_chained_handler()) instead of just silently ignoring it, and in the same way as is done in __setup_irq() (ie. request_irq()).
This give a more consistent interface, and also adheres better to the rule of least surprise.
--- kernel/irq/chip.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)
kernel/irq/chip.c +6 -3
@@ -731,10 +731,13 @@ __set_irq_handler(unsigned int irq, irq_flow_handler_t handle, int is_chained, desc->name = name;if (handle != handle_bad_irq && is_chained) { - desc->status &= ~IRQ_DISABLED ; desc->status |=IRQ_NOREQUEST |IRQ_NOPROBE ; - desc->depth =0 ; - desc->irq_data.chip->irq_startup (&desc->irq_data); +if (!(desc->status &IRQ_NOAUTOEN )) { + desc->depth =0 ; + desc->status &= ~IRQ_DISABLED ; + desc->irq_data.chip->irq_startup (&desc->irq_data); + }else + desc->depth =1 ; }raw_spin_unlock_irqrestore (&desc->lock, flags);chip_bus_sync_unlock (desc);
Signature · 1 line
1.7.1
Quoted 8 lines · On Fri, 18 Mar 2011, [email protected] wrote:
From: Esben Haabendal <[email protected]>
Handle
IRQ_NOAUTOENin__set_irq_handler()(ie. forset_irq_chained_handler()) instead of just silently ignoring it, and in the same way as is done in__setup_irq()(ie.request_irq()).This give a more consistent interface, and also adheres better to the rule of least surprise.
Well, that might be less surprising for you, but you will be surprised that such a change would be a real big surprise for all users of chained handlers in arch/arm. They simply would not work anymore.
So we _cannot_ change the semantics here. All we can do is document it.
Thanks,
tglx
Quoted 8 lines · Thomas Gleixner
Quoted 6 lines · On Fri, 18 Mar 2011, [email protected] wrote:
From: Esben Haabendal <[email protected]>
Handle
IRQ_NOAUTOENin__set_irq_handler()(ie. forset_irq_chained_handler()) instead of just silently ignoring it, and in the same way as is done in__setup_irq()(ie.request_irq()).
Quoted 6 lines
Quoted 2 lines
This give a more consistent interface, and also adheres better to the rule of least surprise.
Well, that might be less surprising for you, but you will be surprised that such a change would be a real big surprise for all users of chained handlers in
arch/arm. They simply would not work anymore.
How is that? I don't see any use of IRQ_NOAUTOEN flag in arch/arm at all. Is there some other way that IRQ_NOAUTOEN get's enabled in arch/arm? Or is my patch broken in some way that it does change irq handler setup when IRQ_NOAUTOEN is not set?
The idea of the patch is that it will do exactly the same as before, unless you specifically set IRQ_NOAUTOEN before calling set_irq_chained_handler...
Quoted 2 lines
So we _cannot_ change the semantics here. All we can do is document it.
With the current semantics, how are one supposed to be able use set_irq_chained_handler without having the handler enabled immediately?
/Esben
Quoted 13 lines · Esben Haabendal
Quoted 11 lines · Thomas Gleixner
Quoted 8 lines · On Fri, 18 Mar 2011, [email protected] wrote:
From: Esben Haabendal <[email protected]>
Handle
IRQ_NOAUTOENin__set_irq_handler()(ie. forset_irq_chained_handler()) instead of just silently ignoring it, and in the same way as is done in__setup_irq()(ie.request_irq()).This give a more consistent interface, and also adheres better to the rule of least surprise.
Quoted 8 lines
Quoted 3 lines
Well, that might be less surprising for you, but you will be surprised that such a change would be a real big surprise for all users of chained handlers in
arch/arm. They simply would not work anymore.How is that? I don't see any use of
IRQ_NOAUTOENflag inarch/armat all. Is there some other way thatIRQ_NOAUTOENget's enabled inarch/arm? Or is my patch broken in some way that it does change irq handler setup whenIRQ_NOAUTOENis not set?
Ooops, sorry. I had it somewhere in the back of my memory that ARM marked all interrupts IRQ_NOAUTOEN by default. Confused that with NOPROBE.
Quoted 3 lines
The idea of the patch is that it will do exactly the same as before, unless you specifically set
IRQ_NOAUTOENbefore callingset_irq_chained_handler...
I understand the patch :)
Quoted 5 lines
Quoted 2 lines
So we _cannot_ change the semantics here. All we can do is document it.
With the current semantics, how are one supposed to be able use
set_irq_chained_handlerwithout having the handler enabled immediately?
Not at all. Why do you want to do that ?
Thanks,
tglx
Quoted 34 lines · Thomas Gleixner
Quoted 13 lines · Esben Haabendal
Quoted 11 lines · Thomas Gleixner
Quoted 8 lines · On Fri, 18 Mar 2011, [email protected] wrote:
From: Esben Haabendal <[email protected]>
Handle
IRQ_NOAUTOENin__set_irq_handler()(ie. forset_irq_chained_handler()) instead of just silently ignoring it, and in the same way as is done in__setup_irq()(ie.request_irq()).This give a more consistent interface, and also adheres better to the rule of least surprise.
Quoted 8 lines
Quoted 3 lines
Well, that might be less surprising for you, but you will be surprised that such a change would be a real big surprise for all users of chained handlers in
arch/arm. They simply would not work anymore.How is that? I don't see any use of
IRQ_NOAUTOENflag inarch/armat all. Is there some other way thatIRQ_NOAUTOENget's enabled inarch/arm? Or is my patch broken in some way that it does change irq handler setup whenIRQ_NOAUTOENis not set?Ooops, sorry. I had it somewhere in the back of my memory that ARM marked all interrupts
IRQ_NOAUTOENby default. Confused that with NOPROBE.Quoted 3 lines
The idea of the patch is that it will do exactly the same as before, unless you specifically set
IRQ_NOAUTOENbefore callingset_irq_chained_handler...I understand the patch :)
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
So we _cannot_ change the semantics here. All we can do is document it.
With the current semantics, how are one supposed to be able use
set_irq_chained_handlerwithout having the handler enabled immediately?Not at all. Why do you want to do that ?
I have a system where
I setup the chained interrupt handler (together with a lot of other stuff related to the CPLD firmware the interrupt controller lives in) in of_platform_driver.probe(). The CPLD may be (re)programmed from user-space, so all driver functionality is disabled until user-space either programs the CPLD or gives a signal that this will not happen.
I thought it would be the cleanest solution to keep driver initialization in the probe() function, and just enable it later on.
And I cannot just set the mask early, as I am not guaranteed how the irq line is behaving and if there actually is a mask register before it is programmed.
/Esben