Re: [PATCH v8 11/26] memory: tegra124-emc: Make driver modular

[Krzysztof Kozlowski]

On Wed, Nov 11, 2020 at 12:17:37PM +0300, Dmitry Osipenko wrote:
> 11.11.2020 12:04, Krzysztof Kozlowski пишет:
> >> -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.
> 
> The CONFIG_TEGRA124_EMC now could be 'm', while the clock code must be
> built-in. The TEGRA124 EMC driver is used by T124 and T132 SoCs.\

Mhmm,  the CONFIG_TEGRA124_EMC depends on ARCH_TEGRA_124_SOC so on the
config !ARCH_TEGRA_124_SOC && ARCH_TEGRA_132_SOC this was not
selected. Now it will be selected.

> 
> ...
> >> 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.
> 
> Because the clock and EMC driver were not separated from each other
> previously. The clock part can't be modularized easily and probably
> shouldn't.
> 
> I'm not sure whether it's actually possible to split this patch without
> taking a closer a look.
> 
> I'm also doubt that it would really worth the effort for a 100 lines of
> a changed code.

Indeed, the clk and emc drivers are so tightly coupled that anyway it
would be big patch to change the interfaces followed up a small one
making emc module. OK.

Best regards,
Krzysztof