[PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport

[Etienne Carriere]

On Tue, 8 Sep 2020 at 17:21, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Etienne,
>
> On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
> <etienne.carriere at linaro.org> wrote:
> >
> > This change implements a SMCCC transport for SCMI exchanges. This
> > implementation follows the Linux kernel as references implementation
> > for SCMI message processing, using the SMT format for communication
> > channel meta-data.
> >
> > Use of SMCCC transport in SCMI FDT bindings are defined in the Linux
> > kernel DT bindings since v5.8. SMCCC with SMT is implemented in OP-TEE
> > from tag 3.9.0 [2].
> >
> > Links: [2] https://github.com/OP-TEE/optee_os/commit/a58c4d706d23
> > 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 v3:
> > - This is a followup of the SCMI agent patches posted in
> >   https://patchwork.ozlabs.org/project/uboot/list/?series=196253
> >   The v3 splits commits and introduces a new uclass as requested.
> > - This patch implements the same Arm SMCCC SCMI agent as presented
> >   in v2 but in its own source file smccc_agent.c, and based in smt.h.
> > ---
> >  drivers/firmware/scmi/Kconfig       |  4 +-
> >  drivers/firmware/scmi/Makefile      |  1 +
> >  drivers/firmware/scmi/smccc_agent.c | 95 +++++++++++++++++++++++++++++
> >  3 files changed, 98 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/firmware/scmi/smccc_agent.c
> >
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> nits below
>
> > diff --git a/drivers/firmware/scmi/Kconfig b/drivers/firmware/scmi/Kconfig
> > index c501bf4943..335d09c821 100644
> > --- a/drivers/firmware/scmi/Kconfig
> > +++ b/drivers/firmware/scmi/Kconfig
> > @@ -15,5 +15,5 @@ config SCMI_FIRMWARE
> >
> >           Communications between agent (client) and the SCMI server are
> >           based on message exchange. Messages can be exchange over tranport
> > -         channels as a mailbox device with some piece of identified shared
> > -         memory.
> > +         channels as a mailbox device or an Arm SMCCC service with some
> > +         piece of identified shared memory.
> > diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
> > index d22f53efe7..2f782bbd55 100644
> > --- a/drivers/firmware/scmi/Makefile
> > +++ b/drivers/firmware/scmi/Makefile
> > @@ -1,4 +1,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_SANDBOX)          += sandbox-scmi_agent.o
> > diff --git a/drivers/firmware/scmi/smccc_agent.c b/drivers/firmware/scmi/smccc_agent.c
> > new file mode 100644
> > index 0000000000..90707710e2
> > --- /dev/null
> > +++ b/drivers/firmware/scmi/smccc_agent.c
> > @@ -0,0 +1,95 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2020 Linaro Limited.
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <errno.h>
> > +#include <scmi_agent.h>
> > +#include <scmi_agent-uclass.h>
> > +#include <dm/devres.h>
> > +#include <linux/arm-smccc.h>
> That should go below the next one.

acked.

> > +
> > +#include <dm/device-internal.h>
> > +#include <linux/compat.h>
> > +
> > +#include "smt.h"
> > +
> > +#define SMCCC_RET_NOT_SUPPORTED         ((unsigned long)-1)
> > +
> > +/**
> > + * struct scmi_smccc_channel - Description of an SCMI SMCCC transport
> > + * @func_id:   SMCCC function ID used by the SCMI transport
> > + * @smt:       Shared memory buffer
> > + */
> > +struct scmi_smccc_channel {
> > +       ulong func_id;
> > +       struct scmi_smt smt;
> > +};
> > +
> > +static struct scmi_smccc_channel *scmi_smccc_get_priv(struct udevice *dev)
> > +{
> > +       return (struct scmi_smccc_channel *)dev_get_priv(dev);
>
> You shouldn't need that cast

acked. So no need for helper scmi_smccc_get_priv(),
caller can call dev_get_priv() straight.

etienne

>
> [..]
>
> Regards,
> Simon