Messages in this thread | | | From | Matthias Brugger <> | Subject | Re: [PATCH v6 10/22] clk: mediatek: Add MT8192 basic clocks support | Date | Wed, 10 Feb 2021 13:46:00 +0100 |
| |
On 22/12/2020 14:09, Weiyi Lu wrote: > Add MT8192 basic clock providers, include topckgen, apmixedsys, > infracfg and pericfg. > > Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com> > --- > drivers/clk/mediatek/Kconfig | 8 + > drivers/clk/mediatek/Makefile | 1 + > drivers/clk/mediatek/clk-mt8192.c | 1326 +++++++++++++++++++++++++++++++++++++ > drivers/clk/mediatek/clk-mux.h | 15 + > 4 files changed, 1350 insertions(+) > create mode 100644 drivers/clk/mediatek/clk-mt8192.c >
[...]
> +static int clk_mt8192_apmixed_probe(struct platform_device *pdev) > +{ > + struct clk_onecell_data *clk_data; > + struct device_node *node = pdev->dev.of_node; > + int r; > + > + clk_data = mtk_alloc_clk_data(CLK_APMIXED_NR_CLK); > + if (!clk_data) > + return -ENOMEM; > + > + mtk_clk_register_plls(node, plls, ARRAY_SIZE(plls), clk_data); > + r = mtk_clk_register_gates(node, apmixed_clks, ARRAY_SIZE(apmixed_clks), clk_data); > + if (r) > + return r; > + > + return of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); > +} > + > +static const struct of_device_id of_match_clk_mt8192[] = { > + { > + .compatible = "mediatek,mt8192-apmixedsys", > + .data = clk_mt8192_apmixed_probe, > + }, { > + .compatible = "mediatek,mt8192-topckgen", > + .data = clk_mt8192_top_probe, > + }, { > + .compatible = "mediatek,mt8192-infracfg", > + .data = clk_mt8192_infra_probe, > + }, { > + .compatible = "mediatek,mt8192-pericfg", > + .data = clk_mt8192_peri_probe, > + }, { > + /* sentinel */ > + } > +}; > + > +static int clk_mt8192_probe(struct platform_device *pdev) > +{ > + int (*clk_probe)(struct platform_device *pdev); > + int r; > + > + clk_probe = of_device_get_match_data(&pdev->dev); > + if (!clk_probe) > + return -EINVAL; > + > + r = clk_probe(pdev); > + if (r) > + dev_err(&pdev->dev, "could not register clock provider: %s: %d\n", pdev->name, r); > + > + return r; > +} > + > +static struct platform_driver clk_mt8192_drv = { > + .probe = clk_mt8192_probe, > + .driver = { > + .name = "clk-mt8192", > + .of_match_table = of_match_clk_mt8192, > + }, > +}; > + > +static int __init clk_mt8192_init(void) > +{ > + return platform_driver_register(&clk_mt8192_drv); > +} > + > +arch_initcall(clk_mt8192_init);
Do we really need all these clocks that early? Why don't we use CLK_OF_DECLARE_DRIVER() then and why do we initialize some clocks CLK_OF_DECLARE_DRIVER and other with arch_initcall?
I know that this is in other drivers for MediaTek SoCs, but that does not mean it's the right approach.
> diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h > index f5625f4..afbc7df 100644 > --- a/drivers/clk/mediatek/clk-mux.h > +++ b/drivers/clk/mediatek/clk-mux.h > @@ -77,6 +77,21 @@ struct mtk_mux { > _width, _gate, _upd_ofs, _upd, \ > CLK_SET_RATE_PARENT) > > +#define MUX_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \ > + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \ > + _upd_ofs, _upd, _flags) \ > + GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \ > + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \ > + 0, _upd_ofs, _upd, _flags, \ > + mtk_mux_clr_set_upd_ops) > + > +#define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, \ > + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \ > + _upd_ofs, _upd) \ > + MUX_CLR_SET_UPD_FLAGS(_id, _name, _parents, \ > + _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift, \ > + _width, _upd_ofs, _upd, CLK_SET_RATE_PARENT) > +
Why can't we do something like:
#define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, \ _mux_set_ofs, _mux_clr_ofs, _shift, _width, \ _upd_ofs, _upd) \ GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \ _mux_set_ofs, _mux_clr_ofs, _shift, _width, \ 0, _upd_ofs, _upd, CLK_SET_RATE_PARENT, \ mtk_mux_clr_set_upd_ops)
> struct clk *mtk_clk_register_mux(const struct mtk_mux *mux, > struct regmap *regmap, > spinlock_t *lock); >
| |