On Tue, May 13, 2014 at 2:41 AM, [Chander Kashyap <chander.kash...@linaro.org> wrote: > From: Chander Kashyap <k.chan...@samsung.com> > > It may be possible to unregister and re-register the cpufreq driver. > One such example is arm big-little IKS cpufreq driver. While > re-registering the driver, same OPPs may get added again. > > This patch detects the duplicacy and discards them.
Nice catch. Thanks for the same. That said, instead of ignoring it (skipping addition), should we do the following: a) if we find the same OPP being added, return error b) add a cleanup routine dev_pm_opp_remove ? Original design required OPP entries added by platform code and used by driver code, but things have changed over time. > > Signed-off-by: Chander Kashyap <k.chan...@samsung.com> > Signed-off-by: Inderpal Singh <inderpa...@samsung.com> > --- > drivers/base/power/opp.c | 28 +++++++++++++++++++--------- > 1 file changed, 19 insertions(+), 9 deletions(-) > > diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c > index 2553867..2e803a9 100644 > --- a/drivers/base/power/opp.c > +++ b/drivers/base/power/opp.c > @@ -443,23 +443,33 @@ int dev_pm_opp_add(struct device *dev, unsigned long > freq, unsigned long u_volt) > new_opp->u_volt = u_volt; > new_opp->available = true; > > - /* Insert new OPP in order of increasing frequency */ > + /* > + * Insert new OPP in order of increasing frequency > + * and discard if already present > + */ > head = &dev_opp->opp_list; > list_for_each_entry_rcu(opp, &dev_opp->opp_list, node) { > - if (new_opp->rate < opp->rate) > + if (new_opp->rate <= opp->rate) > break; > else > head = &opp->node; > } > say we do at this point: if (new_opp->rate == opp->rate) { dev_err(dev, "%s: attempt to add duplicate OPP entry (rate=%ld)\n", __func__, new_opp->rate) kfree(new_opp); return -EINVAL; } we could avoid the change below, right? > - list_add_rcu(&new_opp->node, head); > - mutex_unlock(&dev_opp_list_lock); > + if (new_opp->rate != opp->rate) { > + list_add_rcu(&new_opp->node, head); > + mutex_unlock(&dev_opp_list_lock); > + > + /* > + * Notify the changes in the availability of the operable > + * frequency/voltage list. > + */ > + srcu_notifier_call_chain(&dev_opp->head, > + OPP_EVENT_ADD, new_opp); > + } else { > + mutex_unlock(&dev_opp_list_lock); > + kfree(new_opp); > + } > > - /* > - * Notify the changes in the availability of the operable > - * frequency/voltage list. > - */ > - srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ADD, new_opp); > return 0; > } > EXPORT_SYMBOL_GPL(dev_pm_opp_add); > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/