Re: [PATCH v3 2/2] cpufreq: qoriq: Don't look at clock implementation details
From: Michael Turquette <mturquette@baylibre.com>
Date: 2016-07-07 01:30:17
Also in:
linux-clk, linuxppc-dev
Quoting Scott Wood (2016-06-15 23:21:25)
quoted hunk ↗ jump to hunk
-static struct device_node *cpu_to_clk_node(int cpu) +static struct clk *cpu_to_clk(int cpu) { - struct device_node *np, *clk_np; + struct device_node *np; + struct clk *clk; if (!cpu_present(cpu)) return NULL;@@ -112,37 +80,28 @@ static struct device_node *cpu_to_clk_node(int cpu) if (!np) return NULL; - clk_np = of_parse_phandle(np, "clocks", 0); - if (!clk_np) - return NULL; - + clk = of_clk_get(np, 0);
Why not use devm_clk_get here?
quoted hunk ↗ jump to hunk
@@ -221,17 +180,12 @@ static int qoriq_cpufreq_cpu_init(struct cpufreq_policy *policy) goto err_nomem2; } - pnode = of_parse_phandle(np, "clocks", 0); - if (!pnode) { - pr_err("%s: could not get clock information\n", __func__); - goto err_nomem2; - } + count = clk_get_num_parents(policy->clk);
We already have of_clk_get_parent_count. This is found in clk-provider.h, which doesn't fit perfectly here since the cpufreq driver is not a clock provider, but instead a consumer.
quoted hunk ↗ jump to hunk
@@ -240,23 +194,11 @@ static int qoriq_cpufreq_cpu_init(struct cpufreq_policy *policy) goto err_pclk; } - if (fmask) - mask = fmask[get_cpu_physical_id(cpu)]; - else - mask = 0x0; - for (i = 0; i < count; i++) { - clk = of_clk_get(pnode, i); + clk = clk_get_parent_by_index(policy->clk, i); data->pclk[i] = clk; freq = clk_get_rate(clk); - /* - * the clock is valid if its frequency is not masked - * and large than minimum allowed frequency. - */ - if (freq < min_cpufreq || (mask & (1 << i))) - table[i].frequency = CPUFREQ_ENTRY_INVALID; - else - table[i].frequency = freq / 1000; + table[i].frequency = freq / 1000;
Hmm, so you change cpu clock rate by selecting different clock sources from a cpu clock mux, right? I wonder if you can just have a child mux clock that selects parents from .set_rate (via a .determine_rate callback)? Then you could just call clk_set_rate() on your cpu mux clock and maybe skip most of the stuff this driver does? Regards, Mike