All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] firmware: scmi: voltage regulator
@ 2021-02-18 12:54 Etienne Carriere
  2021-02-18 12:54 ` [PATCH 2/3] firmware: scmi: sandbox test for " Etienne Carriere
  2021-02-18 12:54 ` [PATCH 3/3] firmware: scmi: fix inline comments and minor coding style issues Etienne Carriere
  0 siblings, 2 replies; 5+ messages in thread
From: Etienne Carriere @ 2021-02-18 12:54 UTC (permalink / raw)
  To: u-boot

Implement voltage regulators interfaced by the SCMI voltage domain
protocol. The DT bindings are defined in the Linux kernel since
SCMI voltage domain and regulators patches [1] and [2] integration
in v5.11-rc7.

Link: [1] https://github.com/torvalds/linux/commit/0f80fcec08e9c50b8d2992cf26495673765ebaba
Link: [2] https://github.com/torvalds/linux/commit/2add5cacff3531e54c50b0832128299faa9f0563

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
 doc/device-tree-bindings/arm/arm,scmi.txt |  34 +++++
 drivers/firmware/scmi/scmi_agent-uclass.c |  35 ++++-
 drivers/power/regulator/Kconfig           |   8 +
 drivers/power/regulator/Makefile          |   1 +
 drivers/power/regulator/scmi_regulator.c  | 170 ++++++++++++++++++++++
 include/scmi_protocols.h                  | 113 ++++++++++++++
 6 files changed, 359 insertions(+), 2 deletions(-)
 create mode 100644 drivers/power/regulator/scmi_regulator.c

diff --git a/doc/device-tree-bindings/arm/arm,scmi.txt b/doc/device-tree-bindings/arm/arm,scmi.txt
index 1f293ea24..a76124f4a 100644
--- a/doc/device-tree-bindings/arm/arm,scmi.txt
+++ b/doc/device-tree-bindings/arm/arm,scmi.txt
@@ -62,6 +62,20 @@ 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.
@@ -105,6 +119,7 @@ Required sub-node properties:
 [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:
 
@@ -169,6 +184,25 @@ firmware {
 			reg = <0x16>;
 			#reset-cells = <1>;
 		};
+
+		scmi_voltage: protocol at 17 {
+			reg = <0x17>;
+
+			regulators {
+				regulator_devX: regulator at 0 {
+					reg = <0x0>;
+					regulator-max-microvolt = <3300000>;
+				};
+
+				regulator_devY: regulator at 9 {
+					reg = <0x9>;
+					regulator-min-microvolt = <500000>;
+					regulator-max-microvolt = <4200000>;
+				};
+
+				...
+			};
+		};
 	};
 };
 
diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c
index 516e690ac..03d236426 100644
--- a/drivers/firmware/scmi/scmi_agent-uclass.c
+++ b/drivers/firmware/scmi/scmi_agent-uclass.c
@@ -50,6 +50,29 @@ int scmi_to_linux_errno(s32 scmi_code)
 	return -EPROTO;
 }
 
+static int regulator_devices_bind(struct udevice *dev, struct driver *drv,
+				  ofnode protocol_node)
+{
+	ofnode regulators_node;
+	ofnode node;
+	int ret;
+
+	regulators_node = ofnode_find_subnode(protocol_node, "regulators");
+	if (!ofnode_valid(regulators_node)) {
+		dev_err(dev, "regulators subnode not found\n");
+		return -ENXIO;
+	}
+
+	ofnode_for_each_subnode(node, regulators_node) {
+		ret = device_bind(dev, drv, ofnode_get_name(node), NULL, node,
+				  NULL);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 /*
  * SCMI agent devices binds devices of various uclasses depeding on
  * the FDT description. scmi_bind_protocol() is a generic bind sequence
@@ -79,6 +102,10 @@ static int scmi_bind_protocols(struct udevice *dev)
 			if (IS_ENABLED(CONFIG_RESET_SCMI))
 				drv = DM_DRIVER_GET(scmi_reset_domain);
 			break;
+		case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN:
+			if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI))
+				drv = DM_DRIVER_GET(scmi_voltage_domain);
+			break;
 		default:
 			break;
 		}
@@ -89,8 +116,12 @@ static int scmi_bind_protocols(struct udevice *dev)
 			continue;
 		}
 
-		ret = device_bind(dev, drv, ofnode_get_name(node), NULL, node,
-				  NULL);
+		if (protocol_id == SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN)
+			ret = regulator_devices_bind(dev, drv, node);
+		else
+			ret = device_bind(dev, drv, ofnode_get_name(node),
+					  NULL, node, NULL);
+
 		if (ret)
 			break;
 	}
diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig
index fbbea18c7..5d6180229 100644
--- a/drivers/power/regulator/Kconfig
+++ b/drivers/power/regulator/Kconfig
@@ -353,3 +353,11 @@ config DM_REGULATOR_TPS65941
 	TPS65941 series of PMICs have 5 single phase BUCKs that can also
 	be configured in multi phase modes & 4 LDOs. The driver implements
 	get/set api for value and enable.
+
+config DM_REGULATOR_SCMI
+	bool "Enable driver for SCMI voltage domain regulators"
+	depends on DM_REGULATOR
+	select SCMI_AGENT
+        help
+          Enable this option if you want to support regulators exposed through
+	  the SCMI voltage domain protocol by a SCMI server.
diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile
index 9d58112dc..b2f5972ea 100644
--- a/drivers/power/regulator/Makefile
+++ b/drivers/power/regulator/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_DM_REGULATOR_TPS65910) += tps65910_regulator.o
 obj-$(CONFIG_DM_REGULATOR_TPS62360) += tps62360_regulator.o
 obj-$(CONFIG_$(SPL_)DM_REGULATOR_STPMIC1) += stpmic1.o
 obj-$(CONFIG_DM_REGULATOR_TPS65941) += tps65941_regulator.o
+obj-$(CONFIG_DM_REGULATOR_SCMI) += scmi_regulator.o
diff --git a/drivers/power/regulator/scmi_regulator.c b/drivers/power/regulator/scmi_regulator.c
new file mode 100644
index 000000000..072a978a8
--- /dev/null
+++ b/drivers/power/regulator/scmi_regulator.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020-2021 Linaro Limited
+ */
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <scmi_agent.h>
+#include <scmi_protocols.h>
+#include <asm/types.h>
+#include <dm/device.h>
+#include <dm/device_compat.h>
+#include <linux/kernel.h>
+#include <power/regulator.h>
+
+/**
+ * struct scmi_regulator_platdata - Platform data for a scmi voltage domain regulator
+ * @domain_id: ID representing the regulator for the related SCMI agent
+ */
+struct scmi_regulator_platdata {
+	u32 domain_id;
+};
+
+static int scmi_voltd_set_enable(struct udevice *dev, bool enable)
+{
+	struct scmi_regulator_platdata *pdata = dev_get_plat(dev);
+	struct scmi_voltd_config_set_in in = {
+		.domain_id = pdata->domain_id,
+		.config = enable ? SCMI_VOLTD_CONFIG_ON : SCMI_VOLTD_CONFIG_OFF,
+	};
+	struct scmi_voltd_config_set_out out;
+	struct scmi_msg msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN,
+					  SCMI_VOLTAGE_DOMAIN_CONFIG_SET,
+					  in, out);
+	int ret;
+
+	ret = devm_scmi_process_msg(dev->parent, &msg);
+	if (ret)
+		return ret;
+
+	ret = scmi_to_linux_errno(out.status);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
+static int scmi_voltd_get_enable(struct udevice *dev)
+{
+	struct scmi_regulator_platdata *pdata = dev_get_plat(dev);
+	struct scmi_voltd_config_get_in in = {
+		.domain_id = pdata->domain_id,
+	};
+	struct scmi_voltd_config_get_out out;
+	struct scmi_msg msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN,
+					  SCMI_VOLTAGE_DOMAIN_CONFIG_GET,
+					  in, out);
+	int ret;
+
+	ret = devm_scmi_process_msg(dev->parent, &msg);
+	if (ret < 0)
+		return ret;
+
+	ret = scmi_to_linux_errno(out.status);
+	if (ret < 0)
+		return ret;
+
+	return out.config == SCMI_VOLTD_CONFIG_ON;
+}
+
+static int scmi_voltd_set_voltage_level(struct udevice *dev, int uV)
+{
+	struct scmi_regulator_platdata *pdata = dev_get_plat(dev);
+	struct scmi_voltd_level_set_in in = {
+		.domain_id = pdata->domain_id,
+		.voltage_level = uV,
+	};
+	struct scmi_voltd_level_set_out out;
+	struct scmi_msg msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN,
+					  SCMI_VOLTAGE_DOMAIN_LEVEL_SET,
+					  in, out);
+	int ret;
+
+	ret = devm_scmi_process_msg(dev->parent, &msg);
+	if (ret < 0)
+		return ret;
+
+	return scmi_to_linux_errno(out.status);
+}
+
+static int scmi_voltd_get_voltage_level(struct udevice *dev)
+{
+	struct scmi_regulator_platdata *pdata = dev_get_plat(dev);
+	struct scmi_voltd_level_get_in in = {
+		.domain_id = pdata->domain_id,
+	};
+	struct scmi_voltd_level_get_out out;
+	struct scmi_msg msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN,
+					  SCMI_VOLTAGE_DOMAIN_LEVEL_GET,
+					  in, out);
+	int ret;
+
+	ret = devm_scmi_process_msg(dev->parent, &msg);
+	if (ret < 0)
+		return ret;
+
+	ret = scmi_to_linux_errno(out.status);
+	if (ret < 0)
+		return ret;
+
+	return out.voltage_level;
+}
+
+static int scmi_regulator_of_to_plat(struct udevice *dev)
+{
+	struct scmi_regulator_platdata *pdata = dev_get_plat(dev);
+	fdt_addr_t reg;
+
+	reg = dev_read_addr(dev);
+	if (reg == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	pdata->domain_id = (u32)reg;
+
+	return 0;
+}
+
+static int scmi_regulator_probe(struct udevice *dev)
+{
+	struct scmi_regulator_platdata *pdata = dev_get_plat(dev);
+	struct scmi_voltd_attr_in in = { 0 };
+	struct scmi_voltd_attr_out out = { 0 };
+	struct scmi_msg scmi_msg = {
+		.protocol_id = SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN,
+		.message_id = SCMI_VOLTAGE_DOMAIN_ATTRIBUTES,
+		.in_msg = (u8 *)&in,
+		.in_msg_sz = sizeof(in),
+		.out_msg = (u8 *)&out,
+		.out_msg_sz = sizeof(out),
+	};
+	int ret;
+
+	/* Check voltage domain is known from SCMI server */
+	in.domain_id = pdata->domain_id;
+
+	ret = devm_scmi_process_msg(dev->parent, &scmi_msg);
+	if (ret) {
+		dev_err(dev, "Failed to query voltage domain %u: %d\n",
+			pdata->domain_id, ret);
+		return -ENXIO;
+	}
+
+	return 0;
+}
+
+static const struct dm_regulator_ops scmi_voltd_ops = {
+	.get_value  = scmi_voltd_get_voltage_level,
+	.set_value  = scmi_voltd_set_voltage_level,
+	.get_enable = scmi_voltd_get_enable,
+	.set_enable = scmi_voltd_set_enable,
+};
+
+U_BOOT_DRIVER(scmi_voltage_domain) = {
+	.name = "scmi_voltage_domain",
+	.id = UCLASS_REGULATOR,
+	.ops = &scmi_voltd_ops,
+	.probe = scmi_regulator_probe,
+	.of_to_plat = scmi_regulator_of_to_plat,
+	.plat_auto = sizeof(struct scmi_regulator_platdata),
+};
diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h
index ccab97c96..2db71697e 100644
--- a/include/scmi_protocols.h
+++ b/include/scmi_protocols.h
@@ -23,6 +23,7 @@ enum scmi_std_protocol {
 	SCMI_PROTOCOL_ID_CLOCK = 0x14,
 	SCMI_PROTOCOL_ID_SENSOR = 0x15,
 	SCMI_PROTOCOL_ID_RESET_DOMAIN = 0x16,
+	SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN = 0x17,
 };
 
 enum scmi_status_code {
@@ -176,4 +177,116 @@ struct scmi_rd_reset_out {
 	s32 status;
 };
 
+/*
+ * SCMI Voltage Domain Protocol
+ */
+
+enum scmi_voltage_domain_message_id {
+	SCMI_VOLTAGE_DOMAIN_ATTRIBUTES = 0x3,
+	SCMI_VOLTAGE_DOMAIN_CONFIG_SET = 0x5,
+	SCMI_VOLTAGE_DOMAIN_CONFIG_GET = 0x6,
+	SCMI_VOLTAGE_DOMAIN_LEVEL_SET = 0x7,
+	SCMI_VOLTAGE_DOMAIN_LEVEL_GET = 0x8,
+};
+
+#define SCMI_VOLTD_NAME_LEN		16
+
+#define SCMI_VOLTD_CONFIG_MASK		GENMASK(3, 0)
+#define SCMI_VOLTD_CONFIG_OFF		0
+#define SCMI_VOLTD_CONFIG_ON		0x7
+
+/**
+ * struct scmi_voltd_attr_in - Payload for VOLTAGE_DOMAIN_ATTRIBUTES message
+ * @domain_id:	SCMI voltage domain ID
+ */
+struct scmi_voltd_attr_in {
+	u32 domain_id;
+};
+
+/**
+ * struct scmi_voltd_attr_out - Payload for VOLTAGE_DOMAIN_ATTRIBUTES response
+ * @status:	SCMI command status
+ * @attributes:	Retrieved attributes of the voltage domain
+ * @name:	Voltage domain name
+ */
+struct scmi_voltd_attr_out {
+	s32 status;
+	u32 attributes;
+	char name[SCMI_VOLTD_NAME_LEN];
+};
+
+/**
+ * struct scmi_voltd_config_set_in - Message payload for VOLTAGE_CONFIG_SET cmd
+ * @domain_id:	SCMI voltage domain ID
+ * @config:	Configuration data of the voltage domain
+ */
+struct scmi_voltd_config_set_in {
+	u32 domain_id;
+	u32 config;
+};
+
+/**
+ * struct scmi_voltd_config_set_out - Response for VOLTAGE_CONFIG_SET command
+ * @status:	SCMI command status
+ */
+struct scmi_voltd_config_set_out {
+	s32 status;
+};
+
+/**
+ * struct scmi_voltd_config_get_in - Message payload for VOLTAGE_CONFIG_GET cmd
+ * @domain_id:	SCMI voltage domain ID
+ */
+struct scmi_voltd_config_get_in {
+	u32 domain_id;
+};
+
+/**
+ * struct scmi_voltd_config_get_out - Response for VOLTAGE_CONFIG_GET command
+ * @status:	SCMI command status
+ * @config:	Configuration data of the voltage domain
+ */
+struct scmi_voltd_config_get_out {
+	s32 status;
+	u32 config;
+};
+
+/**
+ * struct scmi_voltd_level_set_in - Message payload for VOLTAGE_LEVEL_SET cmd
+ * @domain_id:		SCMI voltage domain ID
+ * @flags:		Parameter flags for configuring target level
+ * @voltage_level:	Target voltage level in microvolts (uV)
+ */
+struct scmi_voltd_level_set_in {
+	u32 domain_id;
+	u32 flags;
+	s32 voltage_level;
+};
+
+/**
+ * struct scmi_voltd_level_set_out - Response for VOLTAGE_LEVEL_SET command
+ * @status:	SCMI	command status
+ */
+struct scmi_voltd_level_set_out {
+	s32 status;
+};
+
+/**
+ * struct scmi_voltd_level_get_in - Message payload for VOLTAGE_LEVEL_GET cmd
+ * @domain_id:		SCMI voltage domain ID
+ */
+struct scmi_voltd_level_get_in {
+	u32 domain_id;
+};
+
+/**
+ * struct scmi_voltd_level_get_out - Response for VOLTAGE_LEVEL_GET command
+ * @status:		SCMI command status
+ * @voltage_level:	Voltage level in microvolts (uV)
+ */
+struct scmi_voltd_level_get_out {
+	s32 status;
+	s32 voltage_level;
+};
+
 #endif /* _SCMI_PROTOCOLS_H */
-- 
2.17.1

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

* [PATCH 2/3] firmware: scmi: sandbox test for voltage regulator
  2021-02-18 12:54 [PATCH 1/3] firmware: scmi: voltage regulator Etienne Carriere
@ 2021-02-18 12:54 ` Etienne Carriere
  2021-02-18 12:54 ` [PATCH 3/3] firmware: scmi: fix inline comments and minor coding style issues Etienne Carriere
  1 sibling, 0 replies; 5+ messages in thread
From: Etienne Carriere @ 2021-02-18 12:54 UTC (permalink / raw)
  To: u-boot

Implement sandbox regulator devices for SCMI voltage domains
and test them in DM scmi tests.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
 arch/sandbox/dts/test.dts                    |  23 +++
 arch/sandbox/include/asm/scmi_test.h         |  20 ++
 configs/sandbox_defconfig                    |   1 +
 drivers/firmware/scmi/sandbox-scmi_agent.c   | 203 ++++++++++++++++++-
 drivers/firmware/scmi/sandbox-scmi_devices.c |  25 ++-
 test/dm/scmi.c                               |  64 ++++++
 6 files changed, 333 insertions(+), 3 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index e95f4631b..14d6983e7 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -451,6 +451,27 @@
 				reg = <0x16>;
 				#reset-cells = <1>;
 			};
+
+			protocol at 17 {
+				reg = <0x17>;
+
+				regulators {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					regul0_scmi0: reg at 0 {
+						reg = <0>;
+						regulator-name = "sandbox-voltd0";
+						regulator-min-microvolt = <1100000>;
+						regulator-max-microvolt = <3300000>;
+					};
+					regul1_scmi0: reg at 1 {
+						reg = <0x1>;
+						regulator-name = "sandbox-voltd1";
+						regulator-min-microvolt = <1800000>;
+					};
+				};
+			};
 		};
 
 		sandbox-scmi-agent at 1 {
@@ -1217,6 +1238,8 @@
 		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>;
 	};
 
 	pinctrl {
diff --git a/arch/sandbox/include/asm/scmi_test.h b/arch/sandbox/include/asm/scmi_test.h
index 3e8b0068f..9b7031531 100644
--- a/arch/sandbox/include/asm/scmi_test.h
+++ b/arch/sandbox/include/asm/scmi_test.h
@@ -31,6 +31,18 @@ struct sandbox_scmi_reset {
 	bool asserted;
 };
 
+/**
+ * struct sandbox_scmi_voltd - Simulated voltage regulator exposed by SCMI
+ * @id:		Identifier of the voltage domain used in the SCMI protocol
+ * @enabled:	Regulator state: true if on, false if off
+ * @voltage_uv:	Regulator current voltage in microvoltd (uV)
+ */
+struct sandbox_scmi_voltd {
+	uint id;
+	bool enabled;
+	int voltage_uv;
+};
+
 /**
  * struct sandbox_scmi_agent - Simulated SCMI service seen by SCMI agent
  * @idx:	Identifier for the SCMI agent, its index
@@ -38,6 +50,8 @@ struct sandbox_scmi_reset {
  * @clk_count:	Simulated clocks array size
  * @clk:	Simulated reset domains
  * @clk_count:	Simulated reset domains array size
+ * @voltd:	 Simulated voltage domains (regulators)
+ * @voltd_count: Simulated voltage domains array size
  */
 struct sandbox_scmi_agent {
 	uint idx;
@@ -45,6 +59,8 @@ struct sandbox_scmi_agent {
 	size_t clk_count;
 	struct sandbox_scmi_reset *reset;
 	size_t reset_count;
+	struct sandbox_scmi_voltd *voltd;
+	size_t voltd_count;
 };
 
 /**
@@ -63,12 +79,16 @@ struct sandbox_scmi_service {
  * @clk_count:		Number of clock devices probed
  * @reset:		Array the reset controller devices
  * @reset_count:	Number of reset controller devices probed
+ * @regul:		Array regulator devices
+ * @regul_count:	Number of regulator devices probed
  */
 struct sandbox_scmi_devices {
 	struct clk *clk;
 	size_t clk_count;
 	struct reset_ctl *reset;
 	size_t reset_count;
+	struct udevice **regul;
+	size_t regul_count;
 };
 
 #ifdef CONFIG_SCMI_FIRMWARE
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 0c7674efc..790158445 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -214,6 +214,7 @@ CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_REGULATOR_RK8XX=y
 CONFIG_REGULATOR_S5M8767=y
 CONFIG_DM_REGULATOR_SANDBOX=y
+CONFIG_DM_REGULATOR_SCMI=y
 CONFIG_REGULATOR_TPS65090=y
 CONFIG_DM_PWM=y
 CONFIG_PWM_SANDBOX=y
diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
index 35de68c75..3eafc49bd 100644
--- a/drivers/firmware/scmi/sandbox-scmi_agent.c
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -19,14 +19,15 @@
  * SCMI protocols embedded in U-Boot. Currently:
  * - SCMI clock protocol: emulate 2 agents each exposing few clocks
  * - SCMI reset protocol: emulate 1 agents each exposing a reset
+ * - SCMI voltage domain protocol: emulate 1 agent exposing 2 regulators
  *
- * Agent #0 simulates 2 clocks and 1 reset domain.
+ * Agent #0 simulates 2 clocks, 1 reset domain and 1 voltage domain.
  * See IDs in scmi0_clk[]/scmi0_reset[] and "sandbox-scmi-agent at 0" in test.dts.
  *
  * Agent #1 simulates 1 clock.
  * See IDs in scmi1_clk[] and "sandbox-scmi-agent at 1" in test.dts.
  *
- * All clocks are default disabled and reset levels down.
+ * All clocks and regulators are default disabled and reset controller down.
  *
  * This Driver exports sandbox_scmi_service_ct() for the test sequence to
  * get the state of the simulated services (clock state, rate, ...) and
@@ -45,6 +46,11 @@ static struct sandbox_scmi_reset scmi0_reset[] = {
 	{ .id = 3 },
 };
 
+static struct sandbox_scmi_voltd scmi0_voltd[] = {
+	{ .id = 0, .voltage_uv = 3300000 },
+	{ .id = 1, .voltage_uv = 1800000 },
+};
+
 static struct sandbox_scmi_clk scmi1_clk[] = {
 	{ .id = 1, .rate = 44 },
 };
@@ -81,6 +87,13 @@ static void debug_print_agent_state(struct udevice *dev, char *str)
 		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,
+		agent->voltd_count,
+		agent->voltd_count ? agent->voltd[0].enabled : -1,
+		agent->voltd_count ? agent->voltd[0].voltage_uv : -1,
+		agent->voltd_count ? agent->voltd[1].enabled : -1,
+		agent->voltd_count ? agent->voltd[1].voltage_uv : -1);
 };
 
 static struct sandbox_scmi_clk *get_scmi_clk_state(uint agent_id, uint clock_id)
@@ -123,6 +136,20 @@ static struct sandbox_scmi_reset *get_scmi_reset_state(uint agent_id,
 	return NULL;
 }
 
+static struct sandbox_scmi_voltd *get_scmi_voltd_state(uint agent_id,
+						       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;
+	}
+
+	return NULL;
+}
+
 /*
  * Sandbox SCMI agent ops
  */
@@ -290,6 +317,160 @@ static int sandbox_scmi_rd_reset(struct udevice *dev, struct scmi_msg *msg)
 	return 0;
 }
 
+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;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	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);
+	if (!voltd_state) {
+		dev_err(dev, "Unexpected domain ID %u\n", in->domain_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else {
+		memset(out, 0, sizeof(*out));
+		snprintf(out->name, sizeof(out->name), "regu%u", in->domain_id);
+
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
+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;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	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);
+	if (!voltd_state) {
+		dev_err(dev, "Unexpected domain ID %u\n", in->domain_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else if (in->config & ~SCMI_VOLTD_CONFIG_MASK) {
+		dev_err(dev, "Invalid config value 0x%x\n", in->config);
+
+		out->status = SCMI_INVALID_PARAMETERS;
+	} else if (in->config != SCMI_VOLTD_CONFIG_ON &&
+		   in->config != SCMI_VOLTD_CONFIG_OFF) {
+		dev_err(dev, "Unexpected custom value 0x%x\n", in->config);
+
+		out->status = SCMI_INVALID_PARAMETERS;
+	} else {
+		voltd_state->enabled = in->config == SCMI_VOLTD_CONFIG_ON;
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
+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;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	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);
+	if (!voltd_state) {
+		dev_err(dev, "Unexpected domain ID %u\n", in->domain_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else {
+		if (voltd_state->enabled)
+			out->config = SCMI_VOLTD_CONFIG_ON;
+		else
+			out->config = SCMI_VOLTD_CONFIG_OFF;
+
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
+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;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	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);
+	if (!voltd_state) {
+		dev_err(dev, "Unexpected domain ID %u\n", in->domain_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else {
+		voltd_state->voltage_uv = in->voltage_level;
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
+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;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	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);
+	if (!voltd_state) {
+		dev_err(dev, "Unexpected domain ID %u\n", in->domain_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else {
+		out->voltage_level = voltd_state->voltage_uv;
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
 static int sandbox_scmi_test_process_msg(struct udevice *dev,
 					 struct scmi_msg *msg)
 {
@@ -316,6 +497,22 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev,
 			break;
 		}
 		break;
+	case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN:
+		switch (msg->message_id) {
+		case SCMI_VOLTAGE_DOMAIN_ATTRIBUTES:
+			return sandbox_scmi_voltd_attribs(dev, msg);
+		case SCMI_VOLTAGE_DOMAIN_CONFIG_SET:
+			return sandbox_scmi_voltd_config_set(dev, msg);
+		case SCMI_VOLTAGE_DOMAIN_CONFIG_GET:
+			return sandbox_scmi_voltd_config_get(dev, msg);
+		case SCMI_VOLTAGE_DOMAIN_LEVEL_SET:
+			return sandbox_scmi_voltd_level_set(dev, msg);
+		case SCMI_VOLTAGE_DOMAIN_LEVEL_GET:
+			return sandbox_scmi_voltd_level_get(dev, msg);
+		default:
+			break;
+		}
+		break;
 	case SCMI_PROTOCOL_ID_BASE:
 	case SCMI_PROTOCOL_ID_POWER_DOMAIN:
 	case SCMI_PROTOCOL_ID_SYSTEM:
@@ -367,6 +564,8 @@ static int sandbox_scmi_test_probe(struct udevice *dev)
 			.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':
diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c
index 1a6fafbf5..aab5782fd 100644
--- a/drivers/firmware/scmi/sandbox-scmi_devices.c
+++ b/drivers/firmware/scmi/sandbox-scmi_devices.c
@@ -1,16 +1,18 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (C) 2020, Linaro Limited
+ * Copyright (C) 2020-2021, Linaro Limited
  */
 
 #include <common.h>
 #include <clk.h>
 #include <dm.h>
+#include <log.h>
 #include <malloc.h>
 #include <reset.h>
 #include <asm/io.h>
 #include <asm/scmi_test.h>
 #include <dm/device_compat.h>
+#include <power/regulator.h>
 
 /*
  * Simulate to some extent a SCMI exchange.
@@ -21,16 +23,19 @@
 
 #define SCMI_TEST_DEVICES_CLK_COUNT		3
 #define SCMI_TEST_DEVICES_RD_COUNT		1
+#define SCMI_TEST_DEVICES_VOLTD_COUNT		2
 
 /*
  * struct sandbox_scmi_device_priv - Storage for device handles used by test
  * @clk:		Array of clock instances used by tests
  * @reset_clt:		Array of the reset controller instances used by tests
+ * @regulators:		Array of regulator device references used by the tests
  * @devices:		Resources exposed by sandbox_scmi_devices_ctx()
  */
 struct sandbox_scmi_device_priv {
 	struct clk clk[SCMI_TEST_DEVICES_CLK_COUNT];
 	struct reset_ctl reset_ctl[SCMI_TEST_DEVICES_RD_COUNT];
+	struct udevice *regulators[SCMI_TEST_DEVICES_VOLTD_COUNT];
 	struct sandbox_scmi_devices devices;
 };
 
@@ -74,6 +79,8 @@ static int sandbox_scmi_devices_probe(struct udevice *dev)
 		.clk_count = SCMI_TEST_DEVICES_CLK_COUNT,
 		.reset = priv->reset_ctl,
 		.reset_count = SCMI_TEST_DEVICES_RD_COUNT,
+		.regul = priv->regulators,
+		.regul_count = SCMI_TEST_DEVICES_VOLTD_COUNT,
 	};
 
 	for (n = 0; n < SCMI_TEST_DEVICES_CLK_COUNT; n++) {
@@ -92,8 +99,24 @@ static int sandbox_scmi_devices_probe(struct udevice *dev)
 		}
 	}
 
+	for (n = 0; n < SCMI_TEST_DEVICES_VOLTD_COUNT; n++) {
+		char name[32];
+
+		ret = snprintf(name, sizeof(name), "regul%zu-supply", n);
+		assert(ret >= 0 && ret < sizeof(name));
+
+		ret = device_get_supply_regulator(dev, name,
+						  priv->devices.regul + n);
+		if (ret) {
+			dev_err(dev, "%s: Failed on voltd %zu\n", __func__, n);
+			goto err_regul;
+		}
+	}
+
 	return 0;
 
+err_regul:
+	n = SCMI_TEST_DEVICES_RD_COUNT;
 err_reset:
 	for (; n > 0; n--)
 		reset_free(priv->devices.reset + n - 1);
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index be60b44b3..f21a3ff97 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -20,6 +20,7 @@
 #include <dm/device-internal.h>
 #include <dm/test.h>
 #include <linux/kconfig.h>
+#include <power/regulator.h>
 #include <test/ut.h>
 
 static int ut_assert_scmi_state_preprobe(struct unit_test_state *uts)
@@ -47,6 +48,8 @@ static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts,
 		ut_asserteq(3, scmi_devices->clk_count);
 	if (IS_ENABLED(CONFIG_RESET_SCMI))
 		ut_asserteq(1, scmi_devices->reset_count);
+	if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI))
+		ut_asserteq(2, scmi_devices->regul_count);
 
 	/* State of the simulated SCMI server exposed */
 	scmi_ctx = sandbox_scmi_service_ctx();
@@ -58,6 +61,10 @@ static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts,
 	ut_assertnonnull(scmi_ctx->agent[0]->clk);
 	ut_asserteq(1, scmi_ctx->agent[0]->reset_count);
 	ut_assertnonnull(scmi_ctx->agent[0]->reset);
+	if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI)) {
+		ut_asserteq(2, scmi_ctx->agent[0]->voltd_count);
+		ut_assertnonnull(scmi_ctx->agent[0]->voltd);
+	}
 
 	ut_assertnonnull(scmi_ctx->agent[1]);
 	ut_assertnonnull(scmi_ctx->agent[1]->clk);
@@ -201,3 +208,60 @@ static int dm_test_scmi_resets(struct unit_test_state *uts)
 }
 
 DM_TEST(dm_test_scmi_resets, UT_TESTF_SCAN_FDT);
+
+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_voltd *sandbox_voltd;
+	struct dm_regulator_uclass_plat *uc_pdata;
+	struct udevice *dev;
+	struct udevice *regul_dev;
+
+	if (!IS_ENABLED(CONFIG_DM_REGULATOR_SCMI))
+		return 0;
+
+	ut_assertok(load_sandbox_scmi_test_devices(uts, &dev));
+
+	scmi_devices = sandbox_scmi_devices_ctx(dev);
+	scmi_ctx = sandbox_scmi_service_ctx();
+
+	/* Set/Get an SCMI voltage domain level */
+	sandbox_voltd = &scmi_ctx->agent[0]->voltd[0];
+	regul_dev = scmi_devices->regul[0];
+	ut_assert(regul_dev);
+
+	uc_pdata = dev_get_uclass_plat(regul_dev);
+	ut_assert(uc_pdata);
+
+	ut_assertok(regulator_set_value(regul_dev, uc_pdata->min_uV));
+	ut_asserteq(sandbox_voltd->voltage_uv, uc_pdata->min_uV);
+
+	ut_assert(regulator_get_value(regul_dev) == uc_pdata->min_uV);
+
+	ut_assertok(regulator_set_value(regul_dev, uc_pdata->max_uV));
+	ut_asserteq(sandbox_voltd->voltage_uv, uc_pdata->max_uV);
+
+	ut_assert(regulator_get_value(regul_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(!scmi_ctx->agent[0]->voltd[0].enabled);
+	ut_assert(!scmi_ctx->agent[0]->voltd[1].enabled);
+
+	ut_assertok(regulator_set_enable(scmi_devices->regul[0], true));
+	ut_assert(scmi_ctx->agent[0]->voltd[0].enabled);
+	ut_assert(!scmi_ctx->agent[0]->voltd[1].enabled);
+
+	ut_assertok(regulator_set_enable(scmi_devices->regul[1], true));
+	ut_assert(scmi_ctx->agent[0]->voltd[0].enabled);
+	ut_assert(scmi_ctx->agent[0]->voltd[1].enabled);
+
+	ut_assertok(regulator_set_enable(scmi_devices->regul[0], false));
+	ut_assert(!scmi_ctx->agent[0]->voltd[0].enabled);
+	ut_assert(scmi_ctx->agent[0]->voltd[1].enabled);
+
+	return release_sandbox_scmi_test_devices(uts, dev);
+}
+DM_TEST(dm_test_scmi_voltage_domains, UT_TESTF_SCAN_FDT);
-- 
2.17.1

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

* [PATCH 3/3] firmware: scmi: fix inline comments and minor coding style issues
  2021-02-18 12:54 [PATCH 1/3] firmware: scmi: voltage regulator Etienne Carriere
  2021-02-18 12:54 ` [PATCH 2/3] firmware: scmi: sandbox test for " Etienne Carriere
@ 2021-02-18 12:54 ` Etienne Carriere
  2021-02-19  4:52   ` Simon Glass
  1 sibling, 1 reply; 5+ messages in thread
From: Etienne Carriere @ 2021-02-18 12:54 UTC (permalink / raw)
  To: u-boot

Fix inline comments and empty line in scmi driver and test files.
Condition SCMI clock and reset tests to U-Boot configuration.

Change-Id: Iac37398cedc1942cf1cc114fc60cbe04c599313e
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
 arch/sandbox/include/asm/scmi_test.h       |  5 +++--
 drivers/firmware/scmi/sandbox-scmi_agent.c |  4 ++--
 test/dm/scmi.c                             | 21 ++++++++++++---------
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/sandbox/include/asm/scmi_test.h b/arch/sandbox/include/asm/scmi_test.h
index 9b7031531..2930e686d 100644
--- a/arch/sandbox/include/asm/scmi_test.h
+++ b/arch/sandbox/include/asm/scmi_test.h
@@ -24,6 +24,7 @@ struct sandbox_scmi_clk {
 
 /**
  * struct sandbox_scmi_reset - Simulated reset controller exposed by SCMI
+ * @id:		Identifier of the reset controller used in the SCMI protocol
  * @asserted:	Reset control state: true if asserted, false if desasserted
  */
 struct sandbox_scmi_reset {
@@ -48,8 +49,8 @@ struct sandbox_scmi_voltd {
  * @idx:	Identifier for the SCMI agent, its index
  * @clk:	Simulated clocks
  * @clk_count:	Simulated clocks array size
- * @clk:	Simulated reset domains
- * @clk_count:	Simulated reset domains array size
+ * @reset:	Simulated reset domains
+ * @reset_count: Simulated reset domains array size
  * @voltd:	 Simulated voltage domains (regulators)
  * @voltd_count: Simulated voltage domains array size
  */
diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
index 3eafc49bd..56125a57b 100644
--- a/drivers/firmware/scmi/sandbox-scmi_agent.c
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -18,7 +18,7 @@
  * processing. It simulates few of the SCMI services for some of the
  * SCMI protocols embedded in U-Boot. Currently:
  * - SCMI clock protocol: emulate 2 agents each exposing few clocks
- * - SCMI reset protocol: emulate 1 agents each exposing a reset
+ * - SCMI reset protocol: emulate 1 agent exposing a reset controller
  * - SCMI voltage domain protocol: emulate 1 agent exposing 2 regulators
  *
  * Agent #0 simulates 2 clocks, 1 reset domain and 1 voltage domain.
@@ -29,7 +29,7 @@
  *
  * All clocks and regulators are default disabled and reset controller down.
  *
- * This Driver exports sandbox_scmi_service_ct() 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.
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index f21a3ff97..cc611c1e0 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -57,18 +57,24 @@ static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts,
 	ut_asserteq(2, scmi_ctx->agent_count);
 
 	ut_assertnonnull(scmi_ctx->agent[0]);
-	ut_asserteq(2, scmi_ctx->agent[0]->clk_count);
-	ut_assertnonnull(scmi_ctx->agent[0]->clk);
-	ut_asserteq(1, scmi_ctx->agent[0]->reset_count);
-	ut_assertnonnull(scmi_ctx->agent[0]->reset);
+	if (IS_ENABLED(CONFIG_CLK_SCMI)) {
+		ut_asserteq(2, scmi_ctx->agent[0]->clk_count);
+		ut_assertnonnull(scmi_ctx->agent[0]->clk);
+	}
+	if (IS_ENABLED(CONFIG_RESET_SCMI)) {
+		ut_asserteq(1, scmi_ctx->agent[0]->reset_count);
+		ut_assertnonnull(scmi_ctx->agent[0]->reset);
+	}
 	if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI)) {
 		ut_asserteq(2, scmi_ctx->agent[0]->voltd_count);
 		ut_assertnonnull(scmi_ctx->agent[0]->voltd);
 	}
 
 	ut_assertnonnull(scmi_ctx->agent[1]);
-	ut_assertnonnull(scmi_ctx->agent[1]->clk);
-	ut_asserteq(1, scmi_ctx->agent[1]->clk_count);
+	if (IS_ENABLED(CONFIG_CLK_SCMI)) {
+		ut_assertnonnull(scmi_ctx->agent[1]->clk);
+		ut_asserteq(1, scmi_ctx->agent[1]->clk_count);
+	}
 
 	return 0;
 }
@@ -113,7 +119,6 @@ static int dm_test_scmi_sandbox_agent(struct unit_test_state *uts)
 
 	return ret;
 }
-
 DM_TEST(dm_test_scmi_sandbox_agent, UT_TESTF_SCAN_FDT);
 
 static int dm_test_scmi_clocks(struct unit_test_state *uts)
@@ -175,7 +180,6 @@ static int dm_test_scmi_clocks(struct unit_test_state *uts)
 
 	return release_sandbox_scmi_test_devices(uts, dev);
 }
-
 DM_TEST(dm_test_scmi_clocks, UT_TESTF_SCAN_FDT);
 
 static int dm_test_scmi_resets(struct unit_test_state *uts)
@@ -206,7 +210,6 @@ static int dm_test_scmi_resets(struct unit_test_state *uts)
 
 	return release_sandbox_scmi_test_devices(uts, dev);
 }
-
 DM_TEST(dm_test_scmi_resets, UT_TESTF_SCAN_FDT);
 
 static int dm_test_scmi_voltage_domains(struct unit_test_state *uts)
-- 
2.17.1

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

* [PATCH 3/3] firmware: scmi: fix inline comments and minor coding style issues
  2021-02-18 12:54 ` [PATCH 3/3] firmware: scmi: fix inline comments and minor coding style issues Etienne Carriere
@ 2021-02-19  4:52   ` Simon Glass
  2021-02-19  6:17     ` Etienne Carriere
  0 siblings, 1 reply; 5+ messages in thread
From: Simon Glass @ 2021-02-19  4:52 UTC (permalink / raw)
  To: u-boot

Hi Etienne,

On Thu, 18 Feb 2021 at 05:55, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> Fix inline comments and empty line in scmi driver and test files.
> Condition SCMI clock and reset tests to U-Boot configuration.
>
> Change-Id: Iac37398cedc1942cf1cc114fc60cbe04c599313e
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
>  arch/sandbox/include/asm/scmi_test.h       |  5 +++--
>  drivers/firmware/scmi/sandbox-scmi_agent.c |  4 ++--
>  test/dm/scmi.c                             | 21 ++++++++++++---------
>  3 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/arch/sandbox/include/asm/scmi_test.h b/arch/sandbox/include/asm/scmi_test.h
> index 9b7031531..2930e686d 100644
> --- a/arch/sandbox/include/asm/scmi_test.h
> +++ b/arch/sandbox/include/asm/scmi_test.h
> @@ -24,6 +24,7 @@ struct sandbox_scmi_clk {
>
>  /**
>   * struct sandbox_scmi_reset - Simulated reset controller exposed by SCMI
> + * @id:                Identifier of the reset controller used in the SCMI protocol
>   * @asserted:  Reset control state: true if asserted, false if desasserted
>   */
>  struct sandbox_scmi_reset {
> @@ -48,8 +49,8 @@ struct sandbox_scmi_voltd {
>   * @idx:       Identifier for the SCMI agent, its index
>   * @clk:       Simulated clocks
>   * @clk_count: Simulated clocks array size
> - * @clk:       Simulated reset domains
> - * @clk_count: Simulated reset domains array size
> + * @reset:     Simulated reset domains
> + * @reset_count: Simulated reset domains array size
>   * @voltd:      Simulated voltage domains (regulators)
>   * @voltd_count: Simulated voltage domains array size
>   */
> diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
> index 3eafc49bd..56125a57b 100644
> --- a/drivers/firmware/scmi/sandbox-scmi_agent.c
> +++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
> @@ -18,7 +18,7 @@
>   * processing. It simulates few of the SCMI services for some of the
>   * SCMI protocols embedded in U-Boot. Currently:
>   * - SCMI clock protocol: emulate 2 agents each exposing few clocks
> - * - SCMI reset protocol: emulate 1 agents each exposing a reset
> + * - SCMI reset protocol: emulate 1 agent exposing a reset controller
>   * - SCMI voltage domain protocol: emulate 1 agent exposing 2 regulators
>   *
>   * Agent #0 simulates 2 clocks, 1 reset domain and 1 voltage domain.
> @@ -29,7 +29,7 @@
>   *
>   * All clocks and regulators are default disabled and reset controller down.
>   *
> - * This Driver exports sandbox_scmi_service_ct() 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.
> diff --git a/test/dm/scmi.c b/test/dm/scmi.c
> index f21a3ff97..cc611c1e0 100644
> --- a/test/dm/scmi.c
> +++ b/test/dm/scmi.c
> @@ -57,18 +57,24 @@ static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts,
>         ut_asserteq(2, scmi_ctx->agent_count);
>
>         ut_assertnonnull(scmi_ctx->agent[0]);
> -       ut_asserteq(2, scmi_ctx->agent[0]->clk_count);
> -       ut_assertnonnull(scmi_ctx->agent[0]->clk);
> -       ut_asserteq(1, scmi_ctx->agent[0]->reset_count);
> -       ut_assertnonnull(scmi_ctx->agent[0]->reset);
> +       if (IS_ENABLED(CONFIG_CLK_SCMI)) {

Why are these needed? Are these options not enabled on sandbox? The
goal is to test all the code, not part of it.

> +               ut_asserteq(2, scmi_ctx->agent[0]->clk_count);
> +               ut_assertnonnull(scmi_ctx->agent[0]->clk);
> +       }
> +       if (IS_ENABLED(CONFIG_RESET_SCMI)) {
> +               ut_asserteq(1, scmi_ctx->agent[0]->reset_count);
> +               ut_assertnonnull(scmi_ctx->agent[0]->reset);
> +       }
>         if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI)) {
>                 ut_asserteq(2, scmi_ctx->agent[0]->voltd_count);
>                 ut_assertnonnull(scmi_ctx->agent[0]->voltd);
>         }
>
>         ut_assertnonnull(scmi_ctx->agent[1]);
> -       ut_assertnonnull(scmi_ctx->agent[1]->clk);
> -       ut_asserteq(1, scmi_ctx->agent[1]->clk_count);
> +       if (IS_ENABLED(CONFIG_CLK_SCMI)) {
> +               ut_assertnonnull(scmi_ctx->agent[1]->clk);
> +               ut_asserteq(1, scmi_ctx->agent[1]->clk_count);
> +       }
>

Regards,
Simon

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

* [PATCH 3/3] firmware: scmi: fix inline comments and minor coding style issues
  2021-02-19  4:52   ` Simon Glass
@ 2021-02-19  6:17     ` Etienne Carriere
  0 siblings, 0 replies; 5+ messages in thread
From: Etienne Carriere @ 2021-02-19  6:17 UTC (permalink / raw)
  To: u-boot

Hello Simon,

On Fri, 19 Feb 2021 at 05:52, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Etienne,
>
> On Thu, 18 Feb 2021 at 05:55, Etienne Carriere
> <etienne.carriere@linaro.org> wrote:
> >
> > Fix inline comments and empty line in scmi driver and test files.
> > Condition SCMI clock and reset tests to U-Boot configuration.
> >
> > Change-Id: Iac37398cedc1942cf1cc114fc60cbe04c599313e
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > ---
> >  arch/sandbox/include/asm/scmi_test.h       |  5 +++--
> >  drivers/firmware/scmi/sandbox-scmi_agent.c |  4 ++--
> >  test/dm/scmi.c                             | 21 ++++++++++++---------
> >  3 files changed, 17 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/sandbox/include/asm/scmi_test.h b/arch/sandbox/include/asm/scmi_test.h
> > index 9b7031531..2930e686d 100644
> > --- a/arch/sandbox/include/asm/scmi_test.h
> > +++ b/arch/sandbox/include/asm/scmi_test.h
> > @@ -24,6 +24,7 @@ struct sandbox_scmi_clk {
> >
> >  /**
> >   * struct sandbox_scmi_reset - Simulated reset controller exposed by SCMI
> > + * @id:                Identifier of the reset controller used in the SCMI protocol
> >   * @asserted:  Reset control state: true if asserted, false if desasserted
> >   */
> >  struct sandbox_scmi_reset {
> > @@ -48,8 +49,8 @@ struct sandbox_scmi_voltd {
> >   * @idx:       Identifier for the SCMI agent, its index
> >   * @clk:       Simulated clocks
> >   * @clk_count: Simulated clocks array size
> > - * @clk:       Simulated reset domains
> > - * @clk_count: Simulated reset domains array size
> > + * @reset:     Simulated reset domains
> > + * @reset_count: Simulated reset domains array size
> >   * @voltd:      Simulated voltage domains (regulators)
> >   * @voltd_count: Simulated voltage domains array size
> >   */
> > diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
> > index 3eafc49bd..56125a57b 100644
> > --- a/drivers/firmware/scmi/sandbox-scmi_agent.c
> > +++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
> > @@ -18,7 +18,7 @@
> >   * processing. It simulates few of the SCMI services for some of the
> >   * SCMI protocols embedded in U-Boot. Currently:
> >   * - SCMI clock protocol: emulate 2 agents each exposing few clocks
> > - * - SCMI reset protocol: emulate 1 agents each exposing a reset
> > + * - SCMI reset protocol: emulate 1 agent exposing a reset controller
> >   * - SCMI voltage domain protocol: emulate 1 agent exposing 2 regulators
> >   *
> >   * Agent #0 simulates 2 clocks, 1 reset domain and 1 voltage domain.
> > @@ -29,7 +29,7 @@
> >   *
> >   * All clocks and regulators are default disabled and reset controller down.
> >   *
> > - * This Driver exports sandbox_scmi_service_ct() 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.
> > diff --git a/test/dm/scmi.c b/test/dm/scmi.c
> > index f21a3ff97..cc611c1e0 100644
> > --- a/test/dm/scmi.c
> > +++ b/test/dm/scmi.c
> > @@ -57,18 +57,24 @@ static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts,
> >         ut_asserteq(2, scmi_ctx->agent_count);
> >
> >         ut_assertnonnull(scmi_ctx->agent[0]);
> > -       ut_asserteq(2, scmi_ctx->agent[0]->clk_count);
> > -       ut_assertnonnull(scmi_ctx->agent[0]->clk);
> > -       ut_asserteq(1, scmi_ctx->agent[0]->reset_count);
> > -       ut_assertnonnull(scmi_ctx->agent[0]->reset);
> > +       if (IS_ENABLED(CONFIG_CLK_SCMI)) {
>
> Why are these needed? Are these options not enabled on sandbox? The
> goal is to test all the code, not part of it.

I thought this source file should be flexible to adapt to
configuration in case one changes it for some tests.
I saw a few other occurrences in the test/dm/*. But looking again it's
true there are very very few, almost none.

Fair, i'll discard these config tests in my patch series and post a v2.

Thanks,
Etienne

>
> > +               ut_asserteq(2, scmi_ctx->agent[0]->clk_count);
> > +               ut_assertnonnull(scmi_ctx->agent[0]->clk);
> > +       }
> > +       if (IS_ENABLED(CONFIG_RESET_SCMI)) {
> > +               ut_asserteq(1, scmi_ctx->agent[0]->reset_count);
> > +               ut_assertnonnull(scmi_ctx->agent[0]->reset);
> > +       }
> >         if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI)) {
> >                 ut_asserteq(2, scmi_ctx->agent[0]->voltd_count);
> >                 ut_assertnonnull(scmi_ctx->agent[0]->voltd);
> >         }
> >
> >         ut_assertnonnull(scmi_ctx->agent[1]);
> > -       ut_assertnonnull(scmi_ctx->agent[1]->clk);
> > -       ut_asserteq(1, scmi_ctx->agent[1]->clk_count);
> > +       if (IS_ENABLED(CONFIG_CLK_SCMI)) {
> > +               ut_assertnonnull(scmi_ctx->agent[1]->clk);
> > +               ut_asserteq(1, scmi_ctx->agent[1]->clk_count);
> > +       }
> >
>
> Regards,
> Simon

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

end of thread, other threads:[~2021-02-19  6:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-18 12:54 [PATCH 1/3] firmware: scmi: voltage regulator Etienne Carriere
2021-02-18 12:54 ` [PATCH 2/3] firmware: scmi: sandbox test for " Etienne Carriere
2021-02-18 12:54 ` [PATCH 3/3] firmware: scmi: fix inline comments and minor coding style issues Etienne Carriere
2021-02-19  4:52   ` Simon Glass
2021-02-19  6:17     ` Etienne Carriere

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