Re: [PATCH] interconnect: Add kunit tests for core functionality

[Kuan-Wei Chiu <visitorckw@gmail.com>]

Hi Georgi,

Thanks for the review!

On Sat, Jan 10, 2026 at 01:09:03AM +0200, Georgi Djakov wrote:
> On 12/10/25 8:00 PM, Kuan-Wei Chiu wrote:
> > The interconnect framework currently lacks in-tree unit tests to verify
> > the core logic in isolation. This makes it difficult to validate
> > regression stability when modifying the provider/consumer APIs or
> > aggregation logic.
> > 
> > Introduce a kunit test suite that verifies the fundamental behavior of
> > the subsystem. The tests cover:
> > - Provider API (node creation, linking, topology construction).
> > - Consumer API (path enabling/disabling, bandwidth requests).
> > - Standard aggregation logic (accumulating bandwidth across links).
> > - Bulk operations for setting bandwidth on multiple paths.
> > 
> > The suite simulates a simple SoC topology with multiple masters and a
> > shared bus to validate traffic aggregation behavior in a controlled
> > software environment, without requiring specific hardware or Device
> > Tree support.
> > 
> > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > ---
> > Build and kunit tests passed
> > 
> >   drivers/interconnect/Kconfig     |  14 ++
> >   drivers/interconnect/Makefile    |   2 +
> >   drivers/interconnect/icc-kunit.c | 315 +++++++++++++++++++++++++++++++
> >   3 files changed, 331 insertions(+)
> >   create mode 100644 drivers/interconnect/icc-kunit.c
> > 
> > diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> > index f2e49bd97d31..882dcb0b4a5b 100644
> > --- a/drivers/interconnect/Kconfig
> > +++ b/drivers/interconnect/Kconfig
> > @@ -22,4 +22,18 @@ config INTERCONNECT_CLK
> >   	help
> >   	  Support for wrapping clocks into the interconnect nodes.
> > +config INTERCONNECT_KUNIT_TEST
> > +	tristate "KUnit tests for Interconnect framework"
> > +	depends on KUNIT
> > +	default KUNIT_ALL_TESTS
> > +	help
> > +	  This builds the KUnit test suite for the generic system interconnect
> > +	  framework.
> > +
> > +	  The tests cover the core functionality of the interconnect subsystem,
> > +	  including provider/consumer APIs, topology management, and bandwidth
> > +	  aggregation logic.
> > +
> > +	  If unsure, say N.
> > +
> >   endif
> > diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> > index b0a9a6753b9d..dc4c7b657c9d 100644
> > --- a/drivers/interconnect/Makefile
> > +++ b/drivers/interconnect/Makefile
> > @@ -10,3 +10,5 @@ obj-$(CONFIG_INTERCONNECT_QCOM)		+= qcom/
> >   obj-$(CONFIG_INTERCONNECT_SAMSUNG)	+= samsung/
> >   obj-$(CONFIG_INTERCONNECT_CLK)		+= icc-clk.o
> > +
> > +obj-$(CONFIG_INTERCONNECT_KUNIT_TEST) += icc-kunit.o
> > diff --git a/drivers/interconnect/icc-kunit.c b/drivers/interconnect/icc-kunit.c
> > new file mode 100644
> > index 000000000000..2178487f9527
> > --- /dev/null
> > +++ b/drivers/interconnect/icc-kunit.c
> > @@ -0,0 +1,315 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * KUnit tests for the Interconnect framework.
> > + *
> > + * Copyright (c) 2025 Kuan-Wei Chiu <visitorckw@gmail.com>
> > + *
> > + * This suite verifies the behavior of the interconnect core, including
> > + * topology construction, bandwidth aggregation, and path lifecycle.
> > + */
> > +
> > +#include <kunit/platform_device.h>
> > +#include <kunit/test.h>
> > +#include <linux/interconnect-provider.h>
> > +#include <linux/interconnect.h>
> > +#include <linux/list.h>
> > +#include <linux/module.h>
> > +#include <linux/overflow.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +#include "internal.h"
> > +
> > +enum {
> > +	NODE_CPU = 100,
> > +	NODE_GPU,
> > +	NODE_BUS,
> > +	NODE_DDR,
> > +	NODE_MAX
> > +};
> > +
> > +struct test_node_data {
> > +	int id;
> > +	const char *name;
> > +	int num_links;
> > +	int links[2];
> > +};
> > +
> > +/*
> > + * Static Topology:
> > + * CPU -\
> > + * -> BUS -> DDR
> > + * GPU -/
> > + */
> > +static const struct test_node_data test_topology[] = {
> > +	{ NODE_CPU, "cpu", 1, { NODE_BUS } },
> > +	{ NODE_GPU, "gpu", 1, { NODE_BUS } },
> > +	{ NODE_BUS, "bus", 1, { NODE_DDR } },
> > +	{ NODE_DDR, "ddr", 0, { } },
> > +};
> > +
> > +struct icc_test_priv {
> > +	struct icc_provider provider;
> > +	struct platform_device *pdev;
> > +	struct icc_node *nodes[NODE_MAX];
> 
> So nodes[] is a 104-element array? Is this intentional?

That was indeed an oversight.
In v2, I will simplify the test by making the node id 0 based.

> 
> [..]
> > +static void icc_test_set_bw(struct kunit *test)
> > +{
> > +	struct icc_test_priv *priv = test->priv;
> > +	struct icc_path *path;
> > +	struct icc_node *path_nodes[3];
> > +	int ret;
> > +
> > +	/* Path: CPU -> BUS -> DDR */
> > +	path_nodes[0] = get_node(priv, NODE_CPU);
> > +	path_nodes[1] = get_node(priv, NODE_BUS);
> > +	path_nodes[2] = get_node(priv, NODE_DDR);
> > +
> > +	path = icc_test_create_path(test, path_nodes, 3);
> > +
> > +	ret = icc_enable(path);
> > +	KUNIT_ASSERT_EQ(test, ret, 0);
> > +
> > +	ret = icc_set_bw(path, 1000, 2000);
> > +	KUNIT_EXPECT_EQ(test, ret, 0);
> > +
> > +	KUNIT_EXPECT_EQ(test, path_nodes[0]->avg_bw, 1000);
> > +	KUNIT_EXPECT_EQ(test, path_nodes[0]->peak_bw, 2000);
> > +	KUNIT_EXPECT_EQ(test, path_nodes[1]->avg_bw, 1000);
> > +	KUNIT_EXPECT_EQ(test, path_nodes[1]->peak_bw, 2000);
> > +
> > +	icc_set_tag(path, 0xABC);
> > +	KUNIT_EXPECT_EQ(test, path->reqs[0].tag, 0xABC);
> > +
> > +	icc_disable(path);
> > +	KUNIT_EXPECT_EQ(test, path_nodes[0]->avg_bw, 0);
> > +
> > +	icc_test_destroy_path(test, path);
> > +}
> > +
> 
> I also tried to run it and noticed that one of the tests is failing on my board...
> 
> KTAP version 1
> 1..1
>     KTAP version 1
>     # Subtest: interconnect
>     1..4
>     ok 1 icc_test_topology_integrity
>     # icc_test_set_bw: EXPECTATION FAILED at drivers/interconnect/icc-kunit.c:207
>     Expected path_nodes[0]->avg_bw == 1000, but
>         path_nodes[0]->avg_bw == 2147483647 (0x7fffffff)
>     # icc_test_set_bw: EXPECTATION FAILED at drivers/interconnect/icc-kunit.c:208
>     Expected path_nodes[0]->peak_bw == 2000, but
>         path_nodes[0]->peak_bw == 2147483647 (0x7fffffff)
>     # icc_test_set_bw: EXPECTATION FAILED at drivers/interconnect/icc-kunit.c:209
>     Expected path_nodes[1]->avg_bw == 1000, but
>         path_nodes[1]->avg_bw == 2147483647 (0x7fffffff)
>     # icc_test_set_bw: EXPECTATION FAILED at drivers/interconnect/icc-kunit.c:210
>     Expected path_nodes[1]->peak_bw == 2000, but
>         path_nodes[1]->peak_bw == 2147483647 (0x7fffffff)
>     # icc_test_set_bw: EXPECTATION FAILED at drivers/interconnect/icc-kunit.c:216
>     Expected path_nodes[0]->avg_bw == 0, but
>         path_nodes[0]->avg_bw == 2147483647 (0x7fffffff)
>     not ok 2 icc_test_set_bw
>     ok 3 icc_test_aggregation
>     ok 4 icc_test_bulk_ops
>     # module: icc_kunit
> # interconnect: pass:3 fail:1 skip:0 total:4
> # Totals: pass:3 fail:1 skip:0 total:4
> not ok 1 interconnect
> 
> ...and the following diff seem to fix it:
> 
> diff --git a/drivers/interconnect/icc-kunit.c b/drivers/interconnect/icc-kunit.c
> index 2178487f9527..060f640818a5 100644
> --- a/drivers/interconnect/icc-kunit.c
> +++ b/drivers/interconnect/icc-kunit.c
> @@ -79,6 +79,14 @@ static struct icc_node *icc_test_xlate(const struct of_phandle_args *spec, void
>  	return NULL;
>  }
> 
> +static int icc_test_get_bw(struct icc_node *node, u32 *avg, u32 *peak)
> +{
> +	*avg = 0;
> +	*peak = 0;
> +
> +	return 0;
> +}
> +
>  static int icc_test_init(struct kunit *test)
>  {
>  	struct icc_test_priv *priv;
> @@ -95,6 +103,7 @@ static int icc_test_init(struct kunit *test)
> 
>  	priv->provider.set = icc_test_set;
>  	priv->provider.aggregate = icc_test_aggregate;
> +	priv->provider.get_bw = icc_test_get_bw;
>  	priv->provider.xlate = icc_test_xlate;
>  	priv->provider.dev = &priv->pdev->dev;
>  	priv->provider.data = priv;
> 
> 
> Could you please update and re-send?

Thanks for pointing this out.
I will include this callback in v2.

Will send the v2 patch shortly.

Regards,
Kuan-Wei