cpufreq Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12
       [not found] <1380108138-30402-1-git-send-email-l.majewski@samsung.com>
@ 2013-10-09 12:08 ` Lukasz Majewski
  2013-10-09 12:08   ` [PATCH v2 1/2] cpufreq: exynos4x12: Use the common clock framework to set APLL clock rate Lukasz Majewski
                     ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Lukasz Majewski @ 2013-10-09 12:08 UTC (permalink / raw)
  To: Viresh Kumar, Rafael J. Wysocki
  Cc: cpufreq@vger.kernel.org, Linux PM list, Jonghwa Lee,
	Lukasz Majewski, Lukasz Majewski, linux-kernel,
	Bartlomiej Zolnierkiewicz, Myungjoo Ham, Yadwinder Singh Brar,
	Tomasz Figa

Attached commits provide cpufreq regression fixes for Trats and Trats2
Exynos4 boards.
Since v3.12 Exynos4 uses common clock framework for clock manipulation.
Those patches restore correct output for cpuinfo_cur_freq [1] sysfs
attribute.
Without them, the [1] provides default frequency (800 MHz) set at driver
initialization.


Lukasz Majewski (2):
  cpufreq: exynos4x12: Use the common clock framework to set APLL clock
    rate
  cpufreq: exynos4210: Use the common clock framework to set APLL clock
    rate

 drivers/cpufreq/exynos4210-cpufreq.c |   67 ++++-----------------------------
 drivers/cpufreq/exynos4x12-cpufreq.c |   69 ++++------------------------------
 2 files changed, 16 insertions(+), 120 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/2] cpufreq: exynos4x12: Use the common clock framework to set APLL clock rate
  2013-10-09 12:08 ` [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12 Lukasz Majewski
@ 2013-10-09 12:08   ` Lukasz Majewski
  2013-10-09 12:08   ` [PATCH v2 2/2] cpufreq: exynos4210: " Lukasz Majewski
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Lukasz Majewski @ 2013-10-09 12:08 UTC (permalink / raw)
  To: Viresh Kumar, Rafael J. Wysocki
  Cc: cpufreq@vger.kernel.org, Linux PM list, Jonghwa Lee,
	Lukasz Majewski, Lukasz Majewski, linux-kernel,
	Bartlomiej Zolnierkiewicz, Myungjoo Ham, Yadwinder Singh Brar,
	Tomasz Figa

In the exynos4x12_set_apll() function, the APLL frequency is set with
direct register manipulation.

Such approach is not allowed in the common clock framework. The frequency
is changed, but the corresponding clock value is not updated. This causes
wrong frequency read from cpufreq's cpuinfo_cur_freq sysfs attribute.

Also direct manipulation with PLL's S parameter has been removed. It is
already done at PLL35xx code.

Tested at:
- Exynos4412 - Trats2 board (linux 3.12-rc4)

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>

Changes for v2:
- Remove PLL's S parameter setting via registers. It is now done with PLL35xx
  code.
- Remove exynos4x12_pms_change() function.

Change-Id: I6c32d6fd7f634ff8d07cbaa0ebe0e21614f9f3c9
---
 drivers/cpufreq/exynos4x12-cpufreq.c |   69 ++++------------------------------
 1 file changed, 8 insertions(+), 61 deletions(-)

diff --git a/drivers/cpufreq/exynos4x12-cpufreq.c b/drivers/cpufreq/exynos4x12-cpufreq.c
index 08b7477..8683304 100644
--- a/drivers/cpufreq/exynos4x12-cpufreq.c
+++ b/drivers/cpufreq/exynos4x12-cpufreq.c
@@ -128,9 +128,9 @@ static void exynos4x12_set_clkdiv(unsigned int div_index)
 
 static void exynos4x12_set_apll(unsigned int index)
 {
-	unsigned int tmp, pdiv;
+	unsigned int tmp, freq = apll_freq_4x12[index].freq;
 
-	/* 1. MUX_CORE_SEL = MPLL, ARMCLK uses MPLL for lock time */
+	/* MUX_CORE_SEL = MPLL, ARMCLK uses MPLL for lock time */
 	clk_set_parent(moutcore, mout_mpll);
 
 	do {
@@ -140,24 +140,9 @@ static void exynos4x12_set_apll(unsigned int index)
 		tmp &= 0x7;
 	} while (tmp != 0x2);
 
-	/* 2. Set APLL Lock time */
-	pdiv = ((apll_freq_4x12[index].mps >> 8) & 0x3f);
+	clk_set_rate(mout_apll, freq * 1000);
 
-	__raw_writel((pdiv * 250), EXYNOS4_APLL_LOCK);
-
-	/* 3. Change PLL PMS values */
-	tmp = __raw_readl(EXYNOS4_APLL_CON0);
-	tmp &= ~((0x3ff << 16) | (0x3f << 8) | (0x7 << 0));
-	tmp |= apll_freq_4x12[index].mps;
-	__raw_writel(tmp, EXYNOS4_APLL_CON0);
-
-	/* 4. wait_lock_time */
-	do {
-		cpu_relax();
-		tmp = __raw_readl(EXYNOS4_APLL_CON0);
-	} while (!(tmp & (0x1 << EXYNOS4_APLLCON0_LOCKED_SHIFT)));
-
-	/* 5. MUX_CORE_SEL = APLL */
+	/* MUX_CORE_SEL = APLL */
 	clk_set_parent(moutcore, mout_apll);
 
 	do {
@@ -167,52 +152,15 @@ static void exynos4x12_set_apll(unsigned int index)
 	} while (tmp != (0x1 << EXYNOS4_CLKSRC_CPU_MUXCORE_SHIFT));
 }
 
-static bool exynos4x12_pms_change(unsigned int old_index, unsigned int new_index)
-{
-	unsigned int old_pm = apll_freq_4x12[old_index].mps >> 8;
-	unsigned int new_pm = apll_freq_4x12[new_index].mps >> 8;
-
-	return (old_pm == new_pm) ? 0 : 1;
-}
-
 static void exynos4x12_set_frequency(unsigned int old_index,
 				  unsigned int new_index)
 {
-	unsigned int tmp;
-
 	if (old_index > new_index) {
-		if (!exynos4x12_pms_change(old_index, new_index)) {
-			/* 1. Change the system clock divider values */
-			exynos4x12_set_clkdiv(new_index);
-			/* 2. Change just s value in apll m,p,s value */
-			tmp = __raw_readl(EXYNOS4_APLL_CON0);
-			tmp &= ~(0x7 << 0);
-			tmp |= apll_freq_4x12[new_index].mps & 0x7;
-			__raw_writel(tmp, EXYNOS4_APLL_CON0);
-
-		} else {
-			/* Clock Configuration Procedure */
-			/* 1. Change the system clock divider values */
-			exynos4x12_set_clkdiv(new_index);
-			/* 2. Change the apll m,p,s value */
-			exynos4x12_set_apll(new_index);
-		}
+		exynos4x12_set_clkdiv(new_index);
+		exynos4x12_set_apll(new_index);
 	} else if (old_index < new_index) {
-		if (!exynos4x12_pms_change(old_index, new_index)) {
-			/* 1. Change just s value in apll m,p,s value */
-			tmp = __raw_readl(EXYNOS4_APLL_CON0);
-			tmp &= ~(0x7 << 0);
-			tmp |= apll_freq_4x12[new_index].mps & 0x7;
-			__raw_writel(tmp, EXYNOS4_APLL_CON0);
-			/* 2. Change the system clock divider values */
-			exynos4x12_set_clkdiv(new_index);
-		} else {
-			/* Clock Configuration Procedure */
-			/* 1. Change the apll m,p,s value */
-			exynos4x12_set_apll(new_index);
-			/* 2. Change the system clock divider values */
-			exynos4x12_set_clkdiv(new_index);
-		}
+		exynos4x12_set_apll(new_index);
+		exynos4x12_set_clkdiv(new_index);
 	}
 }
 
@@ -250,7 +198,6 @@ int exynos4x12_cpufreq_init(struct exynos_dvfs_info *info)
 	info->volt_table = exynos4x12_volt_table;
 	info->freq_table = exynos4x12_freq_table;
 	info->set_freq = exynos4x12_set_frequency;
-	info->need_apll_change = exynos4x12_pms_change;
 
 	return 0;
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/2] cpufreq: exynos4210: Use the common clock framework to set APLL clock rate
  2013-10-09 12:08 ` [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12 Lukasz Majewski
  2013-10-09 12:08   ` [PATCH v2 1/2] cpufreq: exynos4x12: Use the common clock framework to set APLL clock rate Lukasz Majewski
@ 2013-10-09 12:08   ` Lukasz Majewski
  2013-10-16 22:58     ` Rafael J. Wysocki
  2013-10-11  6:06   ` [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12 Yadwinder Singh Brar
  2013-10-11 11:22   ` Rafael J. Wysocki
  3 siblings, 1 reply; 9+ messages in thread
From: Lukasz Majewski @ 2013-10-09 12:08 UTC (permalink / raw)
  To: Viresh Kumar, Rafael J. Wysocki
  Cc: cpufreq@vger.kernel.org, Linux PM list, Jonghwa Lee,
	Lukasz Majewski, Lukasz Majewski, linux-kernel,
	Bartlomiej Zolnierkiewicz, Myungjoo Ham, Yadwinder Singh Brar,
	Tomasz Figa

In the exynos4210_set_apll() function, the APLL frequency is set with
direct register manipulation.

Such approach is not allowed in the common clock framework. The frequency
is changed, but the corresponding clock value is not updated. This causes
wrong frequency read from cpufreq's cpuinfo_cur_freq sysfs attribute.

Also direct manipulation with PLL's S parameter has been removed. It is
already done at PLL35xx code.

Tested at:
- Exynos4210 - Trats board (linux 3.12-rc4)

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>

Changes for v2:
- Remove PLL's S parameter setting via registers. It is now done with PLL35xx
  code.
- Remove exynos4210_pms_change() function

Change-Id: Ie5fb3c7946ba77b6f3d5e91af72eef2fd06770c1
---
 drivers/cpufreq/exynos4210-cpufreq.c |   67 ++++------------------------------
 1 file changed, 8 insertions(+), 59 deletions(-)

diff --git a/drivers/cpufreq/exynos4210-cpufreq.c b/drivers/cpufreq/exynos4210-cpufreq.c
index add7fbe..f2c7506 100644
--- a/drivers/cpufreq/exynos4210-cpufreq.c
+++ b/drivers/cpufreq/exynos4210-cpufreq.c
@@ -81,9 +81,9 @@ static void exynos4210_set_clkdiv(unsigned int div_index)
 
 static void exynos4210_set_apll(unsigned int index)
 {
-	unsigned int tmp;
+	unsigned int tmp, freq = apll_freq_4210[index].freq;
 
-	/* 1. MUX_CORE_SEL = MPLL, ARMCLK uses MPLL for lock time */
+	/* MUX_CORE_SEL = MPLL, ARMCLK uses MPLL for lock time */
 	clk_set_parent(moutcore, mout_mpll);
 
 	do {
@@ -92,21 +92,9 @@ static void exynos4210_set_apll(unsigned int index)
 		tmp &= 0x7;
 	} while (tmp != 0x2);
 
-	/* 2. Set APLL Lock time */
-	__raw_writel(EXYNOS4_APLL_LOCKTIME, EXYNOS4_APLL_LOCK);
-
-	/* 3. Change PLL PMS values */
-	tmp = __raw_readl(EXYNOS4_APLL_CON0);
-	tmp &= ~((0x3ff << 16) | (0x3f << 8) | (0x7 << 0));
-	tmp |= apll_freq_4210[index].mps;
-	__raw_writel(tmp, EXYNOS4_APLL_CON0);
+	clk_set_rate(mout_apll, freq * 1000);
 
-	/* 4. wait_lock_time */
-	do {
-		tmp = __raw_readl(EXYNOS4_APLL_CON0);
-	} while (!(tmp & (0x1 << EXYNOS4_APLLCON0_LOCKED_SHIFT)));
-
-	/* 5. MUX_CORE_SEL = APLL */
+	/* MUX_CORE_SEL = APLL */
 	clk_set_parent(moutcore, mout_apll);
 
 	do {
@@ -115,53 +103,15 @@ static void exynos4210_set_apll(unsigned int index)
 	} while (tmp != (0x1 << EXYNOS4_CLKSRC_CPU_MUXCORE_SHIFT));
 }
 
-static bool exynos4210_pms_change(unsigned int old_index, unsigned int new_index)
-{
-	unsigned int old_pm = apll_freq_4210[old_index].mps >> 8;
-	unsigned int new_pm = apll_freq_4210[new_index].mps >> 8;
-
-	return (old_pm == new_pm) ? 0 : 1;
-}
-
 static void exynos4210_set_frequency(unsigned int old_index,
 				     unsigned int new_index)
 {
-	unsigned int tmp;
-
 	if (old_index > new_index) {
-		if (!exynos4210_pms_change(old_index, new_index)) {
-			/* 1. Change the system clock divider values */
-			exynos4210_set_clkdiv(new_index);
-
-			/* 2. Change just s value in apll m,p,s value */
-			tmp = __raw_readl(EXYNOS4_APLL_CON0);
-			tmp &= ~(0x7 << 0);
-			tmp |= apll_freq_4210[new_index].mps & 0x7;
-			__raw_writel(tmp, EXYNOS4_APLL_CON0);
-		} else {
-			/* Clock Configuration Procedure */
-			/* 1. Change the system clock divider values */
-			exynos4210_set_clkdiv(new_index);
-			/* 2. Change the apll m,p,s value */
-			exynos4210_set_apll(new_index);
-		}
+		exynos4210_set_clkdiv(new_index);
+		exynos4210_set_apll(new_index);
 	} else if (old_index < new_index) {
-		if (!exynos4210_pms_change(old_index, new_index)) {
-			/* 1. Change just s value in apll m,p,s value */
-			tmp = __raw_readl(EXYNOS4_APLL_CON0);
-			tmp &= ~(0x7 << 0);
-			tmp |= apll_freq_4210[new_index].mps & 0x7;
-			__raw_writel(tmp, EXYNOS4_APLL_CON0);
-
-			/* 2. Change the system clock divider values */
-			exynos4210_set_clkdiv(new_index);
-		} else {
-			/* Clock Configuration Procedure */
-			/* 1. Change the apll m,p,s value */
-			exynos4210_set_apll(new_index);
-			/* 2. Change the system clock divider values */
-			exynos4210_set_clkdiv(new_index);
-		}
+		exynos4210_set_apll(new_index);
+		exynos4210_set_clkdiv(new_index);
 	}
 }
 
@@ -194,7 +144,6 @@ int exynos4210_cpufreq_init(struct exynos_dvfs_info *info)
 	info->volt_table = exynos4210_volt_table;
 	info->freq_table = exynos4210_freq_table;
 	info->set_freq = exynos4210_set_frequency;
-	info->need_apll_change = exynos4210_pms_change;
 
 	return 0;
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12
  2013-10-09 12:08 ` [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12 Lukasz Majewski
  2013-10-09 12:08   ` [PATCH v2 1/2] cpufreq: exynos4x12: Use the common clock framework to set APLL clock rate Lukasz Majewski
  2013-10-09 12:08   ` [PATCH v2 2/2] cpufreq: exynos4210: " Lukasz Majewski
@ 2013-10-11  6:06   ` Yadwinder Singh Brar
  2013-10-11 11:22   ` Rafael J. Wysocki
  3 siblings, 0 replies; 9+ messages in thread
From: Yadwinder Singh Brar @ 2013-10-11  6:06 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Viresh Kumar, Rafael J. Wysocki, cpufreq@vger.kernel.org,
	Linux PM list, Jonghwa Lee, Lukasz Majewski, linux-kernel,
	Bartlomiej Zolnierkiewicz, Myungjoo Ham, Tomasz Figa

On Wed, Oct 9, 2013 at 5:38 PM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> Attached commits provide cpufreq regression fixes for Trats and Trats2
> Exynos4 boards.
> Since v3.12 Exynos4 uses common clock framework for clock manipulation.
> Those patches restore correct output for cpuinfo_cur_freq [1] sysfs
> attribute.
> Without them, the [1] provides default frequency (800 MHz) set at driver
> initialization.
>
>
> Lukasz Majewski (2):
>   cpufreq: exynos4x12: Use the common clock framework to set APLL clock
>     rate
>   cpufreq: exynos4210: Use the common clock framework to set APLL clock
>     rate
>

Just as a fix for 3.12, these patches looks OK to me.

Reviewed-by: Yadwinder Singh Brar <yadi.brar@samsung.com>


Regards,
Yadwinder

>  drivers/cpufreq/exynos4210-cpufreq.c |   67 ++++-----------------------------
>  drivers/cpufreq/exynos4x12-cpufreq.c |   69 ++++------------------------------
>  2 files changed, 16 insertions(+), 120 deletions(-)
>
> --
> 1.7.10.4
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12
  2013-10-09 12:08 ` [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12 Lukasz Majewski
                     ` (2 preceding siblings ...)
  2013-10-11  6:06   ` [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12 Yadwinder Singh Brar
@ 2013-10-11 11:22   ` Rafael J. Wysocki
  2013-10-11 12:10     ` Rafael J. Wysocki
  3 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2013-10-11 11:22 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Viresh Kumar, cpufreq@vger.kernel.org, Linux PM list, Jonghwa Lee,
	Lukasz Majewski, linux-kernel, Bartlomiej Zolnierkiewicz,
	Myungjoo Ham, Yadwinder Singh Brar, Tomasz Figa

On Wednesday, October 09, 2013 02:08:41 PM Lukasz Majewski wrote:
> Attached commits provide cpufreq regression fixes for Trats and Trats2
> Exynos4 boards.
> Since v3.12 Exynos4 uses common clock framework for clock manipulation.
> Those patches restore correct output for cpuinfo_cur_freq [1] sysfs
> attribute.
> Without them, the [1] provides default frequency (800 MHz) set at driver
> initialization.
> 
> 
> Lukasz Majewski (2):
>   cpufreq: exynos4x12: Use the common clock framework to set APLL clock
>     rate
>   cpufreq: exynos4210: Use the common clock framework to set APLL clock
>     rate
> 
>  drivers/cpufreq/exynos4210-cpufreq.c |   67 ++++-----------------------------
>  drivers/cpufreq/exynos4x12-cpufreq.c |   69 ++++------------------------------
>  2 files changed, 16 insertions(+), 120 deletions(-)

Can you please resend [1/2], it got lost somewhere (at least I can't see it
now)?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12
  2013-10-11 11:22   ` Rafael J. Wysocki
@ 2013-10-11 12:10     ` Rafael J. Wysocki
  2013-10-14  5:55       ` Lukasz Majewski
  0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2013-10-11 12:10 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Viresh Kumar, cpufreq@vger.kernel.org, Linux PM list, Jonghwa Lee,
	Lukasz Majewski, linux-kernel, Bartlomiej Zolnierkiewicz,
	Myungjoo Ham, Yadwinder Singh Brar, Tomasz Figa

On Friday, October 11, 2013 01:22:57 PM Rafael J. Wysocki wrote:
> On Wednesday, October 09, 2013 02:08:41 PM Lukasz Majewski wrote:
> > Attached commits provide cpufreq regression fixes for Trats and Trats2
> > Exynos4 boards.
> > Since v3.12 Exynos4 uses common clock framework for clock manipulation.
> > Those patches restore correct output for cpuinfo_cur_freq [1] sysfs
> > attribute.
> > Without them, the [1] provides default frequency (800 MHz) set at driver
> > initialization.
> > 
> > 
> > Lukasz Majewski (2):
> >   cpufreq: exynos4x12: Use the common clock framework to set APLL clock
> >     rate
> >   cpufreq: exynos4210: Use the common clock framework to set APLL clock
> >     rate
> > 
> >  drivers/cpufreq/exynos4210-cpufreq.c |   67 ++++-----------------------------
> >  drivers/cpufreq/exynos4x12-cpufreq.c |   69 ++++------------------------------
> >  2 files changed, 16 insertions(+), 120 deletions(-)
> 
> Can you please resend [1/2], it got lost somewhere (at least I can't see it
> now)?

Scratch this, it's there in patchwork.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12
  2013-10-11 12:10     ` Rafael J. Wysocki
@ 2013-10-14  5:55       ` Lukasz Majewski
  2013-10-14 11:55         ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ messages in thread
From: Lukasz Majewski @ 2013-10-14  5:55 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Viresh Kumar, cpufreq@vger.kernel.org, Linux PM list, Jonghwa Lee,
	Lukasz Majewski, linux-kernel, Bartlomiej Zolnierkiewicz,
	Myungjoo Ham, Yadwinder Singh Brar, Tomasz Figa

Hi Rafael,

> On Friday, October 11, 2013 01:22:57 PM Rafael J. Wysocki wrote:
> > On Wednesday, October 09, 2013 02:08:41 PM Lukasz Majewski wrote:
> > > Attached commits provide cpufreq regression fixes for Trats and
> > > Trats2 Exynos4 boards.
> > > Since v3.12 Exynos4 uses common clock framework for clock
> > > manipulation. Those patches restore correct output for
> > > cpuinfo_cur_freq [1] sysfs attribute.
> > > Without them, the [1] provides default frequency (800 MHz) set at
> > > driver initialization.
> > > 
> > > 
> > > Lukasz Majewski (2):
> > >   cpufreq: exynos4x12: Use the common clock framework to set APLL
> > > clock rate
> > >   cpufreq: exynos4210: Use the common clock framework to set APLL
> > > clock rate
> > > 
> > >  drivers/cpufreq/exynos4210-cpufreq.c |   67
> > > ++++-----------------------------
> > > drivers/cpufreq/exynos4x12-cpufreq.c |   69
> > > ++++------------------------------ 2 files changed, 16
> > > insertions(+), 120 deletions(-)
> > 
> > Can you please resend [1/2], it got lost somewhere (at least I
> > can't see it now)?
> 
> Scratch this, it's there in patchwork.
> 

Rafael, do you need any more help with those fixes? 

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12
  2013-10-14  5:55       ` Lukasz Majewski
@ 2013-10-14 11:55         ` Rafael J. Wysocki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2013-10-14 11:55 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Viresh Kumar, cpufreq@vger.kernel.org, Linux PM list, Jonghwa Lee,
	Lukasz Majewski, linux-kernel, Bartlomiej Zolnierkiewicz,
	Myungjoo Ham, Yadwinder Singh Brar, Tomasz Figa

On Monday, October 14, 2013 07:55:46 AM Lukasz Majewski wrote:
> Hi Rafael,
> 
> > On Friday, October 11, 2013 01:22:57 PM Rafael J. Wysocki wrote:
> > > On Wednesday, October 09, 2013 02:08:41 PM Lukasz Majewski wrote:
> > > > Attached commits provide cpufreq regression fixes for Trats and
> > > > Trats2 Exynos4 boards.
> > > > Since v3.12 Exynos4 uses common clock framework for clock
> > > > manipulation. Those patches restore correct output for
> > > > cpuinfo_cur_freq [1] sysfs attribute.
> > > > Without them, the [1] provides default frequency (800 MHz) set at
> > > > driver initialization.
> > > > 
> > > > 
> > > > Lukasz Majewski (2):
> > > >   cpufreq: exynos4x12: Use the common clock framework to set APLL
> > > > clock rate
> > > >   cpufreq: exynos4210: Use the common clock framework to set APLL
> > > > clock rate
> > > > 
> > > >  drivers/cpufreq/exynos4210-cpufreq.c |   67
> > > > ++++-----------------------------
> > > > drivers/cpufreq/exynos4x12-cpufreq.c |   69
> > > > ++++------------------------------ 2 files changed, 16
> > > > insertions(+), 120 deletions(-)
> > > 
> > > Can you please resend [1/2], it got lost somewhere (at least I
> > > can't see it now)?
> > 
> > Scratch this, it's there in patchwork.
> > 
> 
> Rafael, do you need any more help with those fixes? 

Well, if I do, you'll know that. :-)

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] cpufreq: exynos4210: Use the common clock framework to set APLL clock rate
  2013-10-09 12:08   ` [PATCH v2 2/2] cpufreq: exynos4210: " Lukasz Majewski
@ 2013-10-16 22:58     ` Rafael J. Wysocki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2013-10-16 22:58 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Viresh Kumar, cpufreq@vger.kernel.org, Linux PM list, Jonghwa Lee,
	Lukasz Majewski, linux-kernel, Bartlomiej Zolnierkiewicz,
	Myungjoo Ham, Yadwinder Singh Brar, Tomasz Figa

On Wednesday, October 09, 2013 02:08:43 PM Lukasz Majewski wrote:
> In the exynos4210_set_apll() function, the APLL frequency is set with
> direct register manipulation.
> 
> Such approach is not allowed in the common clock framework. The frequency
> is changed, but the corresponding clock value is not updated. This causes
> wrong frequency read from cpufreq's cpuinfo_cur_freq sysfs attribute.
> 
> Also direct manipulation with PLL's S parameter has been removed. It is
> already done at PLL35xx code.
> 
> Tested at:
> - Exynos4210 - Trats board (linux 3.12-rc4)
> 
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>

I need an ACK or Reviewed-by from someone in the CC list here.

Thanks!

> Changes for v2:
> - Remove PLL's S parameter setting via registers. It is now done with PLL35xx
>   code.
> - Remove exynos4210_pms_change() function
> 
> Change-Id: Ie5fb3c7946ba77b6f3d5e91af72eef2fd06770c1
> ---
>  drivers/cpufreq/exynos4210-cpufreq.c |   67 ++++------------------------------
>  1 file changed, 8 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/cpufreq/exynos4210-cpufreq.c b/drivers/cpufreq/exynos4210-cpufreq.c
> index add7fbe..f2c7506 100644
> --- a/drivers/cpufreq/exynos4210-cpufreq.c
> +++ b/drivers/cpufreq/exynos4210-cpufreq.c
> @@ -81,9 +81,9 @@ static void exynos4210_set_clkdiv(unsigned int div_index)
>  
>  static void exynos4210_set_apll(unsigned int index)
>  {
> -	unsigned int tmp;
> +	unsigned int tmp, freq = apll_freq_4210[index].freq;
>  
> -	/* 1. MUX_CORE_SEL = MPLL, ARMCLK uses MPLL for lock time */
> +	/* MUX_CORE_SEL = MPLL, ARMCLK uses MPLL for lock time */
>  	clk_set_parent(moutcore, mout_mpll);
>  
>  	do {
> @@ -92,21 +92,9 @@ static void exynos4210_set_apll(unsigned int index)
>  		tmp &= 0x7;
>  	} while (tmp != 0x2);
>  
> -	/* 2. Set APLL Lock time */
> -	__raw_writel(EXYNOS4_APLL_LOCKTIME, EXYNOS4_APLL_LOCK);
> -
> -	/* 3. Change PLL PMS values */
> -	tmp = __raw_readl(EXYNOS4_APLL_CON0);
> -	tmp &= ~((0x3ff << 16) | (0x3f << 8) | (0x7 << 0));
> -	tmp |= apll_freq_4210[index].mps;
> -	__raw_writel(tmp, EXYNOS4_APLL_CON0);
> +	clk_set_rate(mout_apll, freq * 1000);
>  
> -	/* 4. wait_lock_time */
> -	do {
> -		tmp = __raw_readl(EXYNOS4_APLL_CON0);
> -	} while (!(tmp & (0x1 << EXYNOS4_APLLCON0_LOCKED_SHIFT)));
> -
> -	/* 5. MUX_CORE_SEL = APLL */
> +	/* MUX_CORE_SEL = APLL */
>  	clk_set_parent(moutcore, mout_apll);
>  
>  	do {
> @@ -115,53 +103,15 @@ static void exynos4210_set_apll(unsigned int index)
>  	} while (tmp != (0x1 << EXYNOS4_CLKSRC_CPU_MUXCORE_SHIFT));
>  }
>  
> -static bool exynos4210_pms_change(unsigned int old_index, unsigned int new_index)
> -{
> -	unsigned int old_pm = apll_freq_4210[old_index].mps >> 8;
> -	unsigned int new_pm = apll_freq_4210[new_index].mps >> 8;
> -
> -	return (old_pm == new_pm) ? 0 : 1;
> -}
> -
>  static void exynos4210_set_frequency(unsigned int old_index,
>  				     unsigned int new_index)
>  {
> -	unsigned int tmp;
> -
>  	if (old_index > new_index) {
> -		if (!exynos4210_pms_change(old_index, new_index)) {
> -			/* 1. Change the system clock divider values */
> -			exynos4210_set_clkdiv(new_index);
> -
> -			/* 2. Change just s value in apll m,p,s value */
> -			tmp = __raw_readl(EXYNOS4_APLL_CON0);
> -			tmp &= ~(0x7 << 0);
> -			tmp |= apll_freq_4210[new_index].mps & 0x7;
> -			__raw_writel(tmp, EXYNOS4_APLL_CON0);
> -		} else {
> -			/* Clock Configuration Procedure */
> -			/* 1. Change the system clock divider values */
> -			exynos4210_set_clkdiv(new_index);
> -			/* 2. Change the apll m,p,s value */
> -			exynos4210_set_apll(new_index);
> -		}
> +		exynos4210_set_clkdiv(new_index);
> +		exynos4210_set_apll(new_index);
>  	} else if (old_index < new_index) {
> -		if (!exynos4210_pms_change(old_index, new_index)) {
> -			/* 1. Change just s value in apll m,p,s value */
> -			tmp = __raw_readl(EXYNOS4_APLL_CON0);
> -			tmp &= ~(0x7 << 0);
> -			tmp |= apll_freq_4210[new_index].mps & 0x7;
> -			__raw_writel(tmp, EXYNOS4_APLL_CON0);
> -
> -			/* 2. Change the system clock divider values */
> -			exynos4210_set_clkdiv(new_index);
> -		} else {
> -			/* Clock Configuration Procedure */
> -			/* 1. Change the apll m,p,s value */
> -			exynos4210_set_apll(new_index);
> -			/* 2. Change the system clock divider values */
> -			exynos4210_set_clkdiv(new_index);
> -		}
> +		exynos4210_set_apll(new_index);
> +		exynos4210_set_clkdiv(new_index);
>  	}
>  }
>  
> @@ -194,7 +144,6 @@ int exynos4210_cpufreq_init(struct exynos_dvfs_info *info)
>  	info->volt_table = exynos4210_volt_table;
>  	info->freq_table = exynos4210_freq_table;
>  	info->set_freq = exynos4210_set_frequency;
> -	info->need_apll_change = exynos4210_pms_change;
>  
>  	return 0;
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-10-16 22:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1380108138-30402-1-git-send-email-l.majewski@samsung.com>
2013-10-09 12:08 ` [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12 Lukasz Majewski
2013-10-09 12:08   ` [PATCH v2 1/2] cpufreq: exynos4x12: Use the common clock framework to set APLL clock rate Lukasz Majewski
2013-10-09 12:08   ` [PATCH v2 2/2] cpufreq: exynos4210: " Lukasz Majewski
2013-10-16 22:58     ` Rafael J. Wysocki
2013-10-11  6:06   ` [PATCH v2 0/2] cpufreq: exynos: Fixes for v3.12 Yadwinder Singh Brar
2013-10-11 11:22   ` Rafael J. Wysocki
2013-10-11 12:10     ` Rafael J. Wysocki
2013-10-14  5:55       ` Lukasz Majewski
2013-10-14 11:55         ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox