On Tue, 14 Nov 2023 at 11:08, Stephan Gerhold
<stephan.gerhold@xxxxxxxxxxxxxxx> wrote:
>
> From the Linux point of view, the power domains used by the CPU must
> stay always-on. This is because we still need the CPU to keep running
> until the last instruction, which will typically be a firmware call that
> shuts down the CPU cleanly.
>
> At the moment the power domain votes (enable + performance state) are
> dropped during system suspend, which means the CPU could potentially
> malfunction while entering suspend.
>
> We need to distinguish between two different setups used with
> qcom-cpufreq-nvmem:
>
> 1. CPR power domain: The backing regulator used by CPR should stay
> always-on in Linux; it is typically disabled automatically by
> hardware when the CPU enters a deep idle state. However, we
> should pause the CPR state machine during system suspend.
>
> 2. RPMPD: The power domains used by the CPU should stay always-on
> in Linux (also across system suspend). The CPU typically only
> uses the *_AO ("active-only") variants of the power domains in
> RPMPD. For those, the RPM firmware will automatically drop
> the votes internally when the CPU enters a deep idle state.
>
> Make this work correctly by calling device_set_awake_path() on the
> virtual genpd devices, so that the votes are maintained across system
> suspend. The power domain drivers need to set GENPD_FLAG_ACTIVE_WAKEUP
> to opt into staying on during system suspend.
>
> For now we only set this for the RPMPD case. For CPR, not setting it
> will ensure the state machine is still paused during system suspend,
> while the backing regulator will stay on with "regulator-always-on".
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@xxxxxxxxxxxxxxx>
Reviewed-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
Kind regards
Uffe
> ---
> This patch can be merged independently from the pmdomain one for RPMPD.
> Both are needed to actually preserve the votes during system suspend but
> there is no compile-time dependency.
> ---
> drivers/cpufreq/qcom-cpufreq-nvmem.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index d239a45ed497..ea05d9d67490 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -23,6 +23,7 @@
> #include <linux/nvmem-consumer.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/pm.h>
> #include <linux/pm_domain.h>
> #include <linux/pm_opp.h>
> #include <linux/pm_runtime.h>
> @@ -426,6 +427,18 @@ static const struct qcom_cpufreq_match_data match_data_ipq8074 = {
> .get_version = qcom_cpufreq_ipq8074_name_version,
> };
>
> +static void qcom_cpufreq_suspend_virt_devs(struct qcom_cpufreq_drv *drv, unsigned int cpu)
> +{
> + const char * const *name = drv->data->genpd_names;
> + int i;
> +
> + if (!drv->cpus[cpu].virt_devs)
> + return;
> +
> + for (i = 0; *name; i++, name++)
> + device_set_awake_path(drv->cpus[cpu].virt_devs[i]);
> +}
> +
> static void qcom_cpufreq_put_virt_devs(struct qcom_cpufreq_drv *drv, unsigned int cpu)
> {
> const char * const *name = drv->data->genpd_names;
> @@ -578,11 +591,25 @@ static void qcom_cpufreq_remove(struct platform_device *pdev)
> }
> }
>
> +static int qcom_cpufreq_suspend(struct device *dev)
> +{
> + struct qcom_cpufreq_drv *drv = dev_get_drvdata(dev);
> + unsigned int cpu;
> +
> + for_each_possible_cpu(cpu)
> + qcom_cpufreq_suspend_virt_devs(drv, cpu);
> +
> + return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(qcom_cpufreq_pm_ops, qcom_cpufreq_suspend, NULL);
> +
> static struct platform_driver qcom_cpufreq_driver = {
> .probe = qcom_cpufreq_probe,
> .remove_new = qcom_cpufreq_remove,
> .driver = {
> .name = "qcom-cpufreq-nvmem",
> + .pm = pm_sleep_ptr(&qcom_cpufreq_pm_ops),
> },
> };
>
>
> --
> 2.39.2
>
