All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/8] firmware: add SCMI agent uclass
@ 2020-09-07 14:49 Etienne Carriere
  2020-09-07 14:49 ` [PATCH v3 2/8] firmware: scmi: mailbox/smt agent device Etienne Carriere
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Etienne Carriere @ 2020-09-07 14:49 UTC (permalink / raw)
  To: u-boot

This change introduces SCMI agent uclass to interact with a firmware
using the SCMI protocols [1].

SCMI agent uclass currently supports a single method to request
processing of the SCMI message by an identified server. A SCMI message
is made of a byte payload associated to a protocol ID and a message ID,
all defined by the SCMI specification [1]. On return from process_msg()
method, the caller gets the service response.

SCMI agent uclass defines a post bind generic sequence for all devices.
The sequence binds all the SCMI protocols listed in the FDT for that
SCMI agent device. Currently none, but later change will introduce
protocols.

This change implements a simple sandbox device for the SCMI agent uclass.
The sandbox nicely answers SCMI_NOT_SUPPORTED to SCMI messages.
To prepare for further test support, the sandbox exposes a architecture
function for test application to read the sandbox emulated devices state.
Currently supports 2 SCMI agents, identified by an ID in the FDT device
name. The simplistic DM test does nothing yet.

SCMI agent uclass is designed for platforms that embed a SCMI server in
a firmware hosted somewhere, for example in a companion co-processor or
in the secure world of the executing processor. SCMI protocols allow an
SCMI agent to discover and access external resources as clock, reset
controllers and more. SCMI agent and server communicate following the
SCMI specification [1]. This SCMI agent implementation complies with
the DT bindings defined in the Linux kernel source tree regarding
SCMI agent description since v5.8.

Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
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:
- Address comments about adding a new uclass and some sandbox test from
  v2 in https://patchwork.ozlabs.org/project/uboot/list/?series=196253
- New directory drivers/firmware/scmi/. The path mimics Linux kernel
  source tree for the equivalent driver.
- Split scmi.h (patch v2) into scmi_protocols.h, scmi_agent.h and
  scmi_agent-uclass.h.
- Create new uclass UCLASS_SCMI_AGENT.
- Introduce a simple sandbox on that agent. Mailbox and smccc agents are
  moved to specific commits in the series.

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.
---
 arch/sandbox/dts/test.dts                  |  16 +++
 arch/sandbox/include/asm/scmi_test.h       |  43 ++++++
 configs/sandbox_defconfig                  |   2 +
 drivers/firmware/Kconfig                   |   2 +
 drivers/firmware/Makefile                  |   1 +
 drivers/firmware/scmi/Kconfig              |  17 +++
 drivers/firmware/scmi/Makefile             |   2 +
 drivers/firmware/scmi/sandbox-scmi_agent.c | 147 +++++++++++++++++++++
 drivers/firmware/scmi/scmi_agent-uclass.c  | 107 +++++++++++++++
 include/dm/uclass-id.h                     |   1 +
 include/scmi_agent-uclass.h                |  24 ++++
 include/scmi_agent.h                       |  68 ++++++++++
 include/scmi_protocols.h                   |  41 ++++++
 test/dm/Makefile                           |   1 +
 test/dm/scmi.c                             |  38 ++++++
 15 files changed, 510 insertions(+)
 create mode 100644 arch/sandbox/include/asm/scmi_test.h
 create mode 100644 drivers/firmware/scmi/Kconfig
 create mode 100644 drivers/firmware/scmi/Makefile
 create mode 100644 drivers/firmware/scmi/sandbox-scmi_agent.c
 create mode 100644 drivers/firmware/scmi/scmi_agent-uclass.c
 create mode 100644 include/scmi_agent-uclass.h
 create mode 100644 include/scmi_agent.h
 create mode 100644 include/scmi_protocols.h
 create mode 100644 test/dm/scmi.c

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 9f45c48e4e..dd3b43885e 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -356,6 +356,22 @@
 		sandbox_firmware: sandbox-firmware {
 			compatible = "sandbox,firmware";
 		};
+
+		sandbox-scmi-agent at 0 {
+			compatible = "sandbox,scmi-agent";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		sandbox-scmi-agent at 1 {
+			compatible = "sandbox,scmi-agent";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			protocol at 10 {
+				reg = <0x10>;
+			};
+		};
 	};
 
 	pinctrl-gpio {
diff --git a/arch/sandbox/include/asm/scmi_test.h b/arch/sandbox/include/asm/scmi_test.h
new file mode 100644
index 0000000000..a811fe19c3
--- /dev/null
+++ b/arch/sandbox/include/asm/scmi_test.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020, Linaro Limited
+ */
+
+#ifndef __SANDBOX_SCMI_TEST_H
+#define __SANDBOX_SCMI_TEST_H
+
+struct udevice;
+struct sandbox_scmi_agent;
+struct sandbox_scmi_service;
+
+/**
+ * struct sandbox_scmi_agent - Simulated SCMI service seen by SCMI agent
+ * @idx:	Identifier for the SCMI agent, its index
+ */
+struct sandbox_scmi_agent {
+	uint idx;
+};
+
+/**
+ * struct sandbox_scmi_service - Reference to simutaed SCMI agents/services
+ * @agent:		Pointer to SCMI sandbox agent pointers array
+ * @agent_count:	Number of emulated agents exposed in array @agent.
+ */
+struct sandbox_scmi_service {
+	struct sandbox_scmi_agent **agent;
+	size_t agent_count;
+};
+
+#ifdef CONFIG_SCMI_FIRMWARE
+/**
+ * sandbox_scmi_service_context - Get the simulated SCMI services context
+ * @return:	Reference to backend simulated resources state
+ */
+struct sandbox_scmi_service *sandbox_scmi_service_ctx(void);
+#else
+static inline struct sandbox_scmi_service *sandbox_scmi_service_ctx(void)
+{
+	return NULL;
+}
+#endif /* CONFIG_SCMI_FIRMWARE */
+#endif /* __SANDBOX_SCMI_TEST_H */
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 6e9f029cc9..2c130c01f0 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -132,6 +132,8 @@ CONFIG_BOARD_SANDBOX=y
 CONFIG_DMA=y
 CONFIG_DMA_CHANNELS=y
 CONFIG_SANDBOX_DMA=y
+CONFIG_FIRMWARE=y
+CONFIG_SCMI_FIRMWARE=y
 CONFIG_GPIO_HOG=y
 CONFIG_DM_GPIO_LOOKUP_LABEL=y
 CONFIG_PM8916_GPIO=y
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index b70a206355..ef958b3a7a 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -36,3 +36,5 @@ config ZYNQMP_FIRMWARE
 	  various platform management services.
 	  Say yes to enable ZynqMP firmware interface driver.
 	  If in doubt, say N.
+
+source "drivers/firmware/scmi/Kconfig"
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index a0c250a473..7ce83d72bd 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_$(SPL_)ARM_PSCI_FW)	+= psci.o
 obj-$(CONFIG_TI_SCI_PROTOCOL)	+= ti_sci.o
 obj-$(CONFIG_SANDBOX)		+= firmware-sandbox.o
 obj-$(CONFIG_ZYNQMP_FIRMWARE)	+= firmware-zynqmp.o
+obj-$(CONFIG_SCMI_FIRMWARE)	+= scmi/
diff --git a/drivers/firmware/scmi/Kconfig b/drivers/firmware/scmi/Kconfig
new file mode 100644
index 0000000000..57e2ebbe42
--- /dev/null
+++ b/drivers/firmware/scmi/Kconfig
@@ -0,0 +1,17 @@
+config SCMI_FIRMWARE
+	bool "Enable SCMI support"
+	select FIRMWARE
+	select OF_TRANSLATE
+	depends on SANDBOX
+	help
+	  System Control and Management Interface (SCMI) is a communication
+	  protocol that defines standard interfaces for power, performance
+	  and system management. The SCMI specification is available at
+	  https://developer.arm.com/architectures/system-architectures/software-standards/scmi
+
+	  An SCMI agent communicates with a related SCMI server firmware
+	  located in another sub-system, as a companion micro controller
+	  or a companion host in the CPU system.
+
+	  Communications between agent (client) and the SCMI server are
+	  based on message exchange.
diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
new file mode 100644
index 0000000000..336ea1f2a3
--- /dev/null
+++ b/drivers/firmware/scmi/Makefile
@@ -0,0 +1,2 @@
+obj-y	+= scmi_agent-uclass.o
+obj-$(CONFIG_SANDBOX)		+= sandbox-scmi_agent.o
diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
new file mode 100644
index 0000000000..ce0d49c951
--- /dev/null
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020, Linaro Limited
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <malloc.h>
+#include <scmi_agent.h>
+#include <scmi_agent-uclass.h>
+#include <scmi_protocols.h>
+#include <asm/io.h>
+#include <asm/scmi_test.h>
+#include <dm/device_compat.h>
+
+/*
+ * The sandbox SCMI agent driver simulates to some extend a SCMI message
+ * processing. It simulates few of the SCMI services for some of the
+ * SCMI protocols embedded in U-Boot. Currently none.
+ *
+ * This driver simulates 2 SCMI agents for test purpose.
+ *
+ * This Driver exports sandbox_scmi_service_ct() for the test sequence to
+ * get the state of the simulated services (clock state, rate, ...) and
+ * check back-end device state reflects the request send through the
+ * various uclass devices, currently nothing.
+ */
+
+#define SANDBOX_SCMI_AGENT_COUNT	2
+
+/* The list saves to simulted end devices references for test purpose */
+struct sandbox_scmi_agent *sandbox_scmi_agent_list[SANDBOX_SCMI_AGENT_COUNT];
+
+static struct sandbox_scmi_service sandbox_scmi_service_state = {
+	.agent = sandbox_scmi_agent_list,
+	.agent_count = SANDBOX_SCMI_AGENT_COUNT,
+};
+
+struct sandbox_scmi_service *sandbox_scmi_service_ctx(void)
+{
+	return &sandbox_scmi_service_state;
+}
+
+struct sandbox_scmi_agent *dev2agent(struct udevice *dev)
+{
+	return (struct sandbox_scmi_agent *)dev_get_priv(dev);
+}
+
+static void debug_print_agent_state(struct udevice *dev, char *str)
+{
+	struct sandbox_scmi_agent *agent = dev2agent(dev);
+
+	dev_dbg(dev, "Dump sandbox_scmi_agent %u: %s\n", agent->idx, str);
+};
+
+static int sandbox_scmi_test_process_msg(struct udevice *dev,
+					 struct scmi_msg *msg)
+{
+	switch (msg->protocol_id) {
+	case SCMI_PROTOCOL_ID_BASE:
+	case SCMI_PROTOCOL_ID_POWER_DOMAIN:
+	case SCMI_PROTOCOL_ID_SYSTEM:
+	case SCMI_PROTOCOL_ID_PERF:
+	case SCMI_PROTOCOL_ID_CLOCK:
+	case SCMI_PROTOCOL_ID_SENSOR:
+	case SCMI_PROTOCOL_ID_RESET_DOMAIN:
+		*(u32 *)msg->out_msg = SCMI_NOT_SUPPORTED;
+		return 0;
+	default:
+		break;
+	}
+
+	dev_err(dev, "%s(%s): Unhandled protocol_id %#x/message_id %#x\n",
+		__func__, dev->name, msg->protocol_id, msg->message_id);
+
+	if (msg->out_msg_sz < sizeof(u32))
+		return -EINVAL;
+
+	/* Intentionnaly report unhandled IDs through the SCMI return code */
+	*(u32 *)msg->out_msg = SCMI_PROTOCOL_ERROR;
+	return 0;
+}
+
+static int sandbox_scmi_test_remove(struct udevice *dev)
+{
+	struct sandbox_scmi_agent *agent = dev2agent(dev);
+
+	debug_print_agent_state(dev, "removed");
+
+	/* We only need to dereference the agent in the context */
+	sandbox_scmi_service_ctx()->agent[agent->idx] = NULL;
+
+	return 0;
+}
+
+static int sandbox_scmi_test_probe(struct udevice *dev)
+{
+	static const char basename[] = "sandbox-scmi-agent@";
+	struct sandbox_scmi_agent *agent = dev2agent(dev);
+	const size_t basename_size = sizeof(basename) - 1;
+
+	if (strncmp(basename, dev->name, basename_size))
+		return -ENOENT;
+
+	switch (dev->name[basename_size]) {
+	case '0':
+		*agent = (struct sandbox_scmi_agent){
+			.idx = 0,
+		};
+		break;
+	case '1':
+		*agent = (struct sandbox_scmi_agent){
+			.idx = 1,
+		};
+		break;
+	default:
+		dev_err(dev, "%s(): Unexpected agent ID %s\n",
+			__func__, dev->name + basename_size);
+		return -ENOENT;
+	}
+
+	debug_print_agent_state(dev, "probed");
+
+	/* Save reference for tests purpose */
+	sandbox_scmi_service_ctx()->agent[agent->idx] = agent;
+
+	return 0;
+};
+
+static const struct udevice_id sandbox_scmi_test_ids[] = {
+	{ .compatible = "sandbox,scmi-agent" },
+	{ }
+};
+
+struct scmi_agent_ops sandbox_scmi_test_ops = {
+	.process_msg = sandbox_scmi_test_process_msg,
+};
+
+U_BOOT_DRIVER(sandbox_scmi_agent) = {
+	.name = "sandbox-scmi_agent",
+	.id = UCLASS_SCMI_AGENT,
+	.of_match = sandbox_scmi_test_ids,
+	.priv_auto_alloc_size = sizeof(struct sandbox_scmi_agent),
+	.probe = sandbox_scmi_test_probe,
+	.remove = sandbox_scmi_test_remove,
+	.ops = &sandbox_scmi_test_ops,
+};
diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c
new file mode 100644
index 0000000000..67a6f907c9
--- /dev/null
+++ b/drivers/firmware/scmi/scmi_agent-uclass.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 Linaro Limited.
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <scmi_agent-uclass.h>
+#include <scmi_protocols.h>
+
+#include <dm/device-internal.h>
+#include <linux/compat.h>
+
+/**
+ * struct error_code - Helper structure for SCMI error code conversion
+ * @scmi:	SCMI error code
+ * @errno:	Related standard error number
+ */
+struct error_code {
+	int scmi;
+	int errno;
+};
+
+static const struct error_code scmi_linux_errmap[] = {
+	{ .scmi = SCMI_NOT_SUPPORTED, .errno = -EOPNOTSUPP, },
+	{ .scmi = SCMI_INVALID_PARAMETERS, .errno = -EINVAL, },
+	{ .scmi = SCMI_DENIED, .errno = -EACCES, },
+	{ .scmi = SCMI_NOT_FOUND, .errno = -ENOENT, },
+	{ .scmi = SCMI_OUT_OF_RANGE, .errno = -ERANGE, },
+	{ .scmi = SCMI_BUSY, .errno = -EBUSY, },
+	{ .scmi = SCMI_COMMS_ERROR, .errno = -ECOMM, },
+	{ .scmi = SCMI_GENERIC_ERROR, .errno = -EIO, },
+	{ .scmi = SCMI_HARDWARE_ERROR, .errno = -EREMOTEIO, },
+	{ .scmi = SCMI_PROTOCOL_ERROR, .errno = -EPROTO, },
+};
+
+int scmi_to_linux_errno(s32 scmi_code)
+{
+	int n;
+
+	if (!scmi_code)
+		return 0;
+
+	for (n = 0; n < ARRAY_SIZE(scmi_linux_errmap); n++)
+		if (scmi_code == scmi_linux_errmap[n].scmi)
+			return scmi_linux_errmap[1].errno;
+
+	return -EPROTO;
+}
+
+/*
+ * SCMI agent devices binds devices of various uclasses depeding on
+ * the FDT description. scmi_bind_protocol() is a generic bind sequence
+ * called by the uclass@bind stage, that is uclass post_bind.
+ */
+static int scmi_bind_protocols(struct udevice *dev)
+{
+	int ret = 0;
+	ofnode node;
+	struct driver *drv;
+
+	dev_for_each_subnode(node, dev) {
+		u32 protocol_id;
+
+		if (!ofnode_is_available(node))
+			continue;
+
+		if (ofnode_read_u32(node, "reg", &protocol_id))
+			continue;
+
+		switch (protocol_id) {
+		default:
+			dev_info(dev, "Ignore unsupported SCMI protocol %#x\n",
+				 protocol_id);
+			continue;
+		}
+
+		ret = device_bind_ofnode(dev, drv, ofnode_get_name(node),
+					 NULL, node, NULL);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+static const struct scmi_agent_ops *transport_dev_ops(struct udevice *dev)
+{
+	return (const struct scmi_agent_ops *)dev->driver->ops;
+}
+
+int devm_scmi_process_msg(struct udevice *dev, struct scmi_msg *msg)
+{
+	const struct scmi_agent_ops *ops = transport_dev_ops(dev);
+
+	if (ops->process_msg)
+		return ops->process_msg(dev, msg);
+
+	return -EPROTONOSUPPORT;
+}
+
+UCLASS_DRIVER(scmi_agent) = {
+	.id		= UCLASS_SCMI_AGENT,
+	.name		= "scmi_agent",
+	.post_bind	= scmi_bind_protocols,
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 4ec5fa6670..88f10c4622 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -94,6 +94,7 @@ enum uclass_id {
 	UCLASS_RESET,		/* Reset controller device */
 	UCLASS_RNG,		/* Random Number Generator */
 	UCLASS_RTC,		/* Real time clock device */
+	UCLASS_SCMI_AGENT,	/* Interface with an SCMI server */
 	UCLASS_SCSI,		/* SCSI device */
 	UCLASS_SERIAL,		/* Serial UART */
 	UCLASS_SIMPLE_BUS,	/* Bus with child devices */
diff --git a/include/scmi_agent-uclass.h b/include/scmi_agent-uclass.h
new file mode 100644
index 0000000000..a501d1b482
--- /dev/null
+++ b/include/scmi_agent-uclass.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2019-2020 Linaro Limited.
+ */
+#ifndef _SCMI_AGENT_UCLASS_H
+#define _SCMI_AGENT_UCLASS_H
+
+struct udevice;
+struct scmi_msg;
+
+/**
+ * struct scmi_transport_ops - The functions that a SCMI transport layer must implement.
+ */
+struct scmi_agent_ops {
+	/*
+	 * process_msg - Request transport to get the SCMI message processed
+	 *
+	 * @agent:		Agent using the transport
+	 * @msg:		SCMI message to be transmitted
+	 */
+	int (*process_msg)(struct udevice *dev, struct scmi_msg *msg);
+};
+
+#endif /* _SCMI_TRANSPORT_UCLASS_H */
diff --git a/include/scmi_agent.h b/include/scmi_agent.h
new file mode 100644
index 0000000000..f1be9ff209
--- /dev/null
+++ b/include/scmi_agent.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
+/*
+ * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights reserved.
+ * Copyright (C) 2019-2020, Linaro Limited
+ *
+ * An SCMI agent device represent on communication path from a
+ * device driver to the remote SCMI server which driver sends
+ * messages to and receives response messages from.
+ */
+#ifndef SCMI_AGENT_H
+#define SCMI_AGENT_H
+
+#include <asm/types.h>
+
+struct udevice;
+
+/*
+ * struct scmi_msg - Context of a SCMI message sent and the response received
+ *
+ * @protocol_id:	SCMI protocol ID
+ * @message_id:		SCMI message ID for a defined protocol ID
+ * @in_msg:		Pointer to the message payload sent by the driver
+ * @in_msg_sz:		Byte size of the message payload sent
+ * @out_msg:		Pointer to buffer to store response message payload
+ * @out_msg_sz:		Byte size of the response buffer and response payload
+ */
+struct scmi_msg {
+	unsigned int protocol_id;
+	unsigned int message_id;
+	u8 *in_msg;
+	size_t in_msg_sz;
+	u8 *out_msg;
+	size_t out_msg_sz;
+};
+
+/* Helper macro to match a message on input/output array references */
+#define SCMI_MSG_IN(_protocol, _message, _in_array, _out_array) \
+	(struct scmi_msg){			\
+		.protocol_id = (_protocol),	\
+		.message_id = (_message),	\
+		.in_msg = (uint8_t *)&(_in_array),	\
+		.in_msg_sz = sizeof(_in_array),	\
+		.out_msg = (uint8_t *)&(_out_array),	\
+		.out_msg_sz = sizeof(_out_array),	\
+	}
+
+/**
+ * scmi_send_and_process_msg() - send and process a SCMI message
+ *
+ * Send a message to a SCMI server through a target SCMI agent device.
+ * Caller sets scmi_msg::out_msg_sz to the output message buffer size.
+ * On return, scmi_msg::out_msg_sz stores the response payload size.
+ *
+ * @dev:	SCMI agent device
+ * @msg:	Message structure reference
+ * @return 0 on success and a negative errno on failure
+ */
+int devm_scmi_process_msg(struct udevice *dev, struct scmi_msg *msg);
+
+/**
+ * scmi_to_linux_errno() - Convert an SCMI error code into a Linux errno code
+ *
+ * @scmi_errno:	SCMI error code value
+ * @return 0 for successful status and a negative errno otherwise
+ */
+int scmi_to_linux_errno(s32 scmi_errno);
+
+#endif /* SCMI_H */
diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h
new file mode 100644
index 0000000000..86a2d109c8
--- /dev/null
+++ b/include/scmi_protocols.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
+/*
+ * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights reserved.
+ * Copyright (C) 2019-2020, Linaro Limited
+ */
+#ifndef _SCMI_PROTOCOLS_H
+#define _SCMI_PROTOCOLS_H
+
+#include <linux/bitops.h>
+
+/*
+ * Subset the SCMI protocols definition
+ * based on SCMI specification v2.0 (DEN0056B)
+ * https://developer.arm.com/docs/den0056/b
+ */
+
+enum scmi_std_protocol {
+	SCMI_PROTOCOL_ID_BASE = 0x10,
+	SCMI_PROTOCOL_ID_POWER_DOMAIN = 0x11,
+	SCMI_PROTOCOL_ID_SYSTEM = 0x12,
+	SCMI_PROTOCOL_ID_PERF = 0x13,
+	SCMI_PROTOCOL_ID_CLOCK = 0x14,
+	SCMI_PROTOCOL_ID_SENSOR = 0x15,
+	SCMI_PROTOCOL_ID_RESET_DOMAIN = 0x16,
+};
+
+enum scmi_status_code {
+	SCMI_SUCCESS =  0,
+	SCMI_NOT_SUPPORTED = -1,
+	SCMI_INVALID_PARAMETERS = -2,
+	SCMI_DENIED = -3,
+	SCMI_NOT_FOUND = -4,
+	SCMI_OUT_OF_RANGE = -5,
+	SCMI_BUSY = -6,
+	SCMI_COMMS_ERROR = -7,
+	SCMI_GENERIC_ERROR = -8,
+	SCMI_HARDWARE_ERROR = -9,
+	SCMI_PROTOCOL_ERROR = -10,
+};
+
+#endif /* _SCMI_PROTOCOLS_H */
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 864c8d0b4c..70ba1b6695 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -80,4 +80,5 @@ obj-$(CONFIG_DM_RNG) += rng.o
 obj-$(CONFIG_CLK_K210_SET_RATE) += k210_pll.o
 obj-$(CONFIG_SIMPLE_PM_BUS) += simple-pm-bus.o
 obj-$(CONFIG_RESET_SYSCON) += syscon-reset.o
+obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o
 endif
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
new file mode 100644
index 0000000000..d8c1e71f12
--- /dev/null
+++ b/test/dm/scmi.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020, Linaro Limited
+ *
+ * Tests scmi_agent uclass and the SCMI drivers implemented in other
+ * uclass devices probe when a SCMI server exposes resources.
+ *
+ * Note in test.dts the protocol at 10 node in agent 1. Protocol 0x10 is not
+ * implemented in U-Boot SCMI components but the implementation is exepected
+ * to not complain on unknown protocol IDs, as long as it is not used. Note
+ * in test.dts tests that SCMI drivers probing does not fail for such an
+ * unknown SCMI protocol ID.
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <asm/scmi_test.h>
+#include <dm/device-internal.h>
+#include <dm/test.h>
+#include <test/ut.h>
+
+/*
+ * Test SCMI states when loading and releasing resources
+ * related to SCMI drivers.
+ */
+static int dm_test_scmi_sandbox_agent(struct unit_test_state *uts)
+{
+	struct sandbox_scmi_service *scmi_ctx = sandbox_scmi_service_ctx();
+
+	ut_assertnonnull(scmi_ctx);
+	ut_asserteq(2, scmi_ctx->agent_count);
+	ut_assertnull(scmi_ctx->agent[0]);
+	ut_assertnull(scmi_ctx->agent[1]);
+
+	return 0;
+}
+
+DM_TEST(dm_test_scmi_sandbox_agent, UT_TESTF_SCAN_FDT);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 2/8] firmware: scmi: mailbox/smt agent device
  2020-09-07 14:49 [PATCH v3 1/8] firmware: add SCMI agent uclass Etienne Carriere
@ 2020-09-07 14:49 ` Etienne Carriere
  2020-09-08 15:20   ` Simon Glass
  2020-09-07 14:49 ` [PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport Etienne Carriere
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Etienne Carriere @ 2020-09-07 14:49 UTC (permalink / raw)
  To: u-boot

This change implements a mailbox transport using SMT format for SCMI
exchanges. This implementation follows the Linux kernel and
SCP-firmware [1] as references implementation for SCMI message
processing using SMT format for communication channel meta-data.

Use of mailboxes in SCMI FDT bindings are defined in the Linux kernel
DT bindings since v4.17.

Links: [1] https://github.com/ARM-software/SCP-firmware
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 mailbox SCMI agent proposed in v2
  but split over few source files.
---
 drivers/firmware/scmi/Kconfig         |   6 +-
 drivers/firmware/scmi/Makefile        |   2 +
 drivers/firmware/scmi/mailbox_agent.c | 107 ++++++++++++++++++++
 drivers/firmware/scmi/smt.c           | 139 ++++++++++++++++++++++++++
 drivers/firmware/scmi/smt.h           |  86 ++++++++++++++++
 5 files changed, 338 insertions(+), 2 deletions(-)
 create mode 100644 drivers/firmware/scmi/mailbox_agent.c
 create mode 100644 drivers/firmware/scmi/smt.c
 create mode 100644 drivers/firmware/scmi/smt.h

diff --git a/drivers/firmware/scmi/Kconfig b/drivers/firmware/scmi/Kconfig
index 57e2ebbe42..c501bf4943 100644
--- a/drivers/firmware/scmi/Kconfig
+++ b/drivers/firmware/scmi/Kconfig
@@ -2,7 +2,7 @@ config SCMI_FIRMWARE
 	bool "Enable SCMI support"
 	select FIRMWARE
 	select OF_TRANSLATE
-	depends on SANDBOX
+	depends on SANDBOX || DM_MAILBOX
 	help
 	  System Control and Management Interface (SCMI) is a communication
 	  protocol that defines standard interfaces for power, performance
@@ -14,4 +14,6 @@ config SCMI_FIRMWARE
 	  or a companion host in the CPU system.
 
 	  Communications between agent (client) and the SCMI server are
-	  based on message exchange.
+	  based on message exchange. Messages can be exchange over tranport
+	  channels as a mailbox device with some piece of identified shared
+	  memory.
diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
index 336ea1f2a3..d22f53efe7 100644
--- a/drivers/firmware/scmi/Makefile
+++ b/drivers/firmware/scmi/Makefile
@@ -1,2 +1,4 @@
 obj-y	+= scmi_agent-uclass.o
+obj-y	+= smt.o
+obj-$(CONFIG_DM_MAILBOX)	+= mailbox_agent.o
 obj-$(CONFIG_SANDBOX)		+= sandbox-scmi_agent.o
diff --git a/drivers/firmware/scmi/mailbox_agent.c b/drivers/firmware/scmi/mailbox_agent.c
new file mode 100644
index 0000000000..9a7b0a5858
--- /dev/null
+++ b/drivers/firmware/scmi/mailbox_agent.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 Linaro Limited.
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <mailbox.h>
+#include <scmi_agent.h>
+#include <scmi_agent-uclass.h>
+#include <dm/devres.h>
+#include <linux/compat.h>
+
+#include "smt.h"
+
+#define TIMEOUT_US_10MS			10000
+
+/**
+ * struct scmi_mbox_channel - Description of an SCMI mailbox transport
+ * @smt:	Shared memory buffer
+ * @mbox:	Mailbox channel description
+ * @timeout_us:	Timeout in microseconds for the mailbox transfer
+ */
+struct scmi_mbox_channel {
+	struct scmi_smt smt;
+	struct mbox_chan mbox;
+	ulong timeout_us;
+};
+
+static struct scmi_mbox_channel *scmi_mbox_get_priv(struct udevice *dev)
+{
+	return (struct scmi_mbox_channel *)dev_get_priv(dev);
+}
+
+static int scmi_mbox_process_msg(struct udevice *dev, struct scmi_msg *msg)
+{
+	struct scmi_mbox_channel *chan = scmi_mbox_get_priv(dev);
+	int ret;
+
+	ret = scmi_write_msg_to_smt(dev, &chan->smt, msg);
+	if (ret)
+		return ret;
+
+	/* Give shm addr to mbox in case it is meaningful */
+	ret = mbox_send(&chan->mbox, chan->smt.buf);
+	if (ret) {
+		dev_err(dev, "Message send failed: %d\n", ret);
+		goto out;
+	}
+
+	/* Receive the response */
+	ret = mbox_recv(&chan->mbox, chan->smt.buf, chan->timeout_us);
+	if (ret) {
+		dev_err(dev, "Response failed: %d, abort\n", ret);
+		goto out;
+	}
+
+	ret = scmi_read_resp_from_smt(dev, &chan->smt, msg);
+
+out:
+	scmi_clear_smt_channel(&chan->smt);
+
+	return ret;
+}
+
+int scmi_mbox_probe(struct udevice *dev)
+{
+	struct scmi_mbox_channel *chan = scmi_mbox_get_priv(dev);
+	int ret;
+
+	chan->timeout_us = TIMEOUT_US_10MS;
+
+	ret = mbox_get_by_index(dev, 0, &chan->mbox);
+	if (ret) {
+		dev_err(dev, "Failed to find mailbox: %d\n", ret);
+		goto out;
+	}
+
+	ret = scmi_dt_get_smt_buffer(dev, &chan->smt);
+	if (ret)
+		dev_err(dev, "Failed to get shm resources: %d\n", ret);
+
+out:
+	if (ret)
+		devm_kfree(dev, chan);
+
+	return ret;
+}
+
+static const struct udevice_id scmi_mbox_ids[] = {
+	{ .compatible = "arm,scmi" },
+	{ }
+};
+
+static const struct scmi_agent_ops scmi_mbox_ops = {
+	.process_msg = scmi_mbox_process_msg,
+};
+
+U_BOOT_DRIVER(scmi_mbox) = {
+	.name		= "scmi-over-mailbox",
+	.id		= UCLASS_SCMI_AGENT,
+	.of_match	= scmi_mbox_ids,
+	.priv_auto_alloc_size = sizeof(struct scmi_mbox_channel),
+	.probe		= scmi_mbox_probe,
+	.ops		= &scmi_mbox_ops,
+};
diff --git a/drivers/firmware/scmi/smt.c b/drivers/firmware/scmi/smt.c
new file mode 100644
index 0000000000..afe95a4736
--- /dev/null
+++ b/drivers/firmware/scmi/smt.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights reserved.
+ * Copyright (C) 2019-2020 Linaro Limited.
+ */
+
+#include <common.h>
+#include <cpu_func.h>
+#include <dm.h>
+#include <errno.h>
+#include <scmi_agent.h>
+#include <asm/cache.h>
+#include <asm/system.h>
+#include <dm/ofnode.h>
+#include <linux/compat.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+
+#include "smt.h"
+
+/**
+ * Get shared memory configuration defined by the referred DT phandle
+ * Return with a errno compliant value.
+ */
+int scmi_dt_get_smt_buffer(struct udevice *dev, struct scmi_smt *smt)
+{
+	int ret;
+	struct ofnode_phandle_args args;
+	struct resource resource;
+	fdt32_t faddr;
+	phys_addr_t paddr;
+
+	ret = dev_read_phandle_with_args(dev, "shmem", NULL, 0, 0, &args);
+	if (ret)
+		return ret;
+
+	ret = ofnode_read_resource(args.node, 0, &resource);
+	if (ret)
+		return ret;
+
+	faddr = cpu_to_fdt32(resource.start);
+	paddr = ofnode_translate_address(args.node, &faddr);
+
+	smt->size = resource_size(&resource);
+	if (smt->size < sizeof(struct scmi_smt_header)) {
+		dev_err(dev, "Shared memory buffer too small\n");
+		return -EINVAL;
+	}
+
+	smt->buf = devm_ioremap(dev, paddr, smt->size);
+	if (!smt->buf)
+		return -ENOMEM;
+
+#ifdef __arm__
+	if (dcache_status())
+		mmu_set_region_dcache_behaviour((uintptr_t)smt->buf,
+						smt->size, DCACHE_OFF);
+#endif
+
+	return 0;
+}
+
+/**
+ * Write SCMI message @msg into a SMT shared buffer @smt.
+ * Return 0 on success and with a negative errno in case of error.
+ */
+int scmi_write_msg_to_smt(struct udevice *dev, struct scmi_smt *smt,
+			  struct scmi_msg *msg)
+{
+	struct scmi_smt_header *hdr = (void *)smt->buf;
+
+	if ((!msg->in_msg && msg->in_msg_sz) ||
+	    (!msg->out_msg && msg->out_msg_sz))
+		return -EINVAL;
+
+	if (!(hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) {
+		dev_dbg(dev, "Channel busy\n");
+		return -EBUSY;
+	}
+
+	if (smt->size < (sizeof(*hdr) + msg->in_msg_sz) ||
+	    smt->size < (sizeof(*hdr) + msg->out_msg_sz)) {
+		dev_dbg(dev, "Buffer too small\n");
+		return -ETOOSMALL;
+	}
+
+	/* Load message in shared memory */
+	hdr->channel_status &= ~SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE;
+	hdr->length = msg->in_msg_sz + sizeof(hdr->msg_header);
+	hdr->msg_header = SMT_HEADER_TOKEN(0) |
+			  SMT_HEADER_MESSAGE_TYPE(0) |
+			  SMT_HEADER_PROTOCOL_ID(msg->protocol_id) |
+			  SMT_HEADER_MESSAGE_ID(msg->message_id);
+
+	memcpy_toio(hdr->msg_payload, msg->in_msg, msg->in_msg_sz);
+
+	return 0;
+}
+
+/**
+ * Read SCMI message from a SMT shared buffer @smt and copy it into @msg.
+ * Return 0 on success and with a negative errno in case of error.
+ */
+int scmi_read_resp_from_smt(struct udevice *dev, struct scmi_smt *smt,
+			    struct scmi_msg *msg)
+{
+	struct scmi_smt_header *hdr = (void *)smt->buf;
+
+	if (!(hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) {
+		dev_err(dev, "Channel unexpectedly busy\n");
+		return -EBUSY;
+	}
+
+	if (hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR) {
+		dev_err(dev, "Channel error reported, reset channel\n");
+		return -ECOMM;
+	}
+
+	if (hdr->length > msg->out_msg_sz + sizeof(hdr->msg_header)) {
+		dev_err(dev, "Buffer to small\n");
+		return -ETOOSMALL;
+	}
+
+	/* Get the data */
+	msg->out_msg_sz = hdr->length - sizeof(hdr->msg_header);
+	memcpy_fromio(msg->out_msg, hdr->msg_payload, msg->out_msg_sz);
+
+	return 0;
+}
+
+/**
+ * Clear SMT flags in shared buffer to allow further message exchange
+ */
+void scmi_clear_smt_channel(struct scmi_smt *smt)
+{
+	struct scmi_smt_header *hdr = (void *)smt->buf;
+
+	hdr->channel_status &= ~SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR;
+}
diff --git a/drivers/firmware/scmi/smt.h b/drivers/firmware/scmi/smt.h
new file mode 100644
index 0000000000..a8c0987bd3
--- /dev/null
+++ b/drivers/firmware/scmi/smt.h
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights reserved.
+ * Copyright (C) 2019-2020 Linaro Limited.
+ */
+#ifndef SCMI_SMT_H
+#define SCMI_SMT_H
+
+#include <asm/types.h>
+
+/**
+ * struct scmi_smt_header - Description of the shared memory message buffer
+ *
+ * SMT stands for Shared Memory based Transport.
+ * SMT uses 28 byte header prior message payload to handle the state of
+ * the communication channel realized by the shared memory area and
+ * to define SCMI protocol information the payload relates to.
+ */
+struct scmi_smt_header {
+	__le32 reserved;
+	__le32 channel_status;
+#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR	BIT(1)
+#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE	BIT(0)
+	__le32 reserved1[2];
+	__le32 flags;
+#define SCMI_SHMEM_FLAG_INTR_ENABLED		BIT(0)
+	__le32 length;
+	__le32 msg_header;
+	u8 msg_payload[0];
+};
+
+#define SMT_HEADER_TOKEN(token)		(((token) << 18) & GENMASK(31, 18))
+#define SMT_HEADER_PROTOCOL_ID(proto)	(((proto) << 10) & GENMASK(17, 10))
+#define SMT_HEADER_MESSAGE_TYPE(type)	(((type) << 18) & GENMASK(9, 8))
+#define SMT_HEADER_MESSAGE_ID(id)	((id) & GENMASK(7, 0))
+
+/**
+ * struct scmi_smt - Description of a SMT memory buffer
+ * @buf:	Shared memory base address
+ * @size:	Shared memory byte size
+ */
+struct scmi_smt {
+	u8 *buf;
+	size_t size;
+};
+
+static inline bool scmi_smt_channel_is_free(struct scmi_smt *smt)
+{
+	struct scmi_smt_header *hdr = (void *)smt->buf;
+
+	return hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE;
+}
+
+static inline bool scmi_smt_channel_reports_error(struct scmi_smt *smt)
+{
+	struct scmi_smt_header *hdr = (void *)smt->buf;
+
+	return hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR;
+}
+
+static inline void scmi_smt_get_channel(struct scmi_smt *smt)
+{
+	struct scmi_smt_header *hdr = (void *)smt->buf;
+
+	hdr->channel_status &= ~SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE;
+}
+
+static inline void scmi_smt_put_channel(struct scmi_smt *smt)
+{
+	struct scmi_smt_header *hdr = (void *)smt->buf;
+
+	hdr->channel_status |= SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE;
+	hdr->channel_status &= ~SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR;
+}
+
+int scmi_dt_get_smt_buffer(struct udevice *dev, struct scmi_smt *smt);
+
+int scmi_write_msg_to_smt(struct udevice *dev, struct scmi_smt *smt,
+			  struct scmi_msg *msg);
+
+int scmi_read_resp_from_smt(struct udevice *dev, struct scmi_smt *smt,
+			    struct scmi_msg *msg);
+
+void scmi_clear_smt_channel(struct scmi_smt *smt);
+
+#endif /* SCMI_SMT_H */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport
  2020-09-07 14:49 [PATCH v3 1/8] firmware: add SCMI agent uclass Etienne Carriere
  2020-09-07 14:49 ` [PATCH v3 2/8] firmware: scmi: mailbox/smt agent device Etienne Carriere
@ 2020-09-07 14:49 ` Etienne Carriere
  2020-09-08 15:20   ` Simon Glass
  2020-09-07 14:49 ` [PATCH v3 4/8] dt-bindings: arm: SCMI bindings documentation Etienne Carriere
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Etienne Carriere @ 2020-09-07 14:49 UTC (permalink / raw)
  To: u-boot

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

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,
+};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 4/8] dt-bindings: arm: SCMI bindings documentation
  2020-09-07 14:49 [PATCH v3 1/8] firmware: add SCMI agent uclass Etienne Carriere
  2020-09-07 14:49 ` [PATCH v3 2/8] firmware: scmi: mailbox/smt agent device Etienne Carriere
  2020-09-07 14:49 ` [PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport Etienne Carriere
@ 2020-09-07 14:49 ` Etienne Carriere
  2020-09-07 14:49 ` [PATCH v3 5/8] clk: add clock driver for SCMI agents Etienne Carriere
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Etienne Carriere @ 2020-09-07 14:49 UTC (permalink / raw)
  To: u-boot

Dump SCMI DT bindings documentation from Linux kernel source
tree v5.8-rc1.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
No change in v3.

Changes in v2:
- No change but added R-b tag.
- Yet a question: do we need to add this binding doc in U-Boot
  since already existing in Linux DT bindings docs? Related to review
  comment https://www.mail-archive.com/u-boot at lists.denx.de/msg377725.html
---
 doc/device-tree-bindings/arm/arm,scmi.txt | 197 ++++++++++++++++++++++
 1 file changed, 197 insertions(+)
 create mode 100644 doc/device-tree-bindings/arm/arm,scmi.txt

diff --git a/doc/device-tree-bindings/arm/arm,scmi.txt b/doc/device-tree-bindings/arm/arm,scmi.txt
new file mode 100644
index 0000000000..1f293ea24c
--- /dev/null
+++ b/doc/device-tree-bindings/arm/arm,scmi.txt
@@ -0,0 +1,197 @@
+System Control and Management Interface (SCMI) Message Protocol
+----------------------------------------------------------
+
+The SCMI is intended to allow agents such as OSPM to manage various functions
+that are provided by the hardware platform it is running on, including power
+and performance functions.
+
+This binding is intended to define the interface the firmware implementing
+the SCMI as described in ARM document number ARM DEN 0056A ("ARM System Control
+and Management Interface Platform Design Document")[0] provide for OSPM in
+the device tree.
+
+Required properties:
+
+The scmi node with the following properties shall be under the /firmware/ node.
+
+- compatible : shall be "arm,scmi" or "arm,scmi-smc" for smc/hvc transports
+- mboxes: List of phandle and mailbox channel specifiers. It should contain
+	  exactly one or two mailboxes, one for transmitting messages("tx")
+	  and another optional for receiving the notifications("rx") if
+	  supported.
+- shmem : List of phandle pointing to the shared memory(SHM) area as per
+	  generic mailbox client binding.
+- #address-cells : should be '1' if the device has sub-nodes, maps to
+	  protocol identifier for a given sub-node.
+- #size-cells : should be '0' as 'reg' property doesn't have any size
+	  associated with it.
+- arm,smc-id : SMC id required when using smc or hvc transports
+
+Optional properties:
+
+- mbox-names: shall be "tx" or "rx" depending on mboxes entries.
+
+See Documentation/devicetree/bindings/mailbox/mailbox.txt for more details
+about the generic mailbox controller and client driver bindings.
+
+The mailbox is the only permitted method of calling the SCMI firmware.
+Mailbox doorbell is used as a mechanism to alert the presence of a
+messages and/or notification.
+
+Each protocol supported shall have a sub-node with corresponding compatible
+as described in the following sections. If the platform supports dedicated
+communication channel for a particular protocol, the 3 properties namely:
+mboxes, mbox-names and shmem shall be present in the sub-node corresponding
+to that protocol.
+
+Clock/Performance bindings for the clocks/OPPs based on SCMI Message Protocol
+------------------------------------------------------------
+
+This binding uses the common clock binding[1].
+
+Required properties:
+- #clock-cells : Should be 1. Contains the Clock ID value used by SCMI commands.
+
+Power domain bindings for the power domains based on SCMI Message Protocol
+------------------------------------------------------------
+
+This binding for the SCMI power domain providers uses the generic power
+domain binding[2].
+
+Required properties:
+ - #power-domain-cells : Should be 1. Contains the device or the power
+			 domain ID value used by SCMI commands.
+
+Sensor bindings for the sensors based on SCMI Message Protocol
+--------------------------------------------------------------
+SCMI provides an API to access the various sensors on the SoC.
+
+Required properties:
+- #thermal-sensor-cells: should be set to 1. This property follows the
+			 thermal device tree bindings[3].
+
+			 Valid cell values are raw identifiers (Sensor ID)
+			 as used by the firmware. Refer to  platform details
+			 for your implementation for the IDs to use.
+
+Reset signal bindings for the reset domains based on SCMI Message Protocol
+------------------------------------------------------------
+
+This binding for the SCMI reset domain providers uses the generic reset
+signal binding[5].
+
+Required properties:
+ - #reset-cells : Should be 1. Contains the reset domain ID value used
+		  by SCMI commands.
+
+SRAM and Shared Memory for SCMI
+-------------------------------
+
+A small area of SRAM is reserved for SCMI communication between application
+processors and SCP.
+
+The properties should follow the generic mmio-sram description found in [4]
+
+Each sub-node represents the reserved area for SCMI.
+
+Required sub-node properties:
+- reg : The base offset and size of the reserved area with the SRAM
+- compatible : should be "arm,scmi-shmem" for Non-secure SRAM based
+	       shared memory
+
+[0] http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/index.html
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+[2] Documentation/devicetree/bindings/power/power-domain.yaml
+[3] Documentation/devicetree/bindings/thermal/thermal.txt
+[4] Documentation/devicetree/bindings/sram/sram.yaml
+[5] Documentation/devicetree/bindings/reset/reset.txt
+
+Example:
+
+sram at 50000000 {
+	compatible = "mmio-sram";
+	reg = <0x0 0x50000000 0x0 0x10000>;
+
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges = <0 0x0 0x50000000 0x10000>;
+
+	cpu_scp_lpri: scp-shmem at 0 {
+		compatible = "arm,scmi-shmem";
+		reg = <0x0 0x200>;
+	};
+
+	cpu_scp_hpri: scp-shmem at 200 {
+		compatible = "arm,scmi-shmem";
+		reg = <0x200 0x200>;
+	};
+};
+
+mailbox at 40000000 {
+	....
+	#mbox-cells = <1>;
+	reg = <0x0 0x40000000 0x0 0x10000>;
+};
+
+firmware {
+
+	...
+
+	scmi {
+		compatible = "arm,scmi";
+		mboxes = <&mailbox 0 &mailbox 1>;
+		mbox-names = "tx", "rx";
+		shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		scmi_devpd: protocol at 11 {
+			reg = <0x11>;
+			#power-domain-cells = <1>;
+		};
+
+		scmi_dvfs: protocol at 13 {
+			reg = <0x13>;
+			#clock-cells = <1>;
+		};
+
+		scmi_clk: protocol at 14 {
+			reg = <0x14>;
+			#clock-cells = <1>;
+		};
+
+		scmi_sensors0: protocol at 15 {
+			reg = <0x15>;
+			#thermal-sensor-cells = <1>;
+		};
+
+		scmi_reset: protocol at 16 {
+			reg = <0x16>;
+			#reset-cells = <1>;
+		};
+	};
+};
+
+cpu at 0 {
+	...
+	reg = <0 0>;
+	clocks = <&scmi_dvfs 0>;
+};
+
+hdlcd at 7ff60000 {
+	...
+	reg = <0 0x7ff60000 0 0x1000>;
+	clocks = <&scmi_clk 4>;
+	power-domains = <&scmi_devpd 1>;
+	resets = <&scmi_reset 10>;
+};
+
+thermal-zones {
+	soc_thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <1000>;
+					/* sensor ID */
+		thermal-sensors = <&scmi_sensors0 3>;
+		...
+	};
+};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 5/8] clk: add clock driver for SCMI agents
  2020-09-07 14:49 [PATCH v3 1/8] firmware: add SCMI agent uclass Etienne Carriere
                   ` (2 preceding siblings ...)
  2020-09-07 14:49 ` [PATCH v3 4/8] dt-bindings: arm: SCMI bindings documentation Etienne Carriere
@ 2020-09-07 14:49 ` Etienne Carriere
  2020-09-08 15:20   ` Simon Glass
  2020-09-07 14:49 ` [PATCH v3 6/8] firmware: scmi: sandbox test for SCMI clocks Etienne Carriere
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Etienne Carriere @ 2020-09-07 14:49 UTC (permalink / raw)
  To: u-boot

This change introduces a clock driver for SCMI agent devices. When
SCMI agent and SCMI clock drivers are enabled, SCMI agent binds a
clock device for each SCMI clock protocol devices enabled in the FDT.

SCMI clock driver is embedded upon CONFIG_CLK_SCMI=y. If enabled,
CONFIG_SCMI_AGENT is also enabled.

SCMI Clock protocol is defined in the SCMI specification [1].

Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Simon Glass <sjg@chromium.org>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
---

Changes in v3:

Changes in v2:
- CONFIG_CLK_SCMI depends on CONFIG_SCMI_FIRMWARE instead of
  selecting CONFIG_SCMI_FIRMWARE.
- Add inline comment description for structures and moves them to
  source file top. Add/fixup some functions inline description comments.
- Replace rc with ret as return value local variable label.
- Fix scmi_clk_get_rate() return value to propagate error number.
- Fix scmi_clk_set_rate() to request synchronous rate set operation:
  drop flag SCMI_CLK_RATE_ASYNC_NORESP in the SCMI message payload.
- Fix scmi_clk_set_rate() return value to return clock effective rate
  on success.
---
 drivers/clk/Kconfig                       |  8 ++
 drivers/clk/Makefile                      |  1 +
 drivers/clk/clk_scmi.c                    | 99 +++++++++++++++++++++++
 drivers/firmware/scmi/scmi_agent-uclass.c |  3 +
 include/scmi_protocols.h                  | 78 ++++++++++++++++++
 5 files changed, 189 insertions(+)
 create mode 100644 drivers/clk/clk_scmi.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 6003e140b5..4dfbad7986 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -159,6 +159,14 @@ config CLK_CDCE9XX
 	   Enable the clock synthesizer driver for CDCE913/925/937/949
 	   series of chips.
 
+config CLK_SCMI
+	bool "Enable SCMI clock driver"
+	depends on SCMI_FIRMWARE
+	help
+	  Enable this option if you want to support clock devices exposed
+	  by a SCMI agent based on SCMI clock protocol communication
+	  with a SCMI server.
+
 source "drivers/clk/analogbits/Kconfig"
 source "drivers/clk/at91/Kconfig"
 source "drivers/clk/exynos/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index cda4b4b605..d1e295ac7c 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_CLK_MPC83XX) += mpc83xx_clk.o
 obj-$(CONFIG_CLK_OCTEON) += clk_octeon.o
 obj-$(CONFIG_CLK_OWL) += owl/
 obj-$(CONFIG_CLK_RENESAS) += renesas/
+obj-$(CONFIG_CLK_SCMI) += clk_scmi.o
 obj-$(CONFIG_CLK_SIFIVE) += sifive/
 obj-$(CONFIG_ARCH_SUNXI) += sunxi/
 obj-$(CONFIG_CLK_STM32F) += clk_stm32f.o
diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
new file mode 100644
index 0000000000..93a4819501
--- /dev/null
+++ b/drivers/clk/clk_scmi.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019-2020 Linaro Limited
+ */
+#include <common.h>
+#include <clk-uclass.h>
+#include <dm.h>
+#include <scmi_agent.h>
+#include <scmi_protocols.h>
+#include <asm/types.h>
+
+static int scmi_clk_gate(struct clk *clk, int enable)
+{
+	struct scmi_clk_state_in in = {
+		.clock_id = clk->id,
+		.attributes = enable,
+	};
+	struct scmi_clk_state_out out;
+	struct scmi_msg msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_CLOCK,
+					  SCMI_CLOCK_CONFIG_SET,
+					  in, out);
+	int ret;
+
+	ret = devm_scmi_process_msg(clk->dev->parent, &msg);
+	if (ret)
+		return ret;
+
+	return scmi_to_linux_errno(out.status);
+}
+
+static int scmi_clk_enable(struct clk *clk)
+{
+	return scmi_clk_gate(clk, 1);
+}
+
+static int scmi_clk_disable(struct clk *clk)
+{
+	return scmi_clk_gate(clk, 0);
+}
+
+static ulong scmi_clk_get_rate(struct clk *clk)
+{
+	struct scmi_clk_rate_get_in in = {
+		.clock_id = clk->id,
+	};
+	struct scmi_clk_rate_get_out out;
+	struct scmi_msg msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_CLOCK,
+					  SCMI_CLOCK_RATE_GET,
+					  in, out);
+	int ret;
+
+	ret = devm_scmi_process_msg(clk->dev->parent, &msg);
+	if (ret < 0)
+		return ret;
+
+	ret = scmi_to_linux_errno(out.status);
+	if (ret < 0)
+		return ret;
+
+	return (ulong)(((u64)out.rate_msb << 32) | out.rate_lsb);
+}
+
+static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
+{
+	struct scmi_clk_rate_set_in in = {
+		.clock_id = clk->id,
+		.flags = SCMI_CLK_RATE_ROUND_CLOSEST,
+		.rate_lsb = (u32)rate,
+		.rate_msb = (u32)((u64)rate >> 32),
+	};
+	struct scmi_clk_rate_set_out out;
+	struct scmi_msg msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_CLOCK,
+					  SCMI_CLOCK_RATE_SET,
+					  in, out);
+	int ret;
+
+	ret = devm_scmi_process_msg(clk->dev->parent, &msg);
+	if (ret < 0)
+		return ret;
+
+	ret = scmi_to_linux_errno(out.status);
+	if (ret < 0)
+		return ret;
+
+	return scmi_clk_get_rate(clk);
+}
+
+static const struct clk_ops scmi_clk_ops = {
+	.enable = scmi_clk_enable,
+	.disable = scmi_clk_disable,
+	.get_rate = scmi_clk_get_rate,
+	.set_rate = scmi_clk_set_rate,
+};
+
+U_BOOT_DRIVER(scmi_clock) = {
+	.name = "scmi_clk",
+	.id = UCLASS_CLK,
+	.ops = &scmi_clk_ops,
+};
diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c
index 67a6f907c9..e1694eff3d 100644
--- a/drivers/firmware/scmi/scmi_agent-uclass.c
+++ b/drivers/firmware/scmi/scmi_agent-uclass.c
@@ -70,6 +70,9 @@ static int scmi_bind_protocols(struct udevice *dev)
 			continue;
 
 		switch (protocol_id) {
+		case SCMI_PROTOCOL_ID_CLOCK:
+			drv = DM_GET_DRIVER(scmi_clock);
+			break;
 		default:
 			dev_info(dev, "Ignore unsupported SCMI protocol %#x\n",
 				 protocol_id);
diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h
index 86a2d109c8..4778bcfc47 100644
--- a/include/scmi_protocols.h
+++ b/include/scmi_protocols.h
@@ -7,6 +7,7 @@
 #define _SCMI_PROTOCOLS_H
 
 #include <linux/bitops.h>
+#include <asm/types.h>
 
 /*
  * Subset the SCMI protocols definition
@@ -38,4 +39,81 @@ enum scmi_status_code {
 	SCMI_PROTOCOL_ERROR = -10,
 };
 
+/*
+ * SCMI Clock Protocol
+ */
+
+enum scmi_clock_message_id {
+	SCMI_CLOCK_RATE_SET = 0x5,
+	SCMI_CLOCK_RATE_GET = 0x6,
+	SCMI_CLOCK_CONFIG_SET = 0x7,
+};
+
+#define SCMI_CLK_RATE_ASYNC_NOTIFY	BIT(0)
+#define SCMI_CLK_RATE_ASYNC_NORESP	(BIT(0) | BIT(1))
+#define SCMI_CLK_RATE_ROUND_DOWN	0
+#define SCMI_CLK_RATE_ROUND_UP		BIT(2)
+#define SCMI_CLK_RATE_ROUND_CLOSEST	BIT(3)
+
+/**
+ * struct scmi_clk_state_in - Message payload for CLOCK_CONFIG_SET command
+ * @clock_id:	SCMI clock ID
+ * @attributes:	Attributes of the targets clock state
+ */
+struct scmi_clk_state_in {
+	u32 clock_id;
+	u32 attributes;
+};
+
+/**
+ * struct scmi_clk_state_out - Response payload for CLOCK_CONFIG_SET command
+ * @status:	SCMI command status
+ */
+struct scmi_clk_state_out {
+	s32 status;
+};
+
+/**
+ * struct scmi_clk_state_in - Message payload for CLOCK_RATE_GET command
+ * @clock_id:	SCMI clock ID
+ * @attributes:	Attributes of the targets clock state
+ */
+struct scmi_clk_rate_get_in {
+	u32 clock_id;
+};
+
+/**
+ * struct scmi_clk_rate_get_out - Response payload for CLOCK_RATE_GET command
+ * @status:	SCMI command status
+ * @rate_lsb:	32bit LSB of the clock rate in Hertz
+ * @rate_msb:	32bit MSB of the clock rate in Hertz
+ */
+struct scmi_clk_rate_get_out {
+	s32 status;
+	u32 rate_lsb;
+	u32 rate_msb;
+};
+
+/**
+ * struct scmi_clk_state_in - Message payload for CLOCK_RATE_SET command
+ * @clock_id:	SCMI clock ID
+ * @flags:	Flags for the clock rate set request
+ * @rate_lsb:	32bit LSB of the clock rate in Hertz
+ * @rate_msb:	32bit MSB of the clock rate in Hertz
+ */
+struct scmi_clk_rate_set_in {
+	u32 clock_id;
+	u32 flags;
+	u32 rate_lsb;
+	u32 rate_msb;
+};
+
+/**
+ * struct scmi_clk_rate_set_out - Response payload for CLOCK_RATE_SET command
+ * @status:	SCMI command status
+ */
+struct scmi_clk_rate_set_out {
+	s32 status;
+};
+
 #endif /* _SCMI_PROTOCOLS_H */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 6/8] firmware: scmi: sandbox test for SCMI clocks
  2020-09-07 14:49 [PATCH v3 1/8] firmware: add SCMI agent uclass Etienne Carriere
                   ` (3 preceding siblings ...)
  2020-09-07 14:49 ` [PATCH v3 5/8] clk: add clock driver for SCMI agents Etienne Carriere
@ 2020-09-07 14:49 ` Etienne Carriere
  2020-09-08 15:20   ` Simon Glass
  2020-09-07 14:49 ` [PATCH v3 7/8] reset: add reset controller driver for SCMI agents Etienne Carriere
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Etienne Carriere @ 2020-09-07 14:49 UTC (permalink / raw)
  To: u-boot

Add tests for SCMI clocks. A test device driver sandbox-scmi_devices.c
is used to get clock resources, allowing further clock manipulation.

Change sandbox-smci_agent to emulate 3 clocks exposed through 2 agents.
Add DM test scmi_clocks to test these 3 clocks.
Update DM test sandbox_scmi_agent with load/remove test sequences
factorized by {load|remove}_sandbox_scmi_test_devices() helper functions.

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:
- New commit in the series, addresses review comments on test support.
  ut_dm_scmi_clocks test SCMI are found and behave as expected for the
  implemented clk uclass methods.
---
 arch/sandbox/dts/test.dts                    |  15 ++
 arch/sandbox/include/asm/scmi_test.h         |  37 ++++
 configs/sandbox_defconfig                    |   1 +
 drivers/firmware/scmi/Makefile               |   2 +-
 drivers/firmware/scmi/sandbox-scmi_agent.c   | 169 ++++++++++++++++++-
 drivers/firmware/scmi/sandbox-scmi_devices.c |  86 ++++++++++
 test/dm/scmi.c                               | 139 ++++++++++++++-
 7 files changed, 438 insertions(+), 11 deletions(-)
 create mode 100644 drivers/firmware/scmi/sandbox-scmi_devices.c

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index dd3b43885e..61acd8d79f 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -361,6 +361,11 @@
 			compatible = "sandbox,scmi-agent";
 			#address-cells = <1>;
 			#size-cells = <0>;
+
+			clk_scmi0: protocol at 14 {
+				reg = <0x14>;
+				#clock-cells = <1>;
+			};
 		};
 
 		sandbox-scmi-agent at 1 {
@@ -368,6 +373,11 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 
+			clk_scmi1: protocol at 14 {
+				reg = <0x14>;
+				#clock-cells = <1>;
+			};
+
 			protocol at 10 {
 				reg = <0x10>;
 			};
@@ -1052,6 +1062,11 @@
 		compatible = "sandbox,virtio2";
 	};
 
+	sandbox_scmi {
+		compatible = "sandbox,scmi-devices";
+		clocks = <&clk_scmi0 7>, <&clk_scmi0 3>, <&clk_scmi1 1>;
+	};
+
 	pinctrl {
 		compatible = "sandbox,pinctrl";
 
diff --git a/arch/sandbox/include/asm/scmi_test.h b/arch/sandbox/include/asm/scmi_test.h
index a811fe19c3..4e09957bc7 100644
--- a/arch/sandbox/include/asm/scmi_test.h
+++ b/arch/sandbox/include/asm/scmi_test.h
@@ -10,12 +10,28 @@ struct udevice;
 struct sandbox_scmi_agent;
 struct sandbox_scmi_service;
 
+/**
+ * struct sandbox_scmi_clk - Simulated clock exposed by SCMI
+ * @id:		Identifier of the clock used in the SCMI protocol
+ * @enabled:	Clock state: true if enabled, false if disabled
+ * @rate:	Clock rate in Hertz
+ */
+struct sandbox_scmi_clk {
+	uint id;
+	bool enabled;
+	ulong rate;
+};
+
 /**
  * struct sandbox_scmi_agent - Simulated SCMI service seen by SCMI agent
  * @idx:	Identifier for the SCMI agent, its index
+ * @clk:	Simulated clocks
+ * @clk_count:	Simulated clocks array size
  */
 struct sandbox_scmi_agent {
 	uint idx;
+	struct sandbox_scmi_clk *clk;
+	size_t clk_count;
 };
 
 /**
@@ -28,16 +44,37 @@ struct sandbox_scmi_service {
 	size_t agent_count;
 };
 
+/**
+ * struct sandbox_scmi_devices - Reference to devices probed through SCMI
+ * @clk:		Array the clock devices
+ * @clk_count:		Number of clock devices probed
+ */
+struct sandbox_scmi_devices {
+	struct clk *clk;
+	size_t clk_count;
+};
+
 #ifdef CONFIG_SCMI_FIRMWARE
 /**
  * sandbox_scmi_service_context - Get the simulated SCMI services context
  * @return:	Reference to backend simulated resources state
  */
 struct sandbox_scmi_service *sandbox_scmi_service_ctx(void);
+
+/**
+ * sandbox_scmi_devices_get_ref - Get references to devices accessed through SCMI
+ * @return:	Reference to the devices probed by the SCMI test
+ */
+struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(void);
 #else
 static inline struct sandbox_scmi_service *sandbox_scmi_service_ctx(void)
 {
 	return NULL;
 }
+
+static inline struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(void)
+{
+	return NULL;
+}
 #endif /* CONFIG_SCMI_FIRMWARE */
 #endif /* __SANDBOX_SCMI_TEST_H */
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 2c130c01f0..7d71c805dc 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -122,6 +122,7 @@ CONFIG_BUTTON=y
 CONFIG_BUTTON_GPIO=y
 CONFIG_CLK=y
 CONFIG_CLK_COMPOSITE_CCF=y
+CONFIG_CLK_SCMI=y
 CONFIG_SANDBOX_CLK_CCF=y
 CONFIG_CPU=y
 CONFIG_DM_DEMO=y
diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
index 2f782bbd55..e1e0224066 100644
--- a/drivers/firmware/scmi/Makefile
+++ b/drivers/firmware/scmi/Makefile
@@ -2,4 +2,4 @@ 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
+obj-$(CONFIG_SANDBOX)		+= sandbox-scmi_agent.o sandbox-scmi_devices.o
diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
index ce0d49c951..262efbd12c 100644
--- a/drivers/firmware/scmi/sandbox-scmi_agent.c
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -16,18 +16,34 @@
 /*
  * The sandbox SCMI agent driver simulates to some extend a SCMI message
  * processing. It simulates few of the SCMI services for some of the
- * SCMI protocols embedded in U-Boot. Currently none.
+ * SCMI protocols embedded in U-Boot. Currently:
+ * - SCMI clock protocol: emulate 2 agents each exposing few clocks
  *
- * This driver simulates 2 SCMI agents for test purpose.
+ * Agent #0 simulates 2 clocks.
+ * See IDs in scmi0_clk[] and "sandbox-scmi-agent at 0" in test.dts.
+ *
+ * Agent #1 simulates 1 clock.
+ * See IDs in scmi1_clk[] and "sandbox-scmi-agent at 1" in test.dts.
+ *
+ * All clocks are default disabled.
  *
  * This Driver exports sandbox_scmi_service_ct() for the test sequence to
  * get the state of the simulated services (clock state, rate, ...) and
  * check back-end device state reflects the request send through the
- * various uclass devices, currently nothing.
+ * various uclass devices, currently only clock controllers.
  */
 
 #define SANDBOX_SCMI_AGENT_COUNT	2
 
+static struct sandbox_scmi_clk scmi0_clk[] = {
+	{ .id = 7, .rate = 1000 },
+	{ .id = 3, .rate = 333 },
+};
+
+static struct sandbox_scmi_clk scmi1_clk[] = {
+	{ .id = 1, .rate = 44 },
+};
+
 /* The list saves to simulted end devices references for test purpose */
 struct sandbox_scmi_agent *sandbox_scmi_agent_list[SANDBOX_SCMI_AGENT_COUNT];
 
@@ -51,17 +67,158 @@ static void debug_print_agent_state(struct udevice *dev, char *str)
 	struct sandbox_scmi_agent *agent = dev2agent(dev);
 
 	dev_dbg(dev, "Dump sandbox_scmi_agent %u: %s\n", agent->idx, str);
+	dev_dbg(dev, " scmi%u_clk   (%zu): %d/%ld, %d/%ld, %d/%ld, ...\n",
+		agent->idx,
+		agent->clk_count,
+		agent->clk_count ? agent->clk[0].enabled : -1,
+		agent->clk_count ? agent->clk[0].rate : -1,
+		agent->clk_count > 1 ? agent->clk[1].enabled : -1,
+		agent->clk_count > 1 ? agent->clk[1].rate : -1,
+		agent->clk_count > 2 ? agent->clk[2].enabled : -1,
+		agent->clk_count > 2 ? agent->clk[2].rate : -1);
 };
 
+static struct sandbox_scmi_clk *get_scmi_clk_state(uint agent_id, uint clock_id)
+{
+	struct sandbox_scmi_clk *target = NULL;
+	size_t target_count = 0;
+	size_t n;
+
+	switch (agent_id) {
+	case 0:
+		target = scmi0_clk;
+		target_count = ARRAY_SIZE(scmi0_clk);
+		break;
+	case 1:
+		target = scmi1_clk;
+		target_count = ARRAY_SIZE(scmi1_clk);
+		break;
+	default:
+		return NULL;
+	}
+
+	for (n = 0; n < target_count; n++)
+		if (target[n].id == clock_id)
+			return target + n;
+
+	return NULL;
+}
+
+/*
+ * Sandbox SCMI agent ops
+ */
+
+static int sandbox_scmi_clock_rate_set(struct udevice *dev,
+				       struct scmi_msg *msg)
+{
+	struct sandbox_scmi_agent *agent = dev2agent(dev);
+	struct scmi_clk_rate_set_in *in = NULL;
+	struct scmi_clk_rate_set_out *out = NULL;
+	struct sandbox_scmi_clk *clk_state = NULL;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	in = (struct scmi_clk_rate_set_in *)msg->in_msg;
+	out = (struct scmi_clk_rate_set_out *)msg->out_msg;
+
+	clk_state = get_scmi_clk_state(agent->idx, in->clock_id);
+	if (!clk_state) {
+		dev_err(dev, "Unexpected clock ID %u\n", in->clock_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else {
+		u64 rate = ((u64)in->rate_msb << 32) + in->rate_lsb;
+
+		clk_state->rate = (ulong)rate;
+
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
+static int sandbox_scmi_clock_rate_get(struct udevice *dev,
+				       struct scmi_msg *msg)
+{
+	struct sandbox_scmi_agent *agent = dev2agent(dev);
+	struct scmi_clk_rate_get_in *in = NULL;
+	struct scmi_clk_rate_get_out *out = NULL;
+	struct sandbox_scmi_clk *clk_state = NULL;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	in = (struct scmi_clk_rate_get_in *)msg->in_msg;
+	out = (struct scmi_clk_rate_get_out *)msg->out_msg;
+
+	clk_state = get_scmi_clk_state(agent->idx, in->clock_id);
+	if (!clk_state) {
+		dev_err(dev, "Unexpected clock ID %u\n", in->clock_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else {
+		out->rate_msb = (u32)((u64)clk_state->rate >> 32);
+		out->rate_lsb = (u32)clk_state->rate;
+
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
+static int sandbox_scmi_clock_gate(struct udevice *dev, struct scmi_msg *msg)
+{
+	struct sandbox_scmi_agent *agent = dev2agent(dev);
+	struct scmi_clk_state_in *in = NULL;
+	struct scmi_clk_state_out *out = NULL;
+	struct sandbox_scmi_clk *clk_state = NULL;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	in = (struct scmi_clk_state_in *)msg->in_msg;
+	out = (struct scmi_clk_state_out *)msg->out_msg;
+
+	clk_state = get_scmi_clk_state(agent->idx, in->clock_id);
+	if (!clk_state) {
+		dev_err(dev, "Unexpected clock ID %u\n", in->clock_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else if (in->attributes > 1) {
+		out->status = SCMI_PROTOCOL_ERROR;
+	} else {
+		clk_state->enabled = in->attributes;
+
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
 static int sandbox_scmi_test_process_msg(struct udevice *dev,
 					 struct scmi_msg *msg)
 {
 	switch (msg->protocol_id) {
+	case SCMI_PROTOCOL_ID_CLOCK:
+		switch (msg->message_id) {
+		case SCMI_CLOCK_RATE_SET:
+			return sandbox_scmi_clock_rate_set(dev, msg);
+		case SCMI_CLOCK_RATE_GET:
+			return sandbox_scmi_clock_rate_get(dev, msg);
+		case SCMI_CLOCK_CONFIG_SET:
+			return sandbox_scmi_clock_gate(dev, msg);
+		default:
+			break;
+		}
+		break;
 	case SCMI_PROTOCOL_ID_BASE:
 	case SCMI_PROTOCOL_ID_POWER_DOMAIN:
 	case SCMI_PROTOCOL_ID_SYSTEM:
 	case SCMI_PROTOCOL_ID_PERF:
-	case SCMI_PROTOCOL_ID_CLOCK:
 	case SCMI_PROTOCOL_ID_SENSOR:
 	case SCMI_PROTOCOL_ID_RESET_DOMAIN:
 		*(u32 *)msg->out_msg = SCMI_NOT_SUPPORTED;
@@ -106,11 +263,15 @@ static int sandbox_scmi_test_probe(struct udevice *dev)
 	case '0':
 		*agent = (struct sandbox_scmi_agent){
 			.idx = 0,
+			.clk = scmi0_clk,
+			.clk_count = ARRAY_SIZE(scmi0_clk),
 		};
 		break;
 	case '1':
 		*agent = (struct sandbox_scmi_agent){
 			.idx = 1,
+			.clk = scmi1_clk,
+			.clk_count = ARRAY_SIZE(scmi1_clk),
 		};
 		break;
 	default:
diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c
new file mode 100644
index 0000000000..2ce8e664df
--- /dev/null
+++ b/drivers/firmware/scmi/sandbox-scmi_devices.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020, Linaro Limited
+ */
+
+#include <common.h>
+#include <clk.h>
+#include <dm.h>
+#include <malloc.h>
+#include <asm/io.h>
+#include <asm/scmi_test.h>
+#include <dm/device_compat.h>
+
+/*
+ * Simulate to some extend a SCMI exhange.
+ * This drivers gets SCMI resources and offers API function to the
+ * SCMI test sequence manipulate the resources.
+ */
+
+#define SCMI_TEST_DEVICES_CLK_COUNT		3
+
+/*
+ * These tables store de handles used in the various uclasses device function
+ * that are instancied when probed through the SCMI agent. Use a static
+ * memory allocation to ease sharing with test sequence implementation.
+ */
+static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT];
+static struct sandbox_scmi_devices sandbox_scmi_devhld;
+
+struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(void)
+{
+	return &sandbox_scmi_devhld;
+}
+
+static void dereference_device_handles(struct sandbox_scmi_devices *devices)
+{
+	devices->clk = NULL;
+	devices->clk_count = 0;
+}
+
+static int sandbox_scmi_devices_remove(struct udevice *dev)
+{
+	struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();
+
+	dereference_device_handles(devices);
+
+	return 0;
+}
+
+static int sandbox_scmi_devices_probe(struct udevice *dev)
+{
+	struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();
+	int rc;
+	size_t n;
+
+	devices->clk = sandbox_scmi_clk_device;
+	devices->clk_count = SCMI_TEST_DEVICES_CLK_COUNT;
+
+	for (n = 0; n < SCMI_TEST_DEVICES_CLK_COUNT; n++) {
+		rc = clk_get_by_index(dev, n, devices->clk + n);
+		if (rc) {
+			dev_err(dev, "%s: Failed on clk %zu\n", __func__, n);
+			goto err_clk;
+		}
+	}
+
+	return 0;
+
+err_clk:
+	dereference_device_handles(devices);
+
+	return rc;
+}
+
+static const struct udevice_id sandbox_scmi_devices_ids[] = {
+	{ .compatible = "sandbox,scmi-devices" },
+	{ }
+};
+
+U_BOOT_DRIVER(sandbox_scmi_devices) = {
+	.name = "sandbox-scmi_devices",
+	.id = UCLASS_MISC,
+	.of_match = sandbox_scmi_devices_ids,
+	.remove = sandbox_scmi_devices_remove,
+	.probe = sandbox_scmi_devices_probe,
+};
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index d8c1e71f12..e652dbf52b 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -13,26 +13,153 @@
  */
 
 #include <common.h>
+#include <clk.h>
 #include <dm.h>
 #include <asm/scmi_test.h>
 #include <dm/device-internal.h>
 #include <dm/test.h>
+#include <linux/kconfig.h>
 #include <test/ut.h>
 
+static int ut_assert_scmi_state_preprobe(struct unit_test_state *uts)
+{
+	struct sandbox_scmi_devices *scmi_devices = sandbox_scmi_devices_ctx();
+	struct sandbox_scmi_service *scmi_ctx = sandbox_scmi_service_ctx();
+
+	ut_assertnonnull(scmi_ctx);
+
+	if (scmi_ctx->agent_count)
+		ut_asserteq(2, scmi_ctx->agent_count);
+
+	ut_assertnonnull(scmi_devices);
+
+	if (scmi_devices->clk_count)
+		ut_asserteq(3, scmi_devices->clk_count);
+
+	return 0;
+}
+
+static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts)
+{
+	struct sandbox_scmi_devices *scmi_devices = sandbox_scmi_devices_ctx();
+	struct sandbox_scmi_service *scmi_ctx = sandbox_scmi_service_ctx();
+
+	/* Device references to check context against test sequence */
+	ut_assertnonnull(scmi_devices);
+	if (IS_ENABLED(CONFIG_CLK_SCMI))
+		ut_asserteq(3, scmi_devices->clk_count);
+
+	/* State of the simulated SCMI server exposed */
+	ut_asserteq(2, scmi_ctx->agent_count);
+
+	ut_assertnonnull(scmi_ctx->agent[0]);
+	ut_asserteq(2, scmi_ctx->agent[0]->clk_count);
+	ut_assertnonnull(scmi_ctx->agent[0]->clk);
+
+	ut_assertnonnull(scmi_ctx->agent[1]);
+	ut_assertnonnull(scmi_ctx->agent[1]->clk);
+	ut_asserteq(1, scmi_ctx->agent[1]->clk_count);
+
+	return 0;
+}
+
+static int load_sandbox_scmi_test_devices(struct unit_test_state *uts,
+					  struct udevice **dev)
+{
+	int rc;
+
+	rc = ut_assert_scmi_state_preprobe(uts);
+	if (rc)
+		return rc;
+
+	ut_assertok(uclass_get_device_by_name(UCLASS_MISC, "sandbox_scmi",
+					      dev));
+
+	return ut_assert_scmi_state_postprobe(uts);
+}
+
+static int release_sandbox_scmi_test_devices(struct unit_test_state *uts,
+					     struct udevice *dev)
+{
+	ut_assertok(device_remove(dev, DM_REMOVE_NORMAL));
+
+	/* Not sure test devices are fully removed, agent may not be visible */
+	return 0;
+}
+
 /*
  * Test SCMI states when loading and releasing resources
  * related to SCMI drivers.
  */
 static int dm_test_scmi_sandbox_agent(struct unit_test_state *uts)
 {
-	struct sandbox_scmi_service *scmi_ctx = sandbox_scmi_service_ctx();
+	struct udevice *dev = NULL;
+	int rc;
 
-	ut_assertnonnull(scmi_ctx);
-	ut_asserteq(2, scmi_ctx->agent_count);
-	ut_assertnull(scmi_ctx->agent[0]);
-	ut_assertnull(scmi_ctx->agent[1]);
+	rc = load_sandbox_scmi_test_devices(uts, &dev);
+	if (!rc)
+		rc = release_sandbox_scmi_test_devices(uts, dev);
 
-	return 0;
+	return rc;
 }
 
 DM_TEST(dm_test_scmi_sandbox_agent, UT_TESTF_SCAN_FDT);
+
+static int dm_test_scmi_clocks(struct unit_test_state *uts)
+{
+	struct sandbox_scmi_devices *scmi_devices = sandbox_scmi_devices_ctx();
+	struct sandbox_scmi_service *scmi_ctx = sandbox_scmi_service_ctx();
+	struct udevice *dev = NULL;
+	int rc_dev;
+	int rc;
+
+	if (!IS_ENABLED(CONFIG_CLK_SCMI))
+		return 0;
+
+	rc = load_sandbox_scmi_test_devices(uts, &dev);
+	if (rc)
+		return rc;
+
+	/* Test SCMI clocks rate manipulation */
+	ut_asserteq(1000, clk_get_rate(&scmi_devices->clk[0]));
+	ut_asserteq(333, clk_get_rate(&scmi_devices->clk[1]));
+	ut_asserteq(44, clk_get_rate(&scmi_devices->clk[2]));
+
+	rc_dev = clk_set_rate(&scmi_devices->clk[1], 1088);
+	ut_assert(!rc_dev || rc_dev == 1088);
+
+	ut_asserteq(1000, scmi_ctx->agent[0]->clk[0].rate);
+	ut_asserteq(1088, scmi_ctx->agent[0]->clk[1].rate);
+	ut_asserteq(44, scmi_ctx->agent[1]->clk[0].rate);
+
+	ut_asserteq(1000, clk_get_rate(&scmi_devices->clk[0]));
+	ut_asserteq(1088, clk_get_rate(&scmi_devices->clk[1]));
+	ut_asserteq(44, clk_get_rate(&scmi_devices->clk[2]));
+
+	/* restore original rate for further tests */
+	rc_dev = clk_set_rate(&scmi_devices->clk[1], 333);
+	ut_assert(!rc_dev || rc_dev == 333);
+
+	/* Test SCMI clocks gating manipulation */
+	ut_assert(!scmi_ctx->agent[0]->clk[0].enabled);
+	ut_assert(!scmi_ctx->agent[0]->clk[1].enabled);
+	ut_assert(!scmi_ctx->agent[1]->clk[0].enabled);
+
+	ut_asserteq(0, clk_enable(&scmi_devices->clk[1]));
+	ut_asserteq(0, clk_enable(&scmi_devices->clk[2]));
+
+	ut_assert(!scmi_ctx->agent[0]->clk[0].enabled);
+	ut_assert(scmi_ctx->agent[0]->clk[1].enabled);
+	ut_assert(scmi_ctx->agent[1]->clk[0].enabled);
+
+	ut_assertok(clk_disable(&scmi_devices->clk[1]));
+	ut_assertok(clk_disable(&scmi_devices->clk[2]));
+
+	ut_assert(!scmi_ctx->agent[0]->clk[0].enabled);
+	ut_assert(!scmi_ctx->agent[0]->clk[1].enabled);
+	ut_assert(!scmi_ctx->agent[1]->clk[0].enabled);
+
+	return release_sandbox_scmi_test_devices(uts, dev);
+}
+
+DM_TEST(dm_test_scmi_clocks, UT_TESTF_SCAN_FDT);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 7/8] reset: add reset controller driver for SCMI agents
  2020-09-07 14:49 [PATCH v3 1/8] firmware: add SCMI agent uclass Etienne Carriere
                   ` (4 preceding siblings ...)
  2020-09-07 14:49 ` [PATCH v3 6/8] firmware: scmi: sandbox test for SCMI clocks Etienne Carriere
@ 2020-09-07 14:49 ` Etienne Carriere
  2020-09-08 15:20   ` Simon Glass
  2020-09-07 14:50 ` [PATCH v3 8/8] firmware: smci: sandbox test for SCMI reset controllers Etienne Carriere
  2020-09-08 15:20 ` [PATCH v3 1/8] firmware: add SCMI agent uclass Simon Glass
  7 siblings, 1 reply; 21+ messages in thread
From: Etienne Carriere @ 2020-09-07 14:49 UTC (permalink / raw)
  To: u-boot

This change introduces a reset controller driver for SCMI agent devices.
When SCMI agent and SCMI reset domain drivers are enabled, SCMI agent
binds a reset controller device for each SCMI reset domain protocol
devices enabled in the FDT.

SCMI reset driver is embedded upon CONFIG_RESET_SCMI=y. If enabled,
CONFIG_SCMI_AGENT is also enabled.

SCMI Reset Domain protocol is defined in the SCMI specification [1].

Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
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:
- Upgrade to rename into devm_scmi_process_msg() and scmi.h split
  into scmi_*.h.
- Fix message ID used in scmi_reset_request().

Changes in v2:
- Change reset request() method to at least check the reset domain
  exists by sending a SCMI RESET_DOMAIN_ATTRIBUTE message.
- Add inline description for the several structures.
- Patch v1 R-b tag not applied since the above changes in this v2.

BACKPORTED FROM v2020.10-rc2 to V2020.04
---
 drivers/firmware/scmi/scmi_agent-uclass.c |  3 +
 drivers/reset/Kconfig                     |  8 +++
 drivers/reset/Makefile                    |  1 +
 drivers/reset/reset-scmi.c                | 81 +++++++++++++++++++++++
 include/scmi_protocols.h                  | 60 +++++++++++++++++
 5 files changed, 153 insertions(+)
 create mode 100644 drivers/reset/reset-scmi.c

diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c
index e1694eff3d..54f798318f 100644
--- a/drivers/firmware/scmi/scmi_agent-uclass.c
+++ b/drivers/firmware/scmi/scmi_agent-uclass.c
@@ -73,6 +73,9 @@ static int scmi_bind_protocols(struct udevice *dev)
 		case SCMI_PROTOCOL_ID_CLOCK:
 			drv = DM_GET_DRIVER(scmi_clock);
 			break;
+		case SCMI_PROTOCOL_ID_RESET_DOMAIN:
+			drv = DM_GET_DRIVER(scmi_reset_domain);
+			break;
 		default:
 			dev_info(dev, "Ignore unsupported SCMI protocol %#x\n",
 				 protocol_id);
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 253902ff57..ee5be0bc2f 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -173,4 +173,12 @@ config RESET_RASPBERRYPI
 	  relevant. This driver provides a reset controller capable of
 	  interfacing with RPi4's co-processor and model these firmware
 	  initialization routines as reset lines.
+
+config RESET_SCMI
+	bool "Enable SCMI reset domain driver"
+	select SCMI_FIRMWARE
+	help
+	  Enable this option if you want to support reset controller
+	  devices exposed by a SCMI agent based on SCMI reset domain
+	  protocol communication with a SCMI server.
 endmenu
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 3c7f066ae3..625ec7168e 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -26,3 +26,4 @@ obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
 obj-$(CONFIG_RESET_SIFIVE) += reset-sifive.o
 obj-$(CONFIG_RESET_SYSCON) += reset-syscon.o
 obj-$(CONFIG_RESET_RASPBERRYPI) += reset-raspberrypi.o
+obj-$(CONFIG_RESET_SCMI) += reset-scmi.o
diff --git a/drivers/reset/reset-scmi.c b/drivers/reset/reset-scmi.c
new file mode 100644
index 0000000000..1bff8075ee
--- /dev/null
+++ b/drivers/reset/reset-scmi.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019-2020 Linaro Limited
+ */
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <reset-uclass.h>
+#include <scmi_agent.h>
+#include <scmi_protocols.h>
+#include <asm/types.h>
+
+static int scmi_reset_set_level(struct reset_ctl *rst, bool assert_not_deassert)
+{
+	struct scmi_rd_reset_in in = {
+		.domain_id = rst->id,
+		.flags = assert_not_deassert ? SCMI_RD_RESET_FLAG_ASSERT : 0,
+		.reset_state = 0,
+	};
+	struct scmi_rd_reset_out out;
+	struct scmi_msg msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_RESET_DOMAIN,
+					  SCMI_RESET_DOMAIN_RESET,
+					  in, out);
+	int ret;
+
+	ret = devm_scmi_process_msg(rst->dev->parent, &msg);
+	if (ret)
+		return ret;
+
+	return scmi_to_linux_errno(out.status);
+}
+
+static int scmi_reset_assert(struct reset_ctl *rst)
+{
+	return scmi_reset_set_level(rst, true);
+}
+
+static int scmi_reset_deassert(struct reset_ctl *rst)
+{
+	return scmi_reset_set_level(rst, false);
+}
+
+static int scmi_reset_request(struct reset_ctl *rst)
+{
+	struct scmi_rd_attr_in in = {
+		.domain_id = rst->id,
+	};
+	struct scmi_rd_attr_out out;
+	struct scmi_msg msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_RESET_DOMAIN,
+					  SCMI_RESET_DOMAIN_ATTRIBUTES,
+					  in, out);
+	int ret;
+
+	/*
+	 * We don't really care about the attribute, just check
+	 * the reset domain exists.
+	 */
+	ret = devm_scmi_process_msg(rst->dev->parent, &msg);
+	if (ret)
+		return ret;
+
+	return scmi_to_linux_errno(out.status);
+}
+
+static int scmi_reset_rfree(struct reset_ctl *rst)
+{
+	return 0;
+}
+
+static const struct reset_ops scmi_reset_domain_ops = {
+	.request	= scmi_reset_request,
+	.rfree		= scmi_reset_rfree,
+	.rst_assert	= scmi_reset_assert,
+	.rst_deassert	= scmi_reset_deassert,
+};
+
+U_BOOT_DRIVER(scmi_reset_domain) = {
+	.name = "scmi_reset_domain",
+	.id = UCLASS_RESET,
+	.ops = &scmi_reset_domain_ops,
+};
diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h
index 4778bcfc47..ccab97c96c 100644
--- a/include/scmi_protocols.h
+++ b/include/scmi_protocols.h
@@ -116,4 +116,64 @@ struct scmi_clk_rate_set_out {
 	s32 status;
 };
 
+/*
+ * SCMI Reset Domain Protocol
+ */
+
+enum scmi_reset_domain_message_id {
+	SCMI_RESET_DOMAIN_ATTRIBUTES = 0x3,
+	SCMI_RESET_DOMAIN_RESET = 0x4,
+};
+
+#define SCMI_RD_NAME_LEN		16
+
+#define SCMI_RD_ATTRIBUTES_FLAG_ASYNC	BIT(31)
+#define SCMI_RD_ATTRIBUTES_FLAG_NOTIF	BIT(30)
+
+#define SCMI_RD_RESET_FLAG_ASYNC	BIT(2)
+#define SCMI_RD_RESET_FLAG_ASSERT	BIT(1)
+#define SCMI_RD_RESET_FLAG_CYCLE	BIT(0)
+
+/**
+ * struct scmi_rd_attr_in - Payload for RESET_DOMAIN_ATTRIBUTES message
+ * @domain_id:	SCMI reset domain ID
+ */
+struct scmi_rd_attr_in {
+	u32 domain_id;
+};
+
+/**
+ * struct scmi_rd_attr_out - Payload for RESET_DOMAIN_ATTRIBUTES response
+ * @status:	SCMI command status
+ * @attributes:	Retrieved attributes of the reset domain
+ * @latency:	Reset cycle max lantency
+ * @name:	Reset domain name
+ */
+struct scmi_rd_attr_out {
+	s32 status;
+	u32 attributes;
+	u32 latency;
+	char name[SCMI_RD_NAME_LEN];
+};
+
+/**
+ * struct scmi_rd_reset_in - Message payload for RESET command
+ * @domain_id:		SCMI reset domain ID
+ * @flags:		Flags for the reset request
+ * @reset_state:	Reset target state
+ */
+struct scmi_rd_reset_in {
+	u32 domain_id;
+	u32 flags;
+	u32 reset_state;
+};
+
+/**
+ * struct scmi_rd_reset_out - Response payload for RESET command
+ * @status:	SCMI command status
+ */
+struct scmi_rd_reset_out {
+	s32 status;
+};
+
 #endif /* _SCMI_PROTOCOLS_H */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 8/8] firmware: smci: sandbox test for SCMI reset controllers
  2020-09-07 14:49 [PATCH v3 1/8] firmware: add SCMI agent uclass Etienne Carriere
                   ` (5 preceding siblings ...)
  2020-09-07 14:49 ` [PATCH v3 7/8] reset: add reset controller driver for SCMI agents Etienne Carriere
@ 2020-09-07 14:50 ` Etienne Carriere
  2020-09-08 15:21   ` Simon Glass
  2020-09-08 15:20 ` [PATCH v3 1/8] firmware: add SCMI agent uclass Simon Glass
  7 siblings, 1 reply; 21+ messages in thread
From: Etienne Carriere @ 2020-09-07 14:50 UTC (permalink / raw)
  To: u-boot

Add tests for SCMI reset controllers. A test device driver
sandbox-scmi_devices.c is used to get reset resources, allowing further
resets manipulation.

Change sandbox-smci_agent to emulate 1 reset controller exposed through
an agent. Add DM test scmi_resets to test this reset controller.

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:
- New commit in the series, addresses review comments on test support.
  ut_dm_scmi_resets() tests SCMI resources are found and behave as
  expected for the implemented reset uclass methods.
---
 arch/sandbox/dts/test.dts                    |   6 +
 arch/sandbox/include/asm/scmi_test.h         |  17 +++
 configs/sandbox_defconfig                    |   1 +
 drivers/firmware/scmi/sandbox-scmi_agent.c   | 117 ++++++++++++++++++-
 drivers/firmware/scmi/sandbox-scmi_devices.c |  30 ++++-
 test/dm/scmi.c                               |  35 ++++++
 6 files changed, 200 insertions(+), 6 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 61acd8d79f..7023f33a67 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -366,6 +366,11 @@
 				reg = <0x14>;
 				#clock-cells = <1>;
 			};
+
+			reset_scmi0: protocol at 16 {
+				reg = <0x16>;
+				#reset-cells = <1>;
+			};
 		};
 
 		sandbox-scmi-agent at 1 {
@@ -1065,6 +1070,7 @@
 	sandbox_scmi {
 		compatible = "sandbox,scmi-devices";
 		clocks = <&clk_scmi0 7>, <&clk_scmi0 3>, <&clk_scmi1 1>;
+		resets = <&reset_scmi0 3>;
 	};
 
 	pinctrl {
diff --git a/arch/sandbox/include/asm/scmi_test.h b/arch/sandbox/include/asm/scmi_test.h
index 4e09957bc7..de3b2bbcc2 100644
--- a/arch/sandbox/include/asm/scmi_test.h
+++ b/arch/sandbox/include/asm/scmi_test.h
@@ -22,16 +22,29 @@ struct sandbox_scmi_clk {
 	ulong rate;
 };
 
+/**
+ * struct sandbox_scmi_reset - Simulated reset controller exposed by SCMI
+ * @asserted:	Reset control state: true if asserted, false if desasserted
+ */
+struct sandbox_scmi_reset {
+	uint id;
+	bool asserted;
+};
+
 /**
  * struct sandbox_scmi_agent - Simulated SCMI service seen by SCMI agent
  * @idx:	Identifier for the SCMI agent, its index
  * @clk:	Simulated clocks
  * @clk_count:	Simulated clocks array size
+ * @clk:	Simulated reset domains
+ * @clk_count:	Simulated reset domains array size
  */
 struct sandbox_scmi_agent {
 	uint idx;
 	struct sandbox_scmi_clk *clk;
 	size_t clk_count;
+	struct sandbox_scmi_reset *reset;
+	size_t reset_count;
 };
 
 /**
@@ -48,10 +61,14 @@ struct sandbox_scmi_service {
  * struct sandbox_scmi_devices - Reference to devices probed through SCMI
  * @clk:		Array the clock devices
  * @clk_count:		Number of clock devices probed
+ * @reset:		Array the reset controller devices
+ * @reset_count:	Number of reset controller devices probed
  */
 struct sandbox_scmi_devices {
 	struct clk *clk;
 	size_t clk_count;
+	struct reset_ctl *reset;
+	size_t reset_count;
 };
 
 #ifdef CONFIG_SCMI_FIRMWARE
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 7d71c805dc..a2ebb3c971 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -220,6 +220,7 @@ CONFIG_REMOTEPROC_SANDBOX=y
 CONFIG_DM_RESET=y
 CONFIG_SANDBOX_RESET=y
 CONFIG_RESET_SYSCON=y
+CONFIG_RESET_SCMI=y
 CONFIG_DM_RNG=y
 CONFIG_DM_RTC=y
 CONFIG_RTC_RV8803=y
diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
index 262efbd12c..01843a9827 100644
--- a/drivers/firmware/scmi/sandbox-scmi_agent.c
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -18,19 +18,20 @@
  * processing. It simulates few of the SCMI services for some of the
  * SCMI protocols embedded in U-Boot. Currently:
  * - SCMI clock protocol: emulate 2 agents each exposing few clocks
+ * - SCMI reset protocol: emulate 1 agents each exposing a reset
  *
- * Agent #0 simulates 2 clocks.
- * See IDs in scmi0_clk[] and "sandbox-scmi-agent at 0" in test.dts.
+ * Agent #0 simulates 2 clocks and 1 reset domain.
+ * See IDs in scmi0_clk[]/scmi0_reset[] and "sandbox-scmi-agent at 0" in test.dts.
  *
  * Agent #1 simulates 1 clock.
  * See IDs in scmi1_clk[] and "sandbox-scmi-agent at 1" in test.dts.
  *
- * All clocks are default disabled.
+ * All clocks are default disabled and reset levels down.
  *
  * This Driver exports sandbox_scmi_service_ct() for the test sequence to
  * get the state of the simulated services (clock state, rate, ...) and
  * check back-end device state reflects the request send through the
- * various uclass devices, currently only clock controllers.
+ * various uclass devices, as clocks and reset controllers.
  */
 
 #define SANDBOX_SCMI_AGENT_COUNT	2
@@ -40,6 +41,10 @@ static struct sandbox_scmi_clk scmi0_clk[] = {
 	{ .id = 3, .rate = 333 },
 };
 
+static struct sandbox_scmi_reset scmi0_reset[] = {
+	{ .id = 3 },
+};
+
 static struct sandbox_scmi_clk scmi1_clk[] = {
 	{ .id = 1, .rate = 44 },
 };
@@ -76,6 +81,11 @@ static void debug_print_agent_state(struct udevice *dev, char *str)
 		agent->clk_count > 1 ? agent->clk[1].rate : -1,
 		agent->clk_count > 2 ? agent->clk[2].enabled : -1,
 		agent->clk_count > 2 ? agent->clk[2].rate : -1);
+	dev_dbg(dev, " scmi%u_reset (%zu): %d, %d, ...\n",
+		agent->idx,
+		agent->reset_count,
+		agent->reset_count ? agent->reset[0].asserted : -1,
+		agent->reset_count > 1 ? agent->reset[1].asserted : -1);
 };
 
 static struct sandbox_scmi_clk *get_scmi_clk_state(uint agent_id, uint clock_id)
@@ -104,6 +114,20 @@ static struct sandbox_scmi_clk *get_scmi_clk_state(uint agent_id, uint clock_id)
 	return NULL;
 }
 
+static struct sandbox_scmi_reset *get_scmi_reset_state(uint agent_id,
+						       uint reset_id)
+{
+	size_t n;
+
+	if (agent_id == 0) {
+		for (n = 0; n < ARRAY_SIZE(scmi0_reset); n++)
+			if (scmi0_reset[n].id == reset_id)
+				return scmi0_reset + n;
+	}
+
+	return NULL;
+}
+
 /*
  * Sandbox SCMI agent ops
  */
@@ -199,6 +223,78 @@ static int sandbox_scmi_clock_gate(struct udevice *dev, struct scmi_msg *msg)
 	return 0;
 }
 
+static int sandbox_scmi_rd_attribs(struct udevice *dev, struct scmi_msg *msg)
+{
+	struct sandbox_scmi_agent *agent = dev2agent(dev);
+	struct scmi_rd_attr_in *in = NULL;
+	struct scmi_rd_attr_out *out = NULL;
+	struct sandbox_scmi_reset *reset_state = NULL;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	in = (struct scmi_rd_attr_in *)msg->in_msg;
+	out = (struct scmi_rd_attr_out *)msg->out_msg;
+
+	reset_state = get_scmi_reset_state(agent->idx, in->domain_id);
+	if (!reset_state) {
+		dev_err(dev, "Unexpected reset domain ID %u\n", in->domain_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else {
+		memset(out, 0, sizeof(*out));
+		snprintf(out->name, sizeof(out->name), "rd%u", in->domain_id);
+
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
+static int sandbox_scmi_rd_reset(struct udevice *dev, struct scmi_msg *msg)
+{
+	struct sandbox_scmi_agent *agent = dev2agent(dev);
+	struct scmi_rd_reset_in *in = NULL;
+	struct scmi_rd_reset_out *out = NULL;
+	struct sandbox_scmi_reset *reset_state = NULL;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	in = (struct scmi_rd_reset_in *)msg->in_msg;
+	out = (struct scmi_rd_reset_out *)msg->out_msg;
+
+	reset_state = get_scmi_reset_state(agent->idx, in->domain_id);
+	if (!reset_state) {
+		dev_err(dev, "Unexpected reset domain ID %u\n", in->domain_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else if (in->reset_state > 1) {
+		dev_err(dev, "Invalid reset domain input attribute value\n");
+
+		out->status = SCMI_INVALID_PARAMETERS;
+	} else {
+		if (in->flags & SCMI_RD_RESET_FLAG_CYCLE) {
+			if (in->flags & SCMI_RD_RESET_FLAG_ASYNC) {
+				out->status = SCMI_NOT_SUPPORTED;
+			} else {
+				/* Ends deasserted whatever current state */
+				reset_state->asserted = false;
+				out->status = SCMI_SUCCESS;
+			}
+		} else {
+			reset_state->asserted = in->flags &
+						SCMI_RD_RESET_FLAG_ASSERT;
+
+			out->status = SCMI_SUCCESS;
+		}
+	}
+
+	return 0;
+}
+
 static int sandbox_scmi_test_process_msg(struct udevice *dev,
 					 struct scmi_msg *msg)
 {
@@ -215,12 +311,21 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev,
 			break;
 		}
 		break;
+	case SCMI_PROTOCOL_ID_RESET_DOMAIN:
+		switch (msg->message_id) {
+		case SCMI_RESET_DOMAIN_ATTRIBUTES:
+			return sandbox_scmi_rd_attribs(dev, msg);
+		case SCMI_RESET_DOMAIN_RESET:
+			return sandbox_scmi_rd_reset(dev, msg);
+		default:
+			break;
+		}
+		break;
 	case SCMI_PROTOCOL_ID_BASE:
 	case SCMI_PROTOCOL_ID_POWER_DOMAIN:
 	case SCMI_PROTOCOL_ID_SYSTEM:
 	case SCMI_PROTOCOL_ID_PERF:
 	case SCMI_PROTOCOL_ID_SENSOR:
-	case SCMI_PROTOCOL_ID_RESET_DOMAIN:
 		*(u32 *)msg->out_msg = SCMI_NOT_SUPPORTED;
 		return 0;
 	default:
@@ -265,6 +370,8 @@ static int sandbox_scmi_test_probe(struct udevice *dev)
 			.idx = 0,
 			.clk = scmi0_clk,
 			.clk_count = ARRAY_SIZE(scmi0_clk),
+			.reset = scmi0_reset,
+			.reset_count = ARRAY_SIZE(scmi0_reset),
 		};
 		break;
 	case '1':
diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c
index 2ce8e664df..001ccbd87e 100644
--- a/drivers/firmware/scmi/sandbox-scmi_devices.c
+++ b/drivers/firmware/scmi/sandbox-scmi_devices.c
@@ -7,6 +7,7 @@
 #include <clk.h>
 #include <dm.h>
 #include <malloc.h>
+#include <reset.h>
 #include <asm/io.h>
 #include <asm/scmi_test.h>
 #include <dm/device_compat.h>
@@ -18,6 +19,7 @@
  */
 
 #define SCMI_TEST_DEVICES_CLK_COUNT		3
+#define SCMI_TEST_DEVICES_RD_COUNT		1
 
 /*
  * These tables store de handles used in the various uclasses device function
@@ -25,6 +27,7 @@
  * memory allocation to ease sharing with test sequence implementation.
  */
 static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT];
+static struct reset_ctl sandbox_scmi_reset_device[SCMI_TEST_DEVICES_RD_COUNT];
 static struct sandbox_scmi_devices sandbox_scmi_devhld;
 
 struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(void)
@@ -36,15 +39,26 @@ static void dereference_device_handles(struct sandbox_scmi_devices *devices)
 {
 	devices->clk = NULL;
 	devices->clk_count = 0;
+	devices->reset = NULL;
+	devices->reset_count = 0;
 }
 
 static int sandbox_scmi_devices_remove(struct udevice *dev)
 {
 	struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();
+	int rc = 0;
+	size_t n;
+
+	for (n = 0; n < SCMI_TEST_DEVICES_RD_COUNT; n++) {
+		int rc2 = reset_free(devices->reset + n);
+
+		if (rc2 && !rc)
+			rc = rc2;
+	}
 
 	dereference_device_handles(devices);
 
-	return 0;
+	return rc;
 }
 
 static int sandbox_scmi_devices_probe(struct udevice *dev)
@@ -54,7 +68,9 @@ static int sandbox_scmi_devices_probe(struct udevice *dev)
 	size_t n;
 
 	devices->clk = sandbox_scmi_clk_device;
+	devices->reset = sandbox_scmi_reset_device;
 	devices->clk_count = SCMI_TEST_DEVICES_CLK_COUNT;
+	devices->reset_count = SCMI_TEST_DEVICES_RD_COUNT;
 
 	for (n = 0; n < SCMI_TEST_DEVICES_CLK_COUNT; n++) {
 		rc = clk_get_by_index(dev, n, devices->clk + n);
@@ -64,8 +80,20 @@ static int sandbox_scmi_devices_probe(struct udevice *dev)
 		}
 	}
 
+	for (n = 0; n < SCMI_TEST_DEVICES_RD_COUNT; n++) {
+		rc = reset_get_by_index(dev, n, devices->reset + n);
+		if (rc) {
+			dev_err(dev, "%s: Failed on reset %zu\n", __func__, n);
+			goto err_reset;
+		}
+	}
+
 	return 0;
 
+err_reset:
+	for (; n > 0; n--)
+		reset_free(devices->reset + n - 1);
+
 err_clk:
 	dereference_device_handles(devices);
 
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index e652dbf52b..aa6ddf883f 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -15,6 +15,7 @@
 #include <common.h>
 #include <clk.h>
 #include <dm.h>
+#include <reset.h>
 #include <asm/scmi_test.h>
 #include <dm/device-internal.h>
 #include <dm/test.h>
@@ -35,6 +36,8 @@ static int ut_assert_scmi_state_preprobe(struct unit_test_state *uts)
 
 	if (scmi_devices->clk_count)
 		ut_asserteq(3, scmi_devices->clk_count);
+	if (scmi_devices->reset_count)
+		ut_asserteq(1, scmi_devices->reset_count);
 
 	return 0;
 }
@@ -48,6 +51,8 @@ static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts)
 	ut_assertnonnull(scmi_devices);
 	if (IS_ENABLED(CONFIG_CLK_SCMI))
 		ut_asserteq(3, scmi_devices->clk_count);
+	if (IS_ENABLED(CONFIG_RESET_SCMI))
+		ut_asserteq(1, scmi_devices->reset_count);
 
 	/* State of the simulated SCMI server exposed */
 	ut_asserteq(2, scmi_ctx->agent_count);
@@ -55,6 +60,8 @@ static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts)
 	ut_assertnonnull(scmi_ctx->agent[0]);
 	ut_asserteq(2, scmi_ctx->agent[0]->clk_count);
 	ut_assertnonnull(scmi_ctx->agent[0]->clk);
+	ut_asserteq(1, scmi_ctx->agent[0]->reset_count);
+	ut_assertnonnull(scmi_ctx->agent[0]->reset);
 
 	ut_assertnonnull(scmi_ctx->agent[1]);
 	ut_assertnonnull(scmi_ctx->agent[1]->clk);
@@ -163,3 +170,31 @@ static int dm_test_scmi_clocks(struct unit_test_state *uts)
 }
 
 DM_TEST(dm_test_scmi_clocks, UT_TESTF_SCAN_FDT);
+
+static int dm_test_scmi_resets(struct unit_test_state *uts)
+{
+	struct sandbox_scmi_service *scmi_ctx = sandbox_scmi_service_ctx();
+	struct sandbox_scmi_devices *scmi_devices = sandbox_scmi_devices_ctx();
+	struct udevice *dev = NULL;
+	int rc;
+
+	if (!IS_ENABLED(CONFIG_RESET_SCMI))
+		return 0;
+
+	rc = load_sandbox_scmi_test_devices(uts, &dev);
+	if (rc)
+		return rc;
+
+	/* Test SCMI resect controller manipulation */
+	ut_assert(!scmi_ctx->agent[0]->reset[0].asserted)
+
+	ut_assertok(reset_assert(&scmi_devices->reset[0]));
+	ut_assert(scmi_ctx->agent[0]->reset[0].asserted)
+
+	ut_assertok(reset_deassert(&scmi_devices->reset[0]));
+	ut_assert(!scmi_ctx->agent[0]->reset[0].asserted);
+
+	return release_sandbox_scmi_test_devices(uts, dev);
+}
+
+DM_TEST(dm_test_scmi_resets, UT_TESTF_SCAN_FDT);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 1/8] firmware: add SCMI agent uclass
  2020-09-07 14:49 [PATCH v3 1/8] firmware: add SCMI agent uclass Etienne Carriere
                   ` (6 preceding siblings ...)
  2020-09-07 14:50 ` [PATCH v3 8/8] firmware: smci: sandbox test for SCMI reset controllers Etienne Carriere
@ 2020-09-08 15:20 ` Simon Glass
  7 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2020-09-08 15:20 UTC (permalink / raw)
  To: u-boot

On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> This change introduces SCMI agent uclass to interact with a firmware
> using the SCMI protocols [1].
>
> SCMI agent uclass currently supports a single method to request
> processing of the SCMI message by an identified server. A SCMI message
> is made of a byte payload associated to a protocol ID and a message ID,
> all defined by the SCMI specification [1]. On return from process_msg()
> method, the caller gets the service response.
>
> SCMI agent uclass defines a post bind generic sequence for all devices.
> The sequence binds all the SCMI protocols listed in the FDT for that
> SCMI agent device. Currently none, but later change will introduce
> protocols.
>
> This change implements a simple sandbox device for the SCMI agent uclass.
> The sandbox nicely answers SCMI_NOT_SUPPORTED to SCMI messages.
> To prepare for further test support, the sandbox exposes a architecture
> function for test application to read the sandbox emulated devices state.
> Currently supports 2 SCMI agents, identified by an ID in the FDT device
> name. The simplistic DM test does nothing yet.
>
> SCMI agent uclass is designed for platforms that embed a SCMI server in
> a firmware hosted somewhere, for example in a companion co-processor or
> in the secure world of the executing processor. SCMI protocols allow an
> SCMI agent to discover and access external resources as clock, reset
> controllers and more. SCMI agent and server communicate following the
> SCMI specification [1]. This SCMI agent implementation complies with
> the DT bindings defined in the Linux kernel source tree regarding
> SCMI agent description since v5.8.
>
> Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
> 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:
> - Address comments about adding a new uclass and some sandbox test from
>   v2 in https://patchwork.ozlabs.org/project/uboot/list/?series=196253
> - New directory drivers/firmware/scmi/. The path mimics Linux kernel
>   source tree for the equivalent driver.
> - Split scmi.h (patch v2) into scmi_protocols.h, scmi_agent.h and
>   scmi_agent-uclass.h.
> - Create new uclass UCLASS_SCMI_AGENT.
> - Introduce a simple sandbox on that agent. Mailbox and smccc agents are
>   moved to specific commits in the series.
>
> 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.
> ---
>  arch/sandbox/dts/test.dts                  |  16 +++
>  arch/sandbox/include/asm/scmi_test.h       |  43 ++++++
>  configs/sandbox_defconfig                  |   2 +
>  drivers/firmware/Kconfig                   |   2 +
>  drivers/firmware/Makefile                  |   1 +
>  drivers/firmware/scmi/Kconfig              |  17 +++
>  drivers/firmware/scmi/Makefile             |   2 +
>  drivers/firmware/scmi/sandbox-scmi_agent.c | 147 +++++++++++++++++++++
>  drivers/firmware/scmi/scmi_agent-uclass.c  | 107 +++++++++++++++
>  include/dm/uclass-id.h                     |   1 +
>  include/scmi_agent-uclass.h                |  24 ++++
>  include/scmi_agent.h                       |  68 ++++++++++
>  include/scmi_protocols.h                   |  41 ++++++
>  test/dm/Makefile                           |   1 +
>  test/dm/scmi.c                             |  38 ++++++
>  15 files changed, 510 insertions(+)
>  create mode 100644 arch/sandbox/include/asm/scmi_test.h
>  create mode 100644 drivers/firmware/scmi/Kconfig
>  create mode 100644 drivers/firmware/scmi/Makefile
>  create mode 100644 drivers/firmware/scmi/sandbox-scmi_agent.c
>  create mode 100644 drivers/firmware/scmi/scmi_agent-uclass.c
>  create mode 100644 include/scmi_agent-uclass.h
>  create mode 100644 include/scmi_agent.h
>  create mode 100644 include/scmi_protocols.h
>  create mode 100644 test/dm/scmi.c
>

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 2/8] firmware: scmi: mailbox/smt agent device
  2020-09-07 14:49 ` [PATCH v3 2/8] firmware: scmi: mailbox/smt agent device Etienne Carriere
@ 2020-09-08 15:20   ` Simon Glass
  2020-09-09  9:33     ` Etienne Carriere
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2020-09-08 15:20 UTC (permalink / raw)
  To: u-boot

Hi Etienne,

On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> This change implements a mailbox transport using SMT format for SCMI
> exchanges. This implementation follows the Linux kernel and
> SCP-firmware [1] as references implementation for SCMI message
> processing using SMT format for communication channel meta-data.
>
> Use of mailboxes in SCMI FDT bindings are defined in the Linux kernel
> DT bindings since v4.17.
>
> Links: [1] https://github.com/ARM-software/SCP-firmware
> 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 mailbox SCMI agent proposed in v2
>   but split over few source files.
> ---
>  drivers/firmware/scmi/Kconfig         |   6 +-
>  drivers/firmware/scmi/Makefile        |   2 +
>  drivers/firmware/scmi/mailbox_agent.c | 107 ++++++++++++++++++++
>  drivers/firmware/scmi/smt.c           | 139 ++++++++++++++++++++++++++
>  drivers/firmware/scmi/smt.h           |  86 ++++++++++++++++
>  5 files changed, 338 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/firmware/scmi/mailbox_agent.c
>  create mode 100644 drivers/firmware/scmi/smt.c
>  create mode 100644 drivers/firmware/scmi/smt.h

Reviewed-by: Simon Glass <sjg@chromium.org>

>
> diff --git a/drivers/firmware/scmi/Kconfig b/drivers/firmware/scmi/Kconfig
> index 57e2ebbe42..c501bf4943 100644
> --- a/drivers/firmware/scmi/Kconfig
> +++ b/drivers/firmware/scmi/Kconfig
> @@ -2,7 +2,7 @@ config SCMI_FIRMWARE
>         bool "Enable SCMI support"
>         select FIRMWARE
>         select OF_TRANSLATE
> -       depends on SANDBOX
> +       depends on SANDBOX || DM_MAILBOX
>         help
>           System Control and Management Interface (SCMI) is a communication
>           protocol that defines standard interfaces for power, performance
> @@ -14,4 +14,6 @@ config SCMI_FIRMWARE
>           or a companion host in the CPU system.
>
>           Communications between agent (client) and the SCMI server are
> -         based on message exchange.
> +         based on message exchange. Messages can be exchange over tranport
> +         channels as a mailbox device with some piece of identified shared
> +         memory.
> diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
> index 336ea1f2a3..d22f53efe7 100644
> --- a/drivers/firmware/scmi/Makefile
> +++ b/drivers/firmware/scmi/Makefile
> @@ -1,2 +1,4 @@
>  obj-y  += scmi_agent-uclass.o
> +obj-y  += smt.o
> +obj-$(CONFIG_DM_MAILBOX)       += mailbox_agent.o
>  obj-$(CONFIG_SANDBOX)          += sandbox-scmi_agent.o
> diff --git a/drivers/firmware/scmi/mailbox_agent.c b/drivers/firmware/scmi/mailbox_agent.c
> new file mode 100644
> index 0000000000..9a7b0a5858
> --- /dev/null
> +++ b/drivers/firmware/scmi/mailbox_agent.c
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020 Linaro Limited.
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <mailbox.h>
> +#include <scmi_agent.h>
> +#include <scmi_agent-uclass.h>
> +#include <dm/devres.h>
> +#include <linux/compat.h>
> +
> +#include "smt.h"
> +
> +#define TIMEOUT_US_10MS                        10000
> +
> +/**
> + * struct scmi_mbox_channel - Description of an SCMI mailbox transport
> + * @smt:       Shared memory buffer
> + * @mbox:      Mailbox channel description
> + * @timeout_us:        Timeout in microseconds for the mailbox transfer
> + */
> +struct scmi_mbox_channel {
> +       struct scmi_smt smt;
> +       struct mbox_chan mbox;
> +       ulong timeout_us;
> +};
> +
> +static struct scmi_mbox_channel *scmi_mbox_get_priv(struct udevice *dev)
> +{
> +       return (struct scmi_mbox_channel *)dev_get_priv(dev);
> +}
> +
> +static int scmi_mbox_process_msg(struct udevice *dev, struct scmi_msg *msg)
> +{
> +       struct scmi_mbox_channel *chan = scmi_mbox_get_priv(dev);
> +       int ret;
> +
> +       ret = scmi_write_msg_to_smt(dev, &chan->smt, msg);
> +       if (ret)
> +               return ret;
> +
> +       /* Give shm addr to mbox in case it is meaningful */
> +       ret = mbox_send(&chan->mbox, chan->smt.buf);
> +       if (ret) {
> +               dev_err(dev, "Message send failed: %d\n", ret);
> +               goto out;
> +       }
> +
> +       /* Receive the response */
> +       ret = mbox_recv(&chan->mbox, chan->smt.buf, chan->timeout_us);
> +       if (ret) {
> +               dev_err(dev, "Response failed: %d, abort\n", ret);
> +               goto out;
> +       }
> +
> +       ret = scmi_read_resp_from_smt(dev, &chan->smt, msg);
> +
> +out:
> +       scmi_clear_smt_channel(&chan->smt);
> +
> +       return ret;
> +}
> +
> +int scmi_mbox_probe(struct udevice *dev)
> +{
> +       struct scmi_mbox_channel *chan = scmi_mbox_get_priv(dev);
> +       int ret;
> +
> +       chan->timeout_us = TIMEOUT_US_10MS;
> +
> +       ret = mbox_get_by_index(dev, 0, &chan->mbox);
> +       if (ret) {
> +               dev_err(dev, "Failed to find mailbox: %d\n", ret);
> +               goto out;
> +       }
> +
> +       ret = scmi_dt_get_smt_buffer(dev, &chan->smt);
> +       if (ret)
> +               dev_err(dev, "Failed to get shm resources: %d\n", ret);
> +
> +out:
> +       if (ret)
> +               devm_kfree(dev, chan);
> +
> +       return ret;
> +}
> +
> +static const struct udevice_id scmi_mbox_ids[] = {
> +       { .compatible = "arm,scmi" },
> +       { }
> +};
> +
> +static const struct scmi_agent_ops scmi_mbox_ops = {
> +       .process_msg = scmi_mbox_process_msg,
> +};
> +
> +U_BOOT_DRIVER(scmi_mbox) = {
> +       .name           = "scmi-over-mailbox",
> +       .id             = UCLASS_SCMI_AGENT,
> +       .of_match       = scmi_mbox_ids,
> +       .priv_auto_alloc_size = sizeof(struct scmi_mbox_channel),
> +       .probe          = scmi_mbox_probe,
> +       .ops            = &scmi_mbox_ops,
> +};
> diff --git a/drivers/firmware/scmi/smt.c b/drivers/firmware/scmi/smt.c
> new file mode 100644
> index 0000000000..afe95a4736
> --- /dev/null
> +++ b/drivers/firmware/scmi/smt.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights reserved.
> + * Copyright (C) 2019-2020 Linaro Limited.
> + */
> +
> +#include <common.h>
> +#include <cpu_func.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <scmi_agent.h>
> +#include <asm/cache.h>
> +#include <asm/system.h>
> +#include <dm/ofnode.h>
> +#include <linux/compat.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +
> +#include "smt.h"
> +
> +/**
> + * Get shared memory configuration defined by the referred DT phandle
> + * Return with a errno compliant value.
> + */
> +int scmi_dt_get_smt_buffer(struct udevice *dev, struct scmi_smt *smt)
> +{
> +       int ret;
> +       struct ofnode_phandle_args args;
> +       struct resource resource;
> +       fdt32_t faddr;
> +       phys_addr_t paddr;
> +
> +       ret = dev_read_phandle_with_args(dev, "shmem", NULL, 0, 0, &args);
> +       if (ret)
> +               return ret;
> +
> +       ret = ofnode_read_resource(args.node, 0, &resource);
> +       if (ret)
> +               return ret;
> +
> +       faddr = cpu_to_fdt32(resource.start);
> +       paddr = ofnode_translate_address(args.node, &faddr);
> +
> +       smt->size = resource_size(&resource);
> +       if (smt->size < sizeof(struct scmi_smt_header)) {
> +               dev_err(dev, "Shared memory buffer too small\n");
> +               return -EINVAL;
> +       }
> +
> +       smt->buf = devm_ioremap(dev, paddr, smt->size);
> +       if (!smt->buf)
> +               return -ENOMEM;
> +
> +#ifdef __arm__

Should that be CONFIG_ARM ?

> +       if (dcache_status())
> +               mmu_set_region_dcache_behaviour((uintptr_t)smt->buf,
> +                                               smt->size, DCACHE_OFF);
> +#endif
> +
> +       return 0;
> +}
> +
[..]

Regards,
Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport
  2020-09-07 14:49 ` [PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport Etienne Carriere
@ 2020-09-08 15:20   ` Simon Glass
  2020-09-09  9:40     ` Etienne Carriere
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2020-09-08 15:20 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 5/8] clk: add clock driver for SCMI agents
  2020-09-07 14:49 ` [PATCH v3 5/8] clk: add clock driver for SCMI agents Etienne Carriere
@ 2020-09-08 15:20   ` Simon Glass
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2020-09-08 15:20 UTC (permalink / raw)
  To: u-boot

On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> This change introduces a clock driver for SCMI agent devices. When
> SCMI agent and SCMI clock drivers are enabled, SCMI agent binds a
> clock device for each SCMI clock protocol devices enabled in the FDT.
>
> SCMI clock driver is embedded upon CONFIG_CLK_SCMI=y. If enabled,
> CONFIG_SCMI_AGENT is also enabled.
>
> SCMI Clock protocol is defined in the SCMI specification [1].
>
> Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> ---
>
> Changes in v3:
>
> Changes in v2:
> - CONFIG_CLK_SCMI depends on CONFIG_SCMI_FIRMWARE instead of
>   selecting CONFIG_SCMI_FIRMWARE.
> - Add inline comment description for structures and moves them to
>   source file top. Add/fixup some functions inline description comments.
> - Replace rc with ret as return value local variable label.
> - Fix scmi_clk_get_rate() return value to propagate error number.
> - Fix scmi_clk_set_rate() to request synchronous rate set operation:
>   drop flag SCMI_CLK_RATE_ASYNC_NORESP in the SCMI message payload.
> - Fix scmi_clk_set_rate() return value to return clock effective rate
>   on success.
> ---
>  drivers/clk/Kconfig                       |  8 ++
>  drivers/clk/Makefile                      |  1 +
>  drivers/clk/clk_scmi.c                    | 99 +++++++++++++++++++++++
>  drivers/firmware/scmi/scmi_agent-uclass.c |  3 +
>  include/scmi_protocols.h                  | 78 ++++++++++++++++++
>  5 files changed, 189 insertions(+)
>  create mode 100644 drivers/clk/clk_scmi.c

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 6/8] firmware: scmi: sandbox test for SCMI clocks
  2020-09-07 14:49 ` [PATCH v3 6/8] firmware: scmi: sandbox test for SCMI clocks Etienne Carriere
@ 2020-09-08 15:20   ` Simon Glass
  2020-09-09  9:58     ` Etienne Carriere
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2020-09-08 15:20 UTC (permalink / raw)
  To: u-boot

Hi Etienne,

On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> Add tests for SCMI clocks. A test device driver sandbox-scmi_devices.c
> is used to get clock resources, allowing further clock manipulation.
>
> Change sandbox-smci_agent to emulate 3 clocks exposed through 2 agents.
> Add DM test scmi_clocks to test these 3 clocks.
> Update DM test sandbox_scmi_agent with load/remove test sequences
> factorized by {load|remove}_sandbox_scmi_test_devices() helper functions.
>
> 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:
> - New commit in the series, addresses review comments on test support.
>   ut_dm_scmi_clocks test SCMI are found and behave as expected for the
>   implemented clk uclass methods.
> ---
>  arch/sandbox/dts/test.dts                    |  15 ++
>  arch/sandbox/include/asm/scmi_test.h         |  37 ++++
>  configs/sandbox_defconfig                    |   1 +
>  drivers/firmware/scmi/Makefile               |   2 +-
>  drivers/firmware/scmi/sandbox-scmi_agent.c   | 169 ++++++++++++++++++-
>  drivers/firmware/scmi/sandbox-scmi_devices.c |  86 ++++++++++
>  test/dm/scmi.c                               | 139 ++++++++++++++-
>  7 files changed, 438 insertions(+), 11 deletions(-)
>  create mode 100644 drivers/firmware/scmi/sandbox-scmi_devices.c

Reviewed-by: Simon Glass <sjg@chromium.org>

Nits below

[..]

> diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c
> new file mode 100644
> index 0000000000..2ce8e664df
> --- /dev/null
> +++ b/drivers/firmware/scmi/sandbox-scmi_devices.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020, Linaro Limited
> + */
> +
> +#include <common.h>
> +#include <clk.h>
> +#include <dm.h>
> +#include <malloc.h>
> +#include <asm/io.h>
> +#include <asm/scmi_test.h>
> +#include <dm/device_compat.h>
> +
> +/*
> + * Simulate to some extend a SCMI exhange.

extend, exchange

> + * This drivers gets SCMI resources and offers API function to the
> + * SCMI test sequence manipulate the resources.
> + */
> +
> +#define SCMI_TEST_DEVICES_CLK_COUNT            3
> +
> +/*
> + * These tables store de handles used in the various uclasses device function

s/de/the/ ?

> + * that are instancied when probed through the SCMI agent. Use a static

spelling

> + * memory allocation to ease sharing with test sequence implementation.
> + */
> +static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT];
> +static struct sandbox_scmi_devices sandbox_scmi_devhld;

This should really be in a struct, I think, pointed to by
dev_get_priv() on this device. I do try to avoid BSS with driver
model, although it is not a hard rule with test code.

> +
> +struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(void)
> +{
> +       return &sandbox_scmi_devhld;
> +}
> +
> +static void dereference_device_handles(struct sandbox_scmi_devices *devices)
> +{
> +       devices->clk = NULL;
> +       devices->clk_count = 0;
> +}
> +
> +static int sandbox_scmi_devices_remove(struct udevice *dev)
> +{
> +       struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();
> +
> +       dereference_device_handles(devices);
> +
> +       return 0;
> +}
> +
> +static int sandbox_scmi_devices_probe(struct udevice *dev)
> +{
> +       struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();
> +       int rc;
> +       size_t n;
> +
> +       devices->clk = sandbox_scmi_clk_device;
> +       devices->clk_count = SCMI_TEST_DEVICES_CLK_COUNT;
> +
> +       for (n = 0; n < SCMI_TEST_DEVICES_CLK_COUNT; n++) {
> +               rc = clk_get_by_index(dev, n, devices->clk + n);
> +               if (rc) {
> +                       dev_err(dev, "%s: Failed on clk %zu\n", __func__, n);
> +                       goto err_clk;
> +               }
> +       }
> +
> +       return 0;
> +
> +err_clk:
> +       dereference_device_handles(devices);
> +
> +       return rc;
> +}
> +
> +static const struct udevice_id sandbox_scmi_devices_ids[] = {
> +       { .compatible = "sandbox,scmi-devices" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(sandbox_scmi_devices) = {
> +       .name = "sandbox-scmi_devices",
> +       .id = UCLASS_MISC,
> +       .of_match = sandbox_scmi_devices_ids,
> +       .remove = sandbox_scmi_devices_remove,
> +       .probe = sandbox_scmi_devices_probe,
> +};
[..]

Regards,
Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 7/8] reset: add reset controller driver for SCMI agents
  2020-09-07 14:49 ` [PATCH v3 7/8] reset: add reset controller driver for SCMI agents Etienne Carriere
@ 2020-09-08 15:20   ` Simon Glass
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2020-09-08 15:20 UTC (permalink / raw)
  To: u-boot

On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> This change introduces a reset controller driver for SCMI agent devices.
> When SCMI agent and SCMI reset domain drivers are enabled, SCMI agent
> binds a reset controller device for each SCMI reset domain protocol
> devices enabled in the FDT.
>
> SCMI reset driver is embedded upon CONFIG_RESET_SCMI=y. If enabled,
> CONFIG_SCMI_AGENT is also enabled.
>
> SCMI Reset Domain protocol is defined in the SCMI specification [1].
>
> Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
> 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:
> - Upgrade to rename into devm_scmi_process_msg() and scmi.h split
>   into scmi_*.h.
> - Fix message ID used in scmi_reset_request().
>
> Changes in v2:
> - Change reset request() method to at least check the reset domain
>   exists by sending a SCMI RESET_DOMAIN_ATTRIBUTE message.
> - Add inline description for the several structures.
> - Patch v1 R-b tag not applied since the above changes in this v2.
>
> BACKPORTED FROM v2020.10-rc2 to V2020.04
> ---
>  drivers/firmware/scmi/scmi_agent-uclass.c |  3 +
>  drivers/reset/Kconfig                     |  8 +++
>  drivers/reset/Makefile                    |  1 +
>  drivers/reset/reset-scmi.c                | 81 +++++++++++++++++++++++
>  include/scmi_protocols.h                  | 60 +++++++++++++++++
>  5 files changed, 153 insertions(+)
>  create mode 100644 drivers/reset/reset-scmi.c

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 8/8] firmware: smci: sandbox test for SCMI reset controllers
  2020-09-07 14:50 ` [PATCH v3 8/8] firmware: smci: sandbox test for SCMI reset controllers Etienne Carriere
@ 2020-09-08 15:21   ` Simon Glass
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2020-09-08 15:21 UTC (permalink / raw)
  To: u-boot

Hi Etienne,

On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> Add tests for SCMI reset controllers. A test device driver
> sandbox-scmi_devices.c is used to get reset resources, allowing further
> resets manipulation.
>
> Change sandbox-smci_agent to emulate 1 reset controller exposed through
> an agent. Add DM test scmi_resets to test this reset controller.
>
> 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:
> - New commit in the series, addresses review comments on test support.
>   ut_dm_scmi_resets() tests SCMI resources are found and behave as
>   expected for the implemented reset uclass methods.
> ---
>  arch/sandbox/dts/test.dts                    |   6 +
>  arch/sandbox/include/asm/scmi_test.h         |  17 +++
>  configs/sandbox_defconfig                    |   1 +
>  drivers/firmware/scmi/sandbox-scmi_agent.c   | 117 ++++++++++++++++++-
>  drivers/firmware/scmi/sandbox-scmi_devices.c |  30 ++++-
>  test/dm/scmi.c                               |  35 ++++++
>  6 files changed, 200 insertions(+), 6 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Similar comment to the other patch about putting the BSS data in a
struct allocated by driver-private data.

Regards,
SImon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 2/8] firmware: scmi: mailbox/smt agent device
  2020-09-08 15:20   ` Simon Glass
@ 2020-09-09  9:33     ` Etienne Carriere
  0 siblings, 0 replies; 21+ messages in thread
From: Etienne Carriere @ 2020-09-09  9:33 UTC (permalink / raw)
  To: u-boot

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 mailbox transport using SMT format for SCMI
> > exchanges. This implementation follows the Linux kernel and
> > SCP-firmware [1] as references implementation for SCMI message
> > processing using SMT format for communication channel meta-data.
> >
> > Use of mailboxes in SCMI FDT bindings are defined in the Linux kernel
> > DT bindings since v4.17.
> >
> > Links: [1] https://github.com/ARM-software/SCP-firmware
> > 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 mailbox SCMI agent proposed in v2
> >   but split over few source files.
> > ---
> >  drivers/firmware/scmi/Kconfig         |   6 +-
> >  drivers/firmware/scmi/Makefile        |   2 +
> >  drivers/firmware/scmi/mailbox_agent.c | 107 ++++++++++++++++++++
> >  drivers/firmware/scmi/smt.c           | 139 ++++++++++++++++++++++++++
> >  drivers/firmware/scmi/smt.h           |  86 ++++++++++++++++
> >  5 files changed, 338 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/firmware/scmi/mailbox_agent.c
> >  create mode 100644 drivers/firmware/scmi/smt.c
> >  create mode 100644 drivers/firmware/scmi/smt.h
>
> Reviewed-by: Simon Glass <sjg@chromium.org>


Thanks for the reviews.


> > diff --git a/drivers/firmware/scmi/smt.c b/drivers/firmware/scmi/smt.c
> > new file mode 100644
> > index 0000000000..afe95a4736
> > --- /dev/null
> > +++ b/drivers/firmware/scmi/smt.c
> > @@ -0,0 +1,139 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights reserved.
> > + * Copyright (C) 2019-2020 Linaro Limited.
> > + */
> > +
> > +#include <common.h>
> > +#include <cpu_func.h>
> > +#include <dm.h>
> > +#include <errno.h>
> > +#include <scmi_agent.h>
> > +#include <asm/cache.h>
> > +#include <asm/system.h>
> > +#include <dm/ofnode.h>
> > +#include <linux/compat.h>
> > +#include <linux/io.h>
> > +#include <linux/ioport.h>
> > +
> > +#include "smt.h"
> > +
> > +/**
> > + * Get shared memory configuration defined by the referred DT phandle
> > + * Return with a errno compliant value.
> > + */
> > +int scmi_dt_get_smt_buffer(struct udevice *dev, struct scmi_smt *smt)
> > +{
> > +       int ret;
> > +       struct ofnode_phandle_args args;
> > +       struct resource resource;
> > +       fdt32_t faddr;
> > +       phys_addr_t paddr;
> > +
> > +       ret = dev_read_phandle_with_args(dev, "shmem", NULL, 0, 0, &args);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = ofnode_read_resource(args.node, 0, &resource);
> > +       if (ret)
> > +               return ret;
> > +
> > +       faddr = cpu_to_fdt32(resource.start);
> > +       paddr = ofnode_translate_address(args.node, &faddr);
> > +
> > +       smt->size = resource_size(&resource);
> > +       if (smt->size < sizeof(struct scmi_smt_header)) {
> > +               dev_err(dev, "Shared memory buffer too small\n");
> > +               return -EINVAL;
> > +       }
> > +
> > +       smt->buf = devm_ioremap(dev, paddr, smt->size);
> > +       if (!smt->buf)
> > +               return -ENOMEM;
> > +
> > +#ifdef __arm__
>
> Should that be CONFIG_ARM ?

Would covers CONFIG_ARM and CONFIG_ARM64, so OK. I'll send a patch v4.

etienne

>
> > +       if (dcache_status())
> > +               mmu_set_region_dcache_behaviour((uintptr_t)smt->buf,
> > +                                               smt->size, DCACHE_OFF);
> > +#endif
> > +
> > +       return 0;
> > +}
> > +
> [..]
>
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport
  2020-09-08 15:20   ` Simon Glass
@ 2020-09-09  9:40     ` Etienne Carriere
  2020-09-09 14:35       ` Simon Glass
  0 siblings, 1 reply; 21+ messages in thread
From: Etienne Carriere @ 2020-09-09  9:40 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 6/8] firmware: scmi: sandbox test for SCMI clocks
  2020-09-08 15:20   ` Simon Glass
@ 2020-09-09  9:58     ` Etienne Carriere
  2020-09-09 14:35       ` Simon Glass
  0 siblings, 1 reply; 21+ messages in thread
From: Etienne Carriere @ 2020-09-09  9:58 UTC (permalink / raw)
  To: u-boot

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:
> >
> > Add tests for SCMI clocks. A test device driver sandbox-scmi_devices.c
> > is used to get clock resources, allowing further clock manipulation.
> >
> > Change sandbox-smci_agent to emulate 3 clocks exposed through 2 agents.
> > Add DM test scmi_clocks to test these 3 clocks.
> > Update DM test sandbox_scmi_agent with load/remove test sequences
> > factorized by {load|remove}_sandbox_scmi_test_devices() helper functions.
> >
> > 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:
> > - New commit in the series, addresses review comments on test support.
> >   ut_dm_scmi_clocks test SCMI are found and behave as expected for the
> >   implemented clk uclass methods.
> > ---
> >  arch/sandbox/dts/test.dts                    |  15 ++
> >  arch/sandbox/include/asm/scmi_test.h         |  37 ++++
> >  configs/sandbox_defconfig                    |   1 +
> >  drivers/firmware/scmi/Makefile               |   2 +-
> >  drivers/firmware/scmi/sandbox-scmi_agent.c   | 169 ++++++++++++++++++-
> >  drivers/firmware/scmi/sandbox-scmi_devices.c |  86 ++++++++++
> >  test/dm/scmi.c                               | 139 ++++++++++++++-
> >  7 files changed, 438 insertions(+), 11 deletions(-)
> >  create mode 100644 drivers/firmware/scmi/sandbox-scmi_devices.c
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Nits below
>
> [..]
>
> > diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c
> > new file mode 100644
> > index 0000000000..2ce8e664df
> > --- /dev/null
> > +++ b/drivers/firmware/scmi/sandbox-scmi_devices.c
> > @@ -0,0 +1,86 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020, Linaro Limited
> > + */
> > +
> > +#include <common.h>
> > +#include <clk.h>
> > +#include <dm.h>
> > +#include <malloc.h>
> > +#include <asm/io.h>
> > +#include <asm/scmi_test.h>
> > +#include <dm/device_compat.h>
> > +
> > +/*
> > + * Simulate to some extend a SCMI exhange.
>
> extend, exchange

acked.

>
> > + * This drivers gets SCMI resources and offers API function to the
> > + * SCMI test sequence manipulate the resources.
> > + */
> > +
> > +#define SCMI_TEST_DEVICES_CLK_COUNT            3
> > +
> > +/*
> > + * These tables store de handles used in the various uclasses device function
>
> s/de/the/ ?

acked.

>
> > + * that are instancied when probed through the SCMI agent. Use a static
>
> spelling

acked, thanks.

>
> > + * memory allocation to ease sharing with test sequence implementation.
> > + */
> > +static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT];
> > +static struct sandbox_scmi_devices sandbox_scmi_devhld;
>
> This should really be in a struct, I think, pointed to by
> dev_get_priv() on this device. I do try to avoid BSS with driver
> model, although it is not a hard rule with test code.

I used a static structure here to ease sharing context reference with
the test sequence implementation.
Context reference returned by sandbox_scmi_devices_ctx() is always
reliable for the sequence.
(not possibly dereference in which case the test may segfault).

I can go this way if you prefer no BSS in drivers (Note this is a
sandbox driver). I'll update the test accordingly.

etienne

>
> > +
> > +struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(void)
> > +{
> > +       return &sandbox_scmi_devhld;
> > +}
> > +
> > +static void dereference_device_handles(struct sandbox_scmi_devices *devices)
> > +{
> > +       devices->clk = NULL;
> > +       devices->clk_count = 0;
> > +}
> > +
> > +static int sandbox_scmi_devices_remove(struct udevice *dev)
> > +{
> > +       struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();
> > +
> > +       dereference_device_handles(devices);
> > +
> > +       return 0;
> > +}
> > +
> > +static int sandbox_scmi_devices_probe(struct udevice *dev)
> > +{
> > +       struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();
> > +       int rc;
> > +       size_t n;
> > +
> > +       devices->clk = sandbox_scmi_clk_device;
> > +       devices->clk_count = SCMI_TEST_DEVICES_CLK_COUNT;
> > +
> > +       for (n = 0; n < SCMI_TEST_DEVICES_CLK_COUNT; n++) {
> > +               rc = clk_get_by_index(dev, n, devices->clk + n);
> > +               if (rc) {
> > +                       dev_err(dev, "%s: Failed on clk %zu\n", __func__, n);
> > +                       goto err_clk;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +
> > +err_clk:
> > +       dereference_device_handles(devices);
> > +
> > +       return rc;
> > +}
> > +
> > +static const struct udevice_id sandbox_scmi_devices_ids[] = {
> > +       { .compatible = "sandbox,scmi-devices" },
> > +       { }
> > +};
> > +
> > +U_BOOT_DRIVER(sandbox_scmi_devices) = {
> > +       .name = "sandbox-scmi_devices",
> > +       .id = UCLASS_MISC,
> > +       .of_match = sandbox_scmi_devices_ids,
> > +       .remove = sandbox_scmi_devices_remove,
> > +       .probe = sandbox_scmi_devices_probe,
> > +};
> [..]
>
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport
  2020-09-09  9:40     ` Etienne Carriere
@ 2020-09-09 14:35       ` Simon Glass
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2020-09-09 14:35 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 6/8] firmware: scmi: sandbox test for SCMI clocks
  2020-09-09  9:58     ` Etienne Carriere
@ 2020-09-09 14:35       ` Simon Glass
  2020-09-09 14:55         ` Etienne Carriere
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2020-09-09 14:35 UTC (permalink / raw)
  To: u-boot

Hi Etienne,

On Wed, 9 Sep 2020 at 03:58, 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:
> > >
> > > Add tests for SCMI clocks. A test device driver sandbox-scmi_devices.c
> > > is used to get clock resources, allowing further clock manipulation.
> > >
> > > Change sandbox-smci_agent to emulate 3 clocks exposed through 2 agents.
> > > Add DM test scmi_clocks to test these 3 clocks.
> > > Update DM test sandbox_scmi_agent with load/remove test sequences
> > > factorized by {load|remove}_sandbox_scmi_test_devices() helper functions.
> > >
> > > 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:
> > > - New commit in the series, addresses review comments on test support.
> > >   ut_dm_scmi_clocks test SCMI are found and behave as expected for the
> > >   implemented clk uclass methods.
> > > ---
> > >  arch/sandbox/dts/test.dts                    |  15 ++
> > >  arch/sandbox/include/asm/scmi_test.h         |  37 ++++
> > >  configs/sandbox_defconfig                    |   1 +
> > >  drivers/firmware/scmi/Makefile               |   2 +-
> > >  drivers/firmware/scmi/sandbox-scmi_agent.c   | 169 ++++++++++++++++++-
> > >  drivers/firmware/scmi/sandbox-scmi_devices.c |  86 ++++++++++
> > >  test/dm/scmi.c                               | 139 ++++++++++++++-
> > >  7 files changed, 438 insertions(+), 11 deletions(-)
> > >  create mode 100644 drivers/firmware/scmi/sandbox-scmi_devices.c
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > Nits below
> >
> > [..]

> > > + * memory allocation to ease sharing with test sequence implementation.
> > > + */
> > > +static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT];
> > > +static struct sandbox_scmi_devices sandbox_scmi_devhld;
> >
> > This should really be in a struct, I think, pointed to by
> > dev_get_priv() on this device. I do try to avoid BSS with driver
> > model, although it is not a hard rule with test code.
>
> I used a static structure here to ease sharing context reference with
> the test sequence implementation.

Is that in a different file?

> Context reference returned by sandbox_scmi_devices_ctx() is always
> reliable for the sequence.
> (not possibly dereference in which case the test may segfault).
>
> I can go this way if you prefer no BSS in drivers (Note this is a
> sandbox driver). I'll update the test accordingly.

If you have a reason to use BSS for a sandbox driver that is OK. I
don't quite understand the problem you are solving though. If private
data is allocated too late, you can allocate the info as platdata
which happens when the device is bound, and then use
dev_get_platdata().

But ultimately it is up to you, as this is a test driver. It just
helps to avoid people copying a pattern to their own driver.

[..]

Regards,
Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 6/8] firmware: scmi: sandbox test for SCMI clocks
  2020-09-09 14:35       ` Simon Glass
@ 2020-09-09 14:55         ` Etienne Carriere
  0 siblings, 0 replies; 21+ messages in thread
From: Etienne Carriere @ 2020-09-09 14:55 UTC (permalink / raw)
  To: u-boot

On Wed, 9 Sep 2020 at 16:35, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Etienne,
>
> On Wed, 9 Sep 2020 at 03:58, 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:
> > > >
> > > > Add tests for SCMI clocks. A test device driver sandbox-scmi_devices.c
> > > > is used to get clock resources, allowing further clock manipulation.
> > > >
> > > > Change sandbox-smci_agent to emulate 3 clocks exposed through 2 agents.
> > > > Add DM test scmi_clocks to test these 3 clocks.
> > > > Update DM test sandbox_scmi_agent with load/remove test sequences
> > > > factorized by {load|remove}_sandbox_scmi_test_devices() helper functions.
> > > >
> > > > 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:
> > > > - New commit in the series, addresses review comments on test support.
> > > >   ut_dm_scmi_clocks test SCMI are found and behave as expected for the
> > > >   implemented clk uclass methods.
> > > > ---
> > > >  arch/sandbox/dts/test.dts                    |  15 ++
> > > >  arch/sandbox/include/asm/scmi_test.h         |  37 ++++
> > > >  configs/sandbox_defconfig                    |   1 +
> > > >  drivers/firmware/scmi/Makefile               |   2 +-
> > > >  drivers/firmware/scmi/sandbox-scmi_agent.c   | 169 ++++++++++++++++++-
> > > >  drivers/firmware/scmi/sandbox-scmi_devices.c |  86 ++++++++++
> > > >  test/dm/scmi.c                               | 139 ++++++++++++++-
> > > >  7 files changed, 438 insertions(+), 11 deletions(-)
> > > >  create mode 100644 drivers/firmware/scmi/sandbox-scmi_devices.c
> > >
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > >
> > > Nits below
> > >
> > > [..]
>
> > > > + * memory allocation to ease sharing with test sequence implementation.
> > > > + */
> > > > +static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT];
> > > > +static struct sandbox_scmi_devices sandbox_scmi_devhld;
> > >
> > > This should really be in a struct, I think, pointed to by
> > > dev_get_priv() on this device. I do try to avoid BSS with driver
> > > model, although it is not a hard rule with test code.
> >
> > I used a static structure here to ease sharing context reference with
> > the test sequence implementation.
>
> Is that in a different file?

I made the helper function sandbox_scmi_devices_ctx() exported by
sandbox-scmi_devices.c that references probed devices.
The function is called from test/dm/scmi.c to run the test (actions &
verifications).


>
> > Context reference returned by sandbox_scmi_devices_ctx() is always
> > reliable for the sequence.
> > (not possibly dereference in which case the test may segfault).
> >
> > I can go this way if you prefer no BSS in drivers (Note this is a
> > sandbox driver). I'll update the test accordingly.
>
> If you have a reason to use BSS for a sandbox driver that is OK. I
> don't quite understand the problem you are solving though. If private
> data is allocated too late, you can allocate the info as platdata
> which happens when the device is bound, and then use
> dev_get_platdata().
>
> But ultimately it is up to you, as this is a test driver. It just
> helps to avoid people copying a pattern to their own driver.

Ok.
I'll try to keep it simple.

Regards,
etienne

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2020-09-09 14:55 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-07 14:49 [PATCH v3 1/8] firmware: add SCMI agent uclass Etienne Carriere
2020-09-07 14:49 ` [PATCH v3 2/8] firmware: scmi: mailbox/smt agent device Etienne Carriere
2020-09-08 15:20   ` Simon Glass
2020-09-09  9:33     ` Etienne Carriere
2020-09-07 14:49 ` [PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport Etienne Carriere
2020-09-08 15:20   ` Simon Glass
2020-09-09  9:40     ` Etienne Carriere
2020-09-09 14:35       ` Simon Glass
2020-09-07 14:49 ` [PATCH v3 4/8] dt-bindings: arm: SCMI bindings documentation Etienne Carriere
2020-09-07 14:49 ` [PATCH v3 5/8] clk: add clock driver for SCMI agents Etienne Carriere
2020-09-08 15:20   ` Simon Glass
2020-09-07 14:49 ` [PATCH v3 6/8] firmware: scmi: sandbox test for SCMI clocks Etienne Carriere
2020-09-08 15:20   ` Simon Glass
2020-09-09  9:58     ` Etienne Carriere
2020-09-09 14:35       ` Simon Glass
2020-09-09 14:55         ` Etienne Carriere
2020-09-07 14:49 ` [PATCH v3 7/8] reset: add reset controller driver for SCMI agents Etienne Carriere
2020-09-08 15:20   ` Simon Glass
2020-09-07 14:50 ` [PATCH v3 8/8] firmware: smci: sandbox test for SCMI reset controllers Etienne Carriere
2020-09-08 15:21   ` Simon Glass
2020-09-08 15:20 ` [PATCH v3 1/8] firmware: add SCMI agent uclass Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.