[PATCH v2 1/1] ARM : omap3 : fix wrong container_of in clock36xx.c
Mike Turquette
mturquette at linaro.org
Fri May 31 15:32:11 EDT 2013
Quoting Jean-Philippe Francois (2013-05-30 01:50:27)
> omap36xx_pwrdn_clk_enable_with_hsdiv_restore expects the parent hw of the clock
> to be a clk_hw_omap. However, looking at cclock3xxx_data.c, all concerned clock
> have parent defined as clk_divider. Instead of using container_of to eventually get
> to the register and directly mess with the divider, change freq via clk_set_rate,
> and let the clock framework toggle the divider value.
> Tested with 3.9 on dm3730.
>
> Signed-off-by: Jean-Philippe Fran��ois <jp.francois at cynove.com>
Did you use git-format-patch to create this patch? Its a bit nicer to
use that or if you just use diff then use "diff -up" or "diff -uprN"
(taken from Documentation/SubmittingPatches.txt).
Also did you test this to make sure it works? I don't mean a boot test,
but actually disabling/re-enabling an HSDIVIDER on 3630? The previous
code just programmed the clksel field to 1, and this code divides the
rate by 2, then restores it. I just used that as an example in my
previous email and it needs to be verified that it works (though it
should if I remember this errata correctly).
If that testing is done and everything looks good then please add:
Acked-by: Mike Turquette <mturquette at linaro.org>
>
> Index: b/arch/arm/mach-omap2/clock36xx.c
> ===================================================================
> --- a/arch/arm/mach-omap2/clock36xx.c
> +++ b/arch/arm/mach-omap2/clock36xx.c
> @@ -39,30 +39,25 @@
> */
> int omap36xx_pwrdn_clk_enable_with_hsdiv_restore(struct clk_hw *clk)
> {
> - struct clk_hw_omap *parent;
> - struct clk_hw *parent_hw;
> - u32 dummy_v, orig_v, clksel_shift;
> int ret;
>
> /* Clear PWRDN bit of HSDIVIDER */
> ret = omap2_dflt_clk_enable(clk);
>
> - parent_hw = __clk_get_hw(__clk_get_parent(clk->clk));
> - parent = to_clk_hw_omap(parent_hw);
> -
> - /* Restore the dividers */
> + /* kick parent's clksel register after toggling PWRDN bit */
> if (!ret) {
> - clksel_shift = __ffs(parent->clksel_mask);
> - orig_v = __raw_readl(parent->clksel_reg);
> - dummy_v = orig_v;
> -
> - /* Write any other value different from the Read value */
> - dummy_v ^= (1 << clksel_shift);
> - __raw_writel(dummy_v, parent->clksel_reg);
> -
> - /* Write the original divider */
> - __raw_writel(orig_v, parent->clksel_reg);
> + struct clk *parent = clk_get_parent(clk->clk);
> + unsigned long parent_rate = clk_get_rate(parent);
> + ret = clk_set_rate(parent, parent_rate/2);
> + if(ret)
> + goto badfreq;
> + ret = clk_set_rate(parent, parent_rate);
> + if(ret)
> + goto badfreq;
> }
> + return ret;
>
> + badfreq :
> + omap2_dflt_clk_disable(clk);
> return ret;
> }
More information about the linux-arm-kernel
mailing list