Message-ID: <b9b478ea-cec7-4b5b-8ae9-d0574c4eb02a@ti.com>
Date: Thu, 5 Feb 2026 14:07:48 -0600
From: Andrew Davis <afd@...com>
To: Arnaud POULIQUEN <arnaud.pouliquen@...s.st.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

On 2/5/26 11:58 AM, Arnaud POULIQUEN wrote:
> 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.
> 

Yes, both name and number based indexing will be platform-dependent, but
they are not purely equivalent. The thing I want to avoid about number based
lookup is in documentation. I see docs already that say something like

> To start the R5F core run this command:
> echo start > /sys/class/remoteproc/remoteproc2/state

And folks (or LLMs being trained on the docs) might assume this is in
any way a portable thing to do. Which we know it is not, the number might
change even between two platforms from the same vendor. Where as if the
instructions said:

> echo start > /sys/class/remoteproc/78000000.r5f/state

It becomes immediately obvious this is valid only for a given platform.

The other thing I want to avoid is the ever-growing alias lists in DT.
Could be done without having to add a list of aliases to every DT. Is
there no other heuristic that we could use to produce an static ordering?

Andrew

> 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