Re: [PATCH v4 13/14] cpuidle: psci: Add support for PM domains by using genpd

On Fri, Dec 20, 2019 at 12:27:39PM +0100, Ulf Hansson wrote:

On Fri, 20 Dec 2019 at 11:07, Sudeep Holla [off-list ref] wrote:

[...]

quoted

Even if you don't create all these genpd domains, it is still degraded
mode and we are anyway not changing that. Let me know if my understanding
is wrong here.

Your understanding is wrong.

If I remove the genpds because psci_set_osi_mode() fails, then in the
current suggested initialization path, that will lead to that the
entire cpuidle-psci driver will fail to initiate (which is because
psci_dt_attach_cpu() returns an error). In other words, only WFI state
will be used by cpuidle as there will be no cpuidle driver registered
at all.

That would not be an acceptable behaviour, as it would make the
situation worse than today.

What we want in this scenario is to keep using all the idle states for
the CPUs, but ignores those for the cluster. That we both agree on,
right?

Yes, I agree and understand that. I was assuming as part of this change
you will fixup psci_dt_cpu_init_idle not to return error but just allow
CPU level idle. Sorry if that was not clear, I was always assuming that.

quoted

I am sure, DTB may get copied to different platform and the firmware may
not support OSI. I know we have logs, but creating and leaving those
genpd domains unused will be just confusing. Please change that.

We are not creating any genpds unless OSI mode is supported. We do not
even try to attach CPUs to the PM domains, unless OSI mode is
supported. So this should already work according to your expectations
and previous requests.

Yes I understand, but checking if "OSI mode is supported" is not same as
"setting OSI mode". Until OSI mode is set, it is default/PC mode, so we
need to work based on that assumption.

To address your concern about removing genpds when psci_set_osi_mode()
fails, we also need to address the problems we get when calling
psci_dt_attach_cpu(). There are two viable options as I see it.

Shouldn't that fail ? Sorry, I might be missing something.

1. Prevent calling psci_dt_attach_cpu() altogether when
psci_set_osi_mode() failed. This means another function needs to be
shared from cpuidle-psci-domain.c to let cpuidle-psci.c know about it.

If we don't create any genpd, will psci_dt_attach_cpu fail ?

2. We can let psci_dt_attach_cpu() return NULL, when
psci_set_osi_mode() failed - as this information is already known by
cpuidle-psci-domain.c.

Yes I was making all the arguments/discussion based on that. Do you see
any issues with that ? Any races possible ?

I vote for option 2, but what do you think?

Me too from the time I started the discussion, I assume a lot and
don't put this into words in the email.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel