Message-ID: <1a175145-d29f-426a-a163-1d185b8b0348@foss.st.com>
Date: Thu, 5 Feb 2026 18:58:49 +0100
From: Arnaud POULIQUEN <arnaud.pouliquen@...s.st.com>
To: Andrew Davis <afd@...com>, Bjorn Andersson <andersson@...nel.org>,
	"Mathieu Poirier" <mathieu.poirier@...aro.org>
CC: <linux-remoteproc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [PATCH v3 1/2] remoteproc: core: support fixed device index from
 DT aliases

Hello,

On 2/4/26 15:57, Andrew Davis wrote:
> 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.

That was also my initial approach, but it is difficult to impose on our 
customers who have legacy applications, especially since they are 
accustomed to using fixed indexes with other framework ABIs.

> 
> 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@...00000/status`

The only benefit I can see in checking 
/sys/class/remoteproc/<name>/status instead of 
/sys/class/remoteproc/remoteproc<x>/name is to avoid iterating over 
devices by name. However, in both cases, the application still needs to 
know the remote processor name, which is platform-dependent and usually 
defined by the device tree.

At the end, using an index here is simply an optional alternative to the 
name, as seen in other framework implementations.

Regards,
Arnaud

> 
> (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 <arnaud.pouliquen@...s.st.com>
>> Tested-by: Peng Fan <peng.fan@....com>
>> ---
>> 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
> 

Powered by blists - more mailing lists