| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGXv+5ErtDLNf3u5OdHrEBdrA-2bPA5wy32S+Bqd1c_1Z9u1pA@mail.gmail.com>
Date: Fri, 18 Jul 2025 16:31:18 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Laura Nao <laura.nao@...labora.com>
Cc: mturquette@...libre.com, sboyd@...nel.org, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, matthias.bgg@...il.com,
angelogioacchino.delregno@...labora.com, p.zabel@...gutronix.de,
richardcochran@...il.com, guangjie.song@...iatek.com,
linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, netdev@...r.kernel.org,
kernel@...labora.com
Subject: Re: [PATCH v2 14/29] clk: mediatek: Add MT8196 vlpckgen clock support
On Tue, Jul 15, 2025 at 3:28 PM Chen-Yu Tsai <wenst@...omium.org> wrote:
>
> Hi,
>
>
> On Tue, Jun 24, 2025 at 10:33 PM Laura Nao <laura.nao@...labora.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@...labora.com>
> > Signed-off-by: Laura Nao <laura.nao@...labora.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"
> > +};
Also, all these audio related clocks (audio_h, aud_engen1, aud_engen2
aud_intbus) have a "vlp_clk26m" clock as their parent. It should be:
- clk26m (clk26m from the top ckgen domain)
- vlp_clk26m (clk26m from the VLP domain)
- (from PLLs)
- (from PLLs)
Moreover, an offline discussion with the audio owner suggests that
of the two 26 MHz clock parents, we really just want the one from
the VLP domain, as that one is usable even under suspend. This
could be done by providing an index table.
ChenYu
> > +
> > +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
> >
Powered by blists - more mailing lists