Re: [PATCH 2/2] cpufreq: add cpufreq driver for Mediatek MT8173 SoC

[Viresh Kumar]

On 9 January 2015 at 15:24, pi-cheng.chen <pi-cheng.chen@xxxxxxxxxx> wrote:
> When doing DVFS on MT8173 SoC, 2 rules should be followed to make sure the
> SoC works properly:
> 1. When doing frequency scaling of CPUs, the clock source should be switched
>    to another PLL, then switched back to the orignal until it's stable.
> 2. When doing voltage scaling of CA57 cluster, Vproc and Vsram need to be
>    controlled concurrently and 2 limitations should be followed:
>    a. Vsram > Vproc
>    b. Vsram - Vproc < 200 mV
>
> To address these needs, we reuse cpufreq-dt but do voltage scaling in the
> cpufreq notifier.

I haven't understood how is all this working..
- How are you switching to intermediate freq when its support isn't there
in cpufreq-dt.
- How is changing of volt after switching to freq working? Specially when freq
is higher than older freq and it requires changing of voltage before freq..

>
> Signed-off-by: pi-cheng.chen <pi-cheng.chen@xxxxxxxxxx>
> ---
>  drivers/cpufreq/Kconfig.arm      |   6 +
>  drivers/cpufreq/Makefile         |   1 +
>  drivers/cpufreq/mt8173-cpufreq.c | 459 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 466 insertions(+)
>  create mode 100644 drivers/cpufreq/mt8173-cpufreq.c
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 0f9a2c3..97ed7dd 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -255,3 +255,9 @@ config ARM_PXA2xx_CPUFREQ
>           This add the CPUFreq driver support for Intel PXA2xx SOCs.
>
>           If in doubt, say N.
> +
> +config ARM_MT8173_CPUFREQ
> +       bool "Mediatek MT8173 CPUFreq support"
> +       depends on ARCH_MEDIATEK && CPUFREQ_DT && REGULATOR
> +       help
> +         This adds the CPUFreq driver support for Mediatek MT8173 SoC.
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index b3ca7b0..67b7f17 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -76,6 +76,7 @@ obj-$(CONFIG_ARM_SA1110_CPUFREQ)      += sa1110-cpufreq.o
>  obj-$(CONFIG_ARM_SPEAR_CPUFREQ)                += spear-cpufreq.o
>  obj-$(CONFIG_ARM_TEGRA_CPUFREQ)                += tegra-cpufreq.o
>  obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
> +obj-$(CONFIG_ARM_MT8173_CPUFREQ)       += mt8173-cpufreq.o

Add this in alphanumeric order please.

>
>  ##################################################################################
>  # PowerPC platform drivers
> diff --git a/drivers/cpufreq/mt8173-cpufreq.c b/drivers/cpufreq/mt8173-cpufreq.c
> new file mode 100644
> index 0000000..b578c10
> --- /dev/null
> +++ b/drivers/cpufreq/mt8173-cpufreq.c
> @@ -0,0 +1,459 @@
> +/*
> +* Copyright (c) 2015 Linaro.
> +* Author: Pi-Cheng Chen <pi-cheng.chen@xxxxxxxxxx>
> +*
> +* This program is free software; you can redistribute it and/or modify
> +* it under the terms of the GNU General Public License version 2 as
> +* published by the Free Software Foundation.
> +*
> +* This program is distributed in the hope that it will be useful,
> +* but WITHOUT ANY WARRANTY; without even the implied warranty of
> +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +* GNU General Public License for more details.
> +*/
> +
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/cpufreq.h>
> +#include <linux/cpufreq-dt.h>
> +#include <linux/cpumask.h>
> +#include <linux/slab.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>

This driver isn't a clock provider, right? Then why are
you including this file?

> +#include <linux/regulator/consumer.h>
> +#include <linux/cpu.h>
> +#include <linux/pm_opp.h>

keep them in alphanumeric order please.

> +
> +#define CPUCLK_MUX_ARMPLL      0x1
> +#define CPUCLK_MUX_MAINPLL     0x2
> +#define VOLT_SHIFT_LIMIT       150000
> +
> +enum {
> +       LITTLE_CLUSTER = 0,
> +       BIG_CLUSTER,
> +       NR_CLUSTERS
> +};
> +
> +static struct cluster_dvfs_info {
> +       int cpu;
> +       struct cpumask cpus;
> +       struct device *cpu_dev;
> +       struct regulator *proc_reg;
> +       struct regulator *sram_reg;
> +       int inter_volt;
> +       u32 volt_tol;
> +} *dvfs_info;
> +
> +static unsigned long mainpll_freq;
> +static void __iomem *clk_mux_regs;
> +static spinlock_t lock;
> +
> +static void cpuclk_mux_set(int cluster, u32 sel)
> +{
> +       u32 val;
> +       u32 mask = 0x3;
> +
> +       if (cluster == BIG_CLUSTER) {
> +               mask <<= 2;
> +               sel <<= 2;
> +       }
> +
> +       spin_lock(&lock);
> +
> +       val = readl(clk_mux_regs);
> +       val = (val & ~mask) | sel;
> +       writel(val, clk_mux_regs);
> +
> +       spin_unlock(&lock);
> +}
> +
> +static int mt8173_cpufreq_voltage_step(struct cluster_dvfs_info *dvfs,
> +                                      unsigned long old_vproc,
> +                                      unsigned long new_vproc)
> +{
> +       int cur_vsram, cur_vproc, target_volt, tol;
> +
> +       if (old_vproc > new_vproc) {
> +               while (1) {
> +                       cur_vsram = regulator_get_voltage(dvfs->sram_reg);
> +                       if (cur_vsram - new_vproc < VOLT_SHIFT_LIMIT &&
> +                           cur_vsram > new_vproc) {
> +                               tol = new_vproc * dvfs->volt_tol / 100;
> +                               regulator_set_voltage_tol(dvfs->proc_reg,
> +                                                         new_vproc, tol);
> +                               break;
> +                       }
> +
> +                       target_volt = cur_vsram - VOLT_SHIFT_LIMIT;
> +                       tol = target_volt * dvfs->volt_tol / 100;
> +                       regulator_set_voltage_tol(dvfs->proc_reg, target_volt,
> +                                                 tol);
> +
> +                       cur_vproc = regulator_get_voltage(dvfs->proc_reg);
> +                       target_volt = cur_vproc + 1;
> +                       tol = target_volt * dvfs->volt_tol / 100;
> +                       regulator_set_voltage_tol(dvfs->sram_reg, target_volt,
> +                                                 tol);
> +               }
> +       } else if (old_vproc < new_vproc) {
> +               while (1) {
> +                       cur_vsram = regulator_get_voltage(dvfs->sram_reg);
> +                       if (cur_vsram - new_vproc < VOLT_SHIFT_LIMIT &&
> +                           cur_vsram > new_vproc) {
> +                               tol = new_vproc * dvfs->volt_tol / 100;
> +                               regulator_set_voltage_tol(dvfs->proc_reg,
> +                                                         new_vproc, tol);
> +                               break;
> +                       }
> +
> +                       cur_vproc = regulator_get_voltage(dvfs->proc_reg);
> +                       target_volt = cur_vproc + VOLT_SHIFT_LIMIT;
> +                       tol = target_volt * dvfs->volt_tol / 100;
> +                       regulator_set_voltage_tol(dvfs->sram_reg, target_volt,
> +                                                 tol);
> +
> +                       if (new_vproc - cur_vproc > VOLT_SHIFT_LIMIT) {
> +                               cur_vsram = regulator_get_voltage(
> +                                                       dvfs->sram_reg);
> +                               target_volt = cur_vsram - 1;
> +                               tol = target_volt * dvfs->volt_tol / 100;
> +                               regulator_set_voltage_tol(dvfs->proc_reg,
> +                                                         target_volt, tol);
> +                       }
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +static int get_opp_voltage(struct device *dev, unsigned long freq_hz)
> +{
> +       struct dev_pm_opp *opp;
> +       int volt;
> +
> +       rcu_read_lock();
> +       opp = dev_pm_opp_find_freq_ceil(dev, &freq_hz);
> +       if (IS_ERR(opp)) {
> +               rcu_read_unlock();
> +               pr_err("%s: failed to find OPP for %lu\n", __func__, freq_hz);
> +               return PTR_ERR(opp);
> +       }
> +       volt = dev_pm_opp_get_voltage(opp);
> +       rcu_read_unlock();
> +
> +       return volt;
> +}
> +
> +static int mt8173_cpufreq_get_intermediate_voltage(int cluster)
> +{
> +       struct cluster_dvfs_info *dvfs = &dvfs_info[cluster];
> +
> +       if (dvfs->inter_volt == 0)
> +               dvfs->inter_volt = get_opp_voltage(dvfs->cpu_dev,
> +                                                  mainpll_freq);
> +
> +       return dvfs->inter_volt;
> +}
> +
> +static void mt8173_cpufreq_set_voltage(int cluster, int old_volt, int new_volt)
> +{
> +       struct cluster_dvfs_info *dvfs = &dvfs_info[cluster];
> +       int ret = 0;
> +
> +       if (cluster == LITTLE_CLUSTER) {
> +               int tol;
> +
> +               tol = new_volt * dvfs->volt_tol / 100;
> +               ret = regulator_set_voltage_tol(dvfs->proc_reg, new_volt, tol);
> +       } else {
> +               ret = mt8173_cpufreq_voltage_step(dvfs, old_volt, new_volt);
> +       }
> +
> +       if (ret)
> +               pr_err("%s: cluster%d failed to scale voltage (%dmV to %dmV)",
> +                      __func__, cluster, old_volt, new_volt);
> +}
> +
> +static int mt8173_cpufreq_notify(struct notifier_block *nb,
> +                                unsigned long action, void *data)
> +{
> +       struct cpufreq_freqs *freqs = data;
> +       struct cluster_dvfs_info *dvfs;
> +       unsigned long old_volt, new_volt, inter_volt;
> +       int cluster;
> +
> +       if (freqs->cpu == dvfs_info[BIG_CLUSTER].cpu)

You should check it against cpumask. freqs->cpu can change on the event
of hotplug of CPUs.

> +               cluster = BIG_CLUSTER;
> +       else if (freqs->cpu == dvfs_info[LITTLE_CLUSTER].cpu)
> +               cluster = LITTLE_CLUSTER;
> +       else
> +               return NOTIFY_DONE;
> +
> +       dvfs = &dvfs_info[cluster];
> +
> +       old_volt = regulator_get_voltage(dvfs->proc_reg);
> +
> +       new_volt = get_opp_voltage(dvfs->cpu_dev, freqs->new * 1000);
> +       inter_volt = mt8173_cpufreq_get_intermediate_voltage(cluster);
> +
> +       if (action == CPUFREQ_PRECHANGE) {
> +               if (old_volt < inter_volt || old_volt < new_volt) {
> +                       new_volt = inter_volt > new_volt ?
> +                                  inter_volt : new_volt;
> +                       mt8173_cpufreq_set_voltage(cluster, old_volt,
> +                                                  new_volt);
> +               }
> +
> +               cpuclk_mux_set(cluster, CPUCLK_MUX_MAINPLL);
> +       } else if (action == CPUFREQ_POSTCHANGE) {
> +               cpuclk_mux_set(cluster, CPUCLK_MUX_ARMPLL);
> +
> +               if (new_volt < old_volt)
> +                       mt8173_cpufreq_set_voltage(cluster, old_volt,
> +                                                  new_volt);
> +       }
> +
> +       return NOTIFY_OK;
> +}
> +
> +static struct notifier_block mt8173_cpufreq_nb = {
> +       .notifier_call = mt8173_cpufreq_notify,
> +};
> +
> +static struct cpufreq_dt_platform_data *pd;
> +
> +static int cpu_in_domain_list(struct list_head *domain_list, int cpu)
> +{
> +       struct list_head *node;
> +
> +       list_for_each(node, domain_list) {
> +               struct cpufreq_cpu_domain *domain;
> +
> +               domain = container_of(node, struct cpufreq_cpu_domain, node);
> +               if (cpumask_test_cpu(cpu, &domain->cpus))
> +                       return 1;
> +       }
> +
> +       return 0;
> +}
> +
> +static void cpufreq_dt_pdata_release(void)
> +{
> +       if (!pd)
> +               return;
> +
> +       if (!list_empty(&pd->domain_list)) {
> +               struct list_head *node, *tmp;
> +               struct cpufreq_cpu_domain *domain;
> +
> +               list_for_each_safe(node, tmp, &pd->domain_list) {
> +                       list_del(node);
> +                       domain = container_of(node, struct cpufreq_cpu_domain,
> +                                             node);
> +                       kfree(domain);
> +               }
> +       }
> +
> +       kfree(pd);
> +}
> +
> +static int cpufreq_dt_pdata_init(void)
> +{
> +       int cpu;
> +
> +       pd = kmalloc(sizeof(*pd), GFP_KERNEL);
> +       if (!pd)
> +               return -ENOMEM;
> +
> +       pd->independent_clocks = 1,
> +       INIT_LIST_HEAD(&pd->domain_list);
> +
> +       for_each_possible_cpu(cpu) {
> +               struct cpufreq_cpu_domain *new_domain;
> +
> +               if (cpu_in_domain_list(&pd->domain_list, cpu))
> +                       continue;
> +
> +               new_domain = kzalloc(sizeof(*new_domain), GFP_KERNEL);
> +               if (!new_domain) {
> +                       cpufreq_dt_pdata_release();
> +                       return -ENOMEM;
> +               }
> +
> +               cpumask_copy(&new_domain->cpus,
> +                            &cpu_topology[cpu].core_sibling);
> +               list_add(&new_domain->node, &pd->domain_list);
> +       }
> +
> +       return 0;
> +}
> +
> +static void mt8173_cpufreq_dvfs_info_release(void)
> +{
> +       int i;
> +
> +       if (!dvfs_info)
> +               return;
> +
> +       for (i = 0; i < NR_CLUSTERS; ++i) {
> +               struct cluster_dvfs_info *dvfs = &dvfs_info[i];
> +
> +               if (!IS_ERR_OR_NULL(dvfs->proc_reg))
> +                       regulator_put(dvfs->proc_reg);
> +
> +               if (!IS_ERR_OR_NULL(dvfs->sram_reg))
> +                       regulator_put(dvfs->sram_reg);
> +       }
> +
> +       if (clk_mux_regs)
> +               iounmap(clk_mux_regs);
> +}
> +
> +static int mt8173_cpufreq_dvfs_info_init(void)
> +{
> +       struct list_head *node;
> +       struct device_node *np;
> +       struct clk *mainpll;
> +       int i, ret;
> +
> +       dvfs_info = kzalloc(sizeof(*dvfs) * NR_CLUSTERS, GFP_KERNEL);
> +       if (!dvfs_info)
> +               return -ENOMEM;
> +
> +       list_for_each(node, &pd->domain_list) {
> +               struct cpufreq_cpu_domain *domain = container_of(
> +                                       node, struct cpufreq_cpu_domain, node);
> +               int cpu = cpumask_next(0, &domain->cpus);

cpumask_first() ?

> +
> +               np = of_get_cpu_node(cpu, NULL);

This can fail..

> +
> +               if (of_property_match_string(np, "compatible",
> +                                            "arm,cortex-a53") >= 0)
> +                       cpumask_copy(&dvfs_info[LITTLE_CLUSTER].cpus,
> +                                    &domain->cpus);
> +               else if (of_property_match_string(np, "compatible",
> +                                                 "arm,cortex-a57") >= 0)
> +                       cpumask_copy(&dvfs_info[BIG_CLUSTER].cpus,
> +                                    &domain->cpus);
> +               else {
> +                       of_node_put(np);
> +                       pr_err("%s: unknown CPU core\n", __func__);
> +                       return -EINVAL;
> +               }
> +
> +               of_node_put(np);
> +       }
> +
> +       for (i = 0; i < NR_CLUSTERS; i++) {

You can merge this for-loop with the above for-loop..

> +               struct cluster_dvfs_info *dvfs = &dvfs_info[i];
> +               int cpu;
> +
> +               for_each_cpu_mask(cpu, dvfs->cpus) {
> +                       struct device_node *np;
> +
> +                       dvfs->cpu_dev = get_cpu_device(cpu);
> +                       dvfs->proc_reg = regulator_get(dvfs->cpu_dev, "proc");
> +                       if (IS_ERR(dvfs->proc_reg))
> +                               continue;
> +
> +                       dvfs->cpu = cpu;
> +                       dvfs->sram_reg = regulator_get(dvfs->cpu_dev, "sram");
> +
> +                       np = of_node_get(dvfs->cpu_dev->of_node);
> +                       of_property_read_u32(np, "voltage-tolerance",
> +                                            &dvfs->volt_tol);
> +                       of_node_put(np);
> +                       break;
> +               }
> +
> +               if (IS_ERR(dvfs->proc_reg)) {
> +                       pr_err("%s: no proc regulator for cluster %d\n",
> +                              __func__, i);
> +                       ret = -ENODEV;
> +                       goto dvfs_info_release;
> +               }
> +
> +               if (IS_ERR(dvfs->sram_reg) && BIG_CLUSTER == i) {
> +                       pr_err("%s: no sram regulator for cluster %d\n",
> +                              __func__, i);
> +                       ret = -ENODEV;
> +                       goto dvfs_info_release;
> +               }
> +       }
> +
> +       mainpll = __clk_lookup("mainpll");
> +       if (!mainpll) {
> +               pr_err("failed to get mainpll clk\n");
> +               ret = -ENOENT;
> +               goto dvfs_info_release;
> +       }
> +       mainpll_freq = clk_get_rate(mainpll);
> +
> +       np = of_find_compatible_node(NULL, NULL, "mediatek,mt8173-infrasys");
> +       if (!np) {
> +               pr_err("failed to find clock mux registers\n");
> +               ret = -ENODEV;
> +               goto dvfs_info_release;
> +       }
> +
> +       clk_mux_regs = of_iomap(np, 0);
> +       if (!clk_mux_regs) {
> +               pr_err("failed to map clock mux registers\n");
> +               ret = -EFAULT;
> +               goto dvfs_info_release;
> +       }
> +       of_node_put(np);
> +
> +       spin_lock_init(&lock);
> +
> +       return 0;
> +
> +dvfs_info_release:
> +       mt8173_cpufreq_dvfs_info_release();
> +       return ret;
> +}
> +
> +static int mt8173_cpufreq_driver_init(void)
> +{
> +       int ret;
> +
> +       if (!of_machine_is_compatible("mediatek,mt8173"))
> +               return -ENODEV;
> +
> +       ret = cpufreq_dt_pdata_init();
> +       if (ret) {
> +               pr_err("failed to initialize platform data: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = mt8173_cpufreq_dvfs_info_init();
> +       if (ret) {
> +               pr_err("failed to initialize mt8173 dvfs info: %d\n", ret);
> +               goto dt_pdata_release;
> +       }
> +
> +       ret = cpufreq_register_notifier(&mt8173_cpufreq_nb,
> +                                       CPUFREQ_TRANSITION_NOTIFIER);
> +       if (ret) {
> +               pr_err("failed to register cpufreq notifier: %d\n", ret);
> +               goto dt_pdata_release;

dvfs_info_release here ?

> +       }
> +
> +       platform_device_register_data(NULL, "cpufreq-dt", -1,
> +                                     pd, sizeof(*pd));

can't this fail?

> +
> +       return 0;
> +
> +dt_pdata_release:
> +       cpufreq_dt_pdata_release();
> +       return ret;
> +}
> +
> +module_init(mt8173_cpufreq_driver_init);
> +
> +MODULE_AUTHOR("Pi-Cheng Chen <pi-cheng.chen@xxxxxxxxxx>");
> +MODULE_DESCRIPTION("Mediatek MT8173 cpufreq driver");
> +MODULE_LICENSE("GPL");
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/