[PATCH 1/4] cpufreq: Add a cpufreq driver for Marvell Dove
Viresh Kumar
viresh.kumar at linaro.org
Tue Oct 22 05:01:03 EDT 2013
On 19 October 2013 18:07, Andrew Lunn <andrew at lunn.ch> wrote:
> +config ARM_DOVE_CPUFREQ
> + def_bool ARCH_DOVE && OF
> + select CPU_FREQ_TABLE
Refer:
3bc28ab cpufreq: remove CONFIG_CPU_FREQ_TABLE
> + help
> + This adds the CPUFreq driver for Marvell Dove
> + SoCs.
Above can come in one line?
> +
> config ARM_DT_BL_CPUFREQ
> tristate "Generic probing via DT for ARM big LITTLE CPUfreq driver"
> depends on ARM_BIG_LITTLE_CPUFREQ && OF
> diff --git a/drivers/cpufreq/dove-cpufreq.c b/drivers/cpufreq/dove-cpufreq.c
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
why do you need this one?
> +#include <linux/cpufreq.h>
> +#include <linux/interrupt.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/of_irq.h>
Can you add them in ascending order please?
> +#include <asm/proc-fns.h>
> +
> +#define DFS_CR 0x00
> +#define DFS_EN BIT(0)
> +#define CPU_SLOW_EN BIT(1)
> +#define L2_RATIO_OFFS 9
> +#define L2_RATIO_MASK (0x3F << L2_RATIO_OFFS)
> +#define DFS_SR 0x04
> +#define CPU_SLOW_MODE_STTS BIT(1)
> +
> +/* PMU_CR */
> +#define MASK_FIQ BIT(28)
> +#define MASK_IRQ BIT(24) /* PMU_CR */
Use space after #define instead of tabs. And then use tabs
consistently after Macro name to keep macro values aligned.
> +/* CPU Clock Divider Control 0 Register */
> +#define DPRATIO_OFFS 24
> +#define DPRATIO_MASK (0x3F << DPRATIO_OFFS)
> +#define XPRATIO_OFFS 16
> +#define XPRATIO_MASK (0x3F << XPRATIO_OFFS)
Here as well.. aligning with earlier macro's might look better.
> +static struct priv
> +{
> + struct clk *cpu_clk;
> + struct clk *ddr_clk;
> + struct device *dev;
> + unsigned long dpratio;
> + unsigned long xpratio;
> + void __iomem *dfs;
> + void __iomem *pmu_cr;
> + void __iomem *pmu_clk_div;
> +} priv;
> +
> +#define STATE_CPU_FREQ 0x01
> +#define STATE_DDR_FREQ 0x02
> +
> +/*
> + * Dove can swap the clock to the CPU between two clocks:
> + *
> + * - cpu clk
> + * - ddr clk
> + *
> + * The frequencies are set at runtime before registering this
> + * table.
> + */
> +static struct cpufreq_frequency_table dove_freq_table[] = {
> + {STATE_CPU_FREQ, 0}, /* CPU uses cpuclk */
> + {STATE_DDR_FREQ, 0}, /* CPU uses ddrclk */
> + {0, CPUFREQ_TABLE_END},
> +};
> +
> +static unsigned int dove_cpufreq_get_cpu_frequency(unsigned int cpu)
> +{
> + unsigned long reg = readl_relaxed(priv.dfs + DFS_SR);
> +
> + if (reg & CPU_SLOW_MODE_STTS)
> + return dove_freq_table[1].frequency;
> + return dove_freq_table[0].frequency;
> +}
> +
> +static void dove_cpufreq_set_cpu_state(struct cpufreq_policy *policy,
> + unsigned int index)
> +{
> + struct cpufreq_freqs freqs;
> + unsigned int state = dove_freq_table[index].driver_data;
> + unsigned long reg, cr;
> +
> + freqs.old = dove_cpufreq_get_cpu_frequency(0);
> + freqs.new = dove_freq_table[index].frequency;
> +
> + cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE);
> +
> + if (freqs.old != freqs.new) {
If this is false, why should we start sending notifications?
> + local_irq_disable();
> +
> + /* Mask IRQ and FIQ to CPU */
> + cr = readl(priv.pmu_cr);
> + cr |= MASK_IRQ | MASK_FIQ;
> + writel(cr, priv.pmu_cr);
> +
> + /* Set/Clear the CPU_SLOW_EN bit */
> + reg = readl_relaxed(priv.dfs + DFS_CR);
> + reg &= ~L2_RATIO_MASK;
> +
> + switch (state) {
> + case STATE_CPU_FREQ:
> + reg |= priv.xpratio;
> + reg &= ~CPU_SLOW_EN;
> + break;
> + case STATE_DDR_FREQ:
> + reg |= (priv.dpratio | CPU_SLOW_EN);
> + break;
> + }
> +
> + /* Start the DFS process */
> + reg |= DFS_EN;
> +
> + writel(reg, priv.dfs + DFS_CR);
> +
> + /* Wait-for-Interrupt, while the hardware changes frequency */
> + cpu_do_idle();
> +
> + local_irq_enable();
> + }
> + cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
> +}
> +
> +static int dove_cpufreq_target(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + unsigned int relation)
there are few patches floating in making target light weight.. This part will
change significantly ones those are in..
> +{
> + unsigned int index = 0;
> +
> + if (cpufreq_frequency_table_target(policy, dove_freq_table,
> + target_freq, relation, &index))
> + return -EINVAL;
> +
> + dove_cpufreq_set_cpu_state(policy, index);
> +
> + return 0;
> +}
> +
> +static int dove_cpufreq_cpu_init(struct cpufreq_policy *policy)
> +{
> + return cpufreq_generic_init(policy, dove_freq_table, 5000);
> +}
> +
> +static irqreturn_t dove_cpufreq_irq(int irq, void *dev)
> +{
> + return IRQ_HANDLED;
> +}
what is this for?
> +static struct cpufreq_driver dove_cpufreq_driver = {
> + .get = dove_cpufreq_get_cpu_frequency,
> + .verify = cpufreq_generic_frequency_table_verify,
> + .target = dove_cpufreq_target,
> + .init = dove_cpufreq_cpu_init,
> + .exit = cpufreq_generic_exit,
> + .name = "dove-cpufreq",
> + .attr = cpufreq_generic_attr,
> +};
> +
> +static int dove_cpufreq_probe(struct platform_device *pdev)
> +{
> + struct device_node *np;
> + struct resource *res;
> + int err;
> + int irq;
Above two can be merged.
> + priv.dev = &pdev->dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + priv.dfs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(priv.dfs))
> + return PTR_ERR(priv.dfs);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + priv.pmu_cr = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(priv.pmu_cr))
> + return PTR_ERR(priv.pmu_cr);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> + priv.pmu_clk_div = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(priv.pmu_clk_div))
> + return PTR_ERR(priv.pmu_clk_div);
> +
> + np = of_find_node_by_path("/cpus/cpu at 0");
> + if (!np)
> + return -ENODEV;
> +
> + priv.cpu_clk = of_clk_get_by_name(np, "cpu_clk");
> + if (IS_ERR(priv.cpu_clk)) {
> + dev_err(priv.dev, "Unable to get cpuclk");
> + return PTR_ERR(priv.cpu_clk);
> + }
> +
> + clk_prepare_enable(priv.cpu_clk);
this can fail..
> + dove_freq_table[0].frequency = clk_get_rate(priv.cpu_clk) / 1000;
> +
> + priv.ddr_clk = of_clk_get_by_name(np, "ddrclk");
> + if (IS_ERR(priv.ddr_clk)) {
> + dev_err(priv.dev, "Unable to get ddrclk");
> + err = PTR_ERR(priv.ddr_clk);
> + goto out_cpu;
> + }
> +
> + clk_prepare_enable(priv.ddr_clk);
and so can this..
More information about the linux-arm-kernel
mailing list