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

[Tom Rini]

On Wed, Dec 10, 2025 at 10:20:34PM +0100, Julien Stephan wrote:
> 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.

I think reworking and cleaning up the mediatek clock drivers is good
follow-up work, and can be submitted on top of your current v2 (baring
other feedback).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20251210/ffedf2e0/attachment.sig>