See my comments below
2010/6/7 Raffaele Recalcati <[email protected]>:
> From: Davide Bonfanti <[email protected]>
> Date: Thu, 3 Jun 2010 14:26:26 +0200
> Subject: [PATCH 10/12] DaVinci: dm365: Added clokout2 management
>
> Signed-off-by: Raffaele Recalcati <[email protected]>
> ---
> arch/arm/mach-davinci/clock.c | 62
> ++++++++++++++++++++++++++++
> arch/arm/mach-davinci/clock.h | 6 +++
> arch/arm/mach-davinci/dm365.c | 58 ++++++++++++++++++++++++-
> arch/arm/mach-davinci/include/mach/clock.h | 3 +
> 4 files changed, 126 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
> index 054c303..f561b73 100644
> --- a/arch/arm/mach-davinci/clock.c
> +++ b/arch/arm/mach-davinci/clock.c
> @@ -144,6 +144,68 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
> }
> EXPORT_SYMBOL(clk_set_rate);
>
> +
> +int clkout_set_rate(struct clk *clk, unsigned long rate)
> +{
> + unsigned long flags;
> + int ret = -EINVAL;
> + int i, err, min_err, i_min_err;
> + u32 regval;
> + struct pll_data *pll;
> +
> +
> + if (clk == NULL || IS_ERR(clk))
> + return ret;
> + pll = clk->parent->pll_data;
> + regval = __raw_readl(IO_ADDRESS(clk->sec_div_reg));
> +
> + if (clk->sec_div_reg)
> + {
> + i_min_err = min_err = INT_MAX;
> + for (i=clk->sec_div_reg_max; i>0; i--)
> + {
> + if (clk->set_rate)
> + {
> + ret = clk_set_rate(clk, rate * i) ;
> + err = clk_get_rate(clk) - rate * i;
> + if (abs(min_err) > abs(err))
> + {
> + min_err = err;
> + i_min_err = i;
> + }
> + }
> + }
> + i = i_min_err;
> + ret = clk->set_rate(clk, rate * i) ;
> +
> + regval &= ~(clk->sec_div_reg_max << clk->sec_div_reg_shift);
> + regval |= (i-1) << clk->sec_div_reg_shift;
> + __raw_writel(regval, IO_ADDRESS(clk->sec_div_reg));
> + regval = __raw_readl( IO_ADDRESS(clk->out_enable_reg));
> + regval |= 1 << clk->out_enable_bit;
> + __raw_writel(regval, IO_ADDRESS(clk->out_enable_reg));
> + rate *= i;
> + }
> + else
> + {
> + if (clk->set_rate)
> + ret = clk->set_rate(clk, rate);
> + }
> +
> + spin_lock_irqsave(&clockfw_lock, flags);
> + if (ret == 0) {
> + if (clk->recalc)
> + clk->rate = clk->recalc(clk);
You end up with the syclk9 rate, since clk->recalc is clk_syclk_recalc.
> + propagate_rate(clk);
> + regval &= ~(1 << clk->out_enable_bit);
> + __raw_writel(regval, IO_ADDRESS(clk->out_enable_reg));
> + }
> + spin_unlock_irqrestore(&clockfw_lock, flags);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(clkout_set_rate);
> +
> int clk_set_parent(struct clk *clk, struct clk *parent)
> {
> unsigned long flags;
> diff --git a/arch/arm/mach-davinci/clock.h b/arch/arm/mach-davinci/clock.h
> index 01e3648..1ca14e7 100644
> --- a/arch/arm/mach-davinci/clock.h
> +++ b/arch/arm/mach-davinci/clock.h
> @@ -95,6 +95,12 @@ struct clk {
> struct list_head childnode; /* parent's child list node */
> struct pll_data *pll_data;
> u32 div_reg;
> + u32 sec_div_reg;
> + u32 sec_div_reg_max;
> + u32 sec_div_reg_shift;
> + u32 out_enable_reg;
> + u32 out_enable_bit;
> +
div reg is an offset with respect to pll base,
and sec_div_reg is not. is adding five u32 for all clock really a good
idea, when
only one clock actually uses them ?
> unsigned long (*recalc) (struct clk *);
> int (*set_rate) (struct clk *clk, unsigned long rate);
> int (*round_rate) (struct clk *clk, unsigned long rate);
> diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
> index e4ead0a..9968c37 100644
> --- a/arch/arm/mach-davinci/dm365.c
> +++ b/arch/arm/mach-davinci/dm365.c
> @@ -40,6 +40,21 @@
> #include "mux.h"
>
> #define DM365_REF_FREQ 24000000 /* 24 MHz on the DM365 EVM */
> +#define PINMUX0 0x00
> +#define PINMUX1 0x04
> +#define PINMUX2 0x08
> +#define PINMUX3 0x0c
> +#define PINMUX4 0x10
> +#define INTMUX 0x18
> +#define EVTMUX 0x1c
> +#define PERI_CLKCTL 0x48
> +#define CLOCKOUT2EN 2
> +#define CLOCKOUT1EN 1
> +#define CLOCKOUT0EN 0
> +
> +
> +int dm365_set_pll1_rate(struct clk *clk, unsigned long rate);
> +
>
> static struct pll_data pll1_data = {
> .num = 1,
> @@ -147,6 +162,20 @@ static struct clk pll1_sysclk9 = {
> .div_reg = PLLDIV9,
> };
>
> +static struct clk clkout2_clk = {
> + .name = "clkout2",
> + .parent = &pll1_clk,
> + .flags = CLK_PLL,
> + .div_reg = PLLDIV9,
> + .sec_div_reg = DAVINCI_SYSTEM_MODULE_BASE + PERI_CLKCTL,
> + .sec_div_reg_max = 0x0F,
> + .sec_div_reg_shift = 3,
> + .out_enable_reg = DAVINCI_SYSTEM_MODULE_BASE + PERI_CLKCTL,
> + .out_enable_bit = CLOCKOUT2EN,
> + .set_rate = dm365_set_pll1_rate,
> +};
> +
> +
> static struct clk pll2_clk = {
> .name = "pll2",
> .parent = &ref_clk,
> @@ -421,6 +450,7 @@ static struct clk_lookup dm365_clks[] = {
> CLK(NULL, "pll1_sysclk7", &pll1_sysclk7),
> CLK(NULL, "pll1_sysclk8", &pll1_sysclk8),
> CLK(NULL, "pll1_sysclk9", &pll1_sysclk9),
> + CLK(NULL, "clkout2", &clkout2_clk),
pll1_sysclk9 and clkout2 are the same clock, so pll1_sysclk9
shoud be removed, or clkout2 should have pll1_sysclk9 has parent.
I think clkout2 should be a child of pll1_syclk9.
> CLK(NULL, "pll2", &pll2_clk),
> CLK(NULL, "pll2_aux", &pll2_aux_clk),
> CLK(NULL, "clkout1", &clkout1_clk),
> @@ -467,9 +497,6 @@ static struct clk_lookup dm365_clks[] = {
>
> /*----------------------------------------------------------------------*/
>
> -#define INTMUX 0x18
> -#define EVTMUX 0x1c
> -
>
> static const struct mux_config dm365_pins[] = {
> #ifdef CONFIG_DAVINCI_MUX
> @@ -657,6 +684,31 @@ static struct resource dm365_spi0_resources[] = {
> },
> };
>
> +int dm365_set_pll1_rate(struct clk *clk, unsigned long rate)
> +{
> + unsigned int prediv, mult, postdiv;
> + u32 clk_freq_min, clk_freq_max, plldiv;
> +
> + prediv = ioread32(IO_ADDRESS(DAVINCI_PLL1_BASE + PREDIV)) & 0x1F;
> + mult = ioread32(IO_ADDRESS(DAVINCI_PLL1_BASE + PLLM)) & 0x3FF;
> + postdiv = ioread32(IO_ADDRESS(DAVINCI_PLL1_BASE + POSTDIV)) & 0x1F;
> +
> + clk_freq_max = DM365_REF_FREQ * 2 * mult / ( (prediv + 1) * (postdiv +
> 1) );
> + clk_freq_min = clk_freq_max / 0x20;
> +
> + if ( (rate < clk_freq_min) || (rate > clk_freq_max) )
> + return -EINVAL;
> +
> + plldiv = clk_freq_max / rate ;
> + if ( (clk_freq_max % rate) < (rate/2))
> + plldiv--;
> + iowrite32(plldiv & 0x1F, IO_ADDRESS(DAVINCI_PLL1_BASE + clk->div_reg));
> + iowrite32(plldiv | 0x8000, IO_ADDRESS(DAVINCI_PLL1_BASE +
> clk->div_reg));
> + clk->rate = rate;
> +
> + return 0;
> +}
This could be more generic, because this is true for all sysclk.
Moreover, this pll rate calculation is already done in
clk_pllclk_recalc in clock.c
You could use this data by accessing the parent clock.
sysclk clocks have the CLK_PLL flag set have and have a pll for
parent, and this parent has a
pll_data field from which you can infer the pll base and the pll rate :
struct pll_data *pll;
if(!clk->parent->pll_data)
return ???;
pll = clk->parent->pll_data;
clk_freq_max = clk->parent->rate;
clk_freq_min = ...
And at the end :
raw_writel(div_value, pll->base + clk->div_reg)
So in the end , I think the following step would lead to something
more consistent :
- make a set_rate functions for all syclk like clock.
- make clkout2 a child of syclk9
- provide a set_rate function for clkout2 where you use the secdiv etc register,
but without adding field to the clock struct, since this fields are
currently used in a single place.
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
