diff mbox series

[v2,2/2] firmware: scmi: fix sandbox and related test since clock discovery

Message ID 20210602095446.21947-2-etienne.carriere@linaro.org
State New
Headers show
Series [v2,1/2] clk: scmi: register all scmi clock by name with CCF | expand

Commit Message

Etienne Carriere June 2, 2021, 9:54 a.m. UTC 
Since SCMI clock are discovered because of integration in the CCF,
update SCMI emulation in sandbox accordingly. Sandbox must emulate all
clocks exposed by SCMI server since CCF clock discovery will query all
of them even if some clocks have no consumer. This change adds clock
discovery support in the sandbox and changes clock IDs so that less
clock shall be managed as each SCMI clock ID is actually an index
identifier.

For consistency, update also sandbox SCMI reset controller test to
conform with the IDs being also indices of exposed resources as per
SCMI specification.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

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

---
Changes since v1:
- Review tag applied
---
 arch/sandbox/dts/test.dts                  |   4 +-
 arch/sandbox/include/asm/scmi_test.h       |   2 -
 drivers/firmware/scmi/sandbox-scmi_agent.c | 102 ++++++++++++++++++---
 test/dm/scmi.c                             |  29 ++++--
 4 files changed, 113 insertions(+), 24 deletions(-)

-- 
2.17.1

Comments

Tom Rini July 22, 2021, 10:13 p.m. UTC | #1 
On Wed, Jun 02, 2021 at 11:54:46AM +0200, Etienne Carriere wrote:

> Since SCMI clock are discovered because of integration in the CCF,

> update SCMI emulation in sandbox accordingly. Sandbox must emulate all

> clocks exposed by SCMI server since CCF clock discovery will query all

> of them even if some clocks have no consumer. This change adds clock

> discovery support in the sandbox and changes clock IDs so that less

> clock shall be managed as each SCMI clock ID is actually an index

> identifier.

> 

> For consistency, update also sandbox SCMI reset controller test to

> conform with the IDs being also indices of exposed resources as per

> SCMI specification.

> 

> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

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

> ---

> Changes since v1:

> - Review tag applied


This breaks the tests as-is.

-- 
Tom
diff mbox series

Patch

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index fe26ced31d..91e488239b 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -1346,8 +1346,8 @@ 
 
 	sandbox_scmi {
 		compatible = "sandbox,scmi-devices";
-		clocks = <&clk_scmi0 7>, <&clk_scmi0 3>, <&clk_scmi1 1>;
-		resets = <&reset_scmi0 3>;
+		clocks = <&clk_scmi0 2>, <&clk_scmi0 0>, <&clk_scmi1 0>;
+		resets = <&reset_scmi0 0>;
 		regul0-supply = <&regul0_scmi0>;
 		regul1-supply = <&regul1_scmi0>;
 	};
diff --git a/arch/sandbox/include/asm/scmi_test.h b/arch/sandbox/include/asm/scmi_test.h
index 2930e686d7..bce18deeea 100644
--- a/arch/sandbox/include/asm/scmi_test.h
+++ b/arch/sandbox/include/asm/scmi_test.h
@@ -17,7 +17,6 @@  struct sandbox_scmi_service;
  * @rate:	Clock rate in Hertz
  */
 struct sandbox_scmi_clk {
-	uint id;
 	bool enabled;
 	ulong rate;
 };
@@ -28,7 +27,6 @@  struct sandbox_scmi_clk {
  * @asserted:	Reset control state: true if asserted, false if desasserted
  */
 struct sandbox_scmi_reset {
-	uint id;
 	bool asserted;
 };
 
diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
index 4b968205c2..dee9e8fb58 100644
--- a/drivers/firmware/scmi/sandbox-scmi_agent.c
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -27,7 +27,7 @@ 
  * See IDs in scmi0_clk[]/scmi0_reset[] and "sandbox-scmi-agent@0" in test.dts.
  *
  * Agent #1 simulates 1 clock.
- * See IDs in scmi1_clk[] and "sandbox-scmi-agent@1" in test.dts.
+ * See scmi1_clk[] and "sandbox-scmi-agent@1" in test.dts.
  *
  * All clocks and regulators are default disabled and reset controller down.
  *
@@ -40,12 +40,13 @@ 
 #define SANDBOX_SCMI_AGENT_COUNT	2
 
 static struct sandbox_scmi_clk scmi0_clk[] = {
-	{ .id = 7, .rate = 1000 },
-	{ .id = 3, .rate = 333 },
+	{ .rate = 333 },
+	{ .rate = 200 },
+	{ .rate = 1000 },
 };
 
 static struct sandbox_scmi_reset scmi0_reset[] = {
-	{ .id = 3 },
+	{ .asserted = false },
 };
 
 static struct sandbox_scmi_voltd scmi0_voltd[] = {
@@ -54,7 +55,7 @@  static struct sandbox_scmi_voltd scmi0_voltd[] = {
 };
 
 static struct sandbox_scmi_clk scmi1_clk[] = {
-	{ .id = 1, .rate = 44 },
+	{ .rate = 44 },
 };
 
 /* The list saves to simulted end devices references for test purpose */
@@ -102,7 +103,6 @@  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:
@@ -117,9 +117,8 @@  static struct sandbox_scmi_clk *get_scmi_clk_state(uint agent_id, uint clock_id)
 		return NULL;
 	}
 
-	for (n = 0; n < target_count; n++)
-		if (target[n].id == clock_id)
-			return target + n;
+	if (clock_id < target_count)
+		return target + clock_id;
 
 	return NULL;
 }
@@ -127,14 +126,21 @@  static struct sandbox_scmi_clk *get_scmi_clk_state(uint agent_id, uint clock_id)
 static struct sandbox_scmi_reset *get_scmi_reset_state(uint agent_id,
 						       uint reset_id)
 {
-	size_t n;
+	struct sandbox_scmi_reset *target = NULL;
+	size_t target_count = 0;
 
-	if (agent_id == 0) {
-		for (n = 0; n < ARRAY_SIZE(scmi0_reset); n++)
-			if (scmi0_reset[n].id == reset_id)
-				return scmi0_reset + n;
+	switch (agent_id) {
+	case 0:
+		target = scmi0_reset;
+		target_count = ARRAY_SIZE(scmi0_reset);
+		break;
+	default:
+		return NULL;
 	}
 
+	if (reset_id < target_count)
+		return target + reset_id;
+
 	return NULL;
 }
 
@@ -156,6 +162,70 @@  static struct sandbox_scmi_voltd *get_scmi_voltd_state(uint agent_id,
  * Sandbox SCMI agent ops
  */
 
+static int sandbox_scmi_clock_protocol_attribs(struct udevice *dev,
+					       struct scmi_msg *msg)
+{
+	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
+	struct scmi_clk_protocol_attr_out *out = NULL;
+
+	if (!msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	out = (struct scmi_clk_protocol_attr_out *)msg->out_msg;
+
+	switch (agent->idx) {
+	case 0:
+		out->attributes = ARRAY_SIZE(scmi0_clk);
+		out->status = SCMI_SUCCESS;
+		break;
+	case 1:
+		out->attributes = ARRAY_SIZE(scmi1_clk);
+		out->status = SCMI_SUCCESS;
+		break;
+	default:
+		out->status = SCMI_INVALID_PARAMETERS;
+		break;
+	}
+
+	return 0;
+}
+
+static int sandbox_scmi_clock_attribs(struct udevice *dev, struct scmi_msg *msg)
+{
+	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
+	struct scmi_clk_attribute_in *in = NULL;
+	struct scmi_clk_attribute_out *out = NULL;
+	struct sandbox_scmi_clk *clk_state = NULL;
+	int ret;
+
+	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_attribute_in *)msg->in_msg;
+	out = (struct scmi_clk_attribute_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 {
+		memset(out, 0, sizeof(*out));
+
+		if (clk_state->enabled)
+			out->attributes = 1;
+
+		ret = snprintf(out->clock_name, sizeof(out->clock_name),
+			       "clk%u", in->clock_id);
+		assert(ret > 0 && ret < sizeof(out->clock_name));
+
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
 static int sandbox_scmi_clock_rate_set(struct udevice *dev,
 				       struct scmi_msg *msg)
 {
@@ -479,6 +549,10 @@  static int sandbox_scmi_test_process_msg(struct udevice *dev,
 	switch (msg->protocol_id) {
 	case SCMI_PROTOCOL_ID_CLOCK:
 		switch (msg->message_id) {
+		case SCMI_PROTOCOL_ATTRIBUTES:
+			return sandbox_scmi_clock_protocol_attribs(dev, msg);
+		case SCMI_CLOCK_ATTRIBUTES:
+			return sandbox_scmi_clock_attribs(dev, msg);
 		case SCMI_CLOCK_RATE_SET:
 			return sandbox_scmi_clock_rate_set(dev, msg);
 		case SCMI_CLOCK_RATE_GET:
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index c938e6d4fc..7d0a6799b7 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -58,7 +58,7 @@  static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts,
 	ut_asserteq(2, scmi_ctx->agent_count);
 
 	ut_assertnonnull(agent0);
-	ut_asserteq(2, agent0->clk_count);
+	ut_asserteq(3, agent0->clk_count);
 	ut_assertnonnull(agent0->clk);
 	ut_asserteq(1, agent0->reset_count);
 	ut_assertnonnull(agent0->reset);
@@ -128,6 +128,15 @@  static int dm_test_scmi_clocks(struct unit_test_state *uts)
 	if (ret)
 		return ret;
 
+	/*
+	 * As stated in sandbox test.dts:
+	 * - Sandbox device clock #0 relates to SCMI server #0 clock #2
+	 * - Sandbox device clock #1 relates to SCMI server #0 clock #0
+	 * - Sandbox device clock #2 relates to SCMI server #1 clock #0
+	 *
+	 * Note there exists a SCMI server #0 clock #1 but is it not
+	 * used but the sandbox test device.
+	 */
 	scmi_devices = sandbox_scmi_devices_ctx(dev);
 	scmi_ctx = sandbox_scmi_service_ctx();
 	agent0 = scmi_ctx->agent[0];
@@ -141,8 +150,9 @@  static int dm_test_scmi_clocks(struct unit_test_state *uts)
 	ret_dev = clk_set_rate(&scmi_devices->clk[1], 1088);
 	ut_assert(!ret_dev || ret_dev == 1088);
 
-	ut_asserteq(1000, agent0->clk[0].rate);
-	ut_asserteq(1088, agent0->clk[1].rate);
+	ut_asserteq(1088, agent0->clk[0].rate);
+	ut_asserteq(200, agent0->clk[1].rate);
+	ut_asserteq(1000, agent0->clk[2	].rate);
 	ut_asserteq(44, agent1->clk[0].rate);
 
 	ut_asserteq(1000, clk_get_rate(&scmi_devices->clk[0]));
@@ -156,20 +166,23 @@  static int dm_test_scmi_clocks(struct unit_test_state *uts)
 	/* Test SCMI clocks gating manipulation */
 	ut_assert(!agent0->clk[0].enabled);
 	ut_assert(!agent0->clk[1].enabled);
+	ut_assert(!agent0->clk[2].enabled);
 	ut_assert(!agent1->clk[0].enabled);
 
-	ut_asserteq(0, clk_enable(&scmi_devices->clk[1]));
+	ut_asserteq(0, clk_enable(&scmi_devices->clk[0]));
 	ut_asserteq(0, clk_enable(&scmi_devices->clk[2]));
 
 	ut_assert(!agent0->clk[0].enabled);
-	ut_assert(agent0->clk[1].enabled);
+	ut_assert(!agent0->clk[1].enabled);
+	ut_assert(agent0->clk[2].enabled);
 	ut_assert(agent1->clk[0].enabled);
 
-	ut_assertok(clk_disable(&scmi_devices->clk[1]));
+	ut_assertok(clk_disable(&scmi_devices->clk[0]));
 	ut_assertok(clk_disable(&scmi_devices->clk[2]));
 
 	ut_assert(!agent0->clk[0].enabled);
 	ut_assert(!agent0->clk[1].enabled);
+	ut_assert(!agent0->clk[2].enabled);
 	ut_assert(!agent1->clk[0].enabled);
 
 	return release_sandbox_scmi_test_devices(uts, dev);
@@ -188,6 +201,10 @@  static int dm_test_scmi_resets(struct unit_test_state *uts)
 	if (ret)
 		return ret;
 
+	/*
+	 * As stated in sandbox test.dts:
+	 * - Sandbox device reset ctrl #0 relates to SCMI #0 reset domain #0
+	 */
 	scmi_devices = sandbox_scmi_devices_ctx(dev);
 	scmi_ctx = sandbox_scmi_service_ctx();
 	agent0 = scmi_ctx->agent[0];