On Wed, Nov 11, 2020 at 04:14:41AM +0300, Dmitry Osipenko wrote: > Add modularization support to the Tegra124 EMC driver, which now can be > compiled as a loadable kernel module. > > Note that EMC clock must be registered at clk-init time, otherwise PLLM > will be disabled as unused clock at boot time if EMC driver is compiled > as a module. Hence add a prepare/complete callbacks. similarly to what is > done for the Tegra20/30 EMC drivers. > > Tested-by: Nicolas Chauvet <kwizart@xxxxxxxxx> > Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx> > --- > drivers/clk/tegra/Makefile | 3 +- > drivers/clk/tegra/clk-tegra124-emc.c | 41 ++++++++++++++++++++++++---- > drivers/clk/tegra/clk-tegra124.c | 27 ++++++++++++++++-- > drivers/clk/tegra/clk.h | 16 +++-------- > drivers/memory/tegra/Kconfig | 2 +- > drivers/memory/tegra/tegra124-emc.c | 31 ++++++++++++++------- > include/linux/clk/tegra.h | 8 ++++++ > include/soc/tegra/emc.h | 16 ----------- > 8 files changed, 96 insertions(+), 48 deletions(-) > delete mode 100644 include/soc/tegra/emc.h > > diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile > index eec2313fd37e..53b76133e905 100644 > --- a/drivers/clk/tegra/Makefile > +++ b/drivers/clk/tegra/Makefile > @@ -22,7 +22,8 @@ obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra20-emc.o > obj-$(CONFIG_ARCH_TEGRA_114_SOC) += clk-tegra114.o > obj-$(CONFIG_ARCH_TEGRA_124_SOC) += clk-tegra124.o > obj-$(CONFIG_TEGRA_CLK_DFLL) += clk-tegra124-dfll-fcpu.o > -obj-$(CONFIG_TEGRA124_EMC) += clk-tegra124-emc.o > +obj-$(CONFIG_ARCH_TEGRA_124_SOC) += clk-tegra124-emc.o > +obj-$(CONFIG_ARCH_TEGRA_132_SOC) += clk-tegra124-emc.o How is it related to modularization? It looks like different issue is fixed here. > obj-$(CONFIG_ARCH_TEGRA_132_SOC) += clk-tegra124.o > obj-y += cvb.o > obj-$(CONFIG_ARCH_TEGRA_210_SOC) += clk-tegra210.o > diff --git a/drivers/clk/tegra/clk-tegra124-emc.c b/drivers/clk/tegra/clk-tegra124-emc.c > index 745f9faa98d8..bdf6f4a51617 100644 > --- a/drivers/clk/tegra/clk-tegra124-emc.c > +++ b/drivers/clk/tegra/clk-tegra124-emc.c > @@ -11,7 +11,9 @@ > #include <linux/clk-provider.h> > #include <linux/clk.h> > #include <linux/clkdev.h> > +#include <linux/clk/tegra.h> > #include <linux/delay.h> > +#include <linux/export.h> > #include <linux/io.h> > #include <linux/module.h> > #include <linux/of_address.h> > @@ -21,7 +23,6 @@ > #include <linux/string.h> > > #include <soc/tegra/fuse.h> > -#include <soc/tegra/emc.h> > > #include "clk.h" > > @@ -80,6 +81,9 @@ struct tegra_clk_emc { > int num_timings; > struct emc_timing *timings; > spinlock_t *lock; > + > + tegra124_emc_prepare_timing_change_cb *prepare_timing_change; > + tegra124_emc_complete_timing_change_cb *complete_timing_change; > }; > > /* Common clock framework callback implementations */ > @@ -176,6 +180,9 @@ static struct tegra_emc *emc_ensure_emc_driver(struct tegra_clk_emc *tegra) > if (tegra->emc) > return tegra->emc; > > + if (!tegra->prepare_timing_change || !tegra->complete_timing_change) > + return NULL; > + > if (!tegra->emc_node) > return NULL; > > @@ -241,7 +248,7 @@ static int emc_set_timing(struct tegra_clk_emc *tegra, > > div = timing->parent_rate / (timing->rate / 2) - 2; > > - err = tegra_emc_prepare_timing_change(emc, timing->rate); > + err = tegra->prepare_timing_change(emc, timing->rate); > if (err) > return err; > > @@ -259,7 +266,7 @@ static int emc_set_timing(struct tegra_clk_emc *tegra, > > spin_unlock_irqrestore(tegra->lock, flags); > > - tegra_emc_complete_timing_change(emc, timing->rate); > + tegra->complete_timing_change(emc, timing->rate); > > clk_hw_reparent(&tegra->hw, __clk_get_hw(timing->parent)); > clk_disable_unprepare(tegra->prev_parent); > @@ -473,8 +480,8 @@ static const struct clk_ops tegra_clk_emc_ops = { > .get_parent = emc_get_parent, > }; > > -struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np, > - spinlock_t *lock) > +struct clk *tegra124_clk_register_emc(void __iomem *base, struct device_node *np, > + spinlock_t *lock) > { > struct tegra_clk_emc *tegra; > struct clk_init_data init; > @@ -538,3 +545,27 @@ struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np, > > return clk; > }; > + > +void tegra124_clk_set_emc_callbacks(tegra124_emc_prepare_timing_change_cb *prep_cb, > + tegra124_emc_complete_timing_change_cb *complete_cb) > +{ > + struct clk *clk = __clk_lookup("emc"); > + struct tegra_clk_emc *tegra; > + struct clk_hw *hw; > + > + if (clk) { > + hw = __clk_get_hw(clk); > + tegra = container_of(hw, struct tegra_clk_emc, hw); > + > + tegra->prepare_timing_change = prep_cb; > + tegra->complete_timing_change = complete_cb; > + } > +} > +EXPORT_SYMBOL_GPL(tegra124_clk_set_emc_callbacks); > + > +bool tegra124_clk_emc_driver_available(struct clk_hw *hw) > +{ > + struct tegra_clk_emc *tegra = container_of(hw, struct tegra_clk_emc, hw); > + > + return tegra->prepare_timing_change && tegra->complete_timing_change; > +} > diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c > index e931319dcc9d..b4f2ae4066a6 100644 > --- a/drivers/clk/tegra/clk-tegra124.c > +++ b/drivers/clk/tegra/clk-tegra124.c > @@ -929,6 +929,7 @@ static struct tegra_clk tegra124_clks[tegra_clk_max] __initdata = { > [tegra_clk_audio4_mux] = { .dt_id = TEGRA124_CLK_AUDIO4_MUX, .present = true }, > [tegra_clk_spdif_mux] = { .dt_id = TEGRA124_CLK_SPDIF_MUX, .present = true }, > [tegra_clk_cec] = { .dt_id = TEGRA124_CLK_CEC, .present = true }, > + [tegra_clk_emc] = { .dt_id = TEGRA124_CLK_EMC, .present = false }, > }; > > static struct tegra_devclk devclks[] __initdata = { > @@ -1500,6 +1501,26 @@ static void __init tegra124_132_clock_init_pre(struct device_node *np) > writel(plld_base, clk_base + PLLD_BASE); > } > > +static struct clk *tegra124_clk_src_onecell_get(struct of_phandle_args *clkspec, > + void *data) > +{ > + struct clk_hw *hw; > + struct clk *clk; > + > + clk = of_clk_src_onecell_get(clkspec, data); > + if (IS_ERR(clk)) > + return clk; > + > + hw = __clk_get_hw(clk); > + > + if (clkspec->args[0] == TEGRA124_CLK_EMC) { > + if (!tegra124_clk_emc_driver_available(hw)) > + return ERR_PTR(-EPROBE_DEFER); > + } > + > + return clk; > +} > + > /** > * tegra124_132_clock_init_post - clock initialization postamble for T124/T132 > * @np: struct device_node * of the DT node for the SoC CAR IP block > @@ -1516,10 +1537,10 @@ static void __init tegra124_132_clock_init_post(struct device_node *np) > &pll_x_params); > tegra_init_special_resets(1, tegra124_reset_assert, > tegra124_reset_deassert); > - tegra_add_of_provider(np, of_clk_src_onecell_get); > + tegra_add_of_provider(np, tegra124_clk_src_onecell_get); > > - clks[TEGRA124_CLK_EMC] = tegra_clk_register_emc(clk_base, np, > - &emc_lock); > + clks[TEGRA124_CLK_EMC] = tegra124_clk_register_emc(clk_base, np, > + &emc_lock); > > tegra_register_devclks(devclks, ARRAY_SIZE(devclks)); > > diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h > index 6b565f6b5f66..2da7c93c1a6c 100644 > --- a/drivers/clk/tegra/clk.h > +++ b/drivers/clk/tegra/clk.h > @@ -881,18 +881,6 @@ void tegra_super_clk_gen5_init(void __iomem *clk_base, > void __iomem *pmc_base, struct tegra_clk *tegra_clks, > struct tegra_clk_pll_params *pll_params); > > -#ifdef CONFIG_TEGRA124_EMC > -struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np, > - spinlock_t *lock); > -#else > -static inline struct clk *tegra_clk_register_emc(void __iomem *base, > - struct device_node *np, > - spinlock_t *lock) > -{ > - return NULL; > -} > -#endif Why clock changes are so tightly coupled with making an EMC driver modular? Usually this should be a separate change - you adjust any dependencies to accept late or deferred probing, exported symbols, loosen the coupling between drivers, etc. and then you convert something to module. Best regards, Krzysztof