On 2/4/26 4:52 AM, Arnaud Pouliquen wrote:
On systems with multiple remote processors, the remoteproc device
enumeration is not stable as it depends on the probe ordering.
As a result, the /sys/class/remoteproc/remoteproc<x> entries do not
always refer to the same remote processor instance, which complicates
userspace applications.


While I will agree it is slightly more complicated in userspace to lookup
the device by name string rather than by some static number, there seems to
be a good reason for not doing this also.

Much like network interfaces where the /dev/eth<x> can change each boot and
attempts to make that static from kernel has been turned down: having static
indexes doesn't make userspace software any more portable.

Say you lock your M33 core to rproc<1> on one SoC, it doesn't mean your next
SoC will have the same rproc order, or even have a M33 at all. So you still
need your userspace code to lookup and check the name, otherwise you make
bad assumptions. Not having static IDs forces software to do the correct
thing here.

The only valid reason I can think up is maybe this makes board specific
documentation easier. One can say:

"On the STM32MP257F-DK, check that the M33 has booted by running
`cat /sys/class/remoteproc/remoteproc3/status`"

without having to first find the right number by checking each
`remoteproc<x>/name`. But wouldn't adding something like a named
sysfs dir syslinks work even better?

`cat /sys/class/remoteproc/m33@76000000/status`

(and yes I know someone here at TI did this alias naming for our
keystone platforms, but if not for possible backwards compat breaks
I'd love to remove that one also)

Andrew

Inspired by the SPI implementation, this commit allows board-specific
numbering to be defined in device tree while still supporting dynamically
registered remote processors.

For instance, on STM32MP25 Soc this can be used by defining:

     aliases {
         rproc0 = &m33_rproc;
         rproc1 = &m0_rproc;
     };

When a "rproc<x>" DT alias is present, use it to assign a fixed
"/sys/class/remoteproc/remoteproc<x>" entry.
If no remoteproc alias is defined, keep the legacy index allocation.
If only some remoteproc instances have an alias, allocate dynamic
index starting after the highest alias index declared.

Signed-off-by: Arnaud Pouliquen <[email protected]>
Tested-by: Peng Fan <[email protected]>
---
V3:
- fix double space typo
- add Peng Fan's Tested-by

V2:
- Introduces rproc_get_index based on Mathieu Poirier's suggestion.
   An update compared to Mathieu's version is that the call to
   ida_alloc_range is retained if an alias is found for the remote device,
   to balance with ida_free().
- Rename DT alias stem from "remoteproc" to "rproc" to be consistent with
   keytone driver.
---
  drivers/remoteproc/remoteproc_core.c | 40 ++++++++++++++++++++++++++--
  include/linux/remoteproc.h           |  3 +++
  2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c 
b/drivers/remoteproc/remoteproc_core.c
index aada2780b343..4a02814c5d04 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -2433,6 +2433,43 @@ static int rproc_alloc_ops(struct rproc *rproc, const 
struct rproc_ops *ops)
        return 0;
  }
+/**
+ * rproc_get_index - assign a unique device index for a remote processor
+ * @dev: device associated with the remote processor
+ *
+ * Look for a static index coming from the "rproc" DT alias
+ * (e.g. "rproc0"). If none is found, start allocating
+ * dynamic IDs after the highest alias in use.
+ *
+ * Return: a non-negative index on success, or a negative error code on 
failure.
+ */
+static int rproc_get_index(struct device *dev)
+{
+       int index;
+
+       /* No DT to deal with */
+       if (!dev->of_node)
+               goto legacy;
+
+       /* See if an alias has been assigned to this remoteproc */
+       index = of_alias_get_id(dev->of_node, RPROC_ALIAS);
+       if (index >= 0)
+               return ida_alloc_range(&rproc_dev_index, index, index,
+                                      GFP_KERNEL);
+       /*
+        * No alias has been assigned to this remoteproc device. See if any
+        * "rproc" aliases have been assigned and start allocating after
+        * the highest one if it is the case.
+        */
+       index = of_alias_get_highest_id(RPROC_ALIAS);
+       if (index >= 0)
+               return ida_alloc_range(&rproc_dev_index, index + 1, ~0,
+                                      GFP_KERNEL);
+
+legacy:
+       return ida_alloc(&rproc_dev_index, GFP_KERNEL);
+}
+
  /**
   * rproc_alloc() - allocate a remote processor handle
   * @dev: the underlying device
@@ -2481,8 +2518,7 @@ struct rproc *rproc_alloc(struct device *dev, const char 
*name,
        rproc->dev.driver_data = rproc;
        idr_init(&rproc->notifyids);
- /* Assign a unique device index and name */
-       rproc->index = ida_alloc(&rproc_dev_index, GFP_KERNEL);
+       rproc->index = rproc_get_index(dev);
        if (rproc->index < 0) {
                dev_err(dev, "ida_alloc failed: %d\n", rproc->index);
                goto put_device;
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index b4795698d8c2..3feb2456ecc4 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -503,6 +503,9 @@ enum rproc_features {
        RPROC_MAX_FEATURES,
  };
+ /* device tree remoteproc Alias stem */
+ #define RPROC_ALIAS "rproc"
+
  /**
   * struct rproc - represents a physical remote processor device
   * @node: list node of this rproc object


Reply via email to