Skip to contentSkip to conversation

set_irq_noprobe shouldn't be __init

PATCH In review Jean Delvare 2 participants 3 messages Jan 5, 2010 – Jan 9, 2010 IRQ

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 set_irq_noprobe shouldn't be __init

Non-__init functions need to call set_irq_noprobe() so this function shouldn't be marked __init. Also remove __init from set_irq_probe() for consistency.

Cc Ralf Baechle <[email protected]>
Cc Samuel Ortiz <[email protected]>
---
 kernel/irq/chip.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/irq/chip.c +2 -2
@@ -682,7 +682,7 @@ set_irq_chip_and_handler_name(unsigned i
 	__set_irq_handler(irq, handle, 0, name);
 }
 
-void __init set_irq_noprobe(unsigned int irq)
+void set_irq_noprobe(unsigned int irq)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 	unsigned long flags;
@@ -697,7 +697,7 @@ void __init set_irq_noprobe(unsigned int
 	raw_spin_unlock_irqrestore(&desc->lock, flags);
 }
 
-void __init set_irq_probe(unsigned int irq)
+void set_irq_probe(unsigned int irq)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 	unsigned long flags;
Signature · 1 line
Jean Delvare
#1 lore raw
Quoted 23 lines · On Tue, 5 Jan 2010 20:38:42 +0100 Jean Delvare <[email protected]> wrote:

Non-__init functions need to call set_irq_noprobe() so this function shouldn't be marked __init. Also remove __init from set_irq_probe() for consistency.

Cc Ralf Baechle <[email protected]>
Cc Samuel Ortiz <[email protected]>
---
 kernel/irq/chip.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/irq/chip.c +1 -1
@@ -682,7 +682,7 @@ set_irq_chip_and_handler_name(unsigned i
 	__set_irq_handler(irq, handle, 0, name);
 }
 
-void __init set_irq_noprobe(unsigned int irq)
+void set_irq_noprobe(unsigned int irq)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 	unsigned long flags;

kernel/irq/chip.c void __init set_irq_noprobe(unsigned int · line 703

Quoted 8 lines
kernel/irq/chip.c +1 -1
@@ -697,7 +697,7 @@ void __init set_irq_noprobe(unsigned int
 	raw_spin_unlock_irqrestore(&desc->lock, flags);
 }
 
-void __init set_irq_probe(unsigned int irq)
+void set_irq_probe(unsigned int irq)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 	unsigned long flags;

These non-__init callers will instantly crash, surely? Are there any reports of such crashes?

If you're talking about new code then what is the status of that code?

Right now I don't know if this patch is needed in 2.6.34, 2.6.33 or 2.6.32.x and earlier.

IOW: the changelog sucks :)

Thanks.

#2 lore raw

Hi Andrew,

Quoted 29 lines · Andrew Morton
Quoted 26 lines · On Tue, 5 Jan 2010 20:38:42 +0100 Jean Delvare <[email protected]> wrote:

Non-__init functions need to call set_irq_noprobe() so this function shouldn't be marked __init. Also remove __init from set_irq_probe() for consistency.

Cc Ralf Baechle <[email protected]>
Cc Samuel Ortiz <[email protected]>
---
 kernel/irq/chip.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/irq/chip.c +1 -1
@@ -682,7 +682,7 @@ set_irq_chip_and_handler_name(unsigned i
 	__set_irq_handler(irq, handle, 0, name);
 }
 
-void __init set_irq_noprobe(unsigned int irq)
+void set_irq_noprobe(unsigned int irq)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 	unsigned long flags;
@@ -697,7 +697,7 @@ void __init set_irq_noprobe(unsigned int
 	raw_spin_unlock_irqrestore(&desc->lock, flags);
 }
 
Quoted 8 lines

kernel/irq/chip.c void __init set_irq_noprobe(unsigned int · line 703

Quoted 5 lines
kernel/irq/chip.c +1 -1
-void __init set_irq_probe(unsigned int irq)
+void set_irq_probe(unsigned int irq)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 	unsigned long flags;

These non-__init callers will instantly crash, surely? Are there any reports of such crashes?

I am not aware of such crashes being reported, but I am not the maintainer of the affected drivers (these are mfd drivers.) I suppose that the crash would only happen if these drivers were built as
modules: if built into the kernel (and they always are, being boolean options in Kconfig), maybe they have time to initialize before the __init code is removed? I'm not familiar enough with this mechanism to tell.

The only thing I'm sure of is that 'make
CONFIG_DEBUG_SECTION_MISMATCH=y' complains.

I initially proposed the most immediate fix, but it might as well be that it is reasonable to leave set_irq_probe() in __init and fix all callers, under the assumption that all drivers which need it will be for specific chips which require built-in support and can't be hot-plugged by design.

What I don't quite get is how to guarantee that the devices in question will be reachable early. These are SPI and I2C devices, so they require the right bus driver to be loaded first. But this cannot happen if they are built as modules (which nothing prevents as far as I can see.) So I suspect that, if everything in the drivers was tagged __init as it would have to if we want set_irq_probe() in __init, but the relevant SPI or I2C bus driver was built as a module, bad things would happen.

The proper fix in that case is somewhat beyond me. This might be one of these cases where I should have refrained from sending a patch when I finally did not know what the best solution to the problem would be. Feel free to ignore me.

Quoted 6 lines

If you're talking about new code then what is the status of that code?

Right now I don't know if this patch is needed in 2.6.34, 2.6.33 or 2.6.32.x and earlier.

IOW: the changelog sucks :)

You're right, sorry about that. The affected drivers are twl4030/twl6030, ezx-pcap and wm831x (all mfd drivers), offending lines are in the kernel tree since 2.6.28/2.6.33, 2.6.31 and 2.6.33, respectively.

Signature · 1 line
Jean Delvare