MIME-Version: 1.0
References: <20230206152928.918562-1-angelogioacchino.delregno@collabora.com> <20230206152928.918562-20-angelogioacchino.delregno@collabora.com>
In-Reply-To: <20230206152928.918562-20-angelogioacchino.delregno@collabora.com>
From:   Chen-Yu Tsai <wenst@chromium.org>
Date:   Tue, 7 Feb 2023 17:58:45 +0800
Message-ID: <CAGXv+5G0ksgGMXUGk-=CXUANtGQa2M5RLY+wG7c-0cMSKS5DJQ@mail.gmail.com>
Subject: Re: [PATCH v1 19/45] clk: mediatek: mt8183: Convert all remaining
 clocks to common probe
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>
Cc:     mturquette@baylibre.com, sboyd@kernel.org, matthias.bgg@gmail.com,
        johnson.wang@mediatek.com, miles.chen@mediatek.com,
        chun-jie.chen@mediatek.com, daniel@makrotopia.org,
        fparent@baylibre.com, msp@baylibre.com, nfraprado@collabora.com,
        rex-bc.chen@mediatek.com, zhaojh329@gmail.com,
        sam.shih@mediatek.com, edward-jw.yang@mediatek.com,
        yangyingliang@huawei.com, granquet@baylibre.com,
        pablo.sun@mediatek.com, sean.wang@mediatek.com,
        chen.zhong@mediatek.com, linux-kernel@vger.kernel.org,
        linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Feb 6, 2023 at 11:30 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Switch to mtk_clk_simple_{probe,remove}() for infracfg and topckgen
> clocks on MT8183 to allow full module build for clock drivers.
> In order to do this, like done for other MediaTek clock drivers, it
> was necessary to join top_early_divs with top_divs and to stop
> registering the `clk13m` clock early.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/clk/mediatek/clk-mt8183.c | 160 ++++++------------------------
>  1 file changed, 28 insertions(+), 132 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mt8183.c b/drivers/clk/mediatek/clk-mt8183.c
> index 0fad2cf7f41b..035fdd02f0be 100644
> --- a/drivers/clk/mediatek/clk-mt8183.c
> +++ b/drivers/clk/mediatek/clk-mt8183.c
> @@ -25,11 +25,8 @@ static const struct mtk_fixed_clk top_fixed_clks[] = {
>         FIXED_CLK(CLK_TOP_UNIVP_192M, "univpll_192m", "univpll", 192000000),
>  };
>
> -static const struct mtk_fixed_factor top_early_divs[] = {
> -       FACTOR(CLK_TOP_CLK13M, "clk13m", "clk26m", 1, 2),
> -};
> -
>  static const struct mtk_fixed_factor top_divs[] = {
> +       FACTOR(CLK_TOP_CLK13M, "clk13m", "clk26m", 1, 2),

A clock with the same name is now present in the DT, and so this clock
would fail to register. We should drop this one completely and point
any references to it internally to "csw_f26m_ck_d2".

>         FACTOR(CLK_TOP_F26M_CK_D2, "csw_f26m_ck_d2", "clk26m", 1, 2),

MT8192 and MT8195 aren't affected because they only have "csw_f26m_ck_d2",
which systimer was referencing.

>         FACTOR_FLAGS(CLK_TOP_SYSPLL_CK, "syspll_ck", "mainpll", 1, 1, 0),
>         FACTOR_FLAGS(CLK_TOP_SYSPLL_D2, "syspll_d2", "syspll_ck", 1, 2, 0),
> @@ -809,26 +806,6 @@ static const struct mtk_clk_rst_desc clk_rst_desc = {
>         .rst_bank_nr = ARRAY_SIZE(infra_rst_ofs),
>  };
>
> -static struct clk_hw_onecell_data *top_clk_data;
> -
> -static void clk_mt8183_top_init_early(struct device_node *node)
> -{
> -       int i;
> -
> -       top_clk_data = mtk_alloc_clk_data(CLK_TOP_NR_CLK);
> -
> -       for (i = 0; i < CLK_TOP_NR_CLK; i++)
> -               top_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);
> -
> -       mtk_clk_register_factors(top_early_divs, ARRAY_SIZE(top_early_divs),
> -                       top_clk_data);

And since we used to not do error checking, the name conflict was OK.
With the new common probe, it's not.

ChenYu