More and more sub-architectures are using only the irqchip_init
function. Make the core code call this function if no init_irq field is
provided in the machine description to remove some boilerplate code.
Signed-off-by: Maxime Ripard <[email protected]>
---
arch/arm/kernel/irq.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
index 8e4ef4c..df6f9a1 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -26,6 +26,7 @@
#include <linux/ioport.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
+#include <linux/irqchip.h>
#include <linux/random.h>
#include <linux/smp.h>
#include <linux/init.h>
@@ -114,7 +115,10 @@ EXPORT_SYMBOL_GPL(set_irq_flags);
void __init init_IRQ(void)
{
- machine_desc->init_irq();
+ if (machine_desc->init_irq)
+ machine_desc->init_irq();
+ else
+ irqchip_init();
}
#ifdef CONFIG_MULTI_IRQ_HANDLER
--
1.7.10.4
On 03/28/2013 04:41 AM, Maxime Ripard wrote:
> More and more sub-architectures are using only the irqchip_init
> function. Make the core code call this function if no init_irq field is
> provided in the machine description to remove some boilerplate code.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
> arch/arm/kernel/irq.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
> index 8e4ef4c..df6f9a1 100644
> --- a/arch/arm/kernel/irq.c
> +++ b/arch/arm/kernel/irq.c
> @@ -26,6 +26,7 @@
> #include <linux/ioport.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> +#include <linux/irqchip.h>
> #include <linux/random.h>
> #include <linux/smp.h>
> #include <linux/init.h>
> @@ -114,7 +115,10 @@ EXPORT_SYMBOL_GPL(set_irq_flags);
>
> void __init init_IRQ(void)
> {
> - machine_desc->init_irq();
> + if (machine_desc->init_irq)
> + machine_desc->init_irq();
> + else
> + irqchip_init();
There needs to be an empty version defined for !OF.
Rob
> }
>
> #ifdef CONFIG_MULTI_IRQ_HANDLER
>
On Thu, Mar 28, 2013 at 09:48:18AM -0500, Rob Herring wrote:
> On 03/28/2013 04:41 AM, Maxime Ripard wrote:
> > + if (machine_desc->init_irq)
> > + machine_desc->init_irq();
> > + else
> > + irqchip_init();
>
> There needs to be an empty version defined for !OF.
Better:
#ifdef CONFIG_OF
if (!machine_desc->init_irq)
irqchip_init();
else
#endif
machine_desc->init_irq();
which means we don't even get the test if !OF, and if someone mistakenly
sets this to NULL for a !OF platform, they get to know about it.
On Thursday 28 March 2013, Russell King - ARM Linux wrote:
> Better:
>
> #ifdef CONFIG_OF
> if (!machine_desc->init_irq)
> irqchip_init();
> else
> #endif
> machine_desc->init_irq();
>
> which means we don't even get the test if !OF, and if someone mistakenly
> sets this to NULL for a !OF platform, they get to know about it.
Right. With the new IS_DEFINED() macro, we could even turn this into
if (IS_DEFINED(CONFIG_OF) && !machine_desc->init_irq)
irqchip_init();
else
machine_desc->init_irq();
to the same effect.
Arnd
On Thu, Mar 28, 2013 at 03:25:42PM +0000, Arnd Bergmann wrote:
> On Thursday 28 March 2013, Russell King - ARM Linux wrote:
> > Better:
> >
> > #ifdef CONFIG_OF
> > if (!machine_desc->init_irq)
> > irqchip_init();
> > else
> > #endif
> > machine_desc->init_irq();
> >
> > which means we don't even get the test if !OF, and if someone mistakenly
> > sets this to NULL for a !OF platform, they get to know about it.
>
> Right. With the new IS_DEFINED() macro, we could even turn this into
>
> if (IS_DEFINED(CONFIG_OF) && !machine_desc->init_irq)
> irqchip_init();
> else
> machine_desc->init_irq();
>
> to the same effect.
Provided irqchip_init() keeps its prototype visible, then yes.
On Thursday 28 March 2013, Russell King - ARM Linux wrote:
> On Thu, Mar 28, 2013 at 03:25:42PM +0000, Arnd Bergmann wrote:
> > if (IS_DEFINED(CONFIG_OF) && !machine_desc->init_irq)
> > irqchip_init();
> > else
> > machine_desc->init_irq();
> >
> > to the same effect.
>
> Provided irqchip_init() keeps its prototype visible, then yes.
Yes, I checked that before my reply. We have a few function declarations
that are unfortunately completely hidden when some config symbol is not
defined, but this is not one of them.
Arnd
Le 28/03/2013 16:49, Arnd Bergmann a ?crit :
> On Thursday 28 March 2013, Russell King - ARM Linux wrote:
>> On Thu, Mar 28, 2013 at 03:25:42PM +0000, Arnd Bergmann wrote:
>>> if (IS_DEFINED(CONFIG_OF) && !machine_desc->init_irq)
>>> irqchip_init();
>>> else
>>> machine_desc->init_irq();
>>>
>>> to the same effect.
>>
>> Provided irqchip_init() keeps its prototype visible, then yes.
>
> Yes, I checked that before my reply. We have a few function declarations
> that are unfortunately completely hidden when some config symbol is not
> defined, but this is not one of them.
I'll send a v2 with the suggested change.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On 03/28/2013 09:51 AM, Russell King - ARM Linux wrote:
> On Thu, Mar 28, 2013 at 09:48:18AM -0500, Rob Herring wrote:
>> On 03/28/2013 04:41 AM, Maxime Ripard wrote:
>>> + if (machine_desc->init_irq)
>>> + machine_desc->init_irq();
>>> + else
>>> + irqchip_init();
>>
>> There needs to be an empty version defined for !OF.
>
> Better:
>
> #ifdef CONFIG_OF
> if (!machine_desc->init_irq)
> irqchip_init();
> else
> #endif
Or the new style:
if (IS_ENABLED(CONFIG_OF) && !machine_desc->init_irq)
irqchip_init();
else
That needs the empty version, but handles your case.
Rob
> machine_desc->init_irq();
>
> which means we don't even get the test if !OF, and if someone mistakenly
> sets this to NULL for a !OF platform, they get to know about it.
>
On Thu, Mar 28, 2013 at 01:40:09PM -0500, Rob Herring wrote:
> On 03/28/2013 09:51 AM, Russell King - ARM Linux wrote:
> > On Thu, Mar 28, 2013 at 09:48:18AM -0500, Rob Herring wrote:
> >> On 03/28/2013 04:41 AM, Maxime Ripard wrote:
> >>> + if (machine_desc->init_irq)
> >>> + machine_desc->init_irq();
> >>> + else
> >>> + irqchip_init();
> >>
> >> There needs to be an empty version defined for !OF.
> >
> > Better:
> >
> > #ifdef CONFIG_OF
> > if (!machine_desc->init_irq)
> > irqchip_init();
> > else
> > #endif
>
> Or the new style:
>
> if (IS_ENABLED(CONFIG_OF) && !machine_desc->init_irq)
> irqchip_init();
> else
>
> That needs the empty version, but handles your case.
It shouldn't need the empty version at all - because the compiler should
optimize the "true" case away entirely.