[PATCH 2/2] clk: mediatek: add MT8188 clock driver

[Julien Stephan]

Le mer. 10 déc. 2025 à 18:56, Tom Rini <trini at konsulko.com> a écrit :
>
> On Tue, Dec 09, 2025 at 11:03:20AM +0100, Kory Maincent wrote:
> > On Tue, 9 Dec 2025 10:27:32 +0100
> > Julien Stephan <jstephan at baylibre.com> wrote:
> >
> > > Le ven. 5 déc. 2025 à 14:47, Kory Maincent <kory.maincent at bootlin.com> a
> > > écrit :
> > > >
> > > > On Wed, 03 Dec 2025 10:31:53 +0100
> > > > Julien Stephan <jstephan at baylibre.com> wrote:
> > > >
> > > > > From: Julien Masson <jmasson at baylibre.com>
> > > > >
> > > > > The following clocks have been added for MT8188 SoC:
> > > > > apmixedsys, topckgen, infracfg, pericfg and imp_iic_wrap
> > > > >
> > > > > These clocks driver are based on the ones present in the kernel:
> > > > > drivers/clk/mediatek/clk-mt8188-*
> > > > >
> > > > > Signed-off-by: Julien Masson <jmasson at baylibre.com>
> > > > > Signed-off-by: Julien Stephan <jstephan at baylibre.com>
> > > > > ---
> > > > >  drivers/clk/mediatek/Makefile     |    1 +
> > > > >  drivers/clk/mediatek/clk-mt8188.c | 1840
> > > > > +++++++++++++++++++++++++++++++++++++ 2 files changed, 1841 insertions(+)
> > > > >
> > > >
> > > > ...
> > > >
> > > > > +
> > > > > +static int mt8188_apmixedsys_probe(struct udevice *dev)
> > > > > +{
> > > > > +     return mtk_common_clk_init(dev, &mt8188_apmixedsys_clk_tree);
> > > > > +}
> > > > > +
> > > > > +static int mt8188_topckgen_probe(struct udevice *dev)
> > > > > +{
> > > > > +     return mtk_common_clk_init(dev, &mt8188_topckgen_clk_tree);
> > > > > +}
> > > > > +
> > > > > +static int mt8188_topckgen_cg_probe(struct udevice *dev)
> > > > > +{
> > > > > +     return mtk_common_clk_gate_init(dev, &mt8188_topckgen_cg_clk_tree,
> > > > > topckgen_cg_clks); +}
> > > > > +
> > > > > +static int mt8188_infracfg_ao_probe(struct udevice *dev)
> > > > > +{
> > > > > +     return mtk_common_clk_gate_init(dev, &mt8188_infracfg_ao_clk_tree,
> > > > > infracfg_ao_clks); +}
> > > > > +
> > > > > +static int mt8188_pericfg_ao_probe(struct udevice *dev)
> > > > > +{
> > > > > +     return mtk_common_clk_gate_init(dev, &mt8188_pericfg_ao_clk_tree,
> > > > > pericfg_ao_clks); +}
> > > > > +
> > > > > +static int mt8188_imp_iic_wrap_c_probe(struct udevice *dev)
> > > > > +{
> > > > > +     return mtk_common_clk_gate_init(dev,
> > > > > &mt8188_imp_iic_wrap_c_clk_tree, imp_iic_wrap_c_clks); +}
> > > > > +
> > > > > +static int mt8188_imp_iic_wrap_w_probe(struct udevice *dev)
> > > > > +{
> > > > > +     return mtk_common_clk_gate_init(dev,
> > > > > &mt8188_imp_iic_wrap_w_clk_tree, imp_iic_wrap_w_clks); +}
> > > > > +
> > > > > +static int mt8188_imp_iic_wrap_en_probe(struct udevice *dev)
> > > > > +{
> > > > > +     return mtk_common_clk_gate_init(dev,
> > > > > &mt8188_imp_iic_wrap_en_clk_tree,
> > > > > +                                     imp_iic_wrap_en_clks);
> > > > > +}
> > > > > +
> > > > > +static const struct udevice_id mt8188_apmixed_compat[] = {
> > > > > +     { .compatible = "mediatek,mt8188-apmixedsys", },
> > > > > +     { }
> > > > > +};
> > > > > +
> > > > > +static const struct udevice_id mt8188_topckgen_compat[] = {
> > > > > +     { .compatible = "mediatek,mt8188-topckgen", },
> > > > > +     { }
> > > > > +};
> > > > > +
> > > > > +static const struct udevice_id mt8188_topckgen_cg_compat[] = {
> > > > > +     { .compatible = "mediatek,mt8188-topckgen-cg", },
> > > > > +     { }
> > > > > +};
> > > > > +
> > > > > +static const struct udevice_id mt8188_infracfg_ao_compat[] = {
> > > > > +     { .compatible = "mediatek,mt8188-infracfg-ao", },
> > > > > +     { }
> > > > > +};
> > > > > +
> > > > > +static const struct udevice_id mt8188_pericfg_ao_compat[] = {
> > > > > +     { .compatible = "mediatek,mt8188-pericfg-ao", },
> > > > > +     { }
> > > > > +};
> > > > > +
> > > > > +static const struct udevice_id mt8188_imp_iic_wrap_c_compat[] = {
> > > > > +     { .compatible = "mediatek,mt8188-imp-iic-wrap-c", },
> > > > > +     { }
> > > > > +};
> > > > > +
> > > > > +static const struct udevice_id mt8188_imp_iic_wrap_w_compat[] = {
> > > > > +     { .compatible = "mediatek,mt8188-imp-iic-wrap-w", },
> > > > > +     { }
> > > > > +};
> > > > > +
> > > > > +static const struct udevice_id mt8188_imp_iic_wrap_en_compat[] = {
> > > > > +     { .compatible = "mediatek,mt8188-imp-iic-wrap-en", },
> > > > > +     { }
> > > > > +};
> > > >
> > > > Why don't you use udevice_id data field to describe each of the
> > > > mediatek,mt8188 version? This would avoid having this number of probe
> > > > function, udevice_id structs and U_BOOT_DRIVER definitions.
> > > >
> > >
> > > Hi Kory,
> > >
> > > I am not the original author, but I guess it's done that way to be as
> > > close as possible to the Kernel.
> >
> > Linux is using one file per clock driver so your argument does not work.
> > And even within a file if there are several sub possibility it uses data field:
> > https://elixir.bootlin.com/linux/v6.18/source/drivers/clk/mediatek/clk-mt8188-imp_iic_wrap.c#L64
> >
> > > Moreover all drivers in drivers/clk/mediatek/ are designed that way,
> > > so we keep consistency between Mediatek clock driverss
> >
> > Indeed this type of design seems quite common in U-boot. I don't really like
> > this as it adds more code for nothing. Anyway ok then.
>
> We can always rework things to be better :)
>

Hi Tom,

Sure!
I already sent a V2, but I can send a V3 reworked if you prefer.
Or do you prefer me to send a series later fixing all mediatek clock
drivers at once?
Let me know.

Cheers
Julien
> --
> Tom