On Tue, 7 Jul 2020 at 14:37, Lukasz Luba [off-list ref] wrote:

On 7/7/20 12:53 PM, Ulf Hansson wrote:

quoted

Hi Lukaz,

On Tue, 30 Jun 2020 at 12:23, Lukasz Luba [off-list ref] wrote:

quoted

Hi Ulf,

On 6/15/20 4:20 PM, Ulf Hansson wrote:

quoted

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!

quoted

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.

quoted

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.

quoted

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.

I was wondering if this is because of HW issue of long setup, thus
we need to wait a bit longer with drivers deferred probing.
But this is not the case as I can see now.

quoted

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.

The late_initcall won't help, this is a really tough requirement:
being a module...

quoted

quoted

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.

I agree, especially when modules are involved.

quoted

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

Yes, thank you. I can see now why you need this explicit ->sync_state()
callback.
I don't see better solution to what you have proposed here.
I will go through the patches once again to check and add some
reviewed-by.

Thanks a lot for your time! I am just about to post a v2, to re-order
the series so patch 3 comes first, according to suggestions from Lina.

Please move over to review that version instead, in a few minutes.

Kind regards
Uffe

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