| Message ID | 1306312622-4901-1-git-send-email-christian.riesch@omicron.at (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
>From Menon, Nishanth [mailto:nm@ti.com] > On Fri, May 27, 2011 at 02:20, Christian Riesch > <christian.riesch@omicron.at> wrote: >> From: Bob Dunlop <bob.dunlop@xyzzy.org.uk> >> >> Currently the input frequency of the SoC is hardcoded in the SoC >> specific da850.c file to 24 MHz. Since the SoC accepts input >> frequencies in a wide range from 12 to 50 MHz, boards with different >> oscillator/crystal frequencies may be built. >> >> This patch allows setting a different input frequency in the board >> specific files to support boards with oscillator/crystal frequencies >> other than 24 MHz. >Curious question: have you considered Documentation/power/opp.txt? Not >entirely sure if that will help, but I am curious to know if there are >reasons why it did not scale for you I took a look at it but I got the impression that the code in mach-davinci does not use your OPP library. Instead the structs that are used for the OPPs are defined locally in arch/arm/mach-davinci/da850.c. The frequency multiplier and divider values are hardcoded there and do not scale with the input clock frequency. Christian
On Fri, May 27, 2011 at 14:50:53, Christian Riesch wrote: > From: Bob Dunlop <bob.dunlop@xyzzy.org.uk> > > Currently the input frequency of the SoC is hardcoded in the SoC specific > da850.c file to 24 MHz. Since the SoC accepts input frequencies in a wide > range from 12 to 50 MHz, boards with different oscillator/crystal > frequencies may be built. The same problem was fixed for dm6467/T EVMs with a different approach. This solution is much better than what was done for dm6467/T. So I am working on converting the dm6467/T code to this approach and should be able to post the updated patchset by tomorrow. Thanks, Sekhar
Christian Riesch <christian.riesch@omicron.at> writes: > From: Bob Dunlop <bob.dunlop@xyzzy.org.uk> > > Currently the input frequency of the SoC is hardcoded in the SoC specific > da850.c file to 24 MHz. Since the SoC accepts input frequencies in a wide > range from 12 to 50 MHz, boards with different oscillator/crystal > frequencies may be built. > > This patch allows setting a different input frequency in the board > specific files to support boards with oscillator/crystal frequencies other > than 24 MHz. > > Signed-off-by: Bob Dunlop <bob.dunlop@xyzzy.org.uk> > Signed-off-by: Christian Riesch <christian.riesch@omicron.at> Why not allow board code to just do a clk_set_rate()? Currently the ref_clk struct clk does not have a .set_rate method implemented, but that should be easy enough to add. Then the default ref_clk.rate would stay the 24MHz, but any boards that want to override that simply use clk_get(), clk_set_rate(), clk_put() Kevin > --- > > Hi, > in private email Bob Dunlop suggested to pass a pointer to a small > structure instead of the frequency value to allow future expansions, > e.g., for the OPP list. Therefore I submit his patch as V2. > Christian > > arch/arm/mach-davinci/board-da850-evm.c | 2 +- > arch/arm/mach-davinci/board-mityomapl138.c | 2 +- > arch/arm/mach-davinci/board-omapl138-hawk.c | 2 +- > arch/arm/mach-davinci/da850.c | 5 ++++- > arch/arm/mach-davinci/include/mach/da8xx.h | 6 +++++- > 5 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c > index a7b41bf..231ff87 100644 > --- a/arch/arm/mach-davinci/board-da850-evm.c > +++ b/arch/arm/mach-davinci/board-da850-evm.c > @@ -1252,7 +1252,7 @@ console_initcall(da850_evm_console_init); > > static void __init da850_evm_map_io(void) > { > - da850_init(); > + da850_init(NULL); > } > > MACHINE_START(DAVINCI_DA850_EVM, "DaVinci DA850/OMAP-L138/AM18x EVM") > diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c > index 606a6f2..362770c 100644 > --- a/arch/arm/mach-davinci/board-mityomapl138.c > +++ b/arch/arm/mach-davinci/board-mityomapl138.c > @@ -561,7 +561,7 @@ console_initcall(mityomapl138_console_init); > > static void __init mityomapl138_map_io(void) > { > - da850_init(); > + da850_init(NULL); > } > > MACHINE_START(MITYOMAPL138, "MityDSP-L138/MityARM-1808") > diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c > index 67c38d0..c43a6c3 100644 > --- a/arch/arm/mach-davinci/board-omapl138-hawk.c > +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c > @@ -334,7 +334,7 @@ console_initcall(omapl138_hawk_console_init); > > static void __init omapl138_hawk_map_io(void) > { > - da850_init(); > + da850_init(NULL); > } > > MACHINE_START(OMAPL138_HAWKBOARD, "AM18x/OMAP-L138 Hawkboard") > diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c > index 133aac4..ebd0603 100644 > --- a/arch/arm/mach-davinci/da850.c > +++ b/arch/arm/mach-davinci/da850.c > @@ -1104,10 +1104,13 @@ static struct davinci_soc_info davinci_soc_info_da850 = { > .reset_device = &da8xx_wdt_device, > }; > > -void __init da850_init(void) > +void __init da850_init(struct da850_init_board_info *board) > { > unsigned int v; > > + if (board && board->ref_clk_rate) > + ref_clk.rate = board->ref_clk_rate; > + > davinci_common_init(&davinci_soc_info_da850); > > da8xx_syscfg0_base = ioremap(DA8XX_SYSCFG0_BASE, SZ_4K); > diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h > index ad64da7..66efc5d 100644 > --- a/arch/arm/mach-davinci/include/mach/da8xx.h > +++ b/arch/arm/mach-davinci/include/mach/da8xx.h > @@ -69,8 +69,12 @@ extern unsigned int da850_max_speed; > #define DA8XX_AEMIF_CTL_BASE 0x68000000 > #define DA8XX_ARM_RAM_BASE 0xffff0000 > > +struct da850_init_board_info { > + unsigned long ref_clk_rate; > +}; > + > void __init da830_init(void); > -void __init da850_init(void); > +void __init da850_init(struct da850_init_board_info *board); > > int da830_register_edma(struct edma_rsv_info *rsv); > int da850_register_edma(struct edma_rsv_info *rsv[2]);
Hi Kevin, On Tue, Jun 07, 2011 at 04:14:59, Hilman, Kevin wrote: > Christian Riesch <christian.riesch@omicron.at> writes: > > > From: Bob Dunlop <bob.dunlop@xyzzy.org.uk> > > > > Currently the input frequency of the SoC is hardcoded in the SoC specific > > da850.c file to 24 MHz. Since the SoC accepts input frequencies in a wide > > range from 12 to 50 MHz, boards with different oscillator/crystal > > frequencies may be built. > > > > This patch allows setting a different input frequency in the board > > specific files to support boards with oscillator/crystal frequencies other > > than 24 MHz. > > > > Signed-off-by: Bob Dunlop <bob.dunlop@xyzzy.org.uk> > > Signed-off-by: Christian Riesch <christian.riesch@omicron.at> > > Why not allow board code to just do a clk_set_rate()? > > Currently the ref_clk struct clk does not have a .set_rate method > implemented, but that should be easy enough to add. > > Then the default ref_clk.rate would stay the 24MHz, but any boards that > want to override that simply use clk_get(), clk_set_rate(), clk_put() That's certainly much more elegant, but this would mean the whole clock tree is traversed again on each boot. I am doing some measurements to see if there is any big difference in boot-time if this is done. Will get back with results. Thanks, Sekhar
"Nori, Sekhar" <nsekhar@ti.com> writes: > Hi Kevin, > > On Tue, Jun 07, 2011 at 04:14:59, Hilman, Kevin wrote: >> Christian Riesch <christian.riesch@omicron.at> writes: >> >> > From: Bob Dunlop <bob.dunlop@xyzzy.org.uk> >> > >> > Currently the input frequency of the SoC is hardcoded in the SoC specific >> > da850.c file to 24 MHz. Since the SoC accepts input frequencies in a wide >> > range from 12 to 50 MHz, boards with different oscillator/crystal >> > frequencies may be built. >> > >> > This patch allows setting a different input frequency in the board >> > specific files to support boards with oscillator/crystal frequencies other >> > than 24 MHz. >> > >> > Signed-off-by: Bob Dunlop <bob.dunlop@xyzzy.org.uk> >> > Signed-off-by: Christian Riesch <christian.riesch@omicron.at> >> >> Why not allow board code to just do a clk_set_rate()? >> >> Currently the ref_clk struct clk does not have a .set_rate method >> implemented, but that should be easy enough to add. >> >> Then the default ref_clk.rate would stay the 24MHz, but any boards that >> want to override that simply use clk_get(), clk_set_rate(), clk_put() > > That's certainly much more elegant, but this would mean the whole > clock tree is traversed again on each boot. > > I am doing some measurements to see if there is any big difference > in boot-time if this is done. Will get back with results. I don't expect this to be a big boot-time impact. However, some of the clock.c assumptions might need to be updated as it currently is written from the perspective that the PLL clocks are the "root" clocks. Setting (and propagating) clock rates is what the clock framework is for, so adding a new interface to set a custom clock rate just doesn't seem right. I understand that the reference oscillator might be considered a special case, but if this can be done with the clock framework, it is much preferred. Kevin
On Tue, Jun 07, 2011 at 21:53:59, Hilman, Kevin wrote: > I don't expect this to be a big boot-time impact. You are right. Using PRINTK_TIME on DM365 I saw no noticeable boot-time change. When I profiled the code using do_gettimeofday(), I saw it was taking ~95 usecs to complete the propagation. I was mainly worried about all the recursion and reading of PLL and sysclk registers. Seems its not so bad. > However, some of the clock.c assumptions might need to be updated as it > currently is written from the perspective that the PLL clocks are the > "root" clocks. Hmm, just calling clk_set_rate() on refclk propagated the rate nicely across the tree. It seems DaVinci clock code is not in such a bad shape :) Or did I miss the concern? > Setting (and propagating) clock rates is what the clock framework is > for, so adding a new interface to set a custom clock rate just doesn't > seem right. I understand that the reference oscillator might be > considered a special case, but if this can be done with the clock > framework, it is much preferred. Okay. Will modify the DM6467/T EVM code to use this method instead. Thanks, Sekhar
Hi Kevin, On Tue, Jun 7, 2011 at 12:44 AM, Kevin Hilman <khilman@ti.com> wrote: > Christian Riesch <christian.riesch@omicron.at> writes: > >> From: Bob Dunlop <bob.dunlop@xyzzy.org.uk> >> >> Currently the input frequency of the SoC is hardcoded in the SoC specific >> da850.c file to 24 MHz. Since the SoC accepts input frequencies in a wide >> range from 12 to 50 MHz, boards with different oscillator/crystal >> frequencies may be built. >> >> This patch allows setting a different input frequency in the board >> specific files to support boards with oscillator/crystal frequencies other >> than 24 MHz. >> >> Signed-off-by: Bob Dunlop <bob.dunlop@xyzzy.org.uk> >> Signed-off-by: Christian Riesch <christian.riesch@omicron.at> > > Why not allow board code to just do a clk_set_rate()? I'm fine with this method (In fact it was the first thing that I tried, I added a .set_rate method to ref_clk, it worked well for me). However I wonder whether first initializing the clock with the wrong value (24 MHz) and later correcting it via clk_set_rate() would break something. In the meantime, the data in the clock tree do not reflect the actual frequencies that are present on the SoC. Christian
Hi Christian, On Thu, Jun 09, 2011 at 15:35:12, Christian Riesch wrote: > Hi Kevin, > > On Tue, Jun 7, 2011 at 12:44 AM, Kevin Hilman <khilman@ti.com> wrote: > > Christian Riesch <christian.riesch@omicron.at> writes: > > > >> From: Bob Dunlop <bob.dunlop@xyzzy.org.uk> > >> > >> Currently the input frequency of the SoC is hardcoded in the SoC specific > >> da850.c file to 24 MHz. Since the SoC accepts input frequencies in a wide > >> range from 12 to 50 MHz, boards with different oscillator/crystal > >> frequencies may be built. > >> > >> This patch allows setting a different input frequency in the board > >> specific files to support boards with oscillator/crystal frequencies other > >> than 24 MHz. > >> > >> Signed-off-by: Bob Dunlop <bob.dunlop@xyzzy.org.uk> > >> Signed-off-by: Christian Riesch <christian.riesch@omicron.at> > > > > Why not allow board code to just do a clk_set_rate()? > > I'm fine with this method (In fact it was the first thing that I > tried, I added a .set_rate method to ref_clk, it worked well for me). > However I wonder whether first initializing the clock with the wrong > value (24 MHz) and later correcting it via clk_set_rate() would break > something. In the meantime, the data in the clock tree do not reflect > the actual frequencies that are present on the SoC. Doing this right after <soc>_init() should be safe. The board should not assume clocks to be setup before this call is made. Thanks, Sekhar
diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index a7b41bf..8984096 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -42,6 +42,8 @@ #include <mach/aemif.h> #include <mach/spi.h> +#define DA850_EVM_REF_FREQ 24000000 + #define DA850_EVM_PHY_ID "0:00" #define DA850_LCD_PWR_PIN GPIO_TO_PIN(2, 8) #define DA850_LCD_BL_PIN GPIO_TO_PIN(2, 15) @@ -1252,7 +1254,7 @@ console_initcall(da850_evm_console_init); static void __init da850_evm_map_io(void) { - da850_init(); + da850_init(DA850_EVM_REF_FREQ); } MACHINE_START(DAVINCI_DA850_EVM, "DaVinci DA850/OMAP-L138/AM18x EVM") diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c index 606a6f2..58fdabb 100644 --- a/arch/arm/mach-davinci/board-mityomapl138.c +++ b/arch/arm/mach-davinci/board-mityomapl138.c @@ -29,6 +29,8 @@ #include <mach/mux.h> #include <mach/spi.h> +#define MITYOMAPL138_REF_FREQ 24000000 + #define MITYOMAPL138_PHY_ID "" #define FACTORY_CONFIG_MAGIC 0x012C0138 @@ -561,7 +563,7 @@ console_initcall(mityomapl138_console_init); static void __init mityomapl138_map_io(void) { - da850_init(); + da850_init(MITYOMAPL138_REF_FREQ); } MACHINE_START(MITYOMAPL138, "MityDSP-L138/MityARM-1808") diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c index 67c38d0..27dd43c 100644 --- a/arch/arm/mach-davinci/board-omapl138-hawk.c +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c @@ -21,6 +21,8 @@ #include <mach/da8xx.h> #include <mach/mux.h> +#define HAWKBOARD_REF_FREQ 24000000 + #define HAWKBOARD_PHY_ID "0:07" #define DA850_HAWK_MMCSD_CD_PIN GPIO_TO_PIN(3, 12) #define DA850_HAWK_MMCSD_WP_PIN GPIO_TO_PIN(3, 13) @@ -334,7 +336,7 @@ console_initcall(omapl138_hawk_console_init); static void __init omapl138_hawk_map_io(void) { - da850_init(); + da850_init(HAWKBOARD_REF_FREQ); } MACHINE_START(OMAPL138_HAWKBOARD, "AM18x/OMAP-L138 Hawkboard") diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c index 133aac4..0bcdc34 100644 --- a/arch/arm/mach-davinci/da850.c +++ b/arch/arm/mach-davinci/da850.c @@ -39,8 +39,6 @@ #define DA850_TIMER64P2_BASE 0x01f0c000 #define DA850_TIMER64P3_BASE 0x01f0d000 -#define DA850_REF_FREQ 24000000 - #define CFGCHIP3_ASYNC3_CLKSRC BIT(4) #define CFGCHIP3_PLL1_MASTER_LOCK BIT(5) #define CFGCHIP0_PLL_MASTER_LOCK BIT(4) @@ -57,7 +55,6 @@ static struct pll_data pll0_data = { static struct clk ref_clk = { .name = "ref_clk", - .rate = DA850_REF_FREQ, }; static struct clk pll0_clk = { @@ -1104,10 +1101,12 @@ static struct davinci_soc_info davinci_soc_info_da850 = { .reset_device = &da8xx_wdt_device, }; -void __init da850_init(void) +void __init da850_init(unsigned long ref_clk_rate) { unsigned int v; + ref_clk.rate = ref_clk_rate; + davinci_common_init(&davinci_soc_info_da850); da8xx_syscfg0_base = ioremap(DA8XX_SYSCFG0_BASE, SZ_4K); diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h index ad64da7..e8e704e 100644 --- a/arch/arm/mach-davinci/include/mach/da8xx.h +++ b/arch/arm/mach-davinci/include/mach/da8xx.h @@ -70,7 +70,7 @@ extern unsigned int da850_max_speed; #define DA8XX_ARM_RAM_BASE 0xffff0000 void __init da830_init(void); -void __init da850_init(void); +void __init da850_init(unsigned long ref_clk_rate); int da830_register_edma(struct edma_rsv_info *rsv); int da850_register_edma(struct edma_rsv_info *rsv[2]);
Currently the input frequency of the SoC is hardcoded in the SoC specific da850.c file to 24 MHz. Since the SoC accepts input frequencies in a wide range from 12 to 50 MHz, boards with different oscillator/crystal frequencies may be built. This patch moves the definition of the input frequency to the board specific files to support boards with oscillator/crystal frequencies other than 24 MHz. Signed-off-by: Christian Riesch <christian.riesch@omicron.at> --- arch/arm/mach-davinci/board-da850-evm.c | 4 +++- arch/arm/mach-davinci/board-mityomapl138.c | 4 +++- arch/arm/mach-davinci/board-omapl138-hawk.c | 4 +++- arch/arm/mach-davinci/da850.c | 7 +++---- arch/arm/mach-davinci/include/mach/da8xx.h | 2 +- 5 files changed, 13 insertions(+), 8 deletions(-)