Re: [PATCH] coresight: Add a KUnit test for coresight_find_default_sink()

From: Leo Yan
Date: Wed Mar 05 2025 - 06:42:26 EST


On Tue, Dec 17, 2024 at 05:11:31PM +0000, James Clark wrote:
>
> Add a test to confirm that default sink selection skips over an ETF
> and returns an ETR even if it's further away.
>
> This also makes it easier to add new unit tests in the future.
>
> Signed-off-by: James Clark <james.clark@xxxxxxxxxx>
> ---
> drivers/hwtracing/coresight/Kconfig | 9 ++
> drivers/hwtracing/coresight/Makefile | 1 +
> drivers/hwtracing/coresight/coresight-core.c | 1 +
> .../coresight/coresight-kunit-tests.c | 90 +++++++++++++++++++
> 4 files changed, 101 insertions(+)
> create mode 100644 drivers/hwtracing/coresight/coresight-kunit-tests.c
>
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 06f0a7594169..056d04bc540a 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -247,4 +247,13 @@ config CORESIGHT_DUMMY
>
> To compile this driver as a module, choose M here: the module will be
> called coresight-dummy.
> +
> +config CORESIGHT_KUNIT_TESTS
> + tristate "Enable Coresight unit tests"
> + depends on KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + Enable Coresight unit tests. Only useful for development and not
> + intended for production.
> +
> endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 4ba478211b31..c170a41b3056 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -51,3 +51,4 @@ coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \
> coresight-cti-sysfs.o
> obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o
> obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o
> +obj-$(CONFIG_CORESIGHT_KUNIT_TESTS) += coresight-kunit-tests.o
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index ea38ecf26fcb..ce63b68d5503 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -869,6 +869,7 @@ coresight_find_default_sink(struct coresight_device *csdev)
> }
> return csdev->def_sink;
> }
> +EXPORT_SYMBOL_GPL(coresight_find_default_sink);
>
> static int coresight_remove_sink_ref(struct device *dev, void *data)
> {
> diff --git a/drivers/hwtracing/coresight/coresight-kunit-tests.c b/drivers/hwtracing/coresight/coresight-kunit-tests.c
> new file mode 100644
> index 000000000000..d022bacc8357
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-kunit-tests.c
> @@ -0,0 +1,90 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <kunit/test.h>
> +#include <kunit/device.h>
> +#include <linux/coresight.h>
> +
> +#include "coresight-priv.h"
> +
> +static struct coresight_device *coresight_test_device(struct device *dev)
> +{
> + struct coresight_device *csdev = devm_kcalloc(dev, 1,
> + sizeof(struct coresight_device),
> + GFP_KERNEL);
> + csdev->pdata = devm_kcalloc(dev, 1,
> + sizeof(struct coresight_platform_data),
> + GFP_KERNEL);
> + return csdev;
> +}
> +
> +static void coresight_free_test_device(struct device *dev,
> + struct coresight_device *csdev)
> +{
> + devm_kfree(dev, csdev->pdata->out_conns);

It will lead memory leaking if we don't release connections pointed by
'out_conns' pointer array. Something like:

for (i = 0; i < csdev->pdata->nr_outconns; i++)
devm_kfree(dev, csdev->pdata->out_conns[i]);
devm_kfree(dev, csdev->pdata->out_conns);

Otherwise, it looks good to me.

Thanks,
Leo

> + devm_kfree(dev, csdev->pdata);
> + devm_kfree(dev, csdev);
> +}
> +
> +static int coresight_test_cpuid(struct coresight_device *csdev)
> +{
> + return 0;
> +}
> +
> +static void test_default_sink(struct kunit *test)
> +{
> + /*
> + * ETM -> ETF -> ETR -> CATU
> + * ^
> + * | default
> + */
> + struct device *dev = kunit_device_register(test, "coresight_kunit");
> + struct coresight_device *etm = coresight_test_device(dev),
> + *etf = coresight_test_device(dev),
> + *etr = coresight_test_device(dev),
> + *catu = coresight_test_device(dev);
> + struct coresight_connection conn = {};
> + struct coresight_ops_source src_ops = {.cpu_id = coresight_test_cpuid };
> + struct coresight_ops etm_cs_ops = { .source_ops = &src_ops };
> +
> + etm->type = CORESIGHT_DEV_TYPE_SOURCE;
> + etm->ops = &etm_cs_ops;
> + etf->type = CORESIGHT_DEV_TYPE_LINKSINK;
> + etf->subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
> + etr->type = CORESIGHT_DEV_TYPE_SINK;
> + etr->subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM;
> + catu->type = CORESIGHT_DEV_TYPE_HELPER;
> +
> + conn.src_dev = etm;
> + conn.dest_dev = etf;
> + coresight_add_out_conn(dev, etm->pdata, &conn);
> +
> + conn.src_dev = etf;
> + conn.dest_dev = etr;
> + coresight_add_out_conn(dev, etf->pdata, &conn);
> +
> + conn.src_dev = etr;
> + conn.dest_dev = catu;
> + coresight_add_out_conn(dev, etr->pdata, &conn);
> +
> + KUNIT_ASSERT_PTR_EQ(test, coresight_find_default_sink(etm), etr);
> +
> + coresight_free_test_device(dev, etm);
> + coresight_free_test_device(dev, etf);
> + coresight_free_test_device(dev, etr);
> + coresight_free_test_device(dev, catu);
> +}
> +
> +static struct kunit_case coresight_testcases[] = {
> + KUNIT_CASE(test_default_sink),
> + {}
> +};
> +
> +static struct kunit_suite coresight_test_suite = {
> + .name = "coresight_test_suite",
> + .test_cases = coresight_testcases,
> +};
> +
> +kunit_test_suites(&coresight_test_suite);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("James Clark <james.clark@xxxxxxxxxx>");
> +MODULE_DESCRIPTION("Arm CoreSight KUnit tests");
> --
> 2.34.1
>
> _______________________________________________
> CoreSight mailing list -- coresight@xxxxxxxxxxxxxxxx
> To unsubscribe send an email to coresight-leave@xxxxxxxxxxxxxxxx