[PATCH 0/5] cpuidle: psci: Various improvements for PSCI PM domains

[Ulf Hansson]

Hi Lukaz,

On Tue, 30 Jun 2020 at 12:23, Lukasz Luba <lukasz.luba at arm.com> wrote:
>
> Hi Ulf,
>
> On 6/15/20 4:20 PM, Ulf Hansson wrote:
> > The main change in this series is done in patch 5/5, which implements support
> > to prevent domain idlestates until all PSCI PM domain consumers are ready for
> > it. To reach that point the corresponding code for cpuidle-psci and
> > cpuidle-psci-domain, needed to be converted into platform drivers, which is
> > done by the earlier changes in the series.
> >
> > Additionally, some improvements have been made to the error path, which becomes
> > easier when the code gets converted to platform drivers.
> >
> > Deployment for a Qcom SoC, which actually takes full benefit of these changes
> > are also in the pipe, but deferring then a bit until $subject series have been
> > discussed.
> >
> > Kind regards
> > Ulf Hansson
> >
> > Ulf Hansson (5):
> >    cpuidle: psci: Fail cpuidle registration if set OSI mode failed
> >    cpuidle: psci: Fix error path via converting to a platform driver
> >    cpuidle: psci: Split into two separate build objects
> >    cpuidle: psci: Convert PM domain to platform driver
> >    cpuidle: psci: Prevent domain idlestates until consumers are ready
> >
> >   drivers/cpuidle/Kconfig.arm           |  10 ++
> >   drivers/cpuidle/Makefile              |   5 +-
> >   drivers/cpuidle/cpuidle-psci-domain.c |  74 +++++++++-----
> >   drivers/cpuidle/cpuidle-psci.c        | 141 +++++++++++++++-----------
> >   drivers/cpuidle/cpuidle-psci.h        |  11 +-
> >   5 files changed, 150 insertions(+), 91 deletions(-)
> >
>
> Since I am interested in some CPU idle statistics (residency, etc),
> I would like to help you and Sudeep in reviewing the patch set.

Thanks, much appreciated!

>
> Could you clarify some bit below?
> 1. There is Qcom SoC which has dependencies between PSCI PM domain
> consumers and this CPU idle - thus we cannot register and use CPU
> idle driver till related drivers fully setup.

I think you got it right, but let me clarify.

On Qcom SoC, PSCI PM domain consumers aren't solely CPU devices, but
also the 'qcom,rpmh-rsc' device is a consumer, for example.

That doesn't mean the CPU idle driver can't be probed/initialized, but
rather that the PSCI PM domain must not be powered off. The power off
needs to wait until all the consumers of the PSCI PM domain have been
registered/probed.

See more details in the commit message of patch5.

> 2. The proposed solution is to use platform driver and plat. device
> for this CPU idle driver, to have access to deferred probe mechanism and
> wait for the consumer drivers fully probed state.

Correct, but let me fill in some more.

I would like to use the ->sync_state() callback of the PSCI PM domain
driver, as a way to understand that all consumers have been probed.

> 3. Do you have maybe some estimations how long it takes for these
> consumers to be fully probed?

I am not sure I understand the reason for the question.

Anyway, at this point, I am looking at the qcom,rpmh-rsc device, which
is being probed by the drivers/soc/qcom/rpmh-rsc.c driver. Moving
forward, in principle it can be any device/driver that is a consumer
of the PSCI PM domain. I am not even excluding that drivers can be
modules. It should work for all cases.

> 4. Changing just this CPU idle driver registration point (to
> late_initcall()) phase in time is not a solution.

Correct, it doesn't work.

Playing with initcalls doesn't guarantee anything when it comes to
making sure all consumers are ready.

Hope this clarifies things a bit for you, but just tell me if there is
anything more I can do to further explain things.

Kind regards
Uffe