Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221024102307.33722-1-angelogioacchino.delregno@collabora.com> <20221024102307.33722-3-angelogioacchino.delregno@collabora.com>
In-Reply-To: <20221024102307.33722-3-angelogioacchino.delregno@collabora.com>
From:   Chen-Yu Tsai <wenst@chromium.org>
Date:   Mon, 24 Oct 2022 14:58:29 -0700
Message-ID: <CAGXv+5FPUchS36ObBZQ73zh7cR5wyfk6bpNO4u5h3AH1GzSP0Q@mail.gmail.com>
Subject: Re: [PATCH 02/10] clk: mediatek: mt8186-topckgen: Drop flags for
 main/univpll fixed factors
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>
Cc:     sboyd@kernel.org, mturquette@baylibre.com, matthias.bgg@gmail.com,
        miles.chen@mediatek.com, nfraprado@collabora.com,
        rex-bc.chen@mediatek.com, chun-jie.chen@mediatek.com,
        jose.exposito89@gmail.com, yangyingliang@huawei.com,
        msp@baylibre.com, linux-clk@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Oct 24, 2022 at 3:23 AM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> The mainpll and univpll clocks are used as clock sources for multiple
> peripherals of different kind, some of which are critical (like AXIs);
> a rate change on any of these two will produce a rate change on many
> devices and that's likely to produce system instability if not done
> correctly: this is the reason why we have "fixed factor" clocks, used
> by MUX clocks to provide different rates based on PLL output dividers.
>
> Though, there's one fundamental issue that must be resolved somehow:
>
> When performing GPU DVFS, we get a rate request that will try to change
> the frequency of MAINPLL due to the CLK_TOP_MFG mux having clk26m,
> mfgpll (the GPU dedicated PLL), mainpll_d3, mainpll_d5 (fixed factor
> dividers) as possible parents.
>
> In order to solve that, there are two ways:
>  1. Add new "fake" mainpll_d3_fixed, mainpll_d5_fixed clocks, clones
>     of mainpll_d3, mainpll_d5 clocks, for the only purpose of not
>     declaring CLK_SET_RATE_PARENT; or
>  2. Simply drop said flag from the original dividers.
>
> After some careful validation, I cannot see anything calling a rate
> change request during runtime for MAINPLL, nor for UNIVPLL (which would,
> again, mean that we're reclocking lots of peripherals at once!), so it
> is safe *and sane* to simply remove the CLK_SET_RATE_PARENT flag to all
> of the main/univpll fixed factor divider clocks.
>
> Besides, if for any (doubtful) reason main/univpll rate change will be
> required in the future, it's still possible to call that on the PLL main
> clocks, so we're still covered anyway.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

AFAIK this matches what we do for some other SoC families, so

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>