[PATCH v4 3/7] clk: kona: don't init clocks at startup time
Mike Turquette
mturquette at linaro.org
Fri May 30 16:37:02 PDT 2014
Quoting Alex Elder (2014-05-30 13:53:04)
> +static int kona_clk_prepare(struct clk_hw *hw)
> {
> + struct kona_clk *bcm_clk = to_kona_clk(hw);
> + struct ccu_data *ccu = bcm_clk->ccu;
> + unsigned long flags;
> + int ret = 0;
> +
> + flags = ccu_lock(ccu);
> + __ccu_write_enable(ccu);
> +
> switch (bcm_clk->type) {
> case bcm_clk_peri:
> - return __peri_clk_init(bcm_clk);
> + if (!__peri_clk_init(bcm_clk))
> + ret = -EINVAL;
> + break;
> default:
> BUG();
> }
The switch-case only has one match, plus a default. Will there be others
in the future? Otherwise it can be replaced with an if-statement.
> - return -EINVAL;
> -}
> -
> -/* Set a CCU and all its clocks into their desired initial state */
> -bool __init kona_ccu_init(struct ccu_data *ccu)
> -{
> - unsigned long flags;
> - unsigned int which;
> - struct clk **clks = ccu->clk_data.clks;
> - bool success = true;
> -
> - flags = ccu_lock(ccu);
> - __ccu_write_enable(ccu);
> -
> - for (which = 0; which < ccu->clk_data.clk_num; which++) {
> - struct kona_clk *bcm_clk;
> -
> - if (!clks[which])
> - continue;
> - bcm_clk = to_kona_clk(__clk_get_hw(clks[which]));
> - success &= __kona_clk_init(bcm_clk);
> - }
>
> __ccu_write_disable(ccu);
> ccu_unlock(ccu, flags);
> - return success;
> +
> + return ret;
> }
Does this prepare callback "enable" a clock? E.g does a line NOT toggle
at a rate prior to this call, and then after this call completes that
same line is now toggling at a rate?
>
> -/* Clock operations */
> +static void kona_clk_unprepare(struct clk_hw *hw)
> +{
> + /* Nothing to do. */
> +}
Is doing nothing the right thing to do? Could power be saved somehow if
the .unprepare callback really gets called? Remember that if .unprepare
actually runs (because struct clk->prepare_count == 0) then the next
call to clk_prepare will actually call your .prepare callback and set up
the prereq clocks again. So the prereq clock initialization is no longer
a one-time thing, which might afford you some optimizations.
Regards,
Mike
>
> static int kona_peri_clk_enable(struct clk_hw *hw)
> {
> @@ -1264,6 +1258,8 @@ static int kona_peri_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> }
>
> struct clk_ops kona_peri_clk_ops = {
> + .prepare = kona_clk_prepare,
> + .unprepare = kona_clk_unprepare,
> .enable = kona_peri_clk_enable,
> .disable = kona_peri_clk_disable,
> .is_enabled = kona_peri_clk_is_enabled,
> diff --git a/drivers/clk/bcm/clk-kona.h b/drivers/clk/bcm/clk-kona.h
> index e9a8466..3409111 100644
> --- a/drivers/clk/bcm/clk-kona.h
> +++ b/drivers/clk/bcm/clk-kona.h
> @@ -511,6 +511,5 @@ extern u64 scaled_div_build(struct bcm_clk_div *div, u32 div_value,
> extern struct clk *kona_clk_setup(struct kona_clk *bcm_clk);
> extern void __init kona_dt_ccu_setup(struct ccu_data *ccu,
> struct device_node *node);
> -extern bool __init kona_ccu_init(struct ccu_data *ccu);
>
> #endif /* _CLK_KONA_H */
> --
> 1.9.1
>
More information about the linux-arm-kernel
mailing list