[3/3] davinci: DA850/OMAP-L138: add CPUFreq support

Sekhar Nori <nsekhar@ti.com> writes:

> add basic CPUFreq support for DA850/OMAP-L138
>
> Currently, frequency scaling only on PLL0 is supported. No scaling of PLL1
> or voltage levels as yet.
>
> Peripherals like MMC/SD which have a clock input synchronous with
> ARM clock will not work well since the clock will change behind their backs.
> Support for notification to such devices to adjust themselves to the
> new frequency will be added in later patches. Current defconfigs keep
> CPUFreq disabled so it will not affect normal operation.
>
> The patch moves Async3 clock source to PLL1 so that frequency scaling
> on PLL0 does not affect those peripherals. Without this the console on UART2
> goes for a toss the moment CPUFreq kicks in.

Can you break this change of ASYNC3 modules out into a separate patch?

Also, I'd rather see this done in a function in da850.c that changes
the parents of these clocks instead of changing them statically.  This
way, when other clocks in ASYNC3 are added, this function will need to
be updated.

Maybe start with something like this (on top of this series, but not
even compile tested):

...then, in each ASYNC3 clock, set that flag.  Then add a function to
change the parents by DA8XX_CFGCHIP3_REG and changing the parent of
every clock that has the DA850_CLK_ASYNC3 flag.

This function should be able to change it back to the default setting
as well.

This way, as the other clocks in ASYNC3 are added (McASP, SPI, etc.)
all that is needed is to make sure they have the DA850_CLK_ASYNC3 flag
set.

> The OPP defintions assume clock input of 24MHz to the SoC. This is inline
> with hardcoding of input frequency in the <soc>.c files. At some point
> this will need to move into board dependent code as boards appear with
> different input clock.
>
> Tested with ondemand governer and a shell script to vary processor load.
>
> Note: This patch also does an off-topic change of making the JTAG id register
> definition derive from SYSCFG base address.

Even though it's trivial, this part should also be broken out into a
separate patch as it is unrelated to $SUBJECT.

> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
>  arch/arm/mach-davinci/da850.c              |  146 +++++++++++++++++++++++++++-
>  arch/arm/mach-davinci/include/mach/da8xx.h |    4 +-
>  2 files changed, 147 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index 4a43ae2..fe71f13 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
> @@ -15,6 +15,7 @@
>  #include <linux/init.h>
>  #include <linux/clk.h>
>  #include <linux/platform_device.h>
> +#include <linux/cpufreq.h>
>  
>  #include <asm/mach/map.h>
>  
> @@ -35,6 +36,8 @@
>  #define DA850_TIMER64P3_BASE	0x01f0d000
>  
>  #define DA850_REF_FREQ		24000000
> +static int da850_set_armrate(struct clk *clk, unsigned long rate);
> +static int da850_round_armrate(struct clk *clk, unsigned long rate);
>  
>  static struct pll_data pll0_data = {
>  	.num		= 1,
> @@ -230,14 +233,14 @@ static struct clk uart0_clk = {
>  
>  static struct clk uart1_clk = {
>  	.name		= "uart1",
> -	.parent		= &pll0_sysclk2,
> +	.parent		= &pll1_sysclk2,
>  	.lpsc		= DA8XX_LPSC1_UART1,
>  	.psc_ctlr	= 1,
>  };
>  
>  static struct clk uart2_clk = {
>  	.name		= "uart2",
> -	.parent		= &pll0_sysclk2,
> +	.parent		= &pll1_sysclk2,
>  	.lpsc		= DA8XX_LPSC1_UART2,
>  	.psc_ctlr	= 1,
>  };
> @@ -276,6 +279,8 @@ static struct clk arm_clk = {
>  	.parent		= &pll0_sysclk6,
>  	.lpsc		= DA8XX_LPSC0_ARM,
>  	.flags		= ALWAYS_ENABLED,
> +	.set_rate	= da850_set_armrate,
> +	.round_rate	= da850_round_armrate,
>  };
>  
>  static struct clk rmii_clk = {
> @@ -601,6 +606,65 @@ static struct davinci_timer_info da850_timer_info = {
>  	.clocksource_id	= T0_TOP,
>  };
>  
> +#define MAX_NUM_OPP	3
> +
> +/*
> + * Notes:
> + * According to the TRM, minimum PLLM results in max power savings.
> + * The OPP definitions below should keep the PLLM as low as possible.
> + *
> + * The output of the PLLM must be between 400 to 600 MHz.
> + * This rules out prediv of anything but divide-by-one for 24Mhz OSC input.
> + */
> +struct da850_opp {
> +	unsigned int	freq;	/* in KHz */
> +	unsigned int	prediv;
> +	unsigned int	mult;
> +	unsigned int	postdiv;
> +};
> +
> +static const struct da850_opp da850_opp_300 = {
> +	.freq		= 300000,
> +	.prediv		= 1,
> +	.mult		= 25,
> +	.postdiv	= 2,
> +};
> +
> +static const struct da850_opp da850_opp_200 = {
> +	.freq		= 200000,
> +	.prediv		= 1,
> +	.mult		= 25,
> +	.postdiv	= 3,
> +};
> +
> +static const struct da850_opp da850_opp_96 = {
> +	.freq		= 96000,
> +	.prediv		= 1,
> +	.mult		= 20,
> +	.postdiv	= 5,
> +};
> +
> +#define OPP(freq) 		\
> +	{				\
> +		.index = (unsigned int) &da850_opp_##freq,	\
> +		.frequency = freq * 1000, \
> +	}
> +
> +static struct cpufreq_frequency_table da850_freq_table[MAX_NUM_OPP+1] = {

Looks like MAX_NUM_OPP is only used here, but is not necessary.  Just
use da850_freq_table[] = ...  and use ARRAY_SIZE() if needed
elsewhere.

> +	OPP(300),
> +	OPP(200),
> +	OPP(96),
> +	{
> +		.index		= 0,
> +		.frequency	= CPUFREQ_TABLE_END,
> +	},
> +};
> +
> +void da850_init_cpufreq_table(struct cpufreq_frequency_table **table)
> +{
> +	*table = &da850_freq_table[0];
> +}
> +
>  static struct davinci_soc_info davinci_soc_info_da850 = {
>  	.io_desc		= da850_io_desc,
>  	.io_desc_num		= ARRAY_SIZE(da850_io_desc),
> @@ -623,9 +687,87 @@ static struct davinci_soc_info davinci_soc_info_da850 = {
>  	.gpio_irq		= IRQ_DA8XX_GPIO0,
>  	.serial_dev		= &da8xx_serial_device,
>  	.emac_pdata		= &da8xx_emac_pdata,
> +	.init_cpufreq_table	= da850_init_cpufreq_table,
>  };
>  
> +static int da850_round_armrate(struct clk *clk, unsigned long rate)
> +{
> +	int i, ret = 0, diff;
> +	unsigned int best = (unsigned int) -1;
> +
> +	rate /= 1000; /* convert to kHz */
> +
> +	for (i = 0; da850_freq_table[i].frequency != CPUFREQ_TABLE_END; i++) {
> +		diff = da850_freq_table[i].frequency - rate;
> +		if (diff < 0)
> +			diff = -diff;
> +
> +		if (diff < best) {
> +			best = diff;
> +			ret = da850_freq_table[i].frequency;
> +		}
> +	}
> +
> +	return ret * 1000;
> +}
> +
> +static int da850_set_armrate(struct clk *clk, unsigned long rate)
> +{
> +	int i, ret;
> +	unsigned int prediv, mult, postdiv;
> +	struct da850_opp *opp;
> +	struct clk *pllclk = &pll0_clk;
> +	struct pll_data *pll = pllclk->pll_data;
> +
> +	/* convert to KHz */
> +	rate /= 1000;
> +
> +	for (i = 0; da850_freq_table[i].frequency != CPUFREQ_TABLE_END; i++) {
> +		if (rate == da850_freq_table[i].frequency)
> +			break;
> +	}
> +
> +	if (da850_freq_table[i].frequency == CPUFREQ_TABLE_END) {
> +		printk(KERN_WARNING "%s: Unsupported ARM clock rate %ld\n",
> +								__func__, rate);
> +		return -EINVAL;
> +	}
> +
> +	opp = (struct da850_opp *) da850_freq_table[i].index;
> +	prediv = opp->prediv;
> +	mult = opp->mult;
> +	postdiv = opp->postdiv;
> +
> +	/* Unlock writing to PLL registers */
> +	__raw_writel(__raw_readl(IO_ADDRESS(DA8XX_CFGCHIP0_REG)) & ~BIT(4),
> +			IO_ADDRESS(DA8XX_CFGCHIP0_REG));

Minor nit, and probably personal preference, but IMHO, it's a bit
cleaner to read if it looks like:

      v = __raw_read(...);
      v & ~CFGCHIP0_PLL_MASTER_LOCK;
      __raw_rite(v, ...); 

Where CFGCHIP0_PLL_MASTER_LOCK is defined towards the top of da850.c.

> +
> +	ret = davinci_set_pllrate(pll, prediv, mult, postdiv);
> +	if (ret)

Maybe a WARN() here?

> +		return ret;
> +
> +	/* Propogate new rate */
> +	pllclk->rate = pllclk->parent->rate;
> +	pllclk->rate /= prediv;
> +	pllclk->rate *= mult;
> +	pllclk->rate /= postdiv;
> +
> +	/* FIXME: unnecessarily re-calculates rates for PLL1 as well */
> +	davinci_clk_recalc_rates(da850_clks);

Hmm, possible update for PATCH 1/3, I'll hold off on pushing 1/3 until
I hear back.

maybe have another flag in the PLL which says that one of the child
clocks has been changed.  Any calls to clk_set_rate() or set_pllrate()
will set that flag in the PLL.  If that flag is not set, then
recalc_rates() can check if that flag is set and only recalc rates as
necessary.

> +	return 0;
> +}
> +
>  void __init da850_init(void)
>  {
>  	davinci_common_init(&davinci_soc_info_da850);
> +
> +	/*
> +	 * set Async3 clock source to come from PLL1 so PLL0 frequency changes
> +	 * dont affect as many peripherals as possible.
> +	 * Note: This changes the clock source for UART1/2. So, low level debug
> +	 * will garble beyond this point if the source rate changes.
> +	 */
> +	__raw_writel(__raw_readl(IO_ADDRESS(DA8XX_CFGCHIP3_REG)) | BIT(4),
> +						IO_ADDRESS(DA8XX_CFGCHIP3_REG));

I'd prefer:

      v = __raw_read(...)
      v |= CFGCHIP3_ASYNC3_CLKSRC;
      __raw_rite(v, ...); 

Where CFGCHIP3_ASYNC3_CLKSRC is defined towards the top of da850.c.

>  }
> diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
> index a8cb570..41ab824 100644
> --- a/arch/arm/mach-davinci/include/mach/da8xx.h
> +++ b/arch/arm/mach-davinci/include/mach/da8xx.h
> @@ -28,10 +28,12 @@
>  #define DA8XX_CP_INTC_VIRT	(IO_VIRT - DA8XX_CP_INTC_SIZE - SZ_4K)
>  
>  #define DA8XX_BOOT_CFG_BASE	(IO_PHYS + 0x14000)
> +#define DA8XX_JTAG_ID_REG	(DA8XX_BOOT_CFG_BASE + 0x18)
> +#define DA8XX_CFGCHIP0_REG	(DA8XX_BOOT_CFG_BASE + 0x17c)
> +#define DA8XX_CFGCHIP3_REG	(DA8XX_BOOT_CFG_BASE + 0x188)
>
>  #define DA8XX_PSC0_BASE		0x01c10000
>  #define DA8XX_PLL0_BASE		0x01c11000
> -#define DA8XX_JTAG_ID_REG	0x01c14018

Again, this really belongs in a separate simple patch.

>  #define DA8XX_TIMER64P0_BASE	0x01c20000
>  #define DA8XX_TIMER64P1_BASE	0x01c21000
>  #define DA8XX_GPIO_BASE		0x01e26000
> -- 
> 1.6.2.4
>
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source@linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

[3/3] davinci: DA850/OMAP-L138: add CPUFreq support

Commit Message

Comments

Patch