| Message ID | 804c420eb23e70448c3c2c93f867a0eab3536bfd.1439187821.git.viresh.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 10-08-15, 12:23, Stephen Boyd wrote: > On 08/10, Viresh Kumar wrote: > > 'dev_opp' will always be NULL in _of_init_opp_table_v2() after creating > > OPPs for a device. There is no point comparing it against NULL there. > > > > Restructure code a bit to make it more efficient. > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > Curious if these are a response to the static checker mails? If > so it would be good to add a reported-by tag. No it wasn't, I had these waiting in my queue even before Rafael applied the earlier ones. I was waiting for them to get in before sending more stuff :) > > --- > > drivers/base/power/opp.c | 21 ++++++++++----------- > > 1 file changed, 10 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c > > index 1daaa1a418a2..c9747fb192b1 100644 > > --- a/drivers/base/power/opp.c > > +++ b/drivers/base/power/opp.c > > @@ -1295,20 +1295,19 @@ static int _of_init_opp_table_v2(struct device *dev, > > if (WARN_ON(!count)) > > goto out; > > > > - if (!ret) { > > - if (!dev_opp) { > > - dev_opp = _find_device_opp(dev); > > - if (WARN_ON(!dev_opp)) > > - goto out; > > - } > > - > > - dev_opp->np = opp_np; > > - dev_opp->shared_opp = of_property_read_bool(opp_np, > > - "opp-shared"); > > - } else { > > + if (ret) { > > of_free_opp_table(dev); > > + goto out; > > } > > > > + dev_opp = _find_device_opp(dev); > > + if (WARN_ON(!dev_opp)) > > + goto out; > > Doesn't ret = 0 in this case? Because ret is already 0, juse see the above if (ret) check. > Why not drop the goto and just > return some error code. Same for the goto out up above. Actually yes, because we don't do anything special in goto now. But it required more (unrelated) code changes, plus I didn't wanted to break the 'return from single place' rule for this function, in case we really need to free some resource or undo some work from the goto place. But if you suggest/insist, then I will do it in a separate patch.
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 1daaa1a418a2..c9747fb192b1 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -1295,20 +1295,19 @@ static int _of_init_opp_table_v2(struct device *dev, if (WARN_ON(!count)) goto out; - if (!ret) { - if (!dev_opp) { - dev_opp = _find_device_opp(dev); - if (WARN_ON(!dev_opp)) - goto out; - } - - dev_opp->np = opp_np; - dev_opp->shared_opp = of_property_read_bool(opp_np, - "opp-shared"); - } else { + if (ret) { of_free_opp_table(dev); + goto out; } + dev_opp = _find_device_opp(dev); + if (WARN_ON(!dev_opp)) + goto out; + + dev_opp->np = opp_np; + dev_opp->shared_opp = of_property_read_bool(opp_np, + "opp-shared"); + out: return ret; }
'dev_opp' will always be NULL in _of_init_opp_table_v2() after creating OPPs for a device. There is no point comparing it against NULL there. Restructure code a bit to make it more efficient. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/base/power/opp.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)