Re: [PATCH 0/7] Register MPM under CPU cluster power domain to manage RPM notification

[Christopher Obbard <chris.obbard@oss.qualcomm.com>]

Hi Sneh,


> MPM irqchip needs to notify RPM (Resource Power Manager) processor to read
> the latest wake up capable interrupts when the CPU cluster is entering the
> deepest idle state. This is done by sending IPC interrupt to RPM and is
> implemented as .power_off() callback by registering MPM as parent power
> domain to CPU cluster.
> 
> Such implementation introduces a hard probe dependency between MPM irqchip
> and CPU cluster power domains. That is MPM irqchip needs to finish probe
> before PSCI power domains are probed. MPM irqchip can be build as module
> and can get later inserted where as PSCI power domains is not a module.
> 
> For in-built driver cases too PSCI domain gets probed first and later MPM
> irqchip leading to failure of CPUidle states.

..snip..


> commit af5376a77e87 ("cpuidle: psci: Transition to the faux device
> interface") transitioned cpuidle-psci to a faux device interface.
> 
> faux_device_create() calls faux_device_create_with_groups(), which ignores
> the probe return value, and destroys the device if dev->driver is not set.

This is no longer true, see Commit 0606f2114e2d ("cpuidle: psci: Fix
support for probe deferral by dropping the faux device").


..snip..


> Currently only 2 SoCs follow this method - Agatti and sm6375. Agatti has
> CPU cluster power domain disabled, which is why idle-states are allowed to
> function there.

Please don't forget about the devices which use the SoC: For Agatti, the
RB1 (qrb2210-rb1) and Arduino UnoQ (qrb2210-arduino-imola) are both
setup
with different power domains (off-topic: I believe this is something
which can be reworked soon as the mpm driver could be in a better place
than before? Didn't check.).


> Move the RPM notification handling to the GENPD_NOTIFY_PRE_OFF callback and
> register MPM under the CPU cluster power domain.  Use runtime PM to report
> the default RPM_SUSPENDED state to genPD so that the CPU cluster power
> domain can enter low power mode.
> 
> This will remove the dependency on probe ordering and allow individual CPU
> idle states, CPU cluster idle states and RPM notification to function
> properly.
> 
> Also enable CPU and CPU cluster LPMs for Shikra.
> 
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> ---
> Sneh Mankad (7):
>       dt-bindings: interrupt-controller: mpm: Document power-domains property
>       irqchip/irq-qcom-mpm: Register MPM under CPU cluster power domain
>       irqchip/irq-qcom-mpm: Prepare common access path for timer and pin regs
>       irqchip/irq-qcom-mpm: Program wakeup timer when CPU cluster goes to LPM
>       arm64: dts: qcom: sm6375: Make MPM device as part of CPU cluster domain
>       arm64: dts: qcom: agatti: Do not mark MPM as power domain
>       arm64: dts: qcom: shikra: Add CPU idle states



I also noticed that patches 2 and 6 are a matched pair but sit four
patches apart. Every commit in between leaves Agatti device with no
working CPU idle and e.g. a bisect can land inside that window. Can you
move the DTS changes first?


Cheers!

Chris