2016-10-10 20:09:52

by Robert Jarzmik

[permalink] [raw]
Subject: [PATCH 0/6] Make pxa core clocks settable

This serie is two fold:
- a set of fixes, patches 1 to 4
- a patch with transfers most of core clock handling from pxa2xx-cpufreq into
clk-pxa*, ie. patch 5
This is the main change, and is providing control over CPU clocks for both
pxa25x and pxa27x through the clock API.

- a last patch (patch 6), which is not to be taken right away, just reviewed,
and which simplifies pxa2xx-cpufreq by removing everything clock related.
This one relies on the former one, but is not necessarily to be taken in the
same cycle, as the cpufreq will continue to work with the former cpufreq
driver even if the clock changes are in.

The patches have been tested on lubbock (pxa25x) and mainstone (pxa27x) boards,
with a cpu-burn and different scaling max frequencies.

Happy review.

--
Robert

Robert Jarzmik (6):
clk: pxa: remove unused variables
clk: pxa: core pll is not affected by t bit
clk: pxa: b bit of clkcfg means fast bus
clk: pxa: export core clocks
clk: pxa: transfer CPU clock setting from pxa2xx-cpufreq
cpufreq: pxa: convert to clock API

drivers/clk/pxa/clk-pxa.c | 128 ++++++++++++++++++++++++++
drivers/clk/pxa/clk-pxa.h | 56 +++++++++++-
drivers/clk/pxa/clk-pxa25x.c | 107 +++++++++++++++++++---
drivers/clk/pxa/clk-pxa27x.c | 168 +++++++++++++++++++++++++++-------
drivers/cpufreq/pxa2xx-cpufreq.c | 191 ++++++++-------------------------------
5 files changed, 453 insertions(+), 197 deletions(-)

--
2.1.4


2016-10-10 20:10:17

by Robert Jarzmik

[permalink] [raw]
Subject: [PATCH 1/6] clk: pxa: remove unused variables

This is a cleanup patch to remove unused values not used in their
respective functions.

Signed-off-by: Robert Jarzmik <[email protected]>
---
drivers/clk/pxa/clk-pxa27x.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/pxa/clk-pxa27x.c b/drivers/clk/pxa/clk-pxa27x.c
index c40b1804f58c..afc395b4148e 100644
--- a/drivers/clk/pxa/clk-pxa27x.c
+++ b/drivers/clk/pxa/clk-pxa27x.c
@@ -221,14 +221,12 @@ static unsigned long clk_pxa27x_core_get_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
unsigned long clkcfg;
- unsigned int t, ht, b, osc_forced;
+ unsigned int ht, osc_forced;
unsigned long ccsr = readl(CCSR);

osc_forced = ccsr & (1 << CCCR_CPDIS_BIT);
asm("mrc\tp14, 0, %0, c6, c0, 0" : "=r" (clkcfg));
- t = clkcfg & (1 << 0);
ht = clkcfg & (1 << 2);
- b = clkcfg & (1 << 3);

if (osc_forced)
return parent_rate;
@@ -241,7 +239,7 @@ static unsigned long clk_pxa27x_core_get_rate(struct clk_hw *hw,
static u8 clk_pxa27x_core_get_parent(struct clk_hw *hw)
{
unsigned long clkcfg;
- unsigned int t, ht, b, osc_forced;
+ unsigned int t, ht, osc_forced;
unsigned long ccsr = readl(CCSR);

osc_forced = ccsr & (1 << CCCR_CPDIS_BIT);
@@ -251,7 +249,6 @@ static u8 clk_pxa27x_core_get_parent(struct clk_hw *hw)
asm("mrc\tp14, 0, %0, c6, c0, 0" : "=r" (clkcfg));
t = clkcfg & (1 << 0);
ht = clkcfg & (1 << 2);
- b = clkcfg & (1 << 3);

if (ht || t)
return PXA_CORE_TURBO;
--
2.1.4

2016-10-10 20:10:16

by Robert Jarzmik

[permalink] [raw]
Subject: [PATCH 2/6] clk: pxa: core pll is not affected by t bit

The t bit of clkfcfg doesn't affect the core pll clock, but it makes core
clock select between core pll clock and core run clock.

As such remove it from the core pll rate reporting function, while it
remains in clk_pxa27x_core_get_parent().

Signed-off-by: Robert Jarzmik <[email protected]>
---
drivers/clk/pxa/clk-pxa25x.c | 4 +---
drivers/clk/pxa/clk-pxa27x.c | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/pxa/clk-pxa25x.c b/drivers/clk/pxa/clk-pxa25x.c
index a98b98e2a9e4..3bb603a27f2b 100644
--- a/drivers/clk/pxa/clk-pxa25x.c
+++ b/drivers/clk/pxa/clk-pxa25x.c
@@ -182,9 +182,7 @@ static unsigned long clk_pxa25x_cpll_get_rate(struct clk_hw *hw,
m = M_clk_mult[(cccr >> 5) & 0x03];
n2 = N2_clk_mult[(cccr >> 7) & 0x07];

- if (t)
- return m * l * n2 * parent_rate / 2;
- return m * l * parent_rate;
+ return m * l * n2 * parent_rate / 2;
}
PARENTS(clk_pxa25x_cpll) = { "osc_3_6864mhz" };
RATE_RO_OPS(clk_pxa25x_cpll, "cpll");
diff --git a/drivers/clk/pxa/clk-pxa27x.c b/drivers/clk/pxa/clk-pxa27x.c
index afc395b4148e..3930053543a3 100644
--- a/drivers/clk/pxa/clk-pxa27x.c
+++ b/drivers/clk/pxa/clk-pxa27x.c
@@ -162,7 +162,7 @@ static unsigned long clk_pxa27x_cpll_get_rate(struct clk_hw *hw,
L = l * parent_rate;
N = (L * n2) / 2;

- return t ? N : L;
+ return N;
}
PARENTS(clk_pxa27x_cpll) = { "osc_13mhz" };
RATE_RO_OPS(clk_pxa27x_cpll, "cpll");
--
2.1.4

2016-10-10 20:10:14

by Robert Jarzmik

[permalink] [raw]
Subject: [PATCH 3/6] clk: pxa: b bit of clkcfg means fast bus

The meaning of this bit was inverted :
- when set to 0, system bus clock is half of the CPU run clock
- when set to 1, system bus clock is the CPU run clock

Signed-off-by: Robert Jarzmik <[email protected]>
---
drivers/clk/pxa/clk-pxa27x.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/pxa/clk-pxa27x.c b/drivers/clk/pxa/clk-pxa27x.c
index 3930053543a3..3b36e8d0f81e 100644
--- a/drivers/clk/pxa/clk-pxa27x.c
+++ b/drivers/clk/pxa/clk-pxa27x.c
@@ -291,9 +291,9 @@ static unsigned long clk_pxa27x_system_bus_get_rate(struct clk_hw *hw,
if (osc_forced)
return parent_rate;
if (b)
- return parent_rate / 2;
- else
return parent_rate;
+ else
+ return parent_rate / 2;
}

static u8 clk_pxa27x_system_bus_get_parent(struct clk_hw *hw)
--
2.1.4

2016-10-10 20:16:57

by Robert Jarzmik

[permalink] [raw]
Subject: [PATCH 4/6] clk: pxa: export core clocks

pxaxxx_get_clk_frequency_khz() needs several clocks to be available
through clk_get(), ie. the cpu clocks, system bus clock and memory
clocks.

Add the missing clkdev so that their rate can be acquired.

Signed-off-by: Robert Jarzmik <[email protected]>
---
drivers/clk/pxa/clk-pxa25x.c | 9 ++++++---
drivers/clk/pxa/clk-pxa27x.c | 13 ++++++++-----
2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/pxa/clk-pxa25x.c b/drivers/clk/pxa/clk-pxa25x.c
index 3bb603a27f2b..6a3009eec830 100644
--- a/drivers/clk/pxa/clk-pxa25x.c
+++ b/drivers/clk/pxa/clk-pxa25x.c
@@ -189,8 +189,10 @@ RATE_RO_OPS(clk_pxa25x_cpll, "cpll");

static void __init pxa25x_register_core(void)
{
- clk_register_clk_pxa25x_cpll();
- clk_register_clk_pxa25x_run();
+ clkdev_pxa_register(CLK_NONE, "cpll", NULL,
+ clk_register_clk_pxa25x_cpll());
+ clkdev_pxa_register(CLK_NONE, "run", NULL,
+ clk_register_clk_pxa25x_run());
clkdev_pxa_register(CLK_CORE, "core", NULL,
clk_register_clk_pxa25x_core());
}
@@ -212,7 +214,8 @@ static void __init pxa25x_base_clocks_init(void)
{
pxa25x_register_plls();
pxa25x_register_core();
- clk_register_clk_pxa25x_memory();
+ clkdev_pxa_register(CLK_NONE, "system_bus", NULL,
+ clk_register_clk_pxa25x_memory());
}

#define DUMMY_CLK(_con_id, _dev_id, _parent) \
diff --git a/drivers/clk/pxa/clk-pxa27x.c b/drivers/clk/pxa/clk-pxa27x.c
index 3b36e8d0f81e..9fb40e884999 100644
--- a/drivers/clk/pxa/clk-pxa27x.c
+++ b/drivers/clk/pxa/clk-pxa27x.c
@@ -270,9 +270,10 @@ RATE_RO_OPS(clk_pxa27x_run, "run");

static void __init pxa27x_register_core(void)
{
- clk_register_clk_pxa27x_cpll();
- clk_register_clk_pxa27x_run();
-
+ clkdev_pxa_register(CLK_NONE, "cpll", NULL,
+ clk_register_clk_pxa27x_cpll());
+ clkdev_pxa_register(CLK_NONE, "run", NULL,
+ clk_register_clk_pxa27x_run());
clkdev_pxa_register(CLK_CORE, "core", NULL,
clk_register_clk_pxa27x_core());
}
@@ -382,8 +383,10 @@ static void __init pxa27x_base_clocks_init(void)
{
pxa27x_register_plls();
pxa27x_register_core();
- clk_register_clk_pxa27x_system_bus();
- clk_register_clk_pxa27x_memory();
+ clkdev_pxa_register(CLK_NONE, "system_bus", NULL,
+ clk_register_clk_pxa27x_system_bus());
+ clkdev_pxa_register(CLK_NONE, "memory", NULL,
+ clk_register_clk_pxa27x_memory());
clk_register_clk_pxa27x_lcd_base();
}

--
2.1.4

2016-10-10 20:17:04

by Robert Jarzmik

[permalink] [raw]
Subject: [PATCH 6/6] cpufreq: pxa: convert to clock API

As the clock settings have been introduced into the clock pxa drivers,
which are now available to change the CPU clock by themselves, remove
the clock handling from this driver, and rely on pxa clock drivers.

Signed-off-by: Robert Jarzmik <[email protected]>
---
drivers/cpufreq/pxa2xx-cpufreq.c | 191 ++++++++-------------------------------
1 file changed, 39 insertions(+), 152 deletions(-)

diff --git a/drivers/cpufreq/pxa2xx-cpufreq.c b/drivers/cpufreq/pxa2xx-cpufreq.c
index ce345bf34d5d..06b024a3e474 100644
--- a/drivers/cpufreq/pxa2xx-cpufreq.c
+++ b/drivers/cpufreq/pxa2xx-cpufreq.c
@@ -58,56 +58,40 @@ module_param(pxa27x_maxfreq, uint, 0);
MODULE_PARM_DESC(pxa27x_maxfreq, "Set the pxa27x maxfreq in MHz"
"(typically 624=>pxa270, 416=>pxa271, 520=>pxa272)");

+struct pxa_cpufreq_data {
+ struct clk *clk_core;
+};
+static struct pxa_cpufreq_data pxa_cpufreq_data;
+
struct pxa_freqs {
unsigned int khz;
- unsigned int membus;
- unsigned int cccr;
- unsigned int div2;
- unsigned int cclkcfg;
int vmin;
int vmax;
};

-/* Define the refresh period in mSec for the SDRAM and the number of rows */
-#define SDRAM_TREF 64 /* standard 64ms SDRAM */
-static unsigned int sdram_rows;
-
-#define CCLKCFG_TURBO 0x1
-#define CCLKCFG_FCS 0x2
-#define CCLKCFG_HALFTURBO 0x4
-#define CCLKCFG_FASTBUS 0x8
-#define MDREFR_DB2_MASK (MDREFR_K2DB2 | MDREFR_K1DB2)
-#define MDREFR_DRI_MASK 0xFFF
-
-#define MDCNFG_DRAC2(mdcnfg) (((mdcnfg) >> 21) & 0x3)
-#define MDCNFG_DRAC0(mdcnfg) (((mdcnfg) >> 5) & 0x3)
-
/*
* PXA255 definitions
*/
-/* Use the run mode frequencies for the CPUFREQ_POLICY_PERFORMANCE policy */
-#define CCLKCFG CCLKCFG_TURBO | CCLKCFG_FCS
-
static const struct pxa_freqs pxa255_run_freqs[] =
{
- /* CPU MEMBUS CCCR DIV2 CCLKCFG run turbo PXbus SDRAM */
- { 99500, 99500, 0x121, 1, CCLKCFG, -1, -1}, /* 99, 99, 50, 50 */
- {132700, 132700, 0x123, 1, CCLKCFG, -1, -1}, /* 133, 133, 66, 66 */
- {199100, 99500, 0x141, 0, CCLKCFG, -1, -1}, /* 199, 199, 99, 99 */
- {265400, 132700, 0x143, 1, CCLKCFG, -1, -1}, /* 265, 265, 133, 66 */
- {331800, 165900, 0x145, 1, CCLKCFG, -1, -1}, /* 331, 331, 166, 83 */
- {398100, 99500, 0x161, 0, CCLKCFG, -1, -1}, /* 398, 398, 196, 99 */
+ /* CPU MEMBUS run turbo PXbus SDRAM */
+ { 99500, -1, -1}, /* 99, 99, 50, 50 */
+ {132700, -1, -1}, /* 133, 133, 66, 66 */
+ {199100, -1, -1}, /* 199, 199, 99, 99 */
+ {265400, -1, -1}, /* 265, 265, 133, 66 */
+ {331800, -1, -1}, /* 331, 331, 166, 83 */
+ {398100, -1, -1}, /* 398, 398, 196, 99 */
};

/* Use the turbo mode frequencies for the CPUFREQ_POLICY_POWERSAVE policy */
static const struct pxa_freqs pxa255_turbo_freqs[] =
{
- /* CPU MEMBUS CCCR DIV2 CCLKCFG run turbo PXbus SDRAM */
- { 99500, 99500, 0x121, 1, CCLKCFG, -1, -1}, /* 99, 99, 50, 50 */
- {199100, 99500, 0x221, 0, CCLKCFG, -1, -1}, /* 99, 199, 50, 99 */
- {298500, 99500, 0x321, 0, CCLKCFG, -1, -1}, /* 99, 287, 50, 99 */
- {298600, 99500, 0x1c1, 0, CCLKCFG, -1, -1}, /* 199, 287, 99, 99 */
- {398100, 99500, 0x241, 0, CCLKCFG, -1, -1}, /* 199, 398, 99, 99 */
+ /* CPU run turbo PXbus SDRAM */
+ { 99500, -1, -1}, /* 99, 99, 50, 50 */
+ {199100, -1, -1}, /* 99, 199, 50, 99 */
+ {298500, -1, -1}, /* 99, 287, 50, 99 */
+ {298600, -1, -1}, /* 199, 287, 99, 99 */
+ {398100, -1, -1}, /* 199, 398, 99, 99 */
};

#define NUM_PXA25x_RUN_FREQS ARRAY_SIZE(pxa255_run_freqs)
@@ -122,47 +106,14 @@ static unsigned int pxa255_turbo_table;
module_param(pxa255_turbo_table, uint, 0);
MODULE_PARM_DESC(pxa255_turbo_table, "Selects the frequency table (0 = run table, !0 = turbo table)");

-/*
- * PXA270 definitions
- *
- * For the PXA27x:
- * Control variables are A, L, 2N for CCCR; B, HT, T for CLKCFG.
- *
- * A = 0 => memory controller clock from table 3-7,
- * A = 1 => memory controller clock = system bus clock
- * Run mode frequency = 13 MHz * L
- * Turbo mode frequency = 13 MHz * L * N
- * System bus frequency = 13 MHz * L / (B + 1)
- *
- * In CCCR:
- * A = 1
- * L = 16 oscillator to run mode ratio
- * 2N = 6 2 * (turbo mode to run mode ratio)
- *
- * In CCLKCFG:
- * B = 1 Fast bus mode
- * HT = 0 Half-Turbo mode
- * T = 1 Turbo mode
- *
- * For now, just support some of the combinations in table 3-7 of
- * PXA27x Processor Family Developer's Manual to simplify frequency
- * change sequences.
- */
-#define PXA27x_CCCR(A, L, N2) (A << 25 | N2 << 7 | L)
-#define CCLKCFG2(B, HT, T) \
- (CCLKCFG_FCS | \
- ((B) ? CCLKCFG_FASTBUS : 0) | \
- ((HT) ? CCLKCFG_HALFTURBO : 0) | \
- ((T) ? CCLKCFG_TURBO : 0))
-
static struct pxa_freqs pxa27x_freqs[] = {
- {104000, 104000, PXA27x_CCCR(1, 8, 2), 0, CCLKCFG2(1, 0, 1), 900000, 1705000 },
- {156000, 104000, PXA27x_CCCR(1, 8, 3), 0, CCLKCFG2(1, 0, 1), 1000000, 1705000 },
- {208000, 208000, PXA27x_CCCR(0, 16, 2), 1, CCLKCFG2(0, 0, 1), 1180000, 1705000 },
- {312000, 208000, PXA27x_CCCR(1, 16, 3), 1, CCLKCFG2(1, 0, 1), 1250000, 1705000 },
- {416000, 208000, PXA27x_CCCR(1, 16, 4), 1, CCLKCFG2(1, 0, 1), 1350000, 1705000 },
- {520000, 208000, PXA27x_CCCR(1, 16, 5), 1, CCLKCFG2(1, 0, 1), 1450000, 1705000 },
- {624000, 208000, PXA27x_CCCR(1, 16, 6), 1, CCLKCFG2(1, 0, 1), 1550000, 1705000 }
+ {104000, 900000, 1705000 },
+ {156000, 1000000, 1705000 },
+ {208000, 1180000, 1705000 },
+ {312000, 1250000, 1705000 },
+ {416000, 1350000, 1705000 },
+ {520000, 1450000, 1705000 },
+ {624000, 1550000, 1705000 }
};

#define NUM_PXA27x_FREQS ARRAY_SIZE(pxa27x_freqs)
@@ -241,51 +192,29 @@ static void pxa27x_guess_max_freq(void)
}
}

-static void init_sdram_rows(void)
-{
- uint32_t mdcnfg = __raw_readl(MDCNFG);
- unsigned int drac2 = 0, drac0 = 0;
-
- if (mdcnfg & (MDCNFG_DE2 | MDCNFG_DE3))
- drac2 = MDCNFG_DRAC2(mdcnfg);
-
- if (mdcnfg & (MDCNFG_DE0 | MDCNFG_DE1))
- drac0 = MDCNFG_DRAC0(mdcnfg);
-
- sdram_rows = 1 << (11 + max(drac0, drac2));
-}
-
-static u32 mdrefr_dri(unsigned int freq)
-{
- u32 interval = freq * SDRAM_TREF / sdram_rows;
-
- return (interval - (cpu_is_pxa27x() ? 31 : 0)) / 32;
-}
-
static unsigned int pxa_cpufreq_get(unsigned int cpu)
{
- return get_clk_frequency_khz(0);
+ struct pxa_cpufreq_data *data = cpufreq_get_driver_data();
+
+ return (unsigned int) clk_get_rate(data->clk_core) / 1000;
}

static int pxa_set_target(struct cpufreq_policy *policy, unsigned int idx)
{
struct cpufreq_frequency_table *pxa_freqs_table;
const struct pxa_freqs *pxa_freq_settings;
- unsigned long flags;
- unsigned int new_freq_cpu, new_freq_mem;
- unsigned int unused, preset_mdrefr, postset_mdrefr, cclkcfg;
+ struct pxa_cpufreq_data *data = cpufreq_get_driver_data();
+ unsigned int new_freq_cpu;
int ret = 0;

/* Get the current policy */
find_freq_tables(&pxa_freqs_table, &pxa_freq_settings);

new_freq_cpu = pxa_freq_settings[idx].khz;
- new_freq_mem = pxa_freq_settings[idx].membus;

if (freq_debug)
- pr_debug("Changing CPU frequency to %d Mhz, (SDRAM %d Mhz)\n",
- new_freq_cpu / 1000, (pxa_freq_settings[idx].div2) ?
- (new_freq_mem / 2000) : (new_freq_mem / 1000));
+ pr_debug("Changing CPU frequency from %d Mhz to %d Mhz\n",
+ policy->cur / 1000, new_freq_cpu / 1000);

if (vcc_core && new_freq_cpu > policy->cur) {
ret = pxa_cpufreq_change_voltage(&pxa_freq_settings[idx]);
@@ -293,53 +222,7 @@ static int pxa_set_target(struct cpufreq_policy *policy, unsigned int idx)
return ret;
}

- /* Calculate the next MDREFR. If we're slowing down the SDRAM clock
- * we need to preset the smaller DRI before the change. If we're
- * speeding up we need to set the larger DRI value after the change.
- */
- preset_mdrefr = postset_mdrefr = __raw_readl(MDREFR);
- if ((preset_mdrefr & MDREFR_DRI_MASK) > mdrefr_dri(new_freq_mem)) {
- preset_mdrefr = (preset_mdrefr & ~MDREFR_DRI_MASK);
- preset_mdrefr |= mdrefr_dri(new_freq_mem);
- }
- postset_mdrefr =
- (postset_mdrefr & ~MDREFR_DRI_MASK) | mdrefr_dri(new_freq_mem);
-
- /* If we're dividing the memory clock by two for the SDRAM clock, this
- * must be set prior to the change. Clearing the divide must be done
- * after the change.
- */
- if (pxa_freq_settings[idx].div2) {
- preset_mdrefr |= MDREFR_DB2_MASK;
- postset_mdrefr |= MDREFR_DB2_MASK;
- } else {
- postset_mdrefr &= ~MDREFR_DB2_MASK;
- }
-
- local_irq_save(flags);
-
- /* Set new the CCCR and prepare CCLKCFG */
- writel(pxa_freq_settings[idx].cccr, CCCR);
- cclkcfg = pxa_freq_settings[idx].cclkcfg;
-
- asm volatile(" \n\
- ldr r4, [%1] /* load MDREFR */ \n\
- b 2f \n\
- .align 5 \n\
-1: \n\
- str %3, [%1] /* preset the MDREFR */ \n\
- mcr p14, 0, %2, c6, c0, 0 /* set CCLKCFG[FCS] */ \n\
- str %4, [%1] /* postset the MDREFR */ \n\
- \n\
- b 3f \n\
-2: b 1b \n\
-3: nop \n\
- "
- : "=&r" (unused)
- : "r" (MDREFR), "r" (cclkcfg),
- "r" (preset_mdrefr), "r" (postset_mdrefr)
- : "r4", "r5");
- local_irq_restore(flags);
+ clk_set_rate(data->clk_core, new_freq_cpu * 1000);

/*
* Even if voltage setting fails, we don't report it, as the frequency
@@ -369,8 +252,6 @@ static int pxa_cpufreq_init(struct cpufreq_policy *policy)

pxa_cpufreq_init_voltages();

- init_sdram_rows();
-
/* set default policy and cpuinfo */
policy->cpuinfo.transition_latency = 1000; /* FIXME: 1 ms, assumed */

@@ -429,11 +310,17 @@ static struct cpufreq_driver pxa_cpufreq_driver = {
.init = pxa_cpufreq_init,
.get = pxa_cpufreq_get,
.name = "PXA2xx",
+ .driver_data = &pxa_cpufreq_data,
};

static int __init pxa_cpu_init(void)
{
int ret = -ENODEV;
+
+ pxa_cpufreq_data.clk_core = clk_get_sys(NULL, "core");
+ if (IS_ERR(pxa_cpufreq_data.clk_core))
+ return PTR_ERR(pxa_cpufreq_data.clk_core);
+
if (cpu_is_pxa25x() || cpu_is_pxa27x())
ret = cpufreq_register_driver(&pxa_cpufreq_driver);
return ret;
--
2.1.4

2016-10-10 20:17:18

by Robert Jarzmik

[permalink] [raw]
Subject: [PATCH 5/6] clk: pxa: transfer CPU clock setting from pxa2xx-cpufreq

This is the initial stage to transfer the pxa25x and pxa27x CPU clocks
handling from cpufreq to the clock API. More precisely, the clocks
transferred are :
- cpll : core pll, known also as the CPU core turbo frequency
- core : core, known also as the CPU actual frequency, being either the
CPU core turbo frequency or the CPU core run frequency

This transfer is a prequel to shrink the code in pxa2xx-cpufreq.c, so
that it can become, at least in devicetree builds, the casual cpufreq-dt
driver.

Signed-off-by: Robert Jarzmik <[email protected]>
---
drivers/clk/pxa/clk-pxa.c | 128 ++++++++++++++++++++++++++++++++++++++
drivers/clk/pxa/clk-pxa.h | 56 ++++++++++++++++-
drivers/clk/pxa/clk-pxa25x.c | 94 ++++++++++++++++++++++++++--
drivers/clk/pxa/clk-pxa27x.c | 144 ++++++++++++++++++++++++++++++++++++-------
4 files changed, 395 insertions(+), 27 deletions(-)

diff --git a/drivers/clk/pxa/clk-pxa.c b/drivers/clk/pxa/clk-pxa.c
index 29cee9e8d4d9..5c59ca9074c0 100644
--- a/drivers/clk/pxa/clk-pxa.c
+++ b/drivers/clk/pxa/clk-pxa.c
@@ -15,9 +15,18 @@
#include <linux/clkdev.h>
#include <linux/of.h>

+#include <mach/pxa2xx-regs.h>
+#include <mach/smemc.h>
+
#include <dt-bindings/clock/pxa-clock.h>
#include "clk-pxa.h"

+#define KHz 1000
+#define MHz (1000 * 1000)
+
+#define MDREFR_DB2_MASK (MDREFR_K2DB2 | MDREFR_K1DB2)
+#define MDREFR_DRI_MASK 0xFFF
+
DEFINE_SPINLOCK(lock);

static struct clk *pxa_clocks[CLK_MAX];
@@ -106,3 +115,122 @@ void __init clk_pxa_dt_common_init(struct device_node *np)
{
of_clk_add_provider(np, of_clk_src_onecell_get, &onecell_data);
}
+
+void pxa2xx_core_turbo_switch(bool on)
+{
+ unsigned long flags;
+ unsigned int unused, clkcfg;
+
+ local_irq_save(flags);
+
+ asm("mrc\tp14, 0, %0, c6, c0, 0" : "=r" (clkcfg));
+ clkcfg &= ~CLKCFG_TURBO & ~CLKCFG_HALFTURBO;
+ if (on)
+ clkcfg |= CLKCFG_TURBO;
+ clkcfg |= CLKCFG_FCS;
+
+ asm volatile(
+ " b 2f\n"
+ " .align 5\n"
+ "1: mcr p14, 0, %1, c6, c0, 0\n"
+ " b 3f\n"
+ "2: b 1b\n"
+ "3: nop\n"
+ : "=&r" (unused)
+ : "r" (clkcfg)
+ : );
+
+ local_irq_restore(flags);
+}
+
+void pxa2xx_cpll_change(struct pxa2xx_freq *freq,
+ u32 (*mdrefr_dri)(unsigned int))
+{
+ unsigned int clkcfg = freq->clkcfg;
+ unsigned int unused, preset_mdrefr, postset_mdrefr;
+ unsigned long flags;
+
+ local_irq_save(flags);
+
+ /* Calculate the next MDREFR. If we're slowing down the SDRAM clock
+ * we need to preset the smaller DRI before the change. If we're
+ * speeding up we need to set the larger DRI value after the change.
+ */
+ preset_mdrefr = postset_mdrefr = __raw_readl(MDREFR);
+ if ((preset_mdrefr & MDREFR_DRI_MASK) > mdrefr_dri(freq->membus_khz)) {
+ preset_mdrefr = (preset_mdrefr & ~MDREFR_DRI_MASK);
+ preset_mdrefr |= mdrefr_dri(freq->membus_khz);
+ }
+ postset_mdrefr =
+ (postset_mdrefr & ~MDREFR_DRI_MASK) |
+ mdrefr_dri(freq->membus_khz);
+
+ /* If we're dividing the memory clock by two for the SDRAM clock, this
+ * must be set prior to the change. Clearing the divide must be done
+ * after the change.
+ */
+ if (freq->div2) {
+ preset_mdrefr |= MDREFR_DB2_MASK;
+ postset_mdrefr |= MDREFR_DB2_MASK;
+ } else {
+ postset_mdrefr &= ~MDREFR_DB2_MASK;
+ }
+
+ /* Set new the CCCR and prepare CLKCFG */
+ writel(freq->cccr, CCCR);
+
+ asm volatile(
+ " ldr r4, [%1]\n"
+ " b 2f\n"
+ " .align 5\n"
+ "1: str %3, [%1] /* preset the MDREFR */\n"
+ " mcr p14, 0, %2, c6, c0, 0 /* set CLKCFG[FCS] */\n"
+ " str %4, [%1] /* postset the MDREFR */\n"
+ " b 3f\n"
+ "2: b 1b\n"
+ "3: nop\n"
+ : "=&r" (unused)
+ : "r" (MDREFR), "r" (clkcfg), "r" (preset_mdrefr),
+ "r" (postset_mdrefr)
+ : "r4", "r5");
+
+ local_irq_restore(flags);
+}
+
+int pxa2xx_determine_rate(struct clk_rate_request *req,
+ struct pxa2xx_freq *freqs, int nb_freqs)
+{
+ int i, closest_below = -1, closest_above = -1;
+ unsigned long rate;
+
+ for (i = 0; i < nb_freqs; i++) {
+ rate = freqs[i].cpll_khz * KHz;
+ if (rate == req->rate)
+ break;
+ if (rate < req->min_rate)
+ continue;
+ if (rate > req->max_rate)
+ continue;
+ if (rate <= req->rate)
+ closest_below = i;
+ if ((rate >= req->rate) && (closest_above == -1))
+ closest_above = i;
+ }
+
+ req->best_parent_hw = NULL;
+
+ if (i < nb_freqs)
+ return 0;
+
+ if (closest_below >= 0) {
+ req->rate = freqs[closest_below].cpll_khz * KHz;
+ return 0;
+ }
+
+ if (closest_above >= 0) {
+ req->rate = freqs[closest_above].cpll_khz * KHz;
+ return 0;
+ }
+
+ return -EINVAL;
+}
diff --git a/drivers/clk/pxa/clk-pxa.h b/drivers/clk/pxa/clk-pxa.h
index d1de805df867..068092605955 100644
--- a/drivers/clk/pxa/clk-pxa.h
+++ b/drivers/clk/pxa/clk-pxa.h
@@ -13,6 +13,11 @@
#ifndef _CLK_PXA_
#define _CLK_PXA_

+#define CLKCFG_TURBO 0x1
+#define CLKCFG_FCS 0x2
+#define CLKCFG_HALFTURBO 0x4
+#define CLKCFG_FASTBUS 0x8
+
#define PARENTS(name) \
static const char *const name ## _parents[] __initconst
#define MUX_RO_RATE_RO_OPS(name, clk_name) \
@@ -35,10 +40,27 @@
NULL, NULL, CLK_GET_RATE_NOCACHE); \
}

-#define RATE_RO_OPS(name, clk_name) \
+#define RATE_RO_OPS(name, clk_name) \
+ static struct clk_hw name ## _rate_hw; \
+ static struct clk_ops name ## _rate_ops = { \
+ .recalc_rate = name ## _get_rate, \
+ }; \
+ static struct clk * __init clk_register_ ## name(void) \
+ { \
+ return clk_register_composite(NULL, clk_name, \
+ name ## _parents, \
+ ARRAY_SIZE(name ## _parents), \
+ NULL, NULL, \
+ &name ## _rate_hw, &name ## _rate_ops, \
+ NULL, NULL, CLK_GET_RATE_NOCACHE); \
+ }
+
+#define RATE_OPS(name, clk_name) \
static struct clk_hw name ## _rate_hw; \
static struct clk_ops name ## _rate_ops = { \
.recalc_rate = name ## _get_rate, \
+ .set_rate = name ## _set_rate, \
+ .determine_rate = name ## _determine_rate, \
}; \
static struct clk * __init clk_register_ ## name(void) \
{ \
@@ -50,6 +72,24 @@
NULL, NULL, CLK_GET_RATE_NOCACHE); \
}

+#define MUX_OPS(name, clk_name, flags) \
+ static struct clk_hw name ## _mux_hw; \
+ static struct clk_ops name ## _mux_ops = { \
+ .get_parent = name ## _get_parent, \
+ .set_parent = name ## _set_parent, \
+ .determine_rate = name ## _determine_rate, \
+ }; \
+ static struct clk * __init clk_register_ ## name(void) \
+ { \
+ return clk_register_composite(NULL, clk_name, \
+ name ## _parents, \
+ ARRAY_SIZE(name ## _parents), \
+ &name ## _mux_hw, &name ## _mux_ops, \
+ NULL, NULL, \
+ NULL, NULL, \
+ CLK_GET_RATE_NOCACHE | flags); \
+ }
+
/*
* CKEN clock type
* This clock takes it source from 2 possible parents :
@@ -95,6 +135,14 @@ struct desc_clk_cken {
PXA_CKEN(dev_id, con_id, name, parents, 1, 1, 1, 1, \
NULL, cken_reg, cken_bit, flag)

+struct pxa2xx_freq {
+ unsigned int cpll_khz;
+ unsigned int membus_khz;
+ unsigned int cccr;
+ unsigned int div2;
+ unsigned int clkcfg;
+};
+
static int dummy_clk_set_parent(struct clk_hw *hw, u8 index)
{
return 0;
@@ -105,4 +153,10 @@ extern void clkdev_pxa_register(int ckid, const char *con_id,
extern int clk_pxa_cken_init(const struct desc_clk_cken *clks, int nb_clks);
void clk_pxa_dt_common_init(struct device_node *np);

+void pxa2xx_core_turbo_switch(bool on);
+void pxa2xx_cpll_change(struct pxa2xx_freq *freq,
+ u32 (*mdrefr_dri)(unsigned int));
+int pxa2xx_determine_rate(struct clk_rate_request *req,
+ struct pxa2xx_freq *freqs, int nb_freqs);
+
#endif
diff --git a/drivers/clk/pxa/clk-pxa25x.c b/drivers/clk/pxa/clk-pxa25x.c
index 6a3009eec830..22f89d2f35cc 100644
--- a/drivers/clk/pxa/clk-pxa25x.c
+++ b/drivers/clk/pxa/clk-pxa25x.c
@@ -18,6 +18,7 @@
#include <linux/io.h>
#include <linux/of.h>
#include <mach/pxa2xx-regs.h>
+#include <mach/smemc.h>

#include <dt-bindings/clock/pxa-clock.h>
#include "clk-pxa.h"
@@ -30,6 +31,17 @@ enum {
PXA_CORE_TURBO,
};

+#define PXA25x_CLKCFG(T) \
+ (CLKCFG_FCS | \
+ ((T) ? CLKCFG_TURBO : 0))
+#define PXA25x_CCCR(N2, M, L) (N2 << 7 | M << 5 | L)
+
+#define MDCNFG_DRAC2(mdcnfg) (((mdcnfg) >> 21) & 0x3)
+#define MDCNFG_DRAC0(mdcnfg) (((mdcnfg) >> 5) & 0x3)
+
+/* Define the refresh period in mSec for the SDRAM and the number of rows */
+#define SDRAM_TREF 64 /* standard 64ms SDRAM */
+
/*
* Various clock factors driven by the CCCR register.
*/
@@ -48,6 +60,34 @@ static const char * const get_freq_khz[] = {
"core", "run", "cpll", "memory"
};

+static int get_sdram_rows(void)
+{
+ static int sdram_rows;
+ unsigned int drac2 = 0, drac0 = 0;
+ u32 mdcnfg;
+
+ if (sdram_rows)
+ return sdram_rows;
+
+ mdcnfg = __raw_readl(MDCNFG);
+
+ if (mdcnfg & (MDCNFG_DE2 | MDCNFG_DE3))
+ drac2 = MDCNFG_DRAC2(mdcnfg);
+
+ if (mdcnfg & (MDCNFG_DE0 | MDCNFG_DE1))
+ drac0 = MDCNFG_DRAC0(mdcnfg);
+
+ sdram_rows = 1 << (11 + max(drac0, drac2));
+ return sdram_rows;
+}
+
+static u32 mdrefr_dri(unsigned int freq_khz)
+{
+ u32 interval = freq_khz * SDRAM_TREF / get_sdram_rows();
+
+ return interval / 32;
+}
+
/*
* Get the clock frequency as reflected by CCCR and the turbo flag.
* We assume these values have been applied via a fcs.
@@ -139,6 +179,15 @@ static struct desc_clk_cken pxa25x_clocks[] __initdata = {
clk_pxa25x_memory_parents, 0),
};

+static struct pxa2xx_freq pxa25x_freqs[] = {
+ /* CPU MEMBUS CCCR DIV2 CCLKCFG */
+ { 99500, 99500, PXA25x_CCCR(2, 1, 1), 1, PXA25x_CLKCFG(1)},
+ {199100, 99500, PXA25x_CCCR(4, 1, 1), 0, PXA25x_CLKCFG(1)},
+ {298500, 99500, PXA25x_CCCR(6, 1, 1), 0, PXA25x_CLKCFG(1)},
+ {298600, 99500, PXA25x_CCCR(2, 1, 16), 0, PXA25x_CLKCFG(1)},
+ {398100, 99500, PXA25x_CCCR(4, 2, 1), 0, PXA25x_CLKCFG(1)},
+};
+
static u8 clk_pxa25x_core_get_parent(struct clk_hw *hw)
{
unsigned long clkcfg;
@@ -151,13 +200,24 @@ static u8 clk_pxa25x_core_get_parent(struct clk_hw *hw)
return PXA_CORE_RUN;
}

-static unsigned long clk_pxa25x_core_get_rate(struct clk_hw *hw,
- unsigned long parent_rate)
+static int clk_pxa25x_core_set_parent(struct clk_hw *hw, u8 index)
{
- return parent_rate;
+ if (index > PXA_CORE_TURBO)
+ return -EINVAL;
+
+ pxa2xx_core_turbo_switch(index == PXA_CORE_TURBO);
+
+ return 0;
+}
+
+static int clk_pxa25x_core_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ return __clk_mux_determine_rate(hw, req);
}
+
PARENTS(clk_pxa25x_core) = { "run", "cpll" };
-MUX_RO_RATE_RO_OPS(clk_pxa25x_core, "core");
+MUX_OPS(clk_pxa25x_core, "core", CLK_SET_RATE_PARENT);

static unsigned long clk_pxa25x_run_get_rate(struct clk_hw *hw,
unsigned long parent_rate)
@@ -184,8 +244,32 @@ static unsigned long clk_pxa25x_cpll_get_rate(struct clk_hw *hw,

return m * l * n2 * parent_rate / 2;
}
+
+static int clk_pxa25x_cpll_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ return pxa2xx_determine_rate(req, pxa25x_freqs,
+ ARRAY_SIZE(pxa25x_freqs));
+}
+
+static int clk_pxa25x_cpll_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pxa25x_freqs); i++)
+ if (pxa25x_freqs[i].cpll_khz * KHz == rate)
+ break;
+
+ if (i >= ARRAY_SIZE(pxa25x_freqs))
+ return -EINVAL;
+
+ pxa2xx_cpll_change(&pxa25x_freqs[i], mdrefr_dri);
+
+ return 0;
+}
PARENTS(clk_pxa25x_cpll) = { "osc_3_6864mhz" };
-RATE_RO_OPS(clk_pxa25x_cpll, "cpll");
+RATE_OPS(clk_pxa25x_cpll, "cpll");

static void __init pxa25x_register_core(void)
{
diff --git a/drivers/clk/pxa/clk-pxa27x.c b/drivers/clk/pxa/clk-pxa27x.c
index 9fb40e884999..2bf450bbdc5a 100644
--- a/drivers/clk/pxa/clk-pxa27x.c
+++ b/drivers/clk/pxa/clk-pxa27x.c
@@ -17,6 +17,8 @@
#include <linux/clkdev.h>
#include <linux/of.h>

+#include <mach/smemc.h>
+
#include <dt-bindings/clock/pxa-clock.h>
#include "clk-pxa.h"

@@ -45,11 +47,52 @@ enum {
PXA_MEM_RUN,
};

+#define PXA27x_CLKCFG(B, HT, T) \
+ (CLKCFG_FCS | \
+ ((B) ? CLKCFG_FASTBUS : 0) | \
+ ((HT) ? CLKCFG_HALFTURBO : 0) | \
+ ((T) ? CLKCFG_TURBO : 0))
+#define PXA27x_CCCR(A, L, N2) (A << 25 | N2 << 7 | L)
+
+#define MDCNFG_DRAC2(mdcnfg) (((mdcnfg) >> 21) & 0x3)
+#define MDCNFG_DRAC0(mdcnfg) (((mdcnfg) >> 5) & 0x3)
+
+/* Define the refresh period in mSec for the SDRAM and the number of rows */
+#define SDRAM_TREF 64 /* standard 64ms SDRAM */
+
static const char * const get_freq_khz[] = {
"core", "run", "cpll", "memory",
"system_bus"
};

+static int get_sdram_rows(void)
+{
+ static int sdram_rows;
+ unsigned int drac2 = 0, drac0 = 0;
+ u32 mdcnfg;
+
+ if (sdram_rows)
+ return sdram_rows;
+
+ mdcnfg = __raw_readl(MDCNFG);
+
+ if (mdcnfg & (MDCNFG_DE2 | MDCNFG_DE3))
+ drac2 = MDCNFG_DRAC2(mdcnfg);
+
+ if (mdcnfg & (MDCNFG_DE0 | MDCNFG_DE1))
+ drac0 = MDCNFG_DRAC0(mdcnfg);
+
+ sdram_rows = 1 << (11 + max(drac0, drac2));
+ return sdram_rows;
+}
+
+static u32 mdrefr_dri(unsigned int freq_khz)
+{
+ u32 interval = freq_khz * SDRAM_TREF / get_sdram_rows();
+
+ return (interval - 31) / 32;
+}
+
/*
* Get the clock frequency as reflected by CCSR and the turbo flag.
* We assume these values have been applied via a fcs.
@@ -145,6 +188,42 @@ static struct desc_clk_cken pxa27x_clocks[] __initdata = {

};

+/*
+ * PXA270 definitions
+ *
+ * For the PXA27x:
+ * Control variables are A, L, 2N for CCCR; B, HT, T for CLKCFG.
+ *
+ * A = 0 => memory controller clock from table 3-7,
+ * A = 1 => memory controller clock = system bus clock
+ * Run mode frequency = 13 MHz * L
+ * Turbo mode frequency = 13 MHz * L * N
+ * System bus frequency = 13 MHz * L / (B + 1)
+ *
+ * In CCCR:
+ * A = 1
+ * L = 16 oscillator to run mode ratio
+ * 2N = 6 2 * (turbo mode to run mode ratio)
+ *
+ * In CCLKCFG:
+ * B = 1 Fast bus mode
+ * HT = 0 Half-Turbo mode
+ * T = 1 Turbo mode
+ *
+ * For now, just support some of the combinations in table 3-7 of
+ * PXA27x Processor Family Developer's Manual to simplify frequency
+ * change sequences.
+ */
+static struct pxa2xx_freq pxa27x_freqs[] = {
+ {104000, 104000, PXA27x_CCCR(1, 8, 2), 0, PXA27x_CLKCFG(1, 0, 1) },
+ {156000, 104000, PXA27x_CCCR(1, 8, 3), 0, PXA27x_CLKCFG(1, 0, 1) },
+ {208000, 208000, PXA27x_CCCR(0, 16, 2), 1, PXA27x_CLKCFG(0, 0, 1) },
+ {312000, 208000, PXA27x_CCCR(1, 16, 3), 1, PXA27x_CLKCFG(1, 0, 1) },
+ {416000, 208000, PXA27x_CCCR(1, 16, 4), 1, PXA27x_CLKCFG(1, 0, 1) },
+ {520000, 208000, PXA27x_CCCR(1, 16, 5), 1, PXA27x_CLKCFG(1, 0, 1) },
+ {624000, 208000, PXA27x_CCCR(1, 16, 6), 1, PXA27x_CLKCFG(1, 0, 1) },
+};
+
static unsigned long clk_pxa27x_cpll_get_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
@@ -164,8 +243,33 @@ static unsigned long clk_pxa27x_cpll_get_rate(struct clk_hw *hw,

return N;
}
+
+
+static int clk_pxa27x_cpll_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ return pxa2xx_determine_rate(req, pxa27x_freqs,
+ ARRAY_SIZE(pxa27x_freqs));
+}
+
+static int clk_pxa27x_cpll_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pxa27x_freqs); i++)
+ if (pxa27x_freqs[i].cpll_khz * KHz == rate)
+ break;
+
+ if (i >= ARRAY_SIZE(pxa27x_freqs))
+ return -EINVAL;
+
+ pxa2xx_cpll_change(&pxa27x_freqs[i], mdrefr_dri);
+ return 0;
+}
+
PARENTS(clk_pxa27x_cpll) = { "osc_13mhz" };
-RATE_RO_OPS(clk_pxa27x_cpll, "cpll");
+RATE_OPS(clk_pxa27x_cpll, "cpll");

static unsigned long clk_pxa27x_lcd_base_get_rate(struct clk_hw *hw,
unsigned long parent_rate)
@@ -217,25 +321,6 @@ static void __init pxa27x_register_plls(void)
clk_register_fixed_factor(NULL, "ppll_312mhz", "osc_13mhz", 0, 24, 1);
}

-static unsigned long clk_pxa27x_core_get_rate(struct clk_hw *hw,
- unsigned long parent_rate)
-{
- unsigned long clkcfg;
- unsigned int ht, osc_forced;
- unsigned long ccsr = readl(CCSR);
-
- osc_forced = ccsr & (1 << CCCR_CPDIS_BIT);
- asm("mrc\tp14, 0, %0, c6, c0, 0" : "=r" (clkcfg));
- ht = clkcfg & (1 << 2);
-
- if (osc_forced)
- return parent_rate;
- if (ht)
- return parent_rate / 2;
- else
- return parent_rate;
-}
-
static u8 clk_pxa27x_core_get_parent(struct clk_hw *hw)
{
unsigned long clkcfg;
@@ -254,8 +339,25 @@ static u8 clk_pxa27x_core_get_parent(struct clk_hw *hw)
return PXA_CORE_TURBO;
return PXA_CORE_RUN;
}
+
+static int clk_pxa27x_core_set_parent(struct clk_hw *hw, u8 index)
+{
+ if (index > PXA_CORE_TURBO)
+ return -EINVAL;
+
+ pxa2xx_core_turbo_switch(index == PXA_CORE_TURBO);
+
+ return 0;
+}
+
+static int clk_pxa27x_core_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ return __clk_mux_determine_rate(hw, req);
+}
+
PARENTS(clk_pxa27x_core) = { "osc_13mhz", "run", "cpll" };
-MUX_RO_RATE_RO_OPS(clk_pxa27x_core, "core");
+MUX_OPS(clk_pxa27x_core, "core", CLK_SET_RATE_PARENT);

static unsigned long clk_pxa27x_run_get_rate(struct clk_hw *hw,
unsigned long parent_rate)
--
2.1.4

2016-10-12 02:36:06

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 6/6] cpufreq: pxa: convert to clock API

On 10-10-16, 22:09, Robert Jarzmik wrote:
> As the clock settings have been introduced into the clock pxa drivers,
> which are now available to change the CPU clock by themselves, remove
> the clock handling from this driver, and rely on pxa clock drivers.
>
> Signed-off-by: Robert Jarzmik <[email protected]>
> ---
> drivers/cpufreq/pxa2xx-cpufreq.c | 191 ++++++++-------------------------------
> 1 file changed, 39 insertions(+), 152 deletions(-)

As you mentioned in the previous patch, why can't you use cpufreq-dt
driver now and delete this one ?

--
viresh

2016-10-12 06:22:45

by Robert Jarzmik

[permalink] [raw]
Subject: Re: [PATCH 6/6] cpufreq: pxa: convert to clock API

Viresh Kumar <[email protected]> writes:

> On 10-10-16, 22:09, Robert Jarzmik wrote:
>> As the clock settings have been introduced into the clock pxa drivers,
>> which are now available to change the CPU clock by themselves, remove
>> the clock handling from this driver, and rely on pxa clock drivers.
>>
>> Signed-off-by: Robert Jarzmik <[email protected]>
>> ---
>> drivers/cpufreq/pxa2xx-cpufreq.c | 191 ++++++++-------------------------------
>> 1 file changed, 39 insertions(+), 152 deletions(-)
>
> As you mentioned in the previous patch, why can't you use cpufreq-dt
> driver now and delete this one ?

PXA architecture have both legacy platform_data based configurations and new
devicetree based ones.

I don't think cpufreq-dt can handle the legacy platform_data ones, can it ?

Cheers.

--
Robert

2016-10-12 06:39:36

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 6/6] cpufreq: pxa: convert to clock API

On 12-10-16, 08:22, Robert Jarzmik wrote:
> Viresh Kumar <[email protected]> writes:
>
> > On 10-10-16, 22:09, Robert Jarzmik wrote:
> >> As the clock settings have been introduced into the clock pxa drivers,
> >> which are now available to change the CPU clock by themselves, remove
> >> the clock handling from this driver, and rely on pxa clock drivers.
> >>
> >> Signed-off-by: Robert Jarzmik <[email protected]>
> >> ---
> >> drivers/cpufreq/pxa2xx-cpufreq.c | 191 ++++++++-------------------------------
> >> 1 file changed, 39 insertions(+), 152 deletions(-)
> >
> > As you mentioned in the previous patch, why can't you use cpufreq-dt
> > driver now and delete this one ?
>
> PXA architecture have both legacy platform_data based configurations and new
> devicetree based ones.

I don't see any platform data specific code in this driver. What am I
missing ?

--
viresh

2016-10-12 08:29:16

by Robert Jarzmik

[permalink] [raw]
Subject: Re: [PATCH 6/6] cpufreq: pxa: convert to clock API

Viresh Kumar <[email protected]> writes:

> On 12-10-16, 08:22, Robert Jarzmik wrote:
>> Viresh Kumar <[email protected]> writes:
>>
>> > On 10-10-16, 22:09, Robert Jarzmik wrote:
>> >> As the clock settings have been introduced into the clock pxa drivers,
>> >> which are now available to change the CPU clock by themselves, remove
>> >> the clock handling from this driver, and rely on pxa clock drivers.
>> >>
>> >> Signed-off-by: Robert Jarzmik <[email protected]>
>> >> ---
>> >> drivers/cpufreq/pxa2xx-cpufreq.c | 191 ++++++++-------------------------------
>> >> 1 file changed, 39 insertions(+), 152 deletions(-)
>> >
>> > As you mentioned in the previous patch, why can't you use cpufreq-dt
>> > driver now and delete this one ?
>>
>> PXA architecture have both legacy platform_data based configurations and new
>> devicetree based ones.
>
> I don't see any platform data specific code in this driver. What am I
> missing ?

In a legacy platform, ie. without devicetree, we have CONFIG_OF=n.
How would cpufreq-dt be usable in this case ?

You can see such a platform in arch/arm/configs/mainstone_defconfig and
arch/arm/mach-pxa/mainstone.c as an example.

Cheers.

--
Robert

2016-10-12 08:46:44

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 6/6] cpufreq: pxa: convert to clock API

On 12-10-16, 10:29, Robert Jarzmik wrote:
> Viresh Kumar <[email protected]> writes:
>
> > On 12-10-16, 08:22, Robert Jarzmik wrote:
> >> Viresh Kumar <[email protected]> writes:
> >>
> >> > On 10-10-16, 22:09, Robert Jarzmik wrote:
> >> >> As the clock settings have been introduced into the clock pxa drivers,
> >> >> which are now available to change the CPU clock by themselves, remove
> >> >> the clock handling from this driver, and rely on pxa clock drivers.
> >> >>
> >> >> Signed-off-by: Robert Jarzmik <[email protected]>
> >> >> ---
> >> >> drivers/cpufreq/pxa2xx-cpufreq.c | 191 ++++++++-------------------------------
> >> >> 1 file changed, 39 insertions(+), 152 deletions(-)
> >> >
> >> > As you mentioned in the previous patch, why can't you use cpufreq-dt
> >> > driver now and delete this one ?
> >>
> >> PXA architecture have both legacy platform_data based configurations and new
> >> devicetree based ones.
> >
> > I don't see any platform data specific code in this driver. What am I
> > missing ?
>
> In a legacy platform, ie. without devicetree, we have CONFIG_OF=n.
> How would cpufreq-dt be usable in this case ?
>
> You can see such a platform in arch/arm/configs/mainstone_defconfig and
> arch/arm/mach-pxa/mainstone.c as an example.

Okay, so its not about platform_data as you said earlier. Rather it is
about the CONFIG_OF option.

In that case, what about making this driver depends_on !CONFIG_OF ? So
that the DT users don't use it anymore.

--
viresh

2016-10-12 19:12:41

by Robert Jarzmik

[permalink] [raw]
Subject: Re: [PATCH 6/6] cpufreq: pxa: convert to clock API

Viresh Kumar <[email protected]> writes:

>> >> PXA architecture have both legacy platform_data based configurations and new
>> >> devicetree based ones.
>> >
>> > I don't see any platform data specific code in this driver. What am I
>> > missing ?
>>
>> In a legacy platform, ie. without devicetree, we have CONFIG_OF=n.
>> How would cpufreq-dt be usable in this case ?
>>
>> You can see such a platform in arch/arm/configs/mainstone_defconfig and
>> arch/arm/mach-pxa/mainstone.c as an example.
>
> Okay, so its not about platform_data as you said earlier. Rather it is
> about the CONFIG_OF option.
We're bickering about terminology.

Mine is that there are globally :
- platform_data _based_ configurations
=> drivers probing devices rely on platform_data structures for initial
configuration
=> that implies CONFIG_OF=n
=> pxa2xx-cpufreq is one of them, even if in this case no platform_data is
used

- devicetree based configurations
=> drivers probing devices rely on device-tree blob for initial configuration
=> that implies CONFIG_OF=y

- ACPI based configurations
=> drivers probing devices rely on ACPI data

- maybe PCI would deserve a place, but I don't think there is much
configuration in it

> In that case, what about making this driver depends_on !CONFIG_OF ? So
> that the DT users don't use it anymore.
Good idea, let me try it.
I'll see if it actually works before make another iteration of this patch.

Cheers.

--
Robert

2016-10-15 19:42:49

by Robert Jarzmik

[permalink] [raw]
Subject: Re: [PATCH 6/6] cpufreq: pxa: convert to clock API

Robert Jarzmik <[email protected]> writes:

>> In that case, what about making this driver depends_on !CONFIG_OF ? So
>> that the DT users don't use it anymore.
> Good idea, let me try it.
> I'll see if it actually works before make another iteration of this patch.

Okay, it works all right.

I'll split out this patch into several others, and make their review
independently of the clock serie.

Cheers.

--
Robert

2016-10-18 07:00:36

by Robert Jarzmik

[permalink] [raw]
Subject: Re: [PATCH 5/6] clk: pxa: transfer CPU clock setting from pxa2xx-cpufreq

Robert Jarzmik <[email protected]> writes:

> This is the initial stage to transfer the pxa25x and pxa27x CPU clocks
> handling from cpufreq to the clock API. More precisely, the clocks
> transferred are :
> - cpll : core pll, known also as the CPU core turbo frequency
> - core : core, known also as the CPU actual frequency, being either the
> CPU core turbo frequency or the CPU core run frequency
>
> This transfer is a prequel to shrink the code in pxa2xx-cpufreq.c, so
> that it can become, at least in devicetree builds, the casual cpufreq-dt
> driver.

Hi Michael and Stephen,

I'm planing on sending a v2 next week with minor corrections, mostly in the data
tables (pxa25x_freqs and pxa27x_freqs), as testing prooved some values were wrong.

If you want me modify this serie, will you have time to review for next week or
should I delay the v2 posting ?

Cheers.

--
Robert

2016-10-18 23:49:11

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 5/6] clk: pxa: transfer CPU clock setting from pxa2xx-cpufreq

On 10/18, Robert Jarzmik wrote:
> Robert Jarzmik <[email protected]> writes:
>
> > This is the initial stage to transfer the pxa25x and pxa27x CPU clocks
> > handling from cpufreq to the clock API. More precisely, the clocks
> > transferred are :
> > - cpll : core pll, known also as the CPU core turbo frequency
> > - core : core, known also as the CPU actual frequency, being either the
> > CPU core turbo frequency or the CPU core run frequency
> >
> > This transfer is a prequel to shrink the code in pxa2xx-cpufreq.c, so
> > that it can become, at least in devicetree builds, the casual cpufreq-dt
> > driver.
>
> Hi Michael and Stephen,
>
> I'm planing on sending a v2 next week with minor corrections, mostly in the data
> tables (pxa25x_freqs and pxa27x_freqs), as testing prooved some values were wrong.
>
> If you want me modify this serie, will you have time to review for next week or
> should I delay the v2 posting ?
>

No need to delay. clk patches look fine with a quick glance. It
would be really neat if we could make cpufreq-dt work without DT.
What's blocking that? OPP tables?

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2016-10-19 20:15:49

by Robert Jarzmik

[permalink] [raw]
Subject: Re: [PATCH 5/6] clk: pxa: transfer CPU clock setting from pxa2xx-cpufreq

Stephen Boyd <[email protected]> writes:

> On 10/18, Robert Jarzmik wrote:
>> Robert Jarzmik <[email protected]> writes:
>> Hi Michael and Stephen,
>>
>> I'm planing on sending a v2 next week with minor corrections, mostly in the data
>> tables (pxa25x_freqs and pxa27x_freqs), as testing prooved some values were wrong.
>>
>> If you want me modify this serie, will you have time to review for next week or
>> should I delay the v2 posting ?
>>
>
> No need to delay. clk patches look fine with a quick glance. It
> would be really neat if we could make cpufreq-dt work without DT.
> What's blocking that? OPP tables?

Heu I'm not the author of cpufreq-dt, so I'm not the best to answer.
To answer the question "without DT", it depends if you mean "with ACPI" or "with
platform_data" or something else.

>From what I've seen so far, the missing/blocking points are :
- the OPP points definition as you said
- probably same thing for the input power supply / regulator
- the cooling parts probably
- and more generaly all the cpufreq-dt is built around device-tree
- last point, the name from KConfig, "Generic DT based cpufreq driver"
=> that strongly suggest it's device-tree only

I'm deeply convinced that Viresh being one of the authors will shed more light
on this.

Cheers.

--
Robert

2016-10-19 20:51:34

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 5/6] clk: pxa: transfer CPU clock setting from pxa2xx-cpufreq

On 10/19, Robert Jarzmik wrote:
> Stephen Boyd <[email protected]> writes:
>
> > On 10/18, Robert Jarzmik wrote:
> >> Robert Jarzmik <[email protected]> writes:
> >> Hi Michael and Stephen,
> >>
> >> I'm planing on sending a v2 next week with minor corrections, mostly in the data
> >> tables (pxa25x_freqs and pxa27x_freqs), as testing prooved some values were wrong.
> >>
> >> If you want me modify this serie, will you have time to review for next week or
> >> should I delay the v2 posting ?
> >>
> >
> > No need to delay. clk patches look fine with a quick glance. It
> > would be really neat if we could make cpufreq-dt work without DT.
> > What's blocking that? OPP tables?
>
> Heu I'm not the author of cpufreq-dt, so I'm not the best to answer.
> To answer the question "without DT", it depends if you mean "with ACPI" or "with
> platform_data" or something else.

I mean platform_data mostly. Do you use ACPI with the clk driver?

>
> From what I've seen so far, the missing/blocking points are :
> - the OPP points definition as you said

Hm.. I thought cpufreq-dt worked with OPP tables populated by
other code (i.e. platform code).

> - probably same thing for the input power supply / regulator

Regulators should be optional I hope. Do you use regulators in
your design that has platform_data?

> - the cooling parts probably
> - and more generaly all the cpufreq-dt is built around device-tree
> - last point, the name from KConfig, "Generic DT based cpufreq driver"
> => that strongly suggest it's device-tree only
>
> I'm deeply convinced that Viresh being one of the authors will shed more light
> on this.

Sure, thanks for the notes.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2016-10-20 03:45:20

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 5/6] clk: pxa: transfer CPU clock setting from pxa2xx-cpufreq

On 19-10-16, 13:51, Stephen Boyd wrote:
> On 10/19, Robert Jarzmik wrote:
> > Stephen Boyd <[email protected]> writes:
> >
> > > On 10/18, Robert Jarzmik wrote:
> > >> Robert Jarzmik <[email protected]> writes:
> > >> Hi Michael and Stephen,
> > >>
> > >> I'm planing on sending a v2 next week with minor corrections, mostly in the data
> > >> tables (pxa25x_freqs and pxa27x_freqs), as testing prooved some values were wrong.
> > >>
> > >> If you want me modify this serie, will you have time to review for next week or
> > >> should I delay the v2 posting ?
> > >>
> > >
> > > No need to delay. clk patches look fine with a quick glance. It
> > > would be really neat if we could make cpufreq-dt work without DT.
> > > What's blocking that? OPP tables?
> >
> > Heu I'm not the author of cpufreq-dt, so I'm not the best to answer.
> > To answer the question "without DT", it depends if you mean "with ACPI" or "with
> > platform_data" or something else.
>
> I mean platform_data mostly. Do you use ACPI with the clk driver?

I would like to see sample code for that, as I am not sure what all changes
would be required in cpufreq-dt in that case. Yes it is more dependent on the
OPP framework than DT right now.

My first impression is that it should be doable..

--
viresh