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

James Clark posted 1 patch 1 year ago
There is a newer version of this series
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
[PATCH] coresight: Add a KUnit test for coresight_find_default_sink()
Posted by James Clark 1 year ago
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@linaro.org>
---
 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);
+	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@linaro.org>");
+MODULE_DESCRIPTION("Arm CoreSight KUnit tests");
-- 
2.34.1
Re: [PATCH] coresight: Add a KUnit test for coresight_find_default_sink()
Posted by Leo Yan 9 months, 2 weeks ago
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@linaro.org>
> ---
>  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@linaro.org>");
> +MODULE_DESCRIPTION("Arm CoreSight KUnit tests");
> --
> 2.34.1
> 
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
Re: [PATCH] coresight: Add a KUnit test for coresight_find_default_sink()
Posted by James Clark 9 months, 2 weeks ago

On 05/03/2025 11:42 am, Leo Yan wrote:
> 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@linaro.org>
>> ---
>>   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
> 

Ah yeah, I'll just free the whole device instead of the individual 
kfrees then and let devm handle it.