Thread (21 messages) 21 messages, 2 authors, 2024-07-31

Re: [PATCH] OPP: Fix support for required OPPs for multiple PM domains

From: Ulf Hansson <hidden>
Date: 2024-07-31 10:36:17
Also in: linux-pm, linux-tegra, lkml, stable

[...]
quoted
That's right, but why do we want to call dev_pm_opp_set_opp() for the
multiple PM domain case then? It makes the behaviour inconsistent.
To have a common path for all required OPP device types, irrespective of the
fact that the required OPP device is a genpd or not. And we are talking about a
required OPP of a separate device here, it must be set via this call only,
technically speaking.

Genpd makes it a little complex, and I agree to that. But I strongly feel this
code needs to be generic and not genpd specific. The OPP core should have as
less genpd specific code as possible. It must handle all device types with a
single code path.
I agree that we really should avoid genpd specific code and that's
exactly what I am working towards too.

However, calling dev_pm_opp_set_opp() from _set_required_opps() looks
to me like it has the exact opposite effect:
*) To solve the bug according to the change you proposed, means more
genpd hacks.
**) To make the code for the required OPPs consistent between the
single/multiple PM domain case, we need additional genpd hacks.
***) We can't remove some of the existing genpd hacks [1], as those
would then still be needed.

Finally, while I understand that you prefer a single code path, we can
still keep _set_required_opps() common and generic. Today, it's used
only for performance-states of PM domains (the involved code isn't
even genpd specific as it calls
dev_pm_domain_set_performance_state()). If tomorrow we see a need to
extend it to additional resources, it's easy also without calling
dev_pm_opp_set_opp() from it.

Kind regards
Uffe

 [1]
https://lore.kernel.org/all/20240718234319.356451-7-ulf.hansson@linaro.org/ (local)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help