Re: [PATCH 08/17] clk: vexpress-osc: Support building as a module

[Sudeep Holla <sudeep.holla@arm.com>]

On Wed, Apr 22, 2020 at 10:08:02PM +0100, Sudeep Holla wrote:
> On Sun, Apr 19, 2020 at 12:08:01PM -0500, Rob Herring wrote:
> > Enable building the vexpress-osc clock driver as a module.
> >
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Michael Turquette <mturquette@baylibre.com>
> > Cc: Stephen Boyd <sboyd@kernel.org>
> > Cc: linux-clk@vger.kernel.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> >  drivers/clk/versatile/Kconfig            |  4 ++--
> >  drivers/clk/versatile/clk-vexpress-osc.c | 10 ++++------
> >  2 files changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
> > index 5bdd5c98990b..9de2396dcf9b 100644
> > --- a/drivers/clk/versatile/Kconfig
> > +++ b/drivers/clk/versatile/Kconfig
> > @@ -15,8 +15,8 @@ config CLK_SP810
> >  	  of the ARM SP810 System Controller cell.
> >
> >  config CLK_VEXPRESS_OSC
> > -	bool "Clock driver for Versatile Express OSC clock generators"
> > -	depends on VEXPRESS_CONFIG || COMPILE_TEST
> > +	tristate "Clock driver for Versatile Express OSC clock generators"
> > +	depends on VEXPRESS_CONFIG
> >  	default y if ARCH_VEXPRESS
> >  	---help---
> >  	  Simple regmap-based driver driving clock generators on Versatile
> > diff --git a/drivers/clk/versatile/clk-vexpress-osc.c b/drivers/clk/versatile/clk-vexpress-osc.c
> > index 5bb1d5a714d0..b2b32fa2d7c3 100644
> > --- a/drivers/clk/versatile/clk-vexpress-osc.c
> > +++ b/drivers/clk/versatile/clk-vexpress-osc.c
> > @@ -7,6 +7,7 @@
> >  #include <linux/clkdev.h>
> >  #include <linux/clk-provider.h>
> >  #include <linux/err.h>
> > +#include <linux/module.h>
> >  #include <linux/of.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/slab.h>
> > @@ -108,6 +109,7 @@ static const struct of_device_id vexpress_osc_of_match[] = {
> >  	{ .compatible = "arm,vexpress-osc", },
> >  	{}
> >  };
> > +MODULE_DEVICE_TABLE(of, vexpress_osc_of_match);
> >
> >  static struct platform_driver vexpress_osc_driver = {
> >  	.driver	= {
> > @@ -116,9 +118,5 @@ static struct platform_driver vexpress_osc_driver = {
> >  	},
> >  	.probe = vexpress_osc_probe,
> >  };
> > -
> > -static int __init vexpress_osc_init(void)
> > -{
> > -	return platform_driver_register(&vexpress_osc_driver);
> > -}
> > -core_initcall(vexpress_osc_init);
> > +module_platform_driver(vexpress_osc_driver);
>
> I am not 100% sure of this. This might break the boot on CA9 and TC2
> at-least. There are loads of MB peripherals that need this. This will
> break the boot. We need to check if all the dependent modules are also
> at module_initcall level and if they deal with deferred probe correctly.
> Lot of them are legacy and may happen to be working by carefully initcall
> level adjustments.
>

OK I managed to try this on my TC2 and it fails to boot. However when I
enable earlyprintk as I see no log without it, it boots just fine.

I also checked adding initcall_debug and I may be wrong on the dependency
part. The modules dependent on vexpress-osc are probed later correctly.

This make it more difficult to debug as I don't have any debugger attached
at the moment to look at the logbuf when it hangs without earlyprintk.

--
Regards,
Sudeep