[PATCH v2 1/7] Introduce the Interconnect Subsystem

[Neil Armstrong]

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.

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

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

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

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

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