Re: [RFC PATCH] remoteproc: core: support fixed device index from DT aliases

[Mathieu Poirier <mathieu.poirier@linaro.org>]

Good morning,

On Tue, Jan 27, 2026 at 02:05:55PM +0100, 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.
> 
> 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 {
>         remoteproc0 = &m33_rproc;
>         remoteproc1 = &m0_rproc;
>     };
> 
> When a "remoteproc<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 <arnaud.pouliquen@foss.st.com>
> ---
> Notes:
> 
> - This patch is submitted as an RFC in this first version.
>   The main reason is that support for the Cortex-M33 and Cortex-M0 on
>   the STM32MP25 SoC is not yet upstream. The primary objective is to
>   trigger discussion on the concept; if there is agreement, I can drop
>   the RFC tag in a next version.

I think this is a good idea.

> 
> - The keystone_remoteproc driver also uses DT aliases. As it uses the
>   "rproc" alias only to construct the firmware name, it should remain
>   compatible with this change.

But we won't have "rproc" and "remoteproc" as aliases - it will have to be the
former since TI is already using it.  I also suggest doing a #define in
include/linux/remoteproc.h.  There is a possibility for backward compatibility
issues but we'll have to let the feature in next long enough to find out.

> ---
>  drivers/remoteproc/remoteproc_core.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index aada2780b343..8da6c410870a 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -2461,6 +2461,8 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
>  			  const char *firmware, int len)
>  {
>  	struct rproc *rproc;
> +	int index = -ENODEV;
> +	int first_dynamic;
>  
>  	if (!dev || !name || !ops)
>  		return NULL;
> @@ -2481,8 +2483,27 @@ 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);

> +	/*
> +	 * Assign a unique device index and name
> +	 * Look for a static index coming from the "remoteproc" DT alias
> +	 * (e.g. "remoteproc0"). If none is found, start allocating
> +	 * dynamic IDs after the highest alias in use.
> +	 */
> +	if (dev->of_node)
> +		index = of_alias_get_id(dev->of_node, "remoteproc");
> +	if (index < 0) {
> +		first_dynamic = of_alias_get_highest_id("remoteproc");
> +		if (first_dynamic < 0)
> +			first_dynamic = 0;
> +		else
> +			first_dynamic++;
> +		rproc->index = ida_alloc_range(&rproc_dev_index, first_dynamic,
> +					       ~0, GFP_KERNEL);
> +	} else {
> +		rproc->index = ida_alloc_range(&rproc_dev_index, index,
> +					       index, GFP_KERNEL);
> +	}
> +

I find the above hard to read and even harder to maintain.  I suggest spinning
off a new function as depicted here:


>>>>>>>>>>>>>>>
/*
 * Assign a unique device index and name
 * 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.
 */
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 index; 

        /*
         * No alias has been assigned to this remoteproc.  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);
}
<<<<<<<<<<<<<<<



>  	if (rproc->index < 0) {
>  		dev_err(dev, "ida_alloc failed: %d\n", rproc->index);
>  		goto put_device;
> 
> base-commit: 63804fed149a6750ffd28610c5c1c98cce6bd377
> -- 
> 2.43.0
>