[PATCH v2 1/7] Introduce the Interconnect Subsystem
Neil Armstrong
neil.armstrong at linaro.org
Thu Oct 9 18:43:46 CEST 2025
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 ?
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