Message-ID: <25df6720-da97-4f81-9124-bdb7e9542d56@foss.st.com>
Date: Mon, 9 Feb 2026 10:51:07 +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



On 2/5/26 21:07, Andrew Davis wrote:
> 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


For the time being, this approach does not align with other /sys/class/
declarations, which are index-based. You would also need to duplicate the
remoteproc device for legacy support (e.g., 
/sys/class/remoteproc/remoteproc0
and /sys/class/remoteproc/78000000.r5f).

However, if you want to promote this approach, feel free to propose a patch
series. From my perspective, the index mechanism combined with DT aliases
seems like a better compromise.

> 
> 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.

For my understanding, is this only your expectation, or is it a general 
direction recommended by the Linux maintainers?

> 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?

Other alternatives I can see are:
- use of the reg property: whould break legacy.
- add a new proc node property: would do the same than the
   existing alias.

Regards,
Arnaud

> 
> 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