Hi Etienne,

On Fri, May 28, 2021 at 11:43:24AM +0200, Etienne Carriere wrote:

Hello Christian,

On Thu, 27 May 2021 at 17:11, Cristian Marussi [off-list ref] wrote:

quoted

[snip]

quoted

quoted

+
+static struct tee_client_driver optee_scmi_driver = {
+     .id_table       = optee_scmi_id_table,
+     .driver         = {
+             .name = DRIVER_NAME,
+             .bus = &tee_bus_type,
+             .probe = optee_scmi_probe,
+             .remove = optee_scmi_remove,
+     },
+};
+
+static int __init optee_scmi_init(void)
+{
+     return driver_register(&optee_scmi_driver.driver);
+}
+
+static void __exit optee_scmi_exit(void)
+{
+     driver_unregister(&optee_scmi_driver.driver);
+}
+
+module_init(optee_scmi_init);
+module_exit(optee_scmi_exit);
+

This cannot compile is the full SCMI statck ARM_SCMI_PROTOCOL is configured as =m

/opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-ld:
drivers/firmware/arm_scmi/optee_service.o: in function
`optee_scmi_init':
/home/crimar01/ARM/dev/src/pdsw/linux/drivers/firmware/arm_scmi/optee_service.c:515:
multiple definition of `init_module';
drivers/firmware/arm_scmi/driver.o:/home/crimar01/ARM/dev/src/pdsw/linux/drivers/firmware/arm_scmi/driver.c:1593:
first defined here
/opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-ld:
drivers/firmware/arm_scmi/optee_service.o: in function
`optee_scmi_exit':
/home/crimar01/ARM/dev/src/pdsw/linux/drivers/firmware/arm_scmi/optee_service.c:520:
multiple definition of `cleanup_module';
drivers/firmware/arm_scmi/driver.o:/home/crimar01/ARM/dev/src/pdsw/linux/drivers/firmware/arm_scmi/driver.c:1611:
first defined here
/home/crimar01/ARM/dev/src/pdsw/linux/scripts/Makefile.build:449: recipe
for target 'drivers/firmware/arm_scmi/scmi-module.o' failed
make[4]: *** [drivers/firmware/arm_scmi/scmi-module.o] Error 1


Indeed it was the same issue we faced in the virtio-scmi series, and it
derives from the fact that SCMI transports 'driver' are really NOT
standalone drivers but only extension of the main SCMI module, so you
cannot have them initialize their stuff using usual kernel module_ machinery.

In order to address this, and avoid a hell of ifdeffery probably,
in the context of virtio-scmi, I added a couple of transport's
optionalily provided init/deinit functions so that a transport can provide
some specific init code and be assured they are called at SCMI stack init,
so definitely even before the SCMI stack is probed and the selected
transport used.

This is the patch from the virtio-scmi series:

https://lore.kernel.org/linux-arm-kernel/20210511002040.802226-3-peter.hilber@opensynergy.com/ (local)

which in that context is used like:

+static int __init virtio_scmi_init(void)
+{
+       return register_virtio_driver(&virtio_scmi_driver);
+}
+
+static void __exit virtio_scmi_exit(void)
+{
+       unregister_virtio_driver(&virtio_scmi_driver);
+}
+
+const struct scmi_desc scmi_virtio_desc = {
+       .init = virtio_scmi_init,
+       .exit = virtio_scmi_exit,
+       .ops = &scmi_virtio_ops,

I'll cleanup further that init/deinit patch and move it out into that bunch
of SCMI core changes that I'm making (in a separate series) to aid in virtio-scmi
devel.
In the meantime for your testing the above lore patch should work fine.

Thanks for the details, i'll upgrade to the series.

This is the udpated patch above about init/exit now included in that
bunch of SCMI core transport changes I'm making:

https://lore.kernel.org/linux-arm-kernel/20210601102421.26581-4-cristian.marussi@arm.com/T/#u (local)

but there's really no functional change from the one I mentioned above
that was part of the virtio-scmi series.

Thanks,
Cristian

Cheers,
Etienne

quoted

Thanks,
Cristian

quoted

+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Etienne Carriere [off-list ref]");
+MODULE_DESCRIPTION("OP-TEE SCMI transport driver");
--
2.17.1

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