* [PATCH v7 1/6] clk: samsung: add infrastructure to register cpu clocks
2014-07-14 13:38 [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms Thomas Abraham
@ 2014-07-14 13:38 ` Thomas Abraham
2014-07-19 12:55 ` Tomasz Figa
2014-07-14 13:38 ` [PATCH v7 2/6] clk: samsung: register exynos5420 apll/kpll configuration data Thomas Abraham
` (6 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Thomas Abraham @ 2014-07-14 13:38 UTC (permalink / raw)
To: linux-arm-kernel
From: Thomas Abraham <thomas.ab@samsung.com>
This patch defines a new clock type for CPU clock provider and adds
infrastructure to register the CPU clock providers for Samsung platforms.
The CPU clock provider supplies the clock to the CPU clock domain. The
composition and organization of the CPU clock provider could vary among
Exynos SoCs and so this new clock type provides a way to encapsulate these
blocks into CPU clock type.
Cc: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
Reviewed-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
Tested-by: Arjun K.V <arjun.kv@samsung.com>
---
drivers/clk/samsung/Makefile | 2 +-
drivers/clk/samsung/clk-cpu.c | 576 +++++++++++++++++++++++++++++++++++++++++
drivers/clk/samsung/clk.h | 5 +
3 files changed, 582 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/samsung/clk-cpu.c
diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
index 69e8177..f4edd31 100644
--- a/drivers/clk/samsung/Makefile
+++ b/drivers/clk/samsung/Makefile
@@ -2,7 +2,7 @@
# Samsung Clock specific Makefile
#
-obj-$(CONFIG_COMMON_CLK) += clk.o clk-pll.o
+obj-$(CONFIG_COMMON_CLK) += clk.o clk-pll.o clk-cpu.o
obj-$(CONFIG_SOC_EXYNOS3250) += clk-exynos3250.o
obj-$(CONFIG_ARCH_EXYNOS4) += clk-exynos4.o
obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o
diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c
new file mode 100644
index 0000000..0d62968
--- /dev/null
+++ b/drivers/clk/samsung/clk-cpu.c
@@ -0,0 +1,576 @@
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Author: Thomas Abraham <thomas.ab@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 functions to register the CPU clocks
+ * for Samsung platforms.
+*/
+
+#include <linux/errno.h>
+#include "clk.h"
+
+#define E4210_SRC_CPU 0x0
+#define E4210_STAT_CPU 0x200
+#define E4210_DIV_CPU0 0x300
+#define E4210_DIV_CPU1 0x304
+#define E4210_DIV_STAT_CPU0 0x400
+#define E4210_DIV_STAT_CPU1 0x404
+
+#define MAX_DIV 8
+#define DIV_MASK 7
+#define DIV_MASK_ALL 0xffffffff
+#define MUX_MASK 7
+
+#define E4210_DIV0_RATIO0_MASK 0x7
+#define E4210_DIV1_HPM_MASK ((0x7 << 4) | (0x7 << 0))
+#define E4210_MUX_HPM_MASK (1 << 20)
+#define E4210_DIV0_ATB_SHIFT 16
+#define E4210_DIV0_ATB_MASK (DIV_MASK << E4210_DIV0_ATB_SHIFT)
+
+#define E4210_CPU_DIV0(apll, pclk_dbg, atb, periph, corem1, corem0) \
+ (((apll) << 24) | ((pclk_dbg) << 20) | ((atb) << 16) | \
+ ((periph) << 12) | ((corem1) << 8) | ((corem0) << 4))
+#define E4210_CPU_DIV1(hpm, copy) \
+ (((hpm) << 4) | ((copy) << 0))
+
+#define E5250_CPU_DIV0(apll, pclk_dbg, atb, periph, acp, cpud) \
+ (((apll << 24) | (pclk_dbg << 20) | (atb << 16) | \
+ (periph << 12) | (acp << 8) | (cpud << 4)))
+#define E5250_CPU_DIV1(hpm, copy) \
+ (((hpm) << 4) | (copy))
+
+#define E5420_EGL_DIV0(apll, pclk_dbg, atb, cpud) \
+ (((apll << 24) | (pclk_dbg << 20) | (atb << 16) | \
+ (cpud << 4)))
+#define E5420_KFC_DIV(kpll, pclk, aclk) \
+ (((kpll << 24) | (pclk << 20) | (aclk << 4)))
+
+enum cpuclk_type {
+ EXYNOS4210,
+ EXYNOS5250,
+ EXYNOS5420,
+};
+
+/**
+ * struct exynos4210_cpuclk_data: config data to setup cpu clocks.
+ * @prate: frequency of the primary parent clock (in KHz).
+ * @div0: value to be programmed in the div_cpu0 register.
+ * @div1: value to be programmed in the div_cpu1 register.
+ *
+ * This structure holds the divider configuration data for dividers in the CPU
+ * clock domain. The parent frequency@which these divider values are valid is
+ * specified in @prate. The @prate is the frequency of the primary parent clock.
+ * For CPU clock domains that do not have a DIV1 register, the @div1 member
+ * is optional.
+ */
+struct exynos4210_cpuclk_data {
+ unsigned long prate;
+ unsigned int div0;
+ unsigned int div1;
+};
+
+/**
+ * struct exynos_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.
+ * @ctrl_base: base address of the clock controller.
+ * @offset: offset from the ctrl_base address where the CPU clock div/mux
+ * registers can be accessed.
+ * @lock: cpu clock domain register access lock.
+ * @type: type of the CPU clock.
+ * @data: optional data which the actual instantiation of this clock
+ * can use.
+ * @clk_nb: clock notifier registered for changes in clock speed of the
+ * primary parent clock.
+ * @pre_rate_cb: callback function to handle PRE_RATE_CHANGE notification
+ * of the primary parent clock.
+ * @post_rate_cb: callback function to handle POST_RATE_CHANGE notification
+ * of the primary parent clock.
+ *
+ * This structure holds information required for programming the cpu clock for
+ * various clock speeds.
+ */
+struct exynos_cpuclk {
+ struct clk_hw hw;
+ struct clk *alt_parent;
+ void __iomem *ctrl_base;
+ unsigned long offset;
+ spinlock_t *lock;
+ enum cpuclk_type type;
+ const void *data;
+ struct notifier_block clk_nb;
+ int (*pre_rate_cb)(struct clk_notifier_data *,
+ struct exynos_cpuclk *,
+ void __iomem *base);
+ int (*post_rate_cb)(struct clk_notifier_data *,
+ struct exynos_cpuclk *,
+ void __iomem *base);
+};
+
+#define to_exynos_cpuclk_hw(hw) container_of(hw, struct exynos_cpuclk, hw)
+#define to_exynos_cpuclk_nb(nb) container_of(nb, struct exynos_cpuclk, clk_nb)
+
+/**
+ * struct exynos_cpuclk_soc_data: soc specific data for cpu clocks.
+ * @ops: clock operations to be used for this clock.
+ * @offset: optional offset from base of clock controller register base, to
+ * be used when accessing clock controller registers related to the
+ * CPU clock.
+ * @data: SoC specific data for cpuclk configuration (optional).
+ * @data_size: size of the data contained in @data member.
+ * @type: type of the CPU clock.
+ * @pre_rate_cb: callback function to handle PRE_RATE_CHANGE notification
+ * of the primary parent clock.
+ * @post_rate_cb: callback function to handle POST_RATE_CHANGE notification
+ * of the primary parent clock.
+ *
+ * This structure provides SoC specific data for CPU clocks. Based on
+ * the compatible value of the clock controller node, the value of the
+ * fields in this structure can be populated.
+ */
+struct exynos_cpuclk_soc_data {
+ const struct clk_ops *ops;
+ unsigned int offset;
+ const void *data;
+ const unsigned int data_size;
+ enum cpuclk_type type;
+ int (*pre_rate_cb)(struct clk_notifier_data *,
+ struct exynos_cpuclk *,
+ void __iomem *base);
+ int (*post_rate_cb)(struct clk_notifier_data *,
+ struct exynos_cpuclk *,
+ void __iomem *base);
+};
+
+/*
+ * Helper function to wait until divider(s) have stabilized after the divider
+ * value has changed.
+ */
+static void wait_until_divider_stable(void __iomem *div_reg, unsigned long mask)
+{
+ unsigned long timeout = jiffies + msecs_to_jiffies(10);
+
+ do {
+ if (!(readl(div_reg) & mask))
+ return;
+ } while (time_before(jiffies, timeout));
+
+ pr_err("%s: timeout in divider stablization\n", __func__);
+}
+
+/*
+ * Helper function to wait until mux has stabilized after the mux selection
+ * value was changed.
+ */
+static void wait_until_mux_stable(void __iomem *mux_reg, u32 mux_pos,
+ unsigned long mux_value)
+{
+ unsigned long timeout = jiffies + msecs_to_jiffies(10);
+
+ do {
+ if (((readl(mux_reg) >> mux_pos) & MUX_MASK) == mux_value)
+ return;
+ } while (time_before(jiffies, timeout));
+
+ pr_err("%s: re-parenting mux timed-out\n", __func__);
+}
+
+/* common round rate callback useable for all types of CPU clocks */
+static long exynos_cpuclk_round_rate(struct clk_hw *hw,
+ unsigned long drate, unsigned long *prate)
+{
+ struct clk *parent = __clk_get_parent(hw->clk);
+ *prate = __clk_round_rate(parent, drate);
+ return *prate;
+}
+
+/* common recalc rate callback useable for all types of CPU clocks */
+static unsigned long exynos_cpuclk_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ return parent_rate;
+}
+
+static const struct clk_ops exynos_cpuclk_clk_ops = {
+ .recalc_rate = exynos_cpuclk_recalc_rate,
+ .round_rate = exynos_cpuclk_round_rate,
+};
+
+/*
+ * Calculates the divider value to be set for deriving drate from prate.
+ * Divider value is actual divider value - 1.
+ */
+static unsigned long _calc_div(unsigned long prate, unsigned long drate)
+{
+ unsigned long div = DIV_ROUND_UP(prate, drate) - 1;
+
+ WARN_ON(div >= MAX_DIV);
+ return div;
+}
+
+/*
+ * This notifier function is called for the pre-rate and post-rate change
+ * notifications of the parent clock of cpuclk.
+ */
+static int exynos_cpuclk_notifier_cb(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct clk_notifier_data *ndata = data;
+ struct exynos_cpuclk *cpuclk = to_exynos_cpuclk_nb(nb);
+ void __iomem *base = cpuclk->ctrl_base + cpuclk->offset;
+ int err = 0;
+
+ if (event == PRE_RATE_CHANGE)
+ err = cpuclk->pre_rate_cb(ndata, cpuclk, base);
+ else if (event == POST_RATE_CHANGE)
+ err = cpuclk->post_rate_cb(ndata, cpuclk, base);
+
+ return notifier_from_errno(err);
+}
+
+/* helper function to register a cpu clock */
+static int __init exynos_cpuclk_register(struct samsung_clk_provider *ctx,
+ unsigned int lookup_id, const char *name, const char *parent,
+ const char *alt_parent, struct device_node *np,
+ const struct exynos_cpuclk_soc_data *soc_data)
+{
+ struct exynos_cpuclk *cpuclk;
+ struct clk_init_data init;
+ struct clk *clk;
+ void *data;
+ int ret = 0;
+
+ cpuclk = kzalloc(sizeof(*cpuclk), GFP_KERNEL);
+ if (!cpuclk)
+ return -ENOMEM;
+
+ data = kmalloc(soc_data->data_size, GFP_KERNEL);
+ if (!data) {
+ ret = -ENOMEM;
+ goto free_cpuclk;
+ }
+
+ init.name = name;
+ init.flags = CLK_SET_RATE_PARENT;
+ init.parent_names = &parent;
+ init.num_parents = 1;
+ init.ops = soc_data->ops;
+
+ cpuclk->hw.init = &init;
+ cpuclk->ctrl_base = ctx->reg_base;
+ cpuclk->lock = &ctx->lock;
+ cpuclk->offset = soc_data->offset;
+ cpuclk->type = soc_data->type;
+ cpuclk->pre_rate_cb = soc_data->pre_rate_cb;
+ cpuclk->post_rate_cb = soc_data->post_rate_cb;
+ memcpy(data, soc_data->data, soc_data->data_size);
+ cpuclk->data = data;
+
+ cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb;
+ ret = clk_notifier_register(__clk_lookup(parent), &cpuclk->clk_nb);
+ if (ret) {
+ pr_err("%s: failed to register clock notifier for %s\n",
+ __func__, name);
+ goto free_cpuclk_data;
+ }
+
+ cpuclk->alt_parent = __clk_lookup(alt_parent);
+ if (!cpuclk->alt_parent) {
+ pr_err("%s: could not lookup alternate parent %s\n",
+ __func__, alt_parent);
+ ret = -EINVAL;
+ goto unregister_clk_nb;
+ }
+
+ clk = 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 unregister_clk_nb;
+ }
+
+ samsung_clk_add_lookup(ctx, clk, lookup_id);
+ return 0;
+
+unregister_clk_nb:
+ clk_notifier_unregister(__clk_lookup(parent), &cpuclk->clk_nb);
+free_cpuclk_data:
+ kfree(cpuclk->data);
+free_cpuclk:
+ kfree(cpuclk);
+ return ret;
+}
+
+/*
+ * Helper function to set the 'safe' dividers for the CPU clock. The parameters
+ * div and mask contain the divider value and the register bit mask of the
+ * dividers to be programmed.
+ */
+static void exynos4210_set_safe_div(void __iomem *base, unsigned long div,
+ unsigned long mask)
+{
+ unsigned long div0;
+
+ div0 = readl(base + E4210_DIV_CPU0);
+ div0 = (div0 & ~mask) | div;
+ writel(div0, base + E4210_DIV_CPU0);
+ wait_until_divider_stable(base + E4210_DIV_STAT_CPU0, mask);
+}
+
+/* handler for pre-rate change notification from parent clock */
+static int exynos4210_cpuclk_pre_rate_change(struct clk_notifier_data *ndata,
+ struct exynos_cpuclk *cpuclk, void __iomem *base)
+{
+ const struct exynos4210_cpuclk_data *cpuclk_data = cpuclk->data;
+ unsigned long alt_prate = clk_get_rate(cpuclk->alt_parent);
+ unsigned long alt_div = 0, alt_div_mask = DIV_MASK;
+ unsigned long div0, div1 = 0, mux_reg;
+
+ /* find out the divider values to use for clock data */
+ while ((cpuclk_data->prate * 1000) != ndata->new_rate) {
+ if (cpuclk_data->prate == 0)
+ return -EINVAL;
+ cpuclk_data++;
+ }
+
+ /* For the selected PLL clock frequency, get the pre-defined divider
+ * values. If the clock for sclk_hpm is not sourced from apll, then
+ * the values for DIV_COPY and DIV_HPM dividers need not be set.
+ */
+ div0 = cpuclk_data->div0;
+ if (cpuclk->type != EXYNOS5420) {
+ div1 = cpuclk_data->div1;
+ if (readl(base + E4210_SRC_CPU) & E4210_MUX_HPM_MASK) {
+ div1 = readl(base + E4210_DIV_CPU1) &
+ E4210_DIV1_HPM_MASK;
+ div1 |= ((cpuclk_data->div1) & ~E4210_DIV1_HPM_MASK);
+ }
+ }
+
+ spin_lock(cpuclk->lock);
+
+ /*
+ * If the new and 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_prate until the dividers are
+ * set.
+ */
+ if (alt_prate > ndata->old_rate) {
+ alt_div = _calc_div(alt_prate, ndata->old_rate);
+ if (cpuclk->type == EXYNOS4210) {
+ /*
+ * In Exynos4210, ATB clock parent is also mout_core. So
+ * ATB clock also needs to be mantained at safe speed.
+ */
+ alt_div |= E4210_DIV0_ATB_MASK;
+ alt_div_mask |= E4210_DIV0_ATB_MASK;
+ }
+ exynos4210_set_safe_div(base, alt_div, alt_div_mask);
+ div0 |= alt_div;
+ }
+
+ /* select sclk_mpll as the alternate parent */
+ mux_reg = readl(base + E4210_SRC_CPU);
+ writel(mux_reg | (1 << 16), base + E4210_SRC_CPU);
+ wait_until_mux_stable(base + E4210_STAT_CPU, 16, 2);
+
+ /* alternate parent is active now. set the dividers */
+ writel(div0, base + E4210_DIV_CPU0);
+ wait_until_divider_stable(base + E4210_DIV_STAT_CPU0, DIV_MASK_ALL);
+
+ if (cpuclk->type != EXYNOS5420) {
+ writel(div1, base + E4210_DIV_CPU1);
+ wait_until_divider_stable(base + E4210_DIV_STAT_CPU1,
+ DIV_MASK_ALL);
+ }
+
+ spin_unlock(cpuclk->lock);
+ return 0;
+}
+
+/* handler for post-rate change notification from parent clock */
+static int exynos4210_cpuclk_post_rate_change(struct clk_notifier_data *ndata,
+ struct exynos_cpuclk *cpuclk, void __iomem *base)
+{
+ const struct exynos4210_cpuclk_data *cpuclk_data = cpuclk->data;
+ unsigned long div = 0, div_mask = DIV_MASK;
+ unsigned long mux_reg;
+
+ spin_lock(cpuclk->lock);
+
+ /* select mout_apll as the alternate parent */
+ mux_reg = readl(base + E4210_SRC_CPU);
+ writel(mux_reg & ~(1 << 16), base + E4210_SRC_CPU);
+ wait_until_mux_stable(base + E4210_STAT_CPU, 16, 1);
+
+ if (cpuclk->type == EXYNOS4210) {
+ /* find out the divider values to use for clock data */
+ while ((cpuclk_data->prate * 1000) != ndata->new_rate) {
+ if (cpuclk_data->prate == 0)
+ return -EINVAL;
+ cpuclk_data++;
+ }
+
+ div |= (cpuclk_data->div0 & E4210_DIV0_ATB_MASK);
+ div_mask |= E4210_DIV0_ATB_MASK;
+ }
+
+ exynos4210_set_safe_div(base, div, div_mask);
+ spin_unlock(cpuclk->lock);
+ return 0;
+}
+
+static const struct exynos4210_cpuclk_data e4210_armclk_d[] __initconst = {
+ { 1200000, E4210_CPU_DIV0(7, 1, 4, 3, 7, 3), E4210_CPU_DIV1(0, 5), },
+ { 1000000, E4210_CPU_DIV0(7, 1, 4, 3, 7, 3), E4210_CPU_DIV1(0, 4), },
+ { 800000, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), },
+ { 500000, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), },
+ { 400000, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), },
+ { 200000, E4210_CPU_DIV0(0, 1, 1, 1, 3, 1), E4210_CPU_DIV1(0, 3), },
+ { 0 },
+};
+
+static const struct exynos4210_cpuclk_data e5250_armclk_d[] __initconst = {
+ { 1700000, E5250_CPU_DIV0(5, 3, 7, 7, 7, 3), E5250_CPU_DIV1(2, 0), },
+ { 1600000, E5250_CPU_DIV0(4, 1, 7, 7, 7, 3), E5250_CPU_DIV1(2, 0), },
+ { 1500000, E5250_CPU_DIV0(4, 1, 7, 7, 7, 2), E5250_CPU_DIV1(2, 0), },
+ { 1400000, E5250_CPU_DIV0(4, 1, 6, 7, 7, 2), E5250_CPU_DIV1(2, 0), },
+ { 1300000, E5250_CPU_DIV0(3, 1, 6, 7, 7, 2), E5250_CPU_DIV1(2, 0), },
+ { 1200000, E5250_CPU_DIV0(3, 1, 5, 7, 7, 2), E5250_CPU_DIV1(2, 0), },
+ { 1100000, E5250_CPU_DIV0(3, 1, 5, 7, 7, 3), E5250_CPU_DIV1(2, 0), },
+ { 1000000, E5250_CPU_DIV0(2, 1, 4, 7, 7, 1), E5250_CPU_DIV1(2, 0), },
+ { 900000, E5250_CPU_DIV0(2, 1, 4, 7, 7, 1), E5250_CPU_DIV1(2, 0), },
+ { 800000, E5250_CPU_DIV0(2, 1, 4, 7, 7, 1), E5250_CPU_DIV1(2, 0), },
+ { 700000, E5250_CPU_DIV0(1, 1, 3, 7, 7, 1), E5250_CPU_DIV1(2, 0), },
+ { 600000, E5250_CPU_DIV0(1, 1, 3, 7, 7, 1), E5250_CPU_DIV1(2, 0), },
+ { 500000, E5250_CPU_DIV0(1, 1, 2, 7, 7, 1), E5250_CPU_DIV1(2, 0), },
+ { 400000, E5250_CPU_DIV0(1, 1, 2, 7, 7, 1), E5250_CPU_DIV1(2, 0), },
+ { 300000, E5250_CPU_DIV0(1, 1, 1, 7, 7, 1), E5250_CPU_DIV1(2, 0), },
+ { 200000, E5250_CPU_DIV0(1, 1, 1, 7, 7, 1), E5250_CPU_DIV1(2, 0), },
+ { 0 },
+};
+
+static const struct exynos4210_cpuclk_data e5420_eglclk_d[] __initconst = {
+ { 1800000, E5420_EGL_DIV0(3, 7, 7, 4), },
+ { 1700000, E5420_EGL_DIV0(3, 7, 7, 3), },
+ { 1600000, E5420_EGL_DIV0(3, 7, 7, 3), },
+ { 1500000, E5420_EGL_DIV0(3, 7, 7, 3), },
+ { 1400000, E5420_EGL_DIV0(3, 7, 7, 3), },
+ { 1300000, E5420_EGL_DIV0(3, 7, 7, 2), },
+ { 1200000, E5420_EGL_DIV0(3, 7, 7, 2), },
+ { 1100000, E5420_EGL_DIV0(3, 7, 7, 2), },
+ { 1000000, E5420_EGL_DIV0(3, 6, 6, 2), },
+ { 900000, E5420_EGL_DIV0(3, 6, 6, 2), },
+ { 800000, E5420_EGL_DIV0(3, 5, 5, 2), },
+ { 700000, E5420_EGL_DIV0(3, 5, 5, 2), },
+ { 600000, E5420_EGL_DIV0(3, 4, 4, 2), },
+ { 500000, E5420_EGL_DIV0(3, 3, 3, 2), },
+ { 400000, E5420_EGL_DIV0(3, 3, 3, 2), },
+ { 300000, E5420_EGL_DIV0(3, 3, 3, 2), },
+ { 200000, E5420_EGL_DIV0(3, 3, 3, 2), },
+ { 0 },
+};
+
+static const struct exynos4210_cpuclk_data e5420_kfcclk_d[] __initconst = {
+ { 1300000, E5420_KFC_DIV(3, 5, 2), },
+ { 1200000, E5420_KFC_DIV(3, 5, 2), },
+ { 1100000, E5420_KFC_DIV(3, 5, 2), },
+ { 1000000, E5420_KFC_DIV(3, 5, 2), },
+ { 900000, E5420_KFC_DIV(3, 5, 2), },
+ { 800000, E5420_KFC_DIV(3, 5, 2), },
+ { 700000, E5420_KFC_DIV(3, 4, 2), },
+ { 600000, E5420_KFC_DIV(3, 4, 2), },
+ { 500000, E5420_KFC_DIV(3, 4, 2), },
+ { 400000, E5420_KFC_DIV(3, 3, 2), },
+ { 300000, E5420_KFC_DIV(3, 3, 2), },
+ { 200000, E5420_KFC_DIV(3, 3, 2), },
+ { 0 },
+};
+
+static const struct exynos_cpuclk_soc_data e4210_clk_soc_data __initconst = {
+ .ops = &exynos_cpuclk_clk_ops,
+ .offset = 0x14200,
+ .data = e4210_armclk_d,
+ .data_size = sizeof(e4210_armclk_d),
+ .type = EXYNOS4210,
+ .pre_rate_cb = exynos4210_cpuclk_pre_rate_change,
+ .post_rate_cb = exynos4210_cpuclk_post_rate_change,
+};
+
+static const struct exynos_cpuclk_soc_data e5250_clk_soc_data __initconst = {
+ .ops = &exynos_cpuclk_clk_ops,
+ .offset = 0x200,
+ .data = e5250_armclk_d,
+ .data_size = sizeof(e5250_armclk_d),
+ .type = EXYNOS5250,
+ .pre_rate_cb = exynos4210_cpuclk_pre_rate_change,
+ .post_rate_cb = exynos4210_cpuclk_post_rate_change,
+};
+
+static const struct exynos_cpuclk_soc_data e5420_clk_soc_data[] __initconst = {
+ {
+ /* Cluster 0 (A15) CPU clock data */
+ .ops = &exynos_cpuclk_clk_ops,
+ .offset = 0x200,
+ .data = e5420_eglclk_d,
+ .data_size = sizeof(e5420_eglclk_d),
+ .type = EXYNOS5420,
+ .pre_rate_cb = exynos4210_cpuclk_pre_rate_change,
+ .post_rate_cb = exynos4210_cpuclk_post_rate_change,
+ }, {
+ /* Cluster 1 (A7) CPU clock data */
+ .ops = &exynos_cpuclk_clk_ops,
+ .offset = 0x28200,
+ .data = e5420_kfcclk_d,
+ .data_size = sizeof(e5420_kfcclk_d),
+ .type = EXYNOS5420,
+ .pre_rate_cb = exynos4210_cpuclk_pre_rate_change,
+ .post_rate_cb = exynos4210_cpuclk_post_rate_change,
+ },
+};
+
+static const struct of_device_id exynos_cpuclk_ids[] __initconst = {
+ { .compatible = "samsung,exynos4210-clock",
+ .data = &e4210_clk_soc_data, },
+ { .compatible = "samsung,exynos5250-clock",
+ .data = &e5250_clk_soc_data, },
+ { .compatible = "samsung,exynos5420-clock",
+ .data = &e5420_clk_soc_data, },
+ { },
+};
+
+/**
+ * exynos_register_cpu_clock: register cpu clock with ccf.
+ * @ctx: driver context.
+ * @cluster_id: cpu cluster number to which this clock is connected.
+ * @lookup_id: cpuclk clock output id for the clock controller.
+ * @name: the name of the cpu clock.
+ * @parent: name of the parent clock for cpuclk.
+ * @alt_parent: name of the alternate clock parent.
+ * @np: device tree node pointer of the clock controller.
+ */
+int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
+ unsigned int cluster_id, unsigned int lookup_id,
+ const char *name, const char *parent,
+ const char *alt_parent, struct device_node *np)
+{
+ const struct of_device_id *match;
+ const struct exynos_cpuclk_soc_data *data = NULL;
+
+ if (!np)
+ return -EINVAL;
+
+ match = of_match_node(exynos_cpuclk_ids, np);
+ if (!match)
+ return -EINVAL;
+
+ data = match->data;
+ data += cluster_id;
+ return exynos_cpuclk_register(ctx, lookup_id, name, parent,
+ alt_parent, np, data);
+}
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index 92131f9..0e05ee9 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -374,4 +374,9 @@ extern struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
const unsigned long *rdump,
unsigned long nr_rdump);
+extern int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
+ unsigned int cluster_id, unsigned int lookup_id,
+ const char *name, const char *parent,
+ const char *alt_parent, struct device_node *np);
+
#endif /* __SAMSUNG_CLK_H */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v7 1/6] clk: samsung: add infrastructure to register cpu clocks
2014-07-14 13:38 ` [PATCH v7 1/6] clk: samsung: add infrastructure to register cpu clocks Thomas Abraham
@ 2014-07-19 12:55 ` Tomasz Figa
2014-07-29 5:35 ` Thomas Abraham
0 siblings, 1 reply; 25+ messages in thread
From: Tomasz Figa @ 2014-07-19 12:55 UTC (permalink / raw)
To: linux-arm-kernel
Hi Thomas,
Please see my comments inline.
On 14.07.2014 15:38, Thomas Abraham wrote:
[snip]
> diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c
> new file mode 100644
> index 0000000..0d62968
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-cpu.c
> @@ -0,0 +1,576 @@
> +/*
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + * Author: Thomas Abraham <thomas.ab@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 functions to register the CPU clocks
> + * for Samsung platforms.
I'd expect few words here or in separate comment on how the code works,
i.e. assumptions made, most important procedures, etc. This is a complex
piece of code for quite complex hardware, so proper documentation is
essential.
> +*/
> +
> +#include <linux/errno.h>
> +#include "clk.h"
> +
> +#define E4210_SRC_CPU 0x0
> +#define E4210_STAT_CPU 0x200
> +#define E4210_DIV_CPU0 0x300
> +#define E4210_DIV_CPU1 0x304
> +#define E4210_DIV_STAT_CPU0 0x400
> +#define E4210_DIV_STAT_CPU1 0x404
> +
> +#define MAX_DIV 8
> +#define DIV_MASK 7
> +#define DIV_MASK_ALL 0xffffffff
> +#define MUX_MASK 7
> +
> +#define E4210_DIV0_RATIO0_MASK 0x7
> +#define E4210_DIV1_HPM_MASK ((0x7 << 4) | (0x7 << 0))
This mask contains two fields, doesn't it? I'd say it would be better
for readability if you split it.
> +#define E4210_MUX_HPM_MASK (1 << 20)
> +#define E4210_DIV0_ATB_SHIFT 16
> +#define E4210_DIV0_ATB_MASK (DIV_MASK << E4210_DIV0_ATB_SHIFT)
> +
> +#define E4210_CPU_DIV0(apll, pclk_dbg, atb, periph, corem1, corem0) \
> + (((apll) << 24) | ((pclk_dbg) << 20) | ((atb) << 16) | \
> + ((periph) << 12) | ((corem1) << 8) | ((corem0) << 4))
> +#define E4210_CPU_DIV1(hpm, copy) \
> + (((hpm) << 4) | ((copy) << 0))
> +
> +#define E5250_CPU_DIV0(apll, pclk_dbg, atb, periph, acp, cpud) \
> + (((apll << 24) | (pclk_dbg << 20) | (atb << 16) | \
> + (periph << 12) | (acp << 8) | (cpud << 4)))
> +#define E5250_CPU_DIV1(hpm, copy) \
> + (((hpm) << 4) | (copy))
> +
> +#define E5420_EGL_DIV0(apll, pclk_dbg, atb, cpud) \
> + (((apll << 24) | (pclk_dbg << 20) | (atb << 16) | \
> + (cpud << 4)))
> +#define E5420_KFC_DIV(kpll, pclk, aclk) \
> + (((kpll << 24) | (pclk << 20) | (aclk << 4)))
Again, used macro arguments should always be surrounded with parentheses.
> +
> +enum cpuclk_type {
> + EXYNOS4210,
> + EXYNOS5250,
> + EXYNOS5420,
> +};
> +
> +/**
> + * struct exynos4210_cpuclk_data: config data to setup cpu clocks.
It seems like this could be used for all Exynos SoCs, so probably should
be called exynos_cpuclk_data.
> + * @prate: frequency of the primary parent clock (in KHz).
> + * @div0: value to be programmed in the div_cpu0 register.
> + * @div1: value to be programmed in the div_cpu1 register.
> + *
> + * This structure holds the divider configuration data for dividers in the CPU
> + * clock domain. The parent frequency at which these divider values are valid is
> + * specified in @prate. The @prate is the frequency of the primary parent clock.
> + * For CPU clock domains that do not have a DIV1 register, the @div1 member
> + * is optional.
> + */
> +struct exynos4210_cpuclk_data {
> + unsigned long prate;
> + unsigned int div0;
> + unsigned int div1;
> +};
> +
> +/**
> + * struct exynos_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.
> + * @ctrl_base: base address of the clock controller.
> + * @offset: offset from the ctrl_base address where the CPU clock div/mux
> + * registers can be accessed.
> + * @lock: cpu clock domain register access lock.
> + * @type: type of the CPU clock.
> + * @data: optional data which the actual instantiation of this clock
> + * can use.
> + * @clk_nb: clock notifier registered for changes in clock speed of the
> + * primary parent clock.
> + * @pre_rate_cb: callback function to handle PRE_RATE_CHANGE notification
> + * of the primary parent clock.
> + * @post_rate_cb: callback function to handle POST_RATE_CHANGE notification
> + * of the primary parent clock.
> + *
> + * This structure holds information required for programming the cpu clock for
> + * various clock speeds.
nit: s/cpu/CPU/
> + */
> +struct exynos_cpuclk {
> + struct clk_hw hw;
> + struct clk *alt_parent;
> + void __iomem *ctrl_base;
> + unsigned long offset;
> + spinlock_t *lock;
> + enum cpuclk_type type;
> + const void *data;
The code always expects this to be const struct exynos4210_cpuclk_data.
Why not make this field so?
Also this is not some plain data, but an array of operating points, so
probably a name like "rates" would be better.
> + struct notifier_block clk_nb;
> + int (*pre_rate_cb)(struct clk_notifier_data *,
> + struct exynos_cpuclk *,
> + void __iomem *base);
> + int (*post_rate_cb)(struct clk_notifier_data *,
> + struct exynos_cpuclk *,
> + void __iomem *base);
All the Exynos SoCs supported by this patch seem to be using exactly the
same notifiers. We don't know what changes in further SoCs, so there is
no guarantee that having these as pointer here will give us any
benefits. I'd recommend just getting rid of this indirection for now. If
it turns out to be needed, it will be trivial to add it back.
> +};
> +
> +#define to_exynos_cpuclk_hw(hw) container_of(hw, struct exynos_cpuclk, hw)
> +#define to_exynos_cpuclk_nb(nb) container_of(nb, struct exynos_cpuclk, clk_nb)
Please make these static inlines for type safety.
> +
> +/**
> + * struct exynos_cpuclk_soc_data: soc specific data for cpu clocks.
> + * @ops: clock operations to be used for this clock.
> + * @offset: optional offset from base of clock controller register base, to
> + * be used when accessing clock controller registers related to the
> + * CPU clock.
> + * @data: SoC specific data for cpuclk configuration (optional).
How is this optional? Can this code work without a list of operating points?
> + * @data_size: size of the data contained in @data member.
Both fields could be probably named "rates" and "num_rates", with the
meaning of the latter changed to mean the number of entries, not size in
bytes.
> + * @type: type of the CPU clock.
> + * @pre_rate_cb: callback function to handle PRE_RATE_CHANGE notification
> + * of the primary parent clock.
> + * @post_rate_cb: callback function to handle POST_RATE_CHANGE notification
> + * of the primary parent clock.
> + *
> + * This structure provides SoC specific data for CPU clocks. Based on
> + * the compatible value of the clock controller node, the value of the
> + * fields in this structure can be populated.
> + */
> +struct exynos_cpuclk_soc_data {
> + const struct clk_ops *ops;
> + unsigned int offset;
> + const void *data;
Same comments as for the data field above.
> + const unsigned int data_size;
If the same struct is always used it would be clearer to
> + enum cpuclk_type type;
> + int (*pre_rate_cb)(struct clk_notifier_data *,
> + struct exynos_cpuclk *,
> + void __iomem *base);
> + int (*post_rate_cb)(struct clk_notifier_data *,
> + struct exynos_cpuclk *,
> + void __iomem *base);
Same comment as above.
> +};
It looks like instead of duplicating most of the fields of this struct
in exynos_cpuclk struct the latter could simply have a pointer to an
instance of the former.
> +
> +/*
> + * Helper function to wait until divider(s) have stabilized after the divider
> + * value has changed.
> + */
How about a kernel doc like comments for functions as well? (same
comment for remaining functions)
> +static void wait_until_divider_stable(void __iomem *div_reg, unsigned long mask)
> +{
> + unsigned long timeout = jiffies + msecs_to_jiffies(10);
> +
> + do {
> + if (!(readl(div_reg) & mask))
> + return;
> + } while (time_before(jiffies, timeout));
> +
> + pr_err("%s: timeout in divider stablization\n", __func__);
> +}
[snip]
> +/* common recalc rate callback useable for all types of CPU clocks */
> +static unsigned long exynos_cpuclk_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
This function is so trivial that it might be reasonable to explain why
nothing else is needed, e.g.
/*
* The CPU clock output (armclk) rate is the same as its parent
* rate. Although there exist certain dividers inside the CPU
* clock block that could be used to divide the parent clock,
* the driver does not make use of them currently, except during
* frequency transitions.
*/
> + return parent_rate;
> +}
> +
> +static const struct clk_ops exynos_cpuclk_clk_ops = {
> + .recalc_rate = exynos_cpuclk_recalc_rate,
> + .round_rate = exynos_cpuclk_round_rate,
> +};
> +
> +/*
> + * Calculates the divider value to be set for deriving drate from prate.
> + * Divider value is actual divider value - 1.
> + */
> +static unsigned long _calc_div(unsigned long prate, unsigned long drate)
> +{
> + unsigned long div = DIV_ROUND_UP(prate, drate) - 1;
> +
> + WARN_ON(div >= MAX_DIV);
> + return div;
> +}
This function seems to be used just once. Not even saying about its
strange semantics - the name would suggest a real divider being
returned, but it's not, it's divider minus one.
So I'd suggest to completely remove this function and simply paste its
contents instead, since it's only used once.
> +
[snip]
> +/* helper function to register a cpu clock */
> +static int __init exynos_cpuclk_register(struct samsung_clk_provider *ctx,
> + unsigned int lookup_id, const char *name, const char *parent,
> + const char *alt_parent, struct device_node *np,
> + const struct exynos_cpuclk_soc_data *soc_data)
> +{
> + struct exynos_cpuclk *cpuclk;
> + struct clk_init_data init;
> + struct clk *clk;
> + void *data;
> + int ret = 0;
> +
> + cpuclk = kzalloc(sizeof(*cpuclk), GFP_KERNEL);
> + if (!cpuclk)
> + return -ENOMEM;
> +
> + data = kmalloc(soc_data->data_size, GFP_KERNEL);
You could simply use kmemdup() here to duplicate the source array. Also
the return value could be already saved in cpuclk->data without the need
for a local variable.
> + if (!data) {
> + ret = -ENOMEM;
> + goto free_cpuclk;
> + }
> +
> + init.name = name;
> + init.flags = CLK_SET_RATE_PARENT;
> + init.parent_names = &parent;
> + init.num_parents = 1;
> + init.ops = soc_data->ops;
> +
> + cpuclk->hw.init = &init;
> + cpuclk->ctrl_base = ctx->reg_base;
> + cpuclk->lock = &ctx->lock;
> + cpuclk->offset = soc_data->offset;
> + cpuclk->type = soc_data->type;
> + cpuclk->pre_rate_cb = soc_data->pre_rate_cb;
> + cpuclk->post_rate_cb = soc_data->post_rate_cb;
> + memcpy(data, soc_data->data, soc_data->data_size);
> + cpuclk->data = data;
> +
> + cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb;
> + ret = clk_notifier_register(__clk_lookup(parent), &cpuclk->clk_nb);
It would be a good idea to check if __clk_lookup() succeeded and error
out otherwise.
> + if (ret) {
> + pr_err("%s: failed to register clock notifier for %s\n",
> + __func__, name);
> + goto free_cpuclk_data;
> + }
> +
[snip]
> +/*
> + * Helper function to set the 'safe' dividers for the CPU clock. The parameters
> + * div and mask contain the divider value and the register bit mask of the
> + * dividers to be programmed.
> + */
> +static void exynos4210_set_safe_div(void __iomem *base, unsigned long div,
> + unsigned long mask)
> +{
> + unsigned long div0;
> +
> + div0 = readl(base + E4210_DIV_CPU0);
> + div0 = (div0 & ~mask) | div;
There is nothing said in the comment above about the assumption that div
has bits not indicated by mask cleared, so to be safe it might be a good
idea to make this
div0 = (div0 & ~mask) | (div & mask);
instead.
> + writel(div0, base + E4210_DIV_CPU0);
> + wait_until_divider_stable(base + E4210_DIV_STAT_CPU0, mask);
> +}
> +
> +/* handler for pre-rate change notification from parent clock */
> +static int exynos4210_cpuclk_pre_rate_change(struct clk_notifier_data *ndata,
> + struct exynos_cpuclk *cpuclk, void __iomem *base)
> +{
> + const struct exynos4210_cpuclk_data *cpuclk_data = cpuclk->data;
> + unsigned long alt_prate = clk_get_rate(cpuclk->alt_parent);
> + unsigned long alt_div = 0, alt_div_mask = DIV_MASK;
> + unsigned long div0, div1 = 0, mux_reg;
> +
> + /* find out the divider values to use for clock data */
> + while ((cpuclk_data->prate * 1000) != ndata->new_rate) {
> + if (cpuclk_data->prate == 0)
> + return -EINVAL;
> + cpuclk_data++;
> + }
> +
> + /* For the selected PLL clock frequency, get the pre-defined divider
> + * values. If the clock for sclk_hpm is not sourced from apll, then
> + * the values for DIV_COPY and DIV_HPM dividers need not be set.
> + */
> + div0 = cpuclk_data->div0;
> + if (cpuclk->type != EXYNOS5420) {
Rather than checking for Exynos5420 explicitly, it would be better to
add a boolean "has_mux_hpm" flag to cpuclk.
> + div1 = cpuclk_data->div1;
> + if (readl(base + E4210_SRC_CPU) & E4210_MUX_HPM_MASK) {
> + div1 = readl(base + E4210_DIV_CPU1) &
> + E4210_DIV1_HPM_MASK;
> + div1 |= ((cpuclk_data->div1) & ~E4210_DIV1_HPM_MASK);
> + }
> + }
> +
> + spin_lock(cpuclk->lock);
> +
> + /*
> + * If the new and 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_prate until the dividers are
> + * set.
> + */
> + if (alt_prate > ndata->old_rate) {
> + alt_div = _calc_div(alt_prate, ndata->old_rate);
> + if (cpuclk->type == EXYNOS4210) {
Again, this could be handled by a boolean "needs_atb_alt_div" flag,
instead of checking for Exynos4210 explicitly.
> + /*
> + * In Exynos4210, ATB clock parent is also mout_core. So
> + * ATB clock also needs to be mantained at safe speed.
> + */
> + alt_div |= E4210_DIV0_ATB_MASK;
> + alt_div_mask |= E4210_DIV0_ATB_MASK;
> + }
> + exynos4210_set_safe_div(base, alt_div, alt_div_mask);
> + div0 |= alt_div;
> + }
> +
> + /* select sclk_mpll as the alternate parent */
> + mux_reg = readl(base + E4210_SRC_CPU);
> + writel(mux_reg | (1 << 16), base + E4210_SRC_CPU);
> + wait_until_mux_stable(base + E4210_STAT_CPU, 16, 2);
> +
> + /* alternate parent is active now. set the dividers */
> + writel(div0, base + E4210_DIV_CPU0);
> + wait_until_divider_stable(base + E4210_DIV_STAT_CPU0, DIV_MASK_ALL);
> +
> + if (cpuclk->type != EXYNOS5420) {
This could be handled by "has_div_cpu1" boolean flag.
> + writel(div1, base + E4210_DIV_CPU1);
> + wait_until_divider_stable(base + E4210_DIV_STAT_CPU1,
> + DIV_MASK_ALL);
> + }
> +
> + spin_unlock(cpuclk->lock);
> + return 0;
> +}
> +
> +/* handler for post-rate change notification from parent clock */
> +static int exynos4210_cpuclk_post_rate_change(struct clk_notifier_data *ndata,
> + struct exynos_cpuclk *cpuclk, void __iomem *base)
> +{
> + const struct exynos4210_cpuclk_data *cpuclk_data = cpuclk->data;
> + unsigned long div = 0, div_mask = DIV_MASK;
> + unsigned long mux_reg;
> +
> + spin_lock(cpuclk->lock);
> +
> + /* select mout_apll as the alternate parent */
> + mux_reg = readl(base + E4210_SRC_CPU);
> + writel(mux_reg & ~(1 << 16), base + E4210_SRC_CPU);
> + wait_until_mux_stable(base + E4210_STAT_CPU, 16, 1);
> +
> + if (cpuclk->type == EXYNOS4210) {
Here the "needs_atb_alt_div" flag could be used again.
> + /* find out the divider values to use for clock data */
> + while ((cpuclk_data->prate * 1000) != ndata->new_rate) {
> + if (cpuclk_data->prate == 0)
> + return -EINVAL;
> + cpuclk_data++;
> + }
> +
> + div |= (cpuclk_data->div0 & E4210_DIV0_ATB_MASK);
> + div_mask |= E4210_DIV0_ATB_MASK;
> + }
> +
> + exynos4210_set_safe_div(base, div, div_mask);
> + spin_unlock(cpuclk->lock);
> + return 0;
> +}
> +
> +static const struct exynos4210_cpuclk_data e4210_armclk_d[] __initconst = {
> + { 1200000, E4210_CPU_DIV0(7, 1, 4, 3, 7, 3), E4210_CPU_DIV1(0, 5), },
> + { 1000000, E4210_CPU_DIV0(7, 1, 4, 3, 7, 3), E4210_CPU_DIV1(0, 4), },
> + { 800000, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), },
> + { 500000, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), },
> + { 400000, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), },
> + { 200000, E4210_CPU_DIV0(0, 1, 1, 1, 3, 1), E4210_CPU_DIV1(0, 3), },
> + { 0 },
> +};
[snip]
> +static const struct exynos_cpuclk_soc_data e4210_clk_soc_data __initconst = {
> + .ops = &exynos_cpuclk_clk_ops,
> + .offset = 0x14200,
> + .data = e4210_armclk_d,
> + .data_size = sizeof(e4210_armclk_d),
> + .type = EXYNOS4210,
> + .pre_rate_cb = exynos4210_cpuclk_pre_rate_change,
> + .post_rate_cb = exynos4210_cpuclk_post_rate_change,
> +};
[snip]
> +/**
> + * exynos_register_cpu_clock: register cpu clock with ccf.
> + * @ctx: driver context.
> + * @cluster_id: cpu cluster number to which this clock is connected.
> + * @lookup_id: cpuclk clock output id for the clock controller.
> + * @name: the name of the cpu clock.
> + * @parent: name of the parent clock for cpuclk.
> + * @alt_parent: name of the alternate clock parent.
> + * @np: device tree node pointer of the clock controller.
> + */
> +int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
> + unsigned int cluster_id, unsigned int lookup_id,
> + const char *name, const char *parent,
> + const char *alt_parent, struct device_node *np)
> +{
> + const struct of_device_id *match;
> + const struct exynos_cpuclk_soc_data *data = NULL;
> +
> + if (!np)
> + return -EINVAL;
> +
> + match = of_match_node(exynos_cpuclk_ids, np);
> + if (!match)
> + return -EINVAL;
> +
> + data = match->data;
> + data += cluster_id;
> + return exynos_cpuclk_register(ctx, lookup_id, name, parent,
> + alt_parent, np, data);
> +}
We now have SoC-specific data hardcoded here, so (as opposed to my
earlier comments when we did not have such) it's now reasonable to move
such data to SoC-specific source files and then just call
exynos_register_cpu_clock() with a pointer to such data. This would also
eliminate the not so good idea of indexing internal data array with
cluster_id passed as an argument from external code.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v7 1/6] clk: samsung: add infrastructure to register cpu clocks
2014-07-19 12:55 ` Tomasz Figa
@ 2014-07-29 5:35 ` Thomas Abraham
2014-07-29 9:57 ` Tomasz Figa
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Abraham @ 2014-07-29 5:35 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
Thanks for your review comments. I have made most of the changes you
have suggested. The suggested modifications which I did not include is
marked below.
On Sat, Jul 19, 2014 at 6:25 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>
>
> Hi Thomas,
>
> Please see my comments inline.
>
> On 14.07.2014 15:38, Thomas Abraham wrote:
>
> [snip]
>
>> diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c
>> new file mode 100644
>> index 0000000..0d62968
>> --- /dev/null
>> +++ b/drivers/clk/samsung/clk-cpu.c
>> @@ -0,0 +1,576 @@
>> +/*
>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>> + * Author: Thomas Abraham <thomas.ab@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 functions to register the CPU clocks
>> + * for Samsung platforms.
>
> I'd expect few words here or in separate comment on how the code works,
> i.e. assumptions made, most important procedures, etc. This is a complex
> piece of code for quite complex hardware, so proper documentation is
> essential.
>
>> +*/
>> +
>> +#include <linux/errno.h>
>> +#include "clk.h"
>> +
>> +#define E4210_SRC_CPU 0x0
>> +#define E4210_STAT_CPU 0x200
>> +#define E4210_DIV_CPU0 0x300
>> +#define E4210_DIV_CPU1 0x304
>> +#define E4210_DIV_STAT_CPU0 0x400
>> +#define E4210_DIV_STAT_CPU1 0x404
>> +
>> +#define MAX_DIV 8
>> +#define DIV_MASK 7
>> +#define DIV_MASK_ALL 0xffffffff
>> +#define MUX_MASK 7
>> +
>> +#define E4210_DIV0_RATIO0_MASK 0x7
>> +#define E4210_DIV1_HPM_MASK ((0x7 << 4) | (0x7 << 0))
>
> This mask contains two fields, doesn't it? I'd say it would be better
> for readability if you split it.
>
>> +#define E4210_MUX_HPM_MASK (1 << 20)
>> +#define E4210_DIV0_ATB_SHIFT 16
>> +#define E4210_DIV0_ATB_MASK (DIV_MASK << E4210_DIV0_ATB_SHIFT)
>> +
>> +#define E4210_CPU_DIV0(apll, pclk_dbg, atb, periph, corem1, corem0) \
>> + (((apll) << 24) | ((pclk_dbg) << 20) | ((atb) << 16) | \
>> + ((periph) << 12) | ((corem1) << 8) | ((corem0) << 4))
>> +#define E4210_CPU_DIV1(hpm, copy) \
>> + (((hpm) << 4) | ((copy) << 0))
>> +
>> +#define E5250_CPU_DIV0(apll, pclk_dbg, atb, periph, acp, cpud) \
>> + (((apll << 24) | (pclk_dbg << 20) | (atb << 16) | \
>> + (periph << 12) | (acp << 8) | (cpud << 4)))
>> +#define E5250_CPU_DIV1(hpm, copy) \
>> + (((hpm) << 4) | (copy))
>> +
>> +#define E5420_EGL_DIV0(apll, pclk_dbg, atb, cpud) \
>> + (((apll << 24) | (pclk_dbg << 20) | (atb << 16) | \
>> + (cpud << 4)))
>> +#define E5420_KFC_DIV(kpll, pclk, aclk) \
>> + (((kpll << 24) | (pclk << 20) | (aclk << 4)))
>
> Again, used macro arguments should always be surrounded with parentheses.
>
>> +
>> +enum cpuclk_type {
>> + EXYNOS4210,
>> + EXYNOS5250,
>> + EXYNOS5420,
>> +};
>> +
>> +/**
>> + * struct exynos4210_cpuclk_data: config data to setup cpu clocks.
>
> It seems like this could be used for all Exynos SoCs, so probably should
> be called exynos_cpuclk_data.
>
>> + * @prate: frequency of the primary parent clock (in KHz).
>> + * @div0: value to be programmed in the div_cpu0 register.
>> + * @div1: value to be programmed in the div_cpu1 register.
>> + *
>> + * This structure holds the divider configuration data for dividers in the CPU
>> + * clock domain. The parent frequency at which these divider values are valid is
>> + * specified in @prate. The @prate is the frequency of the primary parent clock.
>> + * For CPU clock domains that do not have a DIV1 register, the @div1 member
>> + * is optional.
>> + */
>> +struct exynos4210_cpuclk_data {
>> + unsigned long prate;
>> + unsigned int div0;
>> + unsigned int div1;
>> +};
>> +
>> +/**
>> + * struct exynos_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.
>> + * @ctrl_base: base address of the clock controller.
>> + * @offset: offset from the ctrl_base address where the CPU clock div/mux
>> + * registers can be accessed.
>> + * @lock: cpu clock domain register access lock.
>> + * @type: type of the CPU clock.
>> + * @data: optional data which the actual instantiation of this clock
>> + * can use.
>> + * @clk_nb: clock notifier registered for changes in clock speed of the
>> + * primary parent clock.
>> + * @pre_rate_cb: callback function to handle PRE_RATE_CHANGE notification
>> + * of the primary parent clock.
>> + * @post_rate_cb: callback function to handle POST_RATE_CHANGE notification
>> + * of the primary parent clock.
>> + *
>> + * This structure holds information required for programming the cpu clock for
>> + * various clock speeds.
>
> nit: s/cpu/CPU/
>
>> + */
>> +struct exynos_cpuclk {
>> + struct clk_hw hw;
>> + struct clk *alt_parent;
>> + void __iomem *ctrl_base;
>> + unsigned long offset;
>> + spinlock_t *lock;
>> + enum cpuclk_type type;
>> + const void *data;
>
> The code always expects this to be const struct exynos4210_cpuclk_data.
> Why not make this field so?
>
> Also this is not some plain data, but an array of operating points, so
> probably a name like "rates" would be better.
>
>> + struct notifier_block clk_nb;
>> + int (*pre_rate_cb)(struct clk_notifier_data *,
>> + struct exynos_cpuclk *,
>> + void __iomem *base);
>> + int (*post_rate_cb)(struct clk_notifier_data *,
>> + struct exynos_cpuclk *,
>> + void __iomem *base);
>
> All the Exynos SoCs supported by this patch seem to be using exactly the
> same notifiers. We don't know what changes in further SoCs, so there is
> no guarantee that having these as pointer here will give us any
> benefits. I'd recommend just getting rid of this indirection for now. If
> it turns out to be needed, it will be trivial to add it back.
>
>> +};
>> +
>> +#define to_exynos_cpuclk_hw(hw) container_of(hw, struct exynos_cpuclk, hw)
>> +#define to_exynos_cpuclk_nb(nb) container_of(nb, struct exynos_cpuclk, clk_nb)
>
> Please make these static inlines for type safety.
>
>> +
>> +/**
>> + * struct exynos_cpuclk_soc_data: soc specific data for cpu clocks.
>> + * @ops: clock operations to be used for this clock.
>> + * @offset: optional offset from base of clock controller register base, to
>> + * be used when accessing clock controller registers related to the
>> + * CPU clock.
>> + * @data: SoC specific data for cpuclk configuration (optional).
>
> How is this optional? Can this code work without a list of operating points?
>
>> + * @data_size: size of the data contained in @data member.
>
> Both fields could be probably named "rates" and "num_rates", with the
> meaning of the latter changed to mean the number of entries, not size in
> bytes.
>
>> + * @type: type of the CPU clock.
>> + * @pre_rate_cb: callback function to handle PRE_RATE_CHANGE notification
>> + * of the primary parent clock.
>> + * @post_rate_cb: callback function to handle POST_RATE_CHANGE notification
>> + * of the primary parent clock.
>> + *
>> + * This structure provides SoC specific data for CPU clocks. Based on
>> + * the compatible value of the clock controller node, the value of the
>> + * fields in this structure can be populated.
>> + */
>> +struct exynos_cpuclk_soc_data {
>> + const struct clk_ops *ops;
>> + unsigned int offset;
>> + const void *data;
>
> Same comments as for the data field above.
>
>> + const unsigned int data_size;
>
> If the same struct is always used it would be clearer to
>
>> + enum cpuclk_type type;
>> + int (*pre_rate_cb)(struct clk_notifier_data *,
>> + struct exynos_cpuclk *,
>> + void __iomem *base);
>> + int (*post_rate_cb)(struct clk_notifier_data *,
>> + struct exynos_cpuclk *,
>> + void __iomem *base);
>
> Same comment as above.
>
>> +};
>
> It looks like instead of duplicating most of the fields of this struct
> in exynos_cpuclk struct the latter could simply have a pointer to an
> instance of the former.
>
>> +
>> +/*
>> + * Helper function to wait until divider(s) have stabilized after the divider
>> + * value has changed.
>> + */
>
> How about a kernel doc like comments for functions as well? (same
> comment for remaining functions)
Since most of these functions are static and very simple, I have not
added kernel-doc like comments.
>
>> +static void wait_until_divider_stable(void __iomem *div_reg, unsigned long mask)
>> +{
>> + unsigned long timeout = jiffies + msecs_to_jiffies(10);
>> +
>> + do {
>> + if (!(readl(div_reg) & mask))
>> + return;
>> + } while (time_before(jiffies, timeout));
>> +
>> + pr_err("%s: timeout in divider stablization\n", __func__);
>> +}
>
> [snip]
>
>> +/* common recalc rate callback useable for all types of CPU clocks */
>> +static unsigned long exynos_cpuclk_recalc_rate(struct clk_hw *hw,
>> + unsigned long parent_rate)
>> +{
>
> This function is so trivial that it might be reasonable to explain why
> nothing else is needed, e.g.
>
> /*
> * The CPU clock output (armclk) rate is the same as its parent
> * rate. Although there exist certain dividers inside the CPU
> * clock block that could be used to divide the parent clock,
> * the driver does not make use of them currently, except during
> * frequency transitions.
> */
>
>> + return parent_rate;
>> +}
>> +
>> +static const struct clk_ops exynos_cpuclk_clk_ops = {
>> + .recalc_rate = exynos_cpuclk_recalc_rate,
>> + .round_rate = exynos_cpuclk_round_rate,
>> +};
>> +
>> +/*
>> + * Calculates the divider value to be set for deriving drate from prate.
>> + * Divider value is actual divider value - 1.
>> + */
>> +static unsigned long _calc_div(unsigned long prate, unsigned long drate)
>> +{
>> + unsigned long div = DIV_ROUND_UP(prate, drate) - 1;
>> +
>> + WARN_ON(div >= MAX_DIV);
>> + return div;
>> +}
>
> This function seems to be used just once. Not even saying about its
> strange semantics - the name would suggest a real divider being
> returned, but it's not, it's divider minus one.
>
> So I'd suggest to completely remove this function and simply paste its
> contents instead, since it's only used once.
>
>> +
>
> [snip]
>
>> +/* helper function to register a cpu clock */
>> +static int __init exynos_cpuclk_register(struct samsung_clk_provider *ctx,
>> + unsigned int lookup_id, const char *name, const char *parent,
>> + const char *alt_parent, struct device_node *np,
>> + const struct exynos_cpuclk_soc_data *soc_data)
>> +{
>> + struct exynos_cpuclk *cpuclk;
>> + struct clk_init_data init;
>> + struct clk *clk;
>> + void *data;
>> + int ret = 0;
>> +
>> + cpuclk = kzalloc(sizeof(*cpuclk), GFP_KERNEL);
>> + if (!cpuclk)
>> + return -ENOMEM;
>> +
>> + data = kmalloc(soc_data->data_size, GFP_KERNEL);
>
> You could simply use kmemdup() here to duplicate the source array. Also
> the return value could be already saved in cpuclk->data without the need
> for a local variable.
>
>> + if (!data) {
>> + ret = -ENOMEM;
>> + goto free_cpuclk;
>> + }
>> +
>> + init.name = name;
>> + init.flags = CLK_SET_RATE_PARENT;
>> + init.parent_names = &parent;
>> + init.num_parents = 1;
>> + init.ops = soc_data->ops;
>> +
>> + cpuclk->hw.init = &init;
>> + cpuclk->ctrl_base = ctx->reg_base;
>> + cpuclk->lock = &ctx->lock;
>> + cpuclk->offset = soc_data->offset;
>> + cpuclk->type = soc_data->type;
>> + cpuclk->pre_rate_cb = soc_data->pre_rate_cb;
>> + cpuclk->post_rate_cb = soc_data->post_rate_cb;
>> + memcpy(data, soc_data->data, soc_data->data_size);
>> + cpuclk->data = data;
>> +
>> + cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb;
>> + ret = clk_notifier_register(__clk_lookup(parent), &cpuclk->clk_nb);
>
> It would be a good idea to check if __clk_lookup() succeeded and error
> out otherwise.
>
>> + if (ret) {
>> + pr_err("%s: failed to register clock notifier for %s\n",
>> + __func__, name);
>> + goto free_cpuclk_data;
>> + }
>> +
>
> [snip]
>
>> +/*
>> + * Helper function to set the 'safe' dividers for the CPU clock. The parameters
>> + * div and mask contain the divider value and the register bit mask of the
>> + * dividers to be programmed.
>> + */
>> +static void exynos4210_set_safe_div(void __iomem *base, unsigned long div,
>> + unsigned long mask)
>> +{
>> + unsigned long div0;
>> +
>> + div0 = readl(base + E4210_DIV_CPU0);
>> + div0 = (div0 & ~mask) | div;
>
> There is nothing said in the comment above about the assumption that div
> has bits not indicated by mask cleared, so to be safe it might be a good
> idea to make this
>
> div0 = (div0 & ~mask) | (div & mask);
>
> instead.
>
>> + writel(div0, base + E4210_DIV_CPU0);
>> + wait_until_divider_stable(base + E4210_DIV_STAT_CPU0, mask);
>> +}
>> +
>> +/* handler for pre-rate change notification from parent clock */
>> +static int exynos4210_cpuclk_pre_rate_change(struct clk_notifier_data *ndata,
>> + struct exynos_cpuclk *cpuclk, void __iomem *base)
>> +{
>> + const struct exynos4210_cpuclk_data *cpuclk_data = cpuclk->data;
>> + unsigned long alt_prate = clk_get_rate(cpuclk->alt_parent);
>> + unsigned long alt_div = 0, alt_div_mask = DIV_MASK;
>> + unsigned long div0, div1 = 0, mux_reg;
>> +
>> + /* find out the divider values to use for clock data */
>> + while ((cpuclk_data->prate * 1000) != ndata->new_rate) {
>> + if (cpuclk_data->prate == 0)
>> + return -EINVAL;
>> + cpuclk_data++;
>> + }
>> +
>> + /* For the selected PLL clock frequency, get the pre-defined divider
>> + * values. If the clock for sclk_hpm is not sourced from apll, then
>> + * the values for DIV_COPY and DIV_HPM dividers need not be set.
>> + */
>> + div0 = cpuclk_data->div0;
>> + if (cpuclk->type != EXYNOS5420) {
>
> Rather than checking for Exynos5420 explicitly, it would be better to
> add a boolean "has_mux_hpm" flag to cpuclk.
>
>> + div1 = cpuclk_data->div1;
>> + if (readl(base + E4210_SRC_CPU) & E4210_MUX_HPM_MASK) {
>> + div1 = readl(base + E4210_DIV_CPU1) &
>> + E4210_DIV1_HPM_MASK;
>> + div1 |= ((cpuclk_data->div1) & ~E4210_DIV1_HPM_MASK);
>> + }
>> + }
>> +
>> + spin_lock(cpuclk->lock);
>> +
>> + /*
>> + * If the new and 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_prate until the dividers are
>> + * set.
>> + */
>> + if (alt_prate > ndata->old_rate) {
>> + alt_div = _calc_div(alt_prate, ndata->old_rate);
>> + if (cpuclk->type == EXYNOS4210) {
>
> Again, this could be handled by a boolean "needs_atb_alt_div" flag,
> instead of checking for Exynos4210 explicitly.
>
>> + /*
>> + * In Exynos4210, ATB clock parent is also mout_core. So
>> + * ATB clock also needs to be mantained at safe speed.
>> + */
>> + alt_div |= E4210_DIV0_ATB_MASK;
>> + alt_div_mask |= E4210_DIV0_ATB_MASK;
>> + }
>> + exynos4210_set_safe_div(base, alt_div, alt_div_mask);
>> + div0 |= alt_div;
>> + }
>> +
>> + /* select sclk_mpll as the alternate parent */
>> + mux_reg = readl(base + E4210_SRC_CPU);
>> + writel(mux_reg | (1 << 16), base + E4210_SRC_CPU);
>> + wait_until_mux_stable(base + E4210_STAT_CPU, 16, 2);
>> +
>> + /* alternate parent is active now. set the dividers */
>> + writel(div0, base + E4210_DIV_CPU0);
>> + wait_until_divider_stable(base + E4210_DIV_STAT_CPU0, DIV_MASK_ALL);
>> +
>> + if (cpuclk->type != EXYNOS5420) {
>
> This could be handled by "has_div_cpu1" boolean flag.
>
>> + writel(div1, base + E4210_DIV_CPU1);
>> + wait_until_divider_stable(base + E4210_DIV_STAT_CPU1,
>> + DIV_MASK_ALL);
>> + }
>> +
>> + spin_unlock(cpuclk->lock);
>> + return 0;
>> +}
>> +
>> +/* handler for post-rate change notification from parent clock */
>> +static int exynos4210_cpuclk_post_rate_change(struct clk_notifier_data *ndata,
>> + struct exynos_cpuclk *cpuclk, void __iomem *base)
>> +{
>> + const struct exynos4210_cpuclk_data *cpuclk_data = cpuclk->data;
>> + unsigned long div = 0, div_mask = DIV_MASK;
>> + unsigned long mux_reg;
>> +
>> + spin_lock(cpuclk->lock);
>> +
>> + /* select mout_apll as the alternate parent */
>> + mux_reg = readl(base + E4210_SRC_CPU);
>> + writel(mux_reg & ~(1 << 16), base + E4210_SRC_CPU);
>> + wait_until_mux_stable(base + E4210_STAT_CPU, 16, 1);
>> +
>> + if (cpuclk->type == EXYNOS4210) {
>
> Here the "needs_atb_alt_div" flag could be used again.
>
>> + /* find out the divider values to use for clock data */
>> + while ((cpuclk_data->prate * 1000) != ndata->new_rate) {
>> + if (cpuclk_data->prate == 0)
>> + return -EINVAL;
>> + cpuclk_data++;
>> + }
>> +
>> + div |= (cpuclk_data->div0 & E4210_DIV0_ATB_MASK);
>> + div_mask |= E4210_DIV0_ATB_MASK;
>> + }
>> +
>> + exynos4210_set_safe_div(base, div, div_mask);
>> + spin_unlock(cpuclk->lock);
>> + return 0;
>> +}
>> +
>> +static const struct exynos4210_cpuclk_data e4210_armclk_d[] __initconst = {
>> + { 1200000, E4210_CPU_DIV0(7, 1, 4, 3, 7, 3), E4210_CPU_DIV1(0, 5), },
>> + { 1000000, E4210_CPU_DIV0(7, 1, 4, 3, 7, 3), E4210_CPU_DIV1(0, 4), },
>> + { 800000, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), },
>> + { 500000, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), },
>> + { 400000, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), },
>> + { 200000, E4210_CPU_DIV0(0, 1, 1, 1, 3, 1), E4210_CPU_DIV1(0, 3), },
>> + { 0 },
>> +};
>
> [snip]
>
>> +static const struct exynos_cpuclk_soc_data e4210_clk_soc_data __initconst = {
>> + .ops = &exynos_cpuclk_clk_ops,
>> + .offset = 0x14200,
>> + .data = e4210_armclk_d,
>> + .data_size = sizeof(e4210_armclk_d),
>> + .type = EXYNOS4210,
>> + .pre_rate_cb = exynos4210_cpuclk_pre_rate_change,
>> + .post_rate_cb = exynos4210_cpuclk_post_rate_change,
>> +};
>
> [snip]
>
>> +/**
>> + * exynos_register_cpu_clock: register cpu clock with ccf.
>> + * @ctx: driver context.
>> + * @cluster_id: cpu cluster number to which this clock is connected.
>> + * @lookup_id: cpuclk clock output id for the clock controller.
>> + * @name: the name of the cpu clock.
>> + * @parent: name of the parent clock for cpuclk.
>> + * @alt_parent: name of the alternate clock parent.
>> + * @np: device tree node pointer of the clock controller.
>> + */
>> +int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
>> + unsigned int cluster_id, unsigned int lookup_id,
>> + const char *name, const char *parent,
>> + const char *alt_parent, struct device_node *np)
>> +{
>> + const struct of_device_id *match;
>> + const struct exynos_cpuclk_soc_data *data = NULL;
>> +
>> + if (!np)
>> + return -EINVAL;
>> +
>> + match = of_match_node(exynos_cpuclk_ids, np);
>> + if (!match)
>> + return -EINVAL;
>> +
>> + data = match->data;
>> + data += cluster_id;
>> + return exynos_cpuclk_register(ctx, lookup_id, name, parent,
>> + alt_parent, np, data);
>> +}
>
> We now have SoC-specific data hardcoded here, so (as opposed to my
> earlier comments when we did not have such) it's now reasonable to move
> such data to SoC-specific source files and then just call
> exynos_register_cpu_clock() with a pointer to such data. This would also
> eliminate the not so good idea of indexing internal data array with
> cluster_id passed as an argument from external code.
>
> Best regards,
> Tomasz
Thanks,
Thomas.
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v7 1/6] clk: samsung: add infrastructure to register cpu clocks
2014-07-29 5:35 ` Thomas Abraham
@ 2014-07-29 9:57 ` Tomasz Figa
0 siblings, 0 replies; 25+ messages in thread
From: Tomasz Figa @ 2014-07-29 9:57 UTC (permalink / raw)
To: linux-arm-kernel
On 29.07.2014 07:35, Thomas Abraham wrote:
> Hi Tomasz,
>
> Thanks for your review comments. I have made most of the changes you
> have suggested. The suggested modifications which I did not include is
> marked below.
>
> On Sat, Jul 19, 2014 at 6:25 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>>
>>
>> Hi Thomas,
>>
>> Please see my comments inline.
>>
>> On 14.07.2014 15:38, Thomas Abraham wrote:
>>
>> [snip]
>>
>>> diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c
>>> new file mode 100644
>>> index 0000000..0d62968
>>> --- /dev/null
>>> +++ b/drivers/clk/samsung/clk-cpu.c
>>> @@ -0,0 +1,576 @@
>>> +/*
>>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>>> + * Author: Thomas Abraham <thomas.ab@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 functions to register the CPU clocks
>>> + * for Samsung platforms.
>>
>> I'd expect few words here or in separate comment on how the code works,
>> i.e. assumptions made, most important procedures, etc. This is a complex
>> piece of code for quite complex hardware, so proper documentation is
>> essential.
>>
>>> +*/
>>> +
>>> +#include <linux/errno.h>
>>> +#include "clk.h"
>>> +
>>> +#define E4210_SRC_CPU 0x0
>>> +#define E4210_STAT_CPU 0x200
>>> +#define E4210_DIV_CPU0 0x300
>>> +#define E4210_DIV_CPU1 0x304
>>> +#define E4210_DIV_STAT_CPU0 0x400
>>> +#define E4210_DIV_STAT_CPU1 0x404
>>> +
>>> +#define MAX_DIV 8
>>> +#define DIV_MASK 7
>>> +#define DIV_MASK_ALL 0xffffffff
>>> +#define MUX_MASK 7
>>> +
>>> +#define E4210_DIV0_RATIO0_MASK 0x7
>>> +#define E4210_DIV1_HPM_MASK ((0x7 << 4) | (0x7 << 0))
>>
>> This mask contains two fields, doesn't it? I'd say it would be better
>> for readability if you split it.
>>
>>> +#define E4210_MUX_HPM_MASK (1 << 20)
>>> +#define E4210_DIV0_ATB_SHIFT 16
>>> +#define E4210_DIV0_ATB_MASK (DIV_MASK << E4210_DIV0_ATB_SHIFT)
>>> +
>>> +#define E4210_CPU_DIV0(apll, pclk_dbg, atb, periph, corem1, corem0) \
>>> + (((apll) << 24) | ((pclk_dbg) << 20) | ((atb) << 16) | \
>>> + ((periph) << 12) | ((corem1) << 8) | ((corem0) << 4))
>>> +#define E4210_CPU_DIV1(hpm, copy) \
>>> + (((hpm) << 4) | ((copy) << 0))
>>> +
>>> +#define E5250_CPU_DIV0(apll, pclk_dbg, atb, periph, acp, cpud) \
>>> + (((apll << 24) | (pclk_dbg << 20) | (atb << 16) | \
>>> + (periph << 12) | (acp << 8) | (cpud << 4)))
>>> +#define E5250_CPU_DIV1(hpm, copy) \
>>> + (((hpm) << 4) | (copy))
>>> +
>>> +#define E5420_EGL_DIV0(apll, pclk_dbg, atb, cpud) \
>>> + (((apll << 24) | (pclk_dbg << 20) | (atb << 16) | \
>>> + (cpud << 4)))
>>> +#define E5420_KFC_DIV(kpll, pclk, aclk) \
>>> + (((kpll << 24) | (pclk << 20) | (aclk << 4)))
>>
>> Again, used macro arguments should always be surrounded with parentheses.
>>
>>> +
>>> +enum cpuclk_type {
>>> + EXYNOS4210,
>>> + EXYNOS5250,
>>> + EXYNOS5420,
>>> +};
>>> +
>>> +/**
>>> + * struct exynos4210_cpuclk_data: config data to setup cpu clocks.
>>
>> It seems like this could be used for all Exynos SoCs, so probably should
>> be called exynos_cpuclk_data.
>>
>>> + * @prate: frequency of the primary parent clock (in KHz).
>>> + * @div0: value to be programmed in the div_cpu0 register.
>>> + * @div1: value to be programmed in the div_cpu1 register.
>>> + *
>>> + * This structure holds the divider configuration data for dividers in the CPU
>>> + * clock domain. The parent frequency at which these divider values are valid is
>>> + * specified in @prate. The @prate is the frequency of the primary parent clock.
>>> + * For CPU clock domains that do not have a DIV1 register, the @div1 member
>>> + * is optional.
>>> + */
>>> +struct exynos4210_cpuclk_data {
>>> + unsigned long prate;
>>> + unsigned int div0;
>>> + unsigned int div1;
>>> +};
>>> +
>>> +/**
>>> + * struct exynos_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.
>>> + * @ctrl_base: base address of the clock controller.
>>> + * @offset: offset from the ctrl_base address where the CPU clock div/mux
>>> + * registers can be accessed.
>>> + * @lock: cpu clock domain register access lock.
>>> + * @type: type of the CPU clock.
>>> + * @data: optional data which the actual instantiation of this clock
>>> + * can use.
>>> + * @clk_nb: clock notifier registered for changes in clock speed of the
>>> + * primary parent clock.
>>> + * @pre_rate_cb: callback function to handle PRE_RATE_CHANGE notification
>>> + * of the primary parent clock.
>>> + * @post_rate_cb: callback function to handle POST_RATE_CHANGE notification
>>> + * of the primary parent clock.
>>> + *
>>> + * This structure holds information required for programming the cpu clock for
>>> + * various clock speeds.
>>
>> nit: s/cpu/CPU/
>>
>>> + */
>>> +struct exynos_cpuclk {
>>> + struct clk_hw hw;
>>> + struct clk *alt_parent;
>>> + void __iomem *ctrl_base;
>>> + unsigned long offset;
>>> + spinlock_t *lock;
>>> + enum cpuclk_type type;
>>> + const void *data;
>>
>> The code always expects this to be const struct exynos4210_cpuclk_data.
>> Why not make this field so?
>>
>> Also this is not some plain data, but an array of operating points, so
>> probably a name like "rates" would be better.
>>
>>> + struct notifier_block clk_nb;
>>> + int (*pre_rate_cb)(struct clk_notifier_data *,
>>> + struct exynos_cpuclk *,
>>> + void __iomem *base);
>>> + int (*post_rate_cb)(struct clk_notifier_data *,
>>> + struct exynos_cpuclk *,
>>> + void __iomem *base);
>>
>> All the Exynos SoCs supported by this patch seem to be using exactly the
>> same notifiers. We don't know what changes in further SoCs, so there is
>> no guarantee that having these as pointer here will give us any
>> benefits. I'd recommend just getting rid of this indirection for now. If
>> it turns out to be needed, it will be trivial to add it back.
>>
>>> +};
>>> +
>>> +#define to_exynos_cpuclk_hw(hw) container_of(hw, struct exynos_cpuclk, hw)
>>> +#define to_exynos_cpuclk_nb(nb) container_of(nb, struct exynos_cpuclk, clk_nb)
>>
>> Please make these static inlines for type safety.
>>
>>> +
>>> +/**
>>> + * struct exynos_cpuclk_soc_data: soc specific data for cpu clocks.
>>> + * @ops: clock operations to be used for this clock.
>>> + * @offset: optional offset from base of clock controller register base, to
>>> + * be used when accessing clock controller registers related to the
>>> + * CPU clock.
>>> + * @data: SoC specific data for cpuclk configuration (optional).
>>
>> How is this optional? Can this code work without a list of operating points?
>>
>>> + * @data_size: size of the data contained in @data member.
>>
>> Both fields could be probably named "rates" and "num_rates", with the
>> meaning of the latter changed to mean the number of entries, not size in
>> bytes.
>>
>>> + * @type: type of the CPU clock.
>>> + * @pre_rate_cb: callback function to handle PRE_RATE_CHANGE notification
>>> + * of the primary parent clock.
>>> + * @post_rate_cb: callback function to handle POST_RATE_CHANGE notification
>>> + * of the primary parent clock.
>>> + *
>>> + * This structure provides SoC specific data for CPU clocks. Based on
>>> + * the compatible value of the clock controller node, the value of the
>>> + * fields in this structure can be populated.
>>> + */
>>> +struct exynos_cpuclk_soc_data {
>>> + const struct clk_ops *ops;
>>> + unsigned int offset;
>>> + const void *data;
>>
>> Same comments as for the data field above.
>>
>>> + const unsigned int data_size;
>>
>> If the same struct is always used it would be clearer to
>>
>>> + enum cpuclk_type type;
>>> + int (*pre_rate_cb)(struct clk_notifier_data *,
>>> + struct exynos_cpuclk *,
>>> + void __iomem *base);
>>> + int (*post_rate_cb)(struct clk_notifier_data *,
>>> + struct exynos_cpuclk *,
>>> + void __iomem *base);
>>
>> Same comment as above.
>>
>>> +};
>>
>> It looks like instead of duplicating most of the fields of this struct
>> in exynos_cpuclk struct the latter could simply have a pointer to an
>> instance of the former.
>>
>>> +
>>> +/*
>>> + * Helper function to wait until divider(s) have stabilized after the divider
>>> + * value has changed.
>>> + */
>>
>> How about a kernel doc like comments for functions as well? (same
>> comment for remaining functions)
>
> Since most of these functions are static and very simple, I have not
> added kernel-doc like comments.
What I mean is that you already added comments for those functions, so
why they couldn't be in kerneldoc format as in other drivers?
Anyway, this doesn't have any functional meaning, so it might be changed
in further patch, since you already posted next version.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v7 2/6] clk: samsung: register exynos5420 apll/kpll configuration data
2014-07-14 13:38 [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms Thomas Abraham
2014-07-14 13:38 ` [PATCH v7 1/6] clk: samsung: add infrastructure to register cpu clocks Thomas Abraham
@ 2014-07-14 13:38 ` Thomas Abraham
2014-07-19 12:57 ` Tomasz Figa
2014-07-14 13:38 ` [PATCH v7 3/6] clk: exynos: use cpu-clock provider type to represent arm clock Thomas Abraham
` (5 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Thomas Abraham @ 2014-07-14 13:38 UTC (permalink / raw)
To: linux-arm-kernel
From: Thomas Abraham <thomas.ab@samsung.com>
Register the PLL configuration data for APLL and KPLL on Exynos5420. This
configuration data table specifies PLL coefficients for supported PLL
clock speeds when a 24MHz clock is supplied as the input clock source
for these PLLs.
Cc: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
Reviewed-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
Tested-by: Arjun K.V <arjun.kv@samsung.com>
---
drivers/clk/samsung/clk-exynos5420.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index cc14ac8..95f83be 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -1169,6 +1169,28 @@ static struct samsung_gate_clock exynos5x_gate_clks[] __initdata = {
GATE(CLK_G3D, "g3d", "mout_user_aclk_g3d", GATE_IP_G3D, 9, 0, 0),
};
+static const struct samsung_pll_rate_table exynos5420_pll2550x_24mhz_tbl[] = {
+ PLL_35XX_RATE(2000000000, 250, 3, 0),
+ PLL_35XX_RATE(1900000000, 475, 6, 0),
+ PLL_35XX_RATE(1800000000, 225, 3, 0),
+ PLL_35XX_RATE(1700000000, 425, 6, 0),
+ PLL_35XX_RATE(1600000000, 200, 3, 0),
+ PLL_35XX_RATE(1500000000, 250, 4, 0),
+ PLL_35XX_RATE(1400000000, 175, 3, 0),
+ PLL_35XX_RATE(1300000000, 325, 6, 0),
+ PLL_35XX_RATE(1200000000, 200, 2, 1),
+ PLL_35XX_RATE(1100000000, 275, 3, 1),
+ PLL_35XX_RATE(1000000000, 250, 3, 1),
+ PLL_35XX_RATE(900000000, 150, 2, 1),
+ PLL_35XX_RATE(800000000, 200, 3, 1),
+ PLL_35XX_RATE(700000000, 175, 3, 1),
+ PLL_35XX_RATE(600000000, 200, 2, 2),
+ PLL_35XX_RATE(500000000, 250, 3, 2),
+ PLL_35XX_RATE(400000000, 200, 3, 2),
+ PLL_35XX_RATE(300000000, 200, 2, 3),
+ PLL_35XX_RATE(200000000, 200, 3, 3),
+};
+
static struct samsung_pll_clock exynos5x_plls[nr_plls] __initdata = {
[apll] = PLL(pll_2550, CLK_FOUT_APLL, "fout_apll", "fin_pll", APLL_LOCK,
APLL_CON0, NULL),
@@ -1222,6 +1244,12 @@ static void __init exynos5x_clk_init(struct device_node *np,
samsung_clk_of_register_fixed_ext(ctx, exynos5x_fixed_rate_ext_clks,
ARRAY_SIZE(exynos5x_fixed_rate_ext_clks),
ext_clk_match);
+
+ if (_get_rate("fin_pll") == 24 * MHZ) {
+ exynos5x_plls[apll].rate_table = exynos5420_pll2550x_24mhz_tbl;
+ exynos5x_plls[kpll].rate_table = exynos5420_pll2550x_24mhz_tbl;
+ }
+
samsung_clk_register_pll(ctx, exynos5x_plls, ARRAY_SIZE(exynos5x_plls),
reg_base);
samsung_clk_register_fixed_rate(ctx, exynos5x_fixed_rate_clks,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v7 2/6] clk: samsung: register exynos5420 apll/kpll configuration data
2014-07-14 13:38 ` [PATCH v7 2/6] clk: samsung: register exynos5420 apll/kpll configuration data Thomas Abraham
@ 2014-07-19 12:57 ` Tomasz Figa
0 siblings, 0 replies; 25+ messages in thread
From: Tomasz Figa @ 2014-07-19 12:57 UTC (permalink / raw)
To: linux-arm-kernel
On 14.07.2014 15:38, Thomas Abraham wrote:
> From: Thomas Abraham <thomas.ab@samsung.com>
>
> Register the PLL configuration data for APLL and KPLL on Exynos5420. This
> configuration data table specifies PLL coefficients for supported PLL
> clock speeds when a 24MHz clock is supplied as the input clock source
> for these PLLs.
>
> Cc: Tomasz Figa <t.figa@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> Reviewed-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> Tested-by: Arjun K.V <arjun.kv@samsung.com>
> ---
> drivers/clk/samsung/clk-exynos5420.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
Looks good. Will apply.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v7 3/6] clk: exynos: use cpu-clock provider type to represent arm clock
2014-07-14 13:38 [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms Thomas Abraham
2014-07-14 13:38 ` [PATCH v7 1/6] clk: samsung: add infrastructure to register cpu clocks Thomas Abraham
2014-07-14 13:38 ` [PATCH v7 2/6] clk: samsung: register exynos5420 apll/kpll configuration data Thomas Abraham
@ 2014-07-14 13:38 ` Thomas Abraham
2014-07-19 13:06 ` Tomasz Figa
2014-07-14 13:38 ` [PATCH v7 4/6] ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data Thomas Abraham
` (4 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Thomas Abraham @ 2014-07-14 13:38 UTC (permalink / raw)
To: linux-arm-kernel
From: Thomas Abraham <thomas.ab@samsung.com>
Register cpu clocks using the new cpu-clock provider type for exynos platforms.
The differnt clock blocks that are now encapsulated within the cpu-clock can be
marked with read-only attribute.
Cc: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
Reviewed-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
Tested-by: Arjun K.V <arjun.kv@samsung.com>
---
drivers/clk/samsung/clk-exynos4.c | 25 +++++++++++++++++--------
drivers/clk/samsung/clk-exynos5250.c | 16 +++++++++++-----
drivers/clk/samsung/clk-exynos5420.c | 32 +++++++++++++++++++++++---------
include/dt-bindings/clock/exynos5250.h | 1 +
include/dt-bindings/clock/exynos5420.h | 2 ++
5 files changed, 54 insertions(+), 22 deletions(-)
diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index 75573a4..10f1818 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -471,7 +471,8 @@ static struct samsung_mux_clock exynos4210_mux_clks[] __initdata = {
MUX(0, "mout_fimd1", group1_p4210, E4210_SRC_LCD1, 0, 4),
MUX(0, "mout_mipi1", group1_p4210, E4210_SRC_LCD1, 12, 4),
MUX(CLK_SCLK_MPLL, "sclk_mpll", mout_mpll_p, SRC_CPU, 8, 1),
- MUX(CLK_MOUT_CORE, "mout_core", mout_core_p4210, SRC_CPU, 16, 1),
+ MUX_F(CLK_MOUT_CORE, "mout_core", mout_core_p4210, SRC_CPU, 16, 1, 0,
+ CLK_MUX_READ_ONLY),
MUX(CLK_SCLK_VPLL, "sclk_vpll", sclk_vpll_p4210, SRC_TOP0, 8, 1),
MUX(CLK_MOUT_FIMC0, "mout_fimc0", group1_p4210, SRC_CAM, 0, 4),
MUX(CLK_MOUT_FIMC1, "mout_fimc1", group1_p4210, SRC_CAM, 4, 4),
@@ -530,7 +531,8 @@ static struct samsung_mux_clock exynos4x12_mux_clks[] __initdata = {
MUX(0, "mout_jpeg", mout_jpeg_p, E4X12_SRC_CAM1, 8, 1),
MUX(CLK_SCLK_MPLL, "sclk_mpll", mout_mpll_p, SRC_DMC, 12, 1),
MUX(CLK_SCLK_VPLL, "sclk_vpll", mout_vpll_p, SRC_TOP0, 8, 1),
- MUX(CLK_MOUT_CORE, "mout_core", mout_core_p4x12, SRC_CPU, 16, 1),
+ MUX_F(CLK_MOUT_CORE, "mout_core", mout_core_p4x12, SRC_CPU, 16, 1, 0,
+ CLK_MUX_READ_ONLY),
MUX(CLK_MOUT_FIMC0, "mout_fimc0", group1_p4x12, SRC_CAM, 0, 4),
MUX(CLK_MOUT_FIMC1, "mout_fimc1", group1_p4x12, SRC_CAM, 4, 4),
MUX(CLK_MOUT_FIMC2, "mout_fimc2", group1_p4x12, SRC_CAM, 8, 4),
@@ -572,8 +574,10 @@ static struct samsung_mux_clock exynos4x12_mux_clks[] __initdata = {
/* list of divider clocks supported in all exynos4 soc's */
static struct samsung_div_clock exynos4_div_clks[] __initdata = {
- DIV(0, "div_core", "mout_core", DIV_CPU0, 0, 3),
- DIV(0, "div_core2", "div_core", DIV_CPU0, 28, 3),
+ DIV_F(0, "div_core", "mout_core", DIV_CPU0, 0, 3,
+ CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
+ DIV_F(0, "div_core2", "div_core", DIV_CPU0, 28, 3,
+ CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
DIV(0, "div_fimc0", "mout_fimc0", DIV_CAM, 0, 4),
DIV(0, "div_fimc1", "mout_fimc1", DIV_CAM, 4, 4),
DIV(0, "div_fimc2", "mout_fimc2", DIV_CAM, 8, 4),
@@ -619,8 +623,10 @@ static struct samsung_div_clock exynos4_div_clks[] __initdata = {
DIV(0, "div_spi_pre2", "div_spi2", DIV_PERIL2, 8, 8),
DIV(0, "div_audio1", "mout_audio1", DIV_PERIL4, 0, 4),
DIV(0, "div_audio2", "mout_audio2", DIV_PERIL4, 16, 4),
- DIV(CLK_ARM_CLK, "arm_clk", "div_core2", DIV_CPU0, 28, 3),
- DIV(CLK_SCLK_APLL, "sclk_apll", "mout_apll", DIV_CPU0, 24, 3),
+ DIV_F(CLK_ARM_CLK, "arm_clk", "div_core2", DIV_CPU0, 28, 3,
+ CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
+ DIV_F(CLK_SCLK_APLL, "sclk_apll", "mout_apll", DIV_CPU0, 24, 3,
+ CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
DIV_F(0, "div_mipi_pre0", "div_mipi0", DIV_LCD0, 20, 4,
CLK_SET_RATE_PARENT, 0),
DIV_F(0, "div_mmc_pre0", "div_mmc0", DIV_FSYS1, 8, 8,
@@ -1016,7 +1022,6 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
static struct samsung_clock_alias exynos4_aliases[] __initdata = {
ALIAS(CLK_MOUT_CORE, NULL, "moutcore"),
- ALIAS(CLK_ARM_CLK, NULL, "armclk"),
ALIAS(CLK_SCLK_APLL, NULL, "mout_apll"),
};
@@ -1255,6 +1260,8 @@ static void __init exynos4_clk_init(struct device_node *np,
ARRAY_SIZE(exynos4210_gate_clks));
samsung_clk_register_alias(ctx, exynos4210_aliases,
ARRAY_SIZE(exynos4210_aliases));
+ exynos_register_cpu_clock(ctx, 0, CLK_ARM_CLK, "armclk",
+ mout_core_p4210[0], mout_core_p4210[1], np);
} else {
samsung_clk_register_mux(ctx, exynos4x12_mux_clks,
ARRAY_SIZE(exynos4x12_mux_clks));
@@ -1264,6 +1271,8 @@ static void __init exynos4_clk_init(struct device_node *np,
ARRAY_SIZE(exynos4x12_gate_clks));
samsung_clk_register_alias(ctx, exynos4x12_aliases,
ARRAY_SIZE(exynos4x12_aliases));
+ exynos_register_cpu_clock(ctx, 0, CLK_ARM_CLK, "armclk",
+ mout_core_p4x12[0], mout_core_p4x12[1], np);
}
samsung_clk_register_alias(ctx, exynos4_aliases,
@@ -1278,7 +1287,7 @@ static void __init exynos4_clk_init(struct device_node *np,
exynos4_soc == EXYNOS4210 ? "Exynos4210" : "Exynos4x12",
_get_rate("sclk_apll"), _get_rate("sclk_mpll"),
_get_rate("sclk_epll"), _get_rate("sclk_vpll"),
- _get_rate("arm_clk"));
+ _get_rate("armclk"));
}
diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index 5861183..2f78bb0 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -292,7 +292,8 @@ static struct samsung_mux_clock exynos5250_mux_clks[] __initdata = {
*/
MUX_FA(0, "mout_apll", mout_apll_p, SRC_CPU, 0, 1,
CLK_SET_RATE_PARENT, 0, "mout_apll"),
- MUX_A(0, "mout_cpu", mout_cpu_p, SRC_CPU, 16, 1, "mout_cpu"),
+ MUX_FA(0, "mout_cpu", mout_cpu_p, SRC_CPU, 16, 1,
+ 0, CLK_MUX_READ_ONLY, "mout_cpu"),
/*
* CMU_CORE
@@ -379,9 +380,12 @@ static struct samsung_div_clock exynos5250_div_clks[] __initdata = {
/*
* CMU_CPU
*/
- DIV(0, "div_arm", "mout_cpu", DIV_CPU0, 0, 3),
- DIV(0, "div_apll", "mout_apll", DIV_CPU0, 24, 3),
- DIV_A(0, "div_arm2", "div_arm", DIV_CPU0, 28, 3, "armclk"),
+ DIV_F(0, "div_arm", "mout_cpu", DIV_CPU0, 0, 3,
+ CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
+ DIV_F(0, "div_apll", "mout_apll", DIV_CPU0, 24, 3,
+ CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
+ DIV_F(0, "div_arm2", "div_arm", DIV_CPU0, 28, 3,
+ CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
/*
* CMU_TOP
@@ -797,6 +801,8 @@ static void __init exynos5250_clk_init(struct device_node *np)
ARRAY_SIZE(exynos5250_div_clks));
samsung_clk_register_gate(ctx, exynos5250_gate_clks,
ARRAY_SIZE(exynos5250_gate_clks));
+ exynos_register_cpu_clock(ctx, 0, CLK_ARM_CLK, "armclk", mout_cpu_p[0],
+ mout_cpu_p[1], np);
/*
* Enable arm clock down (in idle) and set arm divider
@@ -823,6 +829,6 @@ static void __init exynos5250_clk_init(struct device_node *np)
samsung_clk_of_add_provider(np, ctx);
pr_info("Exynos5250: clock setup completed, armclk=%ld\n",
- _get_rate("div_arm2"));
+ _get_rate("armclk"));
}
CLK_OF_DECLARE(exynos5250_clk, "samsung,exynos5250-clock", exynos5250_clk_init);
diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index 95f83be..6103e01 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -587,10 +587,14 @@ static struct samsung_mux_clock exynos5x_mux_clks[] __initdata = {
MUX(0, "mout_mspll_kfc", mout_mspll_cpu_p, SRC_TOP7, 8, 2),
MUX(0, "mout_mspll_cpu", mout_mspll_cpu_p, SRC_TOP7, 12, 2),
- MUX(0, "mout_apll", mout_apll_p, SRC_CPU, 0, 1),
- MUX(0, "mout_cpu", mout_cpu_p, SRC_CPU, 16, 1),
- MUX(0, "mout_kpll", mout_kpll_p, SRC_KFC, 0, 1),
- MUX(0, "mout_kfc", mout_kfc_p, SRC_KFC, 16, 1),
+ MUX_F(0, "mout_apll", mout_apll_p, SRC_CPU, 0, 1,
+ CLK_SET_RATE_PARENT, 0),
+ MUX_F(0, "mout_cpu", mout_cpu_p, SRC_CPU, 16, 1, 0,
+ CLK_MUX_READ_ONLY),
+ MUX_F(0, "mout_kpll", mout_kpll_p, SRC_KFC, 0, 1,
+ CLK_SET_RATE_PARENT, 0),
+ MUX_F(0, "mout_kfc", mout_kfc_p, SRC_KFC, 16, 1, 0,
+ CLK_MUX_READ_ONLY),
MUX(0, "mout_aclk200", mout_group1_p, SRC_TOP0, 8, 2),
MUX(0, "mout_aclk200_fsys2", mout_group1_p, SRC_TOP0, 12, 2),
@@ -746,11 +750,16 @@ static struct samsung_mux_clock exynos5x_mux_clks[] __initdata = {
};
static struct samsung_div_clock exynos5x_div_clks[] __initdata = {
- DIV(0, "div_arm", "mout_cpu", DIV_CPU0, 0, 3),
- DIV(0, "sclk_apll", "mout_apll", DIV_CPU0, 24, 3),
- DIV(0, "armclk2", "div_arm", DIV_CPU0, 28, 3),
- DIV(0, "div_kfc", "mout_kfc", DIV_KFC0, 0, 3),
- DIV(0, "sclk_kpll", "mout_kpll", DIV_KFC0, 24, 3),
+ DIV_F(0, "div_arm", "mout_cpu", DIV_CPU0, 0, 3,
+ CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
+ DIV_F(0, "sclk_apll", "mout_apll", DIV_CPU0, 24, 3,
+ CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
+ DIV_F(0, "armclk2", "div_arm", DIV_CPU0, 28, 3,
+ CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
+ DIV_F(0, "div_kfc", "mout_kfc", DIV_KFC0, 0, 3,
+ CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
+ DIV_F(0, "sclk_kpll", "mout_kpll", DIV_KFC0, 24, 3,
+ CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
DIV(0, "dout_aclk400_isp", "mout_aclk400_isp", DIV_TOP0, 0, 3),
DIV(0, "dout_aclk400_mscl", "mout_aclk400_mscl", DIV_TOP0, 4, 3),
@@ -1280,6 +1289,11 @@ static void __init exynos5x_clk_init(struct device_node *np,
ARRAY_SIZE(exynos5800_gate_clks));
}
+ exynos_register_cpu_clock(ctx, 0, CLK_ARM_CLK, "armclk",
+ mout_cpu_p[0], mout_cpu_p[1], np);
+ exynos_register_cpu_clock(ctx, 1, CLK_KFC_CLK, "kfcclk",
+ mout_kfc_p[0], mout_kfc_p[1], np);
+
exynos5420_clk_sleep_init();
samsung_clk_of_add_provider(np, ctx);
diff --git a/include/dt-bindings/clock/exynos5250.h b/include/dt-bindings/clock/exynos5250.h
index be6e97c..d3565be 100644
--- a/include/dt-bindings/clock/exynos5250.h
+++ b/include/dt-bindings/clock/exynos5250.h
@@ -21,6 +21,7 @@
#define CLK_FOUT_CPLL 6
#define CLK_FOUT_EPLL 7
#define CLK_FOUT_VPLL 8
+#define CLK_ARM_CLK 12
/* gate for special clocks (sclk) */
#define CLK_SCLK_CAM_BAYER 128
diff --git a/include/dt-bindings/clock/exynos5420.h b/include/dt-bindings/clock/exynos5420.h
index 21d51ae..6fce074 100644
--- a/include/dt-bindings/clock/exynos5420.h
+++ b/include/dt-bindings/clock/exynos5420.h
@@ -25,6 +25,8 @@
#define CLK_FOUT_MPLL 10
#define CLK_FOUT_BPLL 11
#define CLK_FOUT_KPLL 12
+#define CLK_ARM_CLK 13
+#define CLK_KFC_CLK 14
/* gate for special clocks (sclk) */
#define CLK_SCLK_UART0 128
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v7 3/6] clk: exynos: use cpu-clock provider type to represent arm clock
2014-07-14 13:38 ` [PATCH v7 3/6] clk: exynos: use cpu-clock provider type to represent arm clock Thomas Abraham
@ 2014-07-19 13:06 ` Tomasz Figa
0 siblings, 0 replies; 25+ messages in thread
From: Tomasz Figa @ 2014-07-19 13:06 UTC (permalink / raw)
To: linux-arm-kernel
Hi Thomas,
Please see my comments inline.
On 14.07.2014 15:38, Thomas Abraham wrote:
> From: Thomas Abraham <thomas.ab@samsung.com>
>
> Register cpu clocks using the new cpu-clock provider type for exynos platforms.
> The differnt clock blocks that are now encapsulated within the cpu-clock can be
> marked with read-only attribute.
>
> Cc: Tomasz Figa <t.figa@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> Reviewed-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> Tested-by: Arjun K.V <arjun.kv@samsung.com>
> ---
> drivers/clk/samsung/clk-exynos4.c | 25 +++++++++++++++++--------
> drivers/clk/samsung/clk-exynos5250.c | 16 +++++++++++-----
> drivers/clk/samsung/clk-exynos5420.c | 32 +++++++++++++++++++++++---------
> include/dt-bindings/clock/exynos5250.h | 1 +
> include/dt-bindings/clock/exynos5420.h | 2 ++
> 5 files changed, 54 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
> index 75573a4..10f1818 100644
> --- a/drivers/clk/samsung/clk-exynos4.c
> +++ b/drivers/clk/samsung/clk-exynos4.c
> @@ -471,7 +471,8 @@ static struct samsung_mux_clock exynos4210_mux_clks[] __initdata = {
> MUX(0, "mout_fimd1", group1_p4210, E4210_SRC_LCD1, 0, 4),
> MUX(0, "mout_mipi1", group1_p4210, E4210_SRC_LCD1, 12, 4),
> MUX(CLK_SCLK_MPLL, "sclk_mpll", mout_mpll_p, SRC_CPU, 8, 1),
> - MUX(CLK_MOUT_CORE, "mout_core", mout_core_p4210, SRC_CPU, 16, 1),
> + MUX_F(CLK_MOUT_CORE, "mout_core", mout_core_p4210, SRC_CPU, 16, 1, 0,
> + CLK_MUX_READ_ONLY),
> MUX(CLK_SCLK_VPLL, "sclk_vpll", sclk_vpll_p4210, SRC_TOP0, 8, 1),
> MUX(CLK_MOUT_FIMC0, "mout_fimc0", group1_p4210, SRC_CAM, 0, 4),
> MUX(CLK_MOUT_FIMC1, "mout_fimc1", group1_p4210, SRC_CAM, 4, 4),
> @@ -530,7 +531,8 @@ static struct samsung_mux_clock exynos4x12_mux_clks[] __initdata = {
> MUX(0, "mout_jpeg", mout_jpeg_p, E4X12_SRC_CAM1, 8, 1),
> MUX(CLK_SCLK_MPLL, "sclk_mpll", mout_mpll_p, SRC_DMC, 12, 1),
> MUX(CLK_SCLK_VPLL, "sclk_vpll", mout_vpll_p, SRC_TOP0, 8, 1),
> - MUX(CLK_MOUT_CORE, "mout_core", mout_core_p4x12, SRC_CPU, 16, 1),
> + MUX_F(CLK_MOUT_CORE, "mout_core", mout_core_p4x12, SRC_CPU, 16, 1, 0,
> + CLK_MUX_READ_ONLY),
> MUX(CLK_MOUT_FIMC0, "mout_fimc0", group1_p4x12, SRC_CAM, 0, 4),
> MUX(CLK_MOUT_FIMC1, "mout_fimc1", group1_p4x12, SRC_CAM, 4, 4),
> MUX(CLK_MOUT_FIMC2, "mout_fimc2", group1_p4x12, SRC_CAM, 8, 4),
> @@ -572,8 +574,10 @@ static struct samsung_mux_clock exynos4x12_mux_clks[] __initdata = {
>
> /* list of divider clocks supported in all exynos4 soc's */
> static struct samsung_div_clock exynos4_div_clks[] __initdata = {
> - DIV(0, "div_core", "mout_core", DIV_CPU0, 0, 3),
> - DIV(0, "div_core2", "div_core", DIV_CPU0, 28, 3),
> + DIV_F(0, "div_core", "mout_core", DIV_CPU0, 0, 3,
> + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
> + DIV_F(0, "div_core2", "div_core", DIV_CPU0, 28, 3,
> + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
> DIV(0, "div_fimc0", "mout_fimc0", DIV_CAM, 0, 4),
> DIV(0, "div_fimc1", "mout_fimc1", DIV_CAM, 4, 4),
> DIV(0, "div_fimc2", "mout_fimc2", DIV_CAM, 8, 4),
> @@ -619,8 +623,10 @@ static struct samsung_div_clock exynos4_div_clks[] __initdata = {
> DIV(0, "div_spi_pre2", "div_spi2", DIV_PERIL2, 8, 8),
> DIV(0, "div_audio1", "mout_audio1", DIV_PERIL4, 0, 4),
> DIV(0, "div_audio2", "mout_audio2", DIV_PERIL4, 16, 4),
> - DIV(CLK_ARM_CLK, "arm_clk", "div_core2", DIV_CPU0, 28, 3),
> - DIV(CLK_SCLK_APLL, "sclk_apll", "mout_apll", DIV_CPU0, 24, 3),
> + DIV_F(CLK_ARM_CLK, "arm_clk", "div_core2", DIV_CPU0, 28, 3,
> + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
> + DIV_F(CLK_SCLK_APLL, "sclk_apll", "mout_apll", DIV_CPU0, 24, 3,
> + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
> DIV_F(0, "div_mipi_pre0", "div_mipi0", DIV_LCD0, 20, 4,
> CLK_SET_RATE_PARENT, 0),
> DIV_F(0, "div_mmc_pre0", "div_mmc0", DIV_FSYS1, 8, 8,
> @@ -1016,7 +1022,6 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>
> static struct samsung_clock_alias exynos4_aliases[] __initdata = {
> ALIAS(CLK_MOUT_CORE, NULL, "moutcore"),
> - ALIAS(CLK_ARM_CLK, NULL, "armclk"),
> ALIAS(CLK_SCLK_APLL, NULL, "mout_apll"),
> };
These aliases are used only by the old cpufreq driver. If we are
removing support for it, we should also drop them.
However I wonder if we shouldn't rather initially allow both drivers to
coexist with the old still being the default for one release. We want to
avoid regressions so we should first test the new driver for a while and
then, if it proves to be fine, we could drop the old one.
This would probably mean some ifdefs in clock drivers until the old
driver gets removed, but I don't think it's a problem.
>
> @@ -1255,6 +1260,8 @@ static void __init exynos4_clk_init(struct device_node *np,
> ARRAY_SIZE(exynos4210_gate_clks));
> samsung_clk_register_alias(ctx, exynos4210_aliases,
> ARRAY_SIZE(exynos4210_aliases));
> + exynos_register_cpu_clock(ctx, 0, CLK_ARM_CLK, "armclk",
> + mout_core_p4210[0], mout_core_p4210[1], np);
> } else {
> samsung_clk_register_mux(ctx, exynos4x12_mux_clks,
> ARRAY_SIZE(exynos4x12_mux_clks));
> @@ -1264,6 +1271,8 @@ static void __init exynos4_clk_init(struct device_node *np,
> ARRAY_SIZE(exynos4x12_gate_clks));
> samsung_clk_register_alias(ctx, exynos4x12_aliases,
> ARRAY_SIZE(exynos4x12_aliases));
> + exynos_register_cpu_clock(ctx, 0, CLK_ARM_CLK, "armclk",
> + mout_core_p4x12[0], mout_core_p4x12[1], np);
> }
>
> samsung_clk_register_alias(ctx, exynos4_aliases,
> @@ -1278,7 +1287,7 @@ static void __init exynos4_clk_init(struct device_node *np,
> exynos4_soc == EXYNOS4210 ? "Exynos4210" : "Exynos4x12",
> _get_rate("sclk_apll"), _get_rate("sclk_mpll"),
> _get_rate("sclk_epll"), _get_rate("sclk_vpll"),
> - _get_rate("arm_clk"));
> + _get_rate("armclk"));
> }
>
>
> diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
> index 5861183..2f78bb0 100644
> --- a/drivers/clk/samsung/clk-exynos5250.c
> +++ b/drivers/clk/samsung/clk-exynos5250.c
> @@ -292,7 +292,8 @@ static struct samsung_mux_clock exynos5250_mux_clks[] __initdata = {
> */
> MUX_FA(0, "mout_apll", mout_apll_p, SRC_CPU, 0, 1,
> CLK_SET_RATE_PARENT, 0, "mout_apll"),
> - MUX_A(0, "mout_cpu", mout_cpu_p, SRC_CPU, 16, 1, "mout_cpu"),
> + MUX_FA(0, "mout_cpu", mout_cpu_p, SRC_CPU, 16, 1,
> + 0, CLK_MUX_READ_ONLY, "mout_cpu"),
>
Similar comment about aliases here and for few clocks below.
Otherwise the patch looks fine.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v7 4/6] ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data
2014-07-14 13:38 [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms Thomas Abraham
` (2 preceding siblings ...)
2014-07-14 13:38 ` [PATCH v7 3/6] clk: exynos: use cpu-clock provider type to represent arm clock Thomas Abraham
@ 2014-07-14 13:38 ` Thomas Abraham
2014-07-19 13:18 ` Tomasz Figa
2014-07-14 13:38 ` [PATCH v7 5/6] ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250/5420 Thomas Abraham
` (3 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Thomas Abraham @ 2014-07-14 13:38 UTC (permalink / raw)
To: linux-arm-kernel
From: Thomas Abraham <thomas.ab@samsung.com>
For Exynos 4210/5250/5420 based platforms, add CPU nodes, operating points and
cpu clock data for migrating from Exynos specific cpufreq driver to using
generic cpufreq drivers.
Cc: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
Reviewed-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
Tested-by: Arjun K.V <arjun.kv@samsung.com>
---
arch/arm/boot/dts/exynos4210-origen.dts | 6 ++++
arch/arm/boot/dts/exynos4210-trats.dts | 6 ++++
arch/arm/boot/dts/exynos4210-universal_c210.dts | 6 ++++
arch/arm/boot/dts/exynos4210.dtsi | 27 ++++++++++++++++
arch/arm/boot/dts/exynos5250-arndale.dts | 6 ++++
arch/arm/boot/dts/exynos5250-cros-common.dtsi | 6 ++++
arch/arm/boot/dts/exynos5250-smdk5250.dts | 6 ++++
arch/arm/boot/dts/exynos5250.dtsi | 23 ++++++++++++++
arch/arm/boot/dts/exynos5420-smdk5420.dts | 6 ++++
arch/arm/boot/dts/exynos5420.dtsi | 38 +++++++++++++++++++++++
10 files changed, 130 insertions(+)
diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts
index f767c42..49a97fc 100644
--- a/arch/arm/boot/dts/exynos4210-origen.dts
+++ b/arch/arm/boot/dts/exynos4210-origen.dts
@@ -33,6 +33,12 @@
bootargs ="root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC2,115200 init=/linuxrc";
};
+ cpus {
+ cpu at 0 {
+ cpu0-supply = <&buck1_reg>;
+ };
+ };
+
regulators {
compatible = "simple-bus";
#address-cells = <1>;
diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts
index f516da9..fe32b6a 100644
--- a/arch/arm/boot/dts/exynos4210-trats.dts
+++ b/arch/arm/boot/dts/exynos4210-trats.dts
@@ -30,6 +30,12 @@
bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rootwait earlyprintk panic=5";
};
+ cpus {
+ cpu: cpu at 0 {
+ cpu0-supply = <&varm_breg>;
+ };
+ };
+
regulators {
compatible = "simple-bus";
diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts b/arch/arm/boot/dts/exynos4210-universal_c210.dts
index d50eb3a..8ab12d6 100644
--- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
+++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
@@ -28,6 +28,12 @@
bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rw rootwait earlyprintk panic=5 maxcpus=1";
};
+ cpus {
+ cpu: cpu at 0 {
+ cpu0-supply = <&vdd_arm_reg>;
+ };
+ };
+
sysram at 02020000 {
smp-sysram at 0 {
status = "disabled";
diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index ee3001f..c3a73bf 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -31,6 +31,33 @@
pinctrl2 = &pinctrl_2;
};
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ cpu at 0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a9";
+ reg = <0>;
+ clocks = <&clock CLK_ARM_CLK>;
+ clock-names = "cpu";
+
+ operating-points = <
+ 1200000 1250000
+ 1000000 1150000
+ 800000 1075000
+ 500000 975000
+ 400000 975000
+ 200000 950000
+ >;
+ };
+
+ cpu at 1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a9";
+ reg = <1>;
+ };
+ };
+
sysram at 02020000 {
compatible = "mmio-sram";
reg = <0x02020000 0x20000>;
diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
index d0de1f5..d9b803b 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -26,6 +26,12 @@
bootargs = "console=ttySAC2,115200";
};
+ cpus {
+ cpu at 0 {
+ cpu0-supply = <&buck2_reg>;
+ };
+ };
+
rtc at 101E0000 {
status = "okay";
};
diff --git a/arch/arm/boot/dts/exynos5250-cros-common.dtsi b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
index 89ac90f..34bb31c 100644
--- a/arch/arm/boot/dts/exynos5250-cros-common.dtsi
+++ b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
@@ -19,6 +19,12 @@
chosen {
};
+ cpus {
+ cpu at 0 {
+ cpu0-supply = <&buck2_reg>;
+ };
+ };
+
pinctrl at 11400000 {
/*
* Disabled pullups since external part has its own pullups and
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index a794a70..3632f7a 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -27,6 +27,12 @@
bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC2,115200 init=/linuxrc";
};
+ cpus {
+ cpu at 0 {
+ cpu0-supply = <&buck2_reg>;
+ };
+ };
+
rtc at 101E0000 {
status = "okay";
};
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 834fb5a..66b0f98 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -63,6 +63,29 @@
compatible = "arm,cortex-a15";
reg = <0>;
clock-frequency = <1700000000>;
+
+ clocks = <&clock CLK_ARM_CLK>;
+ clock-names = "cpu";
+
+ operating-points = <
+ 1700000 1300000
+ 1600000 1250000
+ 1500000 1225000
+ 1400000 1200000
+ 1300000 1150000
+ 1200000 1125000
+ 1100000 1100000
+ 1000000 1075000
+ 900000 1050000
+ 800000 1025000
+ 700000 1012500
+ 600000 1000000
+ 500000 975000
+ 400000 950000
+ 300000 937500
+ 200000 925000
+ >;
+ clock-latency = <200000>;
};
cpu at 1 {
device_type = "cpu";
diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index 6052aa9..084e587 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -24,6 +24,12 @@
bootargs = "console=ttySAC2,115200 init=/linuxrc";
};
+ cpus {
+ cpu at 4 {
+ cpu0-supply = <&buck6_reg>;
+ };
+ };
+
fixed-rate-clocks {
oscclk {
compatible = "samsung,exynos5420-oscclk";
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 1595722..6bec2e3 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -59,8 +59,26 @@
device_type = "cpu";
compatible = "arm,cortex-a15";
reg = <0x0>;
+ clocks = <&clock CLK_ARM_CLK>;
+ clock-names = "cpu-cluster.0";
clock-frequency = <1800000000>;
cci-control-port = <&cci_control1>;
+ clock-latency = <200000>;
+
+ operating-points = <
+ 1800000 1250000
+ 1700000 1212500
+ 1600000 1175000
+ 1500000 1137500
+ 1400000 1112500
+ 1300000 1062500
+ 1200000 1037500
+ 1100000 1012500
+ 1000000 987500
+ 900000 962500
+ 800000 937500
+ 700000 912500
+ >;
};
cpu1: cpu at 1 {
@@ -69,6 +87,7 @@
reg = <0x1>;
clock-frequency = <1800000000>;
cci-control-port = <&cci_control1>;
+ clock-latency = <200000>;
};
cpu2: cpu at 2 {
@@ -77,6 +96,7 @@
reg = <0x2>;
clock-frequency = <1800000000>;
cci-control-port = <&cci_control1>;
+ clock-latency = <200000>;
};
cpu3: cpu at 3 {
@@ -85,14 +105,29 @@
reg = <0x3>;
clock-frequency = <1800000000>;
cci-control-port = <&cci_control1>;
+ clock-latency = <200000>;
};
cpu4: cpu at 100 {
device_type = "cpu";
compatible = "arm,cortex-a7";
reg = <0x100>;
+ clocks = <&clock CLK_KFC_CLK>;
+ clock-names = "cpu-cluster.1";
clock-frequency = <1000000000>;
cci-control-port = <&cci_control0>;
+ clock-latency = <200000>;
+
+ operating-points = <
+ 1300000 1275000
+ 1200000 1212500
+ 1100000 1162500
+ 1000000 1112500
+ 900000 1062500
+ 800000 1025000
+ 700000 975000
+ 600000 937500
+ >;
};
cpu5: cpu at 101 {
@@ -101,6 +136,7 @@
reg = <0x101>;
clock-frequency = <1000000000>;
cci-control-port = <&cci_control0>;
+ clock-latency = <200000>;
};
cpu6: cpu at 102 {
@@ -109,6 +145,7 @@
reg = <0x102>;
clock-frequency = <1000000000>;
cci-control-port = <&cci_control0>;
+ clock-latency = <200000>;
};
cpu7: cpu at 103 {
@@ -117,6 +154,7 @@
reg = <0x103>;
clock-frequency = <1000000000>;
cci-control-port = <&cci_control0>;
+ clock-latency = <200000>;
};
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v7 4/6] ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data
2014-07-14 13:38 ` [PATCH v7 4/6] ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data Thomas Abraham
@ 2014-07-19 13:18 ` Tomasz Figa
2014-07-29 5:36 ` Thomas Abraham
0 siblings, 1 reply; 25+ messages in thread
From: Tomasz Figa @ 2014-07-19 13:18 UTC (permalink / raw)
To: linux-arm-kernel
Hi Thomas,
Please see my comments inline.
On 14.07.2014 15:38, Thomas Abraham wrote:
> From: Thomas Abraham <thomas.ab@samsung.com>
>
> For Exynos 4210/5250/5420 based platforms, add CPU nodes, operating points and
> cpu clock data for migrating from Exynos specific cpufreq driver to using
> generic cpufreq drivers.
[snip]
> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
> index ee3001f..c3a73bf 100644
> --- a/arch/arm/boot/dts/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/exynos4210.dtsi
> @@ -31,6 +31,33 @@
> pinctrl2 = &pinctrl_2;
> };
>
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + cpu at 0 {
nit: Missing blank line after last property.
The cluster ID field of MPIDR on Exynos4210 is 0x9 not zero, which means
that this should be cpu at 900.
> + device_type = "cpu";
> + compatible = "arm,cortex-a9";
> + reg = <0>;
reg = <0x900>;
> + clocks = <&clock CLK_ARM_CLK>;
> + clock-names = "cpu";
> +
> + operating-points = <
> + 1200000 1250000
> + 1000000 1150000
> + 800000 1075000
> + 500000 975000
> + 400000 975000
> + 200000 950000
> + >;
> + };
> +
> + cpu at 1 {
cpu at 901
> + device_type = "cpu";
> + compatible = "arm,cortex-a9";
> + reg = <1>;
reg = <0x901>;
> + };
In general this wouldn't have even booted, because there were several
places where code relied on CPUs being 0, 1, 2... However I have sent
necessary fixes and they should hit linux-next in few days.
> + };
> +
> sysram at 02020000 {
> compatible = "mmio-sram";
> reg = <0x02020000 0x20000>;
[snip]
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index 834fb5a..66b0f98 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -63,6 +63,29 @@
> compatible = "arm,cortex-a15";
> reg = <0>;
> clock-frequency = <1700000000>;
> +
> + clocks = <&clock CLK_ARM_CLK>;
> + clock-names = "cpu";
> +
> + operating-points = <
> + 1700000 1300000
> + 1600000 1250000
> + 1500000 1225000
> + 1400000 1200000
> + 1300000 1150000
> + 1200000 1125000
> + 1100000 1100000
> + 1000000 1075000
> + 900000 1050000
> + 800000 1025000
> + 700000 1012500
> + 600000 1000000
> + 500000 975000
> + 400000 950000
> + 300000 937500
> + 200000 925000
> + >;
> + clock-latency = <200000>;
I don't see this property specified for Exynos4210. Have you missed it
there?
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v7 4/6] ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data
2014-07-19 13:18 ` Tomasz Figa
@ 2014-07-29 5:36 ` Thomas Abraham
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Abraham @ 2014-07-29 5:36 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
On Sat, Jul 19, 2014 at 6:48 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Thomas,
>
> Please see my comments inline.
>
> On 14.07.2014 15:38, Thomas Abraham wrote:
>> From: Thomas Abraham <thomas.ab@samsung.com>
>>
>> For Exynos 4210/5250/5420 based platforms, add CPU nodes, operating points and
>> cpu clock data for migrating from Exynos specific cpufreq driver to using
>> generic cpufreq drivers.
>
> [snip]
>
>> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
>> index ee3001f..c3a73bf 100644
>> --- a/arch/arm/boot/dts/exynos4210.dtsi
>> +++ b/arch/arm/boot/dts/exynos4210.dtsi
>> @@ -31,6 +31,33 @@
>> pinctrl2 = &pinctrl_2;
>> };
>>
>> + cpus {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + cpu at 0 {
>
> nit: Missing blank line after last property.
>
> The cluster ID field of MPIDR on Exynos4210 is 0x9 not zero, which means
> that this should be cpu at 900.
>
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a9";
>> + reg = <0>;
>
> reg = <0x900>;
>
>> + clocks = <&clock CLK_ARM_CLK>;
>> + clock-names = "cpu";
>> +
>> + operating-points = <
>> + 1200000 1250000
>> + 1000000 1150000
>> + 800000 1075000
>> + 500000 975000
>> + 400000 975000
>> + 200000 950000
>> + >;
>> + };
>> +
>> + cpu at 1 {
>
> cpu at 901
>
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a9";
>> + reg = <1>;
>
> reg = <0x901>;
>
>> + };
>
> In general this wouldn't have even booted, because there were several
> places where code relied on CPUs being 0, 1, 2... However I have sent
> necessary fixes and they should hit linux-next in few days.
>
>> + };
>> +
>> sysram at 02020000 {
>> compatible = "mmio-sram";
>> reg = <0x02020000 0x20000>;
>
> [snip]
>
>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
>> index 834fb5a..66b0f98 100644
>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>> @@ -63,6 +63,29 @@
>> compatible = "arm,cortex-a15";
>> reg = <0>;
>> clock-frequency = <1700000000>;
>> +
>> + clocks = <&clock CLK_ARM_CLK>;
>> + clock-names = "cpu";
>> +
>> + operating-points = <
>> + 1700000 1300000
>> + 1600000 1250000
>> + 1500000 1225000
>> + 1400000 1200000
>> + 1300000 1150000
>> + 1200000 1125000
>> + 1100000 1100000
>> + 1000000 1075000
>> + 900000 1050000
>> + 800000 1025000
>> + 700000 1012500
>> + 600000 1000000
>> + 500000 975000
>> + 400000 950000
>> + 300000 937500
>> + 200000 925000
>> + >;
>> + clock-latency = <200000>;
>
> I don't see this property specified for Exynos4210. Have you missed it
> there?
Okay, I have specified this for Exynos4210 as well.
Thanks,
Thomas.
>
> Best regards,
> Tomasz
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v7 5/6] ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250/5420
2014-07-14 13:38 [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms Thomas Abraham
` (3 preceding siblings ...)
2014-07-14 13:38 ` [PATCH v7 4/6] ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data Thomas Abraham
@ 2014-07-14 13:38 ` Thomas Abraham
2014-07-19 13:27 ` Tomasz Figa
2014-07-14 13:38 ` [PATCH v7 6/6] cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support Thomas Abraham
` (2 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Thomas Abraham @ 2014-07-14 13:38 UTC (permalink / raw)
To: linux-arm-kernel
From: Thomas Abraham <thomas.ab@samsung.com>
Remove the platform device instantiation for exynos cpufreq driver and add the
platform device for generic cpufreq drivers.
Cc: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
Tested-by: Arjun K.V <arjun.kv@samsung.com>
---
arch/arm/mach-exynos/exynos.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 2a43a17..5028b35 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -183,7 +183,20 @@ void __init exynos_cpuidle_init(void)
void __init exynos_cpufreq_init(void)
{
- platform_device_register_simple("exynos-cpufreq", -1, NULL, 0);
+ char *dev_name;
+
+ if (of_machine_is_compatible("samsung,exynos5440"))
+ return;
+ if (of_machine_is_compatible("samsung,exynos5420"))
+ dev_name = "arm-bL-cpufreq-dt";
+ else
+ if (of_machine_is_compatible("samsung,exynos4412") ||
+ of_machine_is_compatible("samsung,exynos4212"))
+ dev_name = "exynos-cpufreq";
+ else
+ dev_name = "cpufreq-cpu0";
+
+ platform_device_register_simple(dev_name, -1, NULL, 0);
}
void __iomem *sysram_base_addr;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v7 5/6] ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250/5420
2014-07-14 13:38 ` [PATCH v7 5/6] ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250/5420 Thomas Abraham
@ 2014-07-19 13:27 ` Tomasz Figa
2014-07-29 5:37 ` Thomas Abraham
0 siblings, 1 reply; 25+ messages in thread
From: Tomasz Figa @ 2014-07-19 13:27 UTC (permalink / raw)
To: linux-arm-kernel
On 14.07.2014 15:38, Thomas Abraham wrote:
> From: Thomas Abraham <thomas.ab@samsung.com>
>
> Remove the platform device instantiation for exynos cpufreq driver and add the
> platform device for generic cpufreq drivers.
>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Reviewed-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> Tested-by: Arjun K.V <arjun.kv@samsung.com>
> ---
> arch/arm/mach-exynos/exynos.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index 2a43a17..5028b35 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -183,7 +183,20 @@ void __init exynos_cpuidle_init(void)
>
> void __init exynos_cpufreq_init(void)
> {
> - platform_device_register_simple("exynos-cpufreq", -1, NULL, 0);
> + char *dev_name;
> +
> + if (of_machine_is_compatible("samsung,exynos5440"))
> + return;
The original code registers the device unconditionally. Why Exynos5440
is excluded now?
> + if (of_machine_is_compatible("samsung,exynos5420"))
> + dev_name = "arm-bL-cpufreq-dt";
> + else
> + if (of_machine_is_compatible("samsung,exynos4412") ||
> + of_machine_is_compatible("samsung,exynos4212"))
> + dev_name = "exynos-cpufreq";
> + else
> + dev_name = "cpufreq-cpu0";
> +
> + platform_device_register_simple(dev_name, -1, NULL, 0);
> }
How about rewriting this to:
static const struct of_device_id exynos_cpufreq_matches[] = {
{ .compatible = "samsung,exynos5420",
.data = "arm-bL-cpufreq-dt" },
{ .compatible = "samsung,exynos5250",
.data = "cpufreq-cpu0" },
{ .compatible = "samsung,exynos4210",
.data = "cpufreq-cpu0" },
{ /* sentinel */ }
};
void __init exynos_cpufreq_init(void)
{
struct device_node *root = of_find_node_by_path("/");
const struct of_device_id *match;
match = of_match_node(exynos_cpufreq_matches, root);
if (!match) {
platform_device_register_simple("exynos-cpufreq", -1,
NULL, 0);
return;
}
platform_device_register_simple(match->data, -1, NULL, 0);
}
This way it is much more readable and original behavior is preserved for
any SoCs not supported by new drivers.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v7 5/6] ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250/5420
2014-07-19 13:27 ` Tomasz Figa
@ 2014-07-29 5:37 ` Thomas Abraham
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Abraham @ 2014-07-29 5:37 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Jul 19, 2014 at 6:57 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On 14.07.2014 15:38, Thomas Abraham wrote:
>> From: Thomas Abraham <thomas.ab@samsung.com>
>>
>> Remove the platform device instantiation for exynos cpufreq driver and add the
>> platform device for generic cpufreq drivers.
>>
>> Cc: Kukjin Kim <kgene.kim@samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>> Reviewed-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> Tested-by: Arjun K.V <arjun.kv@samsung.com>
>> ---
>> arch/arm/mach-exynos/exynos.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
>> index 2a43a17..5028b35 100644
>> --- a/arch/arm/mach-exynos/exynos.c
>> +++ b/arch/arm/mach-exynos/exynos.c
>> @@ -183,7 +183,20 @@ void __init exynos_cpuidle_init(void)
>>
>> void __init exynos_cpufreq_init(void)
>> {
>> - platform_device_register_simple("exynos-cpufreq", -1, NULL, 0);
>> + char *dev_name;
>> +
>> + if (of_machine_is_compatible("samsung,exynos5440"))
>> + return;
>
> The original code registers the device unconditionally. Why Exynos5440
> is excluded now?
>
>> + if (of_machine_is_compatible("samsung,exynos5420"))
>> + dev_name = "arm-bL-cpufreq-dt";
>> + else
>> + if (of_machine_is_compatible("samsung,exynos4412") ||
>> + of_machine_is_compatible("samsung,exynos4212"))
>> + dev_name = "exynos-cpufreq";
>> + else
>> + dev_name = "cpufreq-cpu0";
>> +
>> + platform_device_register_simple(dev_name, -1, NULL, 0);
>> }
>
> How about rewriting this to:
>
> static const struct of_device_id exynos_cpufreq_matches[] = {
> { .compatible = "samsung,exynos5420",
> .data = "arm-bL-cpufreq-dt" },
> { .compatible = "samsung,exynos5250",
> .data = "cpufreq-cpu0" },
> { .compatible = "samsung,exynos4210",
> .data = "cpufreq-cpu0" },
> { /* sentinel */ }
> };
>
> void __init exynos_cpufreq_init(void)
> {
> struct device_node *root = of_find_node_by_path("/");
> const struct of_device_id *match;
>
> match = of_match_node(exynos_cpufreq_matches, root);
> if (!match) {
> platform_device_register_simple("exynos-cpufreq", -1,
> NULL, 0);
> return;
> }
>
> platform_device_register_simple(match->data, -1, NULL, 0);
> }
>
> This way it is much more readable and original behavior is preserved for
> any SoCs not supported by new drivers.
Thanks for the suggestion Tomasz.
>
> Best regards,
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v7 6/6] cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support
2014-07-14 13:38 [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms Thomas Abraham
` (4 preceding siblings ...)
2014-07-14 13:38 ` [PATCH v7 5/6] ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250/5420 Thomas Abraham
@ 2014-07-14 13:38 ` Thomas Abraham
2014-07-19 13:29 ` Tomasz Figa
2014-07-15 3:50 ` [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms Thomas Abraham
2014-07-19 19:20 ` Tobias Jakobi
7 siblings, 1 reply; 25+ messages in thread
From: Thomas Abraham @ 2014-07-14 13:38 UTC (permalink / raw)
To: linux-arm-kernel
From: Thomas Abraham <thomas.ab@samsung.com>
Exynos4210 and Exynos5250 based platforms have switched over to use generic
cpufreq drivers for cpufreq functionality. So the Exynos specific cpufreq
drivers for these platforms can be removed.
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
Tested-by: Arjun K.V <arjun.kv@samsung.com>
---
drivers/cpufreq/Kconfig.arm | 22 ----
drivers/cpufreq/Makefile | 2 -
drivers/cpufreq/exynos4210-cpufreq.c | 184 -----------------------------
drivers/cpufreq/exynos5250-cpufreq.c | 210 ----------------------------------
4 files changed, 418 deletions(-)
delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c
delete mode 100644 drivers/cpufreq/exynos5250-cpufreq.c
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index ebac671..7a2f289 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -28,17 +28,6 @@ config ARM_VEXPRESS_SPC_CPUFREQ
config ARM_EXYNOS_CPUFREQ
bool
-config ARM_EXYNOS4210_CPUFREQ
- bool "SAMSUNG EXYNOS4210"
- depends on CPU_EXYNOS4210
- default y
- select ARM_EXYNOS_CPUFREQ
- help
- This adds the CPUFreq driver for Samsung EXYNOS4210
- SoC (S5PV310 or S5PC210).
-
- If in doubt, say N.
-
config ARM_EXYNOS4X12_CPUFREQ
bool "SAMSUNG EXYNOS4x12"
depends on SOC_EXYNOS4212 || SOC_EXYNOS4412
@@ -50,17 +39,6 @@ config ARM_EXYNOS4X12_CPUFREQ
If in doubt, say N.
-config ARM_EXYNOS5250_CPUFREQ
- bool "SAMSUNG EXYNOS5250"
- depends on SOC_EXYNOS5250
- default y
- select ARM_EXYNOS_CPUFREQ
- help
- This adds the CPUFreq driver for Samsung EXYNOS5250
- SoC.
-
- If in doubt, say N.
-
config ARM_EXYNOS5440_CPUFREQ
bool "SAMSUNG EXYNOS5440"
depends on SOC_EXYNOS5440
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index db6d9a2..fac36ef 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -52,9 +52,7 @@ obj-$(CONFIG_ARM_DT_BL_CPUFREQ) += arm_big_little_dt.o
obj-$(CONFIG_ARCH_DAVINCI) += davinci-cpufreq.o
obj-$(CONFIG_UX500_SOC_DB8500) += dbx500-cpufreq.o
obj-$(CONFIG_ARM_EXYNOS_CPUFREQ) += exynos-cpufreq.o
-obj-$(CONFIG_ARM_EXYNOS4210_CPUFREQ) += exynos4210-cpufreq.o
obj-$(CONFIG_ARM_EXYNOS4X12_CPUFREQ) += exynos4x12-cpufreq.o
-obj-$(CONFIG_ARM_EXYNOS5250_CPUFREQ) += exynos5250-cpufreq.o
obj-$(CONFIG_ARM_EXYNOS5440_CPUFREQ) += exynos5440-cpufreq.o
obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ) += highbank-cpufreq.o
obj-$(CONFIG_ARM_IMX6Q_CPUFREQ) += imx6q-cpufreq.o
diff --git a/drivers/cpufreq/exynos4210-cpufreq.c b/drivers/cpufreq/exynos4210-cpufreq.c
deleted file mode 100644
index 61a5431..0000000
--- a/drivers/cpufreq/exynos4210-cpufreq.c
+++ /dev/null
@@ -1,184 +0,0 @@
-/*
- * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
- * http://www.samsung.com
- *
- * EXYNOS4210 - CPU frequency scaling support
- *
- * 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.
-*/
-
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/err.h>
-#include <linux/clk.h>
-#include <linux/io.h>
-#include <linux/slab.h>
-#include <linux/cpufreq.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-
-#include "exynos-cpufreq.h"
-
-static struct clk *cpu_clk;
-static struct clk *moutcore;
-static struct clk *mout_mpll;
-static struct clk *mout_apll;
-static struct exynos_dvfs_info *cpufreq;
-
-static unsigned int exynos4210_volt_table[] = {
- 1250000, 1150000, 1050000, 975000, 950000,
-};
-
-static struct cpufreq_frequency_table exynos4210_freq_table[] = {
- {0, L0, 1200 * 1000},
- {0, L1, 1000 * 1000},
- {0, L2, 800 * 1000},
- {0, L3, 500 * 1000},
- {0, L4, 200 * 1000},
- {0, 0, CPUFREQ_TABLE_END},
-};
-
-static struct apll_freq apll_freq_4210[] = {
- /*
- * values:
- * freq
- * clock divider for CORE, COREM0, COREM1, PERIPH, ATB, PCLK_DBG, APLL, RESERVED
- * clock divider for COPY, HPM, RESERVED
- * PLL M, P, S
- */
- APLL_FREQ(1200, 0, 3, 7, 3, 4, 1, 7, 0, 5, 0, 0, 150, 3, 1),
- APLL_FREQ(1000, 0, 3, 7, 3, 4, 1, 7, 0, 4, 0, 0, 250, 6, 1),
- APLL_FREQ(800, 0, 3, 7, 3, 3, 1, 7, 0, 3, 0, 0, 200, 6, 1),
- APLL_FREQ(500, 0, 3, 7, 3, 3, 1, 7, 0, 3, 0, 0, 250, 6, 2),
- APLL_FREQ(200, 0, 1, 3, 1, 3, 1, 0, 0, 3, 0, 0, 200, 6, 3),
-};
-
-static void exynos4210_set_clkdiv(unsigned int div_index)
-{
- unsigned int tmp;
-
- /* Change Divider - CPU0 */
-
- tmp = apll_freq_4210[div_index].clk_div_cpu0;
-
- __raw_writel(tmp, cpufreq->cmu_regs + EXYNOS4_CLKDIV_CPU);
-
- do {
- tmp = __raw_readl(cpufreq->cmu_regs + EXYNOS4_CLKDIV_STATCPU);
- } while (tmp & 0x1111111);
-
- /* Change Divider - CPU1 */
-
- tmp = apll_freq_4210[div_index].clk_div_cpu1;
-
- __raw_writel(tmp, cpufreq->cmu_regs + EXYNOS4_CLKDIV_CPU1);
-
- do {
- tmp = __raw_readl(cpufreq->cmu_regs + EXYNOS4_CLKDIV_STATCPU1);
- } while (tmp & 0x11);
-}
-
-static void exynos4210_set_apll(unsigned int index)
-{
- unsigned int tmp, freq = apll_freq_4210[index].freq;
-
- /* MUX_CORE_SEL = MPLL, ARMCLK uses MPLL for lock time */
- clk_set_parent(moutcore, mout_mpll);
-
- do {
- tmp = (__raw_readl(cpufreq->cmu_regs + EXYNOS4_CLKMUX_STATCPU)
- >> EXYNOS4_CLKSRC_CPU_MUXCORE_SHIFT);
- tmp &= 0x7;
- } while (tmp != 0x2);
-
- clk_set_rate(mout_apll, freq * 1000);
-
- /* MUX_CORE_SEL = APLL */
- clk_set_parent(moutcore, mout_apll);
-
- do {
- tmp = __raw_readl(cpufreq->cmu_regs + EXYNOS4_CLKMUX_STATCPU);
- tmp &= EXYNOS4_CLKMUX_STATCPU_MUXCORE_MASK;
- } while (tmp != (0x1 << EXYNOS4_CLKSRC_CPU_MUXCORE_SHIFT));
-}
-
-static void exynos4210_set_frequency(unsigned int old_index,
- unsigned int new_index)
-{
- if (old_index > new_index) {
- exynos4210_set_clkdiv(new_index);
- exynos4210_set_apll(new_index);
- } else if (old_index < new_index) {
- exynos4210_set_apll(new_index);
- exynos4210_set_clkdiv(new_index);
- }
-}
-
-int exynos4210_cpufreq_init(struct exynos_dvfs_info *info)
-{
- struct device_node *np;
- unsigned long rate;
-
- /*
- * HACK: This is a temporary workaround to get access to clock
- * controller registers directly and remove static mappings and
- * dependencies on platform headers. It is necessary to enable
- * Exynos multi-platform support and will be removed together with
- * this whole driver as soon as Exynos gets migrated to use
- * cpufreq-cpu0 driver.
- */
- np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-clock");
- if (!np) {
- pr_err("%s: failed to find clock controller DT node\n",
- __func__);
- return -ENODEV;
- }
-
- info->cmu_regs = of_iomap(np, 0);
- if (!info->cmu_regs) {
- pr_err("%s: failed to map CMU registers\n", __func__);
- return -EFAULT;
- }
-
- cpu_clk = clk_get(NULL, "armclk");
- if (IS_ERR(cpu_clk))
- return PTR_ERR(cpu_clk);
-
- moutcore = clk_get(NULL, "moutcore");
- if (IS_ERR(moutcore))
- goto err_moutcore;
-
- mout_mpll = clk_get(NULL, "mout_mpll");
- if (IS_ERR(mout_mpll))
- goto err_mout_mpll;
-
- rate = clk_get_rate(mout_mpll) / 1000;
-
- mout_apll = clk_get(NULL, "mout_apll");
- if (IS_ERR(mout_apll))
- goto err_mout_apll;
-
- info->mpll_freq_khz = rate;
- /* 800Mhz */
- info->pll_safe_idx = L2;
- info->cpu_clk = cpu_clk;
- info->volt_table = exynos4210_volt_table;
- info->freq_table = exynos4210_freq_table;
- info->set_freq = exynos4210_set_frequency;
-
- cpufreq = info;
-
- return 0;
-
-err_mout_apll:
- clk_put(mout_mpll);
-err_mout_mpll:
- clk_put(moutcore);
-err_moutcore:
- clk_put(cpu_clk);
-
- pr_debug("%s: failed initialization\n", __func__);
- return -EINVAL;
-}
diff --git a/drivers/cpufreq/exynos5250-cpufreq.c b/drivers/cpufreq/exynos5250-cpufreq.c
deleted file mode 100644
index c91ce69..0000000
--- a/drivers/cpufreq/exynos5250-cpufreq.c
+++ /dev/null
@@ -1,210 +0,0 @@
-/*
- * Copyright (c) 2010-20122Samsung Electronics Co., Ltd.
- * http://www.samsung.com
- *
- * EXYNOS5250 - CPU frequency scaling support
- *
- * 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.
-*/
-
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/err.h>
-#include <linux/clk.h>
-#include <linux/io.h>
-#include <linux/slab.h>
-#include <linux/cpufreq.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-
-#include "exynos-cpufreq.h"
-
-static struct clk *cpu_clk;
-static struct clk *moutcore;
-static struct clk *mout_mpll;
-static struct clk *mout_apll;
-static struct exynos_dvfs_info *cpufreq;
-
-static unsigned int exynos5250_volt_table[] = {
- 1300000, 1250000, 1225000, 1200000, 1150000,
- 1125000, 1100000, 1075000, 1050000, 1025000,
- 1012500, 1000000, 975000, 950000, 937500,
- 925000
-};
-
-static struct cpufreq_frequency_table exynos5250_freq_table[] = {
- {0, L0, 1700 * 1000},
- {0, L1, 1600 * 1000},
- {0, L2, 1500 * 1000},
- {0, L3, 1400 * 1000},
- {0, L4, 1300 * 1000},
- {0, L5, 1200 * 1000},
- {0, L6, 1100 * 1000},
- {0, L7, 1000 * 1000},
- {0, L8, 900 * 1000},
- {0, L9, 800 * 1000},
- {0, L10, 700 * 1000},
- {0, L11, 600 * 1000},
- {0, L12, 500 * 1000},
- {0, L13, 400 * 1000},
- {0, L14, 300 * 1000},
- {0, L15, 200 * 1000},
- {0, 0, CPUFREQ_TABLE_END},
-};
-
-static struct apll_freq apll_freq_5250[] = {
- /*
- * values:
- * freq
- * clock divider for ARM, CPUD, ACP, PERIPH, ATB, PCLK_DBG, APLL, ARM2
- * clock divider for COPY, HPM, RESERVED
- * PLL M, P, S
- */
- APLL_FREQ(1700, 0, 3, 7, 7, 7, 3, 5, 0, 0, 2, 0, 425, 6, 0),
- APLL_FREQ(1600, 0, 3, 7, 7, 7, 1, 4, 0, 0, 2, 0, 200, 3, 0),
- APLL_FREQ(1500, 0, 2, 7, 7, 7, 1, 4, 0, 0, 2, 0, 250, 4, 0),
- APLL_FREQ(1400, 0, 2, 7, 7, 6, 1, 4, 0, 0, 2, 0, 175, 3, 0),
- APLL_FREQ(1300, 0, 2, 7, 7, 6, 1, 3, 0, 0, 2, 0, 325, 6, 0),
- APLL_FREQ(1200, 0, 2, 7, 7, 5, 1, 3, 0, 0, 2, 0, 200, 4, 0),
- APLL_FREQ(1100, 0, 3, 7, 7, 5, 1, 3, 0, 0, 2, 0, 275, 6, 0),
- APLL_FREQ(1000, 0, 1, 7, 7, 4, 1, 2, 0, 0, 2, 0, 125, 3, 0),
- APLL_FREQ(900, 0, 1, 7, 7, 4, 1, 2, 0, 0, 2, 0, 150, 4, 0),
- APLL_FREQ(800, 0, 1, 7, 7, 4, 1, 2, 0, 0, 2, 0, 100, 3, 0),
- APLL_FREQ(700, 0, 1, 7, 7, 3, 1, 1, 0, 0, 2, 0, 175, 3, 1),
- APLL_FREQ(600, 0, 1, 7, 7, 3, 1, 1, 0, 0, 2, 0, 200, 4, 1),
- APLL_FREQ(500, 0, 1, 7, 7, 2, 1, 1, 0, 0, 2, 0, 125, 3, 1),
- APLL_FREQ(400, 0, 1, 7, 7, 2, 1, 1, 0, 0, 2, 0, 100, 3, 1),
- APLL_FREQ(300, 0, 1, 7, 7, 1, 1, 1, 0, 0, 2, 0, 200, 4, 2),
- APLL_FREQ(200, 0, 1, 7, 7, 1, 1, 1, 0, 0, 2, 0, 100, 3, 2),
-};
-
-static void set_clkdiv(unsigned int div_index)
-{
- unsigned int tmp;
-
- /* Change Divider - CPU0 */
-
- tmp = apll_freq_5250[div_index].clk_div_cpu0;
-
- __raw_writel(tmp, cpufreq->cmu_regs + EXYNOS5_CLKDIV_CPU0);
-
- while (__raw_readl(cpufreq->cmu_regs + EXYNOS5_CLKDIV_STATCPU0)
- & 0x11111111)
- cpu_relax();
-
- /* Change Divider - CPU1 */
- tmp = apll_freq_5250[div_index].clk_div_cpu1;
-
- __raw_writel(tmp, cpufreq->cmu_regs + EXYNOS5_CLKDIV_CPU1);
-
- while (__raw_readl(cpufreq->cmu_regs + EXYNOS5_CLKDIV_STATCPU1) & 0x11)
- cpu_relax();
-}
-
-static void set_apll(unsigned int index)
-{
- unsigned int tmp;
- unsigned int freq = apll_freq_5250[index].freq;
-
- /* MUX_CORE_SEL = MPLL, ARMCLK uses MPLL for lock time */
- clk_set_parent(moutcore, mout_mpll);
-
- do {
- cpu_relax();
- tmp = (__raw_readl(cpufreq->cmu_regs + EXYNOS5_CLKMUX_STATCPU)
- >> 16);
- tmp &= 0x7;
- } while (tmp != 0x2);
-
- clk_set_rate(mout_apll, freq * 1000);
-
- /* MUX_CORE_SEL = APLL */
- clk_set_parent(moutcore, mout_apll);
-
- do {
- cpu_relax();
- tmp = __raw_readl(cpufreq->cmu_regs + EXYNOS5_CLKMUX_STATCPU);
- tmp &= (0x7 << 16);
- } while (tmp != (0x1 << 16));
-}
-
-static void exynos5250_set_frequency(unsigned int old_index,
- unsigned int new_index)
-{
- if (old_index > new_index) {
- set_clkdiv(new_index);
- set_apll(new_index);
- } else if (old_index < new_index) {
- set_apll(new_index);
- set_clkdiv(new_index);
- }
-}
-
-int exynos5250_cpufreq_init(struct exynos_dvfs_info *info)
-{
- struct device_node *np;
- unsigned long rate;
-
- /*
- * HACK: This is a temporary workaround to get access to clock
- * controller registers directly and remove static mappings and
- * dependencies on platform headers. It is necessary to enable
- * Exynos multi-platform support and will be removed together with
- * this whole driver as soon as Exynos gets migrated to use
- * cpufreq-cpu0 driver.
- */
- np = of_find_compatible_node(NULL, NULL, "samsung,exynos5250-clock");
- if (!np) {
- pr_err("%s: failed to find clock controller DT node\n",
- __func__);
- return -ENODEV;
- }
-
- info->cmu_regs = of_iomap(np, 0);
- if (!info->cmu_regs) {
- pr_err("%s: failed to map CMU registers\n", __func__);
- return -EFAULT;
- }
-
- cpu_clk = clk_get(NULL, "armclk");
- if (IS_ERR(cpu_clk))
- return PTR_ERR(cpu_clk);
-
- moutcore = clk_get(NULL, "mout_cpu");
- if (IS_ERR(moutcore))
- goto err_moutcore;
-
- mout_mpll = clk_get(NULL, "mout_mpll");
- if (IS_ERR(mout_mpll))
- goto err_mout_mpll;
-
- rate = clk_get_rate(mout_mpll) / 1000;
-
- mout_apll = clk_get(NULL, "mout_apll");
- if (IS_ERR(mout_apll))
- goto err_mout_apll;
-
- info->mpll_freq_khz = rate;
- /* 800Mhz */
- info->pll_safe_idx = L9;
- info->cpu_clk = cpu_clk;
- info->volt_table = exynos5250_volt_table;
- info->freq_table = exynos5250_freq_table;
- info->set_freq = exynos5250_set_frequency;
-
- cpufreq = info;
-
- return 0;
-
-err_mout_apll:
- clk_put(mout_mpll);
-err_mout_mpll:
- clk_put(moutcore);
-err_moutcore:
- clk_put(cpu_clk);
-
- pr_err("%s: failed initialization\n", __func__);
- return -EINVAL;
-}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v7 6/6] cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support
2014-07-14 13:38 ` [PATCH v7 6/6] cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support Thomas Abraham
@ 2014-07-19 13:29 ` Tomasz Figa
2014-07-29 5:53 ` Thomas Abraham
0 siblings, 1 reply; 25+ messages in thread
From: Tomasz Figa @ 2014-07-19 13:29 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 14.07.2014 15:38, Thomas Abraham wrote:
> From: Thomas Abraham <thomas.ab@samsung.com>
>
> Exynos4210 and Exynos5250 based platforms have switched over to use generic
> cpufreq drivers for cpufreq functionality. So the Exynos specific cpufreq
> drivers for these platforms can be removed.
>
> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Reviewed-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> Tested-by: Arjun K.V <arjun.kv@samsung.com>
> ---
> drivers/cpufreq/Kconfig.arm | 22 ----
> drivers/cpufreq/Makefile | 2 -
> drivers/cpufreq/exynos4210-cpufreq.c | 184 -----------------------------
> drivers/cpufreq/exynos5250-cpufreq.c | 210 ----------------------------------
> 4 files changed, 418 deletions(-)
> delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c
> delete mode 100644 drivers/cpufreq/exynos5250-cpufreq.c
>
I would hesitate with removing this driver straightaway and replacing it
with a new one that isn't yet well tested. For at least one release I
would keep the old code in place with the possibility of switching
between old and new one in Kconfig.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v7 6/6] cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support
2014-07-19 13:29 ` Tomasz Figa
@ 2014-07-29 5:53 ` Thomas Abraham
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Abraham @ 2014-07-29 5:53 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
On Sat, Jul 19, 2014 at 6:59 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi,
>
> On 14.07.2014 15:38, Thomas Abraham wrote:
>> From: Thomas Abraham <thomas.ab@samsung.com>
>>
>> Exynos4210 and Exynos5250 based platforms have switched over to use generic
>> cpufreq drivers for cpufreq functionality. So the Exynos specific cpufreq
>> drivers for these platforms can be removed.
>>
>> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>> Reviewed-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> Tested-by: Arjun K.V <arjun.kv@samsung.com>
>> ---
>> drivers/cpufreq/Kconfig.arm | 22 ----
>> drivers/cpufreq/Makefile | 2 -
>> drivers/cpufreq/exynos4210-cpufreq.c | 184 -----------------------------
>> drivers/cpufreq/exynos5250-cpufreq.c | 210 ----------------------------------
>> 4 files changed, 418 deletions(-)
>> delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c
>> delete mode 100644 drivers/cpufreq/exynos5250-cpufreq.c
>>
>
> I would hesitate with removing this driver straightaway and replacing it
> with a new one that isn't yet well tested. For at least one release I
> would keep the old code in place with the possibility of switching
> between old and new one in Kconfig.
The new drivers have been tested for three days on
Exynos4210/5250/5420 based boards. So I am fairly confident that there
will be no issues in switching to the generic drivers and the old
drivers have not been retained in the next version.
Thanks,
Thomas.
>
> Best regards,
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms
2014-07-14 13:38 [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms Thomas Abraham
` (5 preceding siblings ...)
2014-07-14 13:38 ` [PATCH v7 6/6] cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support Thomas Abraham
@ 2014-07-15 3:50 ` Thomas Abraham
2014-07-17 5:58 ` Thomas Abraham
2014-07-19 19:20 ` Tobias Jakobi
7 siblings, 1 reply; 25+ messages in thread
From: Thomas Abraham @ 2014-07-15 3:50 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
On Mon, Jul 14, 2014 at 7:08 PM, Thomas Abraham <thomas.ab@samsung.com> wrote:
> Changes since v6:
> - Fixes suggested by Amit Daniel <amit.daniel@samsung.com>.
>
> This patch series removes the use of Exynos4210 and Exynos5250 specific cpufreq
> drivers and enables the use of cpufreq-cpu0 driver for these platforms. This
> series also enabled cpufreq support for Exynos5420 using arm_big_little cpufreq
> driver.
>
> Thomas Abraham (6):
> clk: samsung: add infrastructure to register cpu clocks
> clk: samsung: register exynos5420 apll/kpll configuration data
> clk: exynos: use cpu-clock provider type to represent arm clock
> ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data
> ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250
> cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support
In this series, support for Exynos5420 has been included. If there are
any more changes required, could you please let me know.
Thanks,
Thomas.
>
> arch/arm/boot/dts/exynos4210-origen.dts | 6 +
> arch/arm/boot/dts/exynos4210-trats.dts | 6 +
> arch/arm/boot/dts/exynos4210-universal_c210.dts | 6 +
> arch/arm/boot/dts/exynos4210.dtsi | 27 ++
> arch/arm/boot/dts/exynos5250-arndale.dts | 6 +
> arch/arm/boot/dts/exynos5250-cros-common.dtsi | 6 +
> arch/arm/boot/dts/exynos5250-smdk5250.dts | 6 +
> arch/arm/boot/dts/exynos5250.dtsi | 23 +
> arch/arm/boot/dts/exynos5420-smdk5420.dts | 6 +
> arch/arm/boot/dts/exynos5420.dtsi | 32 ++
> arch/arm/mach-exynos/exynos.c | 15 +-
> drivers/clk/samsung/Makefile | 2 +-
> drivers/clk/samsung/clk-cpu.c | 576 +++++++++++++++++++++++
> drivers/clk/samsung/clk-exynos4.c | 25 +-
> drivers/clk/samsung/clk-exynos5250.c | 16 +-
> drivers/clk/samsung/clk-exynos5420.c | 60 ++-
> drivers/clk/samsung/clk.h | 5 +
> drivers/cpufreq/Kconfig.arm | 22 -
> drivers/cpufreq/Makefile | 2 -
> drivers/cpufreq/exynos4210-cpufreq.c | 184 --------
> drivers/cpufreq/exynos5250-cpufreq.c | 210 ---------
> include/dt-bindings/clock/exynos5250.h | 1 +
> include/dt-bindings/clock/exynos5420.h | 2 +
> 23 files changed, 802 insertions(+), 442 deletions(-)
> create mode 100644 drivers/clk/samsung/clk-cpu.c
> delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c
> delete mode 100644 drivers/cpufreq/exynos5250-cpufreq.c
>
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms
2014-07-15 3:50 ` [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms Thomas Abraham
@ 2014-07-17 5:58 ` Thomas Abraham
2014-07-18 0:44 ` Chanwoo Choi
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Abraham @ 2014-07-17 5:58 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mike,
On Tue, Jul 15, 2014 at 9:20 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
> Hi Tomasz,
>
> On Mon, Jul 14, 2014 at 7:08 PM, Thomas Abraham <thomas.ab@samsung.com> wrote:
>> Changes since v6:
>> - Fixes suggested by Amit Daniel <amit.daniel@samsung.com>.
>>
>> This patch series removes the use of Exynos4210 and Exynos5250 specific cpufreq
>> drivers and enables the use of cpufreq-cpu0 driver for these platforms. This
>> series also enabled cpufreq support for Exynos5420 using arm_big_little cpufreq
>> driver.
>>
>> Thomas Abraham (6):
>> clk: samsung: add infrastructure to register cpu clocks
>> clk: samsung: register exynos5420 apll/kpll configuration data
>> clk: exynos: use cpu-clock provider type to represent arm clock
>> ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data
>> ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250
>> cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support
>
> In this series, support for Exynos5420 has been included. If there are
> any more changes required, could you please let me know.
This patch series migrates exynos cpufreq support to use generic
cpufreq drivers. For this, cpu clock blocks are encapsulated into a
cpu clock type and cpufreq driver operates the cpu clock type.
We had discussed in the [1] about using coordinated clocks. This
series is not using the concept of coordinated clocks but the code in
this series can be migrated to use coordinated clocks when it is
available.
Is it okay to consider this series for v3.17-rc1. I will update this
code to use coordinated clocks when it is available. This series helps
with cpufreq support for newer Exynos SoCs such as Exynos5420/3250.
[1] http://www.spinics.net/lists/cpufreq/msg10042.html
Thanks,
Thomas
>
> Thanks,
> Thomas.
>
>>
>> arch/arm/boot/dts/exynos4210-origen.dts | 6 +
>> arch/arm/boot/dts/exynos4210-trats.dts | 6 +
>> arch/arm/boot/dts/exynos4210-universal_c210.dts | 6 +
>> arch/arm/boot/dts/exynos4210.dtsi | 27 ++
>> arch/arm/boot/dts/exynos5250-arndale.dts | 6 +
>> arch/arm/boot/dts/exynos5250-cros-common.dtsi | 6 +
>> arch/arm/boot/dts/exynos5250-smdk5250.dts | 6 +
>> arch/arm/boot/dts/exynos5250.dtsi | 23 +
>> arch/arm/boot/dts/exynos5420-smdk5420.dts | 6 +
>> arch/arm/boot/dts/exynos5420.dtsi | 32 ++
>> arch/arm/mach-exynos/exynos.c | 15 +-
>> drivers/clk/samsung/Makefile | 2 +-
>> drivers/clk/samsung/clk-cpu.c | 576 +++++++++++++++++++++++
>> drivers/clk/samsung/clk-exynos4.c | 25 +-
>> drivers/clk/samsung/clk-exynos5250.c | 16 +-
>> drivers/clk/samsung/clk-exynos5420.c | 60 ++-
>> drivers/clk/samsung/clk.h | 5 +
>> drivers/cpufreq/Kconfig.arm | 22 -
>> drivers/cpufreq/Makefile | 2 -
>> drivers/cpufreq/exynos4210-cpufreq.c | 184 --------
>> drivers/cpufreq/exynos5250-cpufreq.c | 210 ---------
>> include/dt-bindings/clock/exynos5250.h | 1 +
>> include/dt-bindings/clock/exynos5420.h | 2 +
>> 23 files changed, 802 insertions(+), 442 deletions(-)
>> create mode 100644 drivers/clk/samsung/clk-cpu.c
>> delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c
>> delete mode 100644 drivers/cpufreq/exynos5250-cpufreq.c
>>
>> --
>> 1.7.9.5
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms
2014-07-17 5:58 ` Thomas Abraham
@ 2014-07-18 0:44 ` Chanwoo Choi
2014-07-18 2:04 ` Thomas Abraham
0 siblings, 1 reply; 25+ messages in thread
From: Chanwoo Choi @ 2014-07-18 0:44 UTC (permalink / raw)
To: linux-arm-kernel
Hi Thomas,
On 07/17/2014 02:58 PM, Thomas Abraham wrote:
> Hi Mike,
>
> On Tue, Jul 15, 2014 at 9:20 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
>> Hi Tomasz,
>>
>> On Mon, Jul 14, 2014 at 7:08 PM, Thomas Abraham <thomas.ab@samsung.com> wrote:
>>> Changes since v6:
>>> - Fixes suggested by Amit Daniel <amit.daniel@samsung.com>.
>>>
>>> This patch series removes the use of Exynos4210 and Exynos5250 specific cpufreq
>>> drivers and enables the use of cpufreq-cpu0 driver for these platforms. This
>>> series also enabled cpufreq support for Exynos5420 using arm_big_little cpufreq
>>> driver.
>>>
>>> Thomas Abraham (6):
>>> clk: samsung: add infrastructure to register cpu clocks
>>> clk: samsung: register exynos5420 apll/kpll configuration data
>>> clk: exynos: use cpu-clock provider type to represent arm clock
>>> ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data
>>> ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250
>>> cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support
>>
>> In this series, support for Exynos5420 has been included. If there are
>> any more changes required, could you please let me know.
>
> This patch series migrates exynos cpufreq support to use generic
> cpufreq drivers. For this, cpu clock blocks are encapsulated into a
> cpu clock type and cpufreq driver operates the cpu clock type.
>
> We had discussed in the [1] about using coordinated clocks. This
> series is not using the concept of coordinated clocks but the code in
> this series can be migrated to use coordinated clocks when it is
> available.
>
> Is it okay to consider this series for v3.17-rc1. I will update this
> code to use coordinated clocks when it is available. This series helps
> with cpufreq support for newer Exynos SoCs such as Exynos5420/3250.
>
> [1] http://www.spinics.net/lists/cpufreq/msg10042.html
>
I tested this patchset with Exynos3250 cpufreq patchset[1] on Exynos3250 SoC.
[1] https://lkml.org/lkml/2014/6/18/126
Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
Best Regards,
Chanwoo Choi
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms
2014-07-18 0:44 ` Chanwoo Choi
@ 2014-07-18 2:04 ` Thomas Abraham
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Abraham @ 2014-07-18 2:04 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jul 18, 2014 at 6:14 AM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> Hi Thomas,
>
> On 07/17/2014 02:58 PM, Thomas Abraham wrote:
>> Hi Mike,
>>
>> On Tue, Jul 15, 2014 at 9:20 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
>>> Hi Tomasz,
>>>
>>> On Mon, Jul 14, 2014 at 7:08 PM, Thomas Abraham <thomas.ab@samsung.com> wrote:
>>>> Changes since v6:
>>>> - Fixes suggested by Amit Daniel <amit.daniel@samsung.com>.
>>>>
>>>> This patch series removes the use of Exynos4210 and Exynos5250 specific cpufreq
>>>> drivers and enables the use of cpufreq-cpu0 driver for these platforms. This
>>>> series also enabled cpufreq support for Exynos5420 using arm_big_little cpufreq
>>>> driver.
>>>>
>>>> Thomas Abraham (6):
>>>> clk: samsung: add infrastructure to register cpu clocks
>>>> clk: samsung: register exynos5420 apll/kpll configuration data
>>>> clk: exynos: use cpu-clock provider type to represent arm clock
>>>> ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data
>>>> ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250
>>>> cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support
>>>
>>> In this series, support for Exynos5420 has been included. If there are
>>> any more changes required, could you please let me know.
>>
>> This patch series migrates exynos cpufreq support to use generic
>> cpufreq drivers. For this, cpu clock blocks are encapsulated into a
>> cpu clock type and cpufreq driver operates the cpu clock type.
>>
>> We had discussed in the [1] about using coordinated clocks. This
>> series is not using the concept of coordinated clocks but the code in
>> this series can be migrated to use coordinated clocks when it is
>> available.
>>
>> Is it okay to consider this series for v3.17-rc1. I will update this
>> code to use coordinated clocks when it is available. This series helps
>> with cpufreq support for newer Exynos SoCs such as Exynos5420/3250.
>>
>> [1] http://www.spinics.net/lists/cpufreq/msg10042.html
>>
>
> I tested this patchset with Exynos3250 cpufreq patchset[1] on Exynos3250 SoC.
> [1] https://lkml.org/lkml/2014/6/18/126
>
> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
Hi Chanwoo,
Thanks for using these patches for Exynos3250 cpufreq support.
Regards,
Thomas.
>
> Best Regards,
> Chanwoo Choi
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms
2014-07-14 13:38 [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms Thomas Abraham
` (6 preceding siblings ...)
2014-07-15 3:50 ` [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms Thomas Abraham
@ 2014-07-19 19:20 ` Tobias Jakobi
2014-07-19 20:54 ` Tomasz Figa
7 siblings, 1 reply; 25+ messages in thread
From: Tobias Jakobi @ 2014-07-19 19:20 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
I have a question concerning an older version of this patchset, the one
which still included exynos4x12 support and where the clocking data was
provided through DT.
I tried to get this working for my ODROID-X2, which does work well,
including the usage of the boost frequences. Since the X2 has an
Exynos4412 Prime I added additional clocking data and operating points.
clocking data (comes from the vendor kernel):
<1704000 3 7 0 6 1 2 7 0 7>
<1600000 3 7 0 6 1 2 6 0 7>
operating points (again from vendor kernel):
1704000 1375000
1600000 1350000
Doing this I get correct output from scaling_available_frequencies and
scaling_boost_frequencies and the driver seems to select these
frequences. However when reading from cpufreq_cur_freq, which I think
returns the frequency of the armclk, the maximum freq never exceeds
1.5GHz. So effectively the additional clocking data and opps are not used.
I wonder this happens? Is there some kind of limit enforced?
With best wishes,
Tobias Jakobi
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms
2014-07-19 19:20 ` Tobias Jakobi
@ 2014-07-19 20:54 ` Tomasz Figa
2014-07-19 21:17 ` Tobias Jakobi
0 siblings, 1 reply; 25+ messages in thread
From: Tomasz Figa @ 2014-07-19 20:54 UTC (permalink / raw)
To: linux-arm-kernel
Tobias,
On 19.07.2014 21:20, Tobias Jakobi wrote:
> Hello,
>
> I have a question concerning an older version of this patchset, the one
> which still included exynos4x12 support and where the clocking data was
> provided through DT.
>
> I tried to get this working for my ODROID-X2, which does work well,
> including the usage of the boost frequences. Since the X2 has an
> Exynos4412 Prime I added additional clocking data and operating points.
>
> clocking data (comes from the vendor kernel):
> <1704000 3 7 0 6 1 2 7 0 7>
> <1600000 3 7 0 6 1 2 6 0 7>
>
> operating points (again from vendor kernel):
> 1704000 1375000
> 1600000 1350000
>
> Doing this I get correct output from scaling_available_frequencies and
> scaling_boost_frequencies and the driver seems to select these
> frequences. However when reading from cpufreq_cur_freq, which I think
> returns the frequency of the armclk, the maximum freq never exceeds
> 1.5GHz. So effectively the additional clocking data and opps are not used.
>
> I wonder this happens? Is there some kind of limit enforced?
Have you also extended the array of allowed APLL settings in
drivers/clk/clk-exynos4.c?
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v7 0/6] cpufreq: use generic cpufreq drivers for exynos platforms
2014-07-19 20:54 ` Tomasz Figa
@ 2014-07-19 21:17 ` Tobias Jakobi
0 siblings, 0 replies; 25+ messages in thread
From: Tobias Jakobi @ 2014-07-19 21:17 UTC (permalink / raw)
To: linux-arm-kernel
Tomasz Figa wrote:
> Tobias,
>
> On 19.07.2014 21:20, Tobias Jakobi wrote:
>> Hello,
>>
>> I have a question concerning an older version of this patchset, the one
>> which still included exynos4x12 support and where the clocking data was
>> provided through DT.
>>
>> I tried to get this working for my ODROID-X2, which does work well,
>> including the usage of the boost frequences. Since the X2 has an
>> Exynos4412 Prime I added additional clocking data and operating points.
>>
>> clocking data (comes from the vendor kernel):
>> <1704000 3 7 0 6 1 2 7 0 7>
>> <1600000 3 7 0 6 1 2 6 0 7>
>>
>> operating points (again from vendor kernel):
>> 1704000 1375000
>> 1600000 1350000
>>
>> Doing this I get correct output from scaling_available_frequencies and
>> scaling_boost_frequencies and the driver seems to select these
>> frequences. However when reading from cpufreq_cur_freq, which I think
>> returns the frequency of the armclk, the maximum freq never exceeds
>> 1.5GHz. So effectively the additional clocking data and opps are not used.
>>
>> I wonder this happens? Is there some kind of limit enforced?
>
> Have you also extended the array of allowed APLL settings in
> drivers/clk/clk-exynos4.c?
>
> Best regards,
> Tomasz
>
Thanks Tomasz, that did the trick!
With best wishes,
Tobias
^ permalink raw reply [flat|nested] 25+ messages in thread