linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module
@ 2020-09-07 19:50 Sudeep Holla
  2020-09-07 19:50 ` [PATCH v2 1/4] firmware: smccc: export both smccc functions Sudeep Holla
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Sudeep Holla @ 2020-09-07 19:50 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Mikhail Golubev, Igor Skalkin, Peter Hilber, Anton Yakovlev,
	Sudeep Holla, Cristian Marussi

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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH v2 1/4] firmware: smccc: export both smccc functions
  2020-09-07 19:50 [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module Sudeep Holla
@ 2020-09-07 19:50 ` Sudeep Holla
  2020-09-07 19:50 ` [PATCH v2 2/4] firmware: arm_scmi: Move scmi bus init and exit calls into the driver Sudeep Holla
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Sudeep Holla @ 2020-09-07 19:50 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Mikhail Golubev, Igor Skalkin, Peter Hilber, Anton Yakovlev,
	Sudeep Holla, Cristian Marussi

We need to export both arm_smccc_1_1_get_conduit and arm_smccc_get_version
to allow several modules make use of them. Arm FFA, Arm SCMI and PTP
drivers are few drivers that are planning to use these functions.

Let us export them in preparation to add support for SCMI as module.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/smccc/smccc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
index 4e80921ee212..00c88b809c0c 100644
--- a/drivers/firmware/smccc/smccc.c
+++ b/drivers/firmware/smccc/smccc.c
@@ -24,8 +24,10 @@ enum arm_smccc_conduit arm_smccc_1_1_get_conduit(void)
 
 	return smccc_conduit;
 }
+EXPORT_SYMBOL_GPL(arm_smccc_1_1_get_conduit);
 
 u32 arm_smccc_get_version(void)
 {
 	return smccc_version;
 }
+EXPORT_SYMBOL_GPL(arm_smccc_get_version);
-- 
2.17.1


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v2 2/4] firmware: arm_scmi: Move scmi bus init and exit calls into the driver
  2020-09-07 19:50 [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module Sudeep Holla
  2020-09-07 19:50 ` [PATCH v2 1/4] firmware: smccc: export both smccc functions Sudeep Holla
@ 2020-09-07 19:50 ` Sudeep Holla
  2020-09-07 19:50 ` [PATCH v2 3/4] firmware: arm_scmi: Move scmi protocols registration " Sudeep Holla
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Sudeep Holla @ 2020-09-07 19:50 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Mikhail Golubev, Igor Skalkin, Peter Hilber, Anton Yakovlev,
	Sudeep Holla, Cristian Marussi

In preparation to enable building scmi as a single module, let us move
the scmi bus {de-,}initialisation call into the driver.

The main reason for this is to keep it simple instead of maintaining
it as separate modules and dealing with all possible initcall races
and deferred probe handling. We can move it as separate modules if
needed in future.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/bus.c    |  6 ++----
 drivers/firmware/arm_scmi/common.h |  3 +++
 drivers/firmware/arm_scmi/driver.c | 16 +++++++++++++++-
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index db55c43a2cbd..1377ec76a45d 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -230,7 +230,7 @@ static void scmi_devices_unregister(void)
 	bus_for_each_dev(&scmi_bus_type, NULL, NULL, __scmi_devices_unregister);
 }
 
-static int __init scmi_bus_init(void)
+int __init scmi_bus_init(void)
 {
 	int retval;
 
@@ -240,12 +240,10 @@ static int __init scmi_bus_init(void)
 
 	return retval;
 }
-subsys_initcall(scmi_bus_init);
 
-static void __exit scmi_bus_exit(void)
+void __exit scmi_bus_exit(void)
 {
 	scmi_devices_unregister();
 	bus_unregister(&scmi_bus_type);
 	ida_destroy(&scmi_bus_id);
 }
-module_exit(scmi_bus_exit);
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 233700a42bff..a940c6cf1e51 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -156,6 +156,9 @@ void scmi_setup_protocol_implemented(const struct scmi_handle *handle,
 
 int scmi_base_protocol_init(struct scmi_handle *h);
 
+int __init scmi_bus_init(void);
+void __exit scmi_bus_exit(void);
+
 /* SCMI Transport */
 /**
  * struct scmi_chan_info - Structure representing a SCMI channel information
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index dbec34423f72..b293ee1f25f7 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -929,7 +929,21 @@ static struct platform_driver scmi_driver = {
 	.remove = scmi_remove,
 };
 
-module_platform_driver(scmi_driver);
+static int __init scmi_driver_init(void)
+{
+	scmi_bus_init();
+
+	return platform_driver_register(&scmi_driver);
+}
+module_init(scmi_driver_init);
+
+static void __exit scmi_driver_exit(void)
+{
+	platform_driver_unregister(&scmi_driver);
+
+	scmi_bus_exit();
+}
+module_exit(scmi_driver_exit);
 
 MODULE_ALIAS("platform: arm-scmi");
 MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
-- 
2.17.1


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v2 3/4] firmware: arm_scmi: Move scmi protocols registration into the driver
  2020-09-07 19:50 [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module Sudeep Holla
  2020-09-07 19:50 ` [PATCH v2 1/4] firmware: smccc: export both smccc functions Sudeep Holla
  2020-09-07 19:50 ` [PATCH v2 2/4] firmware: arm_scmi: Move scmi bus init and exit calls into the driver Sudeep Holla
@ 2020-09-07 19:50 ` Sudeep Holla
  2020-09-07 19:50 ` [PATCH v2 4/4] firmware: arm_scmi: Enable building as a single module Sudeep Holla
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Sudeep Holla @ 2020-09-07 19:50 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Mikhail Golubev, Igor Skalkin, Peter Hilber, Anton Yakovlev,
	Sudeep Holla, Cristian Marussi

In preparation to enable building SCMI as a single module, let us move
the SCMI protocol registration call into the driver. This enables us
to also add unregistration of the SCMI protocols.

The main reason for this is to keep it simple instead of maintaining
it as separate modules and dealing with all possible initcall races
and deferred probe handling. We can move it as separate modules if
needed in future.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/clock.c   |  7 +------
 drivers/firmware/arm_scmi/common.h  | 21 +++++++++++++++++++++
 drivers/firmware/arm_scmi/driver.c  | 16 +++++++++++++++-
 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 +------
 8 files changed, 42 insertions(+), 37 deletions(-)

diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index a3b90be28009..c1cfe3ee3d55 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -364,9 +364,4 @@ static int scmi_clock_protocol_init(struct scmi_handle *handle)
 	return 0;
 }
 
-static int __init scmi_clock_init(void)
-{
-	return scmi_protocol_register(SCMI_PROTOCOL_CLOCK,
-				      &scmi_clock_protocol_init);
-}
-subsys_initcall(scmi_clock_init);
+DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(SCMI_PROTOCOL_CLOCK, clock)
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index a940c6cf1e51..37fb583f1bf5 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -159,6 +159,27 @@ int scmi_base_protocol_init(struct scmi_handle *h);
 int __init scmi_bus_init(void);
 void __exit scmi_bus_exit(void);
 
+#define DECLARE_SCMI_REGISTER_UNREGISTER(func)		\
+	int __init scmi_##func##_register(void);	\
+	void __exit scmi_##func##_unregister(void)
+DECLARE_SCMI_REGISTER_UNREGISTER(clock);
+DECLARE_SCMI_REGISTER_UNREGISTER(perf);
+DECLARE_SCMI_REGISTER_UNREGISTER(power);
+DECLARE_SCMI_REGISTER_UNREGISTER(reset);
+DECLARE_SCMI_REGISTER_UNREGISTER(sensors);
+DECLARE_SCMI_REGISTER_UNREGISTER(system);
+
+#define DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(id, name) \
+int __init scmi_##name##_register(void) \
+{ \
+	return scmi_protocol_register((id), &scmi_##name##_protocol_init); \
+} \
+\
+void __exit scmi_##name##_unregister(void) \
+{ \
+	scmi_protocol_unregister((id)); \
+}
+
 /* SCMI Transport */
 /**
  * struct scmi_chan_info - Structure representing a SCMI channel information
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index b293ee1f25f7..98b03f9dfc6c 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -933,14 +933,28 @@ static int __init scmi_driver_init(void)
 {
 	scmi_bus_init();
 
+	scmi_clock_register();
+	scmi_perf_register();
+	scmi_power_register();
+	scmi_reset_register();
+	scmi_sensors_register();
+	scmi_system_register();
+
 	return platform_driver_register(&scmi_driver);
 }
-module_init(scmi_driver_init);
+subsys_initcall(scmi_driver_init);
 
 static void __exit scmi_driver_exit(void)
 {
 	platform_driver_unregister(&scmi_driver);
 
+	scmi_clock_unregister();
+	scmi_perf_unregister();
+	scmi_power_unregister();
+	scmi_reset_unregister();
+	scmi_sensors_unregister();
+	scmi_system_unregister();
+
 	scmi_bus_exit();
 }
 module_exit(scmi_driver_exit);
diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index a3e7b1bfab00..ed475b40bd08 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -890,9 +890,4 @@ static int scmi_perf_protocol_init(struct scmi_handle *handle)
 	return 0;
 }
 
-static int __init scmi_perf_init(void)
-{
-	return scmi_protocol_register(SCMI_PROTOCOL_PERF,
-				      &scmi_perf_protocol_init);
-}
-subsys_initcall(scmi_perf_init);
+DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(SCMI_PROTOCOL_PERF, perf)
diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c
index 32bcf5821ea9..1f37258e9bee 100644
--- a/drivers/firmware/arm_scmi/power.c
+++ b/drivers/firmware/arm_scmi/power.c
@@ -301,9 +301,4 @@ static int scmi_power_protocol_init(struct scmi_handle *handle)
 	return 0;
 }
 
-static int __init scmi_power_init(void)
-{
-	return scmi_protocol_register(SCMI_PROTOCOL_POWER,
-				      &scmi_power_protocol_init);
-}
-subsys_initcall(scmi_power_init);
+DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(SCMI_PROTOCOL_POWER, power)
diff --git a/drivers/firmware/arm_scmi/reset.c b/drivers/firmware/arm_scmi/reset.c
index 4e2dc5fc43d9..f063cfe17e02 100644
--- a/drivers/firmware/arm_scmi/reset.c
+++ b/drivers/firmware/arm_scmi/reset.c
@@ -313,9 +313,4 @@ static int scmi_reset_protocol_init(struct scmi_handle *handle)
 	return 0;
 }
 
-static int __init scmi_reset_init(void)
-{
-	return scmi_protocol_register(SCMI_PROTOCOL_RESET,
-				      &scmi_reset_protocol_init);
-}
-subsys_initcall(scmi_reset_init);
+DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(SCMI_PROTOCOL_RESET, reset)
diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index 7d83680198de..9703cf6356a0 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -365,9 +365,4 @@ static int scmi_sensors_protocol_init(struct scmi_handle *handle)
 	return 0;
 }
 
-static int __init scmi_sensors_init(void)
-{
-	return scmi_protocol_register(SCMI_PROTOCOL_SENSOR,
-				      &scmi_sensors_protocol_init);
-}
-subsys_initcall(scmi_sensors_init);
+DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(SCMI_PROTOCOL_SENSOR, sensors)
diff --git a/drivers/firmware/arm_scmi/system.c b/drivers/firmware/arm_scmi/system.c
index aa1e74f066a0..283e12d5f24b 100644
--- a/drivers/firmware/arm_scmi/system.c
+++ b/drivers/firmware/arm_scmi/system.c
@@ -128,9 +128,4 @@ static int scmi_system_protocol_init(struct scmi_handle *handle)
 	return 0;
 }
 
-static int __init scmi_system_init(void)
-{
-	return scmi_protocol_register(SCMI_PROTOCOL_SYSTEM,
-				      &scmi_system_protocol_init);
-}
-subsys_initcall(scmi_system_init);
+DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(SCMI_PROTOCOL_SYSTEM, system)
-- 
2.17.1


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v2 4/4] firmware: arm_scmi: Enable building as a single module
  2020-09-07 19:50 [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module Sudeep Holla
                   ` (2 preceding siblings ...)
  2020-09-07 19:50 ` [PATCH v2 3/4] firmware: arm_scmi: Move scmi protocols registration " Sudeep Holla
@ 2020-09-07 19:50 ` Sudeep Holla
  2020-09-08  9:08 ` [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module Cristian Marussi
  2020-09-14  6:36 ` Sudeep Holla
  5 siblings, 0 replies; 10+ messages in thread
From: Sudeep Holla @ 2020-09-07 19:50 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Mikhail Golubev, Igor Skalkin, Peter Hilber, Anton Yakovlev,
	Sudeep Holla, Cristian Marussi

Now, with all the plumbing in place to enable building scmi as a module
instead of built-in modules, let us enable the same.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/Kconfig           | 2 +-
 drivers/firmware/Makefile          | 2 +-
 drivers/firmware/arm_scmi/Makefile | 4 +++-
 include/linux/scmi_protocol.h      | 2 +-
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index fbd785dd0513..afdbebba628a 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -7,7 +7,7 @@
 menu "Firmware Drivers"
 
 config ARM_SCMI_PROTOCOL
-	bool "ARM System Control and Management Interface (SCMI) Message Protocol"
+	tristate "ARM System Control and Management Interface (SCMI) Message Protocol"
 	depends on ARM || ARM64 || COMPILE_TEST
 	depends on MAILBOX
 	help
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 99510be9f5ed..5e013b6a3692 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -22,7 +22,7 @@ obj-$(CONFIG_TI_SCI_PROTOCOL)	+= ti_sci.o
 obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
 obj-$(CONFIG_TURRIS_MOX_RWTM)	+= turris-mox-rwtm.o
 
-obj-$(CONFIG_ARM_SCMI_PROTOCOL)	+= arm_scmi/
+obj-y				+= arm_scmi/
 obj-y				+= broadcom/
 obj-y				+= meson/
 obj-$(CONFIG_GOOGLE_FIRMWARE)	+= google/
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index 643f2320f976..bc0d54f8e861 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -1,9 +1,11 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-y	= scmi-bus.o scmi-driver.o scmi-protocols.o scmi-transport.o
 scmi-bus-y = bus.o
 scmi-driver-y = driver.o notify.o
 scmi-transport-y = shmem.o
 scmi-transport-$(CONFIG_MAILBOX) += mailbox.o
 scmi-transport-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) += smc.o
 scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o
+scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \
+		    $(scmi-transport-y)
+obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
 obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 4b10093ad671..9cd312a1ff92 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -345,7 +345,7 @@ struct scmi_driver {
 
 #define to_scmi_driver(d) container_of(d, struct scmi_driver, driver)
 
-#ifdef CONFIG_ARM_SCMI_PROTOCOL
+#if IS_REACHABLE(CONFIG_ARM_SCMI_PROTOCOL)
 int scmi_driver_register(struct scmi_driver *driver,
 			 struct module *owner, const char *mod_name);
 void scmi_driver_unregister(struct scmi_driver *driver);
-- 
2.17.1


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module
  2020-09-07 19:50 [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module Sudeep Holla
                   ` (3 preceding siblings ...)
  2020-09-07 19:50 ` [PATCH v2 4/4] firmware: arm_scmi: Enable building as a single module Sudeep Holla
@ 2020-09-08  9:08 ` Cristian Marussi
  2020-09-08 11:11   ` Sudeep Holla
  2020-09-14  6:36 ` Sudeep Holla
  5 siblings, 1 reply; 10+ messages in thread
From: Cristian Marussi @ 2020-09-08  9:08 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Mikhail Golubev, Igor Skalkin, Peter Hilber, linux-arm-kernel,
	cristian.marussi, Anton Yakovlev

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module
  2020-09-08  9:08 ` [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module Cristian Marussi
@ 2020-09-08 11:11   ` Sudeep Holla
  2020-09-08 11:53     ` Cristian Marussi
  0 siblings, 1 reply; 10+ messages in thread
From: Sudeep Holla @ 2020-09-08 11:11 UTC (permalink / raw)
  To: Cristian Marussi
  Cc: Mikhail Golubev, Peter Hilber, Igor Skalkin, linux-arm-kernel,
	Anton Yakovlev

On Tue, Sep 08, 2020 at 10:08:44AM +0100, Cristian Marussi wrote:
> 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() ?

Now I remember why I had bus_exit before unregistering the driver. I think
that should fix the issue. Let me know.

--
Regards,
Sudeep

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module
  2020-09-08 11:11   ` Sudeep Holla
@ 2020-09-08 11:53     ` Cristian Marussi
  2020-09-08 12:10       ` Sudeep Holla
  0 siblings, 1 reply; 10+ messages in thread
From: Cristian Marussi @ 2020-09-08 11:53 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Mikhail Golubev, Peter Hilber, Igor Skalkin, linux-arm-kernel,
	Anton Yakovlev

On Tue, Sep 08, 2020 at 12:11:14PM +0100, Sudeep Holla wrote:
> On Tue, Sep 08, 2020 at 10:08:44AM +0100, Cristian Marussi wrote:
> > 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() ?
> 
> Now I remember why I had bus_exit before unregistering the driver. I think
> that should fix the issue. Let me know.
> 

Yes that works for me, both as builtin or as a module now.

Cristian

> --
> Regards,
> Sudeep

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module
  2020-09-08 11:53     ` Cristian Marussi
@ 2020-09-08 12:10       ` Sudeep Holla
  0 siblings, 0 replies; 10+ messages in thread
From: Sudeep Holla @ 2020-09-08 12:10 UTC (permalink / raw)
  To: Cristian Marussi
  Cc: Mikhail Golubev, Peter Hilber, Igor Skalkin, linux-arm-kernel,
	Anton Yakovlev

On Tue, Sep 08, 2020 at 12:53:05PM +0100, Cristian Marussi wrote:
> On Tue, Sep 08, 2020 at 12:11:14PM +0100, Sudeep Holla wrote:
> > On Tue, Sep 08, 2020 at 10:08:44AM +0100, Cristian Marussi wrote:
> > > 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() ?
> > 
> > Now I remember why I had bus_exit before unregistering the driver. I think
> > that should fix the issue. Let me know.
> > 
> 
> Yes that works for me, both as builtin or as a module now.
> 

With that fix, can I take this as:

Tested-by: Cristian Marussi <cristian.marussi@arm.com>

-- 
Regards,
Sudeep

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module
  2020-09-07 19:50 [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module Sudeep Holla
                   ` (4 preceding siblings ...)
  2020-09-08  9:08 ` [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module Cristian Marussi
@ 2020-09-14  6:36 ` Sudeep Holla
  5 siblings, 0 replies; 10+ messages in thread
From: Sudeep Holla @ 2020-09-14  6:36 UTC (permalink / raw)
  To: linux-arm-kernel, Sudeep Holla
  Cc: Mikhail Golubev, Igor Skalkin, Peter Hilber, Anton Yakovlev,
	Cristian Marussi

On Mon, 7 Sep 2020 20:50:42 +0100, Sudeep Holla wrote:
> 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.
> 
> [...]


Applied to sudeep.holla/linux (for-next/scmi), thanks!

[1/4] firmware: smccc: Export both smccc functions
      https://git.kernel.org/sudeep.holla/c/6825f17c95
[2/4] firmware: arm_scmi: Move scmi bus init and exit calls into the driver
      https://git.kernel.org/sudeep.holla/c/5a2f0a0bdf
[3/4] firmware: arm_scmi: Move scmi protocols registration into the driver
      https://git.kernel.org/sudeep.holla/c/1eaf18e35a
[4/4] (HEAD -> for-next/scmi) firmware: arm_scmi: Enable building as a single module
      https://git.kernel.org/sudeep.holla/c/66d90f6ece

--

Regards,
Sudeep


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

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-09-14  6:38 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-07 19:50 [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module Sudeep Holla
2020-09-07 19:50 ` [PATCH v2 1/4] firmware: smccc: export both smccc functions Sudeep Holla
2020-09-07 19:50 ` [PATCH v2 2/4] firmware: arm_scmi: Move scmi bus init and exit calls into the driver Sudeep Holla
2020-09-07 19:50 ` [PATCH v2 3/4] firmware: arm_scmi: Move scmi protocols registration " Sudeep Holla
2020-09-07 19:50 ` [PATCH v2 4/4] firmware: arm_scmi: Enable building as a single module Sudeep Holla
2020-09-08  9:08 ` [PATCH v2 0/4] firmware: arm_scmi: Enable building SCMI as module Cristian Marussi
2020-09-08 11:11   ` Sudeep Holla
2020-09-08 11:53     ` Cristian Marussi
2020-09-08 12:10       ` Sudeep Holla
2020-09-14  6:36 ` Sudeep Holla

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).