Re: [PATCH v17 5/7] firmware: arm_rmm: Activate the RMM

On 09/09/2026 05:29, Gavin Shan wrote:

Hi Suzuki,

On 9/7/26 7:59 PM, Suzuki K Poulose wrote:

quoted

From: Steven Price <steven.price@arm.com>

Activate the RMM after the basic configuration. This is a memory 
transferring,
stateful operation.

Signed-off-by: Steven Price <steven.price@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
Changes since v16:
  * Split into a new patch
---
  drivers/firmware/arm_rmm/rmi.c | 17 +++++++++++++++--
  include/linux/arm-rmi-cmds.h   | 11 +++++++++++
  2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_rmm/rmi.c b/drivers/firmware/ 

arm_rmm/rmi.c
index 42c973c3a98bb..d969c8738efde 100644

--- a/drivers/firmware/arm_rmm/rmi.c
+++ b/drivers/firmware/arm_rmm/rmi.c

@@ -641,7 +641,8 @@ static int rmi_configure(void)

  static int __init arm64_init_rmi(void)
  {
-    int ret;
+    int ret = 0;
+    struct rmi_sro_state *sro = NULL;

Why we need to initialize those local variables ('ret' and 'sro')? :-)
Also, we can use the scope-based cleanup function supported by cleanup.h
for @sro.

     struct smi_sro_state *sro __free(kfree) = NULL;

With the scope-based cleanup in place, we needn't explict 'kfree(sro)' at
end of this function.

Ack.

quoted

      /* Continue without realm support if we can't agree on a version */
      ret = rmi_check_version();

@@ -656,7 +657,19 @@ static int __init arm64_init_rmi(void)

      if (ret)
          return ret;
-    return 0;
+    /* Activate the RMM */
+    sro = kmalloc_obj(*sro);
+    if (!sro)
+        return -ENOMEM;

     if (!sro) {
         pr_err("Unable to alloc SRO object for RMM activation\n");
         return -ENOMEM;
     }

Ack

quoted

+
+    ret = rmi_rmm_activate(sro);
+    if (ret) {
+        pr_err("RMM activate failed\n");
+        ret = ret < 0 ? ret : -ENXIO;
+    }
+
+    kfree(sro);
+    return ret;
  }
  /*

diff --git a/include/linux/arm-rmi-cmds.h b/include/linux/arm-rmi-cmds.h
index fed0f3421895d..dea7c7004d35f 100644
--- a/include/linux/arm-rmi-cmds.h
+++ b/include/linux/arm-rmi-cmds.h

@@ -64,6 +64,17 @@ static inline int rmi_rmm_config_set(unsigned long 

cfg_ptr)
      return res.a0;
  }
+/**
+ * rmi_rmm_activate() - Activate the RMM
+ * @sro: Preallocated SRO context to be used
+ *
+ * Return: 0 on success, positive RMI result code or negative Linux 
error code
+ */
+static inline long rmi_rmm_activate(struct rmi_sro_state *sro)
+{
+    return rmi_sro_memxfer_cmd(sro, GFP_KERNEL, SMC_RMI_RMM_ACTIVATE);
+}
+

rmi_rmm_acvivate() would be used for once by rmi.c::arm64_init_rmi(). Lets
not expose the function by combining the logic to rmi.c::arm64_init_rmi().

Have done that already in my local tree.


Thanks for the review.

Cheers
Suzuki