diff mbox series

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

Message ID 20200907145000.30587-3-etienne.carriere@linaro.org
State New
Headers show
Series [v3,1/8] firmware: add SCMI agent uclass | expand

Commit Message

Etienne Carriere Sept. 7, 2020, 2:49 p.m. UTC
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@linaro.org>

Cc: Simon Glass <sjg@chromium.org>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sudeep Holla <sudeep.holla@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

-- 
2.17.1

Comments

Simon Glass Sept. 8, 2020, 3:20 p.m. UTC | #1
Hi Etienne,

On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
<etienne.carriere@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@linaro.org>

> Cc: Simon Glass <sjg@chromium.org>

> Cc: Peng Fan <peng.fan@nxp.com>

> Cc: Sudeep Holla <sudeep.holla@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@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.
> +

> +#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

[..]

Regards,
Simon
Etienne Carriere Sept. 9, 2020, 9:40 a.m. UTC | #2
On Tue, 8 Sep 2020 at 17:21, Simon Glass <sjg@chromium.org> wrote:
>

> Hi Etienne,

>

> On Mon, 7 Sep 2020 at 08:50, Etienne Carriere

> <etienne.carriere@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@linaro.org>

> > Cc: Simon Glass <sjg@chromium.org>

> > Cc: Peng Fan <peng.fan@nxp.com>

> > Cc: Sudeep Holla <sudeep.holla@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@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
Simon Glass Sept. 9, 2020, 2:35 p.m. UTC | #3
Hi Etienne,

On Wed, 9 Sep 2020 at 03:41, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>

> On Tue, 8 Sep 2020 at 17:21, Simon Glass <sjg@chromium.org> wrote:

> >

> > Hi Etienne,

> >

> > On Mon, 7 Sep 2020 at 08:50, Etienne Carriere

> > <etienne.carriere@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@linaro.org>

> > > Cc: Simon Glass <sjg@chromium.org>

> > > Cc: Peng Fan <peng.fan@nxp.com>

> > > Cc: Sudeep Holla <sudeep.holla@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@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.


Actually I think your function is OK if you want it. I was just
talking about the cast.

But yes, a lot of drivers just declare something like this in each function:

struct something_priv *priv = dev_get_priv(dev)

Regards,
Simon
diff mbox series

Patch

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>
+
+#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);
+}
+
+static int scmi_smccc_process_msg(struct udevice *dev, struct scmi_msg *msg)
+{
+	struct scmi_smccc_channel *chan = scmi_smccc_get_priv(dev);
+	struct arm_smccc_res res;
+	int ret;
+
+	ret = scmi_write_msg_to_smt(dev, &chan->smt, msg);
+	if (ret)
+		return ret;
+
+	arm_smccc_smc(chan->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
+	if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
+		ret = -ENXIO;
+	else
+		ret = scmi_read_resp_from_smt(dev, &chan->smt, msg);
+
+	scmi_clear_smt_channel(&chan->smt);
+
+	return ret;
+}
+
+static int scmi_smccc_probe(struct udevice *dev)
+{
+	struct scmi_smccc_channel *chan = scmi_smccc_get_priv(dev);
+	u32 func_id;
+	int ret;
+
+	if (dev_read_u32(dev, "arm,smc-id", &func_id)) {
+		dev_err(dev, "Missing property func-id\n");
+		return -EINVAL;
+	}
+
+	chan->func_id = func_id;
+
+	ret = scmi_dt_get_smt_buffer(dev, &chan->smt);
+	if (ret) {
+		dev_err(dev, "Failed to get smt resources: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct udevice_id scmi_smccc_ids[] = {
+	{ .compatible = "arm,scmi-smc" },
+	{ }
+};
+
+static const struct scmi_agent_ops scmi_smccc_ops = {
+	.process_msg = scmi_smccc_process_msg,
+};
+
+U_BOOT_DRIVER(scmi_smccc) = {
+	.name		= "scmi-over-smccc",
+	.id		= UCLASS_SCMI_AGENT,
+	.of_match	= scmi_smccc_ids,
+	.priv_auto_alloc_size = sizeof(struct scmi_smccc_channel),
+	.probe		= scmi_smccc_probe,
+	.ops		= &scmi_smccc_ops,
+};