Re: [PATCH v2 02/12] firmware: scmi: implement SCMI base protocol
Hi Etienne,
On Thu, Aug 03, 2023 at 09:35:57AM +, Etienne CARRIERE wrote:
> Hello Takahiro-san,
>
> > From: AKASHI Takahiro
> > Sent: Wednesday, July 26, 2023 10:37
> >
> > SCMI base protocol is mandatory according to the SCMI specification.
> >
> > With this patch, SCMI base protocol can be accessed via SCMI transport
> > layers. All the commands, except SCMI_BASE_NOTIFY_ERRORS, are supported.
> > This is because U-Boot doesn't support interrupts and the current transport
> > layers are not able to handle asynchronous messages properly.
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > v2
> > * add helper functions, removing direct uses of ops
> > * add function descriptions for each of functions in ops
>
> A reported typo and a question on use of strcpy().
Thank you for pointing out typos not only in this file, but also others.
While I won't reply each time, I will fix all those typos.
> Otherwise the patch look to me. If you strongly feel strcpy() is safe,
> please get my R-b tag:
My initial thought was that commands in Base protocol are expected
to be used mostly internally so the string size check can be superfluous.
But for more safety, I will change the code so that data for returned
strings are dynamically allocated by helper functions.
This approach will also be helpful and consistent, in the future, when
"exntended name" is supported in other protocols as well so people
don't have to worry about the maximum length in any case.
-Takahiro Akashi
> Reviewed-by: Etienne Carriere
>
> > ---
> > drivers/firmware/scmi/Makefile | 1 +
> > drivers/firmware/scmi/base.c | 637 +
> > include/dm/uclass-id.h | 1 +
> > include/scmi_protocols.h | 345 ++
> > 4 files changed, 984 insertions(+)
> > create mode 100644 drivers/firmware/scmi/base.c
> >
> > diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
> > index b2ff483c75a1..1a23d4981709 100644
> > --- a/drivers/firmware/scmi/Makefile
> > +++ b/drivers/firmware/scmi/Makefile
> > @@ -1,4 +1,5 @@
> > obj-y += scmi_agent-uclass.o
> > +obj-y += base.o
> > obj-y += smt.o
> > obj-$(CONFIG_SCMI_AGENT_SMCCC) += smccc_agent.o
> > obj-$(CONFIG_SCMI_AGENT_MAILBOX) += mailbox_agent.o
> > diff --git a/drivers/firmware/scmi/base.c b/drivers/firmware/scmi/base.c
> > new file mode 100644
> > index ..2b61fa650d15
> > --- /dev/null
> > +++ b/drivers/firmware/scmi/base.c
> > @@ -0,0 +1,637 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * SCMI Base protocol as U-Boot device
> > + *
> > + * Copyright (C) 2023 Linaro Limited
> > + * author: AKASHI Takahiro
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +/**
> > + * scmi_generic_protocol_version - get protocol version
> > + * @dev: SCMI device
> > + * @id: SCMI protocol ID
> > + * @version: Pointer to SCMI protocol version
> > + *
> > + * Obtain the protocol version number in @version.
> > + *
> > + * Return: 0 on success, error code otherwise
> > + */
> > +int scmi_generic_protocol_version(struct udevice *dev,
> > + enum scmi_std_protocol id, u32 *version)
> > +{
> > + struct scmi_protocol_version_out out;
> > + struct scmi_msg msg = {
> > + .protocol_id = id,
> > + .message_id = SCMI_PROTOCOL_VERSION,
> > + .out_msg = (u8 *)&out,
> > + .out_msg_sz = sizeof(out),
> > + };
> > + int ret;
> > +
> > + ret = devm_scmi_process_msg(dev, &msg);
> > + if (ret)
> > + return ret;
> > + if (out.status)
> > + return scmi_to_linux_errno(out.status);
> > +
> > + *version = out.version;
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * scmi_base_protocol_version_int - get Base protocol version
> > + * @dev: SCMI device
> > + * @version: Pointer to SCMI protocol version
> > + *
> > + * Obtain the protocol version number in @version for Base protocol.
> > + *
> > + * Return: 0 on success, error code otherwise
> > + */
> > +static int scmi_base_protocol_version_int(struct udevice *dev, u32
> > *version)
> > +{
> > + return scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_BASE,
> > + version);
> > +}
> > +
> > +/**
> > + * scmi_protocol_attrs_int - get protocol attributes
> > + * @dev: SCMI device
> > + * @num_agents: Number of SCMI agents
> > + * @num_protocols: Number of SCMI protocols
> > + *
> > + * Obtain the protocol attributes, the number of agents and the number
> > + * of protocols, in @num_agents and @num_protocols respectively, that
> > + * the device provides.
> > + *
> > + * Return: 0 on success, error code otherwise
> > + */
> > +static int scmi_protocol_attrs_int(struct udevice
RE: [PATCH v2 02/12] firmware: scmi: implement SCMI base protocol
Hello Takahiro-san,
> From: AKASHI Takahiro
> Sent: Wednesday, July 26, 2023 10:37
>
> SCMI base protocol is mandatory according to the SCMI specification.
>
> With this patch, SCMI base protocol can be accessed via SCMI transport
> layers. All the commands, except SCMI_BASE_NOTIFY_ERRORS, are supported.
> This is because U-Boot doesn't support interrupts and the current transport
> layers are not able to handle asynchronous messages properly.
>
> Signed-off-by: AKASHI Takahiro
> ---
> v2
> * add helper functions, removing direct uses of ops
> * add function descriptions for each of functions in ops
A reported typo and a question on use of strcpy().
Otherwise the patch look to me. If you strongly feel strcpy() is safe, please
get my R-b tag:
Reviewed-by: Etienne Carriere
> ---
> drivers/firmware/scmi/Makefile | 1 +
> drivers/firmware/scmi/base.c | 637 +
> include/dm/uclass-id.h | 1 +
> include/scmi_protocols.h | 345 ++
> 4 files changed, 984 insertions(+)
> create mode 100644 drivers/firmware/scmi/base.c
>
> diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
> index b2ff483c75a1..1a23d4981709 100644
> --- a/drivers/firmware/scmi/Makefile
> +++ b/drivers/firmware/scmi/Makefile
> @@ -1,4 +1,5 @@
> obj-y += scmi_agent-uclass.o
> +obj-y += base.o
> obj-y += smt.o
> obj-$(CONFIG_SCMI_AGENT_SMCCC) += smccc_agent.o
> obj-$(CONFIG_SCMI_AGENT_MAILBOX) += mailbox_agent.o
> diff --git a/drivers/firmware/scmi/base.c b/drivers/firmware/scmi/base.c
> new file mode 100644
> index ..2b61fa650d15
> --- /dev/null
> +++ b/drivers/firmware/scmi/base.c
> @@ -0,0 +1,637 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * SCMI Base protocol as U-Boot device
> + *
> + * Copyright (C) 2023 Linaro Limited
> + * author: AKASHI Takahiro
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/**
> + * scmi_generic_protocol_version - get protocol version
> + * @dev: SCMI device
> + * @id: SCMI protocol ID
> + * @version: Pointer to SCMI protocol version
> + *
> + * Obtain the protocol version number in @version.
> + *
> + * Return: 0 on success, error code otherwise
> + */
> +int scmi_generic_protocol_version(struct udevice *dev,
> + enum scmi_std_protocol id, u32 *version)
> +{
> + struct scmi_protocol_version_out out;
> + struct scmi_msg msg = {
> + .protocol_id = id,
> + .message_id = SCMI_PROTOCOL_VERSION,
> + .out_msg = (u8 *)&out,
> + .out_msg_sz = sizeof(out),
> + };
> + int ret;
> +
> + ret = devm_scmi_process_msg(dev, &msg);
> + if (ret)
> + return ret;
> + if (out.status)
> + return scmi_to_linux_errno(out.status);
> +
> + *version = out.version;
> +
> + return 0;
> +}
> +
> +/**
> + * scmi_base_protocol_version_int - get Base protocol version
> + * @dev: SCMI device
> + * @version: Pointer to SCMI protocol version
> + *
> + * Obtain the protocol version number in @version for Base protocol.
> + *
> + * Return: 0 on success, error code otherwise
> + */
> +static int scmi_base_protocol_version_int(struct udevice *dev, u32 *version)
> +{
> + return scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_BASE,
> + version);
> +}
> +
> +/**
> + * scmi_protocol_attrs_int - get protocol attributes
> + * @dev: SCMI device
> + * @num_agents: Number of SCMI agents
> + * @num_protocols: Number of SCMI protocols
> + *
> + * Obtain the protocol attributes, the number of agents and the number
> + * of protocols, in @num_agents and @num_protocols respectively, that
> + * the device provides.
> + *
> + * Return: 0 on success, error code otherwise
> + */
> +static int scmi_protocol_attrs_int(struct udevice *dev, u32 *num_agents,
> + u32 *num_protocols)
> +{
> + struct scmi_protocol_attrs_out out;
> + struct scmi_msg msg = {
> + .protocol_id = SCMI_PROTOCOL_ID_BASE,
> + .message_id = SCMI_PROTOCOL_ATTRIBUTES,
> + .out_msg = (u8 *)&out,
> + .out_msg_sz = sizeof(out),
> + };
> + int ret;
> +
> + ret = devm_scmi_process_msg(dev, &msg);
> + if (ret)
> + return ret;
> + if (out.status)
> + return scmi_to_linux_errno(out.status);
> +
> + *num_agents = SCMI_PROTOCOL_ATTRS_NUM_AGENTS(out.attributes);
> + *num_protocols = SCMI_PROTOCOL_ATTRS_NUM_PROTOCOLS(out.attributes);
> +
> + return 0;
> +}
> +
> +/**
> + * scmi_protocol_message_attrs_int - get message-specific attributes
> + * @dev: SCMI device
> + * @message_id: SCMI message ID
> + * @attrib
Re: [PATCH v2 02/12] firmware: scmi: implement SCMI base protocol
On Wed, 26 Jul 2023 at 02:38, AKASHI Takahiro wrote: > > SCMI base protocol is mandatory according to the SCMI specification. > > With this patch, SCMI base protocol can be accessed via SCMI transport > layers. All the commands, except SCMI_BASE_NOTIFY_ERRORS, are supported. > This is because U-Boot doesn't support interrupts and the current transport > layers are not able to handle asynchronous messages properly. > > Signed-off-by: AKASHI Takahiro > --- > v2 > * add helper functions, removing direct uses of ops > * add function descriptions for each of functions in ops > --- > drivers/firmware/scmi/Makefile | 1 + > drivers/firmware/scmi/base.c | 637 + > include/dm/uclass-id.h | 1 + > include/scmi_protocols.h | 345 ++ > 4 files changed, 984 insertions(+) > create mode 100644 drivers/firmware/scmi/base.c Reviewed-by: Simon Glass
[PATCH v2 02/12] firmware: scmi: implement SCMI base protocol
SCMI base protocol is mandatory according to the SCMI specification.
With this patch, SCMI base protocol can be accessed via SCMI transport
layers. All the commands, except SCMI_BASE_NOTIFY_ERRORS, are supported.
This is because U-Boot doesn't support interrupts and the current transport
layers are not able to handle asynchronous messages properly.
Signed-off-by: AKASHI Takahiro
---
v2
* add helper functions, removing direct uses of ops
* add function descriptions for each of functions in ops
---
drivers/firmware/scmi/Makefile | 1 +
drivers/firmware/scmi/base.c | 637 +
include/dm/uclass-id.h | 1 +
include/scmi_protocols.h | 345 ++
4 files changed, 984 insertions(+)
create mode 100644 drivers/firmware/scmi/base.c
diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
index b2ff483c75a1..1a23d4981709 100644
--- a/drivers/firmware/scmi/Makefile
+++ b/drivers/firmware/scmi/Makefile
@@ -1,4 +1,5 @@
obj-y += scmi_agent-uclass.o
+obj-y += base.o
obj-y += smt.o
obj-$(CONFIG_SCMI_AGENT_SMCCC) += smccc_agent.o
obj-$(CONFIG_SCMI_AGENT_MAILBOX) += mailbox_agent.o
diff --git a/drivers/firmware/scmi/base.c b/drivers/firmware/scmi/base.c
new file mode 100644
index ..2b61fa650d15
--- /dev/null
+++ b/drivers/firmware/scmi/base.c
@@ -0,0 +1,637 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * SCMI Base protocol as U-Boot device
+ *
+ * Copyright (C) 2023 Linaro Limited
+ * author: AKASHI Takahiro
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * scmi_generic_protocol_version - get protocol version
+ * @dev: SCMI device
+ * @id:SCMI protocol ID
+ * @version: Pointer to SCMI protocol version
+ *
+ * Obtain the protocol version number in @version.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+int scmi_generic_protocol_version(struct udevice *dev,
+ enum scmi_std_protocol id, u32 *version)
+{
+ struct scmi_protocol_version_out out;
+ struct scmi_msg msg = {
+ .protocol_id = id,
+ .message_id = SCMI_PROTOCOL_VERSION,
+ .out_msg = (u8 *)&out,
+ .out_msg_sz = sizeof(out),
+ };
+ int ret;
+
+ ret = devm_scmi_process_msg(dev, &msg);
+ if (ret)
+ return ret;
+ if (out.status)
+ return scmi_to_linux_errno(out.status);
+
+ *version = out.version;
+
+ return 0;
+}
+
+/**
+ * scmi_base_protocol_version_int - get Base protocol version
+ * @dev: SCMI device
+ * @version: Pointer to SCMI protocol version
+ *
+ * Obtain the protocol version number in @version for Base protocol.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int scmi_base_protocol_version_int(struct udevice *dev, u32 *version)
+{
+ return scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_BASE,
+version);
+}
+
+/**
+ * scmi_protocol_attrs_int - get protocol attributes
+ * @dev: SCMI device
+ * @num_agents:Number of SCMI agents
+ * @num_protocols: Number of SCMI protocols
+ *
+ * Obtain the protocol attributes, the number of agents and the number
+ * of protocols, in @num_agents and @num_protocols respectively, that
+ * the device provides.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int scmi_protocol_attrs_int(struct udevice *dev, u32 *num_agents,
+ u32 *num_protocols)
+{
+ struct scmi_protocol_attrs_out out;
+ struct scmi_msg msg = {
+ .protocol_id = SCMI_PROTOCOL_ID_BASE,
+ .message_id = SCMI_PROTOCOL_ATTRIBUTES,
+ .out_msg = (u8 *)&out,
+ .out_msg_sz = sizeof(out),
+ };
+ int ret;
+
+ ret = devm_scmi_process_msg(dev, &msg);
+ if (ret)
+ return ret;
+ if (out.status)
+ return scmi_to_linux_errno(out.status);
+
+ *num_agents = SCMI_PROTOCOL_ATTRS_NUM_AGENTS(out.attributes);
+ *num_protocols = SCMI_PROTOCOL_ATTRS_NUM_PROTOCOLS(out.attributes);
+
+ return 0;
+}
+
+/**
+ * scmi_protocol_message_attrs_int - get message-specific attributes
+ * @dev: SCMI device
+ * @message_id:SCMI message ID
+ * @attributes:Message-specific attributes
+ *
+ * Obtain the message-specific attributes in @attributes.
+ * This command succeeds if the message is implemented and available.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int scmi_protocol_message_attrs_int(struct udevice *dev, u32 message_id,
+ u32 *attributes)
+{
+ struct scmi_protocol_msg_attrs_out out;
+ struct scmi_msg msg = {
+ .protocol_id = SCMI_PROTOCOL_ID_BASE,
+ .message_id = SCMI_PROTO
