Re: [PATCH v2 2/2] firmware: arm_ffa: Split bus and driver into distinct modules

[Sebastian Ene <sebastianene@google.com>]

On Wed, May 15, 2024 at 10:40:28AM +0100, Sudeep Holla wrote:
> Make the FF-A bus on its own as a distinct module initialized at
> subsys_initcall level when builtin.
> 
> Keep the FF-A driver core stack, together with any configured transport,
> in a different module initialized as module_init level.
> 
> FF-A drivers initialization is now changed to module_init level.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/firmware/arm_ffa/Makefile |  6 ++++--
>  drivers/firmware/arm_ffa/bus.c    | 11 +++++++++--
>  drivers/firmware/arm_ffa/common.h |  2 --
>  drivers/firmware/arm_ffa/driver.c | 12 ++----------
>  4 files changed, 15 insertions(+), 16 deletions(-)
> 
> v1->v2
> 	- Move all the workaround related logic into the other patch
> 
> diff --git a/drivers/firmware/arm_ffa/Makefile b/drivers/firmware/arm_ffa/Makefile
> index 9d9f37523200..168990a7e792 100644
> --- a/drivers/firmware/arm_ffa/Makefile
> +++ b/drivers/firmware/arm_ffa/Makefile
> @@ -2,5 +2,7 @@
>  ffa-bus-y = bus.o
>  ffa-driver-y = driver.o
>  ffa-transport-$(CONFIG_ARM_FFA_SMCCC) += smccc.o
> -ffa-module-objs := $(ffa-bus-y) $(ffa-driver-y) $(ffa-transport-y)
> -obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-module.o
> +ffa-core-objs := $(ffa-bus-y)
> +ffa-module-objs := $(ffa-driver-y) $(ffa-transport-y)
> +obj-$(CONFIG_ARM_FFA_TRANSPORT)  = ffa-core.o
> +obj-$(CONFIG_ARM_FFA_TRANSPORT) += ffa-module.o
> diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
> index 4baaec7f0a09..0c83931485f6 100644
> --- a/drivers/firmware/arm_ffa/bus.c
> +++ b/drivers/firmware/arm_ffa/bus.c
> @@ -235,14 +235,21 @@ void ffa_device_unregister(struct ffa_device *ffa_dev)
>  }
>  EXPORT_SYMBOL_GPL(ffa_device_unregister);
> 
> -int arm_ffa_bus_init(void)
> +static int __init arm_ffa_bus_init(void)
>  {
>  	return bus_register(&ffa_bus_type);
>  }
> +subsys_initcall(arm_ffa_bus_init);
> 
> -void arm_ffa_bus_exit(void)
> +static void __exit arm_ffa_bus_exit(void)
>  {
>  	ffa_devices_unregister();
>  	bus_unregister(&ffa_bus_type);
>  	ida_destroy(&ffa_bus_id);
>  }
> +module_exit(arm_ffa_bus_exit);
> +
> +MODULE_ALIAS("ffa-core");
> +MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
> +MODULE_DESCRIPTION("ARM FF-A bus");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/firmware/arm_ffa/common.h b/drivers/firmware/arm_ffa/common.h
> index d6eccf1fd3f6..9c6425a81d0d 100644
> --- a/drivers/firmware/arm_ffa/common.h
> +++ b/drivers/firmware/arm_ffa/common.h
> @@ -14,8 +14,6 @@ typedef struct arm_smccc_1_2_regs ffa_value_t;
> 
>  typedef void (ffa_fn)(ffa_value_t, ffa_value_t *);
> 
> -int arm_ffa_bus_init(void);
> -void arm_ffa_bus_exit(void);
>  bool ffa_device_is_valid(struct ffa_device *ffa_dev);
>  void ffa_device_match_uuid(struct ffa_device *ffa_dev, const uuid_t *uuid);
> 
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index 61d514776e5b..7ba98c7af2e9 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -1608,14 +1608,9 @@ static int __init ffa_init(void)
>  	if (ret)
>  		return ret;
> 
> -	ret = arm_ffa_bus_init();
> -	if (ret)
> -		return ret;
> -
>  	drv_info = kzalloc(sizeof(*drv_info), GFP_KERNEL);
>  	if (!drv_info) {
> -		ret = -ENOMEM;
> -		goto ffa_bus_exit;
> +		return -ENOMEM;
>  	}
> 
>  	ret = ffa_version_check(&drv_info->version);
> @@ -1676,11 +1671,9 @@ static int __init ffa_init(void)
>  	free_pages_exact(drv_info->rx_buffer, RXTX_BUFFER_SIZE);
>  free_drv_info:
>  	kfree(drv_info);
> -ffa_bus_exit:
> -	arm_ffa_bus_exit();
>  	return ret;
>  }
> -subsys_initcall(ffa_init);
> +module_init(ffa_init);
> 
>  static void __exit ffa_exit(void)
>  {
> @@ -1690,7 +1683,6 @@ static void __exit ffa_exit(void)
>  	free_pages_exact(drv_info->tx_buffer, RXTX_BUFFER_SIZE);
>  	free_pages_exact(drv_info->rx_buffer, RXTX_BUFFER_SIZE);
>  	kfree(drv_info);
> -	arm_ffa_bus_exit();
>  }
>  module_exit(ffa_exit);
> 
> --
> 2.43.2

Hello Sudeep,

This looks good to me, thanks.

Acked-by: Sebastian Ene <sebastianene@google.com> 

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