Re: [PATCH v3 2/5] pmdomain: mediatek: Fix possible nullptr KP in HWV cleanup/on-check

[Ulf Hansson]

On Thu, Jul 9, 2026 at 10:46 AM AngeloGioacchino Del Regno
<angelogioacchino.delregno@xxxxxxxxxxxxx> wrote:
>
> Should probe fail for HW_VOTER type power domains, this driver was
> unconditionally trying to perform cleanup for DIRECT_CTL domains,
> but only after checking if the target domain is powered on... with
> the DIRECT_CTL scpsys_domain_is_on() code again.
>
> And there's more: the scpsys_domain_is_on() function is also being
> unconditionally used in the probe path, for any power domain that
> has flag MTK_SCPD_KEEP_DEFAULT_OFF!
>
> This bug was never experienced by anyone because the HWV domains
> never failed probe, and because none of those is declared with the
> aforementioned flag - but it's still something critical.
>
> In order to fix this, add a check for MTCMOS Type and, based on
> that, call the correct functions for an "is on" check, and also
> do the same for the cleanup path, calling the correct functions
> for the "power off" action.
>
> For the latter, since there's a call to pm_genpd_remove() right
> before calling power_off, be cautious and add a variation of the
> power off functions (with a _internal suffix) for those to get a
> pointer to scpsys_domain instead of one to generic_pm_domain as,
> even if that's still working, this is way too much fragile and
> would break at some point.
>
> Fixes: 88914db077b6 ("pmdomain: mediatek: Add support for Hardware Voter power domains")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>

Applied for fixes and by adding a stable tag, thanks!

Kind regards
Uffe


> ---
>  drivers/pmdomain/mediatek/mtk-pm-domains.c | 40 ++++++++++++++++++----
>  1 file changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> index e1cfd4223473..f0a6339affd7 100644
> --- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> +++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> @@ -393,9 +393,8 @@ static int scpsys_hwv_power_on(struct generic_pm_domain *genpd)
>         return ret;
>  };
>
> -static int scpsys_hwv_power_off(struct generic_pm_domain *genpd)
> +static int scpsys_hwv_power_off_internal(struct scpsys_domain *pd)
>  {
> -       struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
>         const struct scpsys_hwv_domain_data *hwv = pd->hwv_data;
>         struct scpsys *scpsys = pd->scpsys;
>         u32 val;
> @@ -464,6 +463,13 @@ static int scpsys_hwv_power_off(struct generic_pm_domain *genpd)
>         return ret;
>  };
>
> +static int scpsys_hwv_power_off(struct generic_pm_domain *genpd)
> +{
> +       struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
> +
> +       return scpsys_hwv_power_off_internal(pd);
> +}
> +
>  static int scpsys_ctl_pwrseq_on(struct scpsys_domain *pd)
>  {
>         struct scpsys *scpsys = pd->scpsys;
> @@ -694,9 +700,8 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
>         return ret;
>  }
>
> -static int scpsys_power_off(struct generic_pm_domain *genpd)
> +static int scpsys_power_off_internal(struct scpsys_domain *pd)
>  {
> -       struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
>         struct scpsys *scpsys = pd->scpsys;
>         bool tmp;
>         int ret;
> @@ -737,6 +742,13 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
>         return 0;
>  }
>
> +static int scpsys_power_off(struct generic_pm_domain *genpd)
> +{
> +       struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
> +
> +       return scpsys_power_off_internal(pd);
> +}
> +
>  static struct
>  generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node)
>  {
> @@ -884,7 +896,14 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
>          * late_init time.
>          */
>         if (MTK_SCPD_CAPS(pd, MTK_SCPD_KEEP_DEFAULT_OFF)) {
> -               if (scpsys_domain_is_on(pd))
> +               bool domain_is_on;
> +
> +               if (scpsys->soc_data->type == SCPSYS_MTCMOS_TYPE_HW_VOTER)
> +                       domain_is_on = scpsys_hwv_domain_is_enable_done(pd);
> +               else
> +                       domain_is_on = scpsys_domain_is_on(pd);
> +
> +               if (domain_is_on)
>                         dev_warn(scpsys->dev,
>                                  "%pOF: A default off power domain has been ON\n", node);
>         } else {
> @@ -973,6 +992,7 @@ static int scpsys_add_subdomain(struct scpsys *scpsys, struct device_node *paren
>
>  static void scpsys_remove_one_domain(struct scpsys_domain *pd)
>  {
> +       struct scpsys *scpsys = pd->scpsys;
>         int ret;
>
>         /*
> @@ -984,8 +1004,14 @@ static void scpsys_remove_one_domain(struct scpsys_domain *pd)
>                 dev_err(pd->scpsys->dev,
>                         "failed to remove domain '%s' : %d - state may be inconsistent\n",
>                         pd->genpd.name, ret);
> -       if (scpsys_domain_is_on(pd))
> -               scpsys_power_off(&pd->genpd);
> +
> +       if (scpsys->soc_data->type == SCPSYS_MTCMOS_TYPE_HW_VOTER) {
> +               if (scpsys_hwv_domain_is_enable_done(pd))
> +                       scpsys_hwv_power_off_internal(pd);
> +       } else {
> +               if (scpsys_domain_is_on(pd))
> +                       scpsys_power_off_internal(pd);
> +       }
>
>         clk_bulk_put(pd->num_clks, pd->clks);
>         clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
> --
> 2.54.0
>