[PATCH v2 1/5] doc: binding: scmi: link to latest Linux kernel binding

[PATCH v2 1/5] doc: binding: scmi: link to latest Linux kernel binding

[Etienne Carriere]

Changes SCMI bindings documentation to relate to Linux kernel
source tree that recently changed the bindings description to YAML
format.

Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes since v1:
- Apply R-b tag.
---
 doc/device-tree-bindings/arm/arm,scmi.txt | 236 +---------------------
 1 file changed, 2 insertions(+), 234 deletions(-)

diff --git a/doc/device-tree-bindings/arm/arm,scmi.txt b/doc/device-tree-bindings/arm/arm,scmi.txt
index 92572eabb5..0a7886da24 100644
--- a/doc/device-tree-bindings/arm/arm,scmi.txt
+++ b/doc/device-tree-bindings/arm/arm,scmi.txt
@@ -1,234 +1,2 @@
-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,
-	  or "linaro,scmi-optee" for OP-TEE transport.
-- 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
-- linaro,optee-channel-id : Channel specifier required when using OP-TEE
-	  transport.
-
-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.
-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, properties shall be present
-in the sub-node corresponding to that protocol. These properties are:
-- mboxes, mbox-names and shmem for mailbox transport
-- arm,smc-id and shmem for smc/hvc transport
-- linaro,optee-channel-id and possibly shmem for OP-TEE transport
-
-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.
-
-Regulator bindings for the SCMI Regulator based on SCMI Message Protocol
-------------------------------------------------------------
-An SCMI Regulator is permanently bound to a well defined SCMI Voltage Domain,
-and should be always positioned as a root regulator.
-It does not support any current operation.
-
-SCMI Regulators are grouped under a 'regulators' node which in turn is a child
-of the SCMI Voltage protocol node inside the desired SCMI instance node.
-
-This binding uses the common regulator binding[6].
-
-Required properties:
- - reg : shall identify an existent SCMI Voltage Domain.
-
-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
-[6] Documentation/devicetree/bindings/regulator/regulator.yaml
-
-Example:
-
-sram@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@0 {
-		compatible = "arm,scmi-shmem";
-		reg = <0x0 0x200>;
-	};
-
-	cpu_scp_hpri: scp-shmem@200 {
-		compatible = "arm,scmi-shmem";
-		reg = <0x200 0x200>;
-	};
-};
-
-mailbox@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@11 {
-			reg = <0x11>;
-			#power-domain-cells = <1>;
-		};
-
-		scmi_dvfs: protocol@13 {
-			reg = <0x13>;
-			#clock-cells = <1>;
-		};
-
-		scmi_clk: protocol@14 {
-			reg = <0x14>;
-			#clock-cells = <1>;
-		};
-
-		scmi_sensors0: protocol@15 {
-			reg = <0x15>;
-			#thermal-sensor-cells = <1>;
-		};
-
-		scmi_reset: protocol@16 {
-			reg = <0x16>;
-			#reset-cells = <1>;
-		};
-
-		scmi_voltage: protocol@17 {
-			reg = <0x17>;
-
-			regulators {
-				regulator_devX: regulator@0 {
-					reg = <0x0>;
-					regulator-max-microvolt = <3300000>;
-				};
-
-				regulator_devY: regulator@9 {
-					reg = <0x9>;
-					regulator-min-microvolt = <500000>;
-					regulator-max-microvolt = <4200000>;
-				};
-
-				...
-			};
-		};
-	};
-};
-
-cpu@0 {
-	...
-	reg = <0 0>;
-	clocks = <&scmi_dvfs 0>;
-};
-
-hdlcd@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>;
-		...
-	};
-};
+See Binding in Linux documentation:
+Documentation/devicetree/bindings/firmware/arm,scmi.yaml
-- 
2.17.1

[PATCH v2 2/5] sandbox: scmi: test against a single scmi agent

[Etienne Carriere]

As per DT bindings since Linux kernel v5.14, the device tree can define
only 1 SCMI agent node that is named scmi [1]. As a consequence, change
implementation of the SCMI driver test through sandbox architecture to
reflect that.

This change updates sandbox test DT and sandbox SCMI driver accordingly
since all these are impacted.

Cc: Simon Glass <sjg@chromium.org>
Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes since v1:
- Remove useless test and fix added/removed lines as per review comments.
- Apply Patrick's R-b tag.
---
 arch/sandbox/dts/test.dts                    |  37 ++--
 arch/sandbox/include/asm/scmi_test.h         |  12 +-
 drivers/firmware/scmi/sandbox-scmi_agent.c   | 169 ++++++-------------
 drivers/firmware/scmi/sandbox-scmi_devices.c |   4 +-
 test/dm/scmi.c                               | 114 ++++++-------
 5 files changed, 124 insertions(+), 212 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 48ca3e1e47..30874b038b 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -574,17 +574,21 @@
 			compatible = "sandbox,firmware";
 		};
 
-		sandbox-scmi-agent@0 {
+		scmi {
 			compatible = "sandbox,scmi-agent";
 			#address-cells = <1>;
 			#size-cells = <0>;
 
-			clk_scmi0: protocol@14 {
+			protocol@10 {
+				reg = <0x10>;
+			};
+
+			clk_scmi: protocol@14 {
 				reg = <0x14>;
 				#clock-cells = <1>;
 			};
 
-			reset_scmi0: protocol@16 {
+			reset_scmi: protocol@16 {
 				reg = <0x16>;
 				#reset-cells = <1>;
 			};
@@ -596,13 +600,13 @@
 					#address-cells = <1>;
 					#size-cells = <0>;
 
-					regul0_scmi0: reg@0 {
+					regul0_scmi: reg@0 {
 						reg = <0>;
 						regulator-name = "sandbox-voltd0";
 						regulator-min-microvolt = <1100000>;
 						regulator-max-microvolt = <3300000>;
 					};
-					regul1_scmi0: reg@1 {
+					regul1_scmi: reg@1 {
 						reg = <0x1>;
 						regulator-name = "sandbox-voltd1";
 						regulator-min-microvolt = <1800000>;
@@ -610,21 +614,6 @@
 				};
 			};
 		};
-
-		sandbox-scmi-agent@1 {
-			compatible = "sandbox,scmi-agent";
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			clk_scmi1: protocol@14 {
-				reg = <0x14>;
-				#clock-cells = <1>;
-			};
-
-			protocol@10 {
-				reg = <0x10>;
-			};
-		};
 	};
 
 	pinctrl-gpio {
@@ -1403,10 +1392,10 @@
 
 	sandbox_scmi {
 		compatible = "sandbox,scmi-devices";
-		clocks = <&clk_scmi0 7>, <&clk_scmi0 3>, <&clk_scmi1 1>;
-		resets = <&reset_scmi0 3>;
-		regul0-supply = <&regul0_scmi0>;
-		regul1-supply = <&regul1_scmi0>;
+		clocks = <&clk_scmi 7>, <&clk_scmi 3>;
+		resets = <&reset_scmi 3>;
+		regul0-supply = <&regul0_scmi>;
+		regul1-supply = <&regul1_scmi>;
 	};
 
 	pinctrl {
diff --git a/arch/sandbox/include/asm/scmi_test.h b/arch/sandbox/include/asm/scmi_test.h
index 2930e686d7..054be5f14e 100644
--- a/arch/sandbox/include/asm/scmi_test.h
+++ b/arch/sandbox/include/asm/scmi_test.h
@@ -46,7 +46,6 @@ struct sandbox_scmi_voltd {
 
 /**
  * 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
  * @reset:	Simulated reset domains
@@ -55,7 +54,6 @@ struct sandbox_scmi_voltd {
  * @voltd_count: Simulated voltage domains array size
  */
 struct sandbox_scmi_agent {
-	uint idx;
 	struct sandbox_scmi_clk *clk;
 	size_t clk_count;
 	struct sandbox_scmi_reset *reset;
@@ -66,12 +64,10 @@ struct sandbox_scmi_agent {
 
 /**
  * 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.
+ * @agent:		Pointer to SCMI sandbox agent or NULL if not probed
  */
 struct sandbox_scmi_service {
-	struct sandbox_scmi_agent **agent;
-	size_t agent_count;
+	struct sandbox_scmi_agent *agent;
 };
 
 /**
@@ -94,13 +90,13 @@ struct sandbox_scmi_devices {
 
 #ifdef CONFIG_SCMI_FIRMWARE
 /**
- * sandbox_scmi_service_context - Get the simulated SCMI services context
+ * sandbox_scmi_service_ctx - 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
+ * sandbox_scmi_devices_ctx - Get references to devices accessed through SCMI
  * @dev:	Reference to the test device used get test resources
  * @return:	Reference to the devices probed by the SCMI test
  */
diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
index 4b968205c2..51474b5760 100644
--- a/drivers/firmware/scmi/sandbox-scmi_agent.c
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -19,51 +19,35 @@
  * 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:
- * - SCMI clock protocol: emulate 2 agents each exposing few clocks
- * - SCMI reset protocol: emulate 1 agent exposing a reset controller
- * - SCMI voltage domain protocol: emulate 1 agent exposing 2 regulators
+ * - SCMI clock protocol emulates an agent exposing 2 clocks
+ * - SCMI reset protocol emulates an agent exposing a reset controller
+ * - SCMI voltage domain protocol emulates an agent exposing 2 regulators
  *
- * Agent #0 simulates 2 clocks, 1 reset domain and 1 voltage domain.
- * 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.
+ * As per DT bindings, the device node name shall be scmi.
  *
  * All clocks and regulators are default disabled and reset controller down.
  *
- * This Driver exports sandbox_scmi_service_ctx() for the test sequence to
+ * This driver exports sandbox_scmi_service_ctx() 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, as clocks and reset controllers.
  */
 
-#define SANDBOX_SCMI_AGENT_COUNT	2
-
-static struct sandbox_scmi_clk scmi0_clk[] = {
+static struct sandbox_scmi_clk scmi_clk[] = {
 	{ .id = 7, .rate = 1000 },
 	{ .id = 3, .rate = 333 },
 };
 
-static struct sandbox_scmi_reset scmi0_reset[] = {
+static struct sandbox_scmi_reset scmi_reset[] = {
 	{ .id = 3 },
 };
 
-static struct sandbox_scmi_voltd scmi0_voltd[] = {
+static struct sandbox_scmi_voltd scmi_voltd[] = {
 	{ .id = 0, .voltage_uv = 3300000 },
 	{ .id = 1, .voltage_uv = 1800000 },
 };
 
-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];
-
-static struct sandbox_scmi_service sandbox_scmi_service_state = {
-	.agent = sandbox_scmi_agent_list,
-	.agent_count = SANDBOX_SCMI_AGENT_COUNT,
-};
+static struct sandbox_scmi_service sandbox_scmi_service_state;
 
 struct sandbox_scmi_service *sandbox_scmi_service_ctx(void)
 {
@@ -74,9 +58,8 @@ static void debug_print_agent_state(struct udevice *dev, char *str)
 {
 	struct sandbox_scmi_agent *agent = dev_get_priv(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,
+	dev_dbg(dev, "Dump sandbox_scmi_agent: %s\n", str);
+	dev_dbg(dev, " scmi_clk   (%zu): %d/%ld, %d/%ld, %d/%ld, ...\n",
 		agent->clk_count,
 		agent->clk_count ? agent->clk[0].enabled : -1,
 		agent->clk_count ? agent->clk[0].rate : -1,
@@ -84,13 +67,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,
+	dev_dbg(dev, " scmi_reset (%zu): %d, %d, ...\n",
 		agent->reset_count,
 		agent->reset_count ? agent->reset[0].asserted : -1,
 		agent->reset_count > 1 ? agent->reset[1].asserted : -1);
-	dev_dbg(dev, " scmi%u_voltd (%zu): %u/%d, %u/%d, ...\n",
-		agent->idx,
+	dev_dbg(dev, " scmi_voltd (%zu): %u/%d, %u/%d, ...\n",
 		agent->voltd_count,
 		agent->voltd_count ? agent->voltd[0].enabled : -1,
 		agent->voltd_count ? agent->voltd[0].voltage_uv : -1,
@@ -98,56 +79,35 @@ static void debug_print_agent_state(struct udevice *dev, char *str)
 		agent->voltd_count ? agent->voltd[1].voltage_uv : -1);
 };
 
-static struct sandbox_scmi_clk *get_scmi_clk_state(uint agent_id, uint clock_id)
+static struct sandbox_scmi_clk *get_scmi_clk_state(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;
+	for (n = 0; n < ARRAY_SIZE(scmi_clk); n++)
+		if (scmi_clk[n].id == clock_id)
+			return scmi_clk + n;
 
 	return NULL;
 }
 
-static struct sandbox_scmi_reset *get_scmi_reset_state(uint agent_id,
-						       uint reset_id)
+static struct sandbox_scmi_reset *get_scmi_reset_state(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;
-	}
+	for (n = 0; n < ARRAY_SIZE(scmi_reset); n++)
+		if (scmi_reset[n].id == reset_id)
+			return scmi_reset + n;
 
 	return NULL;
 }
 
-static struct sandbox_scmi_voltd *get_scmi_voltd_state(uint agent_id,
-						       uint domain_id)
+static struct sandbox_scmi_voltd *get_scmi_voltd_state(uint domain_id)
 {
 	size_t n;
 
-	if (agent_id == 0) {
-		for (n = 0; n < ARRAY_SIZE(scmi0_voltd); n++)
-			if (scmi0_voltd[n].id == domain_id)
-				return scmi0_voltd + n;
-	}
+	for (n = 0; n < ARRAY_SIZE(scmi_voltd); n++)
+		if (scmi_voltd[n].id == domain_id)
+			return scmi_voltd + n;
 
 	return NULL;
 }
@@ -159,7 +119,6 @@ static struct sandbox_scmi_voltd *get_scmi_voltd_state(uint agent_id,
 static int sandbox_scmi_clock_rate_set(struct udevice *dev,
 				       struct scmi_msg *msg)
 {
-	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
 	struct scmi_clk_rate_set_in *in = NULL;
 	struct scmi_clk_rate_set_out *out = NULL;
 	struct sandbox_scmi_clk *clk_state = NULL;
@@ -171,7 +130,7 @@ static int sandbox_scmi_clock_rate_set(struct udevice *dev,
 	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);
+	clk_state = get_scmi_clk_state(in->clock_id);
 	if (!clk_state) {
 		dev_err(dev, "Unexpected clock ID %u\n", in->clock_id);
 
@@ -190,7 +149,6 @@ static int sandbox_scmi_clock_rate_set(struct udevice *dev,
 static int sandbox_scmi_clock_rate_get(struct udevice *dev,
 				       struct scmi_msg *msg)
 {
-	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
 	struct scmi_clk_rate_get_in *in = NULL;
 	struct scmi_clk_rate_get_out *out = NULL;
 	struct sandbox_scmi_clk *clk_state = NULL;
@@ -202,7 +160,7 @@ static int sandbox_scmi_clock_rate_get(struct udevice *dev,
 	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);
+	clk_state = get_scmi_clk_state(in->clock_id);
 	if (!clk_state) {
 		dev_err(dev, "Unexpected clock ID %u\n", in->clock_id);
 
@@ -219,7 +177,6 @@ static int sandbox_scmi_clock_rate_get(struct udevice *dev,
 
 static int sandbox_scmi_clock_gate(struct udevice *dev, struct scmi_msg *msg)
 {
-	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
 	struct scmi_clk_state_in *in = NULL;
 	struct scmi_clk_state_out *out = NULL;
 	struct sandbox_scmi_clk *clk_state = NULL;
@@ -231,7 +188,7 @@ static int sandbox_scmi_clock_gate(struct udevice *dev, struct scmi_msg *msg)
 	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);
+	clk_state = get_scmi_clk_state(in->clock_id);
 	if (!clk_state) {
 		dev_err(dev, "Unexpected clock ID %u\n", in->clock_id);
 
@@ -249,7 +206,6 @@ static int sandbox_scmi_clock_gate(struct udevice *dev, struct scmi_msg *msg)
 
 static int sandbox_scmi_rd_attribs(struct udevice *dev, struct scmi_msg *msg)
 {
-	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
 	struct scmi_rd_attr_in *in = NULL;
 	struct scmi_rd_attr_out *out = NULL;
 	struct sandbox_scmi_reset *reset_state = NULL;
@@ -261,7 +217,7 @@ static int sandbox_scmi_rd_attribs(struct udevice *dev, struct scmi_msg *msg)
 	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);
+	reset_state = get_scmi_reset_state(in->domain_id);
 	if (!reset_state) {
 		dev_err(dev, "Unexpected reset domain ID %u\n", in->domain_id);
 
@@ -278,7 +234,6 @@ static int sandbox_scmi_rd_attribs(struct udevice *dev, struct scmi_msg *msg)
 
 static int sandbox_scmi_rd_reset(struct udevice *dev, struct scmi_msg *msg)
 {
-	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
 	struct scmi_rd_reset_in *in = NULL;
 	struct scmi_rd_reset_out *out = NULL;
 	struct sandbox_scmi_reset *reset_state = NULL;
@@ -290,7 +245,7 @@ static int sandbox_scmi_rd_reset(struct udevice *dev, struct scmi_msg *msg)
 	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);
+	reset_state = get_scmi_reset_state(in->domain_id);
 	if (!reset_state) {
 		dev_err(dev, "Unexpected reset domain ID %u\n", in->domain_id);
 
@@ -321,7 +276,6 @@ static int sandbox_scmi_rd_reset(struct udevice *dev, struct scmi_msg *msg)
 
 static int sandbox_scmi_voltd_attribs(struct udevice *dev, struct scmi_msg *msg)
 {
-	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
 	struct scmi_voltd_attr_in *in = NULL;
 	struct scmi_voltd_attr_out *out = NULL;
 	struct sandbox_scmi_voltd *voltd_state = NULL;
@@ -333,7 +287,7 @@ static int sandbox_scmi_voltd_attribs(struct udevice *dev, struct scmi_msg *msg)
 	in = (struct scmi_voltd_attr_in *)msg->in_msg;
 	out = (struct scmi_voltd_attr_out *)msg->out_msg;
 
-	voltd_state = get_scmi_voltd_state(agent->idx, in->domain_id);
+	voltd_state = get_scmi_voltd_state(in->domain_id);
 	if (!voltd_state) {
 		dev_err(dev, "Unexpected domain ID %u\n", in->domain_id);
 
@@ -351,7 +305,6 @@ static int sandbox_scmi_voltd_attribs(struct udevice *dev, struct scmi_msg *msg)
 static int sandbox_scmi_voltd_config_set(struct udevice *dev,
 					 struct scmi_msg *msg)
 {
-	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
 	struct scmi_voltd_config_set_in *in = NULL;
 	struct scmi_voltd_config_set_out *out = NULL;
 	struct sandbox_scmi_voltd *voltd_state = NULL;
@@ -363,7 +316,7 @@ static int sandbox_scmi_voltd_config_set(struct udevice *dev,
 	in = (struct scmi_voltd_config_set_in *)msg->in_msg;
 	out = (struct scmi_voltd_config_set_out *)msg->out_msg;
 
-	voltd_state = get_scmi_voltd_state(agent->idx, in->domain_id);
+	voltd_state = get_scmi_voltd_state(in->domain_id);
 	if (!voltd_state) {
 		dev_err(dev, "Unexpected domain ID %u\n", in->domain_id);
 
@@ -388,7 +341,6 @@ static int sandbox_scmi_voltd_config_set(struct udevice *dev,
 static int sandbox_scmi_voltd_config_get(struct udevice *dev,
 					 struct scmi_msg *msg)
 {
-	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
 	struct scmi_voltd_config_get_in *in = NULL;
 	struct scmi_voltd_config_get_out *out = NULL;
 	struct sandbox_scmi_voltd *voltd_state = NULL;
@@ -400,7 +352,7 @@ static int sandbox_scmi_voltd_config_get(struct udevice *dev,
 	in = (struct scmi_voltd_config_get_in *)msg->in_msg;
 	out = (struct scmi_voltd_config_get_out *)msg->out_msg;
 
-	voltd_state = get_scmi_voltd_state(agent->idx, in->domain_id);
+	voltd_state = get_scmi_voltd_state(in->domain_id);
 	if (!voltd_state) {
 		dev_err(dev, "Unexpected domain ID %u\n", in->domain_id);
 
@@ -420,7 +372,6 @@ static int sandbox_scmi_voltd_config_get(struct udevice *dev,
 static int sandbox_scmi_voltd_level_set(struct udevice *dev,
 					 struct scmi_msg *msg)
 {
-	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
 	struct scmi_voltd_level_set_in *in = NULL;
 	struct scmi_voltd_level_set_out *out = NULL;
 	struct sandbox_scmi_voltd *voltd_state = NULL;
@@ -432,7 +383,7 @@ static int sandbox_scmi_voltd_level_set(struct udevice *dev,
 	in = (struct scmi_voltd_level_set_in *)msg->in_msg;
 	out = (struct scmi_voltd_level_set_out *)msg->out_msg;
 
-	voltd_state = get_scmi_voltd_state(agent->idx, in->domain_id);
+	voltd_state = get_scmi_voltd_state(in->domain_id);
 	if (!voltd_state) {
 		dev_err(dev, "Unexpected domain ID %u\n", in->domain_id);
 
@@ -448,7 +399,6 @@ static int sandbox_scmi_voltd_level_set(struct udevice *dev,
 static int sandbox_scmi_voltd_level_get(struct udevice *dev,
 					struct scmi_msg *msg)
 {
-	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
 	struct scmi_voltd_level_get_in *in = NULL;
 	struct scmi_voltd_level_get_out *out = NULL;
 	struct sandbox_scmi_voltd *voltd_state = NULL;
@@ -460,7 +410,7 @@ static int sandbox_scmi_voltd_level_get(struct udevice *dev,
 	in = (struct scmi_voltd_level_get_in *)msg->in_msg;
 	out = (struct scmi_voltd_level_get_out *)msg->out_msg;
 
-	voltd_state = get_scmi_voltd_state(agent->idx, in->domain_id);
+	voltd_state = get_scmi_voltd_state(in->domain_id);
 	if (!voltd_state) {
 		dev_err(dev, "Unexpected domain ID %u\n", in->domain_id);
 
@@ -541,52 +491,37 @@ static int sandbox_scmi_test_remove(struct udevice *dev)
 {
 	struct sandbox_scmi_agent *agent = dev_get_priv(dev);
 
+	if (agent != sandbox_scmi_service_ctx()->agent)
+		return -EINVAL;
+
 	debug_print_agent_state(dev, "removed");
 
 	/* We only need to dereference the agent in the context */
-	sandbox_scmi_service_ctx()->agent[agent->idx] = NULL;
+	sandbox_scmi_service_ctx()->agent = NULL;
 
 	return 0;
 }
 
 static int sandbox_scmi_test_probe(struct udevice *dev)
 {
-	static const char basename[] = "sandbox-scmi-agent@";
 	struct sandbox_scmi_agent *agent = dev_get_priv(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,
-			.clk = scmi0_clk,
-			.clk_count = ARRAY_SIZE(scmi0_clk),
-			.reset = scmi0_reset,
-			.reset_count = ARRAY_SIZE(scmi0_reset),
-			.voltd = scmi0_voltd,
-			.voltd_count = ARRAY_SIZE(scmi0_voltd),
-		};
-		break;
-	case '1':
-		*agent = (struct sandbox_scmi_agent){
-			.idx = 1,
-			.clk = scmi1_clk,
-			.clk_count = ARRAY_SIZE(scmi1_clk),
-		};
-		break;
-	default:
-		dev_err(dev, "%s(): Unexpected agent ID %s\n",
-			__func__, dev->name + basename_size);
-		return -ENOENT;
-	}
+
+	if (sandbox_scmi_service_ctx()->agent)
+		return -EINVAL;
+
+	*agent = (struct sandbox_scmi_agent){
+		.clk = scmi_clk,
+		.clk_count = ARRAY_SIZE(scmi_clk),
+		.reset = scmi_reset,
+		.reset_count = ARRAY_SIZE(scmi_reset),
+		.voltd = scmi_voltd,
+		.voltd_count = ARRAY_SIZE(scmi_voltd),
+	};
 
 	debug_print_agent_state(dev, "probed");
 
 	/* Save reference for tests purpose */
-	sandbox_scmi_service_ctx()->agent[agent->idx] = agent;
+	sandbox_scmi_service_ctx()->agent = agent;
 
 	return 0;
 };
diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c
index 66a6792881..9baeb469ec 100644
--- a/drivers/firmware/scmi/sandbox-scmi_devices.c
+++ b/drivers/firmware/scmi/sandbox-scmi_devices.c
@@ -23,7 +23,7 @@
  * and reset controllers.
  */
 
-#define SCMI_TEST_DEVICES_CLK_COUNT		3
+#define SCMI_TEST_DEVICES_CLK_COUNT		2
 #define SCMI_TEST_DEVICES_RD_COUNT		1
 #define SCMI_TEST_DEVICES_VOLTD_COUNT		2
 
@@ -135,7 +135,7 @@ U_BOOT_DRIVER(sandbox_scmi_devices) = {
 	.name = "sandbox-scmi_devices",
 	.id = UCLASS_MISC,
 	.of_match = sandbox_scmi_devices_ids,
-	.priv_auto	= sizeof(struct sandbox_scmi_device_priv),
+	.priv_auto = sizeof(struct sandbox_scmi_device_priv),
 	.remove = sandbox_scmi_devices_remove,
 	.probe = sandbox_scmi_devices_probe,
 };
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index c938e6d4fc..d576b5fd89 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -5,7 +5,7 @@
  * 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@10 node in agent 1. Protocol 0x10 is not
+ * Note in test.dts the protocol@10 node in scmi node. 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
@@ -28,8 +28,7 @@ static int ut_assert_scmi_state_preprobe(struct unit_test_state *uts)
 	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_assertnull(scmi_ctx->agent);
 
 	return 0;
 }
@@ -39,35 +38,26 @@ static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts,
 {
 	struct sandbox_scmi_devices *scmi_devices;
 	struct sandbox_scmi_service *scmi_ctx;
-	struct sandbox_scmi_agent *agent0;
-	struct sandbox_scmi_agent *agent1;
+	struct sandbox_scmi_agent *agent;
 
 	/* Device references to check context against test sequence */
 	scmi_devices = sandbox_scmi_devices_ctx(dev);
-
 	ut_assertnonnull(scmi_devices);
-	ut_asserteq(3, scmi_devices->clk_count);
+	ut_asserteq(2, scmi_devices->clk_count);
 	ut_asserteq(1, scmi_devices->reset_count);
 	ut_asserteq(2, scmi_devices->regul_count);
 
 	/* State of the simulated SCMI server exposed */
 	scmi_ctx = sandbox_scmi_service_ctx();
-	agent0 = scmi_ctx->agent[0];
-	agent1 = scmi_ctx->agent[1];
-
-	ut_asserteq(2, scmi_ctx->agent_count);
-
-	ut_assertnonnull(agent0);
-	ut_asserteq(2, agent0->clk_count);
-	ut_assertnonnull(agent0->clk);
-	ut_asserteq(1, agent0->reset_count);
-	ut_assertnonnull(agent0->reset);
-	ut_asserteq(2, agent0->voltd_count);
-	ut_assertnonnull(agent0->voltd);
-
-	ut_assertnonnull(agent1);
-	ut_assertnonnull(agent1->clk);
-	ut_asserteq(1, agent1->clk_count);
+	ut_assertnonnull(scmi_ctx);
+	agent = scmi_ctx->agent;
+	ut_assertnonnull(agent);
+	ut_asserteq(2, agent->clk_count);
+	ut_assertnonnull(agent->clk);
+	ut_asserteq(1, agent->reset_count);
+	ut_assertnonnull(agent->reset);
+	ut_asserteq(2, agent->voltd_count);
+	ut_assertnonnull(agent->voltd);
 
 	return 0;
 }
@@ -118,9 +108,8 @@ static int dm_test_scmi_clocks(struct unit_test_state *uts)
 {
 	struct sandbox_scmi_devices *scmi_devices;
 	struct sandbox_scmi_service *scmi_ctx;
-	struct sandbox_scmi_agent *agent0;
-	struct sandbox_scmi_agent *agent1;
-	struct udevice *dev = NULL;
+	struct sandbox_scmi_agent *agent;
+	struct udevice *dev;
 	int ret_dev;
 	int ret;
 
@@ -129,48 +118,45 @@ static int dm_test_scmi_clocks(struct unit_test_state *uts)
 		return ret;
 
 	scmi_devices = sandbox_scmi_devices_ctx(dev);
+	ut_assertnonnull(scmi_devices);
 	scmi_ctx = sandbox_scmi_service_ctx();
-	agent0 = scmi_ctx->agent[0];
-	agent1 = scmi_ctx->agent[1];
+	ut_assertnonnull(scmi_ctx);
+	agent = scmi_ctx->agent;
+	ut_assertnonnull(agent);
 
 	/* 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]));
 
 	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(44, agent1->clk[0].rate);
+	ut_asserteq(1000, agent->clk[0].rate);
+	ut_asserteq(1088, agent->clk[1].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 */
 	ret_dev = clk_set_rate(&scmi_devices->clk[1], 333);
 	ut_assert(!ret_dev || ret_dev == 333);
 
 	/* Test SCMI clocks gating manipulation */
-	ut_assert(!agent0->clk[0].enabled);
-	ut_assert(!agent0->clk[1].enabled);
-	ut_assert(!agent1->clk[0].enabled);
+	ut_assert(!agent->clk[0].enabled);
+	ut_assert(!agent->clk[1].enabled);
+	ut_assert(!agent->clk[2].enabled);
 
 	ut_asserteq(0, clk_enable(&scmi_devices->clk[1]));
-	ut_asserteq(0, clk_enable(&scmi_devices->clk[2]));
 
-	ut_assert(!agent0->clk[0].enabled);
-	ut_assert(agent0->clk[1].enabled);
-	ut_assert(agent1->clk[0].enabled);
+	ut_assert(!agent->clk[0].enabled);
+	ut_assert(agent->clk[1].enabled);
+	ut_assert(!agent->clk[2].enabled);
 
 	ut_assertok(clk_disable(&scmi_devices->clk[1]));
-	ut_assertok(clk_disable(&scmi_devices->clk[2]));
 
-	ut_assert(!agent0->clk[0].enabled);
-	ut_assert(!agent0->clk[1].enabled);
-	ut_assert(!agent1->clk[0].enabled);
+	ut_assert(!agent->clk[0].enabled);
+	ut_assert(!agent->clk[1].enabled);
+	ut_assert(!agent->clk[2].enabled);
 
 	return release_sandbox_scmi_test_devices(uts, dev);
 }
@@ -180,7 +166,7 @@ static int dm_test_scmi_resets(struct unit_test_state *uts)
 {
 	struct sandbox_scmi_devices *scmi_devices;
 	struct sandbox_scmi_service *scmi_ctx;
-	struct sandbox_scmi_agent *agent0;
+	struct sandbox_scmi_agent *agent;
 	struct udevice *dev = NULL;
 	int ret;
 
@@ -189,17 +175,20 @@ static int dm_test_scmi_resets(struct unit_test_state *uts)
 		return ret;
 
 	scmi_devices = sandbox_scmi_devices_ctx(dev);
+	ut_assertnonnull(scmi_devices);
 	scmi_ctx = sandbox_scmi_service_ctx();
-	agent0 = scmi_ctx->agent[0];
+	ut_assertnonnull(scmi_ctx);
+	agent = scmi_ctx->agent;
+	ut_assertnonnull(agent);
 
 	/* Test SCMI resect controller manipulation */
-	ut_assert(!agent0->reset[0].asserted)
+	ut_assert(!agent->reset[0].asserted)
 
 	ut_assertok(reset_assert(&scmi_devices->reset[0]));
-	ut_assert(agent0->reset[0].asserted)
+	ut_assert(agent->reset[0].asserted)
 
 	ut_assertok(reset_deassert(&scmi_devices->reset[0]));
-	ut_assert(!agent0->reset[0].asserted);
+	ut_assert(!agent->reset[0].asserted);
 
 	return release_sandbox_scmi_test_devices(uts, dev);
 }
@@ -209,7 +198,7 @@ static int dm_test_scmi_voltage_domains(struct unit_test_state *uts)
 {
 	struct sandbox_scmi_devices *scmi_devices;
 	struct sandbox_scmi_service *scmi_ctx;
-	struct sandbox_scmi_agent *agent0;
+	struct sandbox_scmi_agent *agent;
 	struct dm_regulator_uclass_plat *uc_pdata;
 	struct udevice *dev;
 	struct udevice *regul0_dev;
@@ -217,8 +206,11 @@ static int dm_test_scmi_voltage_domains(struct unit_test_state *uts)
 	ut_assertok(load_sandbox_scmi_test_devices(uts, &dev));
 
 	scmi_devices = sandbox_scmi_devices_ctx(dev);
+	ut_assertnonnull(scmi_devices);
 	scmi_ctx = sandbox_scmi_service_ctx();
-	agent0 = scmi_ctx->agent[0];
+	ut_assertnonnull(scmi_ctx);
+	agent = scmi_ctx->agent;
+	ut_assertnonnull(agent);
 
 	/* Set/Get an SCMI voltage domain level */
 	regul0_dev = scmi_devices->regul[0];
@@ -228,32 +220,32 @@ static int dm_test_scmi_voltage_domains(struct unit_test_state *uts)
 	ut_assert(uc_pdata);
 
 	ut_assertok(regulator_set_value(regul0_dev, uc_pdata->min_uV));
-	ut_asserteq(agent0->voltd[0].voltage_uv, uc_pdata->min_uV);
+	ut_asserteq(agent->voltd[0].voltage_uv, uc_pdata->min_uV);
 
 	ut_assert(regulator_get_value(regul0_dev) == uc_pdata->min_uV);
 
 	ut_assertok(regulator_set_value(regul0_dev, uc_pdata->max_uV));
-	ut_asserteq(agent0->voltd[0].voltage_uv, uc_pdata->max_uV);
+	ut_asserteq(agent->voltd[0].voltage_uv, uc_pdata->max_uV);
 
 	ut_assert(regulator_get_value(regul0_dev) == uc_pdata->max_uV);
 
 	/* Enable/disable SCMI voltage domains */
 	ut_assertok(regulator_set_enable(scmi_devices->regul[0], false));
 	ut_assertok(regulator_set_enable(scmi_devices->regul[1], false));
-	ut_assert(!agent0->voltd[0].enabled);
-	ut_assert(!agent0->voltd[1].enabled);
+	ut_assert(!agent->voltd[0].enabled);
+	ut_assert(!agent->voltd[1].enabled);
 
 	ut_assertok(regulator_set_enable(scmi_devices->regul[0], true));
-	ut_assert(agent0->voltd[0].enabled);
-	ut_assert(!agent0->voltd[1].enabled);
+	ut_assert(agent->voltd[0].enabled);
+	ut_assert(!agent->voltd[1].enabled);
 
 	ut_assertok(regulator_set_enable(scmi_devices->regul[1], true));
-	ut_assert(agent0->voltd[0].enabled);
-	ut_assert(agent0->voltd[1].enabled);
+	ut_assert(agent->voltd[0].enabled);
+	ut_assert(agent->voltd[1].enabled);
 
 	ut_assertok(regulator_set_enable(scmi_devices->regul[0], false));
-	ut_assert(!agent0->voltd[0].enabled);
-	ut_assert(agent0->voltd[1].enabled);
+	ut_assert(!agent->voltd[0].enabled);
+	ut_assert(agent->voltd[1].enabled);
 
 	return release_sandbox_scmi_test_devices(uts, dev);
 }
-- 
2.17.1

Re: [PATCH v2 2/5] sandbox: scmi: test against a single scmi agent

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 640 bytes --]

On Mon, Feb 21, 2022 at 09:22:39AM +0100, Etienne Carriere wrote:

> As per DT bindings since Linux kernel v5.14, the device tree can define
> only 1 SCMI agent node that is named scmi [1]. As a consequence, change
> implementation of the SCMI driver test through sandbox architecture to
> reflect that.
> 
> This change updates sandbox test DT and sandbox SCMI driver accordingly
> since all these are impacted.
> 
> Cc: Simon Glass <sjg@chromium.org>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2 3/5] scmi: change parameter dev in devm_scmi_process_msg

[Etienne Carriere]

Changes devm_scmi_process_msg() first argument from target parent device
to current SCMI device and lookup the SCMI agent device among SCMI device
parents for find the SCMI agent operator needed for communication with
the firmware.

This change is needed in order to support CCF in clk_scmi driver unless
what CCF will fail to find the right udevice related to exposed SCMI
clocks.

This patch allows to simplify the caller sequence, using SCMI device
reference as parameter instead of knowing SCMI uclass topology. This
change also adds some protection in case devm_scmi_process_msg() API
function is called for an invalid device type.

Cc: Lukasz Majewski <lukma@denx.de>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes since v1:
- Apply R-b tag.
---
 drivers/clk/clk_scmi.c                    |  6 +++---
 drivers/firmware/scmi/scmi_agent-uclass.c | 17 +++++++++++++++--
 drivers/power/regulator/scmi_regulator.c  | 10 +++++-----
 drivers/reset/reset-scmi.c                |  4 ++--
 include/scmi_agent.h                      |  2 +-
 5 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index 9a0a6f6643..42fbab0d21 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -24,7 +24,7 @@ static int scmi_clk_gate(struct clk *clk, int enable)
 					  in, out);
 	int ret;
 
-	ret = devm_scmi_process_msg(clk->dev->parent, &msg);
+	ret = devm_scmi_process_msg(clk->dev, &msg);
 	if (ret)
 		return ret;
 
@@ -52,7 +52,7 @@ static ulong scmi_clk_get_rate(struct clk *clk)
 					  in, out);
 	int ret;
 
-	ret = devm_scmi_process_msg(clk->dev->parent, &msg);
+	ret = devm_scmi_process_msg(clk->dev, &msg);
 	if (ret < 0)
 		return ret;
 
@@ -77,7 +77,7 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
 					  in, out);
 	int ret;
 
-	ret = devm_scmi_process_msg(clk->dev->parent, &msg);
+	ret = devm_scmi_process_msg(clk->dev, &msg);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c
index 4f5870b483..3819f2fa99 100644
--- a/drivers/firmware/scmi/scmi_agent-uclass.c
+++ b/drivers/firmware/scmi/scmi_agent-uclass.c
@@ -116,10 +116,23 @@ static const struct scmi_agent_ops *transport_dev_ops(struct udevice *dev)
 
 int devm_scmi_process_msg(struct udevice *dev, struct scmi_msg *msg)
 {
-	const struct scmi_agent_ops *ops = transport_dev_ops(dev);
+	const struct scmi_agent_ops *ops;
+	struct udevice *parent = dev;
+
+	/* Find related SCMI agent device */
+	do {
+		parent = dev_get_parent(parent);
+	} while (parent && device_get_uclass_id(parent) != UCLASS_SCMI_AGENT);
+
+	if (!parent) {
+		dev_err(dev, "Invalid SCMI device, agent not found\n");
+		return -ENODEV;
+	}
+
+	ops = transport_dev_ops(parent);
 
 	if (ops->process_msg)
-		return ops->process_msg(dev, msg);
+		return ops->process_msg(parent, msg);
 
 	return -EPROTONOSUPPORT;
 }
diff --git a/drivers/power/regulator/scmi_regulator.c b/drivers/power/regulator/scmi_regulator.c
index 3ddeaf4adc..2966bdcf83 100644
--- a/drivers/power/regulator/scmi_regulator.c
+++ b/drivers/power/regulator/scmi_regulator.c
@@ -38,7 +38,7 @@ static int scmi_voltd_set_enable(struct udevice *dev, bool enable)
 					  in, out);
 	int ret;
 
-	ret = devm_scmi_process_msg(dev->parent->parent, &msg);
+	ret = devm_scmi_process_msg(dev, &msg);
 	if (ret)
 		return ret;
 
@@ -61,7 +61,7 @@ static int scmi_voltd_get_enable(struct udevice *dev)
 					  in, out);
 	int ret;
 
-	ret = devm_scmi_process_msg(dev->parent->parent, &msg);
+	ret = devm_scmi_process_msg(dev, &msg);
 	if (ret < 0)
 		return ret;
 
@@ -85,7 +85,7 @@ static int scmi_voltd_set_voltage_level(struct udevice *dev, int uV)
 					  in, out);
 	int ret;
 
-	ret = devm_scmi_process_msg(dev->parent->parent, &msg);
+	ret = devm_scmi_process_msg(dev, &msg);
 	if (ret < 0)
 		return ret;
 
@@ -104,7 +104,7 @@ static int scmi_voltd_get_voltage_level(struct udevice *dev)
 					  in, out);
 	int ret;
 
-	ret = devm_scmi_process_msg(dev->parent->parent, &msg);
+	ret = devm_scmi_process_msg(dev, &msg);
 	if (ret < 0)
 		return ret;
 
@@ -147,7 +147,7 @@ static int scmi_regulator_probe(struct udevice *dev)
 	/* Check voltage domain is known from SCMI server */
 	in.domain_id = pdata->domain_id;
 
-	ret = devm_scmi_process_msg(dev->parent->parent, &scmi_msg);
+	ret = devm_scmi_process_msg(dev, &scmi_msg);
 	if (ret) {
 		dev_err(dev, "Failed to query voltage domain %u: %d\n",
 			pdata->domain_id, ret);
diff --git a/drivers/reset/reset-scmi.c b/drivers/reset/reset-scmi.c
index ca0135a420..850cb18886 100644
--- a/drivers/reset/reset-scmi.c
+++ b/drivers/reset/reset-scmi.c
@@ -26,7 +26,7 @@ static int scmi_reset_set_level(struct reset_ctl *rst, bool assert_not_deassert)
 					  in, out);
 	int ret;
 
-	ret = devm_scmi_process_msg(rst->dev->parent, &msg);
+	ret = devm_scmi_process_msg(rst->dev, &msg);
 	if (ret)
 		return ret;
 
@@ -58,7 +58,7 @@ static int scmi_reset_request(struct reset_ctl *rst)
 	 * We don't really care about the attribute, just check
 	 * the reset domain exists.
 	 */
-	ret = devm_scmi_process_msg(rst->dev->parent, &msg);
+	ret = devm_scmi_process_msg(rst->dev, &msg);
 	if (ret)
 		return ret;
 
diff --git a/include/scmi_agent.h b/include/scmi_agent.h
index 5015c06be9..18bcd48a9d 100644
--- a/include/scmi_agent.h
+++ b/include/scmi_agent.h
@@ -51,7 +51,7 @@ struct scmi_msg {
  * 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
+ * @dev:	SCMI device
  * @msg:	Message structure reference
  * Return: 0 on success and a negative errno on failure
  */
-- 
2.17.1

Re: [PATCH v2 3/5] scmi: change parameter dev in devm_scmi_process_msg

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]

On Mon, Feb 21, 2022 at 09:22:40AM +0100, Etienne Carriere wrote:

> Changes devm_scmi_process_msg() first argument from target parent device
> to current SCMI device and lookup the SCMI agent device among SCMI device
> parents for find the SCMI agent operator needed for communication with
> the firmware.
> 
> This change is needed in order to support CCF in clk_scmi driver unless
> what CCF will fail to find the right udevice related to exposed SCMI
> clocks.
> 
> This patch allows to simplify the caller sequence, using SCMI device
> reference as parameter instead of knowing SCMI uclass topology. This
> change also adds some protection in case devm_scmi_process_msg() API
> function is called for an invalid device type.
> 
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Sean Anderson <seanga2@gmail.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2 4/5] firmware: scmi: fix sandbox and related tests for clock discovery

[Etienne Carriere]

Updates sandbox SCMI clock driver and tests since enabling CCF will
mandate clock discovery that is all exposed SCMI clocks shall be
discovered at initialization. For this reason, sandbox SCMI clock
driver must emulate all clocks exposed by SCMI server, not only those
effectively consumed by some other U-Boot devices.

Therefore the sandbox SCMI test driver exposes 3 clocks (IDs 0, 1 and 2)
and sandbox SCMI clock consumer driver gets 2 of them.

Cc: Simon Glass <sjg@chromium.org>
Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes since v1:
- Remove Series-links tag and apply R-b tag.
---
 arch/sandbox/dts/test.dts                  |  2 +-
 arch/sandbox/include/asm/scmi_test.h       |  1 -
 drivers/firmware/scmi/sandbox-scmi_agent.c | 12 +++++-------
 test/dm/scmi.c                             | 15 ++++++++++-----
 4 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 30874b038b..3d206fdb3c 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -1392,7 +1392,7 @@
 
 	sandbox_scmi {
 		compatible = "sandbox,scmi-devices";
-		clocks = <&clk_scmi 7>, <&clk_scmi 3>;
+		clocks = <&clk_scmi 2>, <&clk_scmi 0>;
 		resets = <&reset_scmi 3>;
 		regul0-supply = <&regul0_scmi>;
 		regul1-supply = <&regul1_scmi>;
diff --git a/arch/sandbox/include/asm/scmi_test.h b/arch/sandbox/include/asm/scmi_test.h
index 054be5f14e..c72ec1e1cb 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;
 };
diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
index 51474b5760..fc717a8aea 100644
--- a/drivers/firmware/scmi/sandbox-scmi_agent.c
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -34,8 +34,9 @@
  */
 
 static struct sandbox_scmi_clk scmi_clk[] = {
-	{ .id = 7, .rate = 1000 },
-	{ .id = 3, .rate = 333 },
+	{ .rate = 333 },
+	{ .rate = 200 },
+	{ .rate = 1000 },
 };
 
 static struct sandbox_scmi_reset scmi_reset[] = {
@@ -81,11 +82,8 @@ static void debug_print_agent_state(struct udevice *dev, char *str)
 
 static struct sandbox_scmi_clk *get_scmi_clk_state(uint clock_id)
 {
-	size_t n;
-
-	for (n = 0; n < ARRAY_SIZE(scmi_clk); n++)
-		if (scmi_clk[n].id == clock_id)
-			return scmi_clk + n;
+	if (clock_id < ARRAY_SIZE(scmi_clk))
+		return scmi_clk + clock_id;
 
 	return NULL;
 }
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index d576b5fd89..795f207304 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -52,7 +52,7 @@ static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts,
 	ut_assertnonnull(scmi_ctx);
 	agent = scmi_ctx->agent;
 	ut_assertnonnull(agent);
-	ut_asserteq(2, agent->clk_count);
+	ut_asserteq(3, agent->clk_count);
 	ut_assertnonnull(agent->clk);
 	ut_asserteq(1, agent->reset_count);
 	ut_assertnonnull(agent->reset);
@@ -125,14 +125,19 @@ static int dm_test_scmi_clocks(struct unit_test_state *uts)
 	ut_assertnonnull(agent);
 
 	/* Test SCMI clocks rate manipulation */
+	ut_asserteq(333, agent->clk[0].rate);
+	ut_asserteq(200, agent->clk[1].rate);
+	ut_asserteq(1000, agent->clk[2].rate);
+
 	ut_asserteq(1000, clk_get_rate(&scmi_devices->clk[0]));
 	ut_asserteq(333, clk_get_rate(&scmi_devices->clk[1]));
 
 	ret_dev = clk_set_rate(&scmi_devices->clk[1], 1088);
 	ut_assert(!ret_dev || ret_dev == 1088);
 
-	ut_asserteq(1000, agent->clk[0].rate);
-	ut_asserteq(1088, agent->clk[1].rate);
+	ut_asserteq(1088, agent->clk[0].rate);
+	ut_asserteq(200, agent->clk[1].rate);
+	ut_asserteq(1000, agent->clk[2].rate);
 
 	ut_asserteq(1000, clk_get_rate(&scmi_devices->clk[0]));
 	ut_asserteq(1088, clk_get_rate(&scmi_devices->clk[1]));
@@ -148,8 +153,8 @@ static int dm_test_scmi_clocks(struct unit_test_state *uts)
 
 	ut_asserteq(0, clk_enable(&scmi_devices->clk[1]));
 
-	ut_assert(!agent->clk[0].enabled);
-	ut_assert(agent->clk[1].enabled);
+	ut_assert(agent->clk[0].enabled);
+	ut_assert(!agent->clk[1].enabled);
 	ut_assert(!agent->clk[2].enabled);
 
 	ut_assertok(clk_disable(&scmi_devices->clk[1]));
-- 
2.17.1

Re: [PATCH v2 4/5] firmware: scmi: fix sandbox and related tests for clock discovery

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

On Mon, Feb 21, 2022 at 09:22:41AM +0100, Etienne Carriere wrote:

> Updates sandbox SCMI clock driver and tests since enabling CCF will
> mandate clock discovery that is all exposed SCMI clocks shall be
> discovered at initialization. For this reason, sandbox SCMI clock
> driver must emulate all clocks exposed by SCMI server, not only those
> effectively consumed by some other U-Boot devices.
> 
> Therefore the sandbox SCMI test driver exposes 3 clocks (IDs 0, 1 and 2)
> and sandbox SCMI clock consumer driver gets 2 of them.
> 
> Cc: Simon Glass <sjg@chromium.org>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2 5/5] clk: scmi: register scmi clocks with CCF

[Etienne Carriere]

Implements SCMI APIs to retrieve the number exposed SCMI clocks using
SCMI_PROTOCOL_ATTRIBUTES messages and the names of the clocks using
SCMI_CLOCK_ATTRIBUTES messages.

This change updates sandbox SCMI clock test driver to manage these
2 new message IDs.

Cc: Lukasz Majewski <lukma@denx.de>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Clement Leger <clement.leger@bootlin.com>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes since v1:
- Remove Series-links tag and apply R-b tag.
---
 drivers/clk/clk_scmi.c                     | 90 ++++++++++++++++++++++
 drivers/firmware/scmi/sandbox-scmi_agent.c | 53 +++++++++++++
 include/scmi_protocols.h                   | 43 +++++++++++
 3 files changed, 186 insertions(+)

diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index 42fbab0d21..57022685e2 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -11,6 +11,52 @@
 #include <scmi_agent.h>
 #include <scmi_protocols.h>
 #include <asm/types.h>
+#include <linux/clk-provider.h>
+
+static int scmi_clk_get_num_clock(struct udevice *dev, size_t *num_clocks)
+{
+	struct scmi_clk_protocol_attr_out out;
+	struct scmi_msg msg = {
+		.protocol_id = SCMI_PROTOCOL_ID_CLOCK,
+		.message_id = SCMI_PROTOCOL_ATTRIBUTES,
+		.out_msg = (u8 *)&out,
+		.out_msg_sz = sizeof(out),
+	};
+	int ret;
+
+	ret = devm_scmi_process_msg(dev, &msg);
+	if (ret)
+		return ret;
+
+	*num_clocks = out.attributes & SCMI_CLK_PROTO_ATTR_COUNT_MASK;
+
+	return 0;
+}
+
+static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char **name)
+{
+	struct scmi_clk_attribute_in in = {
+		.clock_id = clkid,
+	};
+	struct scmi_clk_attribute_out out;
+	struct scmi_msg msg = {
+		.protocol_id = SCMI_PROTOCOL_ID_CLOCK,
+		.message_id = SCMI_CLOCK_ATTRIBUTES,
+		.in_msg = (u8 *)&in,
+		.in_msg_sz = sizeof(in),
+		.out_msg = (u8 *)&out,
+		.out_msg_sz = sizeof(out),
+	};
+	int ret;
+
+	ret = devm_scmi_process_msg(dev, &msg);
+	if (ret)
+		return ret;
+
+	*name = out.clock_name;
+
+	return 0;
+}
 
 static int scmi_clk_gate(struct clk *clk, int enable)
 {
@@ -88,6 +134,49 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
 	return scmi_clk_get_rate(clk);
 }
 
+static int scmi_clk_probe(struct udevice *dev)
+{
+	struct clk *clk;
+	size_t num_clocks, i;
+	int ret;
+
+	if (!CONFIG_IS_ENABLED(CLK_CCF))
+		return 0;
+
+	/* register CCF children: CLK UCLASS, no probed again */
+	if (device_get_uclass_id(dev->parent) == UCLASS_CLK)
+		return 0;
+
+	ret = scmi_clk_get_num_clock(dev, &num_clocks);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < num_clocks; i++) {
+		char *name;
+
+		if (!scmi_clk_get_attibute(dev, i, &name)) {
+			char *clock_name = strdup(name);
+
+			clk = kzalloc(sizeof(*clk), GFP_KERNEL);
+			if (!clk || !clock_name)
+				ret = -ENOMEM;
+			else
+				ret = clk_register(clk, dev->driver->name,
+						   clock_name, dev->name);
+
+			if (ret) {
+				free(clk);
+				free(clock_name);
+				return ret;
+			}
+
+			clk_dm(i, clk);
+		}
+	}
+
+	return 0;
+}
+
 static const struct clk_ops scmi_clk_ops = {
 	.enable = scmi_clk_enable,
 	.disable = scmi_clk_disable,
@@ -99,4 +188,5 @@ U_BOOT_DRIVER(scmi_clock) = {
 	.name = "scmi_clk",
 	.id = UCLASS_CLK,
 	.ops = &scmi_clk_ops,
+	.probe = &scmi_clk_probe,
 };
diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
index fc717a8aea..c555164d19 100644
--- a/drivers/firmware/scmi/sandbox-scmi_agent.c
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -114,6 +114,55 @@ static struct sandbox_scmi_voltd *get_scmi_voltd_state(uint domain_id)
  * Sandbox SCMI agent ops
  */
 
+static int sandbox_scmi_clock_protocol_attribs(struct udevice *dev,
+					       struct scmi_msg *msg)
+{
+	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;
+	out->attributes = ARRAY_SIZE(scmi_clk);
+	out->status = SCMI_SUCCESS;
+
+	return 0;
+}
+
+static int sandbox_scmi_clock_attribs(struct udevice *dev, struct scmi_msg *msg)
+{
+	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(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)
 {
@@ -427,6 +476,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/include/scmi_protocols.h b/include/scmi_protocols.h
index ef26e72176..a220cb2a91 100644
--- a/include/scmi_protocols.h
+++ b/include/scmi_protocols.h
@@ -40,22 +40,65 @@ enum scmi_status_code {
 	SCMI_PROTOCOL_ERROR = -10,
 };
 
+/*
+ * Generic message IDs
+ */
+enum scmi_discovery_id {
+	SCMI_PROTOCOL_VERSION = 0x0,
+	SCMI_PROTOCOL_ATTRIBUTES = 0x1,
+	SCMI_PROTOCOL_MESSAGE_ATTRIBUTES = 0x2,
+};
+
 /*
  * SCMI Clock Protocol
  */
 
 enum scmi_clock_message_id {
+	SCMI_CLOCK_ATTRIBUTES = 0x3,
 	SCMI_CLOCK_RATE_SET = 0x5,
 	SCMI_CLOCK_RATE_GET = 0x6,
 	SCMI_CLOCK_CONFIG_SET = 0x7,
 };
 
+#define SCMI_CLK_PROTO_ATTR_COUNT_MASK	GENMASK(15, 0)
 #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)
 
+#define SCMI_CLOCK_NAME_LENGTH_MAX 16
+
+/**
+ * struct scmi_clk_get_nb_out - Response for SCMI_PROTOCOL_ATTRIBUTES command
+ * @status:	SCMI command status
+ * @attributes:	Attributes of the clock protocol, mainly number of clocks exposed
+ */
+struct scmi_clk_protocol_attr_out {
+	s32 status;
+	u32 attributes;
+};
+
+/**
+ * struct scmi_clk_attribute_in - Message payload for SCMI_CLOCK_ATTRIBUTES command
+ * @clock_id:	SCMI clock ID
+ */
+struct scmi_clk_attribute_in {
+	u32 clock_id;
+};
+
+/**
+ * struct scmi_clk_get_nb_out - Response payload for SCMI_CLOCK_ATTRIBUTES command
+ * @status:	SCMI command status
+ * @attributes:	clock attributes
+ * @clock_name:	name of the clock
+ */
+struct scmi_clk_attribute_out {
+	s32 status;
+	u32 attributes;
+	char clock_name[SCMI_CLOCK_NAME_LENGTH_MAX];
+};
+
 /**
  * struct scmi_clk_state_in - Message payload for CLOCK_CONFIG_SET command
  * @clock_id:	SCMI clock ID
-- 
2.17.1

Re: [PATCH v2 5/5] clk: scmi: register scmi clocks with CCF

[Sean Anderson]

Hi Etienne,


On 2/21/22 3:22 AM, Etienne Carriere wrote:
> Implements SCMI APIs to retrieve the number exposed SCMI clocks using
> SCMI_PROTOCOL_ATTRIBUTES messages and the names of the clocks using
> SCMI_CLOCK_ATTRIBUTES messages.
> 
> This change updates sandbox SCMI clock test driver to manage these
> 2 new message IDs.
> 
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Sean Anderson <seanga2@gmail.com>
> Cc: Clement Leger <clement.leger@bootlin.com>
> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> Changes since v1:
> - Remove Series-links tag and apply R-b tag.
> ---
>   drivers/clk/clk_scmi.c                     | 90 ++++++++++++++++++++++
>   drivers/firmware/scmi/sandbox-scmi_agent.c | 53 +++++++++++++
>   include/scmi_protocols.h                   | 43 +++++++++++
>   3 files changed, 186 insertions(+)
> 
> diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
> index 42fbab0d21..57022685e2 100644
> --- a/drivers/clk/clk_scmi.c
> +++ b/drivers/clk/clk_scmi.c
> @@ -11,6 +11,52 @@
>   #include <scmi_agent.h>
>   #include <scmi_protocols.h>
>   #include <asm/types.h>
> +#include <linux/clk-provider.h>
> +
> +static int scmi_clk_get_num_clock(struct udevice *dev, size_t *num_clocks)
> +{
> +	struct scmi_clk_protocol_attr_out out;
> +	struct scmi_msg msg = {
> +		.protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> +		.message_id = SCMI_PROTOCOL_ATTRIBUTES,
> +		.out_msg = (u8 *)&out,
> +		.out_msg_sz = sizeof(out),
> +	};
> +	int ret;
> +
> +	ret = devm_scmi_process_msg(dev, &msg);
> +	if (ret)
> +		return ret;
> +
> +	*num_clocks = out.attributes & SCMI_CLK_PROTO_ATTR_COUNT_MASK;
> +
> +	return 0;
> +}
> +
> +static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char **name)

nit: scmi_clk_get_attribute

> +{
> +	struct scmi_clk_attribute_in in = {
> +		.clock_id = clkid,
> +	};
> +	struct scmi_clk_attribute_out out;
> +	struct scmi_msg msg = {
> +		.protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> +		.message_id = SCMI_CLOCK_ATTRIBUTES,
> +		.in_msg = (u8 *)&in,
> +		.in_msg_sz = sizeof(in),
> +		.out_msg = (u8 *)&out,
> +		.out_msg_sz = sizeof(out),
> +	};
> +	int ret;
> +
> +	ret = devm_scmi_process_msg(dev, &msg);
> +	if (ret)
> +		return ret;
> +
> +	*name = out.clock_name;
> +
> +	return 0;
> +}
>   
>   static int scmi_clk_gate(struct clk *clk, int enable)
>   {
> @@ -88,6 +134,49 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
>   	return scmi_clk_get_rate(clk);
>   }
>   
> +static int scmi_clk_probe(struct udevice *dev)
> +{
> +	struct clk *clk;
> +	size_t num_clocks, i;
> +	int ret;
> +
> +	if (!CONFIG_IS_ENABLED(CLK_CCF))
> +		return 0;
> +
> +	/* register CCF children: CLK UCLASS, no probed again */
> +	if (device_get_uclass_id(dev->parent) == UCLASS_CLK)
> +		return 0;
> +
> +	ret = scmi_clk_get_num_clock(dev, &num_clocks);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < num_clocks; i++) {
> +		char *name;
> +
> +		if (!scmi_clk_get_attibute(dev, i, &name)) {
> +			char *clock_name = strdup(name);
> +
> +			clk = kzalloc(sizeof(*clk), GFP_KERNEL);
> +			if (!clk || !clock_name)
> +				ret = -ENOMEM;
> +			else
> +				ret = clk_register(clk, dev->driver->name,
> +						   clock_name, dev->name);
> +
> +			if (ret) {
> +				free(clk);
> +				free(clock_name);
> +				return ret;
> +			}
> +
> +			clk_dm(i, clk);
> +		}
> +	}
> +
> +	return 0;
> +}
> +

So why not call scmi_clk_get_num_clock during probe() and then check against
it in xlate? I don't really see why we need CCF. This also means you don't
have a driver with copies of itself as children, and is lighter on memory.

--Sean

>   static const struct clk_ops scmi_clk_ops = {
>   	.enable = scmi_clk_enable,
>   	.disable = scmi_clk_disable,
> @@ -99,4 +188,5 @@ U_BOOT_DRIVER(scmi_clock) = {
>   	.name = "scmi_clk",
>   	.id = UCLASS_CLK,
>   	.ops = &scmi_clk_ops,
> +	.probe = &scmi_clk_probe,
>   };
> diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
> index fc717a8aea..c555164d19 100644
> --- a/drivers/firmware/scmi/sandbox-scmi_agent.c
> +++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
> @@ -114,6 +114,55 @@ static struct sandbox_scmi_voltd *get_scmi_voltd_state(uint domain_id)
>    * Sandbox SCMI agent ops
>    */
>   
> +static int sandbox_scmi_clock_protocol_attribs(struct udevice *dev,
> +					       struct scmi_msg *msg)
> +{
> +	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;
> +	out->attributes = ARRAY_SIZE(scmi_clk);
> +	out->status = SCMI_SUCCESS;
> +
> +	return 0;
> +}
> +
> +static int sandbox_scmi_clock_attribs(struct udevice *dev, struct scmi_msg *msg)
> +{
> +	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(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)
>   {
> @@ -427,6 +476,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/include/scmi_protocols.h b/include/scmi_protocols.h
> index ef26e72176..a220cb2a91 100644
> --- a/include/scmi_protocols.h
> +++ b/include/scmi_protocols.h
> @@ -40,22 +40,65 @@ enum scmi_status_code {
>   	SCMI_PROTOCOL_ERROR = -10,
>   };
>   
> +/*
> + * Generic message IDs
> + */
> +enum scmi_discovery_id {
> +	SCMI_PROTOCOL_VERSION = 0x0,
> +	SCMI_PROTOCOL_ATTRIBUTES = 0x1,
> +	SCMI_PROTOCOL_MESSAGE_ATTRIBUTES = 0x2,
> +};
> +
>   /*
>    * SCMI Clock Protocol
>    */
>   
>   enum scmi_clock_message_id {
> +	SCMI_CLOCK_ATTRIBUTES = 0x3,
>   	SCMI_CLOCK_RATE_SET = 0x5,
>   	SCMI_CLOCK_RATE_GET = 0x6,
>   	SCMI_CLOCK_CONFIG_SET = 0x7,
>   };
>   
> +#define SCMI_CLK_PROTO_ATTR_COUNT_MASK	GENMASK(15, 0)
>   #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)
>   
> +#define SCMI_CLOCK_NAME_LENGTH_MAX 16
> +
> +/**
> + * struct scmi_clk_get_nb_out - Response for SCMI_PROTOCOL_ATTRIBUTES command
> + * @status:	SCMI command status
> + * @attributes:	Attributes of the clock protocol, mainly number of clocks exposed
> + */
> +struct scmi_clk_protocol_attr_out {
> +	s32 status;
> +	u32 attributes;
> +};
> +
> +/**
> + * struct scmi_clk_attribute_in - Message payload for SCMI_CLOCK_ATTRIBUTES command
> + * @clock_id:	SCMI clock ID
> + */
> +struct scmi_clk_attribute_in {
> +	u32 clock_id;
> +};
> +
> +/**
> + * struct scmi_clk_get_nb_out - Response payload for SCMI_CLOCK_ATTRIBUTES command
> + * @status:	SCMI command status
> + * @attributes:	clock attributes
> + * @clock_name:	name of the clock
> + */
> +struct scmi_clk_attribute_out {
> +	s32 status;
> +	u32 attributes;
> +	char clock_name[SCMI_CLOCK_NAME_LENGTH_MAX];
> +};
> +
>   /**
>    * struct scmi_clk_state_in - Message payload for CLOCK_CONFIG_SET command
>    * @clock_id:	SCMI clock ID
> 

Re: [PATCH v2 5/5] clk: scmi: register scmi clocks with CCF

[Etienne Carriere]

Hello Sean,

Thanks for the feedback.
Sorry I missed your post end Feb.

On Fri, 25 Feb 2022 at 07:33, Sean Anderson <seanga2@gmail.com> wrote:
>
> Hi Etienne,
>
>
> On 2/21/22 3:22 AM, Etienne Carriere wrote:
> > Implements SCMI APIs to retrieve the number exposed SCMI clocks using
> > SCMI_PROTOCOL_ATTRIBUTES messages and the names of the clocks using
> > SCMI_CLOCK_ATTRIBUTES messages.
> >
> > This change updates sandbox SCMI clock test driver to manage these
> > 2 new message IDs.
> >
> > Cc: Lukasz Majewski <lukma@denx.de>
> > Cc: Sean Anderson <seanga2@gmail.com>
> > Cc: Clement Leger <clement.leger@bootlin.com>
> > Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
> > Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > ---
> > Changes since v1:
> > - Remove Series-links tag and apply R-b tag.
> > ---
> >   drivers/clk/clk_scmi.c                     | 90 ++++++++++++++++++++++
> >   drivers/firmware/scmi/sandbox-scmi_agent.c | 53 +++++++++++++
> >   include/scmi_protocols.h                   | 43 +++++++++++
> >   3 files changed, 186 insertions(+)
> >
> > diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
> > index 42fbab0d21..57022685e2 100644
> > --- a/drivers/clk/clk_scmi.c
> > +++ b/drivers/clk/clk_scmi.c
> > @@ -11,6 +11,52 @@
> >   #include <scmi_agent.h>
> >   #include <scmi_protocols.h>
> >   #include <asm/types.h>
> > +#include <linux/clk-provider.h>
> > +
> > +static int scmi_clk_get_num_clock(struct udevice *dev, size_t *num_clocks)
> > +{
> > +     struct scmi_clk_protocol_attr_out out;
> > +     struct scmi_msg msg = {
> > +             .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> > +             .message_id = SCMI_PROTOCOL_ATTRIBUTES,
> > +             .out_msg = (u8 *)&out,
> > +             .out_msg_sz = sizeof(out),
> > +     };
> > +     int ret;
> > +
> > +     ret = devm_scmi_process_msg(dev, &msg);
> > +     if (ret)
> > +             return ret;
> > +
> > +     *num_clocks = out.attributes & SCMI_CLK_PROTO_ATTR_COUNT_MASK;
> > +
> > +     return 0;
> > +}
> > +
> > +static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char **name)
>
> nit: scmi_clk_get_attribute

right!

>
> > +{
> > +     struct scmi_clk_attribute_in in = {
> > +             .clock_id = clkid,
> > +     };
> > +     struct scmi_clk_attribute_out out;
> > +     struct scmi_msg msg = {
> > +             .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> > +             .message_id = SCMI_CLOCK_ATTRIBUTES,
> > +             .in_msg = (u8 *)&in,
> > +             .in_msg_sz = sizeof(in),
> > +             .out_msg = (u8 *)&out,
> > +             .out_msg_sz = sizeof(out),
> > +     };
> > +     int ret;
> > +
> > +     ret = devm_scmi_process_msg(dev, &msg);
> > +     if (ret)
> > +             return ret;
> > +
> > +     *name = out.clock_name;
> > +
> > +     return 0;
> > +}
> >
> >   static int scmi_clk_gate(struct clk *clk, int enable)
> >   {
> > @@ -88,6 +134,49 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
> >       return scmi_clk_get_rate(clk);
> >   }
> >
> > +static int scmi_clk_probe(struct udevice *dev)
> > +{
> > +     struct clk *clk;
> > +     size_t num_clocks, i;
> > +     int ret;
> > +
> > +     if (!CONFIG_IS_ENABLED(CLK_CCF))
> > +             return 0;
> > +
> > +     /* register CCF children: CLK UCLASS, no probed again */
> > +     if (device_get_uclass_id(dev->parent) == UCLASS_CLK)
> > +             return 0;
> > +
> > +     ret = scmi_clk_get_num_clock(dev, &num_clocks);
> > +     if (ret)
> > +             return ret;
> > +
> > +     for (i = 0; i < num_clocks; i++) {
> > +             char *name;
> > +
> > +             if (!scmi_clk_get_attibute(dev, i, &name)) {
> > +                     char *clock_name = strdup(name);
> > +
> > +                     clk = kzalloc(sizeof(*clk), GFP_KERNEL);
> > +                     if (!clk || !clock_name)
> > +                             ret = -ENOMEM;
> > +                     else
> > +                             ret = clk_register(clk, dev->driver->name,
> > +                                                clock_name, dev->name);
> > +
> > +                     if (ret) {
> > +                             free(clk);
> > +                             free(clock_name);
> > +                             return ret;
> > +                     }
> > +
> > +                     clk_dm(i, clk);
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +
>
> So why not call scmi_clk_get_num_clock during probe() and then check against
> it in xlate? I don't really see why we need CCF. This also means you don't
> have a driver with copies of itself as children, and is lighter on memory.

There is no specific need for CCF for the scmi clock driver itself.
The goal of these changes is rather to allow platforms that do already
enable CCF (for an u-boot native clock driver)
to be able to also embed SCMI clocks support, for clocks controlled
from an external firmware.

Br,
etienne

>
> --Sean
>
> >   static const struct clk_ops scmi_clk_ops = {
> >       .enable = scmi_clk_enable,
> >       .disable = scmi_clk_disable,
> > @@ -99,4 +188,5 @@ U_BOOT_DRIVER(scmi_clock) = {
> >       .name = "scmi_clk",
> >       .id = UCLASS_CLK,
> >       .ops = &scmi_clk_ops,
> > +     .probe = &scmi_clk_probe,
> >   };
> > diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
> > index fc717a8aea..c555164d19 100644
> > --- a/drivers/firmware/scmi/sandbox-scmi_agent.c
> > +++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
> > @@ -114,6 +114,55 @@ static struct sandbox_scmi_voltd *get_scmi_voltd_state(uint domain_id)
> >    * Sandbox SCMI agent ops
> >    */
> >
> > +static int sandbox_scmi_clock_protocol_attribs(struct udevice *dev,
> > +                                            struct scmi_msg *msg)
> > +{
> > +     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;
> > +     out->attributes = ARRAY_SIZE(scmi_clk);
> > +     out->status = SCMI_SUCCESS;
> > +
> > +     return 0;
> > +}
> > +
> > +static int sandbox_scmi_clock_attribs(struct udevice *dev, struct scmi_msg *msg)
> > +{
> > +     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(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)
> >   {
> > @@ -427,6 +476,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/include/scmi_protocols.h b/include/scmi_protocols.h
> > index ef26e72176..a220cb2a91 100644
> > --- a/include/scmi_protocols.h
> > +++ b/include/scmi_protocols.h
> > @@ -40,22 +40,65 @@ enum scmi_status_code {
> >       SCMI_PROTOCOL_ERROR = -10,
> >   };
> >
> > +/*
> > + * Generic message IDs
> > + */
> > +enum scmi_discovery_id {
> > +     SCMI_PROTOCOL_VERSION = 0x0,
> > +     SCMI_PROTOCOL_ATTRIBUTES = 0x1,
> > +     SCMI_PROTOCOL_MESSAGE_ATTRIBUTES = 0x2,
> > +};
> > +
> >   /*
> >    * SCMI Clock Protocol
> >    */
> >
> >   enum scmi_clock_message_id {
> > +     SCMI_CLOCK_ATTRIBUTES = 0x3,
> >       SCMI_CLOCK_RATE_SET = 0x5,
> >       SCMI_CLOCK_RATE_GET = 0x6,
> >       SCMI_CLOCK_CONFIG_SET = 0x7,
> >   };
> >
> > +#define SCMI_CLK_PROTO_ATTR_COUNT_MASK       GENMASK(15, 0)
> >   #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)
> >
> > +#define SCMI_CLOCK_NAME_LENGTH_MAX 16
> > +
> > +/**
> > + * struct scmi_clk_get_nb_out - Response for SCMI_PROTOCOL_ATTRIBUTES command
> > + * @status:  SCMI command status
> > + * @attributes:      Attributes of the clock protocol, mainly number of clocks exposed
> > + */
> > +struct scmi_clk_protocol_attr_out {
> > +     s32 status;
> > +     u32 attributes;
> > +};
> > +
> > +/**
> > + * struct scmi_clk_attribute_in - Message payload for SCMI_CLOCK_ATTRIBUTES command
> > + * @clock_id:        SCMI clock ID
> > + */
> > +struct scmi_clk_attribute_in {
> > +     u32 clock_id;
> > +};
> > +
> > +/**
> > + * struct scmi_clk_get_nb_out - Response payload for SCMI_CLOCK_ATTRIBUTES command
> > + * @status:  SCMI command status
> > + * @attributes:      clock attributes
> > + * @clock_name:      name of the clock
> > + */
> > +struct scmi_clk_attribute_out {
> > +     s32 status;
> > +     u32 attributes;
> > +     char clock_name[SCMI_CLOCK_NAME_LENGTH_MAX];
> > +};
> > +
> >   /**
> >    * struct scmi_clk_state_in - Message payload for CLOCK_CONFIG_SET command
> >    * @clock_id:       SCMI clock ID
> >
>

Re: [PATCH v2 5/5] clk: scmi: register scmi clocks with CCF

[Sean Anderson]

On 3/7/22 5:17 AM, Etienne Carriere wrote:
> Hello Sean,
> 
> Thanks for the feedback.
> Sorry I missed your post end Feb.
> 
> On Fri, 25 Feb 2022 at 07:33, Sean Anderson <seanga2@gmail.com> wrote:
>>
>> Hi Etienne,
>>
>>
>> On 2/21/22 3:22 AM, Etienne Carriere wrote:
>>> Implements SCMI APIs to retrieve the number exposed SCMI clocks using
>>> SCMI_PROTOCOL_ATTRIBUTES messages and the names of the clocks using
>>> SCMI_CLOCK_ATTRIBUTES messages.
>>>
>>> This change updates sandbox SCMI clock test driver to manage these
>>> 2 new message IDs.
>>>
>>> Cc: Lukasz Majewski <lukma@denx.de>
>>> Cc: Sean Anderson <seanga2@gmail.com>
>>> Cc: Clement Leger <clement.leger@bootlin.com>
>>> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
>>> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
>>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
>>> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
>>> ---
>>> Changes since v1:
>>> - Remove Series-links tag and apply R-b tag.
>>> ---
>>>    drivers/clk/clk_scmi.c                     | 90 ++++++++++++++++++++++
>>>    drivers/firmware/scmi/sandbox-scmi_agent.c | 53 +++++++++++++
>>>    include/scmi_protocols.h                   | 43 +++++++++++
>>>    3 files changed, 186 insertions(+)
>>>
>>> diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
>>> index 42fbab0d21..57022685e2 100644
>>> --- a/drivers/clk/clk_scmi.c
>>> +++ b/drivers/clk/clk_scmi.c
>>> @@ -11,6 +11,52 @@
>>>    #include <scmi_agent.h>
>>>    #include <scmi_protocols.h>
>>>    #include <asm/types.h>
>>> +#include <linux/clk-provider.h>
>>> +
>>> +static int scmi_clk_get_num_clock(struct udevice *dev, size_t *num_clocks)
>>> +{
>>> +     struct scmi_clk_protocol_attr_out out;
>>> +     struct scmi_msg msg = {
>>> +             .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
>>> +             .message_id = SCMI_PROTOCOL_ATTRIBUTES,
>>> +             .out_msg = (u8 *)&out,
>>> +             .out_msg_sz = sizeof(out),
>>> +     };
>>> +     int ret;
>>> +
>>> +     ret = devm_scmi_process_msg(dev, &msg);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     *num_clocks = out.attributes & SCMI_CLK_PROTO_ATTR_COUNT_MASK;
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char **name)
>>
>> nit: scmi_clk_get_attribute
> 
> right!
> 
>>
>>> +{
>>> +     struct scmi_clk_attribute_in in = {
>>> +             .clock_id = clkid,
>>> +     };
>>> +     struct scmi_clk_attribute_out out;
>>> +     struct scmi_msg msg = {
>>> +             .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
>>> +             .message_id = SCMI_CLOCK_ATTRIBUTES,
>>> +             .in_msg = (u8 *)&in,
>>> +             .in_msg_sz = sizeof(in),
>>> +             .out_msg = (u8 *)&out,
>>> +             .out_msg_sz = sizeof(out),
>>> +     };
>>> +     int ret;
>>> +
>>> +     ret = devm_scmi_process_msg(dev, &msg);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     *name = out.clock_name;
>>> +
>>> +     return 0;
>>> +}
>>>
>>>    static int scmi_clk_gate(struct clk *clk, int enable)
>>>    {
>>> @@ -88,6 +134,49 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
>>>        return scmi_clk_get_rate(clk);
>>>    }
>>>
>>> +static int scmi_clk_probe(struct udevice *dev)
>>> +{
>>> +     struct clk *clk;
>>> +     size_t num_clocks, i;
>>> +     int ret;
>>> +
>>> +     if (!CONFIG_IS_ENABLED(CLK_CCF))
>>> +             return 0;
>>> +
>>> +     /* register CCF children: CLK UCLASS, no probed again */
>>> +     if (device_get_uclass_id(dev->parent) == UCLASS_CLK)
>>> +             return 0;
>>> +
>>> +     ret = scmi_clk_get_num_clock(dev, &num_clocks);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     for (i = 0; i < num_clocks; i++) {
>>> +             char *name;
>>> +
>>> +             if (!scmi_clk_get_attibute(dev, i, &name)) {
>>> +                     char *clock_name = strdup(name);
>>> +
>>> +                     clk = kzalloc(sizeof(*clk), GFP_KERNEL);
>>> +                     if (!clk || !clock_name)
>>> +                             ret = -ENOMEM;
>>> +                     else
>>> +                             ret = clk_register(clk, dev->driver->name,
>>> +                                                clock_name, dev->name);
>>> +
>>> +                     if (ret) {
>>> +                             free(clk);
>>> +                             free(clock_name);
>>> +                             return ret;
>>> +                     }
>>> +
>>> +                     clk_dm(i, clk);
>>> +             }
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +
>>
>> So why not call scmi_clk_get_num_clock during probe() and then check against
>> it in xlate? I don't really see why we need CCF. This also means you don't
>> have a driver with copies of itself as children, and is lighter on memory.
> 
> There is no specific need for CCF for the scmi clock driver itself.
> The goal of these changes is rather to allow platforms that do already
> enable CCF (for an u-boot native clock driver)
> to be able to also embed SCMI clocks support, for clocks controlled
> from an external firmware.

Sure, but you can still use non-CCF clocks in a CCF clock.

--Sean

Re: [PATCH v2 5/5] clk: scmi: register scmi clocks with CCF

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 771 bytes --]

On Mon, Feb 21, 2022 at 09:22:42AM +0100, Etienne Carriere wrote:

> Implements SCMI APIs to retrieve the number exposed SCMI clocks using
> SCMI_PROTOCOL_ATTRIBUTES messages and the names of the clocks using
> SCMI_CLOCK_ATTRIBUTES messages.
> 
> This change updates sandbox SCMI clock test driver to manage these
> 2 new message IDs.
> 
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Sean Anderson <seanga2@gmail.com>
> Cc: Clement Leger <clement.leger@bootlin.com>
> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 1/5] doc: binding: scmi: link to latest Linux kernel binding

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 395 bytes --]

On Mon, Feb 21, 2022 at 09:22:38AM +0100, Etienne Carriere wrote:

> Changes SCMI bindings documentation to relate to Linux kernel
> source tree that recently changed the bindings description to YAML
> format.
> 
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]