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

[Cristian Marussi <cristian.marussi@arm.com>]

Hi Sudeep

On Mon, Sep 07, 2020 at 08:50:42PM +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.
> 
> Regards,
> Sudeep
> 

I re-tested this v2 (also regarding some interactions with notifications) and
works generally fine for me, both builtin or modularized, BUT I've seen an
issue on core module load/unload/load.
Basically doing this:

(debian-arm64)root@debarm64:~# insmod ./scmi-module.ko 
(debian-arm64)root@debarm64:~# insmod ./scmi-cpufreq.ko 

(debian-arm64)root@debarm64:~# rmmod ./scmi-cpufreq.ko 
(debian-arm64)root@debarm64:~# rmmod ./scmi-module.ko 
(debian-arm64)root@debarm64:~# lsmod 
Module                  Size  Used by
(debian-arm64)root@debarm64:~# insmod ./scmi-module.ko 

I've got this:


[  146.982413] mhu 2b1f0000.mhu: Channel in use
[  146.982433] arm-scmi firmware:scmi: failed to request SCMI Tx mailbox
[  146.982472] arm-scmi: probe of firmware:scmi failed with error -16

and SCMI is broken then after reloading.

Now this is an issue I've seen already in my ongoing WIP on full SCMI Protocols
modularization for custom protocols, and it is related to the fact the the
underlying transport init is bound to the SCMI device creation and not the
protocol initialization, and we are not destroying and re-creating such
devices properly. (things that I'm going to address in that WIP)

Given that the solution to this is not so simple as of now, and given that
unloading of the core as a whole module does not make so much sense anyway
(while it will be needed for single custom protocols modules), couldn't we
just make scmi-module a permanent by droppping module_exit() ?

Thanks

Cristian

> v1[1]->v2:
> 	- Added missing smccc functions exports
> 	- Moved scmi_driver_init to subsys_initcall
> 	- Reorder exit function calls
> 	- Renamed protocol_init/exit to register/unregister
> 	- Rebased on [2]
> 
> [1] https://lore.kernel.org/r/20200907112920.34275-1-sudeep.holla@arm.com/
> [2] git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git for-next/scmi
>    (contains system protocol implementation)
> 
> Sudeep Holla (4):
>   firmware: smccc: export both smccc functions
>   firmware: arm_scmi: Move scmi bus init and exit calls into the driver
>   firmware: arm_scmi: Move scmi protocols registration into the driver
>   firmware: arm_scmi: Enable building as a single module
> 
>  drivers/firmware/Kconfig            |  2 +-
>  drivers/firmware/Makefile           |  2 +-
>  drivers/firmware/arm_scmi/Makefile  |  4 +++-
>  drivers/firmware/arm_scmi/bus.c     |  6 ++----
>  drivers/firmware/arm_scmi/clock.c   |  7 +------
>  drivers/firmware/arm_scmi/common.h  | 24 +++++++++++++++++++++++
>  drivers/firmware/arm_scmi/driver.c  | 30 ++++++++++++++++++++++++++++-
>  drivers/firmware/arm_scmi/perf.c    |  7 +------
>  drivers/firmware/arm_scmi/power.c   |  7 +------
>  drivers/firmware/arm_scmi/reset.c   |  7 +------
>  drivers/firmware/arm_scmi/sensors.c |  7 +------
>  drivers/firmware/arm_scmi/system.c  |  7 +------
>  drivers/firmware/smccc/smccc.c      |  2 ++
>  include/linux/scmi_protocol.h       |  2 +-
>  14 files changed, 69 insertions(+), 45 deletions(-)
> 
> --
> 2.17.1
> 

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