| Message ID | 981905b802879bff26d839b0aab19ad67a3aa1ff.1450777582.git.viresh.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 12/22, Viresh Kumar wrote: > OPP core can handle the regulators by itself, and it allocates the > regulator based on device's name. But for older V1 bindings, many DT > files have used names like 'cpu-supply' instead of 'cpu0-supply'. > > The cpufreq-dt driver needs to tell the right name of the regulator in > this case to the OPP core. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> This whole patch is confusing to me because old style was to be cpu0-supply, and new style is cpu-supply. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11-01-16, 17:53, Stephen Boyd wrote: > On 12/22, Viresh Kumar wrote: > > OPP core can handle the regulators by itself, and it allocates the > > regulator based on device's name. But for older V1 bindings, many DT > > files have used names like 'cpu-supply' instead of 'cpu0-supply'. > > > > The cpufreq-dt driver needs to tell the right name of the regulator in > > this case to the OPP core. > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > This whole patch is confusing to me because old style was to be > cpu0-supply, and new style is cpu-supply. Long back we used to have cpu0-supply, then while I converted cpufreq-dt to support multiple clusters, you asked me to name it to cpu-supply, which I did. Now, looking at the implementation into the generic OPP layer, it looks like <device>-name is a far better and reasonable choice. And so I am moving back to cpu0-supply, will udpate binding doc as well. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/12, Viresh Kumar wrote: > On 11-01-16, 17:53, Stephen Boyd wrote: > > On 12/22, Viresh Kumar wrote: > > > OPP core can handle the regulators by itself, and it allocates the > > > regulator based on device's name. But for older V1 bindings, many DT > > > files have used names like 'cpu-supply' instead of 'cpu0-supply'. > > > > > > The cpufreq-dt driver needs to tell the right name of the regulator in > > > this case to the OPP core. > > > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > > > This whole patch is confusing to me because old style was to be > > cpu0-supply, and new style is cpu-supply. > > Long back we used to have cpu0-supply, then while I converted > cpufreq-dt to support multiple clusters, you asked me to name it to > cpu-supply, which I did. > > Now, looking at the implementation into the generic OPP layer, it > looks like <device>-name is a far better and reasonable choice. It's far easier to implement, but not far better. In most designs the pin is not called <device_name>-supply, but something more mundane like vdd-supply, vddio-supply, vcc-supply, etc. In the case of CPUs, there's probably nothing in the datasheets, so cpu vs cpu0 is not too important to distinguish here. But for things like a GPU, DSP, video encoder, etc. I doubt it's going to be called <device_name>-supply, so making that the norm is misguided. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12-01-16, 16:43, Stephen Boyd wrote: > It's far easier to implement, but not far better. In most designs > the pin is not called <device_name>-supply, but something more > mundane like vdd-supply, vddio-supply, vcc-supply, etc. In the > case of CPUs, there's probably nothing in the datasheets, so cpu > vs cpu0 is not too important to distinguish here. But for things > like a GPU, DSP, video encoder, etc. I doubt it's going to be > called <device_name>-supply, so making that the norm is > misguided. I completely agree with that, but here is the usecase: - A OPP-user driver doesn't need to call any special OPP API and OPP layer can allocate the regulator for it using <device>-name. This will make all drivers (that follow this nomenclature in DT) very simple and straight-forward. - But then there are drivers, which need special supply-name. They can always call opp-set-regulator API to mention that, no one is stopping them from that. And so I still believe, OPP layer has only this option to do it generically. Comments ? -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 13, 2016 at 11:17:50AM +0530, Viresh Kumar wrote: > - A OPP-user driver doesn't need to call any special OPP API and OPP > layer can allocate the regulator for it using <device>-name. This > will make all drivers (that follow this nomenclature in DT) very > simple and straight-forward. Viresh, as we've been through multiple times supplies should always be requested with the name of the supply on the physical device. Please stop trying to create ABIs around whatever Linux internals you're currently working with, this is getting repetitive.
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c index a6f91da7283e..39df4f1a06d2 100644 --- a/drivers/cpufreq/cpufreq-dt.c +++ b/drivers/cpufreq/cpufreq-dt.c @@ -34,6 +34,7 @@ struct private_data { struct regulator *cpu_reg; struct thermal_cooling_device *cdev; unsigned int voltage_tolerance; /* in percentage */ + const char *reg_name; }; static struct freq_attr *cpufreq_dt_attr[] = { @@ -118,6 +119,22 @@ static int set_target(struct cpufreq_policy *policy, unsigned int index) return ret; } +/* + * An earlier version of opp-v1 bindings used to name the regulator + * "cpu-supply", we still need to handle that for backwards compatibility. + */ +static const char *find_supply_name(struct device *dev) +{ + struct regulator *cpu_reg; + + cpu_reg = regulator_get_optional(dev, dev_name(dev)); + if (IS_ERR(cpu_reg)) + return "cpu"; + + regulator_put(cpu_reg); + return NULL; +} + static int allocate_resources(int cpu, struct device **cdev, struct regulator **creg, struct clk **cclk) { @@ -200,6 +217,7 @@ static int cpufreq_init(struct cpufreq_policy *policy) unsigned long min_uV = ~0, max_uV = 0; unsigned int transition_latency; bool opp_v1 = false; + const char *name = NULL; int ret; ret = allocate_resources(policy->cpu, &cpu_dev, &cpu_reg, &cpu_clk); @@ -229,6 +247,25 @@ static int cpufreq_init(struct cpufreq_policy *policy) } /* + * OPP layer will be taking care of regulators now, but it needs to know + * the name of the regulator for v1 bindings. + */ + if (opp_v1) { + name = find_supply_name(cpu_dev); + if (name) { + ret = dev_pm_opp_set_regulator(cpu_dev, name); + if (ret) { + /* + * Regulators aren't compulsory on few + * platforms, don't error out for them. + */ + dev_dbg(cpu_dev, "Failed to set regulator for cpu%d: %d\n", + policy->cpu, ret); + } + } + } + + /* * Initialize OPP tables for all policy->cpus. They will be shared by * all CPUs which have marked their CPUs shared with OPP bindings. * @@ -273,6 +310,7 @@ static int cpufreq_init(struct cpufreq_policy *policy) goto out_free_opp; } + priv->reg_name = name; of_property_read_u32(np, "voltage-tolerance", &priv->voltage_tolerance); transition_latency = dev_pm_opp_get_max_clock_latency(cpu_dev); @@ -366,6 +404,8 @@ static int cpufreq_init(struct cpufreq_policy *policy) kfree(priv); out_free_opp: dev_pm_opp_of_cpumask_remove_table(policy->cpus); + if (opp_v1) + dev_pm_opp_put_regulator(cpu_dev); out_node_put: of_node_put(np); out_put_reg_clk: @@ -383,6 +423,9 @@ static int cpufreq_exit(struct cpufreq_policy *policy) cpufreq_cooling_unregister(priv->cdev); dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table); dev_pm_opp_of_cpumask_remove_table(policy->related_cpus); + if (priv->reg_name) + dev_pm_opp_put_regulator(priv->cpu_dev); + clk_put(policy->clk); if (!IS_ERR(priv->cpu_reg)) regulator_put(priv->cpu_reg);
OPP core can handle the regulators by itself, and it allocates the regulator based on device's name. But for older V1 bindings, many DT files have used names like 'cpu-supply' instead of 'cpu0-supply'. The cpufreq-dt driver needs to tell the right name of the regulator in this case to the OPP core. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/cpufreq/cpufreq-dt.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) -- 2.7.0.rc1.186.g94414c4 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html