2008-11-13 06:29:38

by Mark Nelson

[permalink] [raw]
Subject: commit "genirq: record trigger type" and powerpc

Hi Ingo,

I noticed that after commit 0c5d1eb77a8be917b638344a22afe1398236482b
(genirq: record trigger type), several of our powerpc platforms now
spew out warnings about "No set_type function for IRQ..." (in
particular our Cell blades). This is because in our generic platform
code we call set_irq_type() with information from the device tree
when we establish the interrupt mappings; but we do this regardless
of whether the PIC can actually set a type (it might irrelevant
because the type is essentially hardcoded or as in the case for Cell
the interrupts are just messages being past around that have no real
concept of type, or we could even be dealing with a virtual PIC as
on the PS3).

So, would it be possible to turn the:

pr_warning("No set_type function...

into a pr_debug() in kernel/irq/manage.c so when our users upgrade
to newer kernels they don't get scared by a whole bunch of new
warnings?

Many thanks!

Mark


2008-11-13 07:43:26

by Ingo Molnar

[permalink] [raw]
Subject: Re: commit "genirq: record trigger type" and powerpc


* Mark Nelson <[email protected]> wrote:

> Hi Ingo,
>
> I noticed that after commit 0c5d1eb77a8be917b638344a22afe1398236482b
> (genirq: record trigger type), several of our powerpc platforms now
> spew out warnings about "No set_type function for IRQ..." (in
> particular our Cell blades). This is because in our generic platform
> code we call set_irq_type() with information from the device tree
> when we establish the interrupt mappings; but we do this regardless
> of whether the PIC can actually set a type (it might irrelevant
> because the type is essentially hardcoded or as in the case for Cell
> the interrupts are just messages being past around that have no real
> concept of type, or we could even be dealing with a virtual PIC as
> on the PS3).
>
> So, would it be possible to turn the:
>
> pr_warning("No set_type function...
>
> into a pr_debug() in kernel/irq/manage.c so when our users upgrade
> to newer kernels they don't get scared by a whole bunch of new
> warnings?

Sure, please send a patch with a changelog.

Ingo

2008-11-13 10:38:30

by Mark Nelson

[permalink] [raw]
Subject: [PATCH] genirq: __irq_set_trigger: change pr_warning to pr_debug

Commit 0c5d1eb77a8be917b638344a22afe1398236482b (genirq: record trigger
type) caused powerpc platforms that had no set_type() function in their
struct irq_chip to spew out warnings about "No set_type function for
IRQ...". This warning isn't necessarily justified though because the
generic powerpc platform code calls set_irq_type() (which in turn calls
__irq_set_trigger) with information from the device tree to establish
the interrupt mappings, regardless of whether the PIC can actually set
a type.

A platform's irq_chip might not have a set_type function for a variety
of reasons, for example: the platform may have the type essentially
hard-coded, or as in the case for Cell interrupts are just messages
past around that have no real concept of type, or the platform
could even have a virtual PIC as on the PS3.

Signed-off-by: Mark Nelson <[email protected]>
---
Thanks Ingo! Much appreciated

kernel/irq/manage.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Index: upstream/kernel/irq/manage.c
===================================================================
--- upstream.orig/kernel/irq/manage.c
+++ upstream/kernel/irq/manage.c
@@ -327,7 +327,7 @@ int __irq_set_trigger(struct irq_desc *d
* IRQF_TRIGGER_* but the PIC does not support multiple
* flow-types?
*/
- pr_warning("No set_type function for IRQ %d (%s)\n", irq,
+ pr_debug("No set_type function for IRQ %d (%s)\n", irq,
chip ? (chip->name ? : "unknown") : "unknown");
return 0;
}

2008-11-13 11:12:06

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] genirq: __irq_set_trigger: change pr_warning to pr_debug


* Mark Nelson <[email protected]> wrote:

> Commit 0c5d1eb77a8be917b638344a22afe1398236482b (genirq: record
> trigger type) caused powerpc platforms that had no set_type()
> function in their struct irq_chip to spew out warnings about "No
> set_type function for IRQ...". This warning isn't necessarily
> justified though because the generic powerpc platform code calls
> set_irq_type() (which in turn calls __irq_set_trigger) with
> information from the device tree to establish the interrupt
> mappings, regardless of whether the PIC can actually set a type.
>
> A platform's irq_chip might not have a set_type function for a
> variety of reasons, for example: the platform may have the type
> essentially hard-coded, or as in the case for Cell interrupts are
> just messages past around that have no real concept of type, or the
> platform could even have a virtual PIC as on the PS3.
>
> Signed-off-by: Mark Nelson <[email protected]>
> ---
> Thanks Ingo! Much appreciated
>
> kernel/irq/manage.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

applied to tip/irq/urgent, thanks Mark!

Ingo