On Tue, 9 Nov 2021 at 15:39, Patrick DELAUNAY <patrick.delau...@foss.st.com> wrote: > > 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.delau...@foss.st.com> > > Cc: Wolfgang Denk <w...@denx.de> > > Signed-off-by: Etienne Carriere <etienne.carri...@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"); >
nice catch. Thanks, I'll post a v2. Etienne > > Patrick >