Re: [PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport
Hi Etienne,
On Wed, 9 Sep 2020 at 03:41, Etienne Carriere
wrote:
>
> On Tue, 8 Sep 2020 at 17:21, Simon Glass wrote:
> >
> > Hi Etienne,
> >
> > On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
> > 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
> > > Cc: Simon Glass
> > > Cc: Peng Fan
> > > Cc: Sudeep Holla
> > > ---
> > >
> > > 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
> >
> > 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 00..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
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > That should go below the next one.
>
> acked.
>
> > > +
> > > +#include
> > > +#include
> > > +
> > > +#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
Re: [PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport
On Tue, 8 Sep 2020 at 17:21, Simon Glass wrote:
>
> Hi Etienne,
>
> On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
> 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
> > Cc: Simon Glass
> > Cc: Peng Fan
> > Cc: Sudeep Holla
> > ---
> >
> > 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
>
> 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 00..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
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> That should go below the next one.
acked.
> > +
> > +#include
> > +#include
> > +
> > +#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
Re: [PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport
Hi Etienne,
On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
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
> Cc: Simon Glass
> Cc: Peng Fan
> Cc: Sudeep Holla
> ---
>
> 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
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 00..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
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
That should go below the next one.
> +
> +#include
> +#include
> +
> +#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
[PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport
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
Cc: Simon Glass
Cc: Peng Fan
Cc: Sudeep Holla
---
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
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 00..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
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#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, >smt, msg);
+ if (ret)
+ return ret;
+
+ arm_smccc_smc(chan->func_id, 0, 0, 0, 0, 0, 0, 0, );
+ if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
+ ret = -ENXIO;
+ else
+ ret = scmi_read_resp_from_smt(dev, >smt, msg);
+
+ scmi_clear_smt_channel(>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", _id)) {
+ dev_err(dev, "Missing property func-id\n");
+ return -EINVAL;
+ }
+
+ chan->func_id = func_id;
+
+ ret = scmi_dt_get_smt_buffer(dev, >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= _smccc_ops,
+};
--
2.17.1
