Re: [PATCH v2 4/8] firmware: arm_scmi: Make MBOX transport a standalone driver

Etienne CARRIERE
2024-07-23T13:41:27Z

Hi Cristian,

On Wednesday, July 10, 2024, Cristian Marussi wrote:
> Make SCMI mailbox transport a standalne driver that can be optionally
> loaded as a module.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
>  drivers/firmware/arm_scmi/Kconfig             |  4 +-
>  drivers/firmware/arm_scmi/Makefile            |  3 +-
>  drivers/firmware/arm_scmi/common.h            |  3 --
>  drivers/firmware/arm_scmi/driver.c            |  3 --
>  .../{mailbox.c => scmi_transport_mailbox.c}   | 44 +++++++++++++------
>  5 files changed, 36 insertions(+), 21 deletions(-)
>  rename drivers/firmware/arm_scmi/{mailbox.c => scmi_transport_mailbox.c} (88%)
>
> diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
> index aa5842be19b2..135e34aefd70 100644
> --- a/drivers/firmware/arm_scmi/Kconfig
> +++ b/drivers/firmware/arm_scmi/Kconfig
> @@ -75,7 +75,7 @@ config ARM_SCMI_HAVE_MSG
>            available.
>
>  config ARM_SCMI_TRANSPORT_MAILBOX
> -       bool "SCMI transport based on Mailbox"
> +       tristate "SCMI transport based on Mailbox"
>          depends on MAILBOX
>          select ARM_SCMI_HAVE_TRANSPORT
>          select ARM_SCMI_HAVE_SHMEM
> @@ -85,6 +85,8 @@ config ARM_SCMI_TRANSPORT_MAILBOX
>
>            If you want the ARM SCMI PROTOCOL stack to include support for a
>            transport based on mailboxes, answer Y.
> +         This driver can also be built as a module.  If so, the module

Nitpicking: replace the 2 space char before "if so," with a single one?
Applies also to patch 5/8, 6/8 and 7/8.

Other wise LGTM , but my comment on patch 3/8 that would affect
use of DEFINE_SCMI_TRANSPORT_DRIVER() in patch 5 to 7.

br,
etienne

> +         will be called scmi_transport_mailbox.
>
>  config ARM_SCMI_TRANSPORT_OPTEE
>          bool "SCMI transport based on OP-TEE service"
> (snip)