[PATCH v2 1/7] Introduce the Interconnect Subsystem
Neil Armstrong
neil.armstrong at linaro.org
Thu Oct 9 18:48:42 CEST 2025
On 10/9/25 18:43, Neil Armstrong wrote:
> On 10/9/25 14:28, Simon Glass wrote:
>> Hi Neil,
>>
>> On Thu, 9 Oct 2025 at 05:54, Neil Armstrong <neil.armstrong at linaro.org> wrote:
>>>
>>> On 10/9/25 13:16, Simon Glass wrote:
>>>> Hi Neil,
>>>>
>>>> On Thu, 9 Oct 2025 at 02:18, Neil Armstrong <neil.armstrong at linaro.org> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 10/9/25 02:25, Simon Glass wrote:
>>>>>> Hi Neil,
>>>>>>
>>>>>> On Wed, 8 Oct 2025 at 09:31, Neil Armstrong <neil.armstrong at linaro.org> wrote:
>>>>>>>
>>>>>>> Let's introduce the Interconnect subsystem based on the Linux framework
>>>>>>> which is used to vote for bandwidth across multiple SoC busses.
>>>>>>>
>>>>>>> Each bus endpoints are materialised as "nodes" which are linked together,
>>>>>>> and the DT will specify a pair of nodes to enable and set a bandwidth
>>>>>>> on the route between those endpoints.
>>>>>>>
>>>>>>> The hardware resources that provide those nodes and provides the way
>>>>>>> to vote for the bandwidth are called "providers".
>>>>>>>
>>>>>>> The Interconnect uclass code is heavily based on the Linux one, with
>>>>>>> some small differences:
>>>>>>> - nodes are allocated as udevices instead of Linux idr_alloc()
>>>>>>> - tag management is minimal, only normal xlate is supported
>>>>>>> - dynamic IDs is not implemented
>>>>>>> - getting nodes states at probe is not implemented
>>>>>>> - providers are probed on demand while the nodes links are traversed
>>>>>>> - nodes are populated on bind
>>>>>>>
>>>>>>> Fully tested with associated DM test suite.
>>>>>>>
>>>>>>> Signed-off-by: Neil Armstrong <neil.armstrong at linaro.org>
>>>>>>> ---
>>>>>>> drivers/Kconfig | 2 +
>>>>>>> drivers/Makefile | 1 +
>>>>>>> drivers/interconnect/Kconfig | 10 +
>>>>>>> drivers/interconnect/Makefile | 6 +
>>>>>>> drivers/interconnect/interconnect-uclass.c | 548 +++++++++++++++++++++++++++++
>>>>>>> include/dm/uclass-id.h | 2 +
>>>>>>> include/interconnect-uclass.h | 74 ++++
>>>>>>> include/interconnect.h | 81 +++++
>>>>>>> 8 files changed, 724 insertions(+)
>>>>>>
>>>>>> 'interconnect' is such a long and vague word :-)
>>>>>
>>>>> I didn't chose the name => https://docs.kernel.org/driver-api/interconnect.html
>>>>>
>>>>> I don't see why we should take a different name in U-Boot.
>>>>>
>>>>> But I can definitely use "Generic System Interconnect" in the documentation
>>>>> and commit messages if it's clearer.
>>>>
>>>> Yes, we should use the same name. Docs can help.
>>>>
>>>>>
>>>>>>
>>>>>> There seems to be a piece missing here. The uclass should have
>>>>>> operations, so that it makes sense for different drivers to implement
>>>>>> the methods. I see some there, but also a lot of direct calls from the
>>>>>> test to sandbox. I see quite a few functions which lack a struct
>>>>>> udevice * - I suspect I am missing something?
>>>>>>
>>>>>> We normally have a ..._get_ops() macro in the uclass header. Also we
>>>>>> normally have stub functions which get the ops and call the function.
>>>>>>
>>>>>> I see code to remove children, but driver model does this
>>>>>> automatically[1]. Same with bind(). If there is a bug in this, can you
>>>>>> give a few details, or ideally a test case?
>>>>>
>>>>> All of this is present and well thought, I went into multiple designs:
>>>>> - interconnect uclass provides ops -> interconnect_ops, icc_node doesn't
>>>>> provide any ops since they are just internal objects
>>>>> - there's only a single direct call to the sandbox driver to get the
>>>>> setting resulting of the aggregate() & set() op calls, this is purely for
>>>>> validation of the calculations, the sandbox-interconnect-test only uses
>>>>> the public inteconnect.h API
>>>>
>>>> OK, I missed that.
>>>>
>>>>> - For the lack of udevice, it's because I did use the same interconnect API which
>>>>> return a "icc_path" private structure which contains a set of interconnect
>>>>> nodes which forms path across the different providers.
>>>>
>>>> We do a similar think with clk, although the function signature is
>>>> different in U-Boot from Linux.
>>>>
>>>> I assume the caller would be a driver? If so, passing in a struct
>>>> icc_path * might be more efficient than using malloc() each time.
>>>
>>> I'm not against that but the size of icc_path depends on it's size:
>>>
>>> +struct icc_path {
>>> + size_t num_nodes;
>>> + struct icc_req reqs[];
>>> +};
>>>
>>> So we would still need to alloc the reqs anyway and expose the internals
>>> with no benefits.
>>
>> Indeed. Sometimes in U-Boot we use fixed sizes to avoid realloc(), but
>> I'm not sure if it makes sense here.
>>
>>>
>>>>
>>>>>
>>>>> Setting a bandwidth on a node is meaningless, same for interconnect providers,
>>>>> this is why we set a bandwidth on a set of nodes, which get aggregated with the
>>>>> other path votes from other devices.
>>>>>
>>>>> The key here is the "nodes" which are subdevices of interconnect providers
>>>>> created at bind time, and gets referenced in icc_path objects.
>>>>>
>>>>> I made sure we can't remove either interconnect devices or icc_node devices
>>>>> if they are still referenced in an active icc_path.
>>>>
>>>> OK
>>>>
>>>>> This is the reason of the device_remove() & device_unbind() calls in the test
>>>>> to make sure we're not left with dangling pointers to non-existent icc_node
>>>>> and providers devices.
>>>>
>>>> The specific code is this:
>>>>
>>>> + device_foreach_child(pos, dev) {
>>>> + debug("%s(pos=%s)\n", __func__, pos->name);
>>>> + ret = device_remove(pos, DM_REMOVE_NORMAL);
>>>> + if (ret)
>>>> + return ret;
>>>> + }
>>>>
>>>> This is in the pre_remove() method. But if you just remove that code,
>>>> doesn't driver model remove the children anyway?
>>>
>>> No it only removes the children from the same driver:
>>> =============><=====================
>>> int device_chld_remove(struct udevice *dev, struct driver *drv,
>>> uint flags)
>>> {
>>>
>>> <snip>
>>>
>>> device_foreach_child_safe(pos, n, dev) {
>>> int ret;
>>>
>>> if (drv && (pos->driver != drv))
>>> continue;
>>> =============><=====================
>>> Same for unbind.
>>>
>>> Added in https://lists.denx.de/pipermail/u-boot/2018-June/332597.html with the bind/unbind command.
>>
>> Ick, that is a bug.
>>
>>>
>>> So perhaps I'm doing something unintended here, I was
>>> also expecting removing/unbinding a device would remove all the
>>> childs whatever the uclass.
>>
>> Yes, it should be fixed.
>
> I guess it should be fixed and a test case added.
>
> Should I handle this ?
Wait the code is right, but still it doesn't work properly.
I'll investigate further to understand.
Neil
>
> Neil
>
>>
>> Regards,
>> Simon
>>
>>>
>>> Neil
>>>
>>>>
>>>>>
>>>>> Note I won't re-invent a new API here, the api in interconnect.h is fine and
>>>>> works well, I made sure to use all U-Boot driver model offers to avoir allocating
>>>>> random objects myself, and keep the interconnect core functions strictly
>>>>> identical to the kernel, making it:
>>>>> - well integrated in DM (on-demand probe, nodes as DM devices)
>>>>> - resulting with the same results as in Linux
>>>>> - using the same API
>>>>> - easy to sync back from Linux
>>>>> - easy to port a driver from Linux
>>>>
>>>> Yes that sounds good.
>>>>
>>>>>
>>>>>>
>>>>>> Also, please can you add:
>>>>>>
>>>>>> - comments to the header file, at least
>>>>>> - something in doc/
>>>>>> - a command to list things / provide information
>>>>>
>>>>> Yep, I'll add those.
>>>>
>>>> OK thanks.
>>>>
>>>>>
>>>>> Thanks,
>>>>> Neil
>>>>>
>>>>>>
>>>>>> Great to see the automated tests!
>>>>>>
>>>>>> Regards,
>>>>>> Simon
>>>>>>
>>>>>> [1] https://docs.u-boot.org/en/latest/develop/driver-model/design.html#removal-stage
>>>>>
>>>>
>>>> Regards,
>>>> Simon
>>>
>
More information about the U-Boot
mailing list