Re: [PATCH V2 6/8] PM / OPP: Separate out _generic_opp_set_rate()

On 10/20, Viresh Kumar wrote:

Later patches would add support for custom opp_set_rate callbacks. This

I know the OPP consumer function has "rate" in the name, but it makes more sense to call the callback set_opp instead because we could be doing a lot more than setting the opp rate.

patch separates out the code for generic opp_set_rate handler in order to prepare for that.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 45c70ce07864..96f04392daef 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -596,6 +596,73 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg, return ret; } +static inline int +_generic_opp_set_rate_clk_only(struct device *dev, struct clk *clk,

• 	       unsigned long old_freq, unsigned long freq)
  

+{

• int ret;
• /* Change frequency */
• dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n",

• __func__, old_freq, freq);
  

Perhaps this should stay at the beginning of OPP transitions? Otherwise it can get confusing when multiple switching OPP messages appear on OPP transition failures.

• ret = clk_set_rate(clk, freq);
• if (ret) {

• dev_err(dev, "%s: failed to set clock rate: %d\n", __func__,
  

• 	ret);
  

• }
• return ret;

+}

diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h index d3f0861f9bff..6629c53c0aa1 100644 --- a/drivers/base/power/opp/opp.h +++ b/drivers/base/power/opp/opp.h @@ -47,22 +47,6 @@ extern struct list_head opp_tables; */ /**

• • struct dev_pm_opp_supply - Power supply voltage/current values
• • @u_volt: Target voltage in microvolts corresponding to this OPP
• • @u_volt_min: Minimum voltage in microvolts corresponding to this OPP
• • @u_volt_max: Maximum voltage in microvolts corresponding to this OPP
• • @u_amp: Maximum current drawn by the device in microamperes
• • This structure stores the voltage/current values for a single power supply.
• */

-struct dev_pm_opp_supply {

• unsigned long u_volt;
• unsigned long u_volt_min;
• unsigned long u_volt_max;
• unsigned long u_amp;

-};

-/**

• struct dev_pm_opp - Generic OPP description structure
• @node: opp table node. The nodes are maintained throughout the lifetime

• of boot. It is expected only an optimal set of OPPs are
  

diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 0606b70a8b97..73713a8424b1 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -17,6 +17,7 @@ #include <linux/err.h> #include <linux/notifier.h> +struct clk;

Is struct regulator also forward declared?

struct dev_pm_opp; struct device; @@ -24,6 +25,36 @@ enum dev_pm_opp_event { OPP_EVENT_ADD, OPP_EVENT_REMOVE, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE, }; +/**

• • struct dev_pm_opp_supply - Power supply voltage/current values
• • @u_volt: Target voltage in microvolts corresponding to this OPP
• • @u_volt_min: Minimum voltage in microvolts corresponding to this OPP
• • @u_volt_max: Maximum voltage in microvolts corresponding to this OPP
• • @u_amp: Maximum current drawn by the device in microamperes
• • This structure stores the voltage/current values for a single power supply.
• */

+struct dev_pm_opp_supply {

• unsigned long u_volt;
• unsigned long u_volt_min;
• unsigned long u_volt_max;
• unsigned long u_amp;

+};

This structure moved during this series. Can we avoid that and already have it in the right place to begin with?

+struct dev_pm_opp_info {

• unsigned long rate;
• struct dev_pm_opp_supply *supplies;

+};

+struct dev_pm_set_rate_data {

dev_pm_set_opp_data?

• struct dev_pm_opp_info old_opp;
• struct dev_pm_opp_info new_opp;
• struct regulator **regulators;
• unsigned int regulator_count;
• struct clk *clk;

+};

The above two structures don't get kernel doc?

#if defined(CONFIG_PM_OPP) unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp); -- 2.7.1.410.g6faf27b