Re: [PATCHV2 1/2] OMAP3630: Clock: add clksel_shift to struct clk

2010-01-18 Thread Paul Walmsley
Correcting myself here:

On Mon, 18 Jan 2010, Paul Walmsley wrote:

> This will chew up another (4 bytes * number of struct clks) 

Well, not really, since clksel_shift is a u8, and for the time 
being, it can nestle next to that other u8.  But:

> and it doesn't seem necessary.

It still doesn't seem necessary and so should be avoided.

- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHV2 1/2] OMAP3630: Clock: add clksel_shift to struct clk

2010-01-18 Thread Paul Walmsley
Another comment.

On Mon, 18 Jan 2010, G.N, Vijayakumar wrote:

> diff --git a/arch/arm/mach-omap2/clock34xx_data.c 
> b/arch/arm/mach-omap2/clock34xx_data.c
> index 6473247..ed17501 100755
> --- a/arch/arm/mach-omap2/clock34xx_data.c
> +++ b/arch/arm/mach-omap2/clock34xx_data.c
> @@ -537,6 +537,7 @@ static struct clk dpll3_m3_ck = {
>   .init   = &omap2_init_clksel_parent,
>   .clksel_reg = OMAP_CM_REGADDR(OMAP3430_EMU_MOD, CM_CLKSEL1),
>   .clksel_mask= OMAP3430_DIV_DPLL3_MASK,
> + .clksel_shift   = OMAP3430_DIV_DPLL3_SHIFT,
>   .clksel = div16_dpll3_clksel,
>   .clkdm_name = "dpll3_clkdm",
>   .recalc = &omap2_clksel_recalc,
> @@ -677,6 +678,7 @@ static struct clk dpll4_m2_ck_3630 __initdata  = {
>   .init   = &omap2_init_clksel_parent,
>   .clksel_reg = OMAP_CM_REGADDR(PLL_MOD, OMAP3430_CM_CLKSEL3),
>   .clksel_mask= OMAP3630_DIV_96M_MASK,
> + .clksel_shift   = OMAP3430_DIV_96M_SHIFT,
>   .clksel = div32_dpll4_clksel,
>   .clkdm_name = "dpll4_clkdm",
>   .recalc = &omap2_clksel_recalc,

What commit is this patch supposed to apply on?  If it's not mainline then 
you need to state what commit or branch it's intended to apply on top of.


- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHV2 1/2] OMAP3630: Clock: add clksel_shift to struct clk

2010-01-18 Thread Paul Walmsley
Mike, Vijayakumar,

On Mon, 18 Jan 2010, G.N, Vijayakumar wrote:

> >From f9a7f877368830fdf28f9892940d05517f07a582 Mon Sep 17 00:00:00 2001
> From: Mike Turquette 
> Date: Tue, 12 Jan 2010 16:58:39 +0530
> Subject: [PATCH 1/2] OMAP3630: Clock: add clksel_shift to struct clk

Please don't post all this junk at the top of a patch, otherwise one of us 
has to edit the patch description by hand.  The "From:" line should be all 
that's necessary.

> Introduces clksel_shift to struct clk and populates the same for
> dpll4_m3_ck, dpll4_m4_ck, dpll4_m2_ck, dpll4_m6, dpll4_m5_ck and dpll3_m3.
> The purpose of this change is to make it easy for a generic function to
> refresh specific CM_CLKSEL registers on a per-clock basis by first
> adding then subtracting the shift value from the register.This is the 
> recommended sequence to solve HSDivider PWRDN limitations in OMAP3630.
> 
> Signed-off-by: Mike Turquette 
> Signed-off-by: Vijaykumar GN 
> ---
>  arch/arm/mach-omap2/clock34xx_data.c|6 ++
>  arch/arm/plat-omap/include/plat/clock.h |1 +
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/clock34xx_data.c 
> b/arch/arm/mach-omap2/clock34xx_data.c
> index 6473247..ed17501 100755
> --- a/arch/arm/mach-omap2/clock34xx_data.c
> +++ b/arch/arm/mach-omap2/clock34xx_data.c
> @@ -537,6 +537,7 @@ static struct clk dpll3_m3_ck = {
>   .init   = &omap2_init_clksel_parent,
>   .clksel_reg = OMAP_CM_REGADDR(OMAP3430_EMU_MOD, CM_CLKSEL1),
>   .clksel_mask= OMAP3430_DIV_DPLL3_MASK,
> + .clksel_shift   = OMAP3430_DIV_DPLL3_SHIFT,

Any reason why this is needed if the code can just do __ffs(clksel_mask) 
to get the shift offset?  This will chew up another (4 bytes * number of 
struct clks) and it doesn't seem necessary.


- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html