Re: [PATCH 0/3] firmware: arm_scmi: Enable building SCMI as module

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

On Mon, Sep 07, 2020 at 04:25:13PM +0100, Cristian Marussi wrote:
> Hi Sudeep,
> 
> On Mon, Sep 07, 2020 at 12:29:17PM +0100, Sudeep Holla wrote:
> > Hi,
> > 
> > Though it was initially developed as module, so some reason(I can't
> > recollect why apart from some structuring arounf the way bus and
> > protocols were initialised), it was merged as a built-in only driver.
> > 
> > Now, there is a need to build this as modules. This is mainly needed
> > by virtio transport. This also aligns well with GKI modularisation
> > efforts.
> > 
> 
> This works for me as a module, but it gave me issues when compiled builtin
> since the some SCMI drivers (hwmon, cpufreq) look for the SCMI bus too early
> when both them and the core are compiled as builtins.
>

Thanks for testing.

> [    2.226029] rtc-efi rtc-efi.0: setting system clock to 2020-09-07T10:51:36 UTC (1599475896)
> [    2.235091] rtc-pl031 1c170000.rtc: registered as rtc1
> [    2.240767] i2c /dev entries driver
> [    2.246522] Driver 'scmi-hwmon' was unable to register with bus_type 'scmi_protocol' because the bus was not initialized.
> [    2.259037] sp805-wdt 1c0f0000.wdt: registration successful
> [    2.265464] Driver 'scmi-cpufreq' was unable to register with bus_type 'scmi_protocol' because the bus was not initialized.
> [    2.278905] mmci-pl18x 1c050000.mmci: mmc0: PL180 manf 41 rev0 at 0x1c050000 irq 8,0 (pio)
> 
> This dirty trick below solves for me though the builtin issue (and still runs
> fine when modularized):
>

In fact the bus init was subsys previously, so make sense to move that
too. I don't think it is a hack. Since scmi_bus needs to be available
for all scmi_drivers to be registered, it looks valid for me.

> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 2a1396b74fa5..b69bb174344d 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -939,7 +939,11 @@ static int __init scmi_driver_init(void)
>  
>         return platform_driver_register(&scmi_driver);
>  }
> +#ifdef MODULE
>  module_init(scmi_driver_init);
> +#else
> +subsys_initcall(scmi_driver_init);
> +#endif
>  

Indeed, just subsys_initcall will suffice. It is module_init when built as
module, so no need for us to define that explicitly.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel