On Wed, Dec 10, 2025 at 10:20:34PM +0100, Julien Stephan wrote: > Le mer. 10 déc. 2025 à 18:56, Tom Rini <[email protected]> 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 <[email protected]> wrote: > > > > > > > Le ven. 5 déc. 2025 à 14:47, Kory Maincent <[email protected]> a > > > > écrit : > > > > > > > > > > On Wed, 03 Dec 2025 10:31:53 +0100 > > > > > Julien Stephan <[email protected]> wrote: > > > > > > > > > > > From: Julien Masson <[email protected]> > > > > > > > > > > > > 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 <[email protected]> > > > > > > Signed-off-by: Julien Stephan <[email protected]> > > > > > > --- > > > > > > 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
signature.asc
Description: PGP signature
