On 09-01-17, 14:39, Stephen Boyd wrote:
> On 12/07, Viresh Kumar wrote:
> > @@ -137,15 +121,18 @@ functions return the matching pointer representing
> > the opp if a match is
> > found, else returns error. These errors are expected to be handled by
> > standard
> > error checks such as IS_ERR() and appropriate actions taken by the caller.
> >
> > +Callers of these functions shall call dev_pm_opp_put() after they have
> > used the
> > +OPP. Otherwise the memory for the OPP will never get freed and result in
> > +memleak.
> > +
> > dev_pm_opp_find_freq_exact - Search for an OPP based on an *exact*
> > frequency and
> > availability. This function is especially useful to enable an OPP which
> > is not available by default.
> > Example: In a case when SoC framework detects a situation where a
> > higher frequency could be made available, it can use this function to
> > find the OPP prior to call the dev_pm_opp_enable to actually make it
> > available.
> > - rcu_read_lock();
> > opp = dev_pm_opp_find_freq_exact(dev, 1000000000, false);
> > - rcu_read_unlock();
> > + dev_pm_opp_put(opp);
> > /* dont operate on the pointer.. just do a sanity check.. */
> > if (IS_ERR(opp)) {
> > pr_err("frequency not disabled!\n");
> > @@ -163,9 +150,8 @@ dev_pm_opp_find_freq_floor - Search for an available
> > OPP which is *at most* the
> > frequency.
> > Example: To find the highest opp for a device:
> > freq = ULONG_MAX;
> > - rcu_read_lock();
> > dev_pm_opp_find_freq_floor(dev, &freq);
> > - rcu_read_unlock();
> > + dev_pm_opp_put(opp);
>
> opp doesn't exist in the scope here. Missing an assignment during
> the dev_pm_opp_find_freq_floor() call?
Thanks for noticing this. Following is the diff I am adding to this patch:
diff --git a/Documentation/power/opp.txt b/Documentation/power/opp.txt
index be895e32022d..0c007e250cd1 100644
--- a/Documentation/power/opp.txt
+++ b/Documentation/power/opp.txt
@@ -150,7 +150,7 @@ dev_pm_opp_find_freq_floor - Search for an available OPP
which is *at most* the
frequency.
Example: To find the highest opp for a device:
freq = ULONG_MAX;
- dev_pm_opp_find_freq_floor(dev, &freq);
+ opp = dev_pm_opp_find_freq_floor(dev, &freq);
dev_pm_opp_put(opp);
dev_pm_opp_find_freq_ceil - Search for an available OPP which is *at least* the
@@ -159,7 +159,7 @@ dev_pm_opp_find_freq_ceil - Search for an available OPP
which is *at least* the
frequency.
Example 1: To find the lowest opp for a device:
freq = 0;
- dev_pm_opp_find_freq_ceil(dev, &freq);
+ opp = dev_pm_opp_find_freq_ceil(dev, &freq);
dev_pm_opp_put(opp);
Example 2: A simplified implementation of a SoC cpufreq_driver->target:
soc_cpufreq_target(..)
@@ -252,6 +252,7 @@ dev_pm_opp_get_freq - Retrieve the freq represented by the
opp pointer.
if (!IS_ERR(max_opp) && !IS_ERR(requested_opp))
r = soc_test_validity(max_opp, requested_opp);
dev_pm_opp_put(max_opp);
+ dev_pm_opp_put(requested_opp);
/* do other things */
}
soc_test_validity(..)
Please add your RBY if it looks fine to you now.
--
viresh
