Quoting Viresh Kumar (2018-12-03 21:12:31)
> Hi Taniya,
> 
> Sorry that I haven't been reviewing it much from last few iterations as I was
> letting others get this into a better shape. Thanks for your efforts..
> 
> On 02-12-18, 09:25, Taniya Das wrote:
> > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> 
> > +struct cpufreq_qcom {
> > +     struct cpufreq_frequency_table *table;
> > +     void __iomem *perf_state_reg;
> > +     cpumask_t related_cpus;
> > +};
> > +
> > +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
> 
> Now that the code is much more simplified, I am not sure if you need this
> per-cpu structure at all. The only place where you are using it is in
> qcom_cpufreq_hw_cpu_init() and probe(). Why not merge 
> qcom_cpu_resources_init()
> completely into qcom_cpufreq_hw_cpu_init() and get rid of this structure
> entirely ?
> 

Good point. Here's an untested patch to handle that. It removes the
related functionality and makes an array of pointers to the domains that
are created each time qcom_cpu_resources_init() is called.

---8<----

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c 
b/drivers/cpufreq/qcom-cpufreq-hw.c
index 8dc6b73c2f22..04e7cfd70316 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -23,14 +23,14 @@
 #define REG_LUT_TABLE                  0x110
 #define REG_PERF_STATE                 0x920
 
+#define MAX_FREQ_DOMAINS               2
+
 struct cpufreq_qcom {
        struct cpufreq_frequency_table *table;
        void __iomem *perf_state_reg;
        cpumask_t related_cpus;
 };
 
-static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
-
 static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
                                        unsigned int index)
 {
@@ -76,9 +76,14 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct 
cpufreq_policy *policy,
 
 static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 {
+       struct cpufreq_qcom **freq_domain_map;
        struct cpufreq_qcom *c;
 
-       c = qcom_freq_domain_map[policy->cpu];
+       freq_domain_map = cpufreq_get_driver_data();
+       if (!freq_domain_map)
+               return -ENODEV;
+
+       c = freq_domain_map[policy->cpu];
        if (!c) {
                pr_err("No scaling support for CPU%d\n", policy->cpu);
                return -ENODEV;
@@ -171,39 +176,17 @@ static int qcom_cpufreq_hw_read_lut(struct device *dev, 
struct cpufreq_qcom *c,
        return 0;
 }
 
-static void qcom_get_related_cpus(int index, struct cpumask *m)
-{
-       struct device_node *cpu_np;
-       struct of_phandle_args args;
-       int cpu, ret;
-
-       for_each_possible_cpu(cpu) {
-               cpu_np = of_cpu_device_node_get(cpu);
-               if (!cpu_np)
-                       continue;
-
-               ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
-                                                "#freq-domain-cells", 0,
-                                                 &args);
-               of_node_put(cpu_np);
-               if (ret < 0)
-                       continue;
-
-               if (index == args.args[0])
-                       cpumask_set_cpu(cpu, m);
-       }
-}
-
 static int qcom_cpu_resources_init(struct platform_device *pdev,
                                   unsigned int cpu, int index,
                                   unsigned long xo_rate,
-                                  unsigned long cpu_hw_rate)
+                                  unsigned long cpu_hw_rate,
+                                  struct cpufreq_qcom **freq_domain_map)
 {
        struct cpufreq_qcom *c;
        struct resource *res;
        struct device *dev = &pdev->dev;
        void __iomem *base;
-       int ret, cpu_r;
+       int ret;
 
        c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
        if (!c)
@@ -220,12 +203,6 @@ static int qcom_cpu_resources_init(struct platform_device 
*pdev,
                return -ENODEV;
        }
 
-       qcom_get_related_cpus(index, &c->related_cpus);
-       if (!cpumask_weight(&c->related_cpus)) {
-               dev_err(dev, "Domain-%d failed to get related CPUs\n", index);
-               return -ENOENT;
-       }
-
        c->perf_state_reg = base + REG_PERF_STATE;
 
        ret = qcom_cpufreq_hw_read_lut(dev, c, base, xo_rate, cpu_hw_rate);
@@ -234,8 +211,7 @@ static int qcom_cpu_resources_init(struct platform_device 
*pdev,
                return ret;
        }
 
-       for_each_cpu(cpu_r, &c->related_cpus)
-               qcom_freq_domain_map[cpu_r] = c;
+       freq_domain_map[index] = c;
 
        return 0;
 }
@@ -245,9 +221,16 @@ static int qcom_cpufreq_hw_driver_probe(struct 
platform_device *pdev)
        struct device_node *cpu_np;
        struct of_phandle_args args;
        struct clk *clk;
-       unsigned int cpu;
+       unsigned int cpu, domain;
        unsigned long xo_rate, cpu_hw_rate;
        int ret;
+       struct cpufreq_qcom **freq_domain_map, *freq_domain;
+
+       freq_domain_map = devm_kcalloc(&pdev->dev, MAX_FREQ_DOMAINS,
+                                      sizeof(*freq_domain_map), GFP_KERNEL);
+       cpufreq_qcom_hw_driver.driver_data = freq_domain_map;
+       if (!freq_domain_map)
+               return -ENOMEM;
 
        clk = clk_get(&pdev->dev, "xo");
        if (IS_ERR(clk))
@@ -273,16 +256,28 @@ static int qcom_cpufreq_hw_driver_probe(struct 
platform_device *pdev)
 
                ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
                                                 "#freq-domain-cells", 0,
-                                                 &args);
+                                                &args);
                of_node_put(cpu_np);
                if (ret)
                        return ret;
 
-               if (qcom_freq_domain_map[cpu])
+               domain = args.args[0];
+               if (domain >= MAX_FREQ_DOMAINS)
                        continue;
 
-               ret = qcom_cpu_resources_init(pdev, cpu, args.args[0],
-                                             xo_rate, cpu_hw_rate);
+               /*
+                * If we've already populated the frequency table for this 
domain
+                * just mark it related and get out of here
+                */
+               freq_domain = freq_domain_map[domain];
+               if (freq_domain) {
+                       cpumask_set_cpu(cpu, &freq_domain->related_cpus);
+                       continue;
+               }
+
+               ret = qcom_cpu_resources_init(pdev, cpu, domain,
+                                             xo_rate, cpu_hw_rate,
+                                             freq_domain_map);
                if (ret)
                        return ret;
        }

Reply via email to