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; }