firmware: scmi: add base proto, scmi sample, smc/hvc transport and reset proto #78293

GrygiriiS · 2024-09-11T13:53:13Z

Hi

The main intention of this work is to introduce ARM SCMI SMC/HVC based transport and SCMI sample application, So SCMI can be enabled on RPI5 platform with SCMI compliant firmware [1]. Particularly It's important in case Zephyr is executed in virtualized environment (under XEN in our case) , which requires HW partitioning.

This PR adds following major functionality to the SCMI code:

Base protocol support
Reset domain management protocol support
ARM SCMI SMC/HVC based transport layer
SCMI Sample application with shell interface (see example below)

Patches:

Patches 1-7 are updates/optimization for existing SCMI code in preparation of adding more functionality
patch 8 introduces support for SCMI Base protocol
patch 9 introduces SCMI sample application 'samples/drivers/firmware/arm_scmi' which demonstrates the usage of ARM SCMI. It provides access to the ARM SCMI shell interface for demo and testing purposes of different ARM SCMI protocols.
Patches 10-11 introduce support for SCMI SMC/HVC based transport.
Patches 12-13 introduce support for SCMI Reset domain management protocol

SCMI Sample application output:

	uart:~$ arm_scmi base
	base - SCMI Base proto commands.
	Subcommands:
	  revision           : SCMI Base get revision info
	                      Usage: arm_scmi base revision

	  discover_agent     : SCMI Base discover an agent
	                      Usage: arm_scmi base discover_agent [agent_id]

	  device_permission  : SCMI Base set an agent permissions to access device
	                      Usage: arm_scmi base discover_agent <agent_id> <device_id>
	                      <allow := 0|1>

	  reset_agent_cfg    : SCMI Base reset an agent configuration
	                      Usage: arm_scmi base reset_agent_cfg <agent_id>
	                      <reset_perm := 0|1>

	uart:~$ arm_scmi base revision
	ARM SCMI base protocol v0002.0000
	  vendor        :EPAM
	  subvendor     :
	  fw version    :0x40
	  protocols     :2
	  num_agents    :8

	uart:~$ arm_scmi reset
	reset - SCMI Reset proto commands.
	Subcommands:
	  revision   : SCMI Reset proto show revision information
	              Usage: arm_scmi reset revision

	  list       : SCMI Reset domains list
	              Usage: arm_scmi reset list

	  info       : SCMI Reset domain show info
	              Usage: arm_scmi reset info <domain_id>

	  assert     : SCMI Reset domain assert
	              Usage: arm_scmi reset assert <domain_id>

	  deassert   : SCMI Reset domain de-assert
	              Usage: arm_scmi reset deassert <domain_id>

	  autoreset  : SCMI Reset domain Autonomous reset
	              Usage: arm_scmi reset autoreset <domain_id>

        uart:~$ arm_scmi reset revision
	ARM SCMI Reset protocol version 0x0001.0000 num_domains:4

[1] https://github.com/xen-troops/arm-trusted-firmware/tree/rpi5_dev

GrygiriiS · 2024-09-11T13:54:05Z

@LaurentiuM1234 @yangbolu1991 @oleksiimoisieiev @dbaluta

dbaluta · 2024-09-11T14:34:27Z

include/zephyr/drivers/firmware/scmi/protocol.h

@@ -17,6 +17,25 @@
 #include <stdint.h>
 #include <errno.h>

+#define SCMI_MAX_STR_SIZE        64
+#define SCMI_SHORT_NAME_MAX_SIZE 16
+


I think should be in a separate patch. Initial patch should be for removing SCMI_FIELD_MAKE.

I've moved defines in Base proto patch (first user) and retested build of every patch.

Move SCMI protocols Kconfig options into separate file Kconfig.proto to improve code maintainability. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

The SCMI core defines custom SCMI_FIELD_MAKE macro while Zephyr provides generic FIELD_GET/FIELD_PREP macro. Hence, use generic FIELD_GET/FIELD_PREP and drop SCMI_FIELD_MAKE. This also required to add proper SCMI message header definitions. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

Now the SCMI message header is constructed inside SCMI protocol helpers APIs which doesn't know how to fill "token" field in SCMI message header. As result the "token" considered to be always 0 in scmi_shmem_read_message() wich compares RX header with TX header. This patch adds possibility to generate and process "token" filed of the SCMI message header as part of transport layer functionality. The new callback .channel_get_token() and transport API scmi_transport_channel_get_token() is introduced, so transport layer can provide "token" value to the SCMI core which is injected in the SCMI message header during transfer. The SCMI mailbox transport driver updated with stub which always returns 0 to keep existing behavior intact. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

Add debug prints to SCMI shmem driver to see shmem contents before/after xfer. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

Now the SCMI shmem driver expects that Platfrom will return exactly the same amount of data as specified in RX struct scmi_message-> len. But that is not true, as for some SCMI messages amount of return data can vary (for example: BASE_DISCOVER_LIST_PROTOCOLS or CLOCK_DESCRIBE_RATES). It can be less than rx_msg->len. In general, the Platform will try to fill as more reply data as shmem can fit, but no more than maximum SCMI msg size which is Platform specific (often 128 bytes). Hence, fix SCMI shmem driver to account that the Platform may return less data than expected, but still fail if it's more that rx_msg->len. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

The SCMI protocols defines the "status" field for every reply (RX message, type SCMI_COMMAND). The current SCMI implementation defines "status" field for every reply (RX message) also and handles it in Protocol helpers which adds a lot of repeated code. The same can be done at transport layer which allows to simplify SCMI code. This patch adds "status' field to the struct scmi_message, which is filled by SCMI shmem driver and processed by .read_message() callbacks. The Protocol helpers do not need to process "status" any more as scmi_send_message() will return proper error code if Platform fails. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

Completely drop RX message "status" processing from protocol helpers. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

This patch adds support for SCMI Base protocol. It provides helpers for: - obtaining SCMI firmware revision information: only base protocol version or full if CONFIG_ARM_SCMI_BASE_EXT_REV=y - agent management helpers if CONFIG_ARM_SCMI_BASE_AGENT_HELPERS=y. scmi_base_discover_agent() scmi_base_device_permission() scmi_base_reset_agent_cfg() Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

Add ARM SCMI shell sample application. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

Add DT bindings for ARM System Control and Management Interface (SCMI) with SMC/HVC shmem based transport and for Reset domain management protocol. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

This patch adds SCMI SMC/HVC shmem based transport which works with SCMI compliant ARM TF firmware with ARM SMC/HVC transport. The SCMI core updated to support the case when it should always work in polling mode as the SMC/HVC transport always completes request on exit from SMC/HVC call. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

This patch adds SCMI Reset domain management protocol helpers and Zephyr Reset Controller driver, which uses these helpers. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

Add ARM SCMI Reset shell interface for demo and testing purposes, which is defined as sub-command of *arm_scmi* command. The ARM SCMI Reset shell interface defined as following: $arm_scmi reset reset - SCMI Reset proto commands. Subcommands: revision : SCMI Reset proto show revision information Usage: arm_scmi reset revision list : SCMI Reset domains list Usage: arm_scmi reset list info : SCMI Reset domain show info Usage: arm_scmi reset info <domain_id> assert : SCMI Reset domain assert Usage: arm_scmi reset assert <domain_id> deassert : SCMI Reset domain de-assert Usage: arm_scmi reset deassert <domain_id> autoreset : SCMI Reset domain Autonomous reset Usage: arm_scmi reset autoreset <domain_id> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

GrygiriiS · 2024-09-12T12:06:55Z

Also forgot to note that some generic changes in this PR will conflict with #77437.

LaurentiuM1234

Nice work! Loving the new samples. Messed around a bit with them on the imx95 board.
Left some comments below.

Also, can you please update the documentation? I think it would be worth mentioning the newly supported protocols and perhaps the samples?

@bjarki-andreasen can you please have a look as well? Thanks!

LE: can you please rebase as well? The imx95 support (another SCMI-based platform) was merged recently and might require a rebase for the twister tests to run for that as well.

LaurentiuM1234 · 2024-09-23T10:37:13Z

include/zephyr/drivers/firmware/scmi/pinctrl.h

-	(SCMI_FIELD_MAKE(fid_valid, BIT(1), 10) |			\
-	 SCMI_FIELD_MAKE(cfg_num, GENMASK(7, 0), 2) |			\
-	 SCMI_FIELD_MAKE(selector, GENMASK(1, 0), 0))
+#define SCMI_PINCTRL_CONFIG_ATTRIBUTES(fid_valid, cfg_num, selector)                               \


Should be:

#define SCMI_PINCTRL_CONFIG_ATTRIBUTES(fid_valid, cfg_num, selector)\ (FIELD_PREP(BIT(10), fid_valid) | FIELD_PREP(GENMASK(9, 2), cfg_num) | FIELD_PREP(GENMASK(1, 0), selector))

LaurentiuM1234 · 2024-09-23T10:54:04Z

drivers/firmware/scmi/shmem.c

@@ -76,6 +76,8 @@ int scmi_shmem_read_message(const struct device *shmem, struct scmi_message *msg
 		return -EINVAL;
 	}

+	LOG_HEXDUMP_DBG((void *)layout, 64, "shmem before read");


What if shmem size if < 64 bytes? Should use MIN(64, shmem_size).
Also, should probably add a macro for the magic number. Something like SHMEM_DEBUG_DUMP_BYTES.

LaurentiuM1234 · 2024-09-23T10:55:50Z

drivers/firmware/scmi/shmem.c

@@ -180,6 +184,7 @@ static int scmi_shmem_init(const struct device *dev)
 	}

 	device_map(&data->regmap, cfg->phys_addr, cfg->size, K_MEM_CACHE_NONE);
+	LOG_DBG("shmem phys:%lx dev:%lx", cfg->phys_addr, data->regmap);


Should be virt instead of dev.

Also, is this really useful? You should be able to see the shmem area in the DTS.

LaurentiuM1234 · 2024-09-23T11:12:30Z

drivers/firmware/scmi/mailbox.c


 	mbox_chan = chan->data;

-	return scmi_shmem_read_message(mbox_chan->shmem, msg);
+	ret = scmi_shmem_read_message(mbox_chan->shmem, msg);
+	if (!ret && msg->status != SCMI_SUCCESS) {


I understand that this removes some redundant code from protocols but I really don't think the transport layer should be responsible for checking the status of the messages. IMO all it should do is check if there were errors with the data transmission/retrieval.

Also, what if ret != 0?

LaurentiuM1234 · 2024-09-23T11:15:09Z

drivers/firmware/scmi/shmem.c

@@ -98,8 +98,10 @@ int scmi_shmem_read_message(const struct device *shmem, struct scmi_message *msg
 		return -EINVAL;
 	}

+	msg->status = SCMI_SUCCESS;


What's this for? You're overwriting the message status anyways.

LaurentiuM1234 · 2024-09-23T12:08:22Z

drivers/firmware/scmi/Kconfig.proto

+
+config ARM_SCMI_BASE_HELPERS
+	bool "Helper functions for SCMI base protocol"
+	default y


should depend on one of the transports

LaurentiuM1234 · 2024-09-23T12:14:34Z

drivers/firmware/scmi/core.c

-	if (ret < 0) {
-		LOG_ERR("failed to wait for msg reply");
-		goto out_release_mutex;
+	if (!scmi_transport_channel_is_polling(proto->transport, proto->tx)) {


All transport channels should support polling mode because the send_message function is forced to poll in PRE_KERNEL so this function should always return true. As such, I suggest renaming to scmi_transport_channel_is_polling_only.

LaurentiuM1234 · 2024-09-23T12:18:43Z

drivers/firmware/scmi/core.c

+		}
+	} else {
+		while (!scmi_transport_channel_is_free(proto->transport, proto->tx)) {
+			k_cpu_idle();


Why go into idle mode? What if there's more work to do? I'd suggest using k_yield.

LaurentiuM1234 · 2024-09-23T12:22:24Z

samples/drivers/firmware/arm_scmi/src/shell_reset.c

+#include <zephyr/drivers/reset.h>
+#include <zephyr/shell/shell.h>
+
+#if defined(CONFIG_DT_HAS_ARM_SCMI_RESET_ENABLED)


Should rather depend on RESET_ARM_SCMI otherwise you'd just end up with a build error if built w/o CONFIG_RESET=y. Also, might be able to handle this using Kconfig/CMakeLists.txt.

LE: if that's not possible then can just check if CONFIG_RESET_ARM_SCMI is set. Otherwise, just have a build assert failure.

LaurentiuM1234 · 2024-09-23T12:23:00Z

samples/drivers/firmware/arm_scmi/src/shell_reset.c

+
+#include <zephyr/drivers/firmware/scmi/reset.h>
+#include <zephyr/drivers/reset.h>
+#include <zephyr/shell/shell.h>


Can we have this under a reset directory? Same as base basically.

github-actions · 2025-01-18T00:31:39Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

zephyrbot added the area: Samples Samples label Sep 11, 2024

zephyrbot requested review from decsny, kartben and nashif September 11, 2024 13:53

zephyrbot assigned kartben Sep 11, 2024

dbaluta reviewed Sep 11, 2024

View reviewed changes

decsny requested review from LaurentiuM1234 and danieldegrasse September 11, 2024 16:16

Grygorii Strashko added 13 commits September 12, 2024 14:48

firmware: scmi: move protocols cfgs in separate file

b5473c0

Move SCMI protocols Kconfig options into separate file Kconfig.proto to improve code maintainability. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

firmware: scmi: shmem: add debug prints

68f6abf

Add debug prints to SCMI shmem driver to see shmem contents before/after xfer. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

firmware: scmi: completely drop status from protocol helpers

c3e2c9e

Completely drop RX message "status" processing from protocol helpers. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

samples: drivers: firmware: add arm scmi sample

11c3fb1

Add ARM SCMI shell sample application. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

dts: bindings: firmware: add arm scmi (smc/hvc) bindings

f9bbef1

Add DT bindings for ARM System Control and Management Interface (SCMI) with SMC/HVC shmem based transport and for Reset domain management protocol. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

drivers: reset: scmi: add support for reset domain management protocol

008590c

This patch adds SCMI Reset domain management protocol helpers and Zephyr Reset Controller driver, which uses these helpers. Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

GrygiriiS force-pushed the zm_rpi5_dev_scmi branch from afbd657 to d455d4e Compare September 12, 2024 11:57

LaurentiuM1234 reviewed Sep 23, 2024

View reviewed changes

GrygiriiS mentioned this pull request Sep 24, 2024

boards/rpi_5: add rpi_5_scmi.overlay xen-troops/zephyr-dom0-xt#37

Open

kartben assigned LaurentiuM1234 and unassigned kartben Nov 18, 2024

github-actions bot added the Stale label Jan 18, 2025

github-actions bot closed this Feb 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

firmware: scmi: add base proto, scmi sample, smc/hvc transport and reset proto #78293

firmware: scmi: add base proto, scmi sample, smc/hvc transport and reset proto #78293

GrygiriiS commented Sep 11, 2024

GrygiriiS commented Sep 11, 2024 •

edited

Loading

dbaluta Sep 11, 2024

GrygiriiS Sep 12, 2024

GrygiriiS commented Sep 12, 2024

LaurentiuM1234 left a comment •

edited

Loading

LaurentiuM1234 Sep 23, 2024

LaurentiuM1234 Sep 23, 2024

LaurentiuM1234 Sep 23, 2024

LaurentiuM1234 Sep 23, 2024

LaurentiuM1234 Sep 23, 2024

LaurentiuM1234 Sep 23, 2024

LaurentiuM1234 Sep 23, 2024

LaurentiuM1234 Sep 23, 2024

LaurentiuM1234 Sep 23, 2024 •

edited

Loading

LaurentiuM1234 Sep 23, 2024

github-actions bot commented Jan 18, 2025

firmware: scmi: add base proto, scmi sample, smc/hvc transport and reset proto #78293

firmware: scmi: add base proto, scmi sample, smc/hvc transport and reset proto #78293

Conversation

GrygiriiS commented Sep 11, 2024

GrygiriiS commented Sep 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GrygiriiS commented Sep 12, 2024

LaurentiuM1234 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LaurentiuM1234 Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 18, 2025

GrygiriiS commented Sep 11, 2024 •

edited

Loading

LaurentiuM1234 left a comment •

edited

Loading

LaurentiuM1234 Sep 23, 2024 •

edited

Loading