[PATCH v3 0/3] CPUFreq for kzm9g

[PATCH v3 0/3] CPUFreq for kzm9g

[Guennadi Liakhovetski]

This is v3 of the patch-series, adding CPUFreq support to sh73a0 SoCs and 
to the kzm9g board specifically. It now consists of 3 patches instead of 2, 
as in v2. The first patch is new. It adds waiting to the clock-kicking 
operation for all clocks, controlled by the FRQCRA and FRQCRB operations. 
In principle, this means, that this change affects existing systems, but 
in fact, I don't think any of them currently actively configure rates of 
those clocks, at least I haven't found any occurrences in the mainline. So, 
the risk should be purely theoretical. Other changes are noted in 
respective patches.

Also note, that there seems to be a problem with the TWD clock, when 
adjusting the CPU rate. I'm posting these patches for now for reference, 
but they shouldn't be applied, until the problem is solved.

Thanks
Guennadi

Guennadi Liakhovetski (3):
  ARM: shmobile: sh73a0: wait for completion when kicking the clock
  ARM: shmobile: sh73a0: add support for adjusting CPU frequency
  ARM: shmobile: kzm9g-reference: add CPUFreq support

 arch/arm/boot/dts/sh73a0-kzm9g-reference.dts |   19 ++++-
 arch/arm/mach-shmobile/Kconfig               |    2 +
 arch/arm/mach-shmobile/clock-sh73a0.c        |  118 ++++++++++++++++++++++++--
 3 files changed, 128 insertions(+), 11 deletions(-)

-- 
1.7.2.5

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH v3 1/3] ARM: shmobile: sh73a0: wait for completion when kicking the clock

[Guennadi Liakhovetski]

To reconfigure clocks, controlled by FRQCRA and FRQCRB, a kick bit has to
be set and to make sure the setting has taken effect, it has to be read
back repeatedly until it is cleared by the hardware. This patch adds the
waiting part, that was missing until now.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

As noted in patch 0/3, this patch affects existing systems, but AFAICS 
only theoretically - so far nobody is changing clock rates of any of the 
FRQCRA and FRQCRB clocks. Still, please, handle with care.

 arch/arm/mach-shmobile/clock-sh73a0.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-shmobile/clock-sh73a0.c b/arch/arm/mach-shmobile/clock-sh73a0.c
index 71843dd..34b5c5a 100644
--- a/arch/arm/mach-shmobile/clock-sh73a0.c
+++ b/arch/arm/mach-shmobile/clock-sh73a0.c
@@ -21,6 +21,7 @@
 #include <linux/io.h>
 #include <linux/sh_clk.h>
 #include <linux/clkdev.h>
+#include <asm/processor.h>
 #include <mach/common.h>
 
 #define FRQCRA		IOMEM(0xe6150000)
@@ -234,14 +235,24 @@ static struct clk *main_clks[] = {
 	&sh73a0_extalr_clk,
 };
 
-static void div4_kick(struct clk *clk)
+static int frqcr_kick(void)
 {
-	unsigned long value;
+	int i;
+
+	/* set KICK bit in FRQCRB to update hardware setting, check success */
+	__raw_writel(__raw_readl(FRQCRB) | (1 << 31), FRQCRB);
+	for (i = 1000; i; i--)
+		if (__raw_readl(FRQCRB) & (1 << 31))
+			cpu_relax();
+		else
+			return i;
+
+	return -ETIMEDOUT;
+}
 
-	/* set KICK bit in FRQCRB to update hardware setting */
-	value = __raw_readl(FRQCRB);
-	value |= (1 << 31);
-	__raw_writel(value, FRQCRB);
+static void div4_kick(struct clk *clk)
+{
+	frqcr_kick();
 }
 
 static int divisors[] = { 2, 3, 4, 6, 8, 12, 16, 18,
-- 
1.7.2.5

Re: [PATCH v3 1/3] ARM: shmobile: sh73a0: wait for completion when kicking the clock

[Simon Horman]

On Thu, Feb 28, 2013 at 01:21:58PM +0100, Guennadi Liakhovetski wrote:
> To reconfigure clocks, controlled by FRQCRA and FRQCRB, a kick bit has to
> be set and to make sure the setting has taken effect, it has to be read
> back repeatedly until it is cleared by the hardware. This patch adds the
> waiting part, that was missing until now.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

This patch seems to be ready to be applied.

Magnus, could I get a review from you?

> ---
> 
> As noted in patch 0/3, this patch affects existing systems, but AFAICS 
> only theoretically - so far nobody is changing clock rates of any of the 
> FRQCRA and FRQCRB clocks. Still, please, handle with care.
> 
>  arch/arm/mach-shmobile/clock-sh73a0.c |   23 +++++++++++++++++------
>  1 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/clock-sh73a0.c b/arch/arm/mach-shmobile/clock-sh73a0.c
> index 71843dd..34b5c5a 100644
> --- a/arch/arm/mach-shmobile/clock-sh73a0.c
> +++ b/arch/arm/mach-shmobile/clock-sh73a0.c
> @@ -21,6 +21,7 @@
>  #include <linux/io.h>
>  #include <linux/sh_clk.h>
>  #include <linux/clkdev.h>
> +#include <asm/processor.h>
>  #include <mach/common.h>
>  
>  #define FRQCRA		IOMEM(0xe6150000)
> @@ -234,14 +235,24 @@ static struct clk *main_clks[] = {
>  	&sh73a0_extalr_clk,
>  };
>  
> -static void div4_kick(struct clk *clk)
> +static int frqcr_kick(void)
>  {
> -	unsigned long value;
> +	int i;
> +
> +	/* set KICK bit in FRQCRB to update hardware setting, check success */
> +	__raw_writel(__raw_readl(FRQCRB) | (1 << 31), FRQCRB);
> +	for (i = 1000; i; i--)
> +		if (__raw_readl(FRQCRB) & (1 << 31))
> +			cpu_relax();
> +		else
> +			return i;
> +
> +	return -ETIMEDOUT;
> +}
>  
> -	/* set KICK bit in FRQCRB to update hardware setting */
> -	value = __raw_readl(FRQCRB);
> -	value |= (1 << 31);
> -	__raw_writel(value, FRQCRB);
> +static void div4_kick(struct clk *clk)
> +{
> +	frqcr_kick();
>  }
>  
>  static int divisors[] = { 2, 3, 4, 6, 8, 12, 16, 18,
> -- 
> 1.7.2.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v3 1/3] ARM: shmobile: sh73a0: wait for completion when kicking the clock

[Simon Horman]

On Fri, Mar 01, 2013 at 11:52:25AM +0900, Simon Horman wrote:
> On Thu, Feb 28, 2013 at 01:21:58PM +0100, Guennadi Liakhovetski wrote:
> > To reconfigure clocks, controlled by FRQCRA and FRQCRB, a kick bit has to
> > be set and to make sure the setting has taken effect, it has to be read
> > back repeatedly until it is cleared by the hardware. This patch adds the
> > waiting part, that was missing until now.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> 
> This patch seems to be ready to be applied.
> 
> Magnus, could I get a review from you?

Magnus, ping.

> > ---
> > 
> > As noted in patch 0/3, this patch affects existing systems, but AFAICS 
> > only theoretically - so far nobody is changing clock rates of any of the 
> > FRQCRA and FRQCRB clocks. Still, please, handle with care.
> > 
> >  arch/arm/mach-shmobile/clock-sh73a0.c |   23 +++++++++++++++++------
> >  1 files changed, 17 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/arm/mach-shmobile/clock-sh73a0.c b/arch/arm/mach-shmobile/clock-sh73a0.c
> > index 71843dd..34b5c5a 100644
> > --- a/arch/arm/mach-shmobile/clock-sh73a0.c
> > +++ b/arch/arm/mach-shmobile/clock-sh73a0.c
> > @@ -21,6 +21,7 @@
> >  #include <linux/io.h>
> >  #include <linux/sh_clk.h>
> >  #include <linux/clkdev.h>
> > +#include <asm/processor.h>
> >  #include <mach/common.h>
> >  
> >  #define FRQCRA		IOMEM(0xe6150000)
> > @@ -234,14 +235,24 @@ static struct clk *main_clks[] = {
> >  	&sh73a0_extalr_clk,
> >  };
> >  
> > -static void div4_kick(struct clk *clk)
> > +static int frqcr_kick(void)
> >  {
> > -	unsigned long value;
> > +	int i;
> > +
> > +	/* set KICK bit in FRQCRB to update hardware setting, check success */
> > +	__raw_writel(__raw_readl(FRQCRB) | (1 << 31), FRQCRB);
> > +	for (i = 1000; i; i--)
> > +		if (__raw_readl(FRQCRB) & (1 << 31))
> > +			cpu_relax();
> > +		else
> > +			return i;
> > +
> > +	return -ETIMEDOUT;
> > +}
> >  
> > -	/* set KICK bit in FRQCRB to update hardware setting */
> > -	value = __raw_readl(FRQCRB);
> > -	value |= (1 << 31);
> > -	__raw_writel(value, FRQCRB);
> > +static void div4_kick(struct clk *clk)
> > +{
> > +	frqcr_kick();
> >  }
> >  
> >  static int divisors[] = { 2, 3, 4, 6, 8, 12, 16, 18,
> > -- 
> > 1.7.2.5
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v3 1/3] ARM: shmobile: sh73a0: wait for completion when kicking the clock

[Magnus Damm]

On Tue, Mar 19, 2013 at 12:42 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Mar 01, 2013 at 11:52:25AM +0900, Simon Horman wrote:
>> On Thu, Feb 28, 2013 at 01:21:58PM +0100, Guennadi Liakhovetski wrote:
>> > To reconfigure clocks, controlled by FRQCRA and FRQCRB, a kick bit has to
>> > be set and to make sure the setting has taken effect, it has to be read
>> > back repeatedly until it is cleared by the hardware. This patch adds the
>> > waiting part, that was missing until now.
>> >
>> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>>
>> This patch seems to be ready to be applied.
>>
>> Magnus, could I get a review from you?
>
> Magnus, ping.

Thanks for the ping. This patch looks good to me.

Acked-by: Magnus Damm <damm@opensource.se>

Now if we could do the same thing for the MSTP bits too then that
would be excellent...

/ magnus

Re: [PATCH v3 1/3] ARM: shmobile: sh73a0: wait for completion when kicking the clock

[Simon Horman]

On Tue, Mar 19, 2013 at 02:15:29PM +0900, Magnus Damm wrote:
> On Tue, Mar 19, 2013 at 12:42 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Fri, Mar 01, 2013 at 11:52:25AM +0900, Simon Horman wrote:
> >> On Thu, Feb 28, 2013 at 01:21:58PM +0100, Guennadi Liakhovetski wrote:
> >> > To reconfigure clocks, controlled by FRQCRA and FRQCRB, a kick bit has to
> >> > be set and to make sure the setting has taken effect, it has to be read
> >> > back repeatedly until it is cleared by the hardware. This patch adds the
> >> > waiting part, that was missing until now.
> >> >
> >> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> >>
> >> This patch seems to be ready to be applied.
> >>
> >> Magnus, could I get a review from you?
> >
> > Magnus, ping.
> 
> Thanks for the ping. This patch looks good to me.
> 
> Acked-by: Magnus Damm <damm@opensource.se>

Thanks, applied to the soc branch.

> Now if we could do the same thing for the MSTP bits too then that
> would be excellent...
> 
> / magnus
> 

[PATCH/RFC v3 2/3] ARM: shmobile: sh73a0: add support for adjusting CPU frequency

[Guennadi Liakhovetski]

On SH73A0 the output of PLL0 is supplied to two dividers, feeding clock to
the CPU core and SGX. Lower CPU frequencies allow the use of lower supply
voltages and thus reduce power consumption.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v3:
1. Changed the order of reducing the clock frequency. It turned out, that 
sh73a0 behaves differently when switching for the first time from a 
disabled Z-clock divisor (pass-through from PLL0) to a higher or to a 
lower output frequency. Switching to 1/2 of the input frequency worked 
always, whereas switching to 1/3 didn't work for the first time, even 
though exactly the same code is used. The reason is probably, that 1/2 
division is achieved by writing 0 to the divisor field, which is also the 
default value, and 1/3 is set by writing 1. The problem can be fixed by 
first enabling the divisor, and then setting the division factor.
2. To make the above safer we now always reset the divisor field to 0, 
when disabling the divisor and going to the full PLL0 rate for the 
Z-clock.

 arch/arm/mach-shmobile/clock-sh73a0.c |   95 ++++++++++++++++++++++++++++++++-
 1 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-shmobile/clock-sh73a0.c b/arch/arm/mach-shmobile/clock-sh73a0.c
index 34b5c5a..1108d77 100644
--- a/arch/arm/mach-shmobile/clock-sh73a0.c
+++ b/arch/arm/mach-shmobile/clock-sh73a0.c
@@ -276,6 +276,11 @@ enum { DIV4_I, DIV4_ZG, DIV4_M3, DIV4_B, DIV4_M1, DIV4_M2,
 
 static struct clk div4_clks[DIV4_NR] = {
 	[DIV4_I] = DIV4(FRQCRA, 20, 0xdff, CLK_ENABLE_ON_INIT),
+	/*
+	 * ZG clock is dividing PLL0 frequency to supply SGX. Make sure not to
+	 * exceed maximum frequencies of 201.5MHz for VDD_DVFS=1.175 and
+	 * 239.2MHz for VDD_DVFS=1.315V.
+	 */
 	[DIV4_ZG] = SH_CLK_DIV4(&pll0_clk, FRQCRA, 16, 0xd7f, CLK_ENABLE_ON_INIT),
 	[DIV4_M3] = DIV4(FRQCRA, 12, 0x1dff, CLK_ENABLE_ON_INIT),
 	[DIV4_B] = DIV4(FRQCRA, 8, 0xdff, CLK_ENABLE_ON_INIT),
@@ -288,6 +293,85 @@ static struct clk div4_clks[DIV4_NR] = {
 	[DIV4_HP] = DIV4(FRQCRB, 4, 0xdff, 0),
 };
 
+static int (*div4_set_rate)(struct clk *clk, unsigned long rate);
+static unsigned long (*div4_recalc)(struct clk *clk);
+static long (*div4_round_rate)(struct clk *clk, unsigned long rate);
+
+static int zclk_set_rate(struct clk *clk, unsigned long rate)
+{
+	int ret;
+
+	if (!clk->parent || !__clk_get(clk->parent))
+		return -ENODEV;
+
+	if (readl(FRQCRB) & (1 << 31))
+		return -EBUSY;
+
+	if (rate == clk_get_rate(clk->parent)) {
+		/* 1:1 - switch off divider */
+		__raw_writel(__raw_readl(FRQCRB) & ~(1 << 28), FRQCRB);
+		/* nullify the divider to prepare for the next time */
+		ret = div4_set_rate(clk, rate / 2);
+		if (!ret)
+			ret = frqcr_kick();
+		if (ret > 0)
+			ret = 0;
+	} else {
+		/* Enable the divider */
+		__raw_writel(__raw_readl(FRQCRB) | (1 << 28), FRQCRB);
+
+		ret = frqcr_kick();
+		if (ret >= 0)
+			/*
+			 * set the divider - call the DIV4 method, it will kick
+			 * FRQCRB too
+			 */
+			ret = div4_set_rate(clk, rate);
+		if (ret < 0)
+			goto esetrate;
+	}
+
+esetrate:
+	__clk_put(clk->parent);
+	return ret;
+}
+
+static long zclk_round_rate(struct clk *clk, unsigned long rate)
+{
+	unsigned long div_freq = div4_round_rate(clk, rate),
+		parent_freq = clk_get_rate(clk->parent);
+
+	if (rate > div_freq && abs(parent_freq - rate) < rate - div_freq)
+		return parent_freq;
+
+	return div_freq;
+}
+
+static unsigned long zclk_recalc(struct clk *clk)
+{
+	/*
+	 * Must recalculate frequencies in case PLL0 has been changed, even if
+	 * the divisor is unused ATM!
+	 */
+	unsigned long div_freq = div4_recalc(clk);
+
+	if (__raw_readl(FRQCRB) & (1 << 28))
+		return div_freq;
+
+	return clk_get_rate(clk->parent);
+}
+
+static void zclk_extend(void)
+{
+	/* We extend the DIV4 clock with a 1:1 pass-through case */
+	div4_set_rate = div4_clks[DIV4_Z].ops->set_rate;
+	div4_round_rate = div4_clks[DIV4_Z].ops->round_rate;
+	div4_recalc = div4_clks[DIV4_Z].ops->recalc;
+	div4_clks[DIV4_Z].ops->set_rate = zclk_set_rate;
+	div4_clks[DIV4_Z].ops->round_rate = zclk_round_rate;
+	div4_clks[DIV4_Z].ops->recalc = zclk_recalc;
+}
+
 enum { DIV6_VCK1, DIV6_VCK2, DIV6_VCK3, DIV6_ZB1,
 	DIV6_FLCTL, DIV6_SDHI0, DIV6_SDHI1, DIV6_SDHI2,
 	DIV6_FSIA, DIV6_FSIB, DIV6_SUB,
@@ -485,7 +569,7 @@ static struct clk *late_main_clks[] = {
 };
 
 enum { MSTP001,
-	MSTP129, MSTP128, MSTP127, MSTP126, MSTP125, MSTP118, MSTP116, MSTP100,
+	MSTP129, MSTP128, MSTP127, MSTP126, MSTP125, MSTP118, MSTP116, MSTP112, MSTP100,
 	MSTP219, MSTP218, MSTP217,
 	MSTP207, MSTP206, MSTP204, MSTP203, MSTP202, MSTP201, MSTP200,
 	MSTP331, MSTP329, MSTP328, MSTP325, MSTP323, MSTP322,
@@ -506,6 +590,7 @@ static struct clk mstp_clks[MSTP_NR] = {
 	[MSTP125] = MSTP(&div6_clks[DIV6_SUB], SMSTPCR1, 25, 0), /* TMU0 */
 	[MSTP118] = MSTP(&div4_clks[DIV4_B], SMSTPCR1, 18, 0), /* DSITX0 */
 	[MSTP116] = MSTP(&div4_clks[DIV4_HP], SMSTPCR1, 16, 0), /* IIC0 */
+	[MSTP112] = MSTP(&div4_clks[DIV4_ZG], SMSTPCR1, 12, 0), /* SGX */
 	[MSTP100] = MSTP(&div4_clks[DIV4_B], SMSTPCR1, 0, 0), /* LCDC0 */
 	[MSTP219] = MSTP(&div6_clks[DIV6_SUB], SMSTPCR2, 19, 0), /* SCIFA7 */
 	[MSTP218] = MSTP(&div4_clks[DIV4_HP], SMSTPCR2, 18, 0), /* SY-DMAC */
@@ -547,6 +632,9 @@ static struct clk_lookup lookups[] = {
 	/* main clocks */
 	CLKDEV_CON_ID("r_clk", &r_clk),
 
+	/* DIV4 clocks */
+	CLKDEV_DEV_ID("cpu0", &div4_clks[DIV4_Z]), /* cpufreq-cpu0 */
+
 	/* DIV6 clocks */
 	CLKDEV_CON_ID("vck1_clk", &div6_clks[DIV6_VCK1]),
 	CLKDEV_CON_ID("vck2_clk", &div6_clks[DIV6_VCK2]),
@@ -638,8 +726,11 @@ void __init sh73a0_clock_init(void)
 	for (k = 0; !ret && (k < ARRAY_SIZE(main_clks)); k++)
 		ret = clk_register(main_clks[k]);
 
-	if (!ret)
+	if (!ret) {
 		ret = sh_clk_div4_register(div4_clks, DIV4_NR, &div4_table);
+		if (!ret)
+			zclk_extend();
+	}
 
 	if (!ret)
 		ret = sh_clk_div6_reparent_register(div6_clks, DIV6_NR);
-- 
1.7.2.5

[PATCH/RFC v3 3/3] ARM: shmobile: kzm9g-reference: add CPUFreq support

[Guennadi Liakhovetski]

This patch enables the use of the generic cpufreq-cpu0 driver on kzm9g.
Providing a regulator and a list of OPPs in DT, combined with a clock,
attached to the cpu0 device is everything, the cpufreq-cpu0 driver needs.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v3: no change

 arch/arm/boot/dts/sh73a0-kzm9g-reference.dts |   19 ++++++++++++++++---
 arch/arm/mach-shmobile/Kconfig               |    2 ++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
index 363f1ab..c964e20 100644
--- a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
+++ b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
@@ -18,6 +18,19 @@
 	model = "KZM-A9-GT";
 	compatible = "renesas,kzm9g-reference", "renesas,sh73a0";
 
+	cpus {
+		cpu@0 {
+			cpu0-supply = <&vdd_dvfs>;
+			operating-points = <
+				/* kHz  uV */
+				1196000 1315000
+				 598000 1175000
+				 398667 1065000
+			>;
+			voltage-tolerance = <1>; /* 1% */
+		};
+	};
+
 	chosen {
 		bootargs = "console=tty0 console=ttySC4,115200 root=/dev/nfs ip=dhcp ignore_loglevel earlyprintk=sh-sci.4,115200";
 	};
@@ -113,10 +126,10 @@
 		reg = <0x40>;
 
 		regulators {
-			sd1 {
+			vdd_dvfs: sd1 {
 				regulator-name = "1.315V CPU";
-				regulator-min-microvolt = <1315000>;
-				regulator-max-microvolt = <1335000>;
+				regulator-min-microvolt = <1050000>;
+				regulator-max-microvolt = <1350000>;
 				regulator-always-on;
 				regulator-boot-on;
 			};
diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index eb3a7ff..8a0420b 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -134,6 +134,8 @@ config MACH_KZM9G
 config MACH_KZM9G_REFERENCE
 	bool "KZM-A9-GT board - Reference Device Tree Implementation"
 	depends on ARCH_SH73A0
+	select ARCH_HAS_CPUFREQ
+	select ARCH_HAS_OPP
 	select ARCH_REQUIRE_GPIOLIB
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
 	select SND_SOC_AK4642 if SND_SIMPLE_CARD
-- 
1.7.2.5