Re: [PATCH v2 14/29] clk: mediatek: Add MT8196 vlpckgen clock support

[Chen-Yu Tsai <wenst@chromium.org>]

Hi,


On Tue, Jun 24, 2025 at 10:33 PM Laura Nao <laura.nao@collabora.com> wrote:
>
> Add support for the MT8196 vlpckgen clock controller, which provides
> muxes and dividers for clock selection in other IP blocks.
>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Laura Nao <laura.nao@collabora.com>
> ---
>  drivers/clk/mediatek/Makefile              |   2 +-
>  drivers/clk/mediatek/clk-mt8196-vlpckgen.c | 769 +++++++++++++++++++++
>  2 files changed, 770 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/clk/mediatek/clk-mt8196-vlpckgen.c
>
> diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> index 0688d7bf4979..24683dd51783 100644
> --- a/drivers/clk/mediatek/Makefile
> +++ b/drivers/clk/mediatek/Makefile
> @@ -161,7 +161,7 @@ obj-$(CONFIG_COMMON_CLK_MT8195_VENCSYS) += clk-mt8195-venc.o
>  obj-$(CONFIG_COMMON_CLK_MT8195_VPPSYS) += clk-mt8195-vpp0.o clk-mt8195-vpp1.o
>  obj-$(CONFIG_COMMON_CLK_MT8195_WPESYS) += clk-mt8195-wpe.o
>  obj-$(CONFIG_COMMON_CLK_MT8196) += clk-mt8196-apmixedsys.o clk-mt8196-topckgen.o \
> -                                  clk-mt8196-topckgen2.o
> +                                  clk-mt8196-topckgen2.o clk-mt8196-vlpckgen.o
>  obj-$(CONFIG_COMMON_CLK_MT8365) += clk-mt8365-apmixedsys.o clk-mt8365.o
>  obj-$(CONFIG_COMMON_CLK_MT8365_APU) += clk-mt8365-apu.o
>  obj-$(CONFIG_COMMON_CLK_MT8365_CAM) += clk-mt8365-cam.o
> diff --git a/drivers/clk/mediatek/clk-mt8196-vlpckgen.c b/drivers/clk/mediatek/clk-mt8196-vlpckgen.c
> new file mode 100644
> index 000000000000..23a673dd4c5c
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8196-vlpckgen.c
> @@ -0,0 +1,769 @@

[...]

> +static const char * const vlp_camtg0_parents[] = {
> +       "clk26m",
> +       "univpll_192m_d32",
> +       "univpll_192m_d16",
> +       "clk13m",
> +       "osc_d40",
> +       "osc_d32",
> +       "univpll_192m_d10",
> +       "univpll_192m_d8",
> +       "univpll_d6_d16",
> +       "ulposc3",
> +       "osc_d20",
> +       "ck2_tvdpll1_d16",
> +       "univpll_d6_d8"
> +};

It seems all the vlp_camtg* parents are the same. Please merge them
and just have one list.

> +static const char * const vlp_sspm_26m_parents[] = {
> +       "clk26m",
> +       "osc_d20"
> +};
> +
> +static const char * const vlp_ulposc_sspm_parents[] = {
> +       "clk26m",
> +       "osc_d2",
> +       "mainpll_d4_d2"
> +};
> +
> +static const char * const vlp_vlp_pbus_26m_parents[] = {
> +       "clk26m",
> +       "osc_d20"
> +};
> +
> +static const char * const vlp_debug_err_flag_parents[] = {
> +       "clk26m",
> +       "osc_d20"
> +};
> +
> +static const char * const vlp_dpmsrdma_parents[] = {
> +       "clk26m",
> +       "mainpll_d7_d2"
> +};
> +
> +static const char * const vlp_vlp_pbus_156m_parents[] = {
> +       "clk26m",
> +       "osc_d2",
> +       "mainpll_d7_d2",
> +       "mainpll_d7"
> +};
> +
> +static const char * const vlp_spm_parents[] = {
> +       "clk26m",
> +       "mainpll_d7_d4"
> +};
> +
> +static const char * const vlp_mminfra_parents[] = {
> +       "clk26m",
> +       "osc_d4",
> +       "mainpll_d3"
> +};
> +
> +static const char * const vlp_usb_parents[] = {
> +       "clk26m",
> +       "mainpll_d9"
> +};

The previous and the next one are the same.

> +static const char * const vlp_usb_xhci_parents[] = {
> +       "clk26m",
> +       "mainpll_d9"
> +};
> +
> +static const char * const vlp_noc_vlp_parents[] = {
> +       "clk26m",
> +       "osc_d20",
> +       "mainpll_d9"
> +};
> +
> +static const char * const vlp_audio_h_parents[] = {
> +       "clk26m",
> +       "vlp_apll1",
> +       "vlp_apll2"
> +};
> +
> +static const char * const vlp_aud_engen1_parents[] = {
> +       "clk26m",
> +       "apll1_d8",
> +       "apll1_d4"
> +};

The previous and the next one are the same.

> +static const char * const vlp_aud_engen2_parents[] = {
> +       "clk26m",
> +       "apll2_d8",
> +       "apll2_d4"
> +};
> +
> +static const char * const vlp_aud_intbus_parents[] = {
> +       "clk26m",
> +       "mainpll_d7_d4",
> +       "mainpll_d4_d4"
> +};
> +
> +static const char * const vlp_spvlp_26m

[...]

> +static int clk_mt8196_vlp_probe(struct platform_device *pdev)
> +{
> +       struct clk_hw_onecell_data *clk_data;
> +       int r;
> +       struct device_node *node = pdev->dev.of_node;
> +
> +       clk_data = mtk_alloc_clk_data(ARRAY_SIZE(vlp_muxes) +
> +                                     ARRAY_SIZE(vlp_plls));
> +       if (!clk_data)
> +               return -ENOMEM;
> +
> +       r = mtk_clk_register_muxes(&pdev->dev, vlp_muxes, ARRAY_SIZE(vlp_muxes),
> +                                  node, &mt8196_clk_vlp_lock, clk_data);
> +       if (r)
> +               goto free_clk_data;
> +
> +       r = mtk_clk_register_plls(node, vlp_plls, ARRAY_SIZE(vlp_plls),
> +                                 clk_data);
> +       if (r)
> +               goto unregister_muxes;
> +
> +       r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> +       if (r)
> +               goto unregister_plls;
> +
> +       platform_set_drvdata(pdev, clk_data);
> +
> +       return r;
> +
> +unregister_plls:
> +       mtk_clk_unregister_plls(vlp_plls, ARRAY_SIZE(vlp_plls), clk_data);
> +unregister_muxes:
> +       mtk_clk_unregister_muxes(vlp_muxes, ARRAY_SIZE(vlp_muxes), clk_data);
> +free_clk_data:
> +       mtk_free_clk_data(clk_data);

The AFE driver sets some tuner parameters in the VLPCKGEN block at probe
time. Maybe we could do that here instead?

/* vlp_cksys_clk: 0x1c016000 */
#define VLP_APLL1_TUNER_CON0 0x02a4
#define VLP_APLL2_TUNER_CON0 0x02a8

/* vlp apll1 tuner default value*/
#define VLP_APLL1_TUNER_CON0_VALUE 0x6f28bd4d
/* vlp apll2 tuner default value + 1*/
#define VLP_APLL2_TUNER_CON0_VALUE 0x78fd5265

       regmap_write(afe_priv->vlp_ck, VLP_APLL1_TUNER_CON0,
VLP_APLL1_TUNER_CON0_VALUE);
       regmap_write(afe_priv->vlp_ck, VLP_APLL2_TUNER_CON0,
VLP_APLL2_TUNER_CON0_VALUE);

ChenYu

> +
> +       return r;
> +}
> +
> +static void clk_mt8196_vlp_remove(struct platform_device *pdev)
> +{
> +       struct clk_hw_onecell_data *clk_data = platform_get_drvdata(pdev);
> +       struct device_node *node = pdev->dev.of_node;
> +
> +       of_clk_del_provider(node);
> +       mtk_clk_unregister_plls(vlp_plls, ARRAY_SIZE(vlp_plls), clk_data);
> +       mtk_clk_unregister_muxes(vlp_muxes, ARRAY_SIZE(vlp_muxes), clk_data);
> +       mtk_free_clk_data(clk_data);
> +}
> +
> +static const struct of_device_id of_match_clk_mt8196_vlp_ck[] = {
> +       { .compatible = "mediatek,mt8196-vlpckgen" },
> +       { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, of_match_clk_mt8196_vlp_ck);
> +
> +static struct platform_driver clk_mt8196_vlp_drv = {
> +       .probe = clk_mt8196_vlp_probe,
> +       .remove = clk_mt8196_vlp_remove,
> +       .driver = {
> +               .name = "clk-mt8196-vlpck",
> +               .of_match_table = of_match_clk_mt8196_vlp_ck,
> +       },
> +};
> +
> +MODULE_DESCRIPTION("MediaTek MT8196 VLP clock generator driver");
> +module_platform_driver(clk_mt8196_vlp_drv);
> +MODULE_LICENSE("GPL");
> --
> 2.39.5
>