On 18-06-24, 17:50, Ulf Hansson wrote:
In _set_opp() we are normally bailing out when trying to set an OPP that is the current one. This make perfect sense, but becomes a problem when _set_required_opps() calls it recursively.
More precisely, when a required OPP is being shared by multiple PM domains, we end up skipping to request the corresponding performance-state for all of the PM domains, but the first one. Let's fix the problem, by calling _set_opp_level() from _set_required_opps() instead.
Fixes: e37440e7e2c2 ("OPP: Call dev_pm_opp_set_opp() for required OPPs") Cc: stable@vger.kernel.org Signed-off-by: Ulf Hansson ulf.hansson@linaro.org
drivers/opp/core.c | 47 +++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 23 deletions(-)
/* This is only called for PM domain for now */ static int _set_required_opps(struct device *dev, struct opp_table *opp_table, struct dev_pm_opp *opp, bool up) @@ -1091,7 +1113,8 @@ static int _set_required_opps(struct device *dev, struct opp_table *opp_table, if (devs[index]) { required_opp = opp ? opp->required_opps[index] : NULL;
ret = dev_pm_opp_set_opp(devs[index], required_opp);
ret = _set_opp_level(devs[index], opp_table,required_opp);
No, we won't be doing this I guess. Its like going back instead of moving forward :)
The required OPPs is not just a performance domain thing, but specially with devs[] here, it can be used to set OPP for any device type and so dev_pm_opp_set_opp() is the right call here.
Coming back to the problem you are pointing to, I am not very clear of the whole picture here. Can you please help me get some details on that ?
From what I understand, you have a device which has multiple power domains. Now all these power domains share the same OPP table in the device tree (i.e. to avoid duplication of tables only). Is that right ?
Even if in DT we have the same OPP table for all the domains, the OPP core will have separate OPP tables structures (as the domains aren't connected). And these OPP tables will have their own `current_opp` fields and so we shouldn't really bail out earlier.
Maybe there is a bug somewhere that is causing it. Maybe I can look at the DT to find the issue ? (Hint: The OPP table shouldn't have the `shared` flag set).
Maybe I completely misunderstood the whole thing :)