Re: [PATCH] pmdomain: qcom: rpmhpd: Skip retention by default

[Ulf Hansson]

On Mon, Jun 29, 2026 at 10:37 PM Mike Tipton
<mike.tipton@xxxxxxxxxxxxxxxx> wrote:
>
> Retention is very rarely an operational corner. In the majority of
> cases, HW cannot operate properly at Retention levels and so the minimum
> operational level when enabling a rail is the first corner above
> Retention. A small subset of always-on use cases can operate at
> Retention, but those aren't controlled from HLOS.
>
> Currently, we allow Retention by default and only disallow it special
> cases. But this leaves us open to various failures when the PD is
> enabled without first being voted to an OPP above Retention. Such as
> when a child GDSC PD requests to enable its parent RPMh PD. In this
> case, the GDSC would get stuck powering on.
>
> Newer chips have started supporting Retention on rails that didn't
> previously (such as for MMCX). Instead of adding more special cases to
> skip Retention on MMCX, start skipping Retention by default since it's
> almost never desired from an HLOS perspective.
>
> Signed-off-by: Mike Tipton <mike.tipton@xxxxxxxxxxxxxxxx>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/pmdomain/qcom/rpmhpd.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pmdomain/qcom/rpmhpd.c b/drivers/pmdomain/qcom/rpmhpd.c
> index 63120e703923..35c598b33ac9 100644
> --- a/drivers/pmdomain/qcom/rpmhpd.c
> +++ b/drivers/pmdomain/qcom/rpmhpd.c
> @@ -41,7 +41,6 @@
>   * @addr:              Resource address as looped up using resource name from
>   *                     cmd-db
>   * @state_synced:      Indicator that sync_state has been invoked for the rpmhpd resource
> - * @skip_retention_level: Indicate that retention level should not be used for the power domain
>   */
>  struct rpmhpd {
>         struct device   *dev;
> @@ -58,7 +57,6 @@ struct rpmhpd {
>         const char      *res_name;
>         u32             addr;
>         bool            state_synced;
> -       bool            skip_retention_level;
>  };
>
>  struct rpmhpd_desc {
> @@ -191,7 +189,6 @@ static struct rpmhpd mxc = {
>         .pd = { .name = "mxc", },
>         .peer = &mxc_ao,
>         .res_name = "mxc.lvl",
> -       .skip_retention_level = true,
>  };
>
>  static struct rpmhpd mxc_ao = {
> @@ -199,7 +196,6 @@ static struct rpmhpd mxc_ao = {
>         .active_only = true,
>         .peer = &mxc,
>         .res_name = "mxc.lvl",
> -       .skip_retention_level = true,
>  };
>
>  static struct rpmhpd nsp = {
> @@ -1093,7 +1089,15 @@ static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd)
>                 return -EINVAL;
>
>         for (i = 0; i < rpmhpd->level_count; i++) {
> -               if (rpmhpd->skip_retention_level && buf[i] == RPMH_REGULATOR_LEVEL_RETENTION)
> +               /*
> +                * Most HW won't function properly at Retention. The minimum
> +                * operational level is the first level above Retention. The
> +                * small subset of HW that can operate at Retention isn't
> +                * controlled by HLOS. Skip the Retention level to avoid HW
> +                * failures when the PD is enabled without first having an
> +                * explicit OPP level set.
> +                */
> +               if (buf[i] == RPMH_REGULATOR_LEVEL_RETENTION)
>                         continue;
>
>                 rpmhpd->level[i] = buf[i];
>
> ---
> base-commit: 7de6ae9e12207ec146f2f3f1e58d1a99317e88bc
> change-id: 20260618-rpmhpd-skip-ret-430a25af1cce
>
> Best regards,
> --
> Mike Tipton <mike.tipton@xxxxxxxxxxxxxxxx>
>