On Wed, Aug 12, 2015 at 12:13:09PM +0530, Viresh Kumar wrote:
If the first call to _opp_add_static_v2() fails we call of_free_opp_table() and you say that triggers a WARN().
No it doesn't.
So, coming back to the point you made about freeing table on !count, because there were no nodes present in the DT opp table, we have never tried to add any OPPs. And so there is no need to call of_free_opp_table() in that case.
Do you still think the current code is wrong ?
If it doesn't WARN() then it's not buggy, but it's still ugly. We should not call of_free_opp_table() because we *tried* to add an OPP, we should only call it if we *succeeded*.
The way the code is written and from your emails I was afraid that if you tried to call _opp_add_static_v2() and it fails then it leaves artifacts lying around that need to be cleaned up by the caller. This would be the ugliest scenario. But I looked at _opp_add_static_v2() and looks fine. It cleans up properly on failure. We only need to clean up if it succeeds.
regards, dan carpenter