[PATCH v2 5/7] clk: rockchip: add new clock-type for the cpuclk

Doug Anderson dianders at chromium.org
Mon Sep 15 16:58:08 PDT 2014


Heiko,

On Fri, Sep 12, 2014 at 3:30 PM, Heiko Stuebner <heiko at sntech.de> wrote:
> When changing the armclk on Rockchip SoCs it is supposed to be reparented
> to an alternate parent before changing the underlying pll and back after
> the change. Additionally there exist clocks that are very tightly bound to
> the armclk whose divider values are set according to the armclk rate.
>
> Add a special clock-type to handle all that. The rate table and divider
> values will be supplied from the soc-specific clock controllers.
>
> Signed-off-by: Heiko Stuebner <heiko at sntech.de>
> ---
>  drivers/clk/rockchip/Makefile  |   1 +
>  drivers/clk/rockchip/clk-cpu.c | 316 +++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/rockchip/clk.c     |  20 +++
>  drivers/clk/rockchip/clk.h     |  36 +++++
>  4 files changed, 373 insertions(+)
>  create mode 100644 drivers/clk/rockchip/clk-cpu.c
>
> diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile
> index ee6b077..bd8514d 100644
> --- a/drivers/clk/rockchip/Makefile
> +++ b/drivers/clk/rockchip/Makefile
> @@ -5,6 +5,7 @@
>  obj-y  += clk-rockchip.o
>  obj-y  += clk.o
>  obj-y  += clk-pll.o
> +obj-y  += clk-cpu.o
>  obj-$(CONFIG_RESET_CONTROLLER) += softrst.o
>
>  obj-y  += clk-rk3188.o
> diff --git a/drivers/clk/rockchip/clk-cpu.c b/drivers/clk/rockchip/clk-cpu.c
> new file mode 100644
> index 0000000..b8382b1
> --- /dev/null
> +++ b/drivers/clk/rockchip/clk-cpu.c
> @@ -0,0 +1,316 @@
> +/*
> + * Copyright (c) 2014 MundoReader S.L.
> + * Author: Heiko Stuebner <heiko at sntech.de>
> + *
> + * based on clk/samsung/clk-cpu.c
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + * Author: Thomas Abraham <thomas.ab at samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This file contains the utility function to register CPU clock for Samsung
> + * Exynos platforms. A CPU clock is defined as a clock supplied to a CPU or a
> + * group of CPUs. The CPU clock is typically derived from a hierarchy of clock
> + * blocks which includes mux and divider blocks. There are a number of other
> + * auxiliary clocks supplied to the CPU domain such as the debug blocks and AXI
> + * clock for CPU domain. The rates of these auxiliary clocks are related to the
> + * CPU clock rate and this relation is usually specified in the hardware manual
> + * of the SoC or supplied after the SoC characterization.
> + *
> + * The below implementation of the CPU clock allows the rate changes of the CPU
> + * clock and the corresponding rate changes of the auxillary clocks of the CPU
> + * domain. The platform clock driver provides a clock register configuration
> + * for each configurable rate which is then used to program the clock hardware
> + * registers to acheive a fast co-oridinated rate change for all the CPU domain
> + * clocks.
> + *
> + * On a rate change request for the CPU clock, the rate change is propagated
> + * upto the PLL supplying the clock to the CPU domain clock blocks. While the
> + * CPU domain PLL is reconfigured, the CPU domain clocks are driven using an
> + * alternate clock source. If required, the alternate clock source is divided
> + * down in order to keep the output clock rate within the previous OPP limits.
> + */
> +
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
> +#include <linux/clk-provider.h>
> +#include "clk.h"
> +
> +/**
> + * struct rockchip_cpuclk: information about clock supplied to a CPU core.
> + * @hw:                handle between ccf and cpu clock.
> + * @alt_parent:        alternate parent clock to use when switching the speed
> + *             of the primary parent clock.
> + * @reg_base:  base register for cpu-clock values.
> + * @clk_nb:    clock notifier registered for changes in clock speed of the
> + *             primary parent clock.
> + * @rate_count:        number of rates in the rate_table
> + * @rate_table:        pll-rates and their associated dividers
> + * @reg_data:  cpu-specific register settings
> + * @lock:      clock lock
> + */
> +struct rockchip_cpuclk {
> +       struct clk_hw                           hw;
> +
> +       struct clk_mux                          cpu_mux;
> +       const struct clk_ops                    *cpu_mux_ops;
> +
> +       struct clk                              *alt_parent;
> +       void __iomem                            *reg_base;
> +       struct notifier_block                   clk_nb;
> +       unsigned int                            rate_count;
> +       struct rockchip_cpuclk_rate_table       *rate_table;
> +       const struct rockchip_cpuclk_reg_data   *reg_data;
> +       spinlock_t                              *lock;
> +};
> +
> +#define to_rockchip_cpuclk_hw(hw) container_of(hw, struct rockchip_cpuclk, hw)
> +#define to_rockchip_cpuclk_nb(nb) \
> +                       container_of(nb, struct rockchip_cpuclk, clk_nb)
> +
> +static const struct rockchip_cpuclk_rate_table *rockchip_get_cpuclk_settings(
> +                           struct rockchip_cpuclk *cpuclk, unsigned long rate)
> +{
> +       const struct rockchip_cpuclk_rate_table *rate_table =
> +                                                       cpuclk->rate_table;
> +       int i;
> +
> +       for (i = 0; i < cpuclk->rate_count; i++) {
> +               if (rate == rate_table[i].prate)
> +                       return &rate_table[i];
> +       }
> +
> +       return NULL;
> +}
> +
> +static unsigned long rockchip_cpuclk_recalc_rate(struct clk_hw *hw,
> +                                       unsigned long parent_rate)
> +{
> +       struct rockchip_cpuclk *cpuclk = to_rockchip_cpuclk_hw(hw);
> +       const struct rockchip_cpuclk_reg_data *reg_data = cpuclk->reg_data;
> +       u32 clksel0 = readl_relaxed(cpuclk->reg_base + reg_data->core_reg);
> +
> +       clksel0 >>= reg_data->div_core_shift;
> +       clksel0 &= reg_data->div_core_mask;
> +       return parent_rate / (clksel0 + 1);
> +}
> +
> +static const struct clk_ops rockchip_cpuclk_ops = {
> +       .recalc_rate = rockchip_cpuclk_recalc_rate,
> +};
> +
> +static int rockchip_cpuclk_pre_rate_change(struct rockchip_cpuclk *cpuclk,
> +                                          struct clk_notifier_data *ndata)
> +{
> +       const struct rockchip_cpuclk_reg_data *reg_data = cpuclk->reg_data;
> +       const struct rockchip_cpuclk_rate_table *rate;
> +       unsigned long alt_prate, alt_div;
> +       int i;
> +
> +       rate = rockchip_get_cpuclk_settings(cpuclk, ndata->new_rate);
> +       if (!rate) {
> +               pr_err("%s: Invalid rate : %lu for cpuclk\n",
> +                      __func__, ndata->new_rate);
> +               return -EINVAL;
> +       }
> +
> +       alt_prate = clk_get_rate(cpuclk->alt_parent);
> +
> +       spin_lock(cpuclk->lock);
> +
> +       /*
> +        * If the old parent clock speed is less than the clock speed
> +        * of the alternate parent, then it should be ensured that at no point
> +        * the armclk speed is more than the old_rate until the dividers are
> +        * set.
> +        */
> +       if (alt_prate > ndata->old_rate) {
> +               alt_div =  DIV_ROUND_UP(alt_prate, ndata->old_rate) - 1;
> +               if (alt_div > reg_data->div_core_mask) {
> +                       pr_warn("%s: limiting alt-divider %lu to %d\n",
> +                               __func__, alt_div, reg_data->div_core_mask);
> +                       alt_div = reg_data->div_core_mask;
> +               }
> +
> +               pr_debug("%s: setting div %lu as alt-rate %lu > old-rate %lu\n",
> +                        __func__, alt_div, alt_prate, ndata->old_rate);
> +               writel_relaxed(HIWORD_UPDATE(alt_div,
> +                                            reg_data->div_core_mask,
> +                                            reg_data->div_core_shift),
> +                             cpuclk->reg_base + reg_data->core_reg);
> +       }
> +
> +       /* select alternate parent */
> +       writel(HIWORD_UPDATE(1, 1, reg_data->mux_core_shift),
> +              cpuclk->reg_base + reg_data->core_reg);

In the "alt_prate > ndata->old_rate" case there's a period of time
where you can be running super slow.

If we start as 126000 and we're going to switch to 594000, we decide
we need a divide by 5.  We apply the divide by 5 in one statement and
switch to 594MHz in a second statement.  That means there's a very
short period of time where we're at 126000 / 5 = 25200.  That's 25MHz.

I've found that when I stress out CPUfreq and have a lot of interrupts
coming in (like from USB) the my system hangs here.

Since the "alt div" and reparenting touch the same register, I think
we can do them in a single operation.  That works for me.

...but something isn't adding up for me, so I'm hesitant to suggest
this.  Somehow I only end up with the hang here and not down in the
post_rate_change().  I'll keep debugging...  It strangely enough seems
related to the rockchip_pll_notifier_cb()???


> +
> +       /* alternate parent is active now. set the dividers */
> +       for (i = 0; i < ARRAY_SIZE(rate->divs); i++) {
> +               const struct rockchip_cpuclk_clksel *clksel = &rate->divs[i];
> +
> +               if (!clksel->reg)
> +                       continue;
> +
> +               pr_debug("%s: setting reg 0x%x to 0x%x\n",
> +                        __func__, clksel->reg, clksel->val);
> +               writel(clksel->val , cpuclk->reg_base + clksel->reg);
> +       }
> +
> +       spin_unlock(cpuclk->lock);
> +       return 0;
> +}
> +
> +static int rockchip_cpuclk_post_rate_change(struct rockchip_cpuclk *cpuclk,
> +                                           struct clk_notifier_data *ndata)
> +{
> +       const struct rockchip_cpuclk_reg_data *reg_data = cpuclk->reg_data;
> +
> +       spin_lock(cpuclk->lock);
> +
> +       /* post-rate change event, re-mux back to primary parent */
> +       writel(HIWORD_UPDATE(0, 1, reg_data->mux_core_shift),
> +              cpuclk->reg_base + reg_data->core_reg);
> +
> +       /* remove any core dividers */
> +       writel(HIWORD_UPDATE(0, reg_data->div_core_mask,
> +                            reg_data->div_core_shift),
> +              cpuclk->reg_base + reg_data->core_reg);

I think we can combine the two writes above too.  If we combine the
pre, we should also combine here...


> +
> +       spin_unlock(cpuclk->lock);
> +       return 0;
> +}
> +
> +/*
> + * This clock notifier is called when the frequency of the parent clock
> + * of cpuclk is to be changed. This notifier handles the setting up all
> + * the divider clocks, remux to temporary parent and handling the safe
> + * frequency levels when using temporary parent.
> + */
> +static int rockchip_cpuclk_notifier_cb(struct notifier_block *nb,
> +                                       unsigned long event, void *data)
> +{
> +       struct clk_notifier_data *ndata = data;
> +       struct rockchip_cpuclk *cpuclk = to_rockchip_cpuclk_nb(nb);
> +       int ret = 0;
> +
> +       pr_debug("%s: event %lu, old_rate %lu, new_rate: %lu\n",
> +                __func__, event, ndata->old_rate, ndata->new_rate);
> +       if (event == PRE_RATE_CHANGE)
> +               ret = rockchip_cpuclk_pre_rate_change(cpuclk, ndata);
> +       else if (event == POST_RATE_CHANGE)
> +               ret = rockchip_cpuclk_post_rate_change(cpuclk, ndata);
> +
> +       return notifier_from_errno(ret);
> +}
> +
> +struct clk *rockchip_clk_register_cpuclk(const char *name,
> +                       const char **parent_names, u8 num_parents,
> +                       const struct rockchip_cpuclk_reg_data *reg_data,
> +                       struct rockchip_cpuclk_rate_table *rate_table,
> +                       void __iomem *reg_base, spinlock_t *lock)
> +{
> +       struct rockchip_cpuclk *cpuclk;
> +       struct clk_init_data init;
> +       struct clk *clk, *cclk;
> +       int ret;
> +
> +       if (!reg_data) {
> +               pr_err("%s: no soc register information\n", __func__);
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       if (num_parents != 2) {
> +               pr_err("%s: needs two parent clocks\n", __func__);
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       cpuclk = kzalloc(sizeof(*cpuclk), GFP_KERNEL);
> +       if (!cpuclk)
> +               return ERR_PTR(-ENOMEM);
> +
> +       init.name = name;
> +       init.parent_names = &parent_names[0];
> +       init.num_parents = 1;
> +       init.ops = &rockchip_cpuclk_ops;
> +
> +       /* only allow rate changes when we have a rate table */
> +       init.flags = rate_table ? CLK_SET_RATE_PARENT : 0;
> +
> +       /* disallow automatic parent changes by ccf */
> +       init.flags |= CLK_SET_RATE_NO_REPARENT;

For me it was important to add "CLK_GET_RATE_NOCACHE" here.  We're
mucking with this divider ourselves (above) so we need to make sure
that the clock framework isn't doing something wonky.

If I don't do that and I run this:

cd /sys/devices/system/cpu/cpu0/cpufreq
echo userspace > scaling_governor
while true; do
  echo 126000 > scaling_setspeed
  sleep .1
  echo 216000 > scaling_setspeed
  sleep .1
done

...and then in another session I run:

cd /sys/devices/system/cpu/cpu0/cpufreq
while true; do cat cpuinfo_cur_freq; sleep .2; done

...I see all sorts of wonky results.

> +
> +       cpuclk->reg_base = reg_base;
> +       cpuclk->lock = lock;
> +       cpuclk->reg_data = reg_data;
> +       cpuclk->clk_nb.notifier_call = rockchip_cpuclk_notifier_cb;
> +       cpuclk->hw.init = &init;
> +
> +       cpuclk->alt_parent = __clk_lookup(parent_names[1]);
> +       if (!cpuclk->alt_parent) {
> +               pr_err("%s: could not lookup alternate parent\n",
> +                      __func__);
> +               ret = -EINVAL;
> +               goto free_cpuclk;
> +       }
> +
> +       ret = clk_prepare_enable(cpuclk->alt_parent);
> +       if (ret) {
> +               pr_err("%s: could not enable alternate parent\n",
> +                      __func__);
> +               goto free_cpuclk;
> +       }
> +
> +       clk = __clk_lookup(parent_names[0]);
> +       if (!clk) {
> +               pr_err("%s: could not lookup parent clock %s\n",
> +                      __func__, parent_names[0]);
> +               ret = -EINVAL;
> +               goto free_cpuclk;
> +       }
> +
> +       ret = clk_notifier_register(clk, &cpuclk->clk_nb);
> +       if (ret) {
> +               pr_err("%s: failed to register clock notifier for %s\n",
> +                               __func__, name);
> +               goto free_cpuclk;
> +       }
> +
> +       if (rate_table) {
> +               int nrates;
> +
> +               /* find count of rates in rate_table */
> +               for (nrates = 0; rate_table[nrates].prate != 0; )
> +                       nrates++;
> +
> +               cpuclk->rate_count = nrates;
> +               cpuclk->rate_table = kmemdup(rate_table,
> +                                            sizeof(*rate_table) * nrates,
> +                                            GFP_KERNEL);
> +               if (!cpuclk->rate_table) {
> +                       pr_err("%s: could not allocate memory for cpuclk rates\n",
> +                              __func__);
> +                       ret = -ENOMEM;
> +                       goto unregister_notifier;
> +               }
> +       }
> +
> +       cclk = clk_register(NULL, &cpuclk->hw);
> +       if (IS_ERR(clk)) {
> +               pr_err("%s: could not register cpuclk %s\n", __func__,  name);
> +               ret = PTR_ERR(clk);
> +               goto free_rate_table;
> +       }
> +
> +       return cclk;
> +
> +free_rate_table:
> +       kfree(cpuclk->rate_table);
> +unregister_notifier:
> +       clk_notifier_unregister(clk, &cpuclk->clk_nb);
> +free_cpuclk:
> +       kfree(cpuclk);
> +       return ERR_PTR(ret);
> +}
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index d9c6db2..f87ac4a 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -297,6 +297,26 @@ void __init rockchip_clk_register_branches(
>         }
>  }
>
> +void __init rockchip_clk_register_armclk(unsigned int lookup_id,
> +                       const char *name, const char **parent_names,
> +                       u8 num_parents,
> +                       const struct rockchip_cpuclk_reg_data *reg_data,
> +                       struct rockchip_cpuclk_rate_table *rate_table)
> +{
> +       struct clk *clk;
> +
> +       clk = rockchip_clk_register_cpuclk(name, parent_names, num_parents,
> +                                          reg_data, rate_table, reg_base,
> +                                          &clk_lock);
> +       if (IS_ERR(clk)) {
> +               pr_err("%s: failed to register clock %s: %ld\n",
> +                      __func__, name, PTR_ERR(clk));
> +               return;
> +       }
> +
> +       rockchip_clk_add_lookup(clk, lookup_id);
> +}
> +
>  void __init rockchip_clk_protect_critical(const char *clocks[], int nclocks)
>  {
>         int i;
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> index 2b0bca1..e0ea61e 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -120,6 +120,38 @@ struct clk *rockchip_clk_register_pll(enum rockchip_pll_type pll_type,
>                 struct rockchip_pll_rate_table *rate_table,
>                 spinlock_t *lock);
>
> +struct rockchip_cpuclk_clksel {
> +       int reg;
> +       u32 val;
> +};
> +
> +#define ROCKCHIP_CPUCLK_NUM_DIVIDERS   2
> +struct rockchip_cpuclk_rate_table {
> +       unsigned long prate;
> +       struct rockchip_cpuclk_clksel divs[ROCKCHIP_CPUCLK_NUM_DIVIDERS];
> +};
> +
> +/**
> + * struct rockchip_cpuclk_reg_data: describes register offsets and masks of the cpuclock
> + * @core_reg:          register offset of the core settings register
> + * @div_core_shift:    core divider offset used to divide the pll value
> + * @div_core_mask:     core divider mask
> + * @mux_core_shift:    offset of the core multiplexer
> + */
> +struct rockchip_cpuclk_reg_data {
> +       int             core_reg;
> +       u8              div_core_shift;
> +       u32             div_core_mask;
> +       int             mux_core_reg;
> +       u8              mux_core_shift;
> +};
> +
> +struct clk *rockchip_clk_register_cpuclk(const char *name,
> +                       const char **parent_names, u8 num_parents,
> +                       const struct rockchip_cpuclk_reg_data *reg_data,
> +                       struct rockchip_cpuclk_rate_table *rate_table,
> +                       void __iomem *reg_base, spinlock_t *lock);
> +
>  #define PNAME(x) static const char *x[] __initconst
>
>  enum rockchip_clk_branch_type {
> @@ -329,6 +361,10 @@ void rockchip_clk_register_branches(struct rockchip_clk_branch *clk_list,
>                                     unsigned int nr_clk);
>  void rockchip_clk_register_plls(struct rockchip_pll_clock *pll_list,
>                                 unsigned int nr_pll, int grf_lock_offset);
> +void rockchip_clk_register_armclk(unsigned int lookup_id, const char *name,
> +                       const char **parent_names, u8 num_parents,
> +                       const struct rockchip_cpuclk_reg_data *reg_data,
> +                       struct rockchip_cpuclk_rate_table *rate_table);
>  void rockchip_clk_protect_critical(const char *clocks[], int nclocks);
>
>  #define ROCKCHIP_SOFTRST_HIWORD_MASK   BIT(0)
> --
> 2.0.1
>



More information about the Linux-rockchip mailing list