Messages in this thread | | | Date | Mon, 1 Feb 2016 18:10:46 -0800 | From | Stephen Boyd <> | Subject | Re: [PATCH V2 07/16] PM / OPP: Add dev_pm_opp_set_rate() |
| |
On 01/28, Viresh Kumar wrote: > diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c > index 7d7749ce1ce4..2672f4bfec41 100644 > --- a/drivers/base/power/opp/core.c > +++ b/drivers/base/power/opp/core.c > @@ -529,6 +529,182 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, > } > EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor); > > +/* > + * The caller needs to ensure that device_opp (and hence the clk) isn't freed, > + * while clk returned here is used. > + */ > +static struct clk *_get_opp_clk(struct device *dev) > +{ > + struct device_opp *dev_opp; > + struct clk *clk; > + > + rcu_read_lock(); > + > + dev_opp = _find_device_opp(dev); > + if (IS_ERR(dev_opp)) { > + dev_err(dev, "%s: device opp doesn't exist\n", __func__); > + clk = (struct clk *)dev_opp;
clk = ERR_CAST(dev_opp);
> + goto unlock; > + } > + > + clk = dev_opp->clk; > + if (IS_ERR(clk)) > + dev_err(dev, "%s: No clock available for the device\n", > + __func__); > + > +unlock: > + rcu_read_unlock(); > + return clk; > +} > + > +static int _set_opp_voltage(struct device *dev, struct regulator *reg, > + unsigned long u_volt, unsigned long u_volt_min, > + unsigned long u_volt_max) > +{ > + int ret; > + > + /* Regulator not be available for device */
not available?
> + if (IS_ERR(reg)) { > + dev_dbg(dev, "%s: regulator not available: %ld\n", __func__, > + PTR_ERR(reg)); > + return 0; > + } > + > + dev_dbg(dev, "%s: voltages (mV): %lu %lu %lu\n", __func__, u_volt_min, > + u_volt, u_volt_max); > + > + ret = regulator_set_voltage_triplet(reg, u_volt_min, u_volt, > + u_volt_max); > + if (ret) > + dev_err(dev, "%s: failed to set voltage (%lu %lu %lu mV): %d\n", > + __func__, u_volt_min, u_volt, u_volt_max, ret); > + > + return ret; > +} > + > +/** > + * dev_pm_opp_set_rate() - Configure new OPP based on frequency > + * @dev: device for which we do this operation > + * @target_freq: frequency to achieve > + * > + * This configures the power-supplies and clock source to the levels specified > + * by the OPP corresponding to the target_freq. > + * > + * Locking: This function takes rcu_read_lock(). > + */ > +int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) > +{ > + struct device_opp *dev_opp; > + struct dev_pm_opp *old_opp, *opp; > + struct regulator *reg; > + struct clk *clk; > + unsigned long freq, old_freq; > + unsigned long u_volt, u_volt_min, u_volt_max; > + unsigned long ou_volt, ou_volt_min, ou_volt_max; > + int ret; > + > + if (unlikely(!target_freq)) { > + dev_err(dev, "%s: Invalid target frequency %lu\n", __func__, > + target_freq); > + return -EINVAL; > + } > + > + clk = _get_opp_clk(dev); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + > + freq = clk_round_rate(clk, target_freq); > + if ((long)freq <= 0) > + freq = target_freq; > + > + old_freq = clk_get_rate(clk); > + > + /* Return early if nothing to do */ > + if (old_freq == freq) { > + dev_dbg(dev, "%s: old/new frequencies (%lu Hz) are same, nothing to do\n", > + __func__, freq); > + return 0; > + } > + > + rcu_read_lock(); > + > + dev_opp = _find_device_opp(dev); > + if (IS_ERR(dev_opp)) { > + dev_err(dev, "%s: device opp doesn't exist\n", __func__); > + ret = PTR_ERR(dev_opp); > + goto unlock; > + } > + > + old_opp = dev_pm_opp_find_freq_ceil(dev, &old_freq); > + if (!IS_ERR(old_opp)) { > + ou_volt = old_opp->u_volt; > + ou_volt_min = old_opp->u_volt_min; > + ou_volt_max = old_opp->u_volt_max; > + } else { > + dev_err(dev, "%s: failed to find Current OPP for freq %lu (%ld)\n",
Why is current capitalized?
> + __func__, old_freq, PTR_ERR(old_opp)); > + } > + > + opp = dev_pm_opp_find_freq_ceil(dev, &freq); > + if (IS_ERR(opp)) { > + ret = PTR_ERR(opp); > + dev_err(dev, "%s: failed to find OPP for freq %lu (%d)\n", > + __func__, freq, ret); > + goto unlock; > + } > + > + u_volt = opp->u_volt; > + u_volt_min = opp->u_volt_min; > + u_volt_max = opp->u_volt_max; > + > + reg = dev_opp->regulator; > + > + rcu_read_unlock(); > + > + /* Scaling up? Scale voltage before frequency */ > + if (freq > old_freq) { > + ret = _set_opp_voltage(dev, reg, u_volt, u_volt_min, > + u_volt_max); > + if (ret) > + goto restore_voltage; > + } > + > + /* Change frequency */ > + > + dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n", > + __func__, old_freq, freq); > + > + ret = clk_set_rate(clk, freq); > + if (ret) { > + dev_err(dev, "%s: failed to set clock rate: %d\n", __func__, > + ret); > + goto restore_voltage; > + } > + > + /* Scaling down? Scale voltage after frequency */ > + if (freq < old_freq) { > + ret = _set_opp_voltage(dev, reg, u_volt, u_volt_min, > + u_volt_max); > + if (ret) > + goto restore_freq; > + } > + > + return 0; > + > +restore_freq: > + if (clk_set_rate(clk, old_freq)) > + dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n", > + __func__, old_freq); > +restore_voltage: > + /* This shouldn't harm even if the voltages weren't updated earlier */ > + if (!IS_ERR(old_opp)) > + _set_opp_voltage(dev, reg, ou_volt, ou_volt_min, ou_volt_max); > +unlock: > + rcu_read_unlock();
The error gotos above this don't have rcu lock held, so this is sometimes wrong to do.
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
| |