[PATCH v2 0/4] PXA cpufreq conversion to clock API

[PATCH v2 0/4] PXA cpufreq conversion to clock API

[Robert Jarzmik]

Hi,

This serie is a preparation to shift the cpufreq of pxa2xx platforms to clocks
API.

The first 3 patches are review and merge material :
 - patch 1/4 for Viresh and Rafael
 - patches 2/4 and 3/4 for me

The 4th on is for review but not merge, as the clock changes must be fully
reviewed and go in first as a prequisite

Robert Jarzmik (4):
  cpufreq: pxa: use generic platdev driver for device-tree
  ARM: dts: pxa: add pxa25x cpu operating points
  ARM: dts: pxa: add pxa27x cpu operating points
  cpufreq: pxa: convert to clock API

 arch/arm/boot/dts/pxa25x.dtsi        |  25 +++++
 arch/arm/boot/dts/pxa27x.dtsi        |  40 ++++++++
 drivers/cpufreq/Kconfig.arm          |   2 +-
 drivers/cpufreq/cpufreq-dt-platdev.c |   2 +
 drivers/cpufreq/pxa2xx-cpufreq.c     | 191 +++++++----------------------------
 5 files changed, 107 insertions(+), 153 deletions(-)

-- 
2.1.4

[PATCH v2 1/4] cpufreq: pxa: use generic platdev driver for device-tree

[Robert Jarzmik]

For device-tree based pxa25x and pxa27x platforms, cpufreq-dt driver is
doing the job as well as pxa2xx-cpufreq, so add these platforms to the
compatibility list.

This won't work for legacy non device-tree platforms where
pxa2xx-cpufreq is still required.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/cpufreq/cpufreq-dt-platdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index 0bb44d5b5df4..356825b5c9b8 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -32,6 +32,8 @@ static const struct of_device_id machines[] __initconst = {
 	{ .compatible = "fsl,imx7d", },
 
 	{ .compatible = "marvell,berlin", },
+	{ .compatible = "marvell,pxa250", },
+	{ .compatible = "marvell,pxa270", },
 
 	{ .compatible = "samsung,exynos3250", },
 	{ .compatible = "samsung,exynos4210", },
-- 
2.1.4

[PATCH v2 2/4] ARM: dts: pxa: add pxa25x cpu operating points

[Robert Jarzmik]

Add the relevant data taken from the PXA 25x Electrical, Mechanical, and
Thermal Specfication. This will be input data for cpufreq-dt driver.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 arch/arm/boot/dts/pxa25x.dtsi | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/boot/dts/pxa25x.dtsi b/arch/arm/boot/dts/pxa25x.dtsi
index 0d1e012178c4..16b4e8bad4a5 100644
--- a/arch/arm/boot/dts/pxa25x.dtsi
+++ b/arch/arm/boot/dts/pxa25x.dtsi
@@ -89,4 +89,29 @@
 		clocks = <&clktimer>;
 		status = "okay";
 	};
+
+	pxa250_opp_table: opp_table0 {
+		compatible = "operating-points-v2";
+
+		opp@99500 {
+			opp-hz = /bits/ 64 <99532800>;
+			opp-microvolt = <950000 1000000 1650000>;
+			clock-latency-ns = <20>;
+		};
+		opp@199100 {
+			opp-hz = /bits/ 64 <199065600>;
+			opp-microvolt = <1000000 950000 1650000>;
+			clock-latency-ns = <20>;
+		};
+		opp@298600 {
+			opp-hz = /bits/ 64 <298598400>;
+			opp-microvolt = <1100000 1045000 1650000>;
+			clock-latency-ns = <20>;
+		};
+		opp@398100 {
+			opp-hz = /bits/ 64 <398131200>;
+			opp-microvolt = <1300000 1235000 1650000>;
+			clock-latency-ns = <20>;
+		};
+	};
 };
-- 
2.1.4

[PATCH v2 3/4] ARM: dts: pxa: add pxa27x cpu operating points

[Robert Jarzmik]

Add the relevant data taken from the PXA27x Electrical, Mechanical, and
Thermal Specfication. This will be input data for cpufreq-dt driver.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 arch/arm/boot/dts/pxa27x.dtsi | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/arm/boot/dts/pxa27x.dtsi b/arch/arm/boot/dts/pxa27x.dtsi
index 9e73dc6b3ed3..aef1186856bc 100644
--- a/arch/arm/boot/dts/pxa27x.dtsi
+++ b/arch/arm/boot/dts/pxa27x.dtsi
@@ -137,4 +137,44 @@
 		clocks = <&clks CLK_OSTIMER>;
 		status = "okay";
 	};
+
+	pxa270_opp_table: opp_table0 {
+		compatible = "operating-points-v2";
+
+		opp@104 {
+			opp-hz = /bits/ 64 <104000000>;
+			opp-microvolt = <900000 900000 1705000>;
+			clock-latency-ns = <20>;
+		};
+		opp@156 {
+			opp-hz = /bits/ 64 <156000000>;
+			opp-microvolt = <1000000 1000000 1705000>;
+			clock-latency-ns = <20>;
+		};
+		opp@208 {
+			opp-hz = /bits/ 64 <208000000>;
+			opp-microvolt = <1180000 1180000 1705000>;
+			clock-latency-ns = <20>;
+		};
+		opp@312 {
+			opp-hz = /bits/ 64 <312000000>;
+			opp-microvolt = <1250000 1250000 1705000>;
+			clock-latency-ns = <20>;
+		};
+		opp@416 {
+			opp-hz = /bits/ 64 <416000000>;
+			opp-microvolt = <1350000 1350000 1705000>;
+			clock-latency-ns = <20>;
+		};
+		opp@520 {
+			opp-hz = /bits/ 64 <520000000>;
+			opp-microvolt = <1450000 1450000 1705000>;
+			clock-latency-ns = <20>;
+		};
+		opp@624 {
+			opp-hz = /bits/ 64 <624000000>;
+			opp-microvolt = <1550000 1550000 1705000>;
+			clock-latency-ns = <20>;
+		};
+	};
 };
-- 
2.1.4

[PATCH v2 4/4] cpufreq: pxa: convert to clock API

[Robert Jarzmik]

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 <robert.jarzmik@free.fr>
---
Since v1: added !OF Kconfig dependency
---
 drivers/cpufreq/Kconfig.arm      |   2 +-
 drivers/cpufreq/pxa2xx-cpufreq.c | 191 ++++++++-------------------------------
 2 files changed, 40 insertions(+), 153 deletions(-)

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index d89b8afe23b6..1a4f04351f98 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -236,7 +236,7 @@ config ARM_TEGRA124_CPUFREQ
 
 config ARM_PXA2xx_CPUFREQ
 	tristate "Intel PXA2xx CPUfreq driver"
-	depends on PXA27x || PXA25x
+	depends on (PXA27x || PXA25x) && !OF
 	help
 	  This add the CPUFreq driver support for Intel PXA2xx SOCs.
 
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

Re: [PATCH v2 0/4] PXA cpufreq conversion to clock API

[Russell King - ARM Linux]

On Sat, Oct 15, 2016 at 09:57:26PM +0200, Robert Jarzmik wrote:
> Hi,
> 
> This serie is a preparation to shift the cpufreq of pxa2xx platforms to clocks
> API.
> 
> The first 3 patches are review and merge material :
>  - patch 1/4 for Viresh and Rafael
>  - patches 2/4 and 3/4 for me
> 
> The 4th on is for review but not merge, as the clock changes must be fully
> reviewed and go in first as a prequisite

Have you tested whether this patch set results in functional cpufreq
support - looking at drivers/clk/pxa, the CPU clock is read-only -
it can't be used to change the clock rate.  So, I very much doubt this
has been functionally tested.

Don't forget that changing the CPU clock rate needs other changes
(like the memory clock rate) so all that code you're removing in
patch 4 (which does the actual clock change) needs to go somewhere.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v2 0/4] PXA cpufreq conversion to clock API

[Robert Jarzmik]

Russell King - ARM Linux <linux@armlinux.org.uk> writes:

> On Sat, Oct 15, 2016 at 09:57:26PM +0200, Robert Jarzmik wrote:
>> Hi,
>> 
>> This serie is a preparation to shift the cpufreq of pxa2xx platforms to clocks
>> API.
>> 
>> The first 3 patches are review and merge material :
>>  - patch 1/4 for Viresh and Rafael
>>  - patches 2/4 and 3/4 for me
>> 
>> The 4th on is for review but not merge, as the clock changes must be fully
>> reviewed and go in first as a prequisite
>
> Have you tested whether this patch set results in functional cpufreq
> support - looking at drivers/clk/pxa, the CPU clock is read-only -
> it can't be used to change the clock rate.  So, I very much doubt this
> has been functionally tested.

Oh yes I did, on lubbock and mainstone ... but this requires another patchset
under review, here :
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1247123.html

> Don't forget that changing the CPU clock rate needs other changes
> (like the memory clock rate) so all that code you're removing in
> patch 4 (which does the actual clock change) needs to go somewhere.
Sure, here :
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1247130.html

I would have been wiser to quote these URLs instead of speaking of "clock
changes must be fully review and go in first as a prequisite".

Cheers.

-- 
Robert

WARNING: SOMEONE RECEIVING THIS HAS BEEN HACKED (was: Re: [PATCH v2 0/4] PXA cpufreq conversion to clock API)

[Russell King - ARM Linux]

Guys,

Looks like gmail's been hacked (or something).  Within 50 minutes of
sending the reply below, I've received at least 10 replies with the
following headers:

In-Reply-To: <20161015203421.GG1041@n2100.armlinux.org.uk>
References: <1476561450-28407-1-git-send-email-robert.jarzmik@free.fr> <20161015203421.GG1041@n2100.armlinux.org.uk>

from various gmail.com addresses, each and every one containing some kind
of spam.  Each reply is from a victim of spam - the replies themselves are
people replying to the spam that they have received.  The quoted message
is allegedly from me, and even includes the line:

 "On 15 Oct 2016, at 21:34, Russell King - ARM Linux <linux@armlinux.org.uk>
 wrote:"

which is the time I sent the original message.

It's either that gmail has been hacked, or one of the mailing lists in
the recipients of this message has been hacked.

On Sat, Oct 15, 2016 at 09:34:21PM +0100, Russell King - ARM Linux wrote:
> On Sat, Oct 15, 2016 at 09:57:26PM +0200, Robert Jarzmik wrote:
> > Hi,
> > 
> > This serie is a preparation to shift the cpufreq of pxa2xx platforms to clocks
> > API.
> > 
> > The first 3 patches are review and merge material :
> >  - patch 1/4 for Viresh and Rafael
> >  - patches 2/4 and 3/4 for me
> > 
> > The 4th on is for review but not merge, as the clock changes must be fully
> > reviewed and go in first as a prequisite
> 
> Have you tested whether this patch set results in functional cpufreq
> support - looking at drivers/clk/pxa, the CPU clock is read-only -
> it can't be used to change the clock rate.  So, I very much doubt this
> has been functionally tested.
> 
> Don't forget that changing the CPU clock rate needs other changes
> (like the memory clock rate) so all that code you're removing in
> patch 4 (which does the actual clock change) needs to go somewhere.
> 
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v2 1/4] cpufreq: pxa: use generic platdev driver for device-tree

[Viresh Kumar]

On 15-10-16, 21:57, Robert Jarzmik wrote:
> For device-tree based pxa25x and pxa27x platforms, cpufreq-dt driver is
> doing the job as well as pxa2xx-cpufreq, so add these platforms to the
> compatibility list.
> 
> This won't work for legacy non device-tree platforms where
> pxa2xx-cpufreq is still required.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
>  drivers/cpufreq/cpufreq-dt-platdev.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> index 0bb44d5b5df4..356825b5c9b8 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -32,6 +32,8 @@ static const struct of_device_id machines[] __initconst = {
>  	{ .compatible = "fsl,imx7d", },
>  
>  	{ .compatible = "marvell,berlin", },
> +	{ .compatible = "marvell,pxa250", },
> +	{ .compatible = "marvell,pxa270", },
>  
>  	{ .compatible = "samsung,exynos3250", },
>  	{ .compatible = "samsung,exynos4210", },

Isn't there a race between cpufreq-dt and the platform driver to
register first ?

Also, it seems that atleast the next two patches are required before
applying this? You need to fix the order if that is the case.

-- 
viresh

Re: [PATCH v2 2/4] ARM: dts: pxa: add pxa25x cpu operating points

[Viresh Kumar]

On 15-10-16, 21:57, Robert Jarzmik wrote:
> Add the relevant data taken from the PXA 25x Electrical, Mechanical, and
> Thermal Specfication. This will be input data for cpufreq-dt driver.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
>  arch/arm/boot/dts/pxa25x.dtsi | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/pxa25x.dtsi b/arch/arm/boot/dts/pxa25x.dtsi
> index 0d1e012178c4..16b4e8bad4a5 100644
> --- a/arch/arm/boot/dts/pxa25x.dtsi
> +++ b/arch/arm/boot/dts/pxa25x.dtsi
> @@ -89,4 +89,29 @@
>  		clocks = <&clktimer>;
>  		status = "okay";
>  	};
> +
> +	pxa250_opp_table: opp_table0 {
> +		compatible = "operating-points-v2";
> +
> +		opp@99500 {

We have been keeping the values in ^^^ same as the values present
below. Any specific reason for making it different here ?

> +			opp-hz = /bits/ 64 <99532800>;
> +			opp-microvolt = <950000 1000000 1650000>;
> +			clock-latency-ns = <20>;
> +		};
> +		opp@199100 {
> +			opp-hz = /bits/ 64 <199065600>;
> +			opp-microvolt = <1000000 950000 1650000>;
> +			clock-latency-ns = <20>;
> +		};
> +		opp@298600 {
> +			opp-hz = /bits/ 64 <298598400>;
> +			opp-microvolt = <1100000 1045000 1650000>;
> +			clock-latency-ns = <20>;
> +		};
> +		opp@398100 {
> +			opp-hz = /bits/ 64 <398131200>;
> +			opp-microvolt = <1300000 1235000 1650000>;
> +			clock-latency-ns = <20>;
> +		};
> +	};
>  };
> -- 
> 2.1.4

-- 
viresh

Re: [PATCH v2 4/4] cpufreq: pxa: convert to clock API

[Viresh Kumar]

On 15-10-16, 21:57, 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 <robert.jarzmik@free.fr>
> ---
> Since v1: added !OF Kconfig dependency
> ---
>  drivers/cpufreq/Kconfig.arm      |   2 +-
>  drivers/cpufreq/pxa2xx-cpufreq.c | 191 ++++++++-------------------------------
>  2 files changed, 40 insertions(+), 153 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 2/4] ARM: dts: pxa: add pxa25x cpu operating points

[Robert Jarzmik]

Viresh Kumar <viresh.kumar@linaro.org> writes:

> On 15-10-16, 21:57, Robert Jarzmik wrote:
>> Add the relevant data taken from the PXA 25x Electrical, Mechanical, and
>> Thermal Specfication. This will be input data for cpufreq-dt driver.
>> 
>> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
>> ---
>>  arch/arm/boot/dts/pxa25x.dtsi | 25 +++++++++++++++++++++++++
>>  1 file changed, 25 insertions(+)
>> 
>> diff --git a/arch/arm/boot/dts/pxa25x.dtsi b/arch/arm/boot/dts/pxa25x.dtsi
>> index 0d1e012178c4..16b4e8bad4a5 100644
>> --- a/arch/arm/boot/dts/pxa25x.dtsi
>> +++ b/arch/arm/boot/dts/pxa25x.dtsi
>> @@ -89,4 +89,29 @@
>>  		clocks = <&clktimer>;
>>  		status = "okay";
>>  	};
>> +
>> +	pxa250_opp_table: opp_table0 {
>> +		compatible = "operating-points-v2";
>> +
>> +		opp@99500 {
>
> We have been keeping the values in ^^^ same as the values present
> below. Any specific reason for making it different here ?
No, that's a good comment, I'll change that.

I wrote this incrementaly, first the node, then the opp-hz. Then I realized that
the source crystal, at 3.8684 MHz didn't provide a round 99.5 MHz core clock,
but a 99.5328 MHz clock.

Anyway, I'll change that ... let's say into opp@99533 in this case ?

-- 
Robert

Re: [PATCH v2 1/4] cpufreq: pxa: use generic platdev driver for device-tree

[Robert Jarzmik]

Viresh Kumar <viresh.kumar@linaro.org> writes:

> On 15-10-16, 21:57, Robert Jarzmik wrote:
>> For device-tree based pxa25x and pxa27x platforms, cpufreq-dt driver is
>> doing the job as well as pxa2xx-cpufreq, so add these platforms to the
>> compatibility list.
>> 
>> This won't work for legacy non device-tree platforms where
>> pxa2xx-cpufreq is still required.
>> 
>> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
>> ---
>>  drivers/cpufreq/cpufreq-dt-platdev.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
>> index 0bb44d5b5df4..356825b5c9b8 100644
>> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
>> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
>> @@ -32,6 +32,8 @@ static const struct of_device_id machines[] __initconst = {
>>  	{ .compatible = "fsl,imx7d", },
>>  
>>  	{ .compatible = "marvell,berlin", },
>> +	{ .compatible = "marvell,pxa250", },
>> +	{ .compatible = "marvell,pxa270", },
>>  
>>  	{ .compatible = "samsung,exynos3250", },
>>  	{ .compatible = "samsung,exynos4210", },
>
> Isn't there a race between cpufreq-dt and the platform driver to
> register first ?
Ah, could you be more specific about the race you're talking of ?

My understanding was that cpufreq-dt-platdev does create the device, and
cpufreq-dt is a driver for it, so there is no race but a direct relationship
AFAIU.

> Also, it seems that atleast the next two patches are required before
> applying this? You need to fix the order if that is the case.
Ok, as you wish, let it become number 3 and (2, 3) become (1, 2).

Cheers.

-- 
Robert

Re: [PATCH v2 2/4] ARM: dts: pxa: add pxa25x cpu operating points

[Viresh Kumar]

On 18-10-16, 17:30, Robert Jarzmik wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:
> 
> > On 15-10-16, 21:57, Robert Jarzmik wrote:
> >> Add the relevant data taken from the PXA 25x Electrical, Mechanical, and
> >> Thermal Specfication. This will be input data for cpufreq-dt driver.
> >> 
> >> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> >> ---
> >>  arch/arm/boot/dts/pxa25x.dtsi | 25 +++++++++++++++++++++++++
> >>  1 file changed, 25 insertions(+)
> >> 
> >> diff --git a/arch/arm/boot/dts/pxa25x.dtsi b/arch/arm/boot/dts/pxa25x.dtsi
> >> index 0d1e012178c4..16b4e8bad4a5 100644
> >> --- a/arch/arm/boot/dts/pxa25x.dtsi
> >> +++ b/arch/arm/boot/dts/pxa25x.dtsi
> >> @@ -89,4 +89,29 @@
> >>  		clocks = <&clktimer>;
> >>  		status = "okay";
> >>  	};
> >> +
> >> +	pxa250_opp_table: opp_table0 {
> >> +		compatible = "operating-points-v2";
> >> +
> >> +		opp@99500 {
> >
> > We have been keeping the values in ^^^ same as the values present
> > below. Any specific reason for making it different here ?
> No, that's a good comment, I'll change that.
> 
> I wrote this incrementaly, first the node, then the opp-hz. Then I realized that
> the source crystal, at 3.8684 MHz didn't provide a round 99.5 MHz core clock,
> but a 99.5328 MHz clock.
> 
> Anyway, I'll change that ... let's say into opp@99533 in this case ?

Just write the whole value 99532800.

-- 
viresh

Re: [PATCH v2 1/4] cpufreq: pxa: use generic platdev driver for device-tree

[Viresh Kumar]

On 18-10-16, 17:35, Robert Jarzmik wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:
> 
> > On 15-10-16, 21:57, Robert Jarzmik wrote:
> >> For device-tree based pxa25x and pxa27x platforms, cpufreq-dt driver is
> >> doing the job as well as pxa2xx-cpufreq, so add these platforms to the
> >> compatibility list.
> >> 
> >> This won't work for legacy non device-tree platforms where
> >> pxa2xx-cpufreq is still required.
> >> 
> >> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> >> ---
> >>  drivers/cpufreq/cpufreq-dt-platdev.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >> 
> >> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> >> index 0bb44d5b5df4..356825b5c9b8 100644
> >> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> >> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> >> @@ -32,6 +32,8 @@ static const struct of_device_id machines[] __initconst = {
> >>  	{ .compatible = "fsl,imx7d", },
> >>  
> >>  	{ .compatible = "marvell,berlin", },
> >> +	{ .compatible = "marvell,pxa250", },
> >> +	{ .compatible = "marvell,pxa270", },
> >>  
> >>  	{ .compatible = "samsung,exynos3250", },
> >>  	{ .compatible = "samsung,exynos4210", },
> >
> > Isn't there a race between cpufreq-dt and the platform driver to
> > register first ?
> Ah, could you be more specific about the race you're talking of ?
> 
> My understanding was that cpufreq-dt-platdev does create the device, and
> cpufreq-dt is a driver for it, so there is no race but a direct relationship
> AFAIU.

I mean that both the driver may try to register to the cpufreq core if
they are both compiled in a single image.

-- 
viresh

Re: [PATCH v2 1/4] cpufreq: pxa: use generic platdev driver for device-tree

[Robert Jarzmik]

Viresh Kumar <viresh.kumar@linaro.org> writes:

>> >> +	{ .compatible = "marvell,pxa250", },
>> >> +	{ .compatible = "marvell,pxa270", },
>> >>  
>> >>  	{ .compatible = "samsung,exynos3250", },
>> >>  	{ .compatible = "samsung,exynos4210", },
>> >
>> > Isn't there a race between cpufreq-dt and the platform driver to
>> > register first ?
>> Ah, could you be more specific about the race you're talking of ?
>> 
>> My understanding was that cpufreq-dt-platdev does create the device, and
>> cpufreq-dt is a driver for it, so there is no race but a direct relationship
>> AFAIU.
>
> I mean that both the driver may try to register to the cpufreq core if
> they are both compiled in a single image.
Euh I still don't follow you. The only driver that can register to the cpufreq
core is cpufreq-dt.

Now the only case I see is that there are 2 cpufreq-dt platform_device created
from cpufreq-dt-platdev. Given that there is only 1 call to
platform_device_register_data() in it, I don't see how it is possible.

Now if you are worried that 2 cpufreq-dt devices are created, ie. 1 for pxa25x
and one for pxa27x:
 - this looks impossible given the cpufreq_dt_platdev_init() code
 - no device-tree will ever be compatible with both of them, even if a single
   kernel binary will be compatible with both of them

Tell me if this is the information you're looking for.

Cheers.

-- 
Robert

Re: [PATCH v2 1/4] cpufreq: pxa: use generic platdev driver for device-tree

[Viresh Kumar]

On 19-10-16, 22:06, Robert Jarzmik wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:
> 
> >> >> +	{ .compatible = "marvell,pxa250", },
> >> >> +	{ .compatible = "marvell,pxa270", },
> >> >>  
> >> >>  	{ .compatible = "samsung,exynos3250", },
> >> >>  	{ .compatible = "samsung,exynos4210", },
> >> >
> >> > Isn't there a race between cpufreq-dt and the platform driver to
> >> > register first ?
> >> Ah, could you be more specific about the race you're talking of ?
> >> 
> >> My understanding was that cpufreq-dt-platdev does create the device, and
> >> cpufreq-dt is a driver for it, so there is no race but a direct relationship
> >> AFAIU.
> >
> > I mean that both the driver may try to register to the cpufreq core if
> > they are both compiled in a single image.
> Euh I still don't follow you. The only driver that can register to the cpufreq
> core is cpufreq-dt.

I was wondering on what will happen if both cpufreq-dt and your pxa2xx-cpufreq
driver are present in the same kernel image. In that case the init routines of
both of them will try to call cpufreq_register_driver().

-- 
viresh

Re: [PATCH v2 1/4] cpufreq: pxa: use generic platdev driver for device-tree

[Robert Jarzmik]

Viresh Kumar <viresh.kumar@linaro.org> writes:

> On 19-10-16, 22:06, Robert Jarzmik wrote:
>> Viresh Kumar <viresh.kumar@linaro.org> writes:
>> 
>> >> >> +	{ .compatible = "marvell,pxa250", },
>> >> >> +	{ .compatible = "marvell,pxa270", },
>> >> >>  
>> >> >>  	{ .compatible = "samsung,exynos3250", },
>> >> >>  	{ .compatible = "samsung,exynos4210", },
>> >> >
>> >> > Isn't there a race between cpufreq-dt and the platform driver to
>> >> > register first ?
>> >> Ah, could you be more specific about the race you're talking of ?
>> >> 
>> >> My understanding was that cpufreq-dt-platdev does create the device, and
>> >> cpufreq-dt is a driver for it, so there is no race but a direct relationship
>> >> AFAIU.
>> >
>> > I mean that both the driver may try to register to the cpufreq core if
>> > they are both compiled in a single image.
>> Euh I still don't follow you. The only driver that can register to the cpufreq
>> core is cpufreq-dt.
>
> I was wondering on what will happen if both cpufreq-dt and your pxa2xx-cpufreq
> driver are present in the same kernel image. In that case the init routines of
> both of them will try to call cpufreq_register_driver().
Right.

In my case, cpufreq-dt comes first, and wins.
pxa_cpu_init() calls cpufreq_register_driver() and returns -EEXIST.

Cheers.

-- 
Robert

Re: [PATCH v2 4/4] cpufreq: pxa: convert to clock API

[Robert Jarzmik]

Viresh Kumar <viresh.kumar@linaro.org> writes:

> On 15-10-16, 21:57, 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 <robert.jarzmik@free.fr>
>> ---
>> Since v1: added !OF Kconfig dependency
>> ---
>>  drivers/cpufreq/Kconfig.arm      |   2 +-
>>  drivers/cpufreq/pxa2xx-cpufreq.c | 191 ++++++++-------------------------------
>>  2 files changed, 40 insertions(+), 153 deletions(-)
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Okay, after some more testing, I'd like to remove the !OF for next iteration.
The reason is that I have a usecase where I have a single kernel image with
devictree support (ie CONFIG_OF=y), but with support for both devicetree and
legacy platforms. In this case, a platform such as lubbock can boot :
 - with devicetree
 - with legacy arch/arm/mach-pxa/lubbock.c

In this kernel, the !OF Kconfig prevents the legacy version from working, as
pxa2xx-cpufreq is descarded even if it is needed in the legacy version.

Therefore, I'd like to respin without this !OF.

Cheers.

-- 
Robert

Re: [PATCH v2 4/4] cpufreq: pxa: convert to clock API

[Viresh Kumar]

On 22-10-16, 23:37, Robert Jarzmik wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:
> 
> > On 15-10-16, 21:57, 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 <robert.jarzmik@free.fr>
> >> ---
> >> Since v1: added !OF Kconfig dependency
> >> ---
> >>  drivers/cpufreq/Kconfig.arm      |   2 +-
> >>  drivers/cpufreq/pxa2xx-cpufreq.c | 191 ++++++++-------------------------------
> >>  2 files changed, 40 insertions(+), 153 deletions(-)
> >
> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> 
> Okay, after some more testing, I'd like to remove the !OF for next iteration.
> The reason is that I have a usecase where I have a single kernel image with
> devictree support (ie CONFIG_OF=y), but with support for both devicetree and
> legacy platforms. In this case, a platform such as lubbock can boot :
>  - with devicetree
>  - with legacy arch/arm/mach-pxa/lubbock.c
> 
> In this kernel, the !OF Kconfig prevents the legacy version from working, as
> pxa2xx-cpufreq is descarded even if it is needed in the legacy version.
> 
> Therefore, I'd like to respin without this !OF.

I imagined this case last week as well :)

No issues.

-- 
viresh