[PATCH 2/3] firmware: arm_scmi: Move scmi protocols initialisation into the driver

Cristian Marussi cristian.marussi at arm.com
Mon Sep 7 14:06:01 EDT 2020


On Mon, Sep 07, 2020 at 12:29:19PM +0100, Sudeep Holla wrote:
> In preparation to enable building SCMI as a single module, let us move
> the SCMI protocol initialisation call into the driver. This enables us
> to also add de-initialisation 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 at arm.com>
> ---
>  drivers/firmware/arm_scmi/clock.c   |  7 +------
>  drivers/firmware/arm_scmi/common.h  | 21 +++++++++++++++++++++
>  drivers/firmware/arm_scmi/driver.c  | 10 ++++++++++
>  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 +------
>  7 files changed, 36 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
> index 75e39882746e..606396f748f0 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_INIT_EXIT(SCMI_PROTOCOL_CLOCK, clock)
> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> index 5fa42eba6de7..6d98a6c47005 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_INIT_EXIT(func)		\
> +	int __init scmi_##func##_init(void);	\
> +	void __exit scmi_##func##_exit(void)
> +DECLARE_SCMI_INIT_EXIT(clock);
> +DECLARE_SCMI_INIT_EXIT(perf);
> +DECLARE_SCMI_INIT_EXIT(power);
> +DECLARE_SCMI_INIT_EXIT(reset);
> +DECLARE_SCMI_INIT_EXIT(sensors);
> +DECLARE_SCMI_INIT_EXIT(bus);
> +

Can we call these protocols' functions (and related macros) something like:

	scmi_##PROTO##_load/_unload or _register/_unregister 

given that in SCMI stack we usually intend something else with protocol
initialization and in fact each protocol has its own dedicated protocol_init
function which is called at a different time.


> +#define DEFINE_SCMI_PROTOCOL_INIT_EXIT(id, name)	\
> +int __init scmi_##name##_init(void)			\
> +{ \
> +	return scmi_protocol_register((id), &scmi_##name##_protocol_init); \
> +} \
> +\
> +void __exit scmi_##name##_exit(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 f4d9601c053f..2a1396b74fa5 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -931,6 +931,11 @@ static struct platform_driver scmi_driver = {
>  static int __init scmi_driver_init(void)
>  {
>  	scmi_bus_init();
> +	scmi_clock_init();
> +	scmi_perf_init();
> +	scmi_power_init();
> +	scmi_reset_init();
> +	scmi_sensors_init();
>  
>  	return platform_driver_register(&scmi_driver);
>  }
> @@ -939,6 +944,11 @@ module_init(scmi_driver_init);
>  static void __exit scmi_driver_exit(void)
>  {
>  	scmi_bus_exit();

Shouldn't this bus_exit() be issued in reverse oerder at the end
after protocols have being _exited() ?

Cheers,

Cristian

> +	scmi_clock_exit();
> +	scmi_perf_exit();
> +	scmi_power_exit();
> +	scmi_reset_exit();
> +	scmi_sensors_exit();
>  
>  	return platform_driver_unregister(&scmi_driver);
>  }
> diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
> index 3e1e87012c95..e0ec1a78d2ad 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_INIT_EXIT(SCMI_PROTOCOL_PERF, perf)
> diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c
> index 46f213644c49..54926d1f381c 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_INIT_EXIT(SCMI_PROTOCOL_POWER, power)
> diff --git a/drivers/firmware/arm_scmi/reset.c b/drivers/firmware/arm_scmi/reset.c
> index 3691bafca057..4b91d39cfe21 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_INIT_EXIT(SCMI_PROTOCOL_RESET, reset)
> diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
> index 1af0ad362e82..37dc187cafea 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_INIT_EXIT(SCMI_PROTOCOL_SENSOR, sensors)
> -- 
> 2.17.1
> 



More information about the linux-arm-kernel mailing list