Re: [PATCH] pmdomain: mediatek: Don't power off default-off domains in probe cleanup

[Ulf Hansson]

On Tue, Oct 6, 2026 at 4:53 AM Nils Büchner <nils@xxxxxxxxxxx> wrote:
>
> scpsys_remove_one_domain() runs the power-off sequence for every domain
> that reads back as on. Domains with MTK_SCPD_KEEP_DEFAULT_OFF are
> registered with genpd as off and are never powered on by the driver
> before the provider is added, so their clocks are not enabled. When the
> bootloader left such a domain on and scpsys_probe() fails (typically
> -EPROBE_DEFER) after it was added, the cleanup disables those clocks
> anyway, triggering "already disabled"/"already unprepared" warnings from
> the clock core.
>
> Seen on MT8183 Juniper once the audio domain is KEEP_DEFAULT_OFF: six
> clk_core_disable/unprepare WARNs with backtraces on every boot.
>
> Skip the power-off sequence for these domains; only drop the clock
> references.
>
> Runtime-tested on an Acer Chromebook Spin 311 (MT8183, juniper) with a
> 7.0-based kernel (the hardware is not set up to boot mainline);
> build-tested on mainline with W=1. On the device with the audio
> domain marked MTK_SCPD_KEEP_DEFAULT_OFF: the six clk core warnings on
> the deferred first probe are gone and audio still works.
>
> The change and this changelog were drafted with an AI coding assistant
> during a debugging session on the hardware; they were reviewed and
> tested by the submitter.
>
> Fixes: c1f3163d8ff3 ("soc: mediatek: pm-domains: Add default power off flag")
> Assisted-by: Claude Opus 5.5
> Signed-off-by: Nils Büchner <nils@xxxxxxxxxxx>

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

Kind regards
Uffe


> ---
>
> This is a follow-up to "pmdomain: mediatek: mt8183: Fix silent audio
> from premature sync_state" (sent 2026-10-05), which marks the MT8183
> audio domain MTK_SCPD_KEEP_DEFAULT_OFF. With that applied, the firmware
> leaves the audio domain on at boot, so a deferred first probe of
> scpsys runs into this cleanup path on every boot. The fix itself is
> independent and applies on its own.
>
>  drivers/pmdomain/mediatek/mtk-pm-domains.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> index f0fa79e01..2b41bb8ed 100644
> --- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> +++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> @@ -1100,6 +1100,16 @@ static void scpsys_remove_one_domain(struct scpsys_domain *pd)
>                         "failed to remove domain '%s' : %d - state may be inconsistent\n",
>                         pd->genpd.name, ret);
>
> +       /*
> +        * Domains with MTK_SCPD_KEEP_DEFAULT_OFF are registered as off and were
> +        * never powered on by this driver (nothing can use them before the
> +        * provider is added), so their clocks were never enabled. If the
> +        * bootloader left one on, running the power-off sequence here would
> +        * disable clocks that are not enabled.
> +        */
> +       if (MTK_SCPD_CAPS(pd, MTK_SCPD_KEEP_DEFAULT_OFF))
> +               goto put_clocks;
> +
>         if (scpsys->soc_data->type == SCPSYS_MTCMOS_TYPE_HW_VOTER) {
>                 if (scpsys_hwv_domain_is_enable_done(pd))
>                         scpsys_hwv_power_off_internal(pd);
> @@ -1108,6 +1118,7 @@ static void scpsys_remove_one_domain(struct scpsys_domain *pd)
>                         scpsys_power_off_internal(pd);
>         }
>
> +put_clocks:
>         clk_bulk_put(pd->num_clks, pd->clks);
>         clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
>  }
> --
> 2.53.0
>