ARM: OMAP1: Fix dpll1 reprogramming related issues
Subsystems (5)
cover ARM: OMAP1: Fix dpll1 reprogramming related issues
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(-)
Signature · 1 line
1.7.3.4
PATCH 1/5 ARM: OMAP1: Fix dpll1 default rate reprogramming method
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.
--- 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(-)
arch/arm/mach-omap1/clock.c +1 -5
@@ -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;
arch/arm/mach-omap1/clock_data.c +5 -2
@@ -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);
arch/arm/plat-omap/sram.c +3 -0
@@ -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); }
Signature · 1 line
1.7.3.4
Tony Lindgren
reviewerRe: [PATCH 1/5] ARM: OMAP1: Fix dpll1 default rate reprogramming method
Quoted 8 lines · 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.
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
Quoted 67 lines
Signed-off-by Janusz Krzysztofik <[email protected]>--- 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(-)arch/arm/mach-omap1/clock.c +1
-5@@ -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;arch/arm/mach-omap1/clock_data.c +5
-2@@ -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);arch/arm/plat-omap/sram.c +3
-0@@ -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); }Signature · 1 line
1.7.3.4
PATCH 1/1 Remove unsafe clock values from omap1_defconfig
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.
---
Quoted 10 lines · Tony Lindgren
Quoted 8 lines · 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.
Quoted 6 lines
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
Kconfigoptions for the supported MHz rates. This means that with this changeomap1_defconfigwill 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(-)
arch/arm/configs/omap1_defconfig +0 -5
@@ -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
Signature · 1 line
1.7.3.4
Tony Lindgren
reviewerRe: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig
Quoted 6 lines · Janusz Krzysztofik
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_defconfigbooting on some boards, is questionable, while you posted this comment against patch 1/5, which I don't see how it could break booting, includingomap1_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.
Quoted 2 lines
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
PATCH v2 1/1 ARM: OMAP1: select clock rate by CPU type
In reply to Tony LindgrenDPLL1 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.
---
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(-)
arch/arm/mach-omap1/clock.c +6 -0
@@ -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 ;
arch/arm/mach-omap1/clock_data.c +3 -1
@@ -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 /*
arch/arm/mach-omap1/opp.h +2 -0
@@ -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
arch/arm/mach-omap1/opp_data.c +29 -14
73 lines not on this page · raw message
Signature · 1 line
1.7.3.4
Tony Lindgren
reviewerRe: [PATCH 2a/5 v2] ARM: OMAP1: select clock rate by CPU type
Quoted 16 lines · 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, inomap1_defconfigwe currently rely onKconfigoptions 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_defconfigwill stop booting on boards with imperfect boot loaders, as it would always try to change to 216MHz.Expand
omap1_rate_tablewith flags for different CPU types and match them while selecting clock rates. The idea is stolen from current omap24xx clock rate selection algorithm.
Quoted 5 lines
Signed-off-by Janusz Krzysztofik <[email protected]>---
Hi,
Since there is no response to patch 2a/5 "Remove unsafe clock values fromomap1_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.
Quoted 9 lines
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 fromomap1_defconfigat 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 exceptCK_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.
Quoted 4 lines
arch/arm/mach-omap1/opp_data.c +0
-0@@ -21,38 +22,52 @@ struct mpu_rate omap1_rate_table[] = {* armdiv, dspdiv, dspmmu, tcdiv, perdiv, lcddiv */ #if defined(CONFIG_OMAP_ARM_216MHZ )
arch/arm/mach-omap1/opp_data.c struct mpu_rate omap1_rate_table[] = { · line 32
Quoted 10 lines
arch/arm/mach-omap1/opp_data.c +4
-2- { 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 v2] ARM: OMAP1: select clock rate by CPU type
Quoted 3 lines · Tony Lindgren
We should also now be able to remove all the
CONFIG_OMAP_ARM_XXXMHZoptions 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
Tony Lindgren
reviewerRe: [PATCH 2a/5 v2] ARM: OMAP1: select clock rate by CPU type
Quoted 9 lines · Janusz Krzysztofik
Quoted 3 lines · Tony Lindgren
We should also now be able to remove all the
CONFIG_OMAP_ARM_XXXMHZoptions too, right?Right, but then, perhaps the initial version of patch 2a/5, which already started removing them, from
omap1_defconfigfor 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 v2] ARM: OMAP1: select clock rate by CPU type
Quoted 6 lines · Tony Lindgren
Quoted 5 lines · Janusz Krzysztofik
Quoted 3 lines · Tony Lindgren
We should also now be able to remove all the
CONFIG_OMAP_ARM_XXXMHZoptions too, right?
Quoted 6 lines
Quoted 4 lines
Right, but then, perhaps the initial version of patch 2a/5, which already started removing them, from
omap1_defconfigfor 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.
Quoted 6 lines
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 asomap1_defconfighadOMAP_CLOCKS_SET_BY_BOOTLOADERoption set.
Quoted 6 lines
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
Tony Lindgren
reviewerRe: [PATCH 2a/5 v2] ARM: OMAP1: select clock rate by CPU type
Quoted 12 lines · Janusz Krzysztofik
Quoted 11 lines · Tony Lindgren
Quoted 9 lines · Janusz Krzysztofik
Quoted 3 lines · Tony Lindgren
We should also now be able to remove all the
CONFIG_OMAP_ARM_XXXMHZoptions too, right?Right, but then, perhaps the initial version of patch 2a/5, which already started removing them, from
omap1_defconfigfor now, then going into the right direction while unblocking another regression fix (3/5), _is_ a good candidate for an rc fix?
Quoted 6 lines
Quoted 1 line
But we did not allow dpll1 reprogramming earlier either,
Wrong. Without
OMAP_CLOCKS_SET_BY_BOOTLOADERselected, we always did, but only once, early at boot, beforeck_dpll1_p->ratewas set first fromomap1_clk_init(), and never retried later, that's why that check which I removed with 3/5 was never in the game untile9b7086b80c4d9e354f4edc9e280ae85a60df408.
Yeah you're right. You found what caused the regression :)
Quoted 10 lines
Quoted 10 lines
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 asomap1_defconfighadOMAP_CLOCKS_SET_BY_BOOTLOADERoption 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.
Quoted 7 lines
Quoted 2 lines
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 beforemap_iorelated 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
PATCH 1/1 ARM: OMAP1: Move dpll1 rates selection from config to runtime
In reply to Tony LindgrenFor still better multi-OMAP1 support, expand omap1_rate_table with flags for different SoC types and match them while selecting clock rates. The idea is stolen from current omap24xx clock rate selection algorithm.
Since clkdev platform flag definitions are reused here, those had to be expanded with one extra entry for OMAP1710 subtype, as this is the only SoC for which we allow selection of the highest, 216 MHz rate.
Once done, remove no longer needed clock rate configure time options.
Created against linux-omap/master tip as of Thu Dec 1, commit f83c2a8cbb59981722d1ab610c79adfd034a2667. Tested on Amstrad Delta.
---
Hi,
Here is a list of changes against the initial version, submitted as patch 2a/5 v2 "ARM: OMAP1: select clock rate by CPU type" of the series "ARM: OMAP1: Fix dpll1 reprogramming related issues":
* drop no longer needed .config options, as suggested by Tony (thanks!),
* follow selection conditions of those removed .config clock rate options more closely (I missed that before, ended up reinventing the wheel),
* add extra clkdev platform flag for OMAP1710 to be able to limit the 216 MHz rate selection to this SoC subtype only,
* update commit summary and message.
I still wonder if it is sufficient to select a highest rate for a board based on a SoC type only, not on the board type itself. For example, the Amstrad Delta has been used to be running at 150 MHz since it was introduced first, while from now on it will be running at 168 MHz. I haven't noticed any visible issues so far, but who knows... Any opinions?
Thanks,
Janusz
arch/arm/configs/omap1_defconfig | 1 - arch/arm/mach-omap1/Kconfig | 64 ------------------------- arch/arm/mach-omap1/clock.c | 6 ++ arch/arm/mach-omap1/clock.h | 3 + arch/arm/mach-omap1/clock_data.c | 6 ++- arch/arm/mach-omap1/opp.h | 1 + arch/arm/mach-omap1/opp_data.c | 63 +++++++++++------------- arch/arm/plat-omap/include/plat/clkdev_omap.h | 1 + 8 files changed, 45 insertions(+), 100 deletions(-)
arch/arm/configs/omap1_defconfig +0 -1
@@ -48,7 +48,6 @@ CONFIG_MACH_SX1=y CONFIG_MACH_NOKIA770=y CONFIG_MACH_AMS_DELTA=y CONFIG_MACH_OMAP_GENERIC=y -CONFIG_OMAP_ARM_182MHZ=y # CONFIG_ARM_THUMB is not set CONFIG_PCCARD=y CONFIG_OMAP_CF=y
arch/arm/mach-omap1/Kconfig +0 -64
70 lines not on this page · raw message
arch/arm/mach-omap1/clock.c +6 -0
@@ -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 ;
@@ -290,6 +293,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 ;
arch/arm/mach-omap1/clock.h +3 -0
@@ -111,4 +111,7 @@ extern const struct clkops clkops_dummy;extern const struct clkops clkops_uart_16xx;extern const struct clkops clkops_generic; +/* used for passing SoC type to omap1_{select,round_to}_table_rate() */ +extern u32 cpu_mask; +#endif
arch/arm/mach-omap1/clock_data.c +5 -1
@@ -778,12 +778,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 /*
@@ -808,6 +810,8 @@ int __init omap1_clk_init(void)clk_preinit (c->lk.clk); cpu_mask =0 ; +if (cpu_is_omap1710 ()) + cpu_mask |=CK_1710 ;if (cpu_is_omap16xx ()) cpu_mask |=CK_16XX ;if (cpu_is_omap1510 ())
arch/arm/mach-omap1/opp.h +1 -0
@@ -21,6 +21,7 @@ struct mpu_rate {unsigned long pll_rate;__u16 ckctl_val;__u16 dpllctl_val; +u32 flags; };extern struct mpu_rate omap1_rate_table[];
arch/arm/mach-omap1/opp_data.c +29 -34
75 lines not on this page · raw message
arch/arm/plat-omap/include/plat/clkdev_omap.h +1 -0
@@ -41,6 +41,7 @@ struct omap_clk {#define CK_TI816X (1 << 12)#define CK_446X (1 << 13)#define CK_AM33XX (1 << 14)/* AM33xx specific clocks */ +#define CK_1710 (1 << 15)/* 1710 extra for rate selection */ #define CK_34XX (CK_3430ES1 | CK_3430ES2PLUS)
Signature · 1 line
1.7.3.4
PATCH 1/1 ARM: OMAP1: Move dpll1 rates selection from config to runtime
Quoted 28 lines · Janusz Krzysztofik
For still better multi-OMAP1 support, expand
omap1_rate_tablewith flags for different SoC types and match them while selecting clock rates. The idea is stolen from current omap24xx clock rate selection algorithm.Since clkdev platform flag definitions are reused here, those had to be expanded with one extra entry for OMAP1710 subtype, as this is the only SoC for which we allow selection of the highest, 216 MHz rate.
Once done, remove no longer needed clock rate configure time options.
Created against linux-omap/master tip as of Thu Dec 1, commit
f83c2a8cbb59981722d1ab610c79adfd034a2667. Tested on Amstrad Delta.Signed-off-by Janusz Krzysztofik <[email protected]>---
Hi,
Here is a list of changes against the initial version, submitted as patch 2a/5 v2 "ARM: OMAP1: select clock rate by CPU type" of the series "ARM: OMAP1: Fix dpll1 reprogramming related issues":
* drop no longer needed .config options, as suggested by Tony (thanks!),
* follow selection conditions of those removed .config clock rate options more closely (I missed that before, ended up reinventing the wheel),
* add extra clkdev platform flag for OMAP1710 to be able to limit the 216 MHz rate selection to this SoC subtype only,
* update commit summary and message.
Quoted 6 lines
I still wonder if it is sufficient to select a highest rate for a board based on a SoC type only, not on the board type itself. For example, the Amstrad Delta has been used to be running at 150 MHz since it was introduced first, while from now on it will be running at 168 MHz. I haven't noticed any visible issues so far, but who knows... Any opinions?
Hi,
I've collected one opinion, or observation, myself: one of my boards doesn't work correctly at 168 MHz, but works without problems at 150 MHz.
Tony,
Is it OK for you if we limit the maximum dpll1 rate for all OMAP1510 based boards to 150 MHz (now 168 MHz), or do you prefer me to invent an alternative solution, which would derive the (maximum) rate from the board type?
Thanks,
Janusz
PATCH v2 1/1 ARM: OMAP1: Move dpll1 rates selection from config to runtime
For still better multi-OMAP1 support, expand omap1_rate_table with flags for different SoC types and match them while selecting clock rates. The idea is stolen from current omap24xx clock rate selection algorithm.
Since clkdev platform flag definitions are reused here, those had to be expanded with one extra entry for OMAP1710 subtype, as this is the only SoC for which we allow selection of the highest, 216 MHz rate.
Once done, remove no longer needed clock rate configure time options.
Created against linux-omap/master tip as of Thu Dec 1, commit f83c2a8cbb59981722d1ab610c79adfd034a2667. Tested on Amstrad Delta.
---
v2: limit OMAP1510 rate to 150 MHz, or some boards may not work.
arch/arm/configs/omap1_defconfig | 1 - arch/arm/mach-omap1/Kconfig | 64 ------------------------- arch/arm/mach-omap1/clock.c | 6 ++ arch/arm/mach-omap1/clock.h | 3 + arch/arm/mach-omap1/clock_data.c | 6 ++- arch/arm/mach-omap1/opp.h | 1 + arch/arm/mach-omap1/opp_data.c | 63 +++++++++++------------- arch/arm/plat-omap/include/plat/clkdev_omap.h | 1 + 8 files changed, 45 insertions(+), 100 deletions(-)
arch/arm/configs/omap1_defconfig +0 -1
@@ -48,7 +48,6 @@ CONFIG_MACH_SX1=y CONFIG_MACH_NOKIA770=y CONFIG_MACH_AMS_DELTA=y CONFIG_MACH_OMAP_GENERIC=y -CONFIG_OMAP_ARM_182MHZ=y # CONFIG_ARM_THUMB is not set CONFIG_PCCARD=y CONFIG_OMAP_CF=y
arch/arm/mach-omap1/Kconfig +0 -64
70 lines not on this page · raw message
arch/arm/mach-omap1/clock.c +6 -0
@@ -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 ;
@@ -290,6 +293,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 ;
arch/arm/mach-omap1/clock.h +3 -0
@@ -111,4 +111,7 @@ extern const struct clkops clkops_dummy;extern const struct clkops clkops_uart_16xx;extern const struct clkops clkops_generic; +/* used for passing SoC type to omap1_{select,round_to}_table_rate() */ +extern u32 cpu_mask; +#endif
arch/arm/mach-omap1/clock_data.c +5 -1
@@ -778,12 +778,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 /*
@@ -808,6 +810,8 @@ int __init omap1_clk_init(void)clk_preinit (c->lk.clk); cpu_mask =0 ; +if (cpu_is_omap1710 ()) + cpu_mask |=CK_1710 ;if (cpu_is_omap16xx ()) cpu_mask |=CK_16XX ;if (cpu_is_omap1510 ())
arch/arm/mach-omap1/opp.h +1 -0
@@ -21,6 +21,7 @@ struct mpu_rate {unsigned long pll_rate;__u16 ckctl_val;__u16 dpllctl_val; +u32 flags; };extern struct mpu_rate omap1_rate_table[];
arch/arm/mach-omap1/opp_data.c +29 -34
75 lines not on this page · raw message
arch/arm/plat-omap/include/plat/clkdev_omap.h +1 -0
@@ -41,6 +41,7 @@ struct omap_clk {#define CK_TI816X (1 << 12)#define CK_446X (1 << 13)#define CK_AM33XX (1 << 14)/* AM33xx specific clocks */ +#define CK_1710 (1 << 15)/* 1710 extra for rate selection */ #define CK_34XX (CK_3430ES1 | CK_3430ES2PLUS)
Signature · 1 line
1.7.3.4
Tony Lindgren
reviewerPATCH v2 1/1 ARM: OMAP1: Move dpll1 rates selection from config to runtime
Quoted 10 lines · Janusz Krzysztofik
For still better multi-OMAP1 support, expand
omap1_rate_tablewith flags for different SoC types and match them while selecting clock rates. The idea is stolen from current omap24xx clock rate selection algorithm.Since clkdev platform flag definitions are reused here, those had to be expanded with one extra entry for OMAP1710 subtype, as this is the only SoC for which we allow selection of the highest, 216 MHz rate.
Once done, remove no longer needed clock rate configure time options.
Quoted 7 lines
Created against linux-omap/master tip as of Thu Dec 1, commit
f83c2a8cbb59981722d1ab610c79adfd034a2667. Tested on Amstrad Delta.Signed-off-by Janusz Krzysztofik <[email protected]>---
v2: limit OMAP1510 rate to 150 MHz, or some boards may not work.
Thanks pushing them all into omap1 branch, can you please check that I got all the patches? I've boot tested it here on h2/osk/770.
Tony
PATCH v2 1/1 ARM: OMAP1: Move dpll1 rates selection from config to runtime
Quoted 15 lines · Tony Lindgren
Quoted 14 lines · Janusz Krzysztofik
For still better multi-OMAP1 support, expand
omap1_rate_tablewith flags for different SoC types and match them while selecting clock rates. The idea is stolen from current omap24xx clock rate selection algorithm.Since clkdev platform flag definitions are reused here, those had to be expanded with one extra entry for OMAP1710 subtype, as this is the only SoC for which we allow selection of the highest, 216 MHz rate.
Once done, remove no longer needed clock rate configure time options.
Created against linux-omap/master tip as of Thu Dec 1, commit
f83c2a8cbb59981722d1ab610c79adfd034a2667. Tested on Amstrad Delta.
Quoted 6 lines
Quoted 3 lines
Signed-off-by Janusz Krzysztofik <[email protected]>---
v2: limit OMAP1510 rate to 150 MHz, or some boards may not work.Thanks pushing them all into omap1 branch, can you please check that I got all the patches? I've boot tested it here on h2/osk/770.
Looks like all are there, thank you.
Janusz
Tony Lindgren
reviewerPATCH 1/1 ARM: OMAP1: Set the omap1623 sram size to 16K
In reply to Janusz KrzysztofikNow 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.
arch/arm/plat-omap/sram.c +2 -4
@@ -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 ;
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig
I've unintentionally answered off-line, sorry, re-adding all Cc:'s.
Quoted 21 lines · Tony Lindgren
Quoted 20 lines · Janusz Krzysztofik
Quoted 3 lines · Tony Lindgren
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 ifomap1_defconfigor custom,2a/5: required by 3/5,
Quoted 8 lines
Quoted 4 lines
5/5: without this one, BogoMIPS is not updated after dpll1 reprogramme,
breakingomap_keypadat 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?
Quoted 2 lines
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.
Quoted 6 lines
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] Remove unsafe clock values from omap1_defconfig
Quoted 3 lines · Janusz Krzysztofik
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
Tony Lindgren
reviewerRe: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig
In reply to Janusz KrzysztofikQuoted 5 lines · Janusz Krzysztofik
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.
Quoted 4 lines
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 :)
Quoted 10 lines
Quoted 2 lines
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.
Quoted 2 lines
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
arch/arm/mach-omap1/clock_data.c +1 -1
@@ -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] Remove unsafe clock values from omap1_defconfig
Quoted 1 line · Tony Lindgren
* Janusz Krzysztofik <[email protected]> [111201 01:20]:
[snip]
Quoted 9 lines
Quoted 6 lines
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.
Quoted 5 lines
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]
arch/arm/mach-omap1/clock_data.c int __init omap1_clk_init(void) · line 933
Quoted 12 lines
arch/arm/mach-omap1/clock_data.c +1
-1@@ -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
Tony Lindgren
reviewerRe: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig
Quoted 12 lines · Janusz Krzysztofik
Quoted 11 lines · Tony Lindgren
arch/arm/mach-omap1/clock_data.c +1
-1@@ -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 ;
Quoted 5 lines
arch/arm/mach-omap1/clock_data.c int __init omap1_clk_init(void) · line 933
Quoted 1 line
arch/arm/mach-omap1/clock_data.c +0
-0/* 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.
Quoted 7 lines
Before
e9b7086b80c4d9e354f4edc9e280ae85a60df408,omap1_select_table_rate()was returning the rate selected with .config because it was called early, withck_dpll1_p->rateuninitialized. 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 withomap1_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 <[email protected]>
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 <[email protected]> for figuring this one out.
arch/arm/mach-omap1/clock_data.c +3 -0
@@ -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" );
Janusz Krzysztofik
Acked-byRe: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig
Quoted 20 lines · Tony Lindgren
From: Tony Lindgren <[email protected]>
Date: Thu, 1 Dec 2011 11:00:11 -0800
Subject: [PATCH] ARM: OMAP1: Fix reprogramming of DPLL1 for systems that boot at rates below 60MHzCommit
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_ratecurrently 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->rateuninitialized.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.
Quoted 5 lines
Thanks for Janusz Krzysztofik <[email protected]> for figuring this one out.
Reported-by Janusz Krzysztofik <[email protected]>Signed-off-by Tony Lindgren <[email protected]>
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
Quoted 20 lines
arch/arm/mach-omap1/clock_data.c +3
-1@@ -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 at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tony Lindgren
reviewerRe: [PATCH 2a/5] Remove unsafe clock values from omap1_defconfig
Quoted 24 lines · Janusz Krzysztofik
Quoted 23 lines · Tony Lindgren
From: Tony Lindgren <[email protected]>
Date: Thu, 1 Dec 2011 11:00:11 -0800
Subject: [PATCH] ARM: OMAP1: Fix reprogramming of DPLL1 for systems that boot at rates below 60MHzCommit
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_ratecurrently 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->rateuninitialized.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 <[email protected]> for figuring this one out.
Quoted 8 lines
Quoted 2 lines
Reported-by Janusz Krzysztofik <[email protected]>Signed-off-by Tony Lindgren <[email protected]>However, this way or another, we are back to your mentioned problem of
omap1_defconfigalways switching to 216 MHz, I'm afraid. Then, 2a/5 v1 "Remove unsafe clock values fromomap1_defconfig" can still be helpful.
OK that makes sense now also in case there are other systems that boot at rates below 60MHz.
Quoted 4 lines
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 1/1 ARM: OMAP1: Remove unsafe clock values from omap1_defconfig
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.
---
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(-)
arch/arm/configs/omap1_defconfig +0 -5
@@ -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
Signature · 1 line
1.7.3.4
v2 cover ARM: OMAP1: Fix dpll1 reprogramming related issues
In reply to Janusz KrzysztofikAfter 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(-)
Signature · 1 line
1.7.3.4
PATCH v2 1/1 ARM: OMAP1: Update dpll1 default rate reprogramming method
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.
---
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(-)
arch/arm/mach-omap1/clock.c +1 -5
@@ -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;
arch/arm/mach-omap1/clock_data.c +5 -2
@@ -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);
arch/arm/plat-omap/sram.c +3 -0
@@ -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); }
Signature · 1 line
1.7.3.4
Tony Lindgren
reviewerQuoted 7 lines · Janusz Krzysztofik
After dpll1 reprogramming has been moved from
setup_arch()tokernel_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.
Quoted 6 lines
Janusz Krzysztofik (2):
ARM: OMAP1: Fix ckctl value used for dpll1 defualt rate
ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogramarch/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
Quoted 12 lines · Tony Lindgren
Quoted 11 lines · Janusz Krzysztofik
After dpll1 reprogramming has been moved from
setup_arch()tokernel_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
Quoted 6 lines
Quoted 2 lines
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 v2 1/2 ARM: OMAP1: Fix ckctl value used for dpll1 defualt rate
In reply to Janusz KrzysztofikUse 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.
---
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(-)
arch/arm/mach-omap1/clock_data.c +1 -1
@@ -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);
Signature · 1 line
1.7.3.4
PATCH v2 2/2 ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram
In reply to Janusz KrzysztofikOtherwise 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.
---
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(-)
arch/arm/mach-omap1/clock_data.c +6 -1
@@ -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); }
Signature · 1 line
1.7.3.4
PATCH 1/1 ARM: OMAP1: Always reprogramme dpll1 rate at boot.
In reply to Tony LindgrenDPLL1 reprogramming to a different rate is actually blocked inside omap1_select_table_rate(). However, it is already forced at boot, for boards which boot at unusable clock rates, and this seems to work correctly.
OTOH, we now have a fine, run time performed clock selection algorithm implemented, which prevents less powerfull SoCs from being overclocked unintentionally.
Allow reprogramming of dpll1 by default, and use it for switching to the higest supported clock rate with all boards, including those already booting at a usable rate of 60 MHz or above.
Created against linux-omap/master tip as of Thu Dec 1, commit f83c2a8cbb59981722d1ab610c79adfd034a2667. Requires the just submitted patch "ARM: OMAP1: Move dpll1 rates selection from config to runtime" to prevent from unintentional overclocking. Tested on Amstrad Delta.
--- arch/arm/mach-omap1/clock.c | 4 ---- arch/arm/mach-omap1/clock_data.c | 6 ------ 2 files changed, 0 insertions(+), 10 deletions(-)
arch/arm/mach-omap1/clock.c +0 -4
@@ -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 ;
arch/arm/mach-omap1/clock_data.c +0 -6
@@ -931,12 +931,6 @@ void __init omap1_clk_late_init(void) {unsigned long rate = ck_dpll1.rate; -if (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" );
Signature · 1 line
1.7.3.4
PATCH 2/5 ARM: OMAP1: Fix ckctl value used for dpll1 defualt rate
In reply to Janusz KrzysztofikUse 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".
--- arch/arm/mach-omap1/clock_data.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap1/clock_data.c +1 -1
@@ -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);
Signature · 1 line
1.7.3.4
PATCH 3/5 ARM: OMAP1: Fix dpll1 reprogramming not actually allowed
In reply to Janusz KrzysztofikDPLL1 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.
--- arch/arm/mach-omap1/clock.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)
arch/arm/mach-omap1/clock.c +0 -4
@@ -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 ;
Signature · 1 line
1.7.3.4
Tony Lindgren
reviewerRe: [PATCH 3/5] ARM: OMAP1: Fix dpll1 reprogramming not actually allowed
Quoted 6 lines · 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.
This too first requires something for boards to pass the supported rates.
Tony
Quoted 23 lines
Signed-off-by Janusz Krzysztofik <[email protected]>--- arch/arm/mach-omap1/clock.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)arch/arm/mach-omap1/clock.c +0
-4@@ -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 ;Signature · 1 line
1.7.3.4
PATCH 4/5 init/calibrate.c: allow for recalibration of loops per jiffy
In reply to Janusz KrzysztofikThis 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.
--- include/linux/delay.h | 4 +++- init/calibrate.c | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-)
include/linux/delay.h +3 -1
@@ -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);
init/calibrate.c +4 -2
@@ -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" );
Signature · 1 line
1.7.3.4
Re: [PATCH 4/5] init/calibrate.c: allow for recalibration of loops per jiffy
Quoted 4 lines · Janusz Krzysztofik
This is required on ARM OMAP1 platform, after DPLL reprogramming has been fixed by moving it from
setup_arch()tokernel_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 4/5] init/calibrate.c: allow for recalibration of loops per jiffy
Quoted 9 lines · Russell King - ARM Linux
Quoted 4 lines · Janusz Krzysztofik
This is required on ARM OMAP1 platform, after DPLL reprogramming has been fixed by moving it from
setup_arch()tokernel_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 ARM: OMAP1: recalibrate loops per jiffy after dpll1 reprogram
In reply to Janusz KrzysztofikOtherwise 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.
--- arch/arm/mach-omap1/clock_data.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
arch/arm/mach-omap1/clock_data.c +2 -0
@@ -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 (); }
Signature · 1 line
1.7.3.4
PATCH v2 5/5 ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram
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.
---
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(-)
arch/arm/mach-omap1/clock_data.c +6 -1
@@ -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); }
Signature · 1 line
1.7.3.4
Re: [PATCH 5/5 v2] ARM: OMAP1: recalculate loops per jiffy after dpll1 reprogram
Quoted 3 lines · Janusz Krzysztofik
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
Quoted 6 lines · Russell King - ARM Linux
Quoted 3 lines · Janusz Krzysztofik
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).
Quoted 6 lines
Note that
calibrate_delay()is itself inaccurate - theloops_per_jiffyis 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
Quoted 11 lines · Janusz Krzysztofik
Quoted 6 lines · Russell King - ARM Linux
Quoted 3 lines · Janusz Krzysztofik
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
Quoted 12 lines
Quoted 6 lines
Note that
calibrate_delay()is itself inaccurate - theloops_per_jiffyis 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
In reply to Janusz KrzysztofikQuoted 11 lines · Janusz Krzysztofik
Quoted 6 lines · Russell King - ARM Linux
Quoted 3 lines · Janusz Krzysztofik
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.
Quoted 6 lines
Quoted 5 lines
Note that
calibrate_delay()is itself inaccurate - theloops_per_jiffyis 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.
Quoted 10 lines
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
Quoted 8 lines · Russell King - ARM Linux
Quoted 2 lines · Janusz Krzysztofik
Those were BogoMIPS, ...
I realise that. But which is which - is 70.40 from
recalibrate_delayor 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.
Quoted 10 lines
Quoted 7 lines
... 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