On Tue, May 20, 2014 at 7:05 AM, Viresh Kumar <viresh.ku...@linaro.org> wrote: > On 20 May 2014 16:56, Viresh Kumar <viresh.ku...@linaro.org> wrote: >> But we aren't talking about failure here. Its not failure. The operation >> we are trying to do is already done and nothing should break if the >> OPP was already there or its added now. Its all the same. > > Though after more thought into this I feel this must also be done: > > diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c > index bdf09f5..3f540d8 100644 > --- a/drivers/base/power/opp.c > +++ b/drivers/base/power/opp.c > @@ -453,9 +453,13 @@ int dev_pm_opp_add(struct device *dev, unsigned > long freq, unsigned long u_volt) > } > > if (new_opp->rate == opp->rate) { > + int ret = 0; > + > + if (new_opp->u_volt == opp->u_volt) > + ret = -EEXIST; Else -> we now have two OPPs with the same key (same frequency, but different voltage) -> That does not make sense. Example: why would you add: If you already had {1GHz, 1.2V} and you attempted: {1GHz, 1.1V} (if you could do that, then you should added {1GHz, 1.1V} in the first place) OR {1GHz, 1.3V} (if you could do that, then you should add {1GHz, 1.3V} and the {1GHz, 1.2V} is wrong)
In addition, if old OPP was disabled, that new OPP would be in enabled state -> which also does not make sense either - since we disabled that frequency for some reason. > mutex_unlock(&dev_opp_list_lock); > kfree(new_opp); > - return 0; > + return ret; > } > > list_add_rcu(&new_opp->node, head); The reason I ask to return error is because if we attempt to add two OPPs with the same key (frequency), then it breaks the logic in remaining search logic. Regards, Nishanth Menon -- 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/