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

[Kory Maincent]

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.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com