Re: [PATCH v3 2/3] cpufreq: qcom-nvmem: Preserve PM domain votes in system suspend

[Ulf Hansson]

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
>