Subject: [PATCH 0/6] cpufreq: use generic cpufreq drivers for Exynos4210 platform

Hi,

This patch series removes the use of Exynos4210 specific support
from cpufreq-exynos driver and enables the use of cpufreq-dt driver
for this platform.

It consists of a modified "[PATCH v12 0/6] cpufreq: use generic
cpufreq drivers for exynos platforms" [1] patch series from Thomas
Abraham. As there have not been updates of this patchset since
November 2014 I decided to dust it off myself. I updated Thomas'
patchset to current kernels, fixed bugs that I noticed and removed
non-Exynos4210 support for now (to speed up inclusion in upstream
kernel). Since some modifications were not trivial I dropped all
Reviewed-by:, Tested-by: and Acked-by: tags. Please review/test
this patchset and reply with revelant tag (Thank You!). I also
plan to work on Exynos4x12 support next (which was missing in
the original patchset) and then on Exynos5250/Exynos5420 one.

This patch series has been tested on Exynos4210 based Origen and
Trats boards.

Depends on:
- next-20150330 branch of linux-next kernel tree
(mfd tree contains a crucial fix for MAX8997 PMIC support)
- "[PATCH] clk: samsung: exynos4: Disable ARMCLK down feature on
Exynos4210 SoC" [2]

[1] https://www.marc.info/?l=linux-arm-kernel&m=141657611003803&w=2
[2] https://lkml.org/lkml/2015/3/27/568

Changes over Thomas' code:
- fixed issue with wrong dividers being setup by Common Clock Framework
(by an addition of CLK_RECALC_NEW_RATES clock flag to mout_apll clock,
without this change cpufreq-dt driver showed ~10 mA larger energy
consumption when compared to cpufreq-exynos one when "performance"
cpufreq governor was used on Exynos4210 SoC based Origen board), this
was probably meant to be workarounded by use of CLK_GET_RATE_NOCACHE
and CLK_DIVIDER_READ_ONLY clock flags in the original patchset (in
"[PATCH v12 6/6] clk: samsung: remove unused clock aliases and update
clock flags") but using these flags is not sufficient to fix the issue
observed
- fixed issue with setting lower dividers before the parent clock speed
was lowered (the issue resulted in lockup on Exynos4210 SoC based
Origen board when "ondemand" cpufreq governor was stress tested)
- fixed missing spin_unlock on error in exynos_cpuclk_post_rate_change()
problem by moving cfg_data search outside of the spin locked area
- removed leftover kfree() in exynos_register_cpu_clock() that could
result in dereferencing the NULL pointer on error
- moved spin_lock earlier in exynos_cpuclk_pre_rate_change() to cover
reading of E4210_SRC_CPU and E4210_DIV_CPU1 registers
- added missing "last chance" checks to wait_until_divider_stable() and
wait_until_mux_stable() (needed in case that IRQ handling took long
time to proceed and resulted in function printing incorrect error
message about timeout)
- moved E4210_CPU_DIV[0,1]() macros just before their only users,
this resulted in moving them from patch #2 to patch #3/6 ("clk:
samsung: exynos4: add cpu clock configuration data and instantiate
cpu clock")
- added my Copyrights to drivers/clk/samsung/clk-cpu.c
- updated exynos-cpufreq.[c,h]
- removed non-Exynos4210 support for now
- dropped "[PATCH v12 6/6] clk: samsung: remove unused clock aliases and
update clock flags" altogether for now

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics


Bartlomiej Zolnierkiewicz (1):
clk: add CLK_RECALC_NEW_RATES clock flag for Exynos cpu clock support

Thomas Abraham (5):
clk: samsung: add infrastructure to register cpu clocks
clk: samsung: exynos4: add cpu clock configuration data and
instantiate cpu clock
ARM: dts: Exynos4210: add CPU OPP and regulator supply property
ARM: Exynos: switch to using generic cpufreq driver for Exynos4210
cpufreq: exynos: remove Exynos4210 specific cpufreq driver support

arch/arm/boot/dts/exynos4210-origen.dts | 4 +
arch/arm/boot/dts/exynos4210-trats.dts | 4 +
arch/arm/boot/dts/exynos4210-universal_c210.dts | 4 +
arch/arm/boot/dts/exynos4210.dtsi | 12 +
arch/arm/mach-exynos/exynos.c | 21 +-
drivers/clk/clk.c | 3 +
drivers/clk/samsung/Makefile | 2 +-
drivers/clk/samsung/clk-cpu.c | 349 +++++++++++++++++++++++
drivers/clk/samsung/clk-cpu.h | 73 +++++
drivers/clk/samsung/clk-exynos4.c | 24 +-
drivers/cpufreq/Kconfig.arm | 11 -
drivers/cpufreq/Makefile | 1 -
drivers/cpufreq/exynos-cpufreq.c | 5 +-
drivers/cpufreq/exynos-cpufreq.h | 9 -
drivers/cpufreq/exynos4210-cpufreq.c | 184 ------------
include/linux/clk-provider.h | 1 +
16 files changed, 495 insertions(+), 212 deletions(-)
create mode 100644 drivers/clk/samsung/clk-cpu.c
create mode 100644 drivers/clk/samsung/clk-cpu.h
delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c

--
1.7.9.5


Subject: [PATCH 1/6] clk: add CLK_RECALC_NEW_RATES clock flag for Exynos cpu clock support

This flag is needed to fix the issue with wrong dividers being setup
by Common Clock Framework when using the new Exynos cpu clock support.

The issue happens because clk_core_set_rate_nolock() calls
clk_calc_new_rates(clk, rate) before both pre/post clock notifiers have
a chance to run. In case of Exynos cpu clock support pre/post clock
notifiers are registered for mout_apll clock which is a parent of armclk
cpu clock and dividers are modified in both pre and post clock notifier.
This results in wrong dividers values being later programmed by
clk_change_rate(top). To workaround the problem CLK_RECALC_NEW_RATES
flag is added and it is set for mout_apll clock later so the correct
divider values are re-calculated after both pre and post clock notifiers
had run.

For example when using "performance" governor on Exynos4210 Origen board
the cpufreq-dt driver requests to change the frequency from 1000MHz to
1200MHz and after the change state of the relevant clocks is following:

Without use of CLK_GET_RATE_NOCACHE flag:

fout_apll rate: 1200000000
fout_apll_div_2 rate: 600000000
mout_clkout_cpu rate: 600000000
div_clkout_cpu rate: 600000000
clkout_cpu rate: 600000000
mout_apll rate: 1200000000
armclk rate: 1200000000
mout_hpm rate: 1200000000
div_copy rate: 300000000
div_hpm rate: 300000000
mout_core rate: 1200000000
div_core rate: 1200000000
div_core2 rate: 1200000000
arm_clk_div_2 rate: 600000000
div_corem0 rate: 300000000
div_corem1 rate: 150000000
div_periph rate: 300000000
div_atb rate: 300000000
div_pclk_dbg rate: 150000000
sclk_apll rate: 1200000000
sclk_apll_div_2 rate: 600000000


With use of CLK_GET_RATE_NOCACHE flag:

fout_apll rate: 1200000000
fout_apll_div_2 rate: 600000000
mout_clkout_cpu rate: 600000000
div_clkout_cpu rate: 600000000
clkout_cpu rate: 600000000
mout_apll rate: 1200000000
armclk rate: 1200000000
mout_hpm rate: 1200000000
div_copy rate: 200000000
div_hpm rate: 200000000
mout_core rate: 1200000000
div_core rate: 1200000000
div_core2 rate: 1200000000
arm_clk_div_2 rate: 600000000
div_corem0 rate: 300000000
div_corem1 rate: 150000000
div_periph rate: 300000000
div_atb rate: 240000000
div_pclk_dbg rate: 120000000
sclk_apll rate: 150000000
sclk_apll_div_2 rate: 75000000

Without this change cpufreq-dt driver showed ~10 mA larger energy
consumption when compared to cpufreq-exynos one when "performance"
cpufreq governor was used on Exynos4210 SoC based Origen board.

This issue was probably meant to be workarounded by use of
CLK_GET_RATE_NOCACHE and CLK_DIVIDER_READ_ONLY clock flags in
the original Exynos cpu clock patchset (in "[PATCH v12 6/6] clk:
samsung: remove unused clock aliases and update clock flags" patch)
but usage of these flags is not sufficient to fix the issue observed.

Cc: Thomas Abraham <[email protected]>
Cc: Tomasz Figa <[email protected]>
Cc: Mike Turquette <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/clk/clk.c | 3 +++
include/linux/clk-provider.h | 1 +
2 files changed, 4 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f85c8e2..97cc73e 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1771,6 +1771,9 @@ static void clk_change_rate(struct clk_core *clk)
if (clk->notifier_count && old_rate != clk->rate)
__clk_notify(clk, POST_RATE_CHANGE, old_rate, clk->rate);

+ if (clk->flags & CLK_RECALC_NEW_RATES)
+ (void)clk_calc_new_rates(clk, clk->new_rate);
+
/*
* Use safe iteration, as change_rate can actually swap parents
* for certain clock types.
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 28abf1b..8d1aebe 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -31,6 +31,7 @@
#define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */
#define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
#define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
+#define CLK_RECALC_NEW_RATES BIT(9) /* recalc rates after notifications */

struct clk_hw;
struct clk_core;
--
1.7.9.5

Subject: [PATCH 2/6] clk: samsung: add infrastructure to register cpu clocks

From: Thomas Abraham <[email protected]>

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. A CPU clock provider can be composed of clock mux, dividers
and gates. This patch defines a new clock type for CPU clock provider and
adds infrastructure to register the CPU clock providers for Samsung
platforms.

Changes by Bartlomiej:
- fixed issue with setting lower dividers before the parent clock speed
was lowered (the issue resulted in lockup on Exynos4210 SoC based
Origen board when "ondemand" cpufreq governor was stress tested)
- fixed missing spin_unlock on error in exynos_cpuclk_post_rate_change()
problem by moving cfg_data search outside of the spin locked area
- removed leftover kfree() in exynos_register_cpu_clock() that could
result in dereferencing the NULL pointer on error
- moved spin_lock earlier in exynos_cpuclk_pre_rate_change() to cover
reading of E4210_SRC_CPU and E4210_DIV_CPU1 registers
- added missing "last chance" checks to wait_until_divider_stable() and
wait_until_mux_stable() (needed in case that IRQ handling took long
time to proceed and resulted in function printing incorrect error
message about timeout)
- moved E4210_CPU_DIV[0,1]() macros just before their only users,
this resulted in moving them from patch #2 to patch #3/6 ("clk:
samsung: exynos4: add cpu clock configuration data and instantiate
cpu clock")
- removed E5250_CPU_DIV[0,1](), E5420_EGL_DIV0() and E5420_KFC_DIV()
macros for now
- added my Copyrights to drivers/clk/samsung/clk-cpu.c

Cc: Tomasz Figa <[email protected]>
Cc: Mike Turquette <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Signed-off-by: Thomas Abraham <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/clk/samsung/Makefile | 2 +-
drivers/clk/samsung/clk-cpu.c | 349 +++++++++++++++++++++++++++++++++++++++++
drivers/clk/samsung/clk-cpu.h | 73 +++++++++
3 files changed, 423 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/samsung/clk-cpu.c
create mode 100644 drivers/clk/samsung/clk-cpu.h

diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
index 17e9af7..9e8bd83 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_EXYNOS4415) += clk-exynos4415.o
diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c
new file mode 100644
index 0000000..3a1fe07
--- /dev/null
+++ b/drivers/clk/samsung/clk-cpu.c
@@ -0,0 +1,349 @@
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Author: Thomas Abraham <[email protected]>
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd.
+ * Bartlomiej Zolnierkiewicz <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This file contains the utility function to register CPU clock for Samsung
+ * Exynos platforms. A CPU clock is defined as a clock supplied to a CPU or a
+ * group of CPUs. The CPU clock is typically derived from a hierarchy of clock
+ * blocks which includes mux and divider blocks. There are a number of other
+ * auxiliary clocks supplied to the CPU domain such as the debug blocks and AXI
+ * clock for CPU domain. The rates of these auxiliary clocks are related to the
+ * CPU clock rate and this relation is usually specified in the hardware manual
+ * of the SoC or supplied after the SoC characterization.
+ *
+ * The below implementation of the CPU clock allows the rate changes of the CPU
+ * clock and the corresponding rate changes of the auxillary clocks of the CPU
+ * domain. The platform clock driver provides a clock register configuration
+ * for each configurable rate which is then used to program the clock hardware
+ * registers to acheive a fast co-oridinated rate change for all the CPU domain
+ * clocks.
+ *
+ * On a rate change request for the CPU clock, the rate change is propagated
+ * upto the PLL supplying the clock to the CPU domain clock blocks. While the
+ * CPU domain PLL is reconfigured, the CPU domain clocks are driven using an
+ * alternate clock source. If required, the alternate clock source is divided
+ * down in order to keep the output clock rate within the previous OPP limits.
+*/
+
+#include <linux/errno.h>
+#include "clk-cpu.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 E4210_DIV0_RATIO0_MASK 0x7
+#define E4210_DIV1_HPM_MASK (0x7 << 4)
+#define E4210_DIV1_COPY_MASK (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 MAX_DIV 8
+#define DIV_MASK 7
+#define DIV_MASK_ALL 0xffffffff
+#define MUX_MASK 7
+
+/*
+ * 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));
+
+ if (!(readl(div_reg) & mask))
+ return;
+
+ 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));
+
+ if (((readl(mux_reg) >> mux_pos) & MUX_MASK) == mux_value)
+ return;
+
+ 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)
+{
+ /*
+ * 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,
+};
+
+/*
+ * 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 exynos_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 & mask);
+ 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 exynos_cpuclk_pre_rate_change(struct clk_notifier_data *ndata,
+ struct exynos_cpuclk *cpuclk, void __iomem *base)
+{
+ const struct exynos_cpuclk_cfg_data *cfg_data = cpuclk->cfg;
+ 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 ((cfg_data->prate * 1000) != ndata->new_rate) {
+ if (cfg_data->prate == 0)
+ return -EINVAL;
+ cfg_data++;
+ }
+
+ spin_lock(cpuclk->lock);
+
+ /*
+ * 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 = cfg_data->div0;
+ if (test_bit(CLK_CPU_HAS_DIV1, &cpuclk->flags)) {
+ div1 = cfg_data->div1;
+ if (readl(base + E4210_SRC_CPU) & E4210_MUX_HPM_MASK)
+ div1 = readl(base + E4210_DIV_CPU1) &
+ (E4210_DIV1_HPM_MASK | E4210_DIV1_COPY_MASK);
+ }
+
+ /*
+ * If the old parent clock speed is less than the clock speed of
+ * the alternate parent, then it should be ensured that at no point
+ * the armclk speed is more than the old_prate until the dividers are
+ * set. Also workaround the issue of the dividers being set to lower
+ * values before the parent clock speed is set to new lower speed
+ * (this can result in too high speed of armclk output clocks).
+ */
+ if (alt_prate > ndata->old_rate || ndata->old_rate > ndata->new_rate) {
+ unsigned long tmp_rate = min(ndata->old_rate, ndata->new_rate);
+
+ alt_div = DIV_ROUND_UP(alt_prate, tmp_rate) - 1;
+ WARN_ON(alt_div >= MAX_DIV);
+
+ if (test_bit(CLK_CPU_NEEDS_DEBUG_ALT_DIV, &cpuclk->flags)) {
+ /*
+ * 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;
+ }
+ exynos_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 (test_bit(CLK_CPU_HAS_DIV1, &cpuclk->flags)) {
+ 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 exynos_cpuclk_post_rate_change(struct clk_notifier_data *ndata,
+ struct exynos_cpuclk *cpuclk, void __iomem *base)
+{
+ const struct exynos_cpuclk_cfg_data *cfg_data = cpuclk->cfg;
+ unsigned long div = 0, div_mask = DIV_MASK;
+ unsigned long mux_reg;
+
+ /* find out the divider values to use for clock data */
+ if (test_bit(CLK_CPU_NEEDS_DEBUG_ALT_DIV, &cpuclk->flags)) {
+ while ((cfg_data->prate * 1000) != ndata->new_rate) {
+ if (cfg_data->prate == 0)
+ return -EINVAL;
+ cfg_data++;
+ }
+ }
+
+ 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 (test_bit(CLK_CPU_NEEDS_DEBUG_ALT_DIV, &cpuclk->flags)) {
+ div |= (cfg_data->div0 & E4210_DIV0_ATB_MASK);
+ div_mask |= E4210_DIV0_ATB_MASK;
+ }
+
+ exynos_set_safe_div(base, div, div_mask);
+ spin_unlock(cpuclk->lock);
+ return 0;
+}
+
+/*
+ * 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;
+ void __iomem *base;
+ int err = 0;
+
+ cpuclk = container_of(nb, struct exynos_cpuclk, clk_nb);
+ base = cpuclk->ctrl_base;
+
+ if (event == PRE_RATE_CHANGE)
+ err = exynos_cpuclk_pre_rate_change(ndata, cpuclk, base);
+ else if (event == POST_RATE_CHANGE)
+ err = exynos_cpuclk_post_rate_change(ndata, cpuclk, base);
+
+ return notifier_from_errno(err);
+}
+
+/* helper function to register a CPU clock */
+int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
+ unsigned int lookup_id, const char *name, const char *parent,
+ const char *alt_parent, unsigned long offset,
+ const struct exynos_cpuclk_cfg_data *cfg,
+ unsigned long num_cfgs, unsigned long flags)
+{
+ struct exynos_cpuclk *cpuclk;
+ struct clk_init_data init;
+ struct clk *clk;
+ int ret = 0;
+
+ cpuclk = kzalloc(sizeof(*cpuclk), GFP_KERNEL);
+ if (!cpuclk)
+ return -ENOMEM;
+
+ init.name = name;
+ init.flags = CLK_SET_RATE_PARENT;
+ init.parent_names = &parent;
+ init.num_parents = 1;
+ init.ops = &exynos_cpuclk_clk_ops;
+
+ cpuclk->hw.init = &init;
+ cpuclk->ctrl_base = ctx->reg_base + offset;
+ cpuclk->lock = &ctx->lock;
+ cpuclk->flags = flags;
+ cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb;
+
+ 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 free_cpuclk;
+ }
+
+ clk = __clk_lookup(parent);
+ if (!clk) {
+ pr_err("%s: could not lookup parent clock %s\n",
+ __func__, parent);
+ ret = -EINVAL;
+ goto free_cpuclk;
+ }
+
+ ret = clk_notifier_register(clk, &cpuclk->clk_nb);
+ if (ret) {
+ pr_err("%s: failed to register clock notifier for %s\n",
+ __func__, name);
+ goto free_cpuclk;
+ }
+
+ cpuclk->cfg = kmemdup(cfg, sizeof(*cfg) * num_cfgs, GFP_KERNEL);
+ if (!cpuclk->cfg) {
+ pr_err("%s: could not allocate memory for cpuclk data\n",
+ __func__);
+ ret = -ENOMEM;
+ 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 free_cpuclk_data;
+ }
+
+ samsung_clk_add_lookup(ctx, clk, lookup_id);
+ return 0;
+
+free_cpuclk_data:
+ kfree(cpuclk->cfg);
+unregister_clk_nb:
+ clk_notifier_unregister(__clk_lookup(parent), &cpuclk->clk_nb);
+free_cpuclk:
+ kfree(cpuclk);
+ return ret;
+}
diff --git a/drivers/clk/samsung/clk-cpu.h b/drivers/clk/samsung/clk-cpu.h
new file mode 100644
index 0000000..37874d3
--- /dev/null
+++ b/drivers/clk/samsung/clk-cpu.h
@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *
+ * 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.
+ *
+ * Common Clock Framework support for all PLL's in Samsung platforms
+*/
+
+#ifndef __SAMSUNG_CLK_CPU_H
+#define __SAMSUNG_CLK_CPU_H
+
+#include "clk.h"
+
+/**
+ * struct exynos_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 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
+ * value is not used.
+ */
+struct exynos_cpuclk_cfg_data {
+ unsigned long prate;
+ unsigned long div0;
+ unsigned long 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.
+ * @lock: cpu clock domain register access lock.
+ * @cfg: cpu clock rate configuration data.
+ * @num_cfgs: number of array elements in @cfg array.
+ * @clk_nb: clock notifier registered for changes in clock speed of the
+ * primary parent clock.
+ * @flags: configuration flags for the CPU 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;
+ spinlock_t *lock;
+ const struct exynos_cpuclk_cfg_data *cfg;
+ const unsigned long num_cfgs;
+ struct notifier_block clk_nb;
+ unsigned long flags;
+
+/* The CPU clock registers has DIV1 configuration register */
+#define CLK_CPU_HAS_DIV1 (1 << 0)
+/* When ALT parent is active, debug clocks need safe divider values */
+#define CLK_CPU_NEEDS_DEBUG_ALT_DIV (1 << 1)
+};
+
+extern int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
+ unsigned int lookup_id, const char *name,
+ const char *parent, const char *alt_parent,
+ unsigned long offset,
+ const struct exynos_cpuclk_cfg_data *cfg,
+ unsigned long num_cfgs, unsigned long flags);
+
+#endif /* __SAMSUNG_CLK_CPU_H */
--
1.7.9.5

Subject: [PATCH 3/6] clk: samsung: exynos4: add cpu clock configuration data and instantiate cpu clock

From: Thomas Abraham <[email protected]>

With the addition of the new Samsung specific cpu-clock type, the
arm clock can be represented as a cpu-clock type. Add the CPU clock
configuration data and instantiate the CPU clock type for Exynos4210.

Changes by Bartlomiej:
- fixed issue with wrong dividers being setup by Common Clock Framework
(by an addition of CLK_RECALC_NEW_RATES clock flag to mout_apll clock,
without this change cpufreq-dt driver showed ~10 mA larger energy
consumption when compared to cpufreq-exynos one when "performance"
cpufreq governor was used on Exynos4210 SoC based Origen board), this
was probably meant to be workarounded by use of CLK_GET_RATE_NOCACHE
and CLK_DIVIDER_READ_ONLY clock flags in the original patchset (in
"[PATCH v12 6/6] clk: samsung: remove unused clock aliases and update
clock flags") but using these flags is not sufficient to fix the issue
observed
- removed Exynos5250 and Exynos5420 support for now

Cc: Tomasz Figa <[email protected]>
Cc: Mike Turquette <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Signed-off-by: Thomas Abraham <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/clk/samsung/clk-exynos4.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index 714d6ba..cae2c048 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -19,6 +19,7 @@
#include <linux/syscore_ops.h>

#include "clk.h"
+#include "clk-cpu.h"

/* Exynos4 clock controller register offsets */
#define SRC_LEFTBUS 0x4200
@@ -534,7 +535,8 @@ static struct samsung_fixed_factor_clock exynos4x12_fixed_factor_clks[] __initda
/* list of mux clocks supported in all exynos4 soc's */
static struct samsung_mux_clock exynos4_mux_clks[] __initdata = {
MUX_FA(CLK_MOUT_APLL, "mout_apll", mout_apll_p, SRC_CPU, 0, 1,
- CLK_SET_RATE_PARENT, 0, "mout_apll"),
+ CLK_SET_RATE_PARENT | CLK_RECALC_NEW_RATES, 0,
+ "mout_apll"),
MUX(CLK_MOUT_HDMI, "mout_hdmi", mout_hdmi_p, SRC_TV, 0, 1),
MUX(0, "mout_mfc1", sclk_evpll_p, SRC_MFC, 4, 1),
MUX(0, "mout_mfc", mout_mfc_p, SRC_MFC, 8, 1),
@@ -1378,6 +1380,22 @@ static void __init exynos4x12_core_down_clock(void)
__raw_writel(0x0, reg_base + E4X12_PWR_CTRL2);
}

+#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))
+
+static const struct exynos_cpuclk_cfg_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 },
+};
+
/* register exynos4 clocks */
static void __init exynos4_clk_init(struct device_node *np,
enum exynos4_soc soc)
@@ -1455,6 +1473,10 @@ static void __init exynos4_clk_init(struct device_node *np,
samsung_clk_register_fixed_factor(ctx,
exynos4210_fixed_factor_clks,
ARRAY_SIZE(exynos4210_fixed_factor_clks));
+ exynos_register_cpu_clock(ctx, CLK_ARM_CLK, "armclk",
+ mout_core_p4210[0], mout_core_p4210[1], 0x14200,
+ e4210_armclk_d, ARRAY_SIZE(e4210_armclk_d),
+ CLK_CPU_NEEDS_DEBUG_ALT_DIV | CLK_CPU_HAS_DIV1);
} else {
samsung_clk_register_mux(ctx, exynos4x12_mux_clks,
ARRAY_SIZE(exynos4x12_mux_clks));
--
1.7.9.5

Subject: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

From: Thomas Abraham <[email protected]>

For Exynos4210 platforms, add CPU operating points and CPU
regulator supply properties for migrating from Exynos specific
cpufreq driver to using generic cpufreq driver.

Changes by Bartlomiej:
- removed Exynos5250 and Exynos5420 support for now

Cc: Kukjin Kim <[email protected]>
Cc: Doug Anderson <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Cc: Andreas Faerber <[email protected]>
Cc: Sachin Kamat <[email protected]>
Cc: Andreas Farber <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Signed-off-by: Thomas Abraham <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
arch/arm/boot/dts/exynos4210-origen.dts | 4 ++++
arch/arm/boot/dts/exynos4210-trats.dts | 4 ++++
arch/arm/boot/dts/exynos4210-universal_c210.dts | 4 ++++
arch/arm/boot/dts/exynos4210.dtsi | 12 ++++++++++++
4 files changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts
index b811461..5b0941f 100644
--- a/arch/arm/boot/dts/exynos4210-origen.dts
+++ b/arch/arm/boot/dts/exynos4210-origen.dts
@@ -335,3 +335,7 @@
};
};
};
+
+&cpu0 {
+ cpu0-supply = <&buck1_reg>;
+};
diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts
index 32c5fd8..9b5bdd0 100644
--- a/arch/arm/boot/dts/exynos4210-trats.dts
+++ b/arch/arm/boot/dts/exynos4210-trats.dts
@@ -483,3 +483,7 @@
};
};
};
+
+&cpu0 {
+ cpu0-supply = <&varm_breg>;
+};
diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts b/arch/arm/boot/dts/exynos4210-universal_c210.dts
index d4f2b11..5bf74da 100644
--- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
+++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
@@ -567,3 +567,7 @@
&mdma1 {
reg = <0x12840000 0x1000>;
};
+
+&cpu0 {
+ cpu0-supply = <&vdd_arm_reg>;
+};
diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index be89f83..df075e64 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -40,6 +40,18 @@
device_type = "cpu";
compatible = "arm,cortex-a9";
reg = <0x900>;
+ clocks = <&clock CLK_ARM_CLK>;
+ clock-names = "cpu";
+ clock-latency = <160000>;
+
+ operating-points = <
+ 1200000 1250000
+ 1000000 1150000
+ 800000 1075000
+ 500000 975000
+ 400000 975000
+ 200000 950000
+ >;
cooling-min-level = <4>;
cooling-max-level = <2>;
#cooling-cells = <2>; /* min followed by max */
--
1.7.9.5

Subject: [PATCH 5/6] ARM: Exynos: switch to using generic cpufreq driver for Exynos4210

From: Thomas Abraham <[email protected]>

The new CPU clock type allows the use of generic CPUfreq driver.
Switch Exynos4210 to using generic cpufreq driver.

Changes by Bartlomiej:
- removed non-Exynos4210 support for now

Cc: Tomasz Figa <[email protected]>
Cc: Kukjin Kim <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Signed-off-by: Thomas Abraham <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
arch/arm/mach-exynos/exynos.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index bcde0dd..4e13de5 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -197,6 +197,25 @@ static void __init exynos_init_irq(void)
exynos_map_pmu();
}

+static const struct of_device_id exynos_cpufreq_matches[] = {
+ { .compatible = "samsung,exynos4210", .data = "cpufreq-dt" },
+ { /* sentinel */ }
+};
+
+static 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);
+}
+
static void __init exynos_dt_machine_init(void)
{
/*
@@ -218,7 +237,7 @@ static void __init exynos_dt_machine_init(void)
of_machine_is_compatible("samsung,exynos5250"))
platform_device_register(&exynos_cpuidle);

- platform_device_register_simple("exynos-cpufreq", -1, NULL, 0);
+ exynos_cpufreq_init();

of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
}
--
1.7.9.5

Subject: [PATCH 6/6] cpufreq: exynos: remove Exynos4210 specific cpufreq driver support

From: Thomas Abraham <[email protected]>

Exynos4210 based platforms have switched over to use generic
cpufreq driver for cpufreq functionality. So the Exynos
specific cpufreq support for these platforms can be removed.

Changes by Bartlomiej:
- dropped Exynos5250 support removal for now
- updated exynos-cpufreq.[c,h]

Cc: Viresh Kumar <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Signed-off-by: Thomas Abraham <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/cpufreq/Kconfig.arm | 11 --
drivers/cpufreq/Makefile | 1 -
drivers/cpufreq/exynos-cpufreq.c | 5 +-
drivers/cpufreq/exynos-cpufreq.h | 9 --
drivers/cpufreq/exynos4210-cpufreq.c | 184 ----------------------------------
5 files changed, 1 insertion(+), 209 deletions(-)
delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 1b06fc4..1d7ddc5 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -36,17 +36,6 @@ config ARM_EXYNOS_CPUFREQ

If in doubt, say N.

-config ARM_EXYNOS4210_CPUFREQ
- bool "SAMSUNG EXYNOS4210"
- depends on CPU_EXYNOS4210
- depends on ARM_EXYNOS_CPUFREQ
- default y
- 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
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 26df0ad..cb16319 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -54,7 +54,6 @@ obj-$(CONFIG_ARCH_DAVINCI) += davinci-cpufreq.o
obj-$(CONFIG_UX500_SOC_DB8500) += dbx500-cpufreq.o
obj-$(CONFIG_ARM_EXYNOS_CPUFREQ) += arm-exynos-cpufreq.o
arm-exynos-cpufreq-y := exynos-cpufreq.o
-arm-exynos-cpufreq-$(CONFIG_ARM_EXYNOS4210_CPUFREQ) += exynos4210-cpufreq.o
arm-exynos-cpufreq-$(CONFIG_ARM_EXYNOS4X12_CPUFREQ) += exynos4x12-cpufreq.o
arm-exynos-cpufreq-$(CONFIG_ARM_EXYNOS5250_CPUFREQ) += exynos5250-cpufreq.o
obj-$(CONFIG_ARM_EXYNOS5440_CPUFREQ) += exynos5440-cpufreq.o
diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c
index 01ed166..2d78f63 100644
--- a/drivers/cpufreq/exynos-cpufreq.c
+++ b/drivers/cpufreq/exynos-cpufreq.c
@@ -144,10 +144,7 @@ static int exynos_cpufreq_probe(struct platform_device *pdev)

exynos_info->dev = &pdev->dev;

- if (of_machine_is_compatible("samsung,exynos4210")) {
- exynos_info->type = EXYNOS_SOC_4210;
- ret = exynos4210_cpufreq_init(exynos_info);
- } else if (of_machine_is_compatible("samsung,exynos4212")) {
+ if (of_machine_is_compatible("samsung,exynos4212")) {
exynos_info->type = EXYNOS_SOC_4212;
ret = exynos4x12_cpufreq_init(exynos_info);
} else if (of_machine_is_compatible("samsung,exynos4412")) {
diff --git a/drivers/cpufreq/exynos-cpufreq.h b/drivers/cpufreq/exynos-cpufreq.h
index fe5a105..bf03d7e 100644
--- a/drivers/cpufreq/exynos-cpufreq.h
+++ b/drivers/cpufreq/exynos-cpufreq.h
@@ -18,7 +18,6 @@ enum cpufreq_level_index {
};

enum exynos_soc_type {
- EXYNOS_SOC_4210,
EXYNOS_SOC_4212,
EXYNOS_SOC_4412,
EXYNOS_SOC_5250,
@@ -52,14 +51,6 @@ struct exynos_dvfs_info {
void __iomem *cmu_regs;
};

-#ifdef CONFIG_ARM_EXYNOS4210_CPUFREQ
-extern int exynos4210_cpufreq_init(struct exynos_dvfs_info *);
-#else
-static inline int exynos4210_cpufreq_init(struct exynos_dvfs_info *info)
-{
- return -EOPNOTSUPP;
-}
-#endif
#ifdef CONFIG_ARM_EXYNOS4X12_CPUFREQ
extern int exynos4x12_cpufreq_init(struct exynos_dvfs_info *);
#else
diff --git a/drivers/cpufreq/exynos4210-cpufreq.c b/drivers/cpufreq/exynos4210-cpufreq.c
deleted file mode 100644
index 843ec82..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-dt 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;
-}
--
1.7.9.5

2015-05-08 00:05:42

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 5/6] ARM: Exynos: switch to using generic cpufreq driver for Exynos4210

2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
> From: Thomas Abraham <[email protected]>
>
> The new CPU clock type allows the use of generic CPUfreq driver.
> Switch Exynos4210 to using generic cpufreq driver.
>
> Changes by Bartlomiej:
> - removed non-Exynos4210 support for now
>
> Cc: Tomasz Figa <[email protected]>
> Cc: Kukjin Kim <[email protected]>
> Cc: Javier Martinez Canillas <[email protected]>
> Signed-off-by: Thomas Abraham <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>

I know this is an old patch, it looks good. Just a minor nit below.

> ---
> arch/arm/mach-exynos/exynos.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index bcde0dd..4e13de5 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -197,6 +197,25 @@ static void __init exynos_init_irq(void)
> exynos_map_pmu();
> }
>
> +static const struct of_device_id exynos_cpufreq_matches[] = {
> + { .compatible = "samsung,exynos4210", .data = "cpufreq-dt" },
> + { /* sentinel */ }
> +};
> +
> +static 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);

How about something like:

const char *cpufreq_dev)name = "exynos-cpufreq";
if (match)
cpufreq_dev_name = match->data;
platform_device_register_simple(cpufreq_dev_name -1, NULL, 0);

Less returns and one call to platform_device_register_simple().
Best regards,
Krzysztof

> +}
> +
> static void __init exynos_dt_machine_init(void)
> {
> /*
> @@ -218,7 +237,7 @@ static void __init exynos_dt_machine_init(void)
> of_machine_is_compatible("samsung,exynos5250"))
> platform_device_register(&exynos_cpuidle);
>
> - platform_device_register_simple("exynos-cpufreq", -1, NULL, 0);
> + exynos_cpufreq_init();
>
> of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> }
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-05-08 00:18:23

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
> From: Thomas Abraham <[email protected]>
>
> For Exynos4210 platforms, add CPU operating points and CPU
> regulator supply properties for migrating from Exynos specific
> cpufreq driver to using generic cpufreq driver.
>
> Changes by Bartlomiej:
> - removed Exynos5250 and Exynos5420 support for now
>
> Cc: Kukjin Kim <[email protected]>
> Cc: Doug Anderson <[email protected]>
> Cc: Javier Martinez Canillas <[email protected]>
> Cc: Andreas Faerber <[email protected]>
> Cc: Sachin Kamat <[email protected]>
> Cc: Andreas Farber <[email protected]>
> Cc: Javier Martinez Canillas <[email protected]>
> Signed-off-by: Thomas Abraham <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>

Acked-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

Subject: Re: [PATCH 0/6] cpufreq: use generic cpufreq drivers for Exynos4210 platform


Hi,

On Friday, April 03, 2015 06:43:43 PM Bartlomiej Zolnierkiewicz wrote:
> Hi,
>
> This patch series removes the use of Exynos4210 specific support
> from cpufreq-exynos driver and enables the use of cpufreq-dt driver
> for this platform.

Gentle Ping. Mike/Kukjin/Viresh could you please review/ack relevant
patches (patches #1-3 are for clock subsystem, patches #4-5 for Exynos
mach/dts and patch #6 is for cpufreq subsystem)? Also what is your
preferred way to upstream them (patches are not independent so it would
be best to merge them through one tree, otherwise synchronization of
git pulls between different subsystem trees will be needed)?

I'm still hoping that this patchset will make it into v4.2 as there are
no known issues with it (except minor coding nit for patch #5)...

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> It consists of a modified "[PATCH v12 0/6] cpufreq: use generic
> cpufreq drivers for exynos platforms" [1] patch series from Thomas
> Abraham. As there have not been updates of this patchset since
> November 2014 I decided to dust it off myself. I updated Thomas'
> patchset to current kernels, fixed bugs that I noticed and removed
> non-Exynos4210 support for now (to speed up inclusion in upstream
> kernel). Since some modifications were not trivial I dropped all
> Reviewed-by:, Tested-by: and Acked-by: tags. Please review/test
> this patchset and reply with revelant tag (Thank You!). I also
> plan to work on Exynos4x12 support next (which was missing in
> the original patchset) and then on Exynos5250/Exynos5420 one.
>
> This patch series has been tested on Exynos4210 based Origen and
> Trats boards.
>
> Depends on:
> - next-20150330 branch of linux-next kernel tree
> (mfd tree contains a crucial fix for MAX8997 PMIC support)
> - "[PATCH] clk: samsung: exynos4: Disable ARMCLK down feature on
> Exynos4210 SoC" [2]
>
> [1] https://www.marc.info/?l=linux-arm-kernel&m=141657611003803&w=2
> [2] https://lkml.org/lkml/2015/3/27/568
>
> Changes over Thomas' code:
> - fixed issue with wrong dividers being setup by Common Clock Framework
> (by an addition of CLK_RECALC_NEW_RATES clock flag to mout_apll clock,
> without this change cpufreq-dt driver showed ~10 mA larger energy
> consumption when compared to cpufreq-exynos one when "performance"
> cpufreq governor was used on Exynos4210 SoC based Origen board), this
> was probably meant to be workarounded by use of CLK_GET_RATE_NOCACHE
> and CLK_DIVIDER_READ_ONLY clock flags in the original patchset (in
> "[PATCH v12 6/6] clk: samsung: remove unused clock aliases and update
> clock flags") but using these flags is not sufficient to fix the issue
> observed
> - fixed issue with setting lower dividers before the parent clock speed
> was lowered (the issue resulted in lockup on Exynos4210 SoC based
> Origen board when "ondemand" cpufreq governor was stress tested)
> - fixed missing spin_unlock on error in exynos_cpuclk_post_rate_change()
> problem by moving cfg_data search outside of the spin locked area
> - removed leftover kfree() in exynos_register_cpu_clock() that could
> result in dereferencing the NULL pointer on error
> - moved spin_lock earlier in exynos_cpuclk_pre_rate_change() to cover
> reading of E4210_SRC_CPU and E4210_DIV_CPU1 registers
> - added missing "last chance" checks to wait_until_divider_stable() and
> wait_until_mux_stable() (needed in case that IRQ handling took long
> time to proceed and resulted in function printing incorrect error
> message about timeout)
> - moved E4210_CPU_DIV[0,1]() macros just before their only users,
> this resulted in moving them from patch #2 to patch #3/6 ("clk:
> samsung: exynos4: add cpu clock configuration data and instantiate
> cpu clock")
> - added my Copyrights to drivers/clk/samsung/clk-cpu.c
> - updated exynos-cpufreq.[c,h]
> - removed non-Exynos4210 support for now
> - dropped "[PATCH v12 6/6] clk: samsung: remove unused clock aliases and
> update clock flags" altogether for now
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
>
> Bartlomiej Zolnierkiewicz (1):
> clk: add CLK_RECALC_NEW_RATES clock flag for Exynos cpu clock support
>
> Thomas Abraham (5):
> clk: samsung: add infrastructure to register cpu clocks
> clk: samsung: exynos4: add cpu clock configuration data and
> instantiate cpu clock
> ARM: dts: Exynos4210: add CPU OPP and regulator supply property
> ARM: Exynos: switch to using generic cpufreq driver for Exynos4210
> cpufreq: exynos: remove Exynos4210 specific cpufreq driver support
>
> arch/arm/boot/dts/exynos4210-origen.dts | 4 +
> arch/arm/boot/dts/exynos4210-trats.dts | 4 +
> arch/arm/boot/dts/exynos4210-universal_c210.dts | 4 +
> arch/arm/boot/dts/exynos4210.dtsi | 12 +
> arch/arm/mach-exynos/exynos.c | 21 +-
> drivers/clk/clk.c | 3 +
> drivers/clk/samsung/Makefile | 2 +-
> drivers/clk/samsung/clk-cpu.c | 349 +++++++++++++++++++++++
> drivers/clk/samsung/clk-cpu.h | 73 +++++
> drivers/clk/samsung/clk-exynos4.c | 24 +-
> drivers/cpufreq/Kconfig.arm | 11 -
> drivers/cpufreq/Makefile | 1 -
> drivers/cpufreq/exynos-cpufreq.c | 5 +-
> drivers/cpufreq/exynos-cpufreq.h | 9 -
> drivers/cpufreq/exynos4210-cpufreq.c | 184 ------------
> include/linux/clk-provider.h | 1 +
> 16 files changed, 495 insertions(+), 212 deletions(-)
> create mode 100644 drivers/clk/samsung/clk-cpu.c
> create mode 100644 drivers/clk/samsung/clk-cpu.h
> delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c

2015-05-13 14:13:49

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: add CLK_RECALC_NEW_RATES clock flag for Exynos cpu clock support

On 03/04/15 18:43, Bartlomiej Zolnierkiewicz wrote:
> This flag is needed to fix the issue with wrong dividers being setup
> by Common Clock Framework when using the new Exynos cpu clock support.
>
> The issue happens because clk_core_set_rate_nolock() calls
> clk_calc_new_rates(clk, rate) before both pre/post clock notifiers have
> a chance to run. In case of Exynos cpu clock support pre/post clock
> notifiers are registered for mout_apll clock which is a parent of armclk
> cpu clock and dividers are modified in both pre and post clock notifier.
> This results in wrong dividers values being later programmed by
> clk_change_rate(top). To workaround the problem CLK_RECALC_NEW_RATES
> flag is added and it is set for mout_apll clock later so the correct
> divider values are re-calculated after both pre and post clock notifiers
> had run.
>
> For example when using "performance" governor on Exynos4210 Origen board
> the cpufreq-dt driver requests to change the frequency from 1000MHz to
> 1200MHz and after the change state of the relevant clocks is following:
>
> Without use of CLK_GET_RATE_NOCACHE flag:
>
> fout_apll rate: 1200000000
> fout_apll_div_2 rate: 600000000
> mout_clkout_cpu rate: 600000000
> div_clkout_cpu rate: 600000000
> clkout_cpu rate: 600000000
> mout_apll rate: 1200000000
> armclk rate: 1200000000
> mout_hpm rate: 1200000000
> div_copy rate: 300000000
> div_hpm rate: 300000000
> mout_core rate: 1200000000
> div_core rate: 1200000000
> div_core2 rate: 1200000000
> arm_clk_div_2 rate: 600000000
> div_corem0 rate: 300000000
> div_corem1 rate: 150000000
> div_periph rate: 300000000
> div_atb rate: 300000000
> div_pclk_dbg rate: 150000000
> sclk_apll rate: 1200000000
> sclk_apll_div_2 rate: 600000000
>
>
> With use of CLK_GET_RATE_NOCACHE flag:
>
> fout_apll rate: 1200000000
> fout_apll_div_2 rate: 600000000
> mout_clkout_cpu rate: 600000000
> div_clkout_cpu rate: 600000000
> clkout_cpu rate: 600000000
> mout_apll rate: 1200000000
> armclk rate: 1200000000
> mout_hpm rate: 1200000000
> div_copy rate: 200000000
> div_hpm rate: 200000000
> mout_core rate: 1200000000
> div_core rate: 1200000000
> div_core2 rate: 1200000000
> arm_clk_div_2 rate: 600000000
> div_corem0 rate: 300000000
> div_corem1 rate: 150000000
> div_periph rate: 300000000
> div_atb rate: 240000000
> div_pclk_dbg rate: 120000000
> sclk_apll rate: 150000000
> sclk_apll_div_2 rate: 75000000
>
> Without this change cpufreq-dt driver showed ~10 mA larger energy
> consumption when compared to cpufreq-exynos one when "performance"
> cpufreq governor was used on Exynos4210 SoC based Origen board.
>
> This issue was probably meant to be workarounded by use of
> CLK_GET_RATE_NOCACHE and CLK_DIVIDER_READ_ONLY clock flags in
> the original Exynos cpu clock patchset (in "[PATCH v12 6/6] clk:
> samsung: remove unused clock aliases and update clock flags" patch)
> but usage of these flags is not sufficient to fix the issue observed.
>
> Cc: Thomas Abraham <[email protected]>
> Cc: Tomasz Figa <[email protected]>
> Cc: Mike Turquette <[email protected]>
> Cc: Javier Martinez Canillas <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> ---
> drivers/clk/clk.c | 3 +++
> include/linux/clk-provider.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index f85c8e2..97cc73e 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1771,6 +1771,9 @@ static void clk_change_rate(struct clk_core *clk)
> if (clk->notifier_count && old_rate != clk->rate)
> __clk_notify(clk, POST_RATE_CHANGE, old_rate, clk->rate);
>
> + if (clk->flags & CLK_RECALC_NEW_RATES)
> + (void)clk_calc_new_rates(clk, clk->new_rate);
> +
> /*
> * Use safe iteration, as change_rate can actually swap parents
> * for certain clock types.
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 28abf1b..8d1aebe 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -31,6 +31,7 @@
> #define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */
> #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
> #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
> +#define CLK_RECALC_NEW_RATES BIT(9) /* recalc rates after notifications */

Mike, Stephen, what do you think about this? I'm rather resistant to
this new flag approach, it looks like a hack. I don't seem to have better
ideas to address the missing rate recalculation issue though.
I thought about making the cpu clk notifier callback returning a specific
error value, which would then trigger rate recalculation after
__clk_notify() call in clk_change_rate() function. This way the trigger
of the repeated rate recalculation would come from a notifier callback,
rather than from a clock definition. But in general it would be difficult
to handle multiple notification callbacks, each of which would attempt
to trigger the rate recalculation.

--
Regards,
Sylwester

2015-05-14 04:07:59

by Kukjin Kim

[permalink] [raw]
Subject: Re: [PATCH 0/6] cpufreq: use generic cpufreq drivers for Exynos4210platform

On 05/13/15 23:08, Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
Hi Bart,

> On Friday, April 03, 2015 06:43:43 PM Bartlomiej Zolnierkiewicz wrote:
>> Hi,
>>
>> This patch series removes the use of Exynos4210 specific support
>> from cpufreq-exynos driver and enables the use of cpufreq-dt driver
>> for this platform.
>
> Gentle Ping. Mike/Kukjin/Viresh could you please review/ack relevant
> patches (patches #1-3 are for clock subsystem, patches #4-5 for Exynos
> mach/dts and patch #6 is for cpufreq subsystem)?

Yes, I totally agreed with this patches for arch side changes and this
approach when Thomas posted.

> Also what is your
> preferred way to upstream them (patches are not independent so it would
> be best to merge them through one tree, otherwise synchronization of
> git pulls between different subsystem trees will be needed)?
>
I can provide topic branch for arch side changes even it is small. I
think once Viresh and Mike make each topic branch based on -rc or the
smallest changes from each subsystem then I could handle this series or
Viresh or Mike need to handle this series with merging each topic
branches in subsystem. I'm fine either way.

Viresh and Mike, how do you think about that?

> I'm still hoping that this patchset will make it into v4.2 as there are
> no known issues with it (except minor coding nit for patch #5)...
>
Sure, why not :-)

Thanks,
Kukjin

> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
>> It consists of a modified "[PATCH v12 0/6] cpufreq: use generic
>> cpufreq drivers for exynos platforms" [1] patch series from Thomas
>> Abraham. As there have not been updates of this patchset since
>> November 2014 I decided to dust it off myself. I updated Thomas'
>> patchset to current kernels, fixed bugs that I noticed and removed
>> non-Exynos4210 support for now (to speed up inclusion in upstream
>> kernel). Since some modifications were not trivial I dropped all
>> Reviewed-by:, Tested-by: and Acked-by: tags. Please review/test
>> this patchset and reply with revelant tag (Thank You!). I also
>> plan to work on Exynos4x12 support next (which was missing in
>> the original patchset) and then on Exynos5250/Exynos5420 one.
>>
>> This patch series has been tested on Exynos4210 based Origen and
>> Trats boards.
>>
>> Depends on:
>> - next-20150330 branch of linux-next kernel tree
>> (mfd tree contains a crucial fix for MAX8997 PMIC support)
>> - "[PATCH] clk: samsung: exynos4: Disable ARMCLK down feature on
>> Exynos4210 SoC" [2]
>>
>> [1] https://www.marc.info/?l=linux-arm-kernel&m=141657611003803&w=2
>> [2] https://lkml.org/lkml/2015/3/27/568
>>
>> Changes over Thomas' code:
>> - fixed issue with wrong dividers being setup by Common Clock Framework
>> (by an addition of CLK_RECALC_NEW_RATES clock flag to mout_apll clock,
>> without this change cpufreq-dt driver showed ~10 mA larger energy
>> consumption when compared to cpufreq-exynos one when "performance"
>> cpufreq governor was used on Exynos4210 SoC based Origen board), this
>> was probably meant to be workarounded by use of CLK_GET_RATE_NOCACHE
>> and CLK_DIVIDER_READ_ONLY clock flags in the original patchset (in
>> "[PATCH v12 6/6] clk: samsung: remove unused clock aliases and update
>> clock flags") but using these flags is not sufficient to fix the issue
>> observed
>> - fixed issue with setting lower dividers before the parent clock speed
>> was lowered (the issue resulted in lockup on Exynos4210 SoC based
>> Origen board when "ondemand" cpufreq governor was stress tested)
>> - fixed missing spin_unlock on error in exynos_cpuclk_post_rate_change()
>> problem by moving cfg_data search outside of the spin locked area
>> - removed leftover kfree() in exynos_register_cpu_clock() that could
>> result in dereferencing the NULL pointer on error
>> - moved spin_lock earlier in exynos_cpuclk_pre_rate_change() to cover
>> reading of E4210_SRC_CPU and E4210_DIV_CPU1 registers
>> - added missing "last chance" checks to wait_until_divider_stable() and
>> wait_until_mux_stable() (needed in case that IRQ handling took long
>> time to proceed and resulted in function printing incorrect error
>> message about timeout)
>> - moved E4210_CPU_DIV[0,1]() macros just before their only users,
>> this resulted in moving them from patch #2 to patch #3/6 ("clk:
>> samsung: exynos4: add cpu clock configuration data and instantiate
>> cpu clock")
>> - added my Copyrights to drivers/clk/samsung/clk-cpu.c
>> - updated exynos-cpufreq.[c,h]
>> - removed non-Exynos4210 support for now
>> - dropped "[PATCH v12 6/6] clk: samsung: remove unused clock aliases and
>> update clock flags" altogether for now
>>
>> Best regards,
>> --
>> Bartlomiej Zolnierkiewicz
>> Samsung R&D Institute Poland
>> Samsung Electronics
>>
>>
>> Bartlomiej Zolnierkiewicz (1):
>> clk: add CLK_RECALC_NEW_RATES clock flag for Exynos cpu clock support
>>
>> Thomas Abraham (5):
>> clk: samsung: add infrastructure to register cpu clocks
>> clk: samsung: exynos4: add cpu clock configuration data and
>> instantiate cpu clock
>> ARM: dts: Exynos4210: add CPU OPP and regulator supply property
>> ARM: Exynos: switch to using generic cpufreq driver for Exynos4210
>> cpufreq: exynos: remove Exynos4210 specific cpufreq driver support
>>
>> arch/arm/boot/dts/exynos4210-origen.dts | 4 +
>> arch/arm/boot/dts/exynos4210-trats.dts | 4 +
>> arch/arm/boot/dts/exynos4210-universal_c210.dts | 4 +
>> arch/arm/boot/dts/exynos4210.dtsi | 12 +
>> arch/arm/mach-exynos/exynos.c | 21 +-
>> drivers/clk/clk.c | 3 +
>> drivers/clk/samsung/Makefile | 2 +-
>> drivers/clk/samsung/clk-cpu.c | 349 +++++++++++++++++++++++
>> drivers/clk/samsung/clk-cpu.h | 73 +++++
>> drivers/clk/samsung/clk-exynos4.c | 24 +-
>> drivers/cpufreq/Kconfig.arm | 11 -
>> drivers/cpufreq/Makefile | 1 -
>> drivers/cpufreq/exynos-cpufreq.c | 5 +-
>> drivers/cpufreq/exynos-cpufreq.h | 9 -
>> drivers/cpufreq/exynos4210-cpufreq.c | 184 ------------
>> include/linux/clk-provider.h | 1 +
>> 16 files changed, 495 insertions(+), 212 deletions(-)
>> create mode 100644 drivers/clk/samsung/clk-cpu.c
>> create mode 100644 drivers/clk/samsung/clk-cpu.h
>> delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c

2015-05-14 05:03:21

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 6/6] cpufreq: exynos: remove Exynos4210 specific cpufreq driver support

On 03-04-15, 18:43, Bartlomiej Zolnierkiewicz wrote:
> From: Thomas Abraham <[email protected]>
>
> Exynos4210 based platforms have switched over to use generic
> cpufreq driver for cpufreq functionality. So the Exynos
> specific cpufreq support for these platforms can be removed.
>
> Changes by Bartlomiej:
> - dropped Exynos5250 support removal for now
> - updated exynos-cpufreq.[c,h]
>
> Cc: Viresh Kumar <[email protected]>
> Cc: Javier Martinez Canillas <[email protected]>
> Signed-off-by: Thomas Abraham <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> ---
> drivers/cpufreq/Kconfig.arm | 11 --
> drivers/cpufreq/Makefile | 1 -
> drivers/cpufreq/exynos-cpufreq.c | 5 +-
> drivers/cpufreq/exynos-cpufreq.h | 9 --
> drivers/cpufreq/exynos4210-cpufreq.c | 184 ----------------------------------
> 5 files changed, 1 insertion(+), 209 deletions(-)
> delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c

Acked-by: Viresh Kumar <[email protected]>

2015-05-14 05:07:23

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 5/6] ARM: Exynos: switch to using generic cpufreq driver for Exynos4210

On 03-04-15, 18:43, Bartlomiej Zolnierkiewicz wrote:
> From: Thomas Abraham <[email protected]>
>
> The new CPU clock type allows the use of generic CPUfreq driver.
> Switch Exynos4210 to using generic cpufreq driver.
>
> Changes by Bartlomiej:
> - removed non-Exynos4210 support for now
>
> Cc: Tomasz Figa <[email protected]>
> Cc: Kukjin Kim <[email protected]>
> Cc: Javier Martinez Canillas <[email protected]>
> Signed-off-by: Thomas Abraham <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> ---
> arch/arm/mach-exynos/exynos.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)

Acked-by: Viresh Kumar <[email protected]>

2015-05-14 05:10:51

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 0/6] cpufreq: use generic cpufreq drivers for Exynos4210platform

On 14-05-15, 13:07, Kukjin Kim wrote:
> On 05/13/15 23:08, Bartlomiej Zolnierkiewicz wrote:
> >
> > Hi,
> >
> Hi Bart,
>
> > On Friday, April 03, 2015 06:43:43 PM Bartlomiej Zolnierkiewicz wrote:
> >> Hi,
> >>
> >> This patch series removes the use of Exynos4210 specific support
> >> from cpufreq-exynos driver and enables the use of cpufreq-dt driver
> >> for this platform.
> >
> > Gentle Ping. Mike/Kukjin/Viresh could you please review/ack relevant
> > patches (patches #1-3 are for clock subsystem, patches #4-5 for Exynos
> > mach/dts and patch #6 is for cpufreq subsystem)?

Sorry I thought I already Acked an older version of this set and so
didn't went for it again. Done now.

> Yes, I totally agreed with this patches for arch side changes and this
> approach when Thomas posted.
>
> > Also what is your
> > preferred way to upstream them (patches are not independent so it would
> > be best to merge them through one tree, otherwise synchronization of
> > git pulls between different subsystem trees will be needed)?
> >
> I can provide topic branch for arch side changes even it is small. I
> think once Viresh and Mike make each topic branch based on -rc or the
> smallest changes from each subsystem then I could handle this series or
> Viresh or Mike need to handle this series with merging each topic
> branches in subsystem. I'm fine either way.
>
> Viresh and Mike, how do you think about that?

For cpufreq subsystem changes, you can take them in your tree.

> > I'm still hoping that this patchset will make it into v4.2 as there are
> > no known issues with it (except minor coding nit for patch #5)...
> >
> Sure, why not :-)

One thing that looked wrong to me is the email id of Thomas..
I believe he has already left Samsung and his id wouldn't exist
anymore. Right ?

Then I wouldn't recommend something that doesn't exist to get merged
now. Probably use another email id of his.

Subject: Re: [PATCH 0/6] cpufreq: use generic cpufreq drivers for Exynos4210platform

On Thursday, May 14, 2015 10:40:46 AM Viresh Kumar wrote:
> On 14-05-15, 13:07, Kukjin Kim wrote:
> > On 05/13/15 23:08, Bartlomiej Zolnierkiewicz wrote:
> > >
> > > Hi,
> > >
> > Hi Bart,
> >
> > > On Friday, April 03, 2015 06:43:43 PM Bartlomiej Zolnierkiewicz wrote:
> > >> Hi,
> > >>
> > >> This patch series removes the use of Exynos4210 specific support
> > >> from cpufreq-exynos driver and enables the use of cpufreq-dt driver
> > >> for this platform.
> > >
> > > Gentle Ping. Mike/Kukjin/Viresh could you please review/ack relevant
> > > patches (patches #1-3 are for clock subsystem, patches #4-5 for Exynos
> > > mach/dts and patch #6 is for cpufreq subsystem)?
>
> Sorry I thought I already Acked an older version of this set and so
> didn't went for it again. Done now.

Thanks!

> > Yes, I totally agreed with this patches for arch side changes and this
> > approach when Thomas posted.
> >
> > > Also what is your
> > > preferred way to upstream them (patches are not independent so it would
> > > be best to merge them through one tree, otherwise synchronization of
> > > git pulls between different subsystem trees will be needed)?
> > >
> > I can provide topic branch for arch side changes even it is small. I
> > think once Viresh and Mike make each topic branch based on -rc or the
> > smallest changes from each subsystem then I could handle this series or
> > Viresh or Mike need to handle this series with merging each topic
> > branches in subsystem. I'm fine either way.
> >
> > Viresh and Mike, how do you think about that?
>
> For cpufreq subsystem changes, you can take them in your tree.
>
> > > I'm still hoping that this patchset will make it into v4.2 as there are
> > > no known issues with it (except minor coding nit for patch #5)...
> > >
> > Sure, why not :-)
>
> One thing that looked wrong to me is the email id of Thomas..
> I believe he has already left Samsung and his id wouldn't exist
> anymore. Right ?

This doesn't seem to be a case. His email doesn't bounce and his
id exists (I've just checked this). I think that he is just very
busy with some other work.

> Then I wouldn't recommend something that doesn't exist to get merged
> now. Probably use another email id of his.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

2015-05-14 11:17:25

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 0/6] cpufreq: use generic cpufreq drivers for Exynos4210platform

On 14-05-15, 12:53, Bartlomiej Zolnierkiewicz wrote:
> This doesn't seem to be a case. His email doesn't bounce and his
> id exists (I've just checked this). I think that he is just very
> busy with some other work.

Looks like I was thinking about someone else and thought he was
Thomas. Sorry Thomas. :)

--
viresh

2015-05-14 13:07:29

by Kukjin Kim

[permalink] [raw]
Subject: Re: [PATCH 0/6] cpufreq: use generic cpufreq drivers forExynos4210platform

On 05/14/15 14:10, Viresh Kumar wrote:
> On 14-05-15, 13:07, Kukjin Kim wrote:
>> On 05/13/15 23:08, Bartlomiej Zolnierkiewicz wrote:
>>>
>>> Hi,
>>>
>> Hi Bart,
>>
>>> On Friday, April 03, 2015 06:43:43 PM Bartlomiej Zolnierkiewicz wrote:
>>>> Hi,
>>>>
>>>> This patch series removes the use of Exynos4210 specific support
>>>> from cpufreq-exynos driver and enables the use of cpufreq-dt driver
>>>> for this platform.
>>>
>>> Gentle Ping. Mike/Kukjin/Viresh could you please review/ack relevant
>>> patches (patches #1-3 are for clock subsystem, patches #4-5 for Exynos
>>> mach/dts and patch #6 is for cpufreq subsystem)?
>
> Sorry I thought I already Acked an older version of this set and so
> didn't went for it again. Done now.
>
>> Yes, I totally agreed with this patches for arch side changes and this
>> approach when Thomas posted.
>>
>>> Also what is your
>>> preferred way to upstream them (patches are not independent so it would
>>> be best to merge them through one tree, otherwise synchronization of
>>> git pulls between different subsystem trees will be needed)?
>>>
>> I can provide topic branch for arch side changes even it is small. I
>> think once Viresh and Mike make each topic branch based on -rc or the
>> smallest changes from each subsystem then I could handle this series or
>> Viresh or Mike need to handle this series with merging each topic
>> branches in subsystem. I'm fine either way.
>>
>> Viresh and Mike, how do you think about that?
>
> For cpufreq subsystem changes, you can take them in your tree.
>
Hi Viresh, OK, I will take the cpufreq changes with your ack. Thanks for
your confirmation.

Hi Mike and Sylwester,
How can we handle this series well without any problems? hmm...

>>> I'm still hoping that this patchset will make it into v4.2 as there are
>>> no known issues with it (except minor coding nit for patch #5)...
>>>
>> Sure, why not :-)
>
> One thing that looked wrong to me is the email id of Thomas..
> I believe he has already left Samsung and his id wouldn't exist
> anymore. Right ?
>
> Then I wouldn't recommend something that doesn't exist to get merged
> now. Probably use another email id of his.

As Bart replied, Thomas is still Samsung guy, maybe you meant Tomasz? he
moved to another company last year.

- Kukjin

2015-06-03 23:22:22

by Kukjin Kim

[permalink] [raw]
Subject: Re: [PATCH 0/6] cpufreq: use generic cpufreq drivers forExynos4210platform

On 05/14/15 22:07, Kukjin Kim wrote:
> On 05/14/15 14:10, Viresh Kumar wrote:
>> On 14-05-15, 13:07, Kukjin Kim wrote:
>>> On 05/13/15 23:08, Bartlomiej Zolnierkiewicz wrote:
>>>>
>>>> Hi,
>>>>
>>> Hi Bart,
>>>
>>>> On Friday, April 03, 2015 06:43:43 PM Bartlomiej Zolnierkiewicz wrote:
>>>>> Hi,
>>>>>
>>>>> This patch series removes the use of Exynos4210 specific support
>>>>> from cpufreq-exynos driver and enables the use of cpufreq-dt driver
>>>>> for this platform.
>>>>
>>>> Gentle Ping. Mike/Kukjin/Viresh could you please review/ack relevant
>>>> patches (patches #1-3 are for clock subsystem, patches #4-5 for Exynos
>>>> mach/dts and patch #6 is for cpufreq subsystem)?
>>
>> Sorry I thought I already Acked an older version of this set and so
>> didn't went for it again. Done now.
>>
>>> Yes, I totally agreed with this patches for arch side changes and this
>>> approach when Thomas posted.
>>>
>>>> Also what is your
>>>> preferred way to upstream them (patches are not independent so it would
>>>> be best to merge them through one tree, otherwise synchronization of
>>>> git pulls between different subsystem trees will be needed)?
>>>>
>>> I can provide topic branch for arch side changes even it is small. I
>>> think once Viresh and Mike make each topic branch based on -rc or the
>>> smallest changes from each subsystem then I could handle this series or
>>> Viresh or Mike need to handle this series with merging each topic
>>> branches in subsystem. I'm fine either way.
>>>
>>> Viresh and Mike, how do you think about that?
>>
>> For cpufreq subsystem changes, you can take them in your tree.
>>
> Hi Viresh, OK, I will take the cpufreq changes with your ack. Thanks for
> your confirmation.
>
> Hi Mike and Sylwester,
> How can we handle this series well without any problems? hmm...
>
Still I need to get clock guys' ack or any comments on this series...

- Kukjin

>>>> I'm still hoping that this patchset will make it into v4.2 as there are
>>>> no known issues with it (except minor coding nit for patch #5)...
>>>>
>>> Sure, why not :-)

Subject: Re: [PATCH 0/6] cpufreq: use generic cpufreq drivers forExynos4210platform


Hi,

Mike, could you please take a look at patches #1 and #2 (#1 is a 4 line
change to a Common Clock Framework and #2 is ARM Exynos specific)?

This series has been waiting on your feedback since 3rd of April. :(

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

On Thursday, June 04, 2015 08:22:05 AM Kukjin Kim wrote:
> On 05/14/15 22:07, Kukjin Kim wrote:
> > On 05/14/15 14:10, Viresh Kumar wrote:
> >> On 14-05-15, 13:07, Kukjin Kim wrote:
> >>> On 05/13/15 23:08, Bartlomiej Zolnierkiewicz wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>> Hi Bart,
> >>>
> >>>> On Friday, April 03, 2015 06:43:43 PM Bartlomiej Zolnierkiewicz wrote:
> >>>>> Hi,
> >>>>>
> >>>>> This patch series removes the use of Exynos4210 specific support
> >>>>> from cpufreq-exynos driver and enables the use of cpufreq-dt driver
> >>>>> for this platform.
> >>>>
> >>>> Gentle Ping. Mike/Kukjin/Viresh could you please review/ack relevant
> >>>> patches (patches #1-3 are for clock subsystem, patches #4-5 for Exynos
> >>>> mach/dts and patch #6 is for cpufreq subsystem)?
> >>
> >> Sorry I thought I already Acked an older version of this set and so
> >> didn't went for it again. Done now.
> >>
> >>> Yes, I totally agreed with this patches for arch side changes and this
> >>> approach when Thomas posted.
> >>>
> >>>> Also what is your
> >>>> preferred way to upstream them (patches are not independent so it would
> >>>> be best to merge them through one tree, otherwise synchronization of
> >>>> git pulls between different subsystem trees will be needed)?
> >>>>
> >>> I can provide topic branch for arch side changes even it is small. I
> >>> think once Viresh and Mike make each topic branch based on -rc or the
> >>> smallest changes from each subsystem then I could handle this series or
> >>> Viresh or Mike need to handle this series with merging each topic
> >>> branches in subsystem. I'm fine either way.
> >>>
> >>> Viresh and Mike, how do you think about that?
> >>
> >> For cpufreq subsystem changes, you can take them in your tree.
> >>
> > Hi Viresh, OK, I will take the cpufreq changes with your ack. Thanks for
> > your confirmation.
> >
> > Hi Mike and Sylwester,
> > How can we handle this series well without any problems? hmm...
> >
> Still I need to get clock guys' ack or any comments on this series...
>
> - Kukjin
>
> >>>> I'm still hoping that this patchset will make it into v4.2 as there are
> >>>> no known issues with it (except minor coding nit for patch #5)...
> >>>>
> >>> Sure, why not :-)

Subject: Re: [PATCH 0/6] cpufreq: use generic cpufreq drivers forExynos4210platform

On Thursday, June 18, 2015 07:53:14 PM Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> Mike, could you please take a look at patches #1 and #2 (#1 is a 4 line
> change to a Common Clock Framework and #2 is ARM Exynos specific)?

Oh, and patch #3 (which is also ARM Exynos specific).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> This series has been waiting on your feedback since 3rd of April. :(
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
> On Thursday, June 04, 2015 08:22:05 AM Kukjin Kim wrote:
> > On 05/14/15 22:07, Kukjin Kim wrote:
> > > On 05/14/15 14:10, Viresh Kumar wrote:
> > >> On 14-05-15, 13:07, Kukjin Kim wrote:
> > >>> On 05/13/15 23:08, Bartlomiej Zolnierkiewicz wrote:
> > >>>>
> > >>>> Hi,
> > >>>>
> > >>> Hi Bart,
> > >>>
> > >>>> On Friday, April 03, 2015 06:43:43 PM Bartlomiej Zolnierkiewicz wrote:
> > >>>>> Hi,
> > >>>>>
> > >>>>> This patch series removes the use of Exynos4210 specific support
> > >>>>> from cpufreq-exynos driver and enables the use of cpufreq-dt driver
> > >>>>> for this platform.
> > >>>>
> > >>>> Gentle Ping. Mike/Kukjin/Viresh could you please review/ack relevant
> > >>>> patches (patches #1-3 are for clock subsystem, patches #4-5 for Exynos
> > >>>> mach/dts and patch #6 is for cpufreq subsystem)?
> > >>
> > >> Sorry I thought I already Acked an older version of this set and so
> > >> didn't went for it again. Done now.
> > >>
> > >>> Yes, I totally agreed with this patches for arch side changes and this
> > >>> approach when Thomas posted.
> > >>>
> > >>>> Also what is your
> > >>>> preferred way to upstream them (patches are not independent so it would
> > >>>> be best to merge them through one tree, otherwise synchronization of
> > >>>> git pulls between different subsystem trees will be needed)?
> > >>>>
> > >>> I can provide topic branch for arch side changes even it is small. I
> > >>> think once Viresh and Mike make each topic branch based on -rc or the
> > >>> smallest changes from each subsystem then I could handle this series or
> > >>> Viresh or Mike need to handle this series with merging each topic
> > >>> branches in subsystem. I'm fine either way.
> > >>>
> > >>> Viresh and Mike, how do you think about that?
> > >>
> > >> For cpufreq subsystem changes, you can take them in your tree.
> > >>
> > > Hi Viresh, OK, I will take the cpufreq changes with your ack. Thanks for
> > > your confirmation.
> > >
> > > Hi Mike and Sylwester,
> > > How can we handle this series well without any problems? hmm...
> > >
> > Still I need to get clock guys' ack or any comments on this series...
> >
> > - Kukjin
> >
> > >>>> I'm still hoping that this patchset will make it into v4.2 as there are
> > >>>> no known issues with it (except minor coding nit for patch #5)...
> > >>>>
> > >>> Sure, why not :-)

2015-06-18 19:59:05

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: add CLK_RECALC_NEW_RATES clock flag for Exynos cpu clock support

Quoting Sylwester Nawrocki (2015-05-13 07:13:13)
> On 03/04/15 18:43, Bartlomiej Zolnierkiewicz wrote:
> > This flag is needed to fix the issue with wrong dividers being setup
> > by Common Clock Framework when using the new Exynos cpu clock support.
> >
> > The issue happens because clk_core_set_rate_nolock() calls
> > clk_calc_new_rates(clk, rate) before both pre/post clock notifiers have
> > a chance to run. In case of Exynos cpu clock support pre/post clock
> > notifiers are registered for mout_apll clock which is a parent of armclk
> > cpu clock and dividers are modified in both pre and post clock notifier.
> > This results in wrong dividers values being later programmed by
> > clk_change_rate(top). To workaround the problem CLK_RECALC_NEW_RATES
> > flag is added and it is set for mout_apll clock later so the correct
> > divider values are re-calculated after both pre and post clock notifiers
> > had run.
> >
> > For example when using "performance" governor on Exynos4210 Origen board
> > the cpufreq-dt driver requests to change the frequency from 1000MHz to
> > 1200MHz and after the change state of the relevant clocks is following:
> >
> > Without use of CLK_GET_RATE_NOCACHE flag:
> >
> > fout_apll rate: 1200000000
> > fout_apll_div_2 rate: 600000000
> > mout_clkout_cpu rate: 600000000
> > div_clkout_cpu rate: 600000000
> > clkout_cpu rate: 600000000
> > mout_apll rate: 1200000000
> > armclk rate: 1200000000
> > mout_hpm rate: 1200000000
> > div_copy rate: 300000000
> > div_hpm rate: 300000000
> > mout_core rate: 1200000000
> > div_core rate: 1200000000
> > div_core2 rate: 1200000000
> > arm_clk_div_2 rate: 600000000
> > div_corem0 rate: 300000000
> > div_corem1 rate: 150000000
> > div_periph rate: 300000000
> > div_atb rate: 300000000
> > div_pclk_dbg rate: 150000000
> > sclk_apll rate: 1200000000
> > sclk_apll_div_2 rate: 600000000
> >
> >
> > With use of CLK_GET_RATE_NOCACHE flag:
> >
> > fout_apll rate: 1200000000
> > fout_apll_div_2 rate: 600000000
> > mout_clkout_cpu rate: 600000000
> > div_clkout_cpu rate: 600000000
> > clkout_cpu rate: 600000000
> > mout_apll rate: 1200000000
> > armclk rate: 1200000000
> > mout_hpm rate: 1200000000
> > div_copy rate: 200000000
> > div_hpm rate: 200000000
> > mout_core rate: 1200000000
> > div_core rate: 1200000000
> > div_core2 rate: 1200000000
> > arm_clk_div_2 rate: 600000000
> > div_corem0 rate: 300000000
> > div_corem1 rate: 150000000
> > div_periph rate: 300000000
> > div_atb rate: 240000000
> > div_pclk_dbg rate: 120000000
> > sclk_apll rate: 150000000
> > sclk_apll_div_2 rate: 75000000
> >
> > Without this change cpufreq-dt driver showed ~10 mA larger energy
> > consumption when compared to cpufreq-exynos one when "performance"
> > cpufreq governor was used on Exynos4210 SoC based Origen board.
> >
> > This issue was probably meant to be workarounded by use of
> > CLK_GET_RATE_NOCACHE and CLK_DIVIDER_READ_ONLY clock flags in
> > the original Exynos cpu clock patchset (in "[PATCH v12 6/6] clk:
> > samsung: remove unused clock aliases and update clock flags" patch)
> > but usage of these flags is not sufficient to fix the issue observed.
> >
> > Cc: Thomas Abraham <[email protected]>
> > Cc: Tomasz Figa <[email protected]>
> > Cc: Mike Turquette <[email protected]>
> > Cc: Javier Martinez Canillas <[email protected]>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> > ---
> > drivers/clk/clk.c | 3 +++
> > include/linux/clk-provider.h | 1 +
> > 2 files changed, 4 insertions(+)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index f85c8e2..97cc73e 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -1771,6 +1771,9 @@ static void clk_change_rate(struct clk_core *clk)
> > if (clk->notifier_count && old_rate != clk->rate)
> > __clk_notify(clk, POST_RATE_CHANGE, old_rate, clk->rate);
> >
> > + if (clk->flags & CLK_RECALC_NEW_RATES)
> > + (void)clk_calc_new_rates(clk, clk->new_rate);
> > +
> > /*
> > * Use safe iteration, as change_rate can actually swap parents
> > * for certain clock types.
> > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> > index 28abf1b..8d1aebe 100644
> > --- a/include/linux/clk-provider.h
> > +++ b/include/linux/clk-provider.h
> > @@ -31,6 +31,7 @@
> > #define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */
> > #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
> > #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
> > +#define CLK_RECALC_NEW_RATES BIT(9) /* recalc rates after notifications */
>
> Mike, Stephen, what do you think about this? I'm rather resistant to
> this new flag approach, it looks like a hack. I don't seem to have better
> ideas to address the missing rate recalculation issue though.

I also do not like it. The root of the problem is the use of clk
notifiers to change clk rates. This is also a hack and it points towards
some missing infrastructure in the clock framework.

> I thought about making the cpu clk notifier callback returning a specific
> error value, which would then trigger rate recalculation after
> __clk_notify() call in clk_change_rate() function. This way the trigger
> of the repeated rate recalculation would come from a notifier callback,
> rather than from a clock definition. But in general it would be difficult
> to handle multiple notification callbacks, each of which would attempt
> to trigger the rate recalculation.

The more complexity we add to the notifier callbacks the crazier things
will get. For now I'd like to see Exynos continue to use the
platform-specific CPUfreq drivers until the new coordinated clock rates
infrastructure makes it possible to do this type of stuff without
relying on the notifiers. I'm working on this feature Right Now. I never
like putting dates on upstream stuff but I'd like to see this feature in
4.3 if possible.

Regards,
Mike

>
> --
> Regards,
> Sylwester

Subject: Re: [PATCH 1/6] clk: add CLK_RECALC_NEW_RATES clock flag for Exynos cpu clock support


Hi,

On Thursday, June 18, 2015 12:58:46 PM Michael Turquette wrote:
> Quoting Sylwester Nawrocki (2015-05-13 07:13:13)
> > On 03/04/15 18:43, Bartlomiej Zolnierkiewicz wrote:
> > > This flag is needed to fix the issue with wrong dividers being setup
> > > by Common Clock Framework when using the new Exynos cpu clock support.
> > >
> > > The issue happens because clk_core_set_rate_nolock() calls
> > > clk_calc_new_rates(clk, rate) before both pre/post clock notifiers have
> > > a chance to run. In case of Exynos cpu clock support pre/post clock
> > > notifiers are registered for mout_apll clock which is a parent of armclk
> > > cpu clock and dividers are modified in both pre and post clock notifier.
> > > This results in wrong dividers values being later programmed by
> > > clk_change_rate(top). To workaround the problem CLK_RECALC_NEW_RATES
> > > flag is added and it is set for mout_apll clock later so the correct
> > > divider values are re-calculated after both pre and post clock notifiers
> > > had run.
> > >
> > > For example when using "performance" governor on Exynos4210 Origen board
> > > the cpufreq-dt driver requests to change the frequency from 1000MHz to
> > > 1200MHz and after the change state of the relevant clocks is following:
> > >
> > > Without use of CLK_GET_RATE_NOCACHE flag:
> > >
> > > fout_apll rate: 1200000000
> > > fout_apll_div_2 rate: 600000000
> > > mout_clkout_cpu rate: 600000000
> > > div_clkout_cpu rate: 600000000
> > > clkout_cpu rate: 600000000
> > > mout_apll rate: 1200000000
> > > armclk rate: 1200000000
> > > mout_hpm rate: 1200000000
> > > div_copy rate: 300000000
> > > div_hpm rate: 300000000
> > > mout_core rate: 1200000000
> > > div_core rate: 1200000000
> > > div_core2 rate: 1200000000
> > > arm_clk_div_2 rate: 600000000
> > > div_corem0 rate: 300000000
> > > div_corem1 rate: 150000000
> > > div_periph rate: 300000000
> > > div_atb rate: 300000000
> > > div_pclk_dbg rate: 150000000
> > > sclk_apll rate: 1200000000
> > > sclk_apll_div_2 rate: 600000000
> > >
> > >
> > > With use of CLK_GET_RATE_NOCACHE flag:
> > >
> > > fout_apll rate: 1200000000
> > > fout_apll_div_2 rate: 600000000
> > > mout_clkout_cpu rate: 600000000
> > > div_clkout_cpu rate: 600000000
> > > clkout_cpu rate: 600000000
> > > mout_apll rate: 1200000000
> > > armclk rate: 1200000000
> > > mout_hpm rate: 1200000000
> > > div_copy rate: 200000000
> > > div_hpm rate: 200000000
> > > mout_core rate: 1200000000
> > > div_core rate: 1200000000
> > > div_core2 rate: 1200000000
> > > arm_clk_div_2 rate: 600000000
> > > div_corem0 rate: 300000000
> > > div_corem1 rate: 150000000
> > > div_periph rate: 300000000
> > > div_atb rate: 240000000
> > > div_pclk_dbg rate: 120000000
> > > sclk_apll rate: 150000000
> > > sclk_apll_div_2 rate: 75000000
> > >
> > > Without this change cpufreq-dt driver showed ~10 mA larger energy
> > > consumption when compared to cpufreq-exynos one when "performance"
> > > cpufreq governor was used on Exynos4210 SoC based Origen board.
> > >
> > > This issue was probably meant to be workarounded by use of
> > > CLK_GET_RATE_NOCACHE and CLK_DIVIDER_READ_ONLY clock flags in
> > > the original Exynos cpu clock patchset (in "[PATCH v12 6/6] clk:
> > > samsung: remove unused clock aliases and update clock flags" patch)
> > > but usage of these flags is not sufficient to fix the issue observed.
> > >
> > > Cc: Thomas Abraham <[email protected]>
> > > Cc: Tomasz Figa <[email protected]>
> > > Cc: Mike Turquette <[email protected]>
> > > Cc: Javier Martinez Canillas <[email protected]>
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> > > ---
> > > drivers/clk/clk.c | 3 +++
> > > include/linux/clk-provider.h | 1 +
> > > 2 files changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > > index f85c8e2..97cc73e 100644
> > > --- a/drivers/clk/clk.c
> > > +++ b/drivers/clk/clk.c
> > > @@ -1771,6 +1771,9 @@ static void clk_change_rate(struct clk_core *clk)
> > > if (clk->notifier_count && old_rate != clk->rate)
> > > __clk_notify(clk, POST_RATE_CHANGE, old_rate, clk->rate);
> > >
> > > + if (clk->flags & CLK_RECALC_NEW_RATES)
> > > + (void)clk_calc_new_rates(clk, clk->new_rate);
> > > +
> > > /*
> > > * Use safe iteration, as change_rate can actually swap parents
> > > * for certain clock types.
> > > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> > > index 28abf1b..8d1aebe 100644
> > > --- a/include/linux/clk-provider.h
> > > +++ b/include/linux/clk-provider.h
> > > @@ -31,6 +31,7 @@
> > > #define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */
> > > #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
> > > #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
> > > +#define CLK_RECALC_NEW_RATES BIT(9) /* recalc rates after notifications */
> >
> > Mike, Stephen, what do you think about this? I'm rather resistant to
> > this new flag approach, it looks like a hack. I don't seem to have better
> > ideas to address the missing rate recalculation issue though.
>
> I also do not like it. The root of the problem is the use of clk
> notifiers to change clk rates. This is also a hack and it points towards
> some missing infrastructure in the clock framework.

I'm very surprised by this. Clock changes using clock notifiers in
Thomas' original patchset were Acked by you:

"[PATCH v12 1/6] clk: samsung: add infrastructure to register cpu clocks"
https://www.marc.info/?l=linux-arm-kernel&m=141657613203808&w=2

I've only fixed issues present within the original code (this 4 lines
workaround/hack change to clock subsystem is a result of this), I have
not changed it fundamentally.

> > I thought about making the cpu clk notifier callback returning a specific
> > error value, which would then trigger rate recalculation after
> > __clk_notify() call in clk_change_rate() function. This way the trigger
> > of the repeated rate recalculation would come from a notifier callback,
> > rather than from a clock definition. But in general it would be difficult
> > to handle multiple notification callbacks, each of which would attempt
> > to trigger the rate recalculation.
>
> The more complexity we add to the notifier callbacks the crazier things
> will get. For now I'd like to see Exynos continue to use the
> platform-specific CPUfreq drivers until the new coordinated clock rates
> infrastructure makes it possible to do this type of stuff without
> relying on the notifiers. I'm working on this feature Right Now. I never
> like putting dates on upstream stuff but I'd like to see this feature in
> 4.3 if possible.

So after 2.5 months of waiting on your feedback you're saying that the
current patches (Acked by all other Maintainers and tested on affected
platforms) should be just abandoned and we should wait with re-doing
them on some not yet implemented feature of clock subsystem which would
be available 2 months from now (at earliest)?

Also what should I do about CPUfreq support for SoCs that don't have
platform specific CPUfreq drivers currently and that need current patches
to work (I mean Exynos542x, Exynos5800 and Exynos5433 in the future)?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Subject: Re: [PATCH 1/6] clk: add CLK_RECALC_NEW_RATES clock flag for Exynos cpu clock support

On Friday, June 19, 2015 01:19:06 PM Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> On Thursday, June 18, 2015 12:58:46 PM Michael Turquette wrote:
> > Quoting Sylwester Nawrocki (2015-05-13 07:13:13)
> > > On 03/04/15 18:43, Bartlomiej Zolnierkiewicz wrote:
> > > > This flag is needed to fix the issue with wrong dividers being setup
> > > > by Common Clock Framework when using the new Exynos cpu clock support.
> > > >
> > > > The issue happens because clk_core_set_rate_nolock() calls
> > > > clk_calc_new_rates(clk, rate) before both pre/post clock notifiers have
> > > > a chance to run. In case of Exynos cpu clock support pre/post clock
> > > > notifiers are registered for mout_apll clock which is a parent of armclk
> > > > cpu clock and dividers are modified in both pre and post clock notifier.
> > > > This results in wrong dividers values being later programmed by
> > > > clk_change_rate(top). To workaround the problem CLK_RECALC_NEW_RATES
> > > > flag is added and it is set for mout_apll clock later so the correct
> > > > divider values are re-calculated after both pre and post clock notifiers
> > > > had run.
> > > >
> > > > For example when using "performance" governor on Exynos4210 Origen board
> > > > the cpufreq-dt driver requests to change the frequency from 1000MHz to
> > > > 1200MHz and after the change state of the relevant clocks is following:
> > > >
> > > > Without use of CLK_GET_RATE_NOCACHE flag:
> > > >
> > > > fout_apll rate: 1200000000
> > > > fout_apll_div_2 rate: 600000000
> > > > mout_clkout_cpu rate: 600000000
> > > > div_clkout_cpu rate: 600000000
> > > > clkout_cpu rate: 600000000
> > > > mout_apll rate: 1200000000
> > > > armclk rate: 1200000000
> > > > mout_hpm rate: 1200000000
> > > > div_copy rate: 300000000
> > > > div_hpm rate: 300000000
> > > > mout_core rate: 1200000000
> > > > div_core rate: 1200000000
> > > > div_core2 rate: 1200000000
> > > > arm_clk_div_2 rate: 600000000
> > > > div_corem0 rate: 300000000
> > > > div_corem1 rate: 150000000
> > > > div_periph rate: 300000000
> > > > div_atb rate: 300000000
> > > > div_pclk_dbg rate: 150000000
> > > > sclk_apll rate: 1200000000
> > > > sclk_apll_div_2 rate: 600000000
> > > >
> > > >
> > > > With use of CLK_GET_RATE_NOCACHE flag:
> > > >
> > > > fout_apll rate: 1200000000
> > > > fout_apll_div_2 rate: 600000000
> > > > mout_clkout_cpu rate: 600000000
> > > > div_clkout_cpu rate: 600000000
> > > > clkout_cpu rate: 600000000
> > > > mout_apll rate: 1200000000
> > > > armclk rate: 1200000000
> > > > mout_hpm rate: 1200000000
> > > > div_copy rate: 200000000
> > > > div_hpm rate: 200000000
> > > > mout_core rate: 1200000000
> > > > div_core rate: 1200000000
> > > > div_core2 rate: 1200000000
> > > > arm_clk_div_2 rate: 600000000
> > > > div_corem0 rate: 300000000
> > > > div_corem1 rate: 150000000
> > > > div_periph rate: 300000000
> > > > div_atb rate: 240000000
> > > > div_pclk_dbg rate: 120000000
> > > > sclk_apll rate: 150000000
> > > > sclk_apll_div_2 rate: 75000000
> > > >
> > > > Without this change cpufreq-dt driver showed ~10 mA larger energy
> > > > consumption when compared to cpufreq-exynos one when "performance"
> > > > cpufreq governor was used on Exynos4210 SoC based Origen board.
> > > >
> > > > This issue was probably meant to be workarounded by use of
> > > > CLK_GET_RATE_NOCACHE and CLK_DIVIDER_READ_ONLY clock flags in
> > > > the original Exynos cpu clock patchset (in "[PATCH v12 6/6] clk:
> > > > samsung: remove unused clock aliases and update clock flags" patch)
> > > > but usage of these flags is not sufficient to fix the issue observed.
> > > >
> > > > Cc: Thomas Abraham <[email protected]>
> > > > Cc: Tomasz Figa <[email protected]>
> > > > Cc: Mike Turquette <[email protected]>
> > > > Cc: Javier Martinez Canillas <[email protected]>
> > > > Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> > > > ---
> > > > drivers/clk/clk.c | 3 +++
> > > > include/linux/clk-provider.h | 1 +
> > > > 2 files changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > > > index f85c8e2..97cc73e 100644
> > > > --- a/drivers/clk/clk.c
> > > > +++ b/drivers/clk/clk.c
> > > > @@ -1771,6 +1771,9 @@ static void clk_change_rate(struct clk_core *clk)
> > > > if (clk->notifier_count && old_rate != clk->rate)
> > > > __clk_notify(clk, POST_RATE_CHANGE, old_rate, clk->rate);
> > > >
> > > > + if (clk->flags & CLK_RECALC_NEW_RATES)
> > > > + (void)clk_calc_new_rates(clk, clk->new_rate);
> > > > +
> > > > /*
> > > > * Use safe iteration, as change_rate can actually swap parents
> > > > * for certain clock types.
> > > > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> > > > index 28abf1b..8d1aebe 100644
> > > > --- a/include/linux/clk-provider.h
> > > > +++ b/include/linux/clk-provider.h
> > > > @@ -31,6 +31,7 @@
> > > > #define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */
> > > > #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
> > > > #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
> > > > +#define CLK_RECALC_NEW_RATES BIT(9) /* recalc rates after notifications */
> > >
> > > Mike, Stephen, what do you think about this? I'm rather resistant to
> > > this new flag approach, it looks like a hack. I don't seem to have better
> > > ideas to address the missing rate recalculation issue though.
> >
> > I also do not like it. The root of the problem is the use of clk
> > notifiers to change clk rates. This is also a hack and it points towards
> > some missing infrastructure in the clock framework.
>
> I'm very surprised by this. Clock changes using clock notifiers in
> Thomas' original patchset were Acked by you:
>
> "[PATCH v12 1/6] clk: samsung: add infrastructure to register cpu clocks"
> https://www.marc.info/?l=linux-arm-kernel&m=141657613203808&w=2
>
> I've only fixed issues present within the original code (this 4 lines
> workaround/hack change to clock subsystem is a result of this), I have
> not changed it fundamentally.

Moreover similar changes for rockchip SoCs (which were explicitly based
on Thomas' patches as noted in the code!) have already been merged in
3.18:

http://lkml.iu.edu/hypermail/linux/kernel/1410.1/02644.html

and are available in commit f6fba5f6967dbc062a7c138d67e2314220f5dd04
("clk: rockchip: add new clock-type for the cpuclk").

I understand that my findings have uncovered some clock subsystem
deficiencies which resulted in afterthought about fundamental design
of cpu clocks but I have a feeling that our patches are now being
unjustly punished for making these issues public.

I agree that current patches are not perfect (especially this patch)
but they are good enough IMO. Please also understand that there were
some serious work put into validating and reviewing them.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> > > I thought about making the cpu clk notifier callback returning a specific
> > > error value, which would then trigger rate recalculation after
> > > __clk_notify() call in clk_change_rate() function. This way the trigger
> > > of the repeated rate recalculation would come from a notifier callback,
> > > rather than from a clock definition. But in general it would be difficult
> > > to handle multiple notification callbacks, each of which would attempt
> > > to trigger the rate recalculation.
> >
> > The more complexity we add to the notifier callbacks the crazier things
> > will get. For now I'd like to see Exynos continue to use the
> > platform-specific CPUfreq drivers until the new coordinated clock rates
> > infrastructure makes it possible to do this type of stuff without
> > relying on the notifiers. I'm working on this feature Right Now. I never
> > like putting dates on upstream stuff but I'd like to see this feature in
> > 4.3 if possible.
>
> So after 2.5 months of waiting on your feedback you're saying that the
> current patches (Acked by all other Maintainers and tested on affected
> platforms) should be just abandoned and we should wait with re-doing
> them on some not yet implemented feature of clock subsystem which would
> be available 2 months from now (at earliest)?
>
> Also what should I do about CPUfreq support for SoCs that don't have
> platform specific CPUfreq drivers currently and that need current patches
> to work (I mean Exynos542x, Exynos5800 and Exynos5433 in the future)?
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics

2015-06-20 10:01:32

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: add CLK_RECALC_NEW_RATES clock flag for Exynos cpu clock support

W dniu 19.06.2015 o 23:53, Michael Turquette pisze:
> Quoting Bartlomiej Zolnierkiewicz (2015-06-19 05:35:23)
>> On Friday, June 19, 2015 01:19:06 PM Bartlomiej Zolnierkiewicz wrote:
>>>
>>> Hi,
>>>
>>> On Thursday, June 18, 2015 12:58:46 PM Michael Turquette wrote:
>>>> Quoting Sylwester Nawrocki (2015-05-13 07:13:13)
>>>>> On 03/04/15 18:43, Bartlomiej Zolnierkiewicz wrote:
>>>>>> This flag is needed to fix the issue with wrong dividers being setup
>>>>>> by Common Clock Framework when using the new Exynos cpu clock support.
>>>>>>
>>>>>> The issue happens because clk_core_set_rate_nolock() calls
>>>>>> clk_calc_new_rates(clk, rate) before both pre/post clock notifiers have
>>>>>> a chance to run. In case of Exynos cpu clock support pre/post clock
>>>>>> notifiers are registered for mout_apll clock which is a parent of armclk
>>>>>> cpu clock and dividers are modified in both pre and post clock notifier.
>>>>>> This results in wrong dividers values being later programmed by
>>>>>> clk_change_rate(top). To workaround the problem CLK_RECALC_NEW_RATES
>>>>>> flag is added and it is set for mout_apll clock later so the correct
>>>>>> divider values are re-calculated after both pre and post clock notifiers
>>>>>> had run.
>>>>>>
>>>>>> For example when using "performance" governor on Exynos4210 Origen board
>>>>>> the cpufreq-dt driver requests to change the frequency from 1000MHz to
>>>>>> 1200MHz and after the change state of the relevant clocks is following:
>>>>>>
>>>>>> Without use of CLK_GET_RATE_NOCACHE flag:
>>>>>>
>>>>>> fout_apll rate: 1200000000
>>>>>> fout_apll_div_2 rate: 600000000
>>>>>> mout_clkout_cpu rate: 600000000
>>>>>> div_clkout_cpu rate: 600000000
>>>>>> clkout_cpu rate: 600000000
>>>>>> mout_apll rate: 1200000000
>>>>>> armclk rate: 1200000000
>>>>>> mout_hpm rate: 1200000000
>>>>>> div_copy rate: 300000000
>>>>>> div_hpm rate: 300000000
>>>>>> mout_core rate: 1200000000
>>>>>> div_core rate: 1200000000
>>>>>> div_core2 rate: 1200000000
>>>>>> arm_clk_div_2 rate: 600000000
>>>>>> div_corem0 rate: 300000000
>>>>>> div_corem1 rate: 150000000
>>>>>> div_periph rate: 300000000
>>>>>> div_atb rate: 300000000
>>>>>> div_pclk_dbg rate: 150000000
>>>>>> sclk_apll rate: 1200000000
>>>>>> sclk_apll_div_2 rate: 600000000
>>>>>>
>>>>>>
>>>>>> With use of CLK_GET_RATE_NOCACHE flag:
>>>>>>
>>>>>> fout_apll rate: 1200000000
>>>>>> fout_apll_div_2 rate: 600000000
>>>>>> mout_clkout_cpu rate: 600000000
>>>>>> div_clkout_cpu rate: 600000000
>>>>>> clkout_cpu rate: 600000000
>>>>>> mout_apll rate: 1200000000
>>>>>> armclk rate: 1200000000
>>>>>> mout_hpm rate: 1200000000
>>>>>> div_copy rate: 200000000
>>>>>> div_hpm rate: 200000000
>>>>>> mout_core rate: 1200000000
>>>>>> div_core rate: 1200000000
>>>>>> div_core2 rate: 1200000000
>>>>>> arm_clk_div_2 rate: 600000000
>>>>>> div_corem0 rate: 300000000
>>>>>> div_corem1 rate: 150000000
>>>>>> div_periph rate: 300000000
>>>>>> div_atb rate: 240000000
>>>>>> div_pclk_dbg rate: 120000000
>>>>>> sclk_apll rate: 150000000
>>>>>> sclk_apll_div_2 rate: 75000000
>>>>>>
>>>>>> Without this change cpufreq-dt driver showed ~10 mA larger energy
>>>>>> consumption when compared to cpufreq-exynos one when "performance"
>>>>>> cpufreq governor was used on Exynos4210 SoC based Origen board.
>>>>>>
>>>>>> This issue was probably meant to be workarounded by use of
>>>>>> CLK_GET_RATE_NOCACHE and CLK_DIVIDER_READ_ONLY clock flags in
>>>>>> the original Exynos cpu clock patchset (in "[PATCH v12 6/6] clk:
>>>>>> samsung: remove unused clock aliases and update clock flags" patch)
>>>>>> but usage of these flags is not sufficient to fix the issue observed.
>>>>>>
>>>>>> Cc: Thomas Abraham <[email protected]>
>>>>>> Cc: Tomasz Figa <[email protected]>
>>>>>> Cc: Mike Turquette <[email protected]>
>>>>>> Cc: Javier Martinez Canillas <[email protected]>
>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>>>>>> ---
>>>>>> drivers/clk/clk.c | 3 +++
>>>>>> include/linux/clk-provider.h | 1 +
>>>>>> 2 files changed, 4 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>>>>>> index f85c8e2..97cc73e 100644
>>>>>> --- a/drivers/clk/clk.c
>>>>>> +++ b/drivers/clk/clk.c
>>>>>> @@ -1771,6 +1771,9 @@ static void clk_change_rate(struct clk_core *clk)
>>>>>> if (clk->notifier_count && old_rate != clk->rate)
>>>>>> __clk_notify(clk, POST_RATE_CHANGE, old_rate, clk->rate);
>>>>>>
>>>>>> + if (clk->flags & CLK_RECALC_NEW_RATES)
>>>>>> + (void)clk_calc_new_rates(clk, clk->new_rate);
>>>>>> +
>>>>>> /*
>>>>>> * Use safe iteration, as change_rate can actually swap parents
>>>>>> * for certain clock types.
>>>>>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>>>>>> index 28abf1b..8d1aebe 100644
>>>>>> --- a/include/linux/clk-provider.h
>>>>>> +++ b/include/linux/clk-provider.h
>>>>>> @@ -31,6 +31,7 @@
>>>>>> #define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */
>>>>>> #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
>>>>>> #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
>>>>>> +#define CLK_RECALC_NEW_RATES BIT(9) /* recalc rates after notifications */
>>>>>
>>>>> Mike, Stephen, what do you think about this? I'm rather resistant to
>>>>> this new flag approach, it looks like a hack. I don't seem to have better
>>>>> ideas to address the missing rate recalculation issue though.
>>>>
>>>> I also do not like it. The root of the problem is the use of clk
>>>> notifiers to change clk rates. This is also a hack and it points towards
>>>> some missing infrastructure in the clock framework.
>>>
>>> I'm very surprised by this. Clock changes using clock notifiers in
>>> Thomas' original patchset were Acked by you:
>>>
>>> "[PATCH v12 1/6] clk: samsung: add infrastructure to register cpu clocks"
>>> https://www.marc.info/?l=linux-arm-kernel&m=141657613203808&w=2
>>>
>>> I've only fixed issues present within the original code (this 4 lines
>>> workaround/hack change to clock subsystem is a result of this), I have
>>> not changed it fundamentally.
>>
>> Moreover similar changes for rockchip SoCs (which were explicitly based
>> on Thomas' patches as noted in the code!) have already been merged in
>> 3.18:
>>
>> http://lkml.iu.edu/hypermail/linux/kernel/1410.1/02644.html
>>
>> and are available in commit f6fba5f6967dbc062a7c138d67e2314220f5dd04
>> ("clk: rockchip: add new clock-type for the cpuclk").
>>
>> I understand that my findings have uncovered some clock subsystem
>> deficiencies which resulted in afterthought about fundamental design
>> of cpu clocks but I have a feeling that our patches are now being
>> unjustly punished for making these issues public.
>>
>> I agree that current patches are not perfect (especially this patch)
>> but they are good enough IMO. Please also understand that there were
>> some serious work put into validating and reviewing them.
>
> You know what? You're right.
>
> I don't really like this cpu-clock code (and similarly I don't like
> Tegra's EMC code which requires access to clk_hw_reparent). But I slept
> on this issue overnight and it doesn't seem right for me to hold back
> these patches when the better solution is currently vaporware (I have
> some code but it's far from ready).
>
> It occurs to me that the best decision I can take is to merge it now and
> then force you guys to switch over when the new infrastructure is
> available. That is more reasonable than delaying the patches getting
> pulled.
>
> So how to merge it? Viresh has given his Ack and is OK for the cpufreq
> changes to go through another tree. I can take the whole thing with
> Kukjin's Ack on the ARM dts patch, or we can set up an immutable
> branch.

I already acked the DTS [0] patch and I'm fine with it. However there
would be some conflicts if this went through tree different than
Samsung. There are many Exynos4 DTS changes in the queue for 4.2.

Are there any objections for picking the DTS patch to Samsung tree?

Best regards,
Krzysztof

[0] https://lkml.org/lkml/2015/5/7/999

>
> If you really want to make my life easy you can send a pull request for
> these patches (and the other Exynos cpufreq/cpu-clock series).
>

2015-06-20 19:14:00

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: add CLK_RECALC_NEW_RATES clock flag for Exynos cpu clock support

Quoting Krzysztof Kozlowski (2015-06-20 03:01:12)
> W dniu 19.06.2015 o 23:53, Michael Turquette pisze:
> > Quoting Bartlomiej Zolnierkiewicz (2015-06-19 05:35:23)
> >> On Friday, June 19, 2015 01:19:06 PM Bartlomiej Zolnierkiewicz wrote:
> >>>
> >>> Hi,
> >>>
> >>> On Thursday, June 18, 2015 12:58:46 PM Michael Turquette wrote:
> >>>> Quoting Sylwester Nawrocki (2015-05-13 07:13:13)
> >>>>> On 03/04/15 18:43, Bartlomiej Zolnierkiewicz wrote:
> >>>>>> This flag is needed to fix the issue with wrong dividers being setup
> >>>>>> by Common Clock Framework when using the new Exynos cpu clock support.
> >>>>>>
> >>>>>> The issue happens because clk_core_set_rate_nolock() calls
> >>>>>> clk_calc_new_rates(clk, rate) before both pre/post clock notifiers have
> >>>>>> a chance to run. In case of Exynos cpu clock support pre/post clock
> >>>>>> notifiers are registered for mout_apll clock which is a parent of armclk
> >>>>>> cpu clock and dividers are modified in both pre and post clock notifier.
> >>>>>> This results in wrong dividers values being later programmed by
> >>>>>> clk_change_rate(top). To workaround the problem CLK_RECALC_NEW_RATES
> >>>>>> flag is added and it is set for mout_apll clock later so the correct
> >>>>>> divider values are re-calculated after both pre and post clock notifiers
> >>>>>> had run.
> >>>>>>
> >>>>>> For example when using "performance" governor on Exynos4210 Origen board
> >>>>>> the cpufreq-dt driver requests to change the frequency from 1000MHz to
> >>>>>> 1200MHz and after the change state of the relevant clocks is following:
> >>>>>>
> >>>>>> Without use of CLK_GET_RATE_NOCACHE flag:
> >>>>>>
> >>>>>> fout_apll rate: 1200000000
> >>>>>> fout_apll_div_2 rate: 600000000
> >>>>>> mout_clkout_cpu rate: 600000000
> >>>>>> div_clkout_cpu rate: 600000000
> >>>>>> clkout_cpu rate: 600000000
> >>>>>> mout_apll rate: 1200000000
> >>>>>> armclk rate: 1200000000
> >>>>>> mout_hpm rate: 1200000000
> >>>>>> div_copy rate: 300000000
> >>>>>> div_hpm rate: 300000000
> >>>>>> mout_core rate: 1200000000
> >>>>>> div_core rate: 1200000000
> >>>>>> div_core2 rate: 1200000000
> >>>>>> arm_clk_div_2 rate: 600000000
> >>>>>> div_corem0 rate: 300000000
> >>>>>> div_corem1 rate: 150000000
> >>>>>> div_periph rate: 300000000
> >>>>>> div_atb rate: 300000000
> >>>>>> div_pclk_dbg rate: 150000000
> >>>>>> sclk_apll rate: 1200000000
> >>>>>> sclk_apll_div_2 rate: 600000000
> >>>>>>
> >>>>>>
> >>>>>> With use of CLK_GET_RATE_NOCACHE flag:
> >>>>>>
> >>>>>> fout_apll rate: 1200000000
> >>>>>> fout_apll_div_2 rate: 600000000
> >>>>>> mout_clkout_cpu rate: 600000000
> >>>>>> div_clkout_cpu rate: 600000000
> >>>>>> clkout_cpu rate: 600000000
> >>>>>> mout_apll rate: 1200000000
> >>>>>> armclk rate: 1200000000
> >>>>>> mout_hpm rate: 1200000000
> >>>>>> div_copy rate: 200000000
> >>>>>> div_hpm rate: 200000000
> >>>>>> mout_core rate: 1200000000
> >>>>>> div_core rate: 1200000000
> >>>>>> div_core2 rate: 1200000000
> >>>>>> arm_clk_div_2 rate: 600000000
> >>>>>> div_corem0 rate: 300000000
> >>>>>> div_corem1 rate: 150000000
> >>>>>> div_periph rate: 300000000
> >>>>>> div_atb rate: 240000000
> >>>>>> div_pclk_dbg rate: 120000000
> >>>>>> sclk_apll rate: 150000000
> >>>>>> sclk_apll_div_2 rate: 75000000
> >>>>>>
> >>>>>> Without this change cpufreq-dt driver showed ~10 mA larger energy
> >>>>>> consumption when compared to cpufreq-exynos one when "performance"
> >>>>>> cpufreq governor was used on Exynos4210 SoC based Origen board.
> >>>>>>
> >>>>>> This issue was probably meant to be workarounded by use of
> >>>>>> CLK_GET_RATE_NOCACHE and CLK_DIVIDER_READ_ONLY clock flags in
> >>>>>> the original Exynos cpu clock patchset (in "[PATCH v12 6/6] clk:
> >>>>>> samsung: remove unused clock aliases and update clock flags" patch)
> >>>>>> but usage of these flags is not sufficient to fix the issue observed.
> >>>>>>
> >>>>>> Cc: Thomas Abraham <[email protected]>
> >>>>>> Cc: Tomasz Figa <[email protected]>
> >>>>>> Cc: Mike Turquette <[email protected]>
> >>>>>> Cc: Javier Martinez Canillas <[email protected]>
> >>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> >>>>>> ---
> >>>>>> drivers/clk/clk.c | 3 +++
> >>>>>> include/linux/clk-provider.h | 1 +
> >>>>>> 2 files changed, 4 insertions(+)
> >>>>>>
> >>>>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> >>>>>> index f85c8e2..97cc73e 100644
> >>>>>> --- a/drivers/clk/clk.c
> >>>>>> +++ b/drivers/clk/clk.c
> >>>>>> @@ -1771,6 +1771,9 @@ static void clk_change_rate(struct clk_core *clk)
> >>>>>> if (clk->notifier_count && old_rate != clk->rate)
> >>>>>> __clk_notify(clk, POST_RATE_CHANGE, old_rate, clk->rate);
> >>>>>>
> >>>>>> + if (clk->flags & CLK_RECALC_NEW_RATES)
> >>>>>> + (void)clk_calc_new_rates(clk, clk->new_rate);
> >>>>>> +
> >>>>>> /*
> >>>>>> * Use safe iteration, as change_rate can actually swap parents
> >>>>>> * for certain clock types.
> >>>>>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> >>>>>> index 28abf1b..8d1aebe 100644
> >>>>>> --- a/include/linux/clk-provider.h
> >>>>>> +++ b/include/linux/clk-provider.h
> >>>>>> @@ -31,6 +31,7 @@
> >>>>>> #define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */
> >>>>>> #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
> >>>>>> #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
> >>>>>> +#define CLK_RECALC_NEW_RATES BIT(9) /* recalc rates after notifications */
> >>>>>
> >>>>> Mike, Stephen, what do you think about this? I'm rather resistant to
> >>>>> this new flag approach, it looks like a hack. I don't seem to have better
> >>>>> ideas to address the missing rate recalculation issue though.
> >>>>
> >>>> I also do not like it. The root of the problem is the use of clk
> >>>> notifiers to change clk rates. This is also a hack and it points towards
> >>>> some missing infrastructure in the clock framework.
> >>>
> >>> I'm very surprised by this. Clock changes using clock notifiers in
> >>> Thomas' original patchset were Acked by you:
> >>>
> >>> "[PATCH v12 1/6] clk: samsung: add infrastructure to register cpu clocks"
> >>> https://www.marc.info/?l=linux-arm-kernel&m=141657613203808&w=2
> >>>
> >>> I've only fixed issues present within the original code (this 4 lines
> >>> workaround/hack change to clock subsystem is a result of this), I have
> >>> not changed it fundamentally.
> >>
> >> Moreover similar changes for rockchip SoCs (which were explicitly based
> >> on Thomas' patches as noted in the code!) have already been merged in
> >> 3.18:
> >>
> >> http://lkml.iu.edu/hypermail/linux/kernel/1410.1/02644.html
> >>
> >> and are available in commit f6fba5f6967dbc062a7c138d67e2314220f5dd04
> >> ("clk: rockchip: add new clock-type for the cpuclk").
> >>
> >> I understand that my findings have uncovered some clock subsystem
> >> deficiencies which resulted in afterthought about fundamental design
> >> of cpu clocks but I have a feeling that our patches are now being
> >> unjustly punished for making these issues public.
> >>
> >> I agree that current patches are not perfect (especially this patch)
> >> but they are good enough IMO. Please also understand that there were
> >> some serious work put into validating and reviewing them.
> >
> > You know what? You're right.
> >
> > I don't really like this cpu-clock code (and similarly I don't like
> > Tegra's EMC code which requires access to clk_hw_reparent). But I slept
> > on this issue overnight and it doesn't seem right for me to hold back
> > these patches when the better solution is currently vaporware (I have
> > some code but it's far from ready).
> >
> > It occurs to me that the best decision I can take is to merge it now and
> > then force you guys to switch over when the new infrastructure is
> > available. That is more reasonable than delaying the patches getting
> > pulled.
> >
> > So how to merge it? Viresh has given his Ack and is OK for the cpufreq
> > changes to go through another tree. I can take the whole thing with
> > Kukjin's Ack on the ARM dts patch, or we can set up an immutable
> > branch.
>
> I already acked the DTS [0] patch and I'm fine with it. However there
> would be some conflicts if this went through tree different than
> Samsung. There are many Exynos4 DTS changes in the queue for 4.2.
>
> Are there any objections for picking the DTS patch to Samsung tree?

That's fine with me. I don't foresee any build problems if I take
patches 1-3 and 5-6. So I'll take those and you'll take #4, sound good?

Regards,
Mike

>
> Best regards,
> Krzysztof
>
> [0] https://lkml.org/lkml/2015/5/7/999
>
> >
> > If you really want to make my life easy you can send a pull request for
> > these patches (and the other Exynos cpufreq/cpu-clock series).
> >

2015-06-22 00:07:15

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: add CLK_RECALC_NEW_RATES clock flag for Exynos cpu clock support

On 21.06.2015 04:13, Michael Turquette wrote:
> Quoting Krzysztof Kozlowski (2015-06-20 03:01:12)
>> W dniu 19.06.2015 o 23:53, Michael Turquette pisze:
>>> Quoting Bartlomiej Zolnierkiewicz (2015-06-19 05:35:23)
>>>> On Friday, June 19, 2015 01:19:06 PM Bartlomiej Zolnierkiewicz wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On Thursday, June 18, 2015 12:58:46 PM Michael Turquette wrote:
>>>>>> Quoting Sylwester Nawrocki (2015-05-13 07:13:13)
>>>>>>> On 03/04/15 18:43, Bartlomiej Zolnierkiewicz wrote:
>>>>>>>> This flag is needed to fix the issue with wrong dividers being setup
>>>>>>>> by Common Clock Framework when using the new Exynos cpu clock support.
>>>>>>>>
>>>>>>>> The issue happens because clk_core_set_rate_nolock() calls
>>>>>>>> clk_calc_new_rates(clk, rate) before both pre/post clock notifiers have
>>>>>>>> a chance to run. In case of Exynos cpu clock support pre/post clock
>>>>>>>> notifiers are registered for mout_apll clock which is a parent of armclk
>>>>>>>> cpu clock and dividers are modified in both pre and post clock notifier.
>>>>>>>> This results in wrong dividers values being later programmed by
>>>>>>>> clk_change_rate(top). To workaround the problem CLK_RECALC_NEW_RATES
>>>>>>>> flag is added and it is set for mout_apll clock later so the correct
>>>>>>>> divider values are re-calculated after both pre and post clock notifiers
>>>>>>>> had run.
>>>>>>>>
>>>>>>>> For example when using "performance" governor on Exynos4210 Origen board
>>>>>>>> the cpufreq-dt driver requests to change the frequency from 1000MHz to
>>>>>>>> 1200MHz and after the change state of the relevant clocks is following:
>>>>>>>>
>>>>>>>> Without use of CLK_GET_RATE_NOCACHE flag:
>>>>>>>>
>>>>>>>> fout_apll rate: 1200000000
>>>>>>>> fout_apll_div_2 rate: 600000000
>>>>>>>> mout_clkout_cpu rate: 600000000
>>>>>>>> div_clkout_cpu rate: 600000000
>>>>>>>> clkout_cpu rate: 600000000
>>>>>>>> mout_apll rate: 1200000000
>>>>>>>> armclk rate: 1200000000
>>>>>>>> mout_hpm rate: 1200000000
>>>>>>>> div_copy rate: 300000000
>>>>>>>> div_hpm rate: 300000000
>>>>>>>> mout_core rate: 1200000000
>>>>>>>> div_core rate: 1200000000
>>>>>>>> div_core2 rate: 1200000000
>>>>>>>> arm_clk_div_2 rate: 600000000
>>>>>>>> div_corem0 rate: 300000000
>>>>>>>> div_corem1 rate: 150000000
>>>>>>>> div_periph rate: 300000000
>>>>>>>> div_atb rate: 300000000
>>>>>>>> div_pclk_dbg rate: 150000000
>>>>>>>> sclk_apll rate: 1200000000
>>>>>>>> sclk_apll_div_2 rate: 600000000
>>>>>>>>
>>>>>>>>
>>>>>>>> With use of CLK_GET_RATE_NOCACHE flag:
>>>>>>>>
>>>>>>>> fout_apll rate: 1200000000
>>>>>>>> fout_apll_div_2 rate: 600000000
>>>>>>>> mout_clkout_cpu rate: 600000000
>>>>>>>> div_clkout_cpu rate: 600000000
>>>>>>>> clkout_cpu rate: 600000000
>>>>>>>> mout_apll rate: 1200000000
>>>>>>>> armclk rate: 1200000000
>>>>>>>> mout_hpm rate: 1200000000
>>>>>>>> div_copy rate: 200000000
>>>>>>>> div_hpm rate: 200000000
>>>>>>>> mout_core rate: 1200000000
>>>>>>>> div_core rate: 1200000000
>>>>>>>> div_core2 rate: 1200000000
>>>>>>>> arm_clk_div_2 rate: 600000000
>>>>>>>> div_corem0 rate: 300000000
>>>>>>>> div_corem1 rate: 150000000
>>>>>>>> div_periph rate: 300000000
>>>>>>>> div_atb rate: 240000000
>>>>>>>> div_pclk_dbg rate: 120000000
>>>>>>>> sclk_apll rate: 150000000
>>>>>>>> sclk_apll_div_2 rate: 75000000
>>>>>>>>
>>>>>>>> Without this change cpufreq-dt driver showed ~10 mA larger energy
>>>>>>>> consumption when compared to cpufreq-exynos one when "performance"
>>>>>>>> cpufreq governor was used on Exynos4210 SoC based Origen board.
>>>>>>>>
>>>>>>>> This issue was probably meant to be workarounded by use of
>>>>>>>> CLK_GET_RATE_NOCACHE and CLK_DIVIDER_READ_ONLY clock flags in
>>>>>>>> the original Exynos cpu clock patchset (in "[PATCH v12 6/6] clk:
>>>>>>>> samsung: remove unused clock aliases and update clock flags" patch)
>>>>>>>> but usage of these flags is not sufficient to fix the issue observed.
>>>>>>>>
>>>>>>>> Cc: Thomas Abraham <[email protected]>
>>>>>>>> Cc: Tomasz Figa <[email protected]>
>>>>>>>> Cc: Mike Turquette <[email protected]>
>>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
>>>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>>>>>>>> ---
>>>>>>>> drivers/clk/clk.c | 3 +++
>>>>>>>> include/linux/clk-provider.h | 1 +
>>>>>>>> 2 files changed, 4 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>>>>>>>> index f85c8e2..97cc73e 100644
>>>>>>>> --- a/drivers/clk/clk.c
>>>>>>>> +++ b/drivers/clk/clk.c
>>>>>>>> @@ -1771,6 +1771,9 @@ static void clk_change_rate(struct clk_core *clk)
>>>>>>>> if (clk->notifier_count && old_rate != clk->rate)
>>>>>>>> __clk_notify(clk, POST_RATE_CHANGE, old_rate, clk->rate);
>>>>>>>>
>>>>>>>> + if (clk->flags & CLK_RECALC_NEW_RATES)
>>>>>>>> + (void)clk_calc_new_rates(clk, clk->new_rate);
>>>>>>>> +
>>>>>>>> /*
>>>>>>>> * Use safe iteration, as change_rate can actually swap parents
>>>>>>>> * for certain clock types.
>>>>>>>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>>>>>>>> index 28abf1b..8d1aebe 100644
>>>>>>>> --- a/include/linux/clk-provider.h
>>>>>>>> +++ b/include/linux/clk-provider.h
>>>>>>>> @@ -31,6 +31,7 @@
>>>>>>>> #define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */
>>>>>>>> #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
>>>>>>>> #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
>>>>>>>> +#define CLK_RECALC_NEW_RATES BIT(9) /* recalc rates after notifications */
>>>>>>>
>>>>>>> Mike, Stephen, what do you think about this? I'm rather resistant to
>>>>>>> this new flag approach, it looks like a hack. I don't seem to have better
>>>>>>> ideas to address the missing rate recalculation issue though.
>>>>>>
>>>>>> I also do not like it. The root of the problem is the use of clk
>>>>>> notifiers to change clk rates. This is also a hack and it points towards
>>>>>> some missing infrastructure in the clock framework.
>>>>>
>>>>> I'm very surprised by this. Clock changes using clock notifiers in
>>>>> Thomas' original patchset were Acked by you:
>>>>>
>>>>> "[PATCH v12 1/6] clk: samsung: add infrastructure to register cpu clocks"
>>>>> https://www.marc.info/?l=linux-arm-kernel&m=141657613203808&w=2
>>>>>
>>>>> I've only fixed issues present within the original code (this 4 lines
>>>>> workaround/hack change to clock subsystem is a result of this), I have
>>>>> not changed it fundamentally.
>>>>
>>>> Moreover similar changes for rockchip SoCs (which were explicitly based
>>>> on Thomas' patches as noted in the code!) have already been merged in
>>>> 3.18:
>>>>
>>>> http://lkml.iu.edu/hypermail/linux/kernel/1410.1/02644.html
>>>>
>>>> and are available in commit f6fba5f6967dbc062a7c138d67e2314220f5dd04
>>>> ("clk: rockchip: add new clock-type for the cpuclk").
>>>>
>>>> I understand that my findings have uncovered some clock subsystem
>>>> deficiencies which resulted in afterthought about fundamental design
>>>> of cpu clocks but I have a feeling that our patches are now being
>>>> unjustly punished for making these issues public.
>>>>
>>>> I agree that current patches are not perfect (especially this patch)
>>>> but they are good enough IMO. Please also understand that there were
>>>> some serious work put into validating and reviewing them.
>>>
>>> You know what? You're right.
>>>
>>> I don't really like this cpu-clock code (and similarly I don't like
>>> Tegra's EMC code which requires access to clk_hw_reparent). But I slept
>>> on this issue overnight and it doesn't seem right for me to hold back
>>> these patches when the better solution is currently vaporware (I have
>>> some code but it's far from ready).
>>>
>>> It occurs to me that the best decision I can take is to merge it now and
>>> then force you guys to switch over when the new infrastructure is
>>> available. That is more reasonable than delaying the patches getting
>>> pulled.
>>>
>>> So how to merge it? Viresh has given his Ack and is OK for the cpufreq
>>> changes to go through another tree. I can take the whole thing with
>>> Kukjin's Ack on the ARM dts patch, or we can set up an immutable
>>> branch.
>>
>> I already acked the DTS [0] patch and I'm fine with it. However there
>> would be some conflicts if this went through tree different than
>> Samsung. There are many Exynos4 DTS changes in the queue for 4.2.
>>
>> Are there any objections for picking the DTS patch to Samsung tree?
>
> That's fine with me. I don't foresee any build problems if I take
> patches 1-3 and 5-6. So I'll take those and you'll take #4, sound good?

Yes, sounds good.

Bartlomiej, let us know if you suspect any problems with such approach.

Best regards,
Krzysztof

2015-06-22 00:31:23

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
>> From: Thomas Abraham <[email protected]>
>>
>> For Exynos4210 platforms, add CPU operating points and CPU
>> regulator supply properties for migrating from Exynos specific
>> cpufreq driver to using generic cpufreq driver.
>>
>> Changes by Bartlomiej:
>> - removed Exynos5250 and Exynos5420 support for now
>>
>> Cc: Kukjin Kim <[email protected]>
>> Cc: Doug Anderson <[email protected]>
>> Cc: Javier Martinez Canillas <[email protected]>
>> Cc: Andreas Faerber <[email protected]>
>> Cc: Sachin Kamat <[email protected]>
>> Cc: Andreas Farber <[email protected]>
>> Cc: Javier Martinez Canillas <[email protected]>
>> Signed-off-by: Thomas Abraham <[email protected]>
>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>
> Acked-by: Krzysztof Kozlowski <[email protected]>

Rebased and applied to my tree, I'll sent it later to Kukjin unless he
picks it by himself from LKML.

Best regards,
Krzysztof

2015-06-22 01:39:12

by Kukjin Kim

[permalink] [raw]
Subject: RE: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

Krzysztof Kozlowski wrote:
> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
> > 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
> >> From: Thomas Abraham <[email protected]>
> >>
> >> For Exynos4210 platforms, add CPU operating points and CPU
> >> regulator supply properties for migrating from Exynos specific
> >> cpufreq driver to using generic cpufreq driver.
> >>
> >> Changes by Bartlomiej:
> >> - removed Exynos5250 and Exynos5420 support for now
> >>
> >> Cc: Kukjin Kim <[email protected]>
> >> Cc: Doug Anderson <[email protected]>
> >> Cc: Javier Martinez Canillas <[email protected]>
> >> Cc: Andreas Faerber <[email protected]>
> >> Cc: Sachin Kamat <[email protected]>
> >> Cc: Andreas Farber <[email protected]>
> >> Cc: Javier Martinez Canillas <[email protected]>
> >> Signed-off-by: Thomas Abraham <[email protected]>
> >> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> >
> > Acked-by: Krzysztof Kozlowski <[email protected]>
>
> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
> picks it by himself from LKML.
>
Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
v4.2-rc1 after v4.2-rc1 release.

Thanks,
Kukjin

2015-06-22 01:42:37

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

On 22.06.2015 10:38, Kukjin Kim wrote:
> Krzysztof Kozlowski wrote:
>> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
>>> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
>>>> From: Thomas Abraham <[email protected]>
>>>>
>>>> For Exynos4210 platforms, add CPU operating points and CPU
>>>> regulator supply properties for migrating from Exynos specific
>>>> cpufreq driver to using generic cpufreq driver.
>>>>
>>>> Changes by Bartlomiej:
>>>> - removed Exynos5250 and Exynos5420 support for now
>>>>
>>>> Cc: Kukjin Kim <[email protected]>
>>>> Cc: Doug Anderson <[email protected]>
>>>> Cc: Javier Martinez Canillas <[email protected]>
>>>> Cc: Andreas Faerber <[email protected]>
>>>> Cc: Sachin Kamat <[email protected]>
>>>> Cc: Andreas Farber <[email protected]>
>>>> Cc: Javier Martinez Canillas <[email protected]>
>>>> Signed-off-by: Thomas Abraham <[email protected]>
>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>>>
>>> Acked-by: Krzysztof Kozlowski <[email protected]>
>>
>> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
>> picks it by himself from LKML.
>>
> Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
> v4.2-rc1 after v4.2-rc1 release.

You mean it is for v4.3, not v4.2?

Best regards,
Krzysztof

2015-06-22 01:46:38

by Kukjin Kim

[permalink] [raw]
Subject: RE: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

Krzysztof Kozlowski wrote:
>
> On 22.06.2015 10:38, Kukjin Kim wrote:
> > Krzysztof Kozlowski wrote:
> >> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
> >>> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
> >>>> From: Thomas Abraham <[email protected]>
> >>>>
> >>>> For Exynos4210 platforms, add CPU operating points and CPU
> >>>> regulator supply properties for migrating from Exynos specific
> >>>> cpufreq driver to using generic cpufreq driver.
> >>>>
> >>>> Changes by Bartlomiej:
> >>>> - removed Exynos5250 and Exynos5420 support for now
> >>>>
> >>>> Cc: Kukjin Kim <[email protected]>
> >>>> Cc: Doug Anderson <[email protected]>
> >>>> Cc: Javier Martinez Canillas <[email protected]>
> >>>> Cc: Andreas Faerber <[email protected]>
> >>>> Cc: Sachin Kamat <[email protected]>
> >>>> Cc: Andreas Farber <[email protected]>
> >>>> Cc: Javier Martinez Canillas <[email protected]>
> >>>> Signed-off-by: Thomas Abraham <[email protected]>
> >>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> >>>
> >>> Acked-by: Krzysztof Kozlowski <[email protected]>
> >>
> >> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
> >> picks it by himself from LKML.
> >>
> > Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
> > v4.2-rc1 after v4.2-rc1 release.
>
> You mean it is for v4.3, not v4.2?
>
Oops, yes v4.3.

Thanks for the correction.
Kukjin

2015-06-22 15:04:50

by Michael Turquette

[permalink] [raw]
Subject: RE: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

Quoting Kukjin Kim (2015-06-21 18:46:26)
> Krzysztof Kozlowski wrote:
> >
> > On 22.06.2015 10:38, Kukjin Kim wrote:
> > > Krzysztof Kozlowski wrote:
> > >> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
> > >>> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
> > >>>> From: Thomas Abraham <[email protected]>
> > >>>>
> > >>>> For Exynos4210 platforms, add CPU operating points and CPU
> > >>>> regulator supply properties for migrating from Exynos specific
> > >>>> cpufreq driver to using generic cpufreq driver.
> > >>>>
> > >>>> Changes by Bartlomiej:
> > >>>> - removed Exynos5250 and Exynos5420 support for now
> > >>>>
> > >>>> Cc: Kukjin Kim <[email protected]>
> > >>>> Cc: Doug Anderson <[email protected]>
> > >>>> Cc: Javier Martinez Canillas <[email protected]>
> > >>>> Cc: Andreas Faerber <[email protected]>
> > >>>> Cc: Sachin Kamat <[email protected]>
> > >>>> Cc: Andreas Farber <[email protected]>
> > >>>> Cc: Javier Martinez Canillas <[email protected]>
> > >>>> Signed-off-by: Thomas Abraham <[email protected]>
> > >>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> > >>>
> > >>> Acked-by: Krzysztof Kozlowski <[email protected]>
> > >>
> > >> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
> > >> picks it by himself from LKML.
> > >>
> > > Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
> > > v4.2-rc1 after v4.2-rc1 release.
> >
> > You mean it is for v4.3, not v4.2?
> >
> Oops, yes v4.3.
>
> Thanks for the correction.

Kukjin & Krzysztof,

I'm confused on this point. I was planning to take patches 1, 2, 3, 5
and 6 towards 4.2 (e.g. in the pull request that I'll send out this
week).

Is patch 4 going out for 4.2 or 4.3?

Regards,
Mike

> Kukjin
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> Please read the FAQ at http://www.tux.org/lkml/

2015-06-22 23:46:55

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

On 23.06.2015 00:04, Michael Turquette wrote:
> Quoting Kukjin Kim (2015-06-21 18:46:26)
>> Krzysztof Kozlowski wrote:
>>>
>>> On 22.06.2015 10:38, Kukjin Kim wrote:
>>>> Krzysztof Kozlowski wrote:
>>>>> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
>>>>>> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
>>>>>>> From: Thomas Abraham <[email protected]>
>>>>>>>
>>>>>>> For Exynos4210 platforms, add CPU operating points and CPU
>>>>>>> regulator supply properties for migrating from Exynos specific
>>>>>>> cpufreq driver to using generic cpufreq driver.
>>>>>>>
>>>>>>> Changes by Bartlomiej:
>>>>>>> - removed Exynos5250 and Exynos5420 support for now
>>>>>>>
>>>>>>> Cc: Kukjin Kim <[email protected]>
>>>>>>> Cc: Doug Anderson <[email protected]>
>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
>>>>>>> Cc: Andreas Faerber <[email protected]>
>>>>>>> Cc: Sachin Kamat <[email protected]>
>>>>>>> Cc: Andreas Farber <[email protected]>
>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
>>>>>>> Signed-off-by: Thomas Abraham <[email protected]>
>>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>>>>>>
>>>>>> Acked-by: Krzysztof Kozlowski <[email protected]>
>>>>>
>>>>> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
>>>>> picks it by himself from LKML.
>>>>>
>>>> Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
>>>> v4.2-rc1 after v4.2-rc1 release.
>>>
>>> You mean it is for v4.3, not v4.2?
>>>
>> Oops, yes v4.3.
>>
>> Thanks for the correction.
>
> Kukjin & Krzysztof,
>
> I'm confused on this point. I was planning to take patches 1, 2, 3, 5
> and 6 towards 4.2 (e.g. in the pull request that I'll send out this
> week).
>
> Is patch 4 going out for 4.2 or 4.3?

It is quite late for sending pull request to arm-soc for 4.2.
For example SoCFPGA pull request from last week was rejected:
http://comments.gmane.org/gmane.linux.ports.arm.kernel/417980

If you want to take it for 4.2 then I am fine with it but this will
cause some easy but annoying conflicts. There aren't difficult - just
most of nodes in board DTS changed their place.

Example of resolution (target file after merge, with cpu nodes reordered
alphabetically):
https://github.com/krzk/linux/commit/2cec3cb48abaf44848c62f1c0836b772eb4680dd

Best regards,
Krzysztof

2015-06-23 00:24:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

On 23.06.2015 08:46, Krzysztof Kozlowski wrote:
> On 23.06.2015 00:04, Michael Turquette wrote:
>> Quoting Kukjin Kim (2015-06-21 18:46:26)
>>> Krzysztof Kozlowski wrote:
>>>>
>>>> On 22.06.2015 10:38, Kukjin Kim wrote:
>>>>> Krzysztof Kozlowski wrote:
>>>>>> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
>>>>>>> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
>>>>>>>> From: Thomas Abraham <[email protected]>
>>>>>>>>
>>>>>>>> For Exynos4210 platforms, add CPU operating points and CPU
>>>>>>>> regulator supply properties for migrating from Exynos specific
>>>>>>>> cpufreq driver to using generic cpufreq driver.
>>>>>>>>
>>>>>>>> Changes by Bartlomiej:
>>>>>>>> - removed Exynos5250 and Exynos5420 support for now
>>>>>>>>
>>>>>>>> Cc: Kukjin Kim <[email protected]>
>>>>>>>> Cc: Doug Anderson <[email protected]>
>>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
>>>>>>>> Cc: Andreas Faerber <[email protected]>
>>>>>>>> Cc: Sachin Kamat <[email protected]>
>>>>>>>> Cc: Andreas Farber <[email protected]>
>>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
>>>>>>>> Signed-off-by: Thomas Abraham <[email protected]>
>>>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>>>>>>>
>>>>>>> Acked-by: Krzysztof Kozlowski <[email protected]>
>>>>>>
>>>>>> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
>>>>>> picks it by himself from LKML.
>>>>>>
>>>>> Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
>>>>> v4.2-rc1 after v4.2-rc1 release.
>>>>
>>>> You mean it is for v4.3, not v4.2?
>>>>
>>> Oops, yes v4.3.
>>>
>>> Thanks for the correction.
>>
>> Kukjin & Krzysztof,
>>
>> I'm confused on this point. I was planning to take patches 1, 2, 3, 5
>> and 6 towards 4.2 (e.g. in the pull request that I'll send out this
>> week).
>>
>> Is patch 4 going out for 4.2 or 4.3?
>
> It is quite late for sending pull request to arm-soc for 4.2.
> For example SoCFPGA pull request from last week was rejected:
> http://comments.gmane.org/gmane.linux.ports.arm.kernel/417980

Oh, that was wrong link. Here it is:
http://www.spinics.net/lists/arm-kernel/msg425911.html

> If you want to take it for 4.2 then I am fine with it but this will
> cause some easy but annoying conflicts. There aren't difficult - just
> most of nodes in board DTS changed their place.
>
> Example of resolution (target file after merge, with cpu nodes reordered
> alphabetically):
> https://github.com/krzk/linux/commit/2cec3cb48abaf44848c62f1c0836b772eb4680dd

Best regards,
Krzysztof

Subject: Re: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property


Hi,

On Tuesday, June 23, 2015 08:46:33 AM Krzysztof Kozlowski wrote:
> On 23.06.2015 00:04, Michael Turquette wrote:
> > Quoting Kukjin Kim (2015-06-21 18:46:26)
> >> Krzysztof Kozlowski wrote:
> >>>
> >>> On 22.06.2015 10:38, Kukjin Kim wrote:
> >>>> Krzysztof Kozlowski wrote:
> >>>>> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
> >>>>>> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
> >>>>>>> From: Thomas Abraham <[email protected]>
> >>>>>>>
> >>>>>>> For Exynos4210 platforms, add CPU operating points and CPU
> >>>>>>> regulator supply properties for migrating from Exynos specific
> >>>>>>> cpufreq driver to using generic cpufreq driver.
> >>>>>>>
> >>>>>>> Changes by Bartlomiej:
> >>>>>>> - removed Exynos5250 and Exynos5420 support for now
> >>>>>>>
> >>>>>>> Cc: Kukjin Kim <[email protected]>
> >>>>>>> Cc: Doug Anderson <[email protected]>
> >>>>>>> Cc: Javier Martinez Canillas <[email protected]>
> >>>>>>> Cc: Andreas Faerber <[email protected]>
> >>>>>>> Cc: Sachin Kamat <[email protected]>
> >>>>>>> Cc: Andreas Farber <[email protected]>
> >>>>>>> Cc: Javier Martinez Canillas <[email protected]>
> >>>>>>> Signed-off-by: Thomas Abraham <[email protected]>
> >>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> >>>>>>
> >>>>>> Acked-by: Krzysztof Kozlowski <[email protected]>
> >>>>>
> >>>>> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
> >>>>> picks it by himself from LKML.
> >>>>>
> >>>> Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
> >>>> v4.2-rc1 after v4.2-rc1 release.
> >>>
> >>> You mean it is for v4.3, not v4.2?
> >>>
> >> Oops, yes v4.3.
> >>
> >> Thanks for the correction.
> >
> > Kukjin & Krzysztof,
> >
> > I'm confused on this point. I was planning to take patches 1, 2, 3, 5
> > and 6 towards 4.2 (e.g. in the pull request that I'll send out this
> > week).
> >
> > Is patch 4 going out for 4.2 or 4.3?
>
> It is quite late for sending pull request to arm-soc for 4.2.
> For example SoCFPGA pull request from last week was rejected:
> http://comments.gmane.org/gmane.linux.ports.arm.kernel/417980
>
> If you want to take it for 4.2 then I am fine with it but this will
> cause some easy but annoying conflicts. There aren't difficult - just
> most of nodes in board DTS changed their place.

Taking all patches together would be a preferred solution from my POV.

It would also be great if it could happen in v4.2 (all code changes are
practically limited to our Exynos4210 platform so I hope that it should
be fine) as it would be easier to work on Exynos4x12, Exynos5250 and
Exynos542x/5800 patch series.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> Example of resolution (target file after merge, with cpu nodes reordered
> alphabetically):
> https://github.com/krzk/linux/commit/2cec3cb48abaf44848c62f1c0836b772eb4680dd
>
> Best regards,
> Krzysztof

Subject: Re: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property


Hi,

On Tuesday, June 23, 2015 09:24:40 AM Krzysztof Kozlowski wrote:
> On 23.06.2015 08:46, Krzysztof Kozlowski wrote:
> > On 23.06.2015 00:04, Michael Turquette wrote:
> >> Quoting Kukjin Kim (2015-06-21 18:46:26)
> >>> Krzysztof Kozlowski wrote:
> >>>>
> >>>> On 22.06.2015 10:38, Kukjin Kim wrote:
> >>>>> Krzysztof Kozlowski wrote:
> >>>>>> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
> >>>>>>> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
> >>>>>>>> From: Thomas Abraham <[email protected]>
> >>>>>>>>
> >>>>>>>> For Exynos4210 platforms, add CPU operating points and CPU
> >>>>>>>> regulator supply properties for migrating from Exynos specific
> >>>>>>>> cpufreq driver to using generic cpufreq driver.
> >>>>>>>>
> >>>>>>>> Changes by Bartlomiej:
> >>>>>>>> - removed Exynos5250 and Exynos5420 support for now
> >>>>>>>>
> >>>>>>>> Cc: Kukjin Kim <[email protected]>
> >>>>>>>> Cc: Doug Anderson <[email protected]>
> >>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
> >>>>>>>> Cc: Andreas Faerber <[email protected]>
> >>>>>>>> Cc: Sachin Kamat <[email protected]>
> >>>>>>>> Cc: Andreas Farber <[email protected]>
> >>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
> >>>>>>>> Signed-off-by: Thomas Abraham <[email protected]>
> >>>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> >>>>>>>
> >>>>>>> Acked-by: Krzysztof Kozlowski <[email protected]>
> >>>>>>
> >>>>>> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
> >>>>>> picks it by himself from LKML.
> >>>>>>
> >>>>> Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
> >>>>> v4.2-rc1 after v4.2-rc1 release.
> >>>>
> >>>> You mean it is for v4.3, not v4.2?
> >>>>
> >>> Oops, yes v4.3.
> >>>
> >>> Thanks for the correction.
> >>
> >> Kukjin & Krzysztof,
> >>
> >> I'm confused on this point. I was planning to take patches 1, 2, 3, 5
> >> and 6 towards 4.2 (e.g. in the pull request that I'll send out this
> >> week).
> >>
> >> Is patch 4 going out for 4.2 or 4.3?
> >
> > It is quite late for sending pull request to arm-soc for 4.2.
> > For example SoCFPGA pull request from last week was rejected:
> > http://comments.gmane.org/gmane.linux.ports.arm.kernel/417980
>
> Oh, that was wrong link. Here it is:
> http://www.spinics.net/lists/arm-kernel/msg425911.html
>
> > If you want to take it for 4.2 then I am fine with it but this will
> > cause some easy but annoying conflicts. There aren't difficult - just
> > most of nodes in board DTS changed their place.
> >
> > Example of resolution (target file after merge, with cpu nodes reordered
> > alphabetically):
> > https://github.com/krzk/linux/commit/2cec3cb48abaf44848c62f1c0836b772eb4680dd

This patch is needed for v4.2 as other changes has been already
merged.

Krzysztof/Kukjin, could you please take care of it?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

2015-07-13 11:10:25

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

2015-07-13 20:02 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
>
> Hi,
>
> On Tuesday, June 23, 2015 09:24:40 AM Krzysztof Kozlowski wrote:
>> On 23.06.2015 08:46, Krzysztof Kozlowski wrote:
>> > On 23.06.2015 00:04, Michael Turquette wrote:
>> >> Quoting Kukjin Kim (2015-06-21 18:46:26)
>> >>> Krzysztof Kozlowski wrote:
>> >>>>
>> >>>> On 22.06.2015 10:38, Kukjin Kim wrote:
>> >>>>> Krzysztof Kozlowski wrote:
>> >>>>>> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
>> >>>>>>> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
>> >>>>>>>> From: Thomas Abraham <[email protected]>
>> >>>>>>>>
>> >>>>>>>> For Exynos4210 platforms, add CPU operating points and CPU
>> >>>>>>>> regulator supply properties for migrating from Exynos specific
>> >>>>>>>> cpufreq driver to using generic cpufreq driver.
>> >>>>>>>>
>> >>>>>>>> Changes by Bartlomiej:
>> >>>>>>>> - removed Exynos5250 and Exynos5420 support for now
>> >>>>>>>>
>> >>>>>>>> Cc: Kukjin Kim <[email protected]>
>> >>>>>>>> Cc: Doug Anderson <[email protected]>
>> >>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
>> >>>>>>>> Cc: Andreas Faerber <[email protected]>
>> >>>>>>>> Cc: Sachin Kamat <[email protected]>
>> >>>>>>>> Cc: Andreas Farber <[email protected]>
>> >>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
>> >>>>>>>> Signed-off-by: Thomas Abraham <[email protected]>
>> >>>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>> >>>>>>>
>> >>>>>>> Acked-by: Krzysztof Kozlowski <[email protected]>
>> >>>>>>
>> >>>>>> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
>> >>>>>> picks it by himself from LKML.
>> >>>>>>
>> >>>>> Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
>> >>>>> v4.2-rc1 after v4.2-rc1 release.
>> >>>>
>> >>>> You mean it is for v4.3, not v4.2?
>> >>>>
>> >>> Oops, yes v4.3.
>> >>>
>> >>> Thanks for the correction.
>> >>
>> >> Kukjin & Krzysztof,
>> >>
>> >> I'm confused on this point. I was planning to take patches 1, 2, 3, 5
>> >> and 6 towards 4.2 (e.g. in the pull request that I'll send out this
>> >> week).
>> >>
>> >> Is patch 4 going out for 4.2 or 4.3?
>> >
>> > It is quite late for sending pull request to arm-soc for 4.2.
>> > For example SoCFPGA pull request from last week was rejected:
>> > http://comments.gmane.org/gmane.linux.ports.arm.kernel/417980
>>
>> Oh, that was wrong link. Here it is:
>> http://www.spinics.net/lists/arm-kernel/msg425911.html
>>
>> > If you want to take it for 4.2 then I am fine with it but this will
>> > cause some easy but annoying conflicts. There aren't difficult - just
>> > most of nodes in board DTS changed their place.
>> >
>> > Example of resolution (target file after merge, with cpu nodes reordered
>> > alphabetically):
>> > https://github.com/krzk/linux/commit/2cec3cb48abaf44848c62f1c0836b772eb4680dd
>
> This patch is needed for v4.2 as other changes has been already
> merged.
>
> Krzysztof/Kukjin, could you please take care of it?

Of course! It is already in my queue. I'll send it later to Kukjin for
4.3 (unless he picks it also).

BTW for other patchsets you still need acks from Samsung clock
maintainers. Did you poke Sylwester or Tomasz about it?

Best regards,
Krzysztof

Subject: Re: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

On Monday, July 13, 2015 08:10:21 PM Krzysztof Kozlowski wrote:
> 2015-07-13 20:02 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
> >
> > Hi,
> >
> > On Tuesday, June 23, 2015 09:24:40 AM Krzysztof Kozlowski wrote:
> >> On 23.06.2015 08:46, Krzysztof Kozlowski wrote:
> >> > On 23.06.2015 00:04, Michael Turquette wrote:
> >> >> Quoting Kukjin Kim (2015-06-21 18:46:26)
> >> >>> Krzysztof Kozlowski wrote:
> >> >>>>
> >> >>>> On 22.06.2015 10:38, Kukjin Kim wrote:
> >> >>>>> Krzysztof Kozlowski wrote:
> >> >>>>>> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
> >> >>>>>>> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
> >> >>>>>>>> From: Thomas Abraham <[email protected]>
> >> >>>>>>>>
> >> >>>>>>>> For Exynos4210 platforms, add CPU operating points and CPU
> >> >>>>>>>> regulator supply properties for migrating from Exynos specific
> >> >>>>>>>> cpufreq driver to using generic cpufreq driver.
> >> >>>>>>>>
> >> >>>>>>>> Changes by Bartlomiej:
> >> >>>>>>>> - removed Exynos5250 and Exynos5420 support for now
> >> >>>>>>>>
> >> >>>>>>>> Cc: Kukjin Kim <[email protected]>
> >> >>>>>>>> Cc: Doug Anderson <[email protected]>
> >> >>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
> >> >>>>>>>> Cc: Andreas Faerber <[email protected]>
> >> >>>>>>>> Cc: Sachin Kamat <[email protected]>
> >> >>>>>>>> Cc: Andreas Farber <[email protected]>
> >> >>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
> >> >>>>>>>> Signed-off-by: Thomas Abraham <[email protected]>
> >> >>>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> >> >>>>>>>
> >> >>>>>>> Acked-by: Krzysztof Kozlowski <[email protected]>
> >> >>>>>>
> >> >>>>>> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
> >> >>>>>> picks it by himself from LKML.
> >> >>>>>>
> >> >>>>> Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
> >> >>>>> v4.2-rc1 after v4.2-rc1 release.
> >> >>>>
> >> >>>> You mean it is for v4.3, not v4.2?
> >> >>>>
> >> >>> Oops, yes v4.3.
> >> >>>
> >> >>> Thanks for the correction.
> >> >>
> >> >> Kukjin & Krzysztof,
> >> >>
> >> >> I'm confused on this point. I was planning to take patches 1, 2, 3, 5
> >> >> and 6 towards 4.2 (e.g. in the pull request that I'll send out this
> >> >> week).
> >> >>
> >> >> Is patch 4 going out for 4.2 or 4.3?
> >> >
> >> > It is quite late for sending pull request to arm-soc for 4.2.
> >> > For example SoCFPGA pull request from last week was rejected:
> >> > http://comments.gmane.org/gmane.linux.ports.arm.kernel/417980
> >>
> >> Oh, that was wrong link. Here it is:
> >> http://www.spinics.net/lists/arm-kernel/msg425911.html
> >>
> >> > If you want to take it for 4.2 then I am fine with it but this will
> >> > cause some easy but annoying conflicts. There aren't difficult - just
> >> > most of nodes in board DTS changed their place.
> >> >
> >> > Example of resolution (target file after merge, with cpu nodes reordered
> >> > alphabetically):
> >> > https://github.com/krzk/linux/commit/2cec3cb48abaf44848c62f1c0836b772eb4680dd
> >
> > This patch is needed for v4.2 as other changes has been already
> > merged.
> >
> > Krzysztof/Kukjin, could you please take care of it?
>
> Of course! It is already in my queue. I'll send it later to Kukjin for
> 4.3 (unless he picks it also).

It is in your queue for v4.3 but the patch is needed for v4.2,
without it cpufreq support will not work for Exynos4210 platforms.

> BTW for other patchsets you still need acks from Samsung clock
> maintainers. Did you poke Sylwester or Tomasz about it?

Sylwester, please review/ack Samsung specific clock changes in
Exynos5250 cpufreq and Exynos4x12 cpufreq patch series.

Patch series:

* [PATCH v3 0/4] cpufreq: use generic cpufreq drivers for Exynos5250 platform
(http://lkml.org/lkml/2015/7/1/311)

* [PATCH v2 0/7] cpufreq: use generic cpufreq drivers for Exynos4x12 platform
(http://lkml.org/lkml/2015/7/9/419)

Samsung clock specific changes:

* [PATCH v3 1/4] clk: samsung: exynos5250: add cpu clock configuration data
and instantiate cpu clock
(http://lkml.org/lkml/2015/7/1/313)

* [PATCH v2 4/7] clk: samsung: exynos4x12: add cpu clock configuration data
and instantiate cpu clock
(http://lkml.org/lkml/2015/7/9/424)

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

2015-07-13 11:50:44

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

2015-07-13 20:20 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
> On Monday, July 13, 2015 08:10:21 PM Krzysztof Kozlowski wrote:
>> 2015-07-13 20:02 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
>> >
>> > Hi,
>> >
>> > On Tuesday, June 23, 2015 09:24:40 AM Krzysztof Kozlowski wrote:
>> >> On 23.06.2015 08:46, Krzysztof Kozlowski wrote:
>> >> > On 23.06.2015 00:04, Michael Turquette wrote:
>> >> >> Quoting Kukjin Kim (2015-06-21 18:46:26)
>> >> >>> Krzysztof Kozlowski wrote:
>> >> >>>>
>> >> >>>> On 22.06.2015 10:38, Kukjin Kim wrote:
>> >> >>>>> Krzysztof Kozlowski wrote:
>> >> >>>>>> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
>> >> >>>>>>> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
>> >> >>>>>>>> From: Thomas Abraham <[email protected]>
>> >> >>>>>>>>
>> >> >>>>>>>> For Exynos4210 platforms, add CPU operating points and CPU
>> >> >>>>>>>> regulator supply properties for migrating from Exynos specific
>> >> >>>>>>>> cpufreq driver to using generic cpufreq driver.
>> >> >>>>>>>>
>> >> >>>>>>>> Changes by Bartlomiej:
>> >> >>>>>>>> - removed Exynos5250 and Exynos5420 support for now
>> >> >>>>>>>>
>> >> >>>>>>>> Cc: Kukjin Kim <[email protected]>
>> >> >>>>>>>> Cc: Doug Anderson <[email protected]>
>> >> >>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
>> >> >>>>>>>> Cc: Andreas Faerber <[email protected]>
>> >> >>>>>>>> Cc: Sachin Kamat <[email protected]>
>> >> >>>>>>>> Cc: Andreas Farber <[email protected]>
>> >> >>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
>> >> >>>>>>>> Signed-off-by: Thomas Abraham <[email protected]>
>> >> >>>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>> >> >>>>>>>
>> >> >>>>>>> Acked-by: Krzysztof Kozlowski <[email protected]>
>> >> >>>>>>
>> >> >>>>>> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
>> >> >>>>>> picks it by himself from LKML.
>> >> >>>>>>
>> >> >>>>> Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
>> >> >>>>> v4.2-rc1 after v4.2-rc1 release.
>> >> >>>>
>> >> >>>> You mean it is for v4.3, not v4.2?
>> >> >>>>
>> >> >>> Oops, yes v4.3.
>> >> >>>
>> >> >>> Thanks for the correction.
>> >> >>
>> >> >> Kukjin & Krzysztof,
>> >> >>
>> >> >> I'm confused on this point. I was planning to take patches 1, 2, 3, 5
>> >> >> and 6 towards 4.2 (e.g. in the pull request that I'll send out this
>> >> >> week).
>> >> >>
>> >> >> Is patch 4 going out for 4.2 or 4.3?
>> >> >
>> >> > It is quite late for sending pull request to arm-soc for 4.2.
>> >> > For example SoCFPGA pull request from last week was rejected:
>> >> > http://comments.gmane.org/gmane.linux.ports.arm.kernel/417980
>> >>
>> >> Oh, that was wrong link. Here it is:
>> >> http://www.spinics.net/lists/arm-kernel/msg425911.html
>> >>
>> >> > If you want to take it for 4.2 then I am fine with it but this will
>> >> > cause some easy but annoying conflicts. There aren't difficult - just
>> >> > most of nodes in board DTS changed their place.
>> >> >
>> >> > Example of resolution (target file after merge, with cpu nodes reordered
>> >> > alphabetically):
>> >> > https://github.com/krzk/linux/commit/2cec3cb48abaf44848c62f1c0836b772eb4680dd
>> >
>> > This patch is needed for v4.2 as other changes has been already
>> > merged.
>> >
>> > Krzysztof/Kukjin, could you please take care of it?
>>
>> Of course! It is already in my queue. I'll send it later to Kukjin for
>> 4.3 (unless he picks it also).
>
> It is in your queue for v4.3 but the patch is needed for v4.2,
> without it cpufreq support will not work for Exynos4210 platforms.

Hmm... it makes sense to push it during this rc-cycle to bring back
crippled feature.

Best regards,
Krzysztof

Subject: Re: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

On Monday, July 13, 2015 01:20:41 PM Bartlomiej Zolnierkiewicz wrote:
> On Monday, July 13, 2015 08:10:21 PM Krzysztof Kozlowski wrote:
> > 2015-07-13 20:02 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
> > >
> > > Hi,
> > >
> > > On Tuesday, June 23, 2015 09:24:40 AM Krzysztof Kozlowski wrote:
> > >> On 23.06.2015 08:46, Krzysztof Kozlowski wrote:
> > >> > On 23.06.2015 00:04, Michael Turquette wrote:
> > >> >> Quoting Kukjin Kim (2015-06-21 18:46:26)
> > >> >>> Krzysztof Kozlowski wrote:
> > >> >>>>
> > >> >>>> On 22.06.2015 10:38, Kukjin Kim wrote:
> > >> >>>>> Krzysztof Kozlowski wrote:
> > >> >>>>>> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
> > >> >>>>>>> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
> > >> >>>>>>>> From: Thomas Abraham <[email protected]>
> > >> >>>>>>>>
> > >> >>>>>>>> For Exynos4210 platforms, add CPU operating points and CPU
> > >> >>>>>>>> regulator supply properties for migrating from Exynos specific
> > >> >>>>>>>> cpufreq driver to using generic cpufreq driver.
> > >> >>>>>>>>
> > >> >>>>>>>> Changes by Bartlomiej:
> > >> >>>>>>>> - removed Exynos5250 and Exynos5420 support for now
> > >> >>>>>>>>
> > >> >>>>>>>> Cc: Kukjin Kim <[email protected]>
> > >> >>>>>>>> Cc: Doug Anderson <[email protected]>
> > >> >>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
> > >> >>>>>>>> Cc: Andreas Faerber <[email protected]>
> > >> >>>>>>>> Cc: Sachin Kamat <[email protected]>
> > >> >>>>>>>> Cc: Andreas Farber <[email protected]>
> > >> >>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
> > >> >>>>>>>> Signed-off-by: Thomas Abraham <[email protected]>
> > >> >>>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> > >> >>>>>>>
> > >> >>>>>>> Acked-by: Krzysztof Kozlowski <[email protected]>
> > >> >>>>>>
> > >> >>>>>> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
> > >> >>>>>> picks it by himself from LKML.
> > >> >>>>>>
> > >> >>>>> Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
> > >> >>>>> v4.2-rc1 after v4.2-rc1 release.
> > >> >>>>
> > >> >>>> You mean it is for v4.3, not v4.2?
> > >> >>>>
> > >> >>> Oops, yes v4.3.
> > >> >>>
> > >> >>> Thanks for the correction.
> > >> >>
> > >> >> Kukjin & Krzysztof,
> > >> >>
> > >> >> I'm confused on this point. I was planning to take patches 1, 2, 3, 5
> > >> >> and 6 towards 4.2 (e.g. in the pull request that I'll send out this
> > >> >> week).
> > >> >>
> > >> >> Is patch 4 going out for 4.2 or 4.3?
> > >> >
> > >> > It is quite late for sending pull request to arm-soc for 4.2.
> > >> > For example SoCFPGA pull request from last week was rejected:
> > >> > http://comments.gmane.org/gmane.linux.ports.arm.kernel/417980
> > >>
> > >> Oh, that was wrong link. Here it is:
> > >> http://www.spinics.net/lists/arm-kernel/msg425911.html
> > >>
> > >> > If you want to take it for 4.2 then I am fine with it but this will
> > >> > cause some easy but annoying conflicts. There aren't difficult - just
> > >> > most of nodes in board DTS changed their place.
> > >> >
> > >> > Example of resolution (target file after merge, with cpu nodes reordered
> > >> > alphabetically):
> > >> > https://github.com/krzk/linux/commit/2cec3cb48abaf44848c62f1c0836b772eb4680dd
> > >
> > > This patch is needed for v4.2 as other changes has been already
> > > merged.
> > >
> > > Krzysztof/Kukjin, could you please take care of it?
> >
> > Of course! It is already in my queue. I'll send it later to Kukjin for
> > 4.3 (unless he picks it also).
>
> It is in your queue for v4.3 but the patch is needed for v4.2,
> without it cpufreq support will not work for Exynos4210 platforms.
>
> > BTW for other patchsets you still need acks from Samsung clock
> > maintainers. Did you poke Sylwester or Tomasz about it?
>
> Sylwester, please review/ack Samsung specific clock changes in
> Exynos5250 cpufreq and Exynos4x12 cpufreq patch series.
>
> Patch series:
>
> * [PATCH v3 0/4] cpufreq: use generic cpufreq drivers for Exynos5250 platform
> (http://lkml.org/lkml/2015/7/1/311)
>
> * [PATCH v2 0/7] cpufreq: use generic cpufreq drivers for Exynos4x12 platform
> (http://lkml.org/lkml/2015/7/9/419)
>
> Samsung clock specific changes:
>
> * [PATCH v3 1/4] clk: samsung: exynos5250: add cpu clock configuration data
> and instantiate cpu clock
> (http://lkml.org/lkml/2015/7/1/313)
>
> * [PATCH v2 4/7] clk: samsung: exynos4x12: add cpu clock configuration data
> and instantiate cpu clock
> (http://lkml.org/lkml/2015/7/9/424)

BTW corresponding changes for Exynos4210 has already been merged and 2 above
patches are quite obvious so review should be very quick & simple.

[ Unfortunately Sylwester is currently very busy with a more priority work so
it may still take some time to get his ACKs. ]

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

2015-07-14 00:02:35

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: dts: Exynos4210: add CPU OPP and regulator supply property

On 13.07.2015 23:27, Bartlomiej Zolnierkiewicz wrote:
> On Monday, July 13, 2015 01:20:41 PM Bartlomiej Zolnierkiewicz wrote:
>> On Monday, July 13, 2015 08:10:21 PM Krzysztof Kozlowski wrote:
>>> 2015-07-13 20:02 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
>>>>
>>>> Hi,
>>>>
>>>> On Tuesday, June 23, 2015 09:24:40 AM Krzysztof Kozlowski wrote:
>>>>> On 23.06.2015 08:46, Krzysztof Kozlowski wrote:
>>>>>> On 23.06.2015 00:04, Michael Turquette wrote:
>>>>>>> Quoting Kukjin Kim (2015-06-21 18:46:26)
>>>>>>>> Krzysztof Kozlowski wrote:
>>>>>>>>>
>>>>>>>>> On 22.06.2015 10:38, Kukjin Kim wrote:
>>>>>>>>>> Krzysztof Kozlowski wrote:
>>>>>>>>>>> 2015-05-08 9:18 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
>>>>>>>>>>>> 2015-04-04 1:43 GMT+09:00 Bartlomiej Zolnierkiewicz <[email protected]>:
>>>>>>>>>>>>> From: Thomas Abraham <[email protected]>
>>>>>>>>>>>>>
>>>>>>>>>>>>> For Exynos4210 platforms, add CPU operating points and CPU
>>>>>>>>>>>>> regulator supply properties for migrating from Exynos specific
>>>>>>>>>>>>> cpufreq driver to using generic cpufreq driver.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Changes by Bartlomiej:
>>>>>>>>>>>>> - removed Exynos5250 and Exynos5420 support for now
>>>>>>>>>>>>>
>>>>>>>>>>>>> Cc: Kukjin Kim <[email protected]>
>>>>>>>>>>>>> Cc: Doug Anderson <[email protected]>
>>>>>>>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
>>>>>>>>>>>>> Cc: Andreas Faerber <[email protected]>
>>>>>>>>>>>>> Cc: Sachin Kamat <[email protected]>
>>>>>>>>>>>>> Cc: Andreas Farber <[email protected]>
>>>>>>>>>>>>> Cc: Javier Martinez Canillas <[email protected]>
>>>>>>>>>>>>> Signed-off-by: Thomas Abraham <[email protected]>
>>>>>>>>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>>>>>>>>>>>>
>>>>>>>>>>>> Acked-by: Krzysztof Kozlowski <[email protected]>
>>>>>>>>>>>
>>>>>>>>>>> Rebased and applied to my tree, I'll sent it later to Kukjin unless he
>>>>>>>>>>> picks it by himself from LKML.
>>>>>>>>>>>
>>>>>>>>>> Hi, as far as I know, this is for v4.2 not v4.1 so it will be applied based on
>>>>>>>>>> v4.2-rc1 after v4.2-rc1 release.
>>>>>>>>>
>>>>>>>>> You mean it is for v4.3, not v4.2?
>>>>>>>>>
>>>>>>>> Oops, yes v4.3.
>>>>>>>>
>>>>>>>> Thanks for the correction.
>>>>>>>
>>>>>>> Kukjin & Krzysztof,
>>>>>>>
>>>>>>> I'm confused on this point. I was planning to take patches 1, 2, 3, 5
>>>>>>> and 6 towards 4.2 (e.g. in the pull request that I'll send out this
>>>>>>> week).
>>>>>>>
>>>>>>> Is patch 4 going out for 4.2 or 4.3?
>>>>>>
>>>>>> It is quite late for sending pull request to arm-soc for 4.2.
>>>>>> For example SoCFPGA pull request from last week was rejected:
>>>>>> http://comments.gmane.org/gmane.linux.ports.arm.kernel/417980
>>>>>
>>>>> Oh, that was wrong link. Here it is:
>>>>> http://www.spinics.net/lists/arm-kernel/msg425911.html
>>>>>
>>>>>> If you want to take it for 4.2 then I am fine with it but this will
>>>>>> cause some easy but annoying conflicts. There aren't difficult - just
>>>>>> most of nodes in board DTS changed their place.
>>>>>>
>>>>>> Example of resolution (target file after merge, with cpu nodes reordered
>>>>>> alphabetically):
>>>>>> https://github.com/krzk/linux/commit/2cec3cb48abaf44848c62f1c0836b772eb4680dd
>>>>
>>>> This patch is needed for v4.2 as other changes has been already
>>>> merged.
>>>>
>>>> Krzysztof/Kukjin, could you please take care of it?
>>>
>>> Of course! It is already in my queue. I'll send it later to Kukjin for
>>> 4.3 (unless he picks it also).
>>
>> It is in your queue for v4.3 but the patch is needed for v4.2,
>> without it cpufreq support will not work for Exynos4210 platforms.
>>
>>> BTW for other patchsets you still need acks from Samsung clock
>>> maintainers. Did you poke Sylwester or Tomasz about it?
>>
>> Sylwester, please review/ack Samsung specific clock changes in
>> Exynos5250 cpufreq and Exynos4x12 cpufreq patch series.
>>
>> Patch series:
>>
>> * [PATCH v3 0/4] cpufreq: use generic cpufreq drivers for Exynos5250 platform
>> (http://lkml.org/lkml/2015/7/1/311)
>>
>> * [PATCH v2 0/7] cpufreq: use generic cpufreq drivers for Exynos4x12 platform
>> (http://lkml.org/lkml/2015/7/9/419)
>>
>> Samsung clock specific changes:
>>
>> * [PATCH v3 1/4] clk: samsung: exynos5250: add cpu clock configuration data
>> and instantiate cpu clock
>> (http://lkml.org/lkml/2015/7/1/313)
>>
>> * [PATCH v2 4/7] clk: samsung: exynos4x12: add cpu clock configuration data
>> and instantiate cpu clock
>> (http://lkml.org/lkml/2015/7/9/424)
>
> BTW corresponding changes for Exynos4210 has already been merged and 2 above
> patches are quite obvious so review should be very quick & simple.
>
> [ Unfortunately Sylwester is currently very busy with a more priority work so
> it may still take some time to get his ACKs. ]

The review would be fast and already Javier or me provided such.

Anyway if you want to merge it through different tree it would be
expected to get an ack from bypassed maintainer.

Especially that the distance between you and the bypassed maintainer was
~1 meter for the most of the time :) .

It is much longer distance between you and me now!

Best regards,
Krzysztof