2022-10-25 12:44:29

by Sudeep Holla

[permalink] [raw]
Subject: [PATCH] PM: domains: Fix handling of unavailable/disabled idle states

Platforms can provide the information about the availability of each
idle states via status flag. Platforms may have to disable one or more
idle states for various reasons like broken firmware or other unmet
dependencies.

Fix handling of such unavailable/disabled idle states by ignoring them
while parsing the states.

Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Ulf Hansson <[email protected]>
Fixes: a3381e3a65cb ("PM / domains: Fix up domain-idle-states OF parsing")
Signed-off-by: Sudeep Holla <[email protected]>
---
drivers/base/power/domain.c | 4 ++++
1 file changed, 4 insertions(+)

Hi Ulf,

As you already know, this change alone doesn't fix the issue reported here[1].
It also needs the fixes you have posted [2].

Regards,
Sudeep

[1] https://lore.kernel.org/all/[email protected]
[2] https://lore.kernel.org/all/[email protected]

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index ead135c7044c..6471b559230e 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2952,6 +2952,10 @@ static int genpd_iterate_idle_states(struct device_node *dn,
np = it.node;
if (!of_match_node(idle_state_match, np))
continue;
+
+ if (!of_device_is_available(np))
+ continue;
+
if (states) {
ret = genpd_parse_state(&states[i], np);
if (ret) {
--
2.38.1



2022-10-25 13:59:35

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH] PM: domains: Fix handling of unavailable/disabled idle states

On Tue, 25 Oct 2022 at 14:34, Sudeep Holla <[email protected]> wrote:
>
> Platforms can provide the information about the availability of each
> idle states via status flag. Platforms may have to disable one or more
> idle states for various reasons like broken firmware or other unmet
> dependencies.
>
> Fix handling of such unavailable/disabled idle states by ignoring them
> while parsing the states.
>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Ulf Hansson <[email protected]>
> Fixes: a3381e3a65cb ("PM / domains: Fix up domain-idle-states OF parsing")
> Signed-off-by: Sudeep Holla <[email protected]>

I think this should be tagged for stable kernels too. Rafael, can you
pick this up as a fix for v6.1rc[n]?

Reviewed-by: Ulf Hansson <[email protected]>

Kind regards
Uffe

> ---
> drivers/base/power/domain.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> Hi Ulf,
>
> As you already know, this change alone doesn't fix the issue reported here[1].
> It also needs the fixes you have posted [2].
>
> Regards,
> Sudeep
>
> [1] https://lore.kernel.org/all/[email protected]
> [2] https://lore.kernel.org/all/[email protected]

Well, I think it's better if we replace [1] with a patch that only
disables the cluster idle state. In that case, it would be sufficient
with $subject patch. In that case, we can just manage [2] separately.

Amit, can you submit a new version and test it together with $subject patch?

Kind regards
Uffe

>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index ead135c7044c..6471b559230e 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2952,6 +2952,10 @@ static int genpd_iterate_idle_states(struct device_node *dn,
> np = it.node;
> if (!of_match_node(idle_state_match, np))
> continue;
> +
> + if (!of_device_is_available(np))
> + continue;
> +
> if (states) {
> ret = genpd_parse_state(&states[i], np);
> if (ret) {
> --
> 2.38.1
>

2022-10-26 11:50:47

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] PM: domains: Fix handling of unavailable/disabled idle states

On Tue, Oct 25, 2022 at 2:59 PM Ulf Hansson <[email protected]> wrote:
>
> On Tue, 25 Oct 2022 at 14:34, Sudeep Holla <[email protected]> wrote:
> >
> > Platforms can provide the information about the availability of each
> > idle states via status flag. Platforms may have to disable one or more
> > idle states for various reasons like broken firmware or other unmet
> > dependencies.
> >
> > Fix handling of such unavailable/disabled idle states by ignoring them
> > while parsing the states.
> >
> > Cc: "Rafael J. Wysocki" <[email protected]>
> > Cc: Ulf Hansson <[email protected]>
> > Fixes: a3381e3a65cb ("PM / domains: Fix up domain-idle-states OF parsing")
> > Signed-off-by: Sudeep Holla <[email protected]>
>
> I think this should be tagged for stable kernels too. Rafael, can you
> pick this up as a fix for v6.1rc[n]?
>
> Reviewed-by: Ulf Hansson <[email protected]>

Applied as 6.1-rc material, thanks!

>
> > ---
> > drivers/base/power/domain.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > Hi Ulf,
> >
> > As you already know, this change alone doesn't fix the issue reported here[1].
> > It also needs the fixes you have posted [2].
> >
> > Regards,
> > Sudeep
> >
> > [1] https://lore.kernel.org/all/[email protected]
> > [2] https://lore.kernel.org/all/[email protected]
>
> Well, I think it's better if we replace [1] with a patch that only
> disables the cluster idle state. In that case, it would be sufficient
> with $subject patch. In that case, we can just manage [2] separately.
>
> Amit, can you submit a new version and test it together with $subject patch?
>
> Kind regards
> Uffe
>
> >
> > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> > index ead135c7044c..6471b559230e 100644
> > --- a/drivers/base/power/domain.c
> > +++ b/drivers/base/power/domain.c
> > @@ -2952,6 +2952,10 @@ static int genpd_iterate_idle_states(struct device_node *dn,
> > np = it.node;
> > if (!of_match_node(idle_state_match, np))
> > continue;
> > +
> > + if (!of_device_is_available(np))
> > + continue;
> > +
> > if (states) {
> > ret = genpd_parse_state(&states[i], np);
> > if (ret) {
> > --
> > 2.38.1
> >