[PATCH V4 05/62] ST SPEAr: Formalized timer support

[jamie@jamieiles.com (Jamie Iles)]

Hi,

On Tue, Jan 18, 2011 at 12:41:33PM +0530, Viresh Kumar wrote:
> --- a/arch/arm/mach-spear3xx/spear3xx.c
> +++ b/arch/arm/mach-spear3xx/spear3xx.c
> @@ -523,5 +523,35 @@ struct pmx_dev pmx_plgpio_45_46_49_50 = {
>  	.mode_count = ARRAY_SIZE(pmx_plgpio_45_46_49_50_modes),
>  	.enb_on_reset = 1,
>  };
> +#endif /* CONFIG_MACH_SPEAR310 || CONFIG_MACH_SPEAR320 */
>  
> -#endif
> +static void __init spear3xx_timer_init(void)
> +{
> +	char pclk_name[] = "pll3_48m_clk";
> +	struct clk *gpt_clk, *pclk;
> +
> +	/* get the system timer clock */
> +	gpt_clk = clk_get_sys("gpt0", NULL);
> +	if (!gpt_clk) {
> +		pr_err("%s:couldn't get clk for gpt\n", __func__);
> +		BUG();
> +	}
> +
> +	/* get the suitable parent clock for timer*/
> +	pclk = clk_get(NULL, pclk_name);
> +	if (!pclk) {
> +		pr_err("%s:couldn't get %s as parent for gpt\n",
> +				__func__, pclk_name);
> +		BUG();
> +	}

These should probably be using IS_ERR() rather than checking against NULL to 
be consistent with the API.

[...]
> diff --git a/arch/arm/mach-spear6xx/spear600_evb.c b/arch/arm/mach-spear6xx/spear600_evb.c
> index daff8d0..bdd5b76 100644
> --- a/arch/arm/mach-spear6xx/spear600_evb.c
> +++ b/arch/arm/mach-spear6xx/spear600_evb.c
> @@ -42,10 +42,11 @@ static void __init spear600_evb_init(void)
>  		amba_device_register(amba_devs[i], &iomem_resource);
>  }
>  
> +

Nitpick, why the extra newline?

>  MACHINE_START(SPEAR600, "ST-SPEAR600-EVB")
>  	.boot_params	=	0x00000100,
>  	.map_io		=	spear6xx_map_io,
>  	.init_irq	=	spear6xx_init_irq,
> -	.timer		=	&spear_sys_timer,
> +	.timer		=	&spear6xx_timer,
>  	.init_machine	=	spear600_evb_init,
>  MACHINE_END
> diff --git a/arch/arm/mach-spear6xx/spear6xx.c b/arch/arm/mach-spear6xx/spear6xx.c
> index f2fe14e..ea4c356 100644
> --- a/arch/arm/mach-spear6xx/spear6xx.c
> +++ b/arch/arm/mach-spear6xx/spear6xx.c
> @@ -155,3 +155,34 @@ void __init spear6xx_map_io(void)
>  	/* This will initialize clock framework */
>  	clk_init();
>  }
> +
> +static void __init spear6xx_timer_init(void)
> +{
> +	char pclk_name[] = "pll3_48m_clk";
> +	struct clk *gpt_clk, *pclk;
> +
> +	/* get the system timer clock */
> +	gpt_clk = clk_get_sys("gpt0", NULL);
> +	if (!gpt_clk) {
> +		pr_err("%s:couldn't get clk for gpt\n", __func__);
> +		BUG();
> +	}
> +
> +	/* get the suitable parent clock for timer*/
> +	pclk = clk_get(NULL, pclk_name);
> +	if (!pclk) {
> +		pr_err("%s:couldn't get %s as parent for gpt\n",
> +				__func__, pclk_name);
> +		BUG();
> +	}

As above, IS_ERR() would be better here.

Jamie