Re: [PATCH 1/2] ARM: VIC: Create common infrastructure for vic DT bindings

[Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 03/23/2012 02:49 AM, Viresh Kumar wrote:
> Few platforms define there own struct of_device_id[] for vic pl19*. They don't
> do anything specific in init_irq() other than of_irq_init().
> 
> This patch creates this struct array in vic.c and make other platforms use it.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
> ---
>  arch/arm/common/vic.c               |   14 +++++++++++++-
>  arch/arm/include/asm/hardware/vic.h |    3 ++-
>  arch/arm/mach-picoxcell/common.c    |   12 +-----------
>  arch/arm/mach-spear6xx/spear6xx.c   |   12 +-----------
>  4 files changed, 17 insertions(+), 24 deletions(-)
> 

NAK

None of these platforms have gpio interrupts or any other board
interrupt controllers which need to be initialized early? I don't think
DT conversions are complete enough to tell.

We could do something like this, but there only needs to be a single
match list of all controllers and function. We should consolidate the
irqchips to drivers/irqchip first though. I have that action from Connect...

Rob

> diff --git a/arch/arm/common/vic.c b/arch/arm/common/vic.c
> index 7a66311..190813e 100644
> --- a/arch/arm/common/vic.c
> +++ b/arch/arm/common/vic.c
> @@ -398,7 +398,8 @@ void __init vic_init(void __iomem *base, unsigned int irq_start,
>  }
>  
>  #ifdef CONFIG_OF
> -int __init vic_of_init(struct device_node *node, struct device_node *parent)
> +static int __init vic_of_init(struct device_node *node, struct device_node
> +		*parent)
>  {
>  	void __iomem *regs;
>  	int irq_base;
> @@ -423,6 +424,17 @@ int __init vic_of_init(struct device_node *node, struct device_node *parent)
>  
>  	return -EIO;
>  }
> +
> +static const struct of_device_id vic_of_match[] __initconst = {
> +	{ .compatible = "arm,pl190-vic", .data = vic_of_init, },
> +	{ .compatible = "arm,pl192-vic", .data = vic_of_init, },
> +	{ /* Sentinel */ }
> +};
> +
> +void __init vic_init_irq(void)
> +{
> +	of_irq_init(vic_of_match);
> +}
>  #endif /* CONFIG OF */
>  
>  /*
> diff --git a/arch/arm/include/asm/hardware/vic.h b/arch/arm/include/asm/hardware/vic.h
> index e14af1a..02056a8 100644
> --- a/arch/arm/include/asm/hardware/vic.h
> +++ b/arch/arm/include/asm/hardware/vic.h
> @@ -42,6 +42,7 @@
>  
>  #ifndef __ASSEMBLY__
>  #include <linux/compiler.h>
> +#include <linux/init.h>
>  #include <linux/types.h>
>  
>  struct device_node;
> @@ -50,7 +51,7 @@ struct pt_regs;
>  void __vic_init(void __iomem *base, unsigned int irq_start, u32 vic_sources,
>  		u32 resume_sources, struct device_node *node);
>  void vic_init(void __iomem *base, unsigned int irq_start, u32 vic_sources, u32 resume_sources);
> -int vic_of_init(struct device_node *node, struct device_node *parent);
> +void __init vic_init_irq(void);
>  void vic_handle_irq(struct pt_regs *regs);
>  
>  #endif /* __ASSEMBLY__ */
> diff --git a/arch/arm/mach-picoxcell/common.c b/arch/arm/mach-picoxcell/common.c
> index a2e8ae8..fa0d7e9 100644
> --- a/arch/arm/mach-picoxcell/common.c
> +++ b/arch/arm/mach-picoxcell/common.c
> @@ -68,16 +68,6 @@ static const char *picoxcell_dt_match[] = {
>  	NULL
>  };
>  
> -static const struct of_device_id vic_of_match[] __initconst = {
> -	{ .compatible = "arm,pl192-vic", .data = vic_of_init, },
> -	{ /* Sentinel */ }
> -};
> -
> -static void __init picoxcell_init_irq(void)
> -{
> -	of_irq_init(vic_of_match);
> -}
> -
>  static void picoxcell_wdt_restart(char mode, const char *cmd)
>  {
>  	/*
> @@ -95,7 +85,7 @@ static void picoxcell_wdt_restart(char mode, const char *cmd)
>  DT_MACHINE_START(PICOXCELL, "Picochip picoXcell")
>  	.map_io		= picoxcell_map_io,
>  	.nr_irqs	= NR_IRQS_LEGACY,
> -	.init_irq	= picoxcell_init_irq,
> +	.init_irq	= vic_init_irq,
>  	.handle_irq	= vic_handle_irq,
>  	.timer		= &picoxcell_timer,
>  	.init_machine	= picoxcell_init_machine,
> diff --git a/arch/arm/mach-spear6xx/spear6xx.c b/arch/arm/mach-spear6xx/spear6xx.c
> index 2ed8b14..d436118 100644
> --- a/arch/arm/mach-spear6xx/spear6xx.c
> +++ b/arch/arm/mach-spear6xx/spear6xx.c
> @@ -102,19 +102,9 @@ static const char *spear600_dt_board_compat[] = {
>  	NULL
>  };
>  
> -static const struct of_device_id vic_of_match[] __initconst = {
> -	{ .compatible = "arm,pl190-vic", .data = vic_of_init, },
> -	{ /* Sentinel */ }
> -};
> -
> -static void __init spear6xx_dt_init_irq(void)
> -{
> -	of_irq_init(vic_of_match);
> -}
> -
>  DT_MACHINE_START(SPEAR600_DT, "ST SPEAr600 (Flattened Device Tree)")
>  	.map_io		=	spear6xx_map_io,
> -	.init_irq	=	spear6xx_dt_init_irq,
> +	.init_irq	=	vic_init_irq,
>  	.handle_irq	=	vic_handle_irq,
>  	.timer		=	&spear6xx_timer,
>  	.init_machine	=	spear600_dt_init,