[PATCH 5/5] firmware: scmi: Add OP-TEE transport

[Patrick DELAUNAY]

Hi Etienne,

one missing remark, don't see in my first review.

On 11/4/21 3:14 PM, Etienne Carriere wrote:
> This change implements an SCMI transport for agent interfacing the
> OP-TEE SCMI service. OP-TEE provides an SCMI PTA (Pseudo-TA) for
> non-secure world to send SCMI messages over an identified channel.
> The driver implemented here uses a SMT shared memory for passing
> messages between client and server.
>
> The implementation opens and releases channel resources for each
> passed SCMI message so that resources allocated (sessions) or
> registered (shared memory areas) in OP-TEE firmware are released for
> example before relocation as the driver will likely allocate/register
> them back when probed after relocation.
>
> The integration of the driver using dedicated config switch
> CONFIG_SCMI_AGENT_OPTEE is designed on the model posted to the
> U-Boot ML by Patrick Delaunay [1].
>
> Link: [1] https://lore.kernel.org/all/20211028191222.v3.4.Ib2e58ee67f4d023823d8b5404332dc4d7e847277@changeid/
> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
> Cc: Wolfgang Denk <wd at denx.de>
> Signed-off-by: Etienne Carriere <etienne.carriere at linaro.org>
> ---
>   drivers/firmware/scmi/Kconfig       |  14 +-
>   drivers/firmware/scmi/Makefile      |   1 +
>   drivers/firmware/scmi/optee_agent.c | 312 ++++++++++++++++++++++++++++
>   3 files changed, 324 insertions(+), 3 deletions(-)
>   create mode 100644 drivers/firmware/scmi/optee_agent.c
>
> diff --git a/drivers/firmware/scmi/Kconfig b/drivers/firmware/scmi/Kconfig
> index c3a109beac..f9c69e5352 100644
> --- a/drivers/firmware/scmi/Kconfig
> +++ b/drivers/firmware/scmi/Kconfig
> @@ -2,7 +2,7 @@ config SCMI_FIRMWARE
>   	bool "Enable SCMI support"
>   	select FIRMWARE
>   	select OF_TRANSLATE
> -	depends on SANDBOX || DM_MAILBOX || ARM_SMCCC
> +	depends on SANDBOX || DM_MAILBOX || ARM_SMCCC || OPTEE
>   	help
>   	  System Control and Management Interface (SCMI) is a communication
>   	  protocol that defines standard interfaces for power, performance
> @@ -14,6 +14,14 @@ config SCMI_FIRMWARE
>   	  or a companion host in the CPU system.
>   
>   	  Communications between agent (client) and the SCMI server are
> -	  based on message exchange. Messages can be exchange over tranport
> +	  based on message exchange. Messages can be exchanged over transport
>   	  channels as a mailbox device or an Arm SMCCC service with some
> -	  piece of identified shared memory.
> +	  piece of identified shared memory, or an OP-TEE SCMI service.
> +
> +config SCMI_AGENT_OPTEE
> +	bool "Enable SCMI agent OP-TEE"
> +	depends on SCMI_FIRMWARE && OPTEE
> +	default y
> +	help
> +	  Enable the SCMI communication channel based on OP-TEE transport
> +	  for compatible "linaro,scmi-optee".
> diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
> index 966475ec10..f72d06a400 100644
> --- a/drivers/firmware/scmi/Makefile
> +++ b/drivers/firmware/scmi/Makefile
> @@ -2,4 +2,5 @@ obj-y	+= scmi_agent-uclass.o
>   obj-y	+= smt.o
>   obj-$(CONFIG_ARM_SMCCC)		+= smccc_agent.o
>   obj-$(CONFIG_DM_MAILBOX)	+= mailbox_agent.o
> +obj-$(CONFIG_SCMI_AGENT_OPTEE)	+= optee_agent.o
>   obj-$(CONFIG_SANDBOX)		+= sandbox-scmi_agent.o sandbox-scmi_devices.o
> diff --git a/drivers/firmware/scmi/optee_agent.c b/drivers/firmware/scmi/optee_agent.c
> new file mode 100644
> index 0000000000..8232af2f41
> --- /dev/null
> +++ b/drivers/firmware/scmi/optee_agent.c
> @@ -0,0 +1,312 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020-2021 Linaro Limited.
> + */
> +

(...)

> +
> +static int scmi_optee_of_to_plat(struct udevice *dev)
> +{
> +	struct scmi_optee_channel *chan = dev_get_plat(dev);
> +	int ret;
> +
> +	if (dev_read_u32(dev, "linaro,optee-channel-id", &chan->channel_id)) {
> +		dev_err(dev, "Missing property linaro,channel-id\n");

Can you use the correct property name in the error message = "linaro,optee-channel-id"

+		dev_err(dev, "Missing property linaro,optee-channel-id\n");


Patrick