[PATCH v2 1/4] firmware: add new driver for SCMI firmwares

[Simon Glass]

Hi Etienne,

On Tue, 18 Aug 2020 at 09:44, Etienne Carriere
<etienne.carriere at linaro.org> wrote:
>
> This change introduces SCMI agent driver in U-Boot in the firmware
> U-class.
>
> SCMI agent driver is designed for platforms that embed a SCMI server in
> a firmware hosted for example by a companion co-processor or the secure
> world of the executing processor.
>
> SCMI protocols allow an SCMI agent to discover and access external
> resources as clock, reset controllers and many more. SCMI agent and
> server communicate following the SCMI specification [1]. SCMI agent
> complies with the DT bindings defined in the Linux kernel source tree
> regarding SCMI agent description since v5.8-rc1.
>
> These bindings describe 2 supported message transport layer: using
> mailbox uclass devices or using Arm SMC invocation instruction. Both
> use a piece or shared memory for message data exchange.
>
> In the current state, the SCMI agent driver does not bind to any SCMI
> protocol to a U-Boot device driver. Former changes will implement
> dedicated driver (i.e. an SCMI clock driver or an SCMI reset controller
> driver) and add bind supported SCMI protocols in scmi_agent_bind().
>
> Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
> Signed-off-by: Etienne Carriere <etienne.carriere at linaro.org>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Peng Fan <peng.fan at nxp.com>
> Cc: Sudeep Holla <sudeep.holla at arm.com>
> ---
>
> Changes in v2:
> - Fix CONFIG_SCMI_FIRMWARE description with explicit SCMI reference.
> - Move struct, enum and macro definitions at source file top and
>   add inline comment description for the structures and local functions.
> - Replace rc with ret as return value local variable label.
> - Use explicit return 0 on successful return paths.
> - Replace EINVAL with more accurate error numbers.
> - Use dev_read_u32() instead of ofnode_read_u32(dev_ofnode(), ...).
> - Use memcpy_toio()/memcpy_fromio() when copying message payload
>   to/from IO memory.
> - Embed mailbox transport resources upon CONFIG_DM_MAILBOX and
>   SMCCC transport resources upon CONFIG_ARM_SMCCC.
>
> Note: review comments on defining a uclass and sandbox for SCMI
> transport drivers are NOT addressed in this v2. Main issue is that
> there is no driver/device defined for SCMI transport layer as well as
> and no defined compatible ID in the SCMI DT bindings documentation.

I'd still like to see this. You can define an API with a header file.
It is certainly easier if the DT binding can cover the transport type
with a separate subnode. But that doesn't stop you creating a uclass
for the transport. It will also allow you to create a sandbox impl so
you can add a test for this code.

Also the two interfaces should really be in separate files rather than
using #ifdefs, I think.

> ---
>  drivers/firmware/Kconfig  |  20 ++
>  drivers/firmware/Makefile |   1 +
>  drivers/firmware/scmi.c   | 490 ++++++++++++++++++++++++++++++++++++++
>  include/scmi.h            |  82 +++++++
>  4 files changed, 593 insertions(+)
>  create mode 100644 drivers/firmware/scmi.c
>  create mode 100644 include/scmi.h
>

Regards,
Simon