[PATCH 0/5] ARM: OMAP1: Fix dpll1 reprogramming related issues

[PATCH 0/5] ARM: OMAP1: Fix dpll1 reprogramming related issues

[Janusz Krzysztofik]

After dpll1 reprogramming has been moved from setup_arch() to
kernel_init(), I've been observing several issues, resulting in
undesired system behaviour on my Amstrad Delta. This series fixes
those issues.

Janusz Krzysztofik (5):
  ARM: OMAP1: Fix dpll1 default rate reprogramming method
  ARM: OMAP1: Fix ckctl value used for dpll1 defualt rate
  ARM: OMAP1: Fix dpll1 reprogramming not actually allowed
  init/calibrate.c: allow for recalibration of loops per jiffy
  ARM: OMAP1: recalibrate loops per jiffy after dpll1 reprogram

 arch/arm/mach-omap1/clock.c      |   10 +---------
 arch/arm/mach-omap1/clock_data.c |    9 +++++++--
 arch/arm/plat-omap/sram.c        |    3 +++
 include/linux/delay.h            |    4 +++-
 init/calibrate.c                 |    6 ++++--
 5 files changed, 18 insertions(+), 14 deletions(-)

-- 
1.7.3.4

[PATCH 1/5] ARM: OMAP1: Fix dpll1 default rate reprogramming method

[Janusz Krzysztofik]

According to comments in omap1_select_table_rate(), reprogramming dpll1
is tricky, and should always be done from SRAM.

While being at it, move OMAP730 special case handling inside
omap_sram_reprogram_clock().

Created on top of omap-fixes, not yet in mainline as of 3.2-rc3.
Tested on Amstrad Delta.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---
 arch/arm/mach-omap1/clock.c      |    6 +-----
 arch/arm/mach-omap1/clock_data.c |    7 +++++--
 arch/arm/plat-omap/sram.c        |    3 +++
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap1/clock.c b/arch/arm/mach-omap1/clock.c
index 84ef704..3899176 100644
--- a/arch/arm/mach-omap1/clock.c
+++ b/arch/arm/mach-omap1/clock.c
@@ -215,12 +215,8 @@ int omap1_select_table_rate(struct clk *clk, unsigned long rate)
 	/*
 	 * In most cases we should not need to reprogram DPLL.
 	 * Reprogramming the DPLL is tricky, it must be done from SRAM.
-	 * (on 730, bit 13 must always be 1)
 	 */
-	if (cpu_is_omap7xx())
-		omap_sram_reprogram_clock(ptr->dpllctl_val, ptr->ckctl_val | 0x2000);
-	else
-		omap_sram_reprogram_clock(ptr->dpllctl_val, ptr->ckctl_val);
+	omap_sram_reprogram_clock(ptr->dpllctl_val, ptr->ckctl_val);
 
 	/* XXX Do we need to recalculate the tree below DPLL1 at this point? */
 	ck_dpll1_p->rate = ptr->pll_rate;
diff --git a/arch/arm/mach-omap1/clock_data.c b/arch/arm/mach-omap1/clock_data.c
index 1297bb5..4550415 100644
--- a/arch/arm/mach-omap1/clock_data.c
+++ b/arch/arm/mach-omap1/clock_data.c
@@ -23,6 +23,7 @@
 #include <plat/clock.h>
 #include <plat/cpu.h>
 #include <plat/clkdev_omap.h>
+#include <plat/sram.h>	/* for omap_sram_reprogram_clock() */
 #include <plat/usb.h>   /* for OTG_BASE */
 
 #include "clock.h"
@@ -933,8 +934,10 @@ void __init omap1_clk_late_init(void)
 	/* Find the highest supported frequency and enable it */
 	if (omap1_select_table_rate(&virtual_ck_mpu, ~0)) {
 		pr_err("System frequencies not set, using default. Check your config.\n");
-		omap_writew(0x2290, DPLL_CTL);
-		omap_writew(cpu_is_omap7xx() ? 0x3005 : 0x1005, ARM_CKCTL);
+		/*
+		 * Reprogramming the DPLL is tricky, it must be done from SRAM.
+		 */
+		omap_sram_reprogram_clock(0x2290, 0x1005);
 		ck_dpll1.rate = OMAP1_DPLL1_SANE_VALUE;
 	}
 	propagate_rate(&ck_dpll1);
diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
index 8b28664..312bee8 100644
--- a/arch/arm/plat-omap/sram.c
+++ b/arch/arm/plat-omap/sram.c
@@ -224,6 +224,9 @@ static void (*_omap_sram_reprogram_clock)(u32 dpllctl, u32 ckctl);
 void omap_sram_reprogram_clock(u32 dpllctl, u32 ckctl)
 {
 	BUG_ON(!_omap_sram_reprogram_clock);
+	/* On 730, bit 13 must always be 1 */
+	if (cpu_is_omap7xx())
+		ckctl |= 0x2000;
 	_omap_sram_reprogram_clock(dpllctl, ckctl);
 }
 
-- 
1.7.3.4

[PATCH 2/5] ARM: OMAP1: Fix ckctl value used for dpll1 defualt rate

[Janusz Krzysztofik]

Use the exact value found in omap1_rate_table, otherwise I have been
experiencing issues with correct timekeeping on my Amstrad Delta.

Created and tested on top of patch "ARM: OMAP1: Fix dpll1 default rate
reprogramming method".

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---
 arch/arm/mach-omap1/clock_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap1/clock_data.c b/arch/arm/mach-omap1/clock_data.c
index 4550415..201b1f2 100644
--- a/arch/arm/mach-omap1/clock_data.c
+++ b/arch/arm/mach-omap1/clock_data.c
@@ -937,7 +937,7 @@ void __init omap1_clk_late_init(void)
 		/*
 		 * Reprogramming the DPLL is tricky, it must be done from SRAM.
 		 */
-		omap_sram_reprogram_clock(0x2290, 0x1005);
+		omap_sram_reprogram_clock(0x2290, 0x0005);
 		ck_dpll1.rate = OMAP1_DPLL1_SANE_VALUE;
 	}
 	propagate_rate(&ck_dpll1);
-- 
1.7.3.4

[PATCH 3/5] ARM: OMAP1: Fix dpll1 reprogramming not actually allowed

[Janusz Krzysztofik]

DPLL1 reprogramming to a different rate is actually blocked inside
omap1_select_table_rate(), resulting in the defalut rate of 60 MHz
always used instead of the one selected in .config. Fix it.

Created against linux-3.2-rc2, tested on Amstrad Delta on top of
linux-omap/fixes.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---
 arch/arm/mach-omap1/clock.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap1/clock.c b/arch/arm/mach-omap1/clock.c
index 3899176..962adbc 100644
--- a/arch/arm/mach-omap1/clock.c
+++ b/arch/arm/mach-omap1/clock.c
@@ -200,10 +200,6 @@ int omap1_select_table_rate(struct clk *clk, unsigned long rate)
 		if (ptr->xtal != ref_rate)
 			continue;
 
-		/* DPLL1 cannot be reprogrammed without risking system crash */
-		if (likely(dpll1_rate != 0) && ptr->pll_rate != dpll1_rate)
-			continue;
-
 		/* Can check only after xtal frequency check */
 		if (ptr->rate <= rate)
 			break;
-- 
1.7.3.4

[PATCH 4/5] init/calibrate.c: allow for recalibration of loops per jiffy

[Janusz Krzysztofik]

This is required on ARM OMAP1 platform, after DPLL reprogramming has
been fixed by moving it from setup_arch() to kernel_init().

Created against linux-3.2-rc2.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Sameer Nanda <snanda@chromium.org>
---
 include/linux/delay.h |    4 +++-
 init/calibrate.c      |    6 ++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/linux/delay.h b/include/linux/delay.h
index a6ecb34..ed0fbbe 100644
--- a/include/linux/delay.h
+++ b/include/linux/delay.h
@@ -42,7 +42,9 @@ static inline void ndelay(unsigned long x)
 #endif
 
 extern unsigned long lpj_fine;
-void calibrate_delay(void);
+void _calibrate_delay(bool redo);
+#define calibrate_delay() _calibrate_delay(0)
+#define recalibrate_delay() _calibrate_delay(1)
 void msleep(unsigned int msecs);
 unsigned long msleep_interruptible(unsigned int msecs);
 void usleep_range(unsigned long min, unsigned long max);
diff --git a/init/calibrate.c b/init/calibrate.c
index 24df797..f7c1593 100644
--- a/init/calibrate.c
+++ b/init/calibrate.c
@@ -246,13 +246,15 @@ recalibrate:
 
 static DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
 
-void __cpuinit calibrate_delay(void)
+void __cpuinit _calibrate_delay(bool redo)
 {
 	unsigned long lpj;
 	static bool printed;
 	int this_cpu = smp_processor_id();
 
-	if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
+	if (redo)
+		printed = 0;
+	if (!redo && per_cpu(cpu_loops_per_jiffy, this_cpu)) {
 		lpj = per_cpu(cpu_loops_per_jiffy, this_cpu);
 		pr_info("Calibrating delay loop (skipped) "
 				"already calibrated this CPU");
-- 
1.7.3.4

[PATCH 5/5] ARM: OMAP1: recalibrate loops per jiffy after dpll1 reprogram

[Janusz Krzysztofik]

Otherwise timing is not accurate, resulting in devices which depend
on it, like omap-keypad, broken.

Created against linux-3.2-rc2.
Depends on patch "init: allow for recalibration of loops per jiffy".
Tested on top of linux-omap/fixes on Amstrad Delta.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Sameer Nanda <snanda@chromium.org>
---
 arch/arm/mach-omap1/clock_data.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap1/clock_data.c b/arch/arm/mach-omap1/clock_data.c
index 201b1f2..7fa6b8b 100644
--- a/arch/arm/mach-omap1/clock_data.c
+++ b/arch/arm/mach-omap1/clock_data.c
@@ -16,6 +16,7 @@
 
 #include <linux/kernel.h>
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/io.h>
 
 #include <asm/mach-types.h>  /* for machine_is_* */
@@ -942,4 +943,5 @@ void __init omap1_clk_late_init(void)
 	}
 	propagate_rate(&ck_dpll1);
 	omap1_show_rates();
+	recalibrate_delay();
 }
-- 
1.7.3.4

Re: [PATCH 4/5] init/calibrate.c: allow for recalibration of loops per jiffy

[Russell King - ARM Linux]

On Sun, Nov 27, 2011 at 04:32:30AM +0100, Janusz Krzysztofik wrote:
> This is required on ARM OMAP1 platform, after DPLL reprogramming has
> been fixed by moving it from setup_arch() to kernel_init().
> 
> Created against linux-3.2-rc2.

Why do you need this?  If you know what the original CPU clock rate and
bogomips were, and the new CPU clock rate, then you simply scale the
bogomips from the original accordingly.  That's what cpufreq does.

You don't have to waste 10s of milliseconds redoing the bogomips
calibration.

Re: [PATCH 1/5] ARM: OMAP1: Fix dpll1 default rate reprogramming method

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111126 19:06]:
> According to comments in omap1_select_table_rate(), reprogramming dpll1
> is tricky, and should always be done from SRAM.
> 
> While being at it, move OMAP730 special case handling inside
> omap_sram_reprogram_clock().
> 
> Created on top of omap-fixes, not yet in mainline as of 3.2-rc3.
> Tested on Amstrad Delta.

Thanks for looking into these issues. For most of this we should plan on
getting them done for the next merge window because we currently rely
on Kconfig options for the supported MHz rates. This means that with
this change omap1_defconfig will stop booting on most systems as it
would try to change to 216MHz. To fix that issue we should allow the
boards to set the supported rates before applying this patch.

Regards,

Tony
 
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> ---
>  arch/arm/mach-omap1/clock.c      |    6 +-----
>  arch/arm/mach-omap1/clock_data.c |    7 +++++--
>  arch/arm/plat-omap/sram.c        |    3 +++
>  3 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/clock.c b/arch/arm/mach-omap1/clock.c
> index 84ef704..3899176 100644
> --- a/arch/arm/mach-omap1/clock.c
> +++ b/arch/arm/mach-omap1/clock.c
> @@ -215,12 +215,8 @@ int omap1_select_table_rate(struct clk *clk, unsigned long rate)
>  	/*
>  	 * In most cases we should not need to reprogram DPLL.
>  	 * Reprogramming the DPLL is tricky, it must be done from SRAM.
> -	 * (on 730, bit 13 must always be 1)
>  	 */
> -	if (cpu_is_omap7xx())
> -		omap_sram_reprogram_clock(ptr->dpllctl_val, ptr->ckctl_val | 0x2000);
> -	else
> -		omap_sram_reprogram_clock(ptr->dpllctl_val, ptr->ckctl_val);
> +	omap_sram_reprogram_clock(ptr->dpllctl_val, ptr->ckctl_val);
>  
>  	/* XXX Do we need to recalculate the tree below DPLL1 at this point? */
>  	ck_dpll1_p->rate = ptr->pll_rate;
> diff --git a/arch/arm/mach-omap1/clock_data.c b/arch/arm/mach-omap1/clock_data.c
> index 1297bb5..4550415 100644
> --- a/arch/arm/mach-omap1/clock_data.c
> +++ b/arch/arm/mach-omap1/clock_data.c
> @@ -23,6 +23,7 @@
>  #include <plat/clock.h>
>  #include <plat/cpu.h>
>  #include <plat/clkdev_omap.h>
> +#include <plat/sram.h>	/* for omap_sram_reprogram_clock() */
>  #include <plat/usb.h>   /* for OTG_BASE */
>  
>  #include "clock.h"
> @@ -933,8 +934,10 @@ void __init omap1_clk_late_init(void)
>  	/* Find the highest supported frequency and enable it */
>  	if (omap1_select_table_rate(&virtual_ck_mpu, ~0)) {
>  		pr_err("System frequencies not set, using default. Check your config.\n");
> -		omap_writew(0x2290, DPLL_CTL);
> -		omap_writew(cpu_is_omap7xx() ? 0x3005 : 0x1005, ARM_CKCTL);
> +		/*
> +		 * Reprogramming the DPLL is tricky, it must be done from SRAM.
> +		 */
> +		omap_sram_reprogram_clock(0x2290, 0x1005);
>  		ck_dpll1.rate = OMAP1_DPLL1_SANE_VALUE;
>  	}
>  	propagate_rate(&ck_dpll1);
> diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
> index 8b28664..312bee8 100644
> --- a/arch/arm/plat-omap/sram.c
> +++ b/arch/arm/plat-omap/sram.c
> @@ -224,6 +224,9 @@ static void (*_omap_sram_reprogram_clock)(u32 dpllctl, u32 ckctl);
>  void omap_sram_reprogram_clock(u32 dpllctl, u32 ckctl)
>  {
>  	BUG_ON(!_omap_sram_reprogram_clock);
> +	/* On 730, bit 13 must always be 1 */
> +	if (cpu_is_omap7xx())
> +		ckctl |= 0x2000;
>  	_omap_sram_reprogram_clock(dpllctl, ckctl);
>  }
>  
> -- 
> 1.7.3.4
> 

Re: [PATCH 3/5] ARM: OMAP1: Fix dpll1 reprogramming not actually allowed

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111126 19:06]:
> DPLL1 reprogramming to a different rate is actually blocked inside
> omap1_select_table_rate(), resulting in the defalut rate of 60 MHz
> always used instead of the one selected in .config. Fix it.
> 
> Created against linux-3.2-rc2, tested on Amstrad Delta on top of
> linux-omap/fixes.

This too first requires something for boards to pass the supported
rates.

Tony
 
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> ---
>  arch/arm/mach-omap1/clock.c |    4 ----
>  1 files changed, 0 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/clock.c b/arch/arm/mach-omap1/clock.c
> index 3899176..962adbc 100644
> --- a/arch/arm/mach-omap1/clock.c
> +++ b/arch/arm/mach-omap1/clock.c
> @@ -200,10 +200,6 @@ int omap1_select_table_rate(struct clk *clk, unsigned long rate)
>  		if (ptr->xtal != ref_rate)
>  			continue;
>  
> -		/* DPLL1 cannot be reprogrammed without risking system crash */
> -		if (likely(dpll1_rate != 0) && ptr->pll_rate != dpll1_rate)
> -			continue;
> -
>  		/* Can check only after xtal frequency check */
>  		if (ptr->rate <= rate)
>  			break;
> -- 
> 1.7.3.4
> 

[PATCH 2a/5] Remove unsafe clock values from omap1_defconfig

[Janusz Krzysztofik]

DPLL1 reprogramming to a different rate is actually blocked inside
omap1_select_table_rate(), resulting in the defalut rate of 60 MHz
always used instead of the one selected in .config. OTOH, in
omap1_defconfig we currently rely on Kconfig options for the supported
MHz rates in case of boards which boot with dpll1 not set correctly by
their boot loaders.

This means that before we allow for reprogramming of dpll1 rate, we
should remove all unsafe clock selections from omap1_defconfig,
otherwise it will stop booting on boards with imperfect boot loaders,
as it would always try to change to 216MHz.

Keep only one safe clock rate per each supported xtal frequency, i.e.
60MHZ dpll1 for 12MHz xtal and 182MHz dpll1 for 13MHz xtal.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---
On Monday 28 of November 2011 at 18:45:09, Tony Lindgren wrote:
> * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111126 19:06]:
> > According to comments in omap1_select_table_rate(), reprogramming dpll1
> > is tricky, and should always be done from SRAM.
> > 
> > While being at it, move OMAP730 special case handling inside
> > omap_sram_reprogram_clock().
> > 
> > Created on top of omap-fixes, not yet in mainline as of 3.2-rc3.
> > Tested on Amstrad Delta.
> 
> Thanks for looking into these issues. For most of this we should plan on
> getting them done for the next merge window because we currently rely
> on Kconfig options for the supported MHz rates. This means that with
> this change omap1_defconfig will stop booting on most systems as it
> would try to change to 216MHz. To fix that issue we should allow the
> boards to set the supported rates before applying this patch.

Hi Tony,
I'm not sure which of the patches you meant not ready for 3.2-rc. From
your comment I would conclude that only patch 3/5, which would really
break omap1_defconfig booting on some boards, is questionable, while you
posted this comment against patch 1/5, which I don't see how it could
break booting, including omap1_defconfig.

Anyway, I hope that the additional patch below is addressing all your
concerns.

Thanks,
Janusz

P.S. A replacement for patches 4 and 5, addressing Russell's comment,
in progress.


 arch/arm/configs/omap1_defconfig |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/arch/arm/configs/omap1_defconfig b/arch/arm/configs/omap1_defconfig
index a7e7775..945a34f 100644
--- a/arch/arm/configs/omap1_defconfig
+++ b/arch/arm/configs/omap1_defconfig
@@ -48,12 +48,7 @@ CONFIG_MACH_SX1=y
 CONFIG_MACH_NOKIA770=y
 CONFIG_MACH_AMS_DELTA=y
 CONFIG_MACH_OMAP_GENERIC=y
-CONFIG_OMAP_ARM_216MHZ=y
-CONFIG_OMAP_ARM_195MHZ=y
-CONFIG_OMAP_ARM_192MHZ=y
 CONFIG_OMAP_ARM_182MHZ=y
-CONFIG_OMAP_ARM_168MHZ=y
-# CONFIG_OMAP_ARM_60MHZ is not set
 # CONFIG_ARM_THUMB is not set
 CONFIG_PCCARD=y
 CONFIG_OMAP_CF=y
-- 
1.7.3.4

Re: [PATCH 4/5] init/calibrate.c: allow for recalibration of loops per jiffy

[Janusz Krzysztofik]

On Monday 28 of November 2011 at 16:39:00, Russell King - ARM Linux wrote:
> On Sun, Nov 27, 2011 at 04:32:30AM +0100, Janusz Krzysztofik wrote:
> > This is required on ARM OMAP1 platform, after DPLL reprogramming has
> > been fixed by moving it from setup_arch() to kernel_init().
> > 
> > Created against linux-3.2-rc2.
> 
> Why do you need this?  If you know what the original CPU clock rate and
> bogomips were, and the new CPU clock rate, then you simply scale the
> bogomips from the original accordingly.  That's what cpufreq does.

Hi,
Please ignore this patch. I'm going to post a replacement for patch 5/5 
which, thanks to Russell's help, will no longer require any changes in 
init/calibrate.c.

Russell,
Thanks for pointing me into the right direction.

Janusz

[PATCH 5/5 v2] ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram

[Janusz Krzysztofik]

Otherwise timing is inaccurate, resulting in devices which depend on 
delay(), like omap-keypad, broken.

Created and tested on top of linux-omap/fixes on Amstrad Delta.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>
---
Hi,
This version no longer depends on changes to init/calibrate.c, using the
method kindly advised by Russell instead.  Then, patch 4/5 can be
dropped.

However, the result of cpufreq_scale() differs from that of
(re)calibrate_delay() by ca. 6%, i.e., 70.40 vs. 74.54. Please advise if
this approximation is acceptable.

Thanks, 
Janusz


 arch/arm/mach-omap1/clock_data.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap1/clock_data.c b/arch/arm/mach-omap1/clock_data.c
index 201b1f2..e616042 100644
--- a/arch/arm/mach-omap1/clock_data.c
+++ b/arch/arm/mach-omap1/clock_data.c
@@ -16,6 +16,8 @@
 
 #include <linux/kernel.h>
 #include <linux/clk.h>
+#include <linux/cpufreq.h>
+#include <linux/delay.h>
 #include <linux/io.h>
 
 #include <asm/mach-types.h>  /* for machine_is_* */
@@ -928,7 +930,9 @@ int __init omap1_clk_init(void)
 
 void __init omap1_clk_late_init(void)
 {
-	if (ck_dpll1.rate >= OMAP1_DPLL1_SANE_VALUE)
+	unsigned long rate = ck_dpll1.rate;
+
+	if (rate >= OMAP1_DPLL1_SANE_VALUE)
 		return;
 
 	/* Find the highest supported frequency and enable it */
@@ -942,4 +946,5 @@ void __init omap1_clk_late_init(void)
 	}
 	propagate_rate(&ck_dpll1);
 	omap1_show_rates();
+	loops_per_jiffy = cpufreq_scale(loops_per_jiffy, rate, ck_dpll1.rate);
 }
-- 
1.7.3.4

[PATCH 2a/5 v2] ARM: OMAP1: select clock rate by CPU type

[Janusz Krzysztofik]

DPLL1 reprogramming to a different rate is actually blocked inside
omap1_select_table_rate(), resulting in the defalut rate of 60 MHz
always used instead of the one selected in .config. OTOH, in
omap1_defconfig we currently rely on Kconfig options for the supported
MHz rates in case of boards which boot with dpll1 not set correctly by
their boot loaders.

This means that before we allow for reprogramming of dpll1 rate, we
should prevent from incompatible clock rates being selected, otherwise
omap1_defconfig will stop booting on boards with imperfect boot loaders,
as it would always try to change to 216MHz.

Expand omap1_rate_table with flags for different CPU types and match
them while selecting clock rates. The idea is stolen from current
omap24xx clock rate selection algorithm.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---
Hi,
Since there is no response to patch 2a/5 "Remove unsafe clock values 
from omap1_defconfig" yet, while rc4 is probably almost out:

If there are reasons for keeping high clock rates selected in
omap1_defconfig, and those reasons are more important than fixing bugs
which prevent some boards from working correctly under 3.2, here is an
alternative, low intrusive solution which should allow booting from
omap1_defconfig at correct speeds, selected by CPU type. If the idea is
acceptable, please review the CPU per clock rate selections I made,
since I'm not sure about their correctness, perhaps except CK_1510,
which is based on my own experience with Amstrad Delta. I'll fix that if
necessary.

Thanks,
Janusz


 arch/arm/mach-omap1/clock.c      |    6 +++++
 arch/arm/mach-omap1/clock_data.c |    4 ++-
 arch/arm/mach-omap1/opp.h        |    2 +
 arch/arm/mach-omap1/opp_data.c   |   43 +++++++++++++++++++++++++------------
 4 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-omap1/clock.c b/arch/arm/mach-omap1/clock.c
index 3899176..6d8f7c6 100644
--- a/arch/arm/mach-omap1/clock.c
+++ b/arch/arm/mach-omap1/clock.c
@@ -197,6 +197,9 @@ int omap1_select_table_rate(struct clk *clk, unsigned long rate)
 	ref_rate = ck_ref_p->rate;
 
 	for (ptr = omap1_rate_table; ptr->rate; ptr++) {
+		if (!(ptr->flags & cpu_mask))
+			continue;
+
 		if (ptr->xtal != ref_rate)
 			continue;
 
@@ -286,6 +289,9 @@ long omap1_round_to_table_rate(struct clk *clk, unsigned long rate)
 	highest_rate = -EINVAL;
 
 	for (ptr = omap1_rate_table; ptr->rate; ptr++) {
+		if (!(ptr->flags & cpu_mask))
+			continue;
+
 		if (ptr->xtal != ref_rate)
 			continue;
 
diff --git a/arch/arm/mach-omap1/clock_data.c b/arch/arm/mach-omap1/clock_data.c
index 201b1f2..398a7bb 100644
--- a/arch/arm/mach-omap1/clock_data.c
+++ b/arch/arm/mach-omap1/clock_data.c
@@ -777,12 +777,14 @@ static void __init omap1_show_rates(void)
 		arm_ck.rate / 1000000, (arm_ck.rate / 100000) % 10);
 }
 
+u32 cpu_mask;
+
 int __init omap1_clk_init(void)
 {
 	struct omap_clk *c;
 	const struct omap_clock_config *info;
 	int crystal_type = 0; /* Default 12 MHz */
-	u32 reg, cpu_mask;
+	u32 reg;
 
 #ifdef CONFIG_DEBUG_LL
 	/*
diff --git a/arch/arm/mach-omap1/opp.h b/arch/arm/mach-omap1/opp.h
index 07074d7..f5dcbf1 100644
--- a/arch/arm/mach-omap1/opp.h
+++ b/arch/arm/mach-omap1/opp.h
@@ -21,8 +21,10 @@ struct mpu_rate {
 	unsigned long		pll_rate;
 	__u16			ckctl_val;
 	__u16			dpllctl_val;
+	u32			flags;
 };
 
 extern struct mpu_rate omap1_rate_table[];
+extern u32 cpu_mask;
 
 #endif
diff --git a/arch/arm/mach-omap1/opp_data.c b/arch/arm/mach-omap1/opp_data.c
index 75a5465..d8d451c 100644
--- a/arch/arm/mach-omap1/opp_data.c
+++ b/arch/arm/mach-omap1/opp_data.c
@@ -10,6 +10,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <plat/clkdev_omap.h>
 #include "opp.h"
 
 /*-------------------------------------------------------------------------
@@ -21,38 +22,52 @@ struct mpu_rate omap1_rate_table[] = {
 	 * armdiv, dspdiv, dspmmu, tcdiv, perdiv, lcddiv
 	 */
 #if defined(CONFIG_OMAP_ARM_216MHZ)
-	{ 216000000, 12000000, 216000000, 0x050d, 0x2910 }, /* 1/1/2/2/2/8 */
+	{ 216000000, 12000000, 216000000, 0x050d, 0x2910, /* 1/1/2/2/2/8 */
+			CK_16XX },
 #endif
 #if defined(CONFIG_OMAP_ARM_195MHZ)
-	{ 195000000, 13000000, 195000000, 0x050e, 0x2790 }, /* 1/1/2/2/4/8 */
+	{ 195000000, 13000000, 195000000, 0x050e, 0x2790, /* 1/1/2/2/4/8 */
+			CK_7XX|CK_16XX },
 #endif
 #if defined(CONFIG_OMAP_ARM_192MHZ)
-	{ 192000000, 19200000, 192000000, 0x050f, 0x2510 }, /* 1/1/2/2/8/8 */
-	{ 192000000, 12000000, 192000000, 0x050f, 0x2810 }, /* 1/1/2/2/8/8 */
-	{  96000000, 12000000, 192000000, 0x055f, 0x2810 }, /* 2/2/2/2/8/8 */
-	{  48000000, 12000000, 192000000, 0x0baf, 0x2810 }, /* 4/4/4/8/8/8 */
-	{  24000000, 12000000, 192000000, 0x0fff, 0x2810 }, /* 8/8/8/8/8/8 */
+	{ 192000000, 19200000, 192000000, 0x050f, 0x2510, /* 1/1/2/2/8/8 */
+			CK_7XX|CK_16XX },
+	{ 192000000, 12000000, 192000000, 0x050f, 0x2810, /* 1/1/2/2/8/8 */
+			CK_7XX|CK_16XX },
+	{  96000000, 12000000, 192000000, 0x055f, 0x2810, /* 2/2/2/2/8/8 */
+			CK_7XX|CK_16XX },
+	{  48000000, 12000000, 192000000, 0x0baf, 0x2810, /* 4/4/4/8/8/8 */
+			CK_7XX|CK_16XX },
+	{  24000000, 12000000, 192000000, 0x0fff, 0x2810, /* 8/8/8/8/8/8 */
+			CK_7XX|CK_16XX },
 #endif
 #if defined(CONFIG_OMAP_ARM_182MHZ)
-	{ 182000000, 13000000, 182000000, 0x050e, 0x2710 }, /* 1/1/2/2/4/8 */
+	{ 182000000, 13000000, 182000000, 0x050e, 0x2710, /* 1/1/2/2/4/8 */
+			CK_7XX|CK_16XX },
 #endif
 #if defined(CONFIG_OMAP_ARM_168MHZ)
-	{ 168000000, 12000000, 168000000, 0x010f, 0x2710 }, /* 1/1/1/2/8/8 */
+	{ 168000000, 12000000, 168000000, 0x010f, 0x2710, /* 1/1/1/2/8/8 */
+			CK_7XX|CK_16XX },
 #endif
 #if defined(CONFIG_OMAP_ARM_150MHZ)
-	{ 150000000, 12000000, 150000000, 0x010a, 0x2cb0 }, /* 1/1/1/2/4/4 */
+	{ 150000000, 12000000, 150000000, 0x010a, 0x2cb0, /* 1/1/1/2/4/4 */
+			CK_1510|CK_7XX|CK_16XX },
 #endif
 #if defined(CONFIG_OMAP_ARM_120MHZ)
-	{ 120000000, 12000000, 120000000, 0x010a, 0x2510 }, /* 1/1/1/2/4/4 */
+	{ 120000000, 12000000, 120000000, 0x010a, 0x2510, /* 1/1/1/2/4/4 */
+			CK_310|CK_1510|CK_7XX|CK_16XX },
 #endif
 #if defined(CONFIG_OMAP_ARM_96MHZ)
-	{  96000000, 12000000,  96000000, 0x0005, 0x2410 }, /* 1/1/1/1/2/2 */
+	{  96000000, 12000000,  96000000, 0x0005, 0x2410, /* 1/1/1/1/2/2 */
+			CK_310|CK_1510|CK_7XX|CK_16XX },
 #endif
 #if defined(CONFIG_OMAP_ARM_60MHZ)
-	{  60000000, 12000000,  60000000, 0x0005, 0x2290 }, /* 1/1/1/1/2/2 */
+	{  60000000, 12000000,  60000000, 0x0005, 0x2290, /* 1/1/1/1/2/2 */
+			CK_310|CK_1510|CK_7XX|CK_16XX },
 #endif
 #if defined(CONFIG_OMAP_ARM_30MHZ)
-	{  30000000, 12000000,  60000000, 0x0555, 0x2290 }, /* 2/2/2/2/2/2 */
+	{  30000000, 12000000,  60000000, 0x0555, 0x2290, /* 2/2/2/2/2/2 */
+			CK_310|CK_1510|CK_7XX|CK_16XX },
 #endif
 	{ 0, 0, 0, 0, 0 },
 };
-- 
1.7.3.4

Re: [PATCH 2a/5 v2] ARM: OMAP1: select clock rate by CPU type

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111130 12:23]:
> DPLL1 reprogramming to a different rate is actually blocked inside
> omap1_select_table_rate(), resulting in the defalut rate of 60 MHz
> always used instead of the one selected in .config. OTOH, in
> omap1_defconfig we currently rely on Kconfig options for the supported
> MHz rates in case of boards which boot with dpll1 not set correctly by
> their boot loaders.
> 
> This means that before we allow for reprogramming of dpll1 rate, we
> should prevent from incompatible clock rates being selected, otherwise
> omap1_defconfig will stop booting on boards with imperfect boot loaders,
> as it would always try to change to 216MHz.
> 
> Expand omap1_rate_table with flags for different CPU types and match
> them while selecting clock rates. The idea is stolen from current
> omap24xx clock rate selection algorithm.
> 
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> ---
> Hi,
> Since there is no response to patch 2a/5 "Remove unsafe clock values 
> from omap1_defconfig" yet, while rc4 is probably almost out:

Well I'm still wondering what would be the absolute minimal fixe(s)
for the -rc cycle?

Is the problem just that currently with omap1_defconfig boards boot at
the bootloader rate, or at the safe 60MHz unless the .config is changed?
If so, I think that's OK for now as fixing it properly requires what
this patch is doing.
 
> If there are reasons for keeping high clock rates selected in
> omap1_defconfig, and those reasons are more important than fixing bugs
> which prevent some boards from working correctly under 3.2, here is an
> alternative, low intrusive solution which should allow booting from
> omap1_defconfig at correct speeds, selected by CPU type. If the idea is
> acceptable, please review the CPU per clock rate selections I made,
> since I'm not sure about their correctness, perhaps except CK_1510,
> which is based on my own experience with Amstrad Delta. I'll fix that if
> necessary.

Great, this looks like pretty much what I had in mind for the next
merge window! Just one comment below.

 
> @@ -21,38 +22,52 @@ struct mpu_rate omap1_rate_table[] = {
>  	 * armdiv, dspdiv, dspmmu, tcdiv, perdiv, lcddiv
>  	 */
>  #if defined(CONFIG_OMAP_ARM_216MHZ)
> -	{ 216000000, 12000000, 216000000, 0x050d, 0x2910 }, /* 1/1/2/2/2/8 */
> +	{ 216000000, 12000000, 216000000, 0x050d, 0x2910, /* 1/1/2/2/2/8 */
> +			CK_16XX },
>  #endif
>  #if defined(CONFIG_OMAP_ARM_195MHZ)
> -	{ 195000000, 13000000, 195000000, 0x050e, 0x2790 }, /* 1/1/2/2/4/8 */
> +	{ 195000000, 13000000, 195000000, 0x050e, 0x2790, /* 1/1/2/2/4/8 */
> +			CK_7XX|CK_16XX },
>  #endif
>  #if defined(CONFIG_OMAP_ARM_192MHZ)
...

We should also now be able to remove all the CONFIG_OMAP_ARM_XXXMHZ options
too, right?

Regards,

Tony

Re: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111128 13:26]:
>
> I'm not sure which of the patches you meant not ready for 3.2-rc. From
> your comment I would conclude that only patch 3/5, which would really
> break omap1_defconfig booting on some boards, is questionable, while you
> posted this comment against patch 1/5, which I don't see how it could
> break booting, including omap1_defconfig.

Sorry for the delay in replying, yes you're right looks like it's allowing
the rate changes currently that would break things in 3/5.
 
> Anyway, I hope that the additional patch below is addressing all your
> concerns.

I'd rather not modify omap1_defconfig as a fix assuming that we'll be
removing all those options anyways the next merge window together with
your new patch.
 
Can you please split your series into two: Fix(es) for the -rc cycle,
then patches that can be left for the next merge window.

Cheers,

Tony

Re: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig

[Janusz Krzysztofik]

I've unintentionally answered off-line, sorry, re-adding all Cc:'s.

On Thursday 01 of December 2011 at 03:27:51, Tony Lindgren wrote:
> * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111130 17:40]:
> > On Wednesday 30 of November 2011 at 23:32:42, Tony Lindgren wrote:
> > >  
> > > Can you please split your series into two: Fix(es) for the -rc cycle,
> > > then patches that can be left for the next merge window.
> > 
> > From my point of view, all 5 are important fixes. Please decide 
> > yourself, having the following information provided:
> > 
> > 1/5: inspired by in-line comments about running from sram requirement
> > 	(works without this one for me),
> > 
> > 2/5: without this one, system clock runs way too fast if dpll1 is 
> > 	reprogrammed to default rate,
> > 
> > 3/5: without this one, all boards with bootloaders not setting rate 
> > 	correctly, like Amstrad Delta, will run at default rate, despite 
> > 	any .config selections, no matter if omap1_defconfig or custom,
> > 
> > 2a/5: required by 3/5,
> > 
> > 5/5: without this one, BogoMIPS is not updated after dpll1 reprogramme, 
> > 	breaking omap_keypad at least.
> > 
> > and please let me know which I should resend as fixes and which not.
> 
> How about 2 and 5 as fixes during the -rc, then the rest for the
> merge window? That is assuming that those are enough for you to have
> things mostly working.

If you still ask me for my opinion: with patch 3/5 omitted, then not 
being able to run at any other frequency than 60 MHz instead of usual 
150 since the board support was introduced first, isn't this a 
regression? Having a choice of upgrading to 3.2 and running my 
application on not very powerfull board at 60 MHz, or keep running 3.1 
at 150, guess what I chose? If I were a distro kernel package 
maintainer, guess what I would chose?

> It seems that we've had the issue of not actually changing the rate
> for a while, right?

This was not an issue before dpll1 reprogramming has been moved out from 
omap1_clk_init(), as an rc fix to another bug introduced in 3.2. Perhaps 
we should rather think of reverting a few commits which caused all these 
problems if fixing them all during rc cycle seems not possible? I 
haven't bisected them yet, rather concentrated on providing fixes, but I 
can still try to do it, starting back from the original issue 
(http://www.spinics.net/lists/linux-omap/msg60052.html), if so decided.

> If that's the case, I'd rather not start messing
> with that during the -rc cycle.
> 
> Regards,
> 
> Tony

I don't feel like a person who makes the final decision.

Anyway, did you mean resending those 2/5 and 5/5 without any changes, 
only renumbered as 1/2 and 2/2?

Thanks,
Janusz

Re: [PATCH 2a/5 v2] ARM: OMAP1: select clock rate by CPU type

[Janusz Krzysztofik]

On Wednesday 30 of November 2011 at 23:28:38, Tony Lindgren wrote:
> 
> We should also now be able to remove all the CONFIG_OMAP_ARM_XXXMHZ options
> too, right?

Right, but then, perhaps the initial version of patch 2a/5, which 
already started removing them, from omap1_defconfig for now, then going 
into the right direction while unblocking another regression fix (3/5), 
_is_ a good candidate for an rc fix?

Thanks,
Janusz

Re: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig

[Janusz Krzysztofik]

On Thursday 01 of December 2011 at 10:54:09, Janusz Krzysztofik wrote:
> 
> Anyway, did you mean resending those 2/5 and 5/5 without any changes, 
> only renumbered as 1/2 and 2/2?

This was not a very clever question, sorry. Answering myself: 2/5 must 
be refreshed if not on top of 1/5.

Thanks,
Janusz

Re: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111201 01:20]:
> 
> If you still ask me for my opinion: with patch 3/5 omitted, then not 
> being able to run at any other frequency than 60 MHz instead of usual 
> 150 since the board support was introduced first, isn't this a 
> regression?

Yes, assuming that the behaviour for your board has changed by something
after v3.1, such as commit e9b7086b80c4d9e354f4edc9e280ae85a60df408.

> Having a choice of upgrading to 3.2 and running my 
> application on not very powerfull board at 60 MHz, or keep running 3.1 
> at 150, guess what I chose? If I were a distro kernel package 
> maintainer, guess what I would chose?

Right, that's certainly not intentional :)

> > It seems that we've had the issue of not actually changing the rate
> > for a while, right?
> 
> This was not an issue before dpll1 reprogramming has been moved out from 
> omap1_clk_init(), as an rc fix to another bug introduced in 3.2. Perhaps 
> we should rather think of reverting a few commits which caused all these 
> problems if fixing them all during rc cycle seems not possible? I 
> haven't bisected them yet, rather concentrated on providing fixes, but I 
> can still try to do it, starting back from the original issue 
> (http://www.spinics.net/lists/linux-omap/msg60052.html), if so decided.

We can't revert that because the SRAM init has been moved to later for
map_io. But if that patch changed the behaviour on your board, then that's
the problem we should fix.

If you're now stuck at 60MHz rate, care to see if the following patch
makes the kernel behave the same way as before for you? Sorry for dragging
this on, but I'd like to find out what exactly changed the behaviour for
your board.
 
> Anyway, did you mean resending those 2/5 and 5/5 without any changes, 
> only renumbered as 1/2 and 2/2?

Well the numbers will not show up in git, so no changes needed there :)

But let's figure out first what changed the behaviour for your board.

Regards,

Tony

--- a/arch/arm/mach-omap1/clock_data.c
+++ b/arch/arm/mach-omap1/clock_data.c
@@ -927,7 +927,7 @@ int __init omap1_clk_init(void)
 
 void __init omap1_clk_late_init(void)
 {
-	if (ck_dpll1.rate >= OMAP1_DPLL1_SANE_VALUE)
+	if (ck_dpll1.rate > OMAP1_DPLL1_SANE_VALUE)
 		return;
 
 	/* Find the highest supported frequency and enable it */

Re: [PATCH 2a/5 v2] ARM: OMAP1: select clock rate by CPU type

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111201 01:35]:
> On Wednesday 30 of November 2011 at 23:28:38, Tony Lindgren wrote:
> > 
> > We should also now be able to remove all the CONFIG_OMAP_ARM_XXXMHZ options
> > too, right?
> 
> Right, but then, perhaps the initial version of patch 2a/5, which 
> already started removing them, from omap1_defconfig for now, then going 
> into the right direction while unblocking another regression fix (3/5), 
> _is_ a good candidate for an rc fix?

But we did not allow dpll1 reprogramming earlier either, so we should
not need to make all these changes during the -rc cycle. I'm suspecting
that we've had this same behaviour for a really long time, and we just
have not seen it as omap1_defconfig had OMAP_CLOCKS_SET_BY_BOOTLOADER
option set.

So I'm baffled how your board would be booting at a different rate
compared to v3.1, it seems that the logic has not changed there. Or
else we have some simple bug somewhere.

Care to try to verify at what point your system started booting at
60MHz rate?

Regards,

Tony

Re: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig

[Janusz Krzysztofik]

On Thursday 01 of December 2011 at 18:17:58, Tony Lindgren wrote:
> * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111201 01:20]:
[snip]
> > Perhaps 
> > we should rather think of reverting a few commits which caused all these 
> > problems if fixing them all during rc cycle seems not possible? I 
> > haven't bisected them yet, rather concentrated on providing fixes, but I 
> > can still try to do it, starting back from the original issue 
> > (http://www.spinics.net/lists/linux-omap/msg60052.html), if so decided.
> 
> We can't revert that because the SRAM init has been moved to later for
> map_io. 

Yes, I know that. What I don't know yet is what else should be reverted 
to fix the original issue other than applying  
e9b7086b80c4d9e354f4edc9e280ae85a60df408, which seems to introduce (or 
maybe 'exhibit' is a better word here) more bugs than it fixes.

> But if that patch changed the behaviour on your board, then that's
> the problem we should fix.
> 
> If you're now stuck at 60MHz rate, care to see if the following patch
> makes the kernel behave the same way as before for you?
[snip]
> 
> --- a/arch/arm/mach-omap1/clock_data.c
> +++ b/arch/arm/mach-omap1/clock_data.c
> @@ -927,7 +927,7 @@ int __init omap1_clk_init(void)
>  
>  void __init omap1_clk_late_init(void)
>  {
> -	if (ck_dpll1.rate >= OMAP1_DPLL1_SANE_VALUE)
> +	if (ck_dpll1.rate > OMAP1_DPLL1_SANE_VALUE)
>  		return;
>  
>  	/* Find the highest supported frequency and enable it */

This change really makes sense to me, however, knowing the initial 
(bootloader selected) clock rate my board boots at, which is 
unfortunately raw 12 MHz, I would be surprised if that helped.

Before e9b7086b80c4d9e354f4edc9e280ae85a60df408, 
omap1_select_table_rate() was returning the rate selected with .config 
because it was called early, with ck_dpll1_p->rate uninitialized. Now it 
is not, and returns nothing, resulting in 60 MHz default. Then, the only 
way I can see to correct the problem is something like patch 3/5, which 
you are justifiably affraid of of always switching to 216 MHz with 
omap1_defconfig.

Thanks,
Janusz

Re: [PATCH 2a/5 v2] ARM: OMAP1: select clock rate by CPU type

[Janusz Krzysztofik]

On Thursday 01 of December 2011 at 19:22:54, Tony Lindgren wrote:
> * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111201 01:35]:
> > On Wednesday 30 of November 2011 at 23:28:38, Tony Lindgren wrote:
> > > 
> > > We should also now be able to remove all the CONFIG_OMAP_ARM_XXXMHZ options
> > > too, right?
> > 
> > Right, but then, perhaps the initial version of patch 2a/5, which 
> > already started removing them, from omap1_defconfig for now, then going 
> > into the right direction while unblocking another regression fix (3/5), 
> > _is_ a good candidate for an rc fix?
> 
> But we did not allow dpll1 reprogramming earlier either,

Wrong. Without OMAP_CLOCKS_SET_BY_BOOTLOADER selected, we always did, 
but only once, early at boot, before ck_dpll1_p->rate was set first from 
omap1_clk_init(), and never retried later, that's why that check which I 
removed with 3/5 was never in the game until e9b7086b80c4d9e354f4edc9e280ae85a60df408.

> so we should
> not need to make all these changes during the -rc cycle. I'm suspecting
> that we've had this same behaviour for a really long time, and we just
> have not seen it as omap1_defconfig had OMAP_CLOCKS_SET_BY_BOOTLOADER
> option set.
> 
> So I'm baffled how your board would be booting at a different rate
> compared to v3.1, it seems that the logic has not changed there. Or
> else we have some simple bug somewhere.
> 
> Care to try to verify at what point your system started booting at
> 60MHz rate?

Since e9b7086b80c4d9e354f4edc9e280ae85a60df408, I guess, and it's hard 
to confirm wituout bisecting the issue with too early sram call, back 
until things still worked like before map_io related changes. I will do 
that if you decide we should try to revert.

Thanks,
Janusz

Re: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111201 10:04]:
> On Thursday 01 of December 2011 at 18:17:58, Tony Lindgren wrote:
> > 
> > --- a/arch/arm/mach-omap1/clock_data.c
> > +++ b/arch/arm/mach-omap1/clock_data.c
> > @@ -927,7 +927,7 @@ int __init omap1_clk_init(void)
> >  
> >  void __init omap1_clk_late_init(void)
> >  {
> > -	if (ck_dpll1.rate >= OMAP1_DPLL1_SANE_VALUE)
> > +	if (ck_dpll1.rate > OMAP1_DPLL1_SANE_VALUE)
> >  		return;
> >  
> >  	/* Find the highest supported frequency and enable it */
> 
> This change really makes sense to me, however, knowing the initial 
> (bootloader selected) clock rate my board boots at, which is 
> unfortunately raw 12 MHz, I would be surprised if that helped.

OK, that's not it then. BTW, we should be able to drop that test once
we have your patches applied.
 
> Before e9b7086b80c4d9e354f4edc9e280ae85a60df408, 
> omap1_select_table_rate() was returning the rate selected with .config 
> because it was called early, with ck_dpll1_p->rate uninitialized. Now it 
> is not, and returns nothing, resulting in 60 MHz default. Then, the only 
> way I can see to correct the problem is something like patch 3/5, which 
> you are justifiably affraid of of always switching to 216 MHz with 
> omap1_defconfig.

Ah, you got it! That's what causes the regression. How about the following
fix for the -rc cycle then?

Tony


From: Tony Lindgren <tony@atomide.com>
Date: Thu, 1 Dec 2011 11:00:11 -0800
Subject: [PATCH] ARM: OMAP1: Fix reprogramming of DPLL1 for systems that boot at rates below 60MHz

Commit e9b7086b80c4d9e354f4edc9e280ae85a60df408 (ARM: OMAP: Fix
reprogramming of dpll1 rate) fixed a regression for systems that
did not rely on bootloader set rates.

However, it also introduced a new problem where the rates selected
in .config would not take affect as omap1_select_table_rate
currently refuses to reprogram DPLL1 if it's already initialized.

This was not a problem earlier, as the reprogramming was done
earlier with ck_dpll1_p->rate uninitialized.

Fix this by forcing the reprogramming on systems booting at rates
below 60MHz. Note that the long term fix is to make the rates
SoC specific later on.

Thanks for Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> for figuring
this one out.

Reported-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
Signed-off-by: Tony Lindgren <tony@atomide.com>

diff --git a/arch/arm/mach-omap1/clock_data.c b/arch/arm/mach-omap1/clock_data.c
index 1297bb5..3f30561 100644
--- a/arch/arm/mach-omap1/clock_data.c
+++ b/arch/arm/mach-omap1/clock_data.c
@@ -930,6 +930,9 @@ void __init omap1_clk_late_init(void)
 	if (ck_dpll1.rate >= OMAP1_DPLL1_SANE_VALUE)
 		return;
 
+	/* System booting at unusable rate, force reprogramming of DPLL1 */
+	ck_dpll1_p->rate = 0;
+
 	/* Find the highest supported frequency and enable it */
 	if (omap1_select_table_rate(&virtual_ck_mpu, ~0)) {
 		pr_err("System frequencies not set, using default. Check your config.\n");

Re: [PATCH 2a/5 v2] ARM: OMAP1: select clock rate by CPU type

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111201 10:19]:
> On Thursday 01 of December 2011 at 19:22:54, Tony Lindgren wrote:
> > * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111201 01:35]:
> > > On Wednesday 30 of November 2011 at 23:28:38, Tony Lindgren wrote:
> > > > 
> > > > We should also now be able to remove all the CONFIG_OMAP_ARM_XXXMHZ options
> > > > too, right?
> > > 
> > > Right, but then, perhaps the initial version of patch 2a/5, which 
> > > already started removing them, from omap1_defconfig for now, then going 
> > > into the right direction while unblocking another regression fix (3/5), 
> > > _is_ a good candidate for an rc fix?
> > 
> > But we did not allow dpll1 reprogramming earlier either,
> 
> Wrong. Without OMAP_CLOCKS_SET_BY_BOOTLOADER selected, we always did, 
> but only once, early at boot, before ck_dpll1_p->rate was set first from 
> omap1_clk_init(), and never retried later, that's why that check which I 
> removed with 3/5 was never in the game until e9b7086b80c4d9e354f4edc9e280ae85a60df408.

Yeah you're right. You found what caused the regression :)
 
> > so we should
> > not need to make all these changes during the -rc cycle. I'm suspecting
> > that we've had this same behaviour for a really long time, and we just
> > have not seen it as omap1_defconfig had OMAP_CLOCKS_SET_BY_BOOTLOADER
> > option set.
> > 
> > So I'm baffled how your board would be booting at a different rate
> > compared to v3.1, it seems that the logic has not changed there. Or
> > else we have some simple bug somewhere.
> > 
> > Care to try to verify at what point your system started booting at
> > 60MHz rate?
> 
> Since e9b7086b80c4d9e354f4edc9e280ae85a60df408, I guess, and it's hard 
> to confirm wituout bisecting the issue with too early sram call, back 
> until things still worked like before map_io related changes. I will do 
> that if you decide we should try to revert.

No need to bisect, I think we can just reset ck_dpll1_p->rate for
systems booting at below 60MHz rate to force the reprogramming.

Regards,

Tony

Re: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig

[Janusz Krzysztofik]

On Thursday 01 of December 2011 at 20:04:55, Tony Lindgren wrote:
> 
> From: Tony Lindgren <tony@atomide.com>
> Date: Thu, 1 Dec 2011 11:00:11 -0800
> Subject: [PATCH] ARM: OMAP1: Fix reprogramming of DPLL1 for systems that boot at rates below 60MHz
> 
> Commit e9b7086b80c4d9e354f4edc9e280ae85a60df408 (ARM: OMAP: Fix
> reprogramming of dpll1 rate) fixed a regression for systems that
> did not rely on bootloader set rates.
> 
> However, it also introduced a new problem where the rates selected
> in .config would not take affect as omap1_select_table_rate
> currently refuses to reprogram DPLL1 if it's already initialized.
> 
> This was not a problem earlier, as the reprogramming was done
> earlier with ck_dpll1_p->rate uninitialized.
> 
> Fix this by forcing the reprogramming on systems booting at rates
> below 60MHz. Note that the long term fix is to make the rates
> SoC specific later on.
> 
> Thanks for Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> for figuring
> this one out.
> 
> Reported-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> Signed-off-by: Tony Lindgren <tony@atomide.com>

Acked-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

However, this way or another, we are back to your mentioned problem of 
omap1_defconfig always switching to 216 MHz, I'm afraid. Then, 2a/5 v1 
"Remove unsafe clock values from omap1_defconfig" can still be helpful.

Anyway, I'm resending (refreshed) 2/5 and 5/5 as rc fixes as you 
suggested before, 1/5 "ARM: OMAP1: Fix dpll1 default rate reprogramming 
method" intended for next, and 2a/5 v2 "ARM: OMAP1: select clock rate by 
CPU type" also for next but as an RFC. 

Thanks,
Janusz

> 
> diff --git a/arch/arm/mach-omap1/clock_data.c b/arch/arm/mach-omap1/clock_data.c
> index 1297bb5..3f30561 100644
> --- a/arch/arm/mach-omap1/clock_data.c
> +++ b/arch/arm/mach-omap1/clock_data.c
> @@ -930,6 +930,9 @@ void __init omap1_clk_late_init(void)
>  	if (ck_dpll1.rate >= OMAP1_DPLL1_SANE_VALUE)
>  		return;
>  
> +	/* System booting at unusable rate, force reprogramming of DPLL1 */
> +	ck_dpll1_p->rate = 0;
> +
>  	/* Find the highest supported frequency and enable it */
>  	if (omap1_select_table_rate(&virtual_ck_mpu, ~0)) {
>  		pr_err("System frequencies not set, using default. Check your config.\n");
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111201 10:48]:
> On Thursday 01 of December 2011 at 20:04:55, Tony Lindgren wrote:
> > 
> > From: Tony Lindgren <tony@atomide.com>
> > Date: Thu, 1 Dec 2011 11:00:11 -0800
> > Subject: [PATCH] ARM: OMAP1: Fix reprogramming of DPLL1 for systems that boot at rates below 60MHz
> > 
> > Commit e9b7086b80c4d9e354f4edc9e280ae85a60df408 (ARM: OMAP: Fix
> > reprogramming of dpll1 rate) fixed a regression for systems that
> > did not rely on bootloader set rates.
> > 
> > However, it also introduced a new problem where the rates selected
> > in .config would not take affect as omap1_select_table_rate
> > currently refuses to reprogram DPLL1 if it's already initialized.
> > 
> > This was not a problem earlier, as the reprogramming was done
> > earlier with ck_dpll1_p->rate uninitialized.
> > 
> > Fix this by forcing the reprogramming on systems booting at rates
> > below 60MHz. Note that the long term fix is to make the rates
> > SoC specific later on.
> > 
> > Thanks for Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> for figuring
> > this one out.
> > 
> > Reported-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
> Acked-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> 
> However, this way or another, we are back to your mentioned problem of 
> omap1_defconfig always switching to 216 MHz, I'm afraid. Then, 2a/5 v1 
> "Remove unsafe clock values from omap1_defconfig" can still be helpful.

OK that makes sense now also in case there are other systems that
boot at rates below 60MHz.
 
> Anyway, I'm resending (refreshed) 2/5 and 5/5 as rc fixes as you 
> suggested before, 1/5 "ARM: OMAP1: Fix dpll1 default rate reprogramming 
> method" intended for next, and 2a/5 v2 "ARM: OMAP1: select clock rate by 
> CPU type" also for next but as an RFC. 

Great, sounds like we got the fixes needed for the -rc cycle then.

Regards,

Tony

[PATCH 0/2 v2] ARM: OMAP1: Fix dpll1 reprogramming related issues

[Janusz Krzysztofik]

After dpll1 reprogramming has been moved from setup_arch() to
kernel_init(), I've been observing several issues, resulting in
undesired system behaviour on my Amstrad Delta. This series fixes
those most important (2/5 and 5/5 v2 previously), with a "stuck at 60MHz
rate" issue going to be fixed with Tony's proposed solution instead of
my previous 3/5.

Janusz Krzysztofik (2):
  ARM: OMAP1: Fix ckctl value used for dpll1 defualt rate
  ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram

 arch/arm/mach-omap1/clock_data.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

-- 
1.7.3.4

[PATCH 1/2 v2] ARM: OMAP1: Fix ckctl value used for dpll1 defualt rate

[Janusz Krzysztofik]

Use the exact value found in omap1_rate_table, otherwise I have been
experiencing issues with correct timekeeping on my Amstrad Delta.

Created and tested on top of linux-omap/fixes, not yet in 3.2-rc.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---
v2 changes:
* previous 2/5, refreshed directly on linux-omap/fixes after 1/5
  left for the next merge window.

 arch/arm/mach-omap1/clock_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap1/clock_data.c b/arch/arm/mach-omap1/clock_data.c
index 1297bb5..b001299 100644
--- a/arch/arm/mach-omap1/clock_data.c
+++ b/arch/arm/mach-omap1/clock_data.c
@@ -934,7 +934,7 @@ void __init omap1_clk_late_init(void)
 	if (omap1_select_table_rate(&virtual_ck_mpu, ~0)) {
 		pr_err("System frequencies not set, using default. Check your config.\n");
 		omap_writew(0x2290, DPLL_CTL);
-		omap_writew(cpu_is_omap7xx() ? 0x3005 : 0x1005, ARM_CKCTL);
+		omap_writew(cpu_is_omap7xx() ? 0x2005 : 0x0005, ARM_CKCTL);
 		ck_dpll1.rate = OMAP1_DPLL1_SANE_VALUE;
 	}
 	propagate_rate(&ck_dpll1);
-- 
1.7.3.4

[PATCH 2/2 v2] ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram

[Janusz Krzysztofik]

Otherwise timing is inaccurate, resulting in devices which depend on it,
like omap-keypad, broken.

Created and tested on top of linux-omap/fixes on Amstrad Delta.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---
v2 changes:
* was 5/5 v2, rebased on 1/2.

 arch/arm/mach-omap1/clock_data.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap1/clock_data.c b/arch/arm/mach-omap1/clock_data.c
index b001299..0ef1e7d 100644
--- a/arch/arm/mach-omap1/clock_data.c
+++ b/arch/arm/mach-omap1/clock_data.c
@@ -16,6 +16,8 @@
 
 #include <linux/kernel.h>
 #include <linux/clk.h>
+#include <linux/cpufreq.h>
+#include <linux/delay.h>
 #include <linux/io.h>
 
 #include <asm/mach-types.h>  /* for machine_is_* */
@@ -927,7 +929,9 @@ int __init omap1_clk_init(void)
 
 void __init omap1_clk_late_init(void)
 {
-	if (ck_dpll1.rate >= OMAP1_DPLL1_SANE_VALUE)
+	unsigned long rate = ck_dpll1.rate;
+
+	if (rate >= OMAP1_DPLL1_SANE_VALUE)
 		return;
 
 	/* Find the highest supported frequency and enable it */
@@ -939,4 +943,5 @@ void __init omap1_clk_late_init(void)
 	}
 	propagate_rate(&ck_dpll1);
 	omap1_show_rates();
+	loops_per_jiffy = cpufreq_scale(loops_per_jiffy, rate, ck_dpll1.rate);
 }
-- 
1.7.3.4

[PATCH] ARM: OMAP1: Remove unsafe clock values from omap1_defconfig

[Janusz Krzysztofik]

DPLL1 reprogramming to a different rate is actually blocked inside
omap1_select_table_rate(), resulting in the defalut rate of 60 MHz
always used instead of the one selected in .config. OTOH, in
omap1_defconfig we currently rely on Kconfig options for the supported
MHz rates in case of boards which boot with dpll1 not set correctly by
their boot loaders.

This means that before we allow for reprogramming of dpll1 rate, we
should remove all unsafe clock selections from omap1_defconfig,
otherwise it will stop booting on boards with imperfect boot loaders,
as it would always try to change to 216MHz.

Keep only one safe clock rate per each supported xtal frequency, i.e.
60MHZ dpll1 for 12MHz xtal and 182MHz dpll1 for 13MHz xtal.

BTW, this change goes into the direction of removing all OMAP1 clock
rate options, planned for next merge window.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---
Resending with commit summary tagged correctly and commit message
updated with info about next merge window planned direction.

 arch/arm/configs/omap1_defconfig |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/arch/arm/configs/omap1_defconfig b/arch/arm/configs/omap1_defconfig
index a7e7775..945a34f 100644
--- a/arch/arm/configs/omap1_defconfig
+++ b/arch/arm/configs/omap1_defconfig
@@ -48,12 +48,7 @@ CONFIG_MACH_SX1=y
 CONFIG_MACH_NOKIA770=y
 CONFIG_MACH_AMS_DELTA=y
 CONFIG_MACH_OMAP_GENERIC=y
-CONFIG_OMAP_ARM_216MHZ=y
-CONFIG_OMAP_ARM_195MHZ=y
-CONFIG_OMAP_ARM_192MHZ=y
 CONFIG_OMAP_ARM_182MHZ=y
-CONFIG_OMAP_ARM_168MHZ=y
-# CONFIG_OMAP_ARM_60MHZ is not set
 # CONFIG_ARM_THUMB is not set
 CONFIG_PCCARD=y
 CONFIG_OMAP_CF=y
-- 
1.7.3.4

[PATCH v2] ARM: OMAP1: Update dpll1 default rate reprogramming method

[Janusz Krzysztofik]

According to comments in omap1_select_table_rate(), reprogramming dpll1
is tricky, and should always be done from SRAM.

While being at it, move OMAP730 special case handling inside
omap_sram_reprogram_clock().

Created on top of version 2 of the series "ARM: OMAP1: Fix dpll1
reprogramming related issues", which it depends on.
Tested on Amstrad Delta.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---
v2 changes:
* was 1/5, now separated with the intention of being deferred until next
  merge window, and refreshed on top of that series,
* Fix replaced with Update in the summary.


 arch/arm/mach-omap1/clock.c      |    6 +-----
 arch/arm/mach-omap1/clock_data.c |    7 +++++--
 arch/arm/plat-omap/sram.c        |    3 +++
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap1/clock.c b/arch/arm/mach-omap1/clock.c
index 84ef704..3899176 100644
--- a/arch/arm/mach-omap1/clock.c
+++ b/arch/arm/mach-omap1/clock.c
@@ -215,12 +215,8 @@ int omap1_select_table_rate(struct clk *clk, unsigned long rate)
 	/*
 	 * In most cases we should not need to reprogram DPLL.
 	 * Reprogramming the DPLL is tricky, it must be done from SRAM.
-	 * (on 730, bit 13 must always be 1)
 	 */
-	if (cpu_is_omap7xx())
-		omap_sram_reprogram_clock(ptr->dpllctl_val, ptr->ckctl_val | 0x2000);
-	else
-		omap_sram_reprogram_clock(ptr->dpllctl_val, ptr->ckctl_val);
+	omap_sram_reprogram_clock(ptr->dpllctl_val, ptr->ckctl_val);
 
 	/* XXX Do we need to recalculate the tree below DPLL1 at this point? */
 	ck_dpll1_p->rate = ptr->pll_rate;
diff --git a/arch/arm/mach-omap1/clock_data.c b/arch/arm/mach-omap1/clock_data.c
index 0ef1e7d..e616042 100644
--- a/arch/arm/mach-omap1/clock_data.c
+++ b/arch/arm/mach-omap1/clock_data.c
@@ -25,6 +25,7 @@
 #include <plat/clock.h>
 #include <plat/cpu.h>
 #include <plat/clkdev_omap.h>
+#include <plat/sram.h>	/* for omap_sram_reprogram_clock() */
 #include <plat/usb.h>   /* for OTG_BASE */
 
 #include "clock.h"
@@ -937,8 +938,10 @@ void __init omap1_clk_late_init(void)
 	/* Find the highest supported frequency and enable it */
 	if (omap1_select_table_rate(&virtual_ck_mpu, ~0)) {
 		pr_err("System frequencies not set, using default. Check your config.\n");
-		omap_writew(0x2290, DPLL_CTL);
-		omap_writew(cpu_is_omap7xx() ? 0x2005 : 0x0005, ARM_CKCTL);
+		/*
+		 * Reprogramming the DPLL is tricky, it must be done from SRAM.
+		 */
+		omap_sram_reprogram_clock(0x2290, 0x0005);
 		ck_dpll1.rate = OMAP1_DPLL1_SANE_VALUE;
 	}
 	propagate_rate(&ck_dpll1);
diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
index 8b28664..312bee8 100644
--- a/arch/arm/plat-omap/sram.c
+++ b/arch/arm/plat-omap/sram.c
@@ -224,6 +224,9 @@ static void (*_omap_sram_reprogram_clock)(u32 dpllctl, u32 ckctl);
 void omap_sram_reprogram_clock(u32 dpllctl, u32 ckctl)
 {
 	BUG_ON(!_omap_sram_reprogram_clock);
+	/* On 730, bit 13 must always be 1 */
+	if (cpu_is_omap7xx())
+		ckctl |= 0x2000;
 	_omap_sram_reprogram_clock(dpllctl, ckctl);
 }
 
-- 
1.7.3.4

Re: [PATCH 0/2 v2] ARM: OMAP1: Fix dpll1 reprogramming related issues

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111201 11:38]:
> After dpll1 reprogramming has been moved from setup_arch() to
> kernel_init(), I've been observing several issues, resulting in
> undesired system behaviour on my Amstrad Delta. This series fixes
> those most important (2/5 and 5/5 v2 previously), with a "stuck at 60MHz
> rate" issue going to be fixed with Tony's proposed solution instead of
> my previous 3/5.
> 
> Janusz Krzysztofik (2):
>   ARM: OMAP1: Fix ckctl value used for dpll1 defualt rate
>   ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram
> 
>  arch/arm/mach-omap1/clock_data.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)

Thanks, got these three patches now applied into fixes on top of -rc4.

Can you please try it out to make sure it's all working for you now?

Tony

Re: [PATCH 0/2 v2] ARM: OMAP1: Fix dpll1 reprogramming related issues

[Janusz Krzysztofik]

On Friday 02 of December 2011 at 03:09:37, Tony Lindgren wrote:
> * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111201 11:38]:
> > After dpll1 reprogramming has been moved from setup_arch() to
> > kernel_init(), I've been observing several issues, resulting in
> > undesired system behaviour on my Amstrad Delta. This series fixes
> > those most important (2/5 and 5/5 v2 previously), with a "stuck at 60MHz
> > rate" issue going to be fixed with Tony's proposed solution instead of
> > my previous 3/5.
> > 
> > Janusz Krzysztofik (2):
> >   ARM: OMAP1: Fix ckctl value used for dpll1 defualt rate
> >   ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram
> > 
> >  arch/arm/mach-omap1/clock_data.c |    9 +++++++--
> >  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> Thanks, got these three patches now applied into fixes on top of -rc4.
> 
> Can you please try it out to make sure it's all working for you now?

Yes, it works as expected, re-tested with a custom 150 MHz rate only 
.config to the extent possible without removing all rates form .config 
/ omap1_rate_table[], i.e., without triggering the issue addressed by 
fc92dd3a77bfc59822f869627ba81a6418eed987 "ARM: OMAP1: Fix ckctl value 
used for dpll1 defualt rate". Can re-test other cases (omap1_defconfig, 
no rates selected at all) if required.

My only concern left, which I don't feel capable to justify myself, is 
if 6% inaccuracy in BogoMIPS recalculation, compared to the BogoMIPS 
value got either with 3.1 or as a result of recalibration, does matter 
and should still be addressed somehow for next merge window, or can be 
ignored. 

Russell, can you please share your opinion?

Thanks,
Janusz

[PATCH] ARM: OMAP1: Set the omap1623 sram size to 16K

[Tony Lindgren]

Now that we're always reprogramming the core clock we must make
sure SRAM works. It seems that neither omap1621 or omap1623
has 256K of SRAM. Set the SRAM size to safe value of 16K.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/plat-omap/sram.c
+++ b/arch/arm/plat-omap/sram.c
@@ -141,11 +141,9 @@ static void __init omap_detect_sram(void)
 			omap_sram_size = 0x32000;	/* 200K */
 		else if (cpu_is_omap15xx())
 			omap_sram_size = 0x30000;	/* 192K */
-		else if (cpu_is_omap1610() || cpu_is_omap1621() ||
-		     cpu_is_omap1710())
+		else if (cpu_is_omap1610() || cpu_is_omap1611() ||
+				cpu_is_omap1621() || cpu_is_omap1710())
 			omap_sram_size = 0x4000;	/* 16K */
-		else if (cpu_is_omap1611())
-			omap_sram_size = SZ_256K;
 		else {
 			pr_err("Could not detect SRAM size\n");
 			omap_sram_size = 0x4000;

Re: [PATCH 5/5 v2] ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram

[Russell King - ARM Linux]

On Tue, Nov 29, 2011 at 01:25:32AM +0100, Janusz Krzysztofik wrote:
> However, the result of cpufreq_scale() differs from that of
> (re)calibrate_delay() by ca. 6%, i.e., 70.40 vs. 74.54. Please advise if
> this approximation is acceptable.

You don't say which figure is what.

Note that calibrate_delay() is itself inaccurate - the loops_per_jiffy
is the number of loops which can be executed between two timer ticks
_minus_ the time to process the timer interrupt itself.  So, it's
actually always a little less than the theoretical number of loops
within that time period.

Re: [PATCH 5/5 v2] ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram

[Janusz Krzysztofik]

On Friday 09 of December 2011 at 09:42:45, Russell King - ARM Linux wrote:
> On Tue, Nov 29, 2011 at 01:25:32AM +0100, Janusz Krzysztofik wrote:
> > However, the result of cpufreq_scale() differs from that of
> > (re)calibrate_delay() by ca. 6%, i.e., 70.40 vs. 74.54. Please advise if
> > this approximation is acceptable.
> 
> You don't say which figure is what.

Hi,
Those were BogoMIPS, which you were talking about in your comment 
(http://www.spinics.net/lists/linux-omap/msg60811.html).
> 
> Note that calibrate_delay() is itself inaccurate - the loops_per_jiffy
> is the number of loops which can be executed between two timer ticks
> _minus_ the time to process the timer interrupt itself.  So, it's
> actually always a little less than the theoretical number of loops
> within that time period.

I see. Then, in case of a machine always booting at, let's say, 12 and then reprogrammed to 150 MHz, we actually scale up that less then the theoretical number, with a side effect of scaling up its error as well. Perhaps in this case, when the machine is going to run at that target rate until rebooted, we should rather decide to recalibrate to keep that error proportionally small compared to the target loops per jiffy value, like it worked in my initial proposal? I think that your argument about unnecessarily wasting 10s of milliseconds has marginal importance here because we will be redoing that calibration only once, at boot time, and never later until next reboot.

Thanks,
Janusz

Re: [PATCH 5/5 v2] ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram

[Janusz Krzysztofik]

On Friday 09 of December 2011 at 11:00:01, Janusz Krzysztofik wrote:
> On Friday 09 of December 2011 at 09:42:45, Russell King - ARM Linux wrote:
> > On Tue, Nov 29, 2011 at 01:25:32AM +0100, Janusz Krzysztofik wrote:
> > > However, the result of cpufreq_scale() differs from that of
> > > (re)calibrate_delay() by ca. 6%, i.e., 70.40 vs. 74.54. Please advise if
> > > this approximation is acceptable.
> > 
> > You don't say which figure is what.
> 
> Hi,
> Those were BogoMIPS, which you were talking about in your comment 
> (http://www.spinics.net/lists/linux-omap/msg60811.html).

Should also mention that I was getting the lower number as a result of 
cpufreq_scale(), and the higher when doing recalibrate_delay().

Janusz

> > 
> > Note that calibrate_delay() is itself inaccurate - the loops_per_jiffy
> > is the number of loops which can be executed between two timer ticks
> > _minus_ the time to process the timer interrupt itself.  So, it's
> > actually always a little less than the theoretical number of loops
> > within that time period.
> 
> I see. Then, in case of a machine always booting at, let's say, 12 and then reprogrammed to 150 MHz, we actually scale up that less then the theoretical number, with a side effect of scaling up its error as well. Perhaps in this case, when the machine is going to run at that target rate until rebooted, we should rather decide to recalibrate to keep that error proportionally small compared to the target loops per jiffy value, like it worked in my initial proposal? I think that your argument about unnecessarily wasting 10s of milliseconds has marginal importance here because we will be redoing that calibration only once, at boot time, and never later until next reboot.
> 
> Thanks,
> Janusz
> 

Re: [PATCH 5/5 v2] ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram

[Russell King - ARM Linux]

On Fri, Dec 09, 2011 at 11:00:01AM +0100, Janusz Krzysztofik wrote:
> On Friday 09 of December 2011 at 09:42:45, Russell King - ARM Linux wrote:
> > On Tue, Nov 29, 2011 at 01:25:32AM +0100, Janusz Krzysztofik wrote:
> > > However, the result of cpufreq_scale() differs from that of
> > > (re)calibrate_delay() by ca. 6%, i.e., 70.40 vs. 74.54. Please advise if
> > > this approximation is acceptable.
> > 
> > You don't say which figure is what.
> 
> Hi,
> Those were BogoMIPS, which you were talking about in your comment 
> (http://www.spinics.net/lists/linux-omap/msg60811.html).

I realise that.  But which is which - is 70.40 from recalibrate_delay
or is it 74.54?  Your message is too vague to be able to interpret your
results because it's impossible to work out what figure refers to which
method.

> > Note that calibrate_delay() is itself inaccurate - the loops_per_jiffy
> > is the number of loops which can be executed between two timer ticks
> > _minus_ the time to process the timer interrupt itself.  So, it's
> > actually always a little less than the theoretical number of loops
> > within that time period.
> 
> I see. Then, in case of a machine always booting at, let's say, 12 and
> then reprogrammed to 150 MHz, we actually scale up that less then the
> theoretical number, with a side effect of scaling up its error as well.
> Perhaps in this case, when the machine is going to run at that target
> rate until rebooted, we should rather decide to recalibrate to keep
> that error proportionally small compared to the target loops per
> jiffy value, like it worked in my initial proposal? I think that
> your argument about unnecessarily wasting 10s of milliseconds has
> marginal importance here because we will be redoing that calibration
> only once, at boot time, and never later until next reboot.

It really doesn't matter - udelay() etc is not designed to be mega
accurate but good enough.  The fact is that it has always produced a
delay of approximately the value requested of it (and normally it
would produce a slightly shorter delay) and that's a fact of life
that driver authors should already be dealing with.

So, your patch is fine.

Re: [PATCH 5/5 v2] ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram

[Janusz Krzysztofik]

On Saturday 10 of December 2011 at 01:25:03, Russell King - ARM Linux wrote:
> On Fri, Dec 09, 2011 at 11:00:01AM +0100, Janusz Krzysztofik wrote:
> > 
> > Those were BogoMIPS, ...
> 
> I realise that.  But which is which - is 70.40 from recalibrate_delay
> or is it 74.54?  Your message is too vague to be able to interpret your
> results because it's impossible to work out what figure refers to which
> method.

Yes, I realised what you had actually asked about after re-reading your 
answer, which I unfortunately did after I had already replied, sorry.

> > ... Then, in case of a machine always booting at, let's say, 12 and
> > then reprogrammed to 150 MHz, we actually scale up that less then the
> > theoretical number, with a side effect of scaling up its error as well.
> > Perhaps in this case, when the machine is going to run at that target
> > rate until rebooted, we should rather decide to recalibrate to keep
> > that error proportionally small compared to the target loops per
> > jiffy value ...
> 
> It really doesn't matter - udelay() etc is not designed to be mega
> accurate but good enough... 

Great, that's the answer to my initial question: loops per jiffy 
inaccuracy of 6% shouldn't matter.

Thanks,
Janusz