[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <154396249199.88331.1800559141437859959@swboyd.mtv.corp.google.com>
Date: Tue, 04 Dec 2018 14:28:11 -0800
From: Stephen Boyd <sboyd@...nel.org>
To: Taniya Das <tdas@...eaurora.org>,
Viresh Kumar <viresh.kumar@...aro.org>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
Rajendra Nayak <rnayak@...eaurora.org>,
devicetree@...r.kernel.org, robh@...nel.org,
skannan@...eaurora.org, linux-arm-msm@...r.kernel.org,
evgreen@...gle.com
Subject: Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
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;
}
Powered by blists - more mailing lists