2023-06-16 16:00:49

by Marco Pagani

[permalink] [raw]
Subject: [PATCH v7 0/4] fpga: add initial KUnit tests for the subsystem

This patch set introduces an initial set of KUnit test suites for the
core components of the FPGA subsystem.

Tests can be run using:
[user@localhost linux]$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/fpga/tests

v7:
- Dropped RFC prefix
- Add comments to explain fakes and helper functions
- Changed the implementation of the Bridge used in the Region suite

v6:
- Restructured the code into self-contained test modules
- Added tests for the basic behaviors of the components
- Improved programming tests for the FPGA Manager
- Fixed code/comments mismatch in the list of Bridges test case

v5:
- Removed most of the exported functions using shared buffers for stats
- Moved all KUnit expectations/assertions to the main test module
- Removed standalone use case to simplify the code
- Removed instances counters from fake components (using device.id instead)
- Set header size in the .parse_header op
- Improved bridge get_put_list test case

v4:
- Fix build error

v3:
- Calling fpga_bridges_put() between reconfigurations
- Functions for registering fake modules allocate and return context structs

v2:
- Restructured code into multiple suites to test components in isolation
- Reduced code duplication using init and exit methods
- Using a get_bridges() method to build the list of bridges just before programming
- Regions and Bridges are organized topologically
- Changed bitstream/bit to images
- Allocate images dynamically
- Renamed fpga-tests to fpga-test
- Simplified Kconfig
- Add license info to the fpga-test module

Marco Pagani (4):
fpga: add an initial KUnit suite for the FPGA Manager
fpga: add an initial KUnit suite for the FPGA Bridge
fpga: add an initial KUnit suite for the FPGA Region
fpga: add configuration for the FPGA KUnit test suites.

drivers/fpga/Kconfig | 2 +
drivers/fpga/Makefile | 3 +
drivers/fpga/tests/.kunitconfig | 5 +
drivers/fpga/tests/Kconfig | 11 +
drivers/fpga/tests/Makefile | 5 +
drivers/fpga/tests/fpga-bridge-test.c | 175 +++++++++++++++
drivers/fpga/tests/fpga-mgr-test.c | 302 ++++++++++++++++++++++++++
drivers/fpga/tests/fpga-region-test.c | 211 ++++++++++++++++++
8 files changed, 714 insertions(+)
create mode 100644 drivers/fpga/tests/.kunitconfig
create mode 100644 drivers/fpga/tests/Kconfig
create mode 100644 drivers/fpga/tests/Makefile
create mode 100644 drivers/fpga/tests/fpga-bridge-test.c
create mode 100644 drivers/fpga/tests/fpga-mgr-test.c
create mode 100644 drivers/fpga/tests/fpga-region-test.c


base-commit: 858fd168a95c5b9669aac8db6c14a9aeab446375
--
2.40.1



2023-06-16 16:03:29

by Marco Pagani

[permalink] [raw]
Subject: [PATCH v7 1/4] fpga: add an initial KUnit suite for the FPGA Manager

The suite tests the basic behaviors of the FPGA Manager including
programming using a single contiguous buffer and a scatter gather table.

Signed-off-by: Marco Pagani <[email protected]>
---
drivers/fpga/tests/fpga-mgr-test.c | 302 +++++++++++++++++++++++++++++
1 file changed, 302 insertions(+)
create mode 100644 drivers/fpga/tests/fpga-mgr-test.c

diff --git a/drivers/fpga/tests/fpga-mgr-test.c b/drivers/fpga/tests/fpga-mgr-test.c
new file mode 100644
index 000000000000..70e897dad3b6
--- /dev/null
+++ b/drivers/fpga/tests/fpga-mgr-test.c
@@ -0,0 +1,302 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for the FPGA Manager
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani <[email protected]>
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/scatterlist.h>
+#include <kunit/test.h>
+#include <linux/fpga/fpga-mgr.h>
+
+#define HEADER_FILL 'H'
+#define IMAGE_FILL 'P'
+#define IMAGE_BLOCK 1024
+
+#define HEADER_SIZE IMAGE_BLOCK
+#define IMAGE_SIZE (IMAGE_BLOCK * 4)
+
+struct mgr_stats {
+ bool header_match;
+ bool image_match;
+ u32 seq_num;
+ u32 op_parse_header_seq;
+ u32 op_write_init_seq;
+ u32 op_write_seq;
+ u32 op_write_sg_seq;
+ u32 op_write_complete_seq;
+ enum fpga_mgr_states op_parse_header_state;
+ enum fpga_mgr_states op_write_init_state;
+ enum fpga_mgr_states op_write_state;
+ enum fpga_mgr_states op_write_sg_state;
+ enum fpga_mgr_states op_write_complete_state;
+};
+
+struct mgr_ctx {
+ struct fpga_image_info *img_info;
+ struct fpga_manager *mgr;
+ struct platform_device *pdev;
+ struct mgr_stats stats;
+};
+
+/**
+ * init_test_buffer() - Allocate and initialize a test image in a buffer.
+ * @test: KUnit test context object.
+ * @count: image size in bytes.
+ *
+ * Return: pointer to the newly allocated image.
+ */
+static char *init_test_buffer(struct kunit *test, size_t count)
+{
+ char *buf;
+ size_t i;
+
+ buf = kunit_kzalloc(test, count, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
+
+ for (i = 0; i < count; i++)
+ buf[i] = i < HEADER_SIZE ? HEADER_FILL : IMAGE_FILL;
+
+ return buf;
+}
+
+static int op_parse_header(struct fpga_manager *mgr, struct fpga_image_info *info,
+ const char *buf, size_t count)
+{
+ struct mgr_stats *stats = mgr->priv;
+ size_t i;
+
+ /* Set header_size and data_size for later */
+ info->header_size = HEADER_SIZE;
+ info->data_size = info->count - HEADER_SIZE;
+
+ stats->header_match = true;
+
+ /* Check header */
+ for (i = 0; i < info->header_size; i++)
+ if (buf[i] != HEADER_FILL)
+ stats->header_match = false;
+
+ stats->op_parse_header_state = mgr->state;
+ stats->op_parse_header_seq = stats->seq_num++;
+
+ return 0;
+}
+
+static int op_write_init(struct fpga_manager *mgr, struct fpga_image_info *info,
+ const char *buf, size_t count)
+{
+ struct mgr_stats *stats = mgr->priv;
+
+ stats->op_write_init_state = mgr->state;
+ stats->op_write_init_seq = stats->seq_num++;
+
+ return 0;
+}
+
+static int op_write(struct fpga_manager *mgr, const char *buf, size_t count)
+{
+ struct mgr_stats *stats = mgr->priv;
+ size_t i;
+
+ /* Check image */
+ stats->image_match = true;
+ for (i = 0; i < count; i++)
+ if (buf[i] != IMAGE_FILL)
+ stats->image_match = false;
+
+ stats->op_write_state = mgr->state;
+ stats->op_write_seq = stats->seq_num++;
+
+ return 0;
+}
+
+static int op_write_sg(struct fpga_manager *mgr, struct sg_table *sgt)
+{
+ struct mgr_stats *stats = mgr->priv;
+ struct scatterlist *sg;
+ char *img;
+ unsigned int si, i, j = 0;
+
+ stats->image_match = true;
+
+ /* Check image (write_sg will still get whole image in sg_table) */
+ for_each_sgtable_sg(sgt, sg, si) {
+ img = sg_virt(sg);
+ for (i = 0; i < sg->length; i++) {
+ if (i + j > HEADER_SIZE && img[i] != IMAGE_FILL)
+ stats->image_match = false;
+ }
+ j += i;
+ }
+
+ stats->op_write_sg_state = mgr->state;
+ stats->op_write_sg_seq = stats->seq_num++;
+
+ return 0;
+}
+
+static int op_write_complete(struct fpga_manager *mgr, struct fpga_image_info *info)
+{
+ struct mgr_stats *stats = mgr->priv;
+
+ stats->op_write_complete_state = mgr->state;
+ stats->op_write_complete_seq = stats->seq_num++;
+
+ return 0;
+}
+
+/*
+ * Fake Manager that implements all ops required to check the programming
+ * sequence using a single contiguous buffer and a scatter gather table.
+ */
+static const struct fpga_manager_ops fake_mgr_ops = {
+ .skip_header = true,
+ .parse_header = op_parse_header,
+ .write_init = op_write_init,
+ .write = op_write,
+ .write_sg = op_write_sg,
+ .write_complete = op_write_complete,
+};
+
+static void fpga_mgr_test_get(struct kunit *test)
+{
+ struct mgr_ctx *ctx = test->priv;
+ struct fpga_manager *mgr;
+
+ mgr = fpga_mgr_get(&ctx->pdev->dev);
+ KUNIT_EXPECT_PTR_EQ(test, mgr, ctx->mgr);
+
+ fpga_mgr_put(ctx->mgr);
+}
+
+static void fpga_mgr_test_lock(struct kunit *test)
+{
+ struct mgr_ctx *ctx = test->priv;
+ int ret;
+
+ ret = fpga_mgr_lock(ctx->mgr);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ ret = fpga_mgr_lock(ctx->mgr);
+ KUNIT_EXPECT_EQ(test, ret, -EBUSY);
+
+ fpga_mgr_unlock(ctx->mgr);
+}
+
+/* Check the programming sequence using an image in a buffer */
+static void fpga_mgr_test_img_load_buf(struct kunit *test)
+{
+ struct mgr_ctx *ctx = test->priv;
+ char *img_buf;
+ int ret;
+
+ img_buf = init_test_buffer(test, IMAGE_SIZE);
+
+ ctx->img_info->count = IMAGE_SIZE;
+ ctx->img_info->buf = img_buf;
+
+ ret = fpga_mgr_load(ctx->mgr, ctx->img_info);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_TRUE(test, ctx->stats.header_match);
+ KUNIT_EXPECT_TRUE(test, ctx->stats.image_match);
+
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_parse_header_state, FPGA_MGR_STATE_PARSE_HEADER);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_state, FPGA_MGR_STATE_WRITE_INIT);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_state, FPGA_MGR_STATE_WRITE);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_state, FPGA_MGR_STATE_WRITE_COMPLETE);
+
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_seq, ctx->stats.op_parse_header_seq + 1);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_seq, ctx->stats.op_parse_header_seq + 2);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_seq, ctx->stats.op_parse_header_seq + 3);
+}
+
+/* Check the programming sequence using an image in a scatter gather table */
+static void fpga_mgr_test_img_load_sgt(struct kunit *test)
+{
+ struct mgr_ctx *ctx = test->priv;
+ struct sg_table *sgt;
+ char *img_buf;
+ int ret;
+
+ img_buf = init_test_buffer(test, IMAGE_SIZE);
+
+ sgt = kunit_kzalloc(test, sizeof(*sgt), GFP_KERNEL);
+ ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+ sg_init_one(sgt->sgl, img_buf, IMAGE_SIZE);
+
+ ctx->img_info->sgt = sgt;
+
+ ret = fpga_mgr_load(ctx->mgr, ctx->img_info);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_TRUE(test, ctx->stats.header_match);
+ KUNIT_EXPECT_TRUE(test, ctx->stats.image_match);
+
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_parse_header_state, FPGA_MGR_STATE_PARSE_HEADER);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_state, FPGA_MGR_STATE_WRITE_INIT);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_sg_state, FPGA_MGR_STATE_WRITE);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_state, FPGA_MGR_STATE_WRITE_COMPLETE);
+
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_seq, ctx->stats.op_parse_header_seq + 1);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_sg_seq, ctx->stats.op_parse_header_seq + 2);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_seq, ctx->stats.op_parse_header_seq + 3);
+
+ sg_free_table(ctx->img_info->sgt);
+}
+
+static int fpga_mgr_test_init(struct kunit *test)
+{
+ struct mgr_ctx *ctx;
+
+ ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ ctx->pdev = platform_device_register_simple("mgr_pdev", PLATFORM_DEVID_AUTO, NULL, 0);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->pdev);
+
+ ctx->mgr = devm_fpga_mgr_register(&ctx->pdev->dev, "Fake FPGA Manager", &fake_mgr_ops,
+ &ctx->stats);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(ctx->mgr));
+
+ ctx->img_info = fpga_image_info_alloc(&ctx->pdev->dev);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->img_info);
+
+ test->priv = ctx;
+
+ return 0;
+}
+
+static void fpga_mgr_test_exit(struct kunit *test)
+{
+ struct mgr_ctx *ctx = test->priv;
+
+ fpga_image_info_free(ctx->img_info);
+ platform_device_unregister(ctx->pdev);
+}
+
+static struct kunit_case fpga_mgr_test_cases[] = {
+ KUNIT_CASE(fpga_mgr_test_get),
+ KUNIT_CASE(fpga_mgr_test_lock),
+ KUNIT_CASE(fpga_mgr_test_img_load_buf),
+ KUNIT_CASE(fpga_mgr_test_img_load_sgt),
+ {}
+};
+
+static struct kunit_suite fpga_mgr_suite = {
+ .name = "fpga_mgr",
+ .init = fpga_mgr_test_init,
+ .exit = fpga_mgr_test_exit,
+ .test_cases = fpga_mgr_test_cases,
+};
+
+kunit_test_suite(fpga_mgr_suite);
+
+MODULE_LICENSE("GPL");
--
2.40.1


2023-06-16 16:03:48

by Marco Pagani

[permalink] [raw]
Subject: [PATCH v7 3/4] fpga: add an initial KUnit suite for the FPGA Region

The suite tests the basic behaviors of the FPGA Region including
the programming and the function for finding a specific Region.

Signed-off-by: Marco Pagani <[email protected]>
---
drivers/fpga/tests/fpga-region-test.c | 211 ++++++++++++++++++++++++++
1 file changed, 211 insertions(+)
create mode 100644 drivers/fpga/tests/fpga-region-test.c

diff --git a/drivers/fpga/tests/fpga-region-test.c b/drivers/fpga/tests/fpga-region-test.c
new file mode 100644
index 000000000000..a502f3f2560d
--- /dev/null
+++ b/drivers/fpga/tests/fpga-region-test.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for the FPGA Region
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani <[email protected]>
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <kunit/test.h>
+#include <linux/platform_device.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/fpga/fpga-bridge.h>
+#include <linux/fpga/fpga-region.h>
+
+struct mgr_stats {
+ u32 write_count;
+};
+
+struct bridge_stats {
+ bool enable;
+ u32 cycles_count;
+};
+
+struct test_ctx {
+ struct fpga_manager *mgr;
+ struct platform_device *mgr_pdev;
+ struct fpga_bridge *bridge;
+ struct platform_device *bridge_pdev;
+ struct fpga_region *region;
+ struct platform_device *region_pdev;
+ struct bridge_stats bridge_stats;
+ struct mgr_stats mgr_stats;
+};
+
+static int op_write(struct fpga_manager *mgr, const char *buf, size_t count)
+{
+ struct mgr_stats *stats = mgr->priv;
+
+ stats->write_count++;
+
+ return 0;
+}
+
+/*
+ * Fake Manager that implements only the write op to count the number of
+ * programming cycles. The internals of the programming sequence are
+ * tested in the Manager suite since they are outside the responsibility
+ * of the Region.
+ */
+static const struct fpga_manager_ops fake_mgr_ops = {
+ .write = op_write,
+};
+
+static int op_enable_set(struct fpga_bridge *bridge, bool enable)
+{
+ struct bridge_stats *stats = bridge->priv;
+
+ if (!stats->enable && enable)
+ stats->cycles_count++;
+
+ stats->enable = enable;
+
+ return 0;
+}
+
+/*
+ * Fake Bridge that implements only enable_set op to count the number of
+ * activation cycles.
+ */
+static const struct fpga_bridge_ops fake_bridge_ops = {
+ .enable_set = op_enable_set
+};
+
+static int fake_region_get_bridges(struct fpga_region *region)
+{
+ struct fpga_bridge *bridge = region->priv;
+
+ return fpga_bridge_get_to_list(bridge->dev.parent, region->info, &region->bridge_list);
+}
+
+static int fake_region_match(struct device *dev, const void *data)
+{
+ return dev->parent == data;
+}
+
+static void fpga_region_test_class_find(struct kunit *test)
+{
+ struct test_ctx *ctx = test->priv;
+ struct fpga_region *region;
+
+ region = fpga_region_class_find(NULL, &ctx->region_pdev->dev, fake_region_match);
+ KUNIT_EXPECT_PTR_EQ(test, region, ctx->region);
+}
+
+/*
+ * Region programming test. The Region must call get_bridges() to get and
+ * control the bridges, and then the Manager for the actual programming.
+ */
+static void fpga_region_test_program_fpga(struct kunit *test)
+{
+ struct test_ctx *ctx = test->priv;
+ struct fpga_image_info *img_info;
+ char img_buf[4];
+ int ret;
+
+ img_info = fpga_image_info_alloc(&ctx->mgr_pdev->dev);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, img_info);
+
+ img_info->buf = img_buf;
+ img_info->count = sizeof(img_buf);
+
+ ctx->region->info = img_info;
+ ret = fpga_region_program_fpga(ctx->region);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_EQ(test, 1, ctx->mgr_stats.write_count);
+ KUNIT_EXPECT_EQ(test, 1, ctx->bridge_stats.cycles_count);
+
+ fpga_bridges_put(&ctx->region->bridge_list);
+
+ ret = fpga_region_program_fpga(ctx->region);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_EQ(test, 2, ctx->mgr_stats.write_count);
+ KUNIT_EXPECT_EQ(test, 2, ctx->bridge_stats.cycles_count);
+
+ fpga_bridges_put(&ctx->region->bridge_list);
+
+ fpga_image_info_free(img_info);
+}
+
+/*
+ * The configuration used in this test suite uses a single bridge to
+ * limit the code under test to a single unit. The functions used by the
+ * Region for getting and controlling bridges are tested (with a list of
+ * multiple bridges) in the Bridge suite.
+ */
+static int fpga_region_test_init(struct kunit *test)
+{
+ struct test_ctx *ctx;
+ struct fpga_region_info region_info = { 0 };
+
+ ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ ctx->mgr_pdev = platform_device_register_simple("mgr_pdev", PLATFORM_DEVID_AUTO, NULL, 0);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->mgr_pdev);
+
+ ctx->mgr = devm_fpga_mgr_register(&ctx->mgr_pdev->dev, "Fake FPGA Manager", &fake_mgr_ops,
+ &ctx->mgr_stats);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(ctx->mgr));
+
+ ctx->bridge_pdev = platform_device_register_simple("bridge_pdev", PLATFORM_DEVID_AUTO,
+ NULL, 0);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->bridge_pdev);
+
+ ctx->bridge = fpga_bridge_register(&ctx->bridge_pdev->dev, "Fake FPGA Bridge",
+ &fake_bridge_ops, &ctx->bridge_stats);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(ctx->bridge));
+
+ ctx->bridge_stats.enable = true;
+
+ ctx->region_pdev = platform_device_register_simple("region_pdev", PLATFORM_DEVID_AUTO,
+ NULL, 0);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->region_pdev);
+
+ region_info.mgr = ctx->mgr;
+ region_info.priv = ctx->bridge;
+ region_info.get_bridges = fake_region_get_bridges;
+
+ ctx->region = fpga_region_register_full(&ctx->region_pdev->dev, &region_info);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(ctx->region));
+
+ test->priv = ctx;
+
+ return 0;
+}
+
+static void fpga_region_test_exit(struct kunit *test)
+{
+ struct test_ctx *ctx = test->priv;
+
+ fpga_region_unregister(ctx->region);
+ platform_device_unregister(ctx->region_pdev);
+
+ fpga_bridge_unregister(ctx->bridge);
+ platform_device_unregister(ctx->bridge_pdev);
+
+ platform_device_unregister(ctx->mgr_pdev);
+}
+
+static struct kunit_case fpga_region_test_cases[] = {
+ KUNIT_CASE(fpga_region_test_class_find),
+ KUNIT_CASE(fpga_region_test_program_fpga),
+
+ {}
+};
+
+static struct kunit_suite fpga_region_suite = {
+ .name = "fpga_mgr",
+ .init = fpga_region_test_init,
+ .exit = fpga_region_test_exit,
+ .test_cases = fpga_region_test_cases,
+};
+
+kunit_test_suite(fpga_region_suite);
+
+MODULE_LICENSE("GPL");
--
2.40.1


2023-06-16 16:10:40

by Marco Pagani

[permalink] [raw]
Subject: [PATCH v7 2/4] fpga: add an initial KUnit suite for the FPGA Bridge

The suite tests the basic behaviors of the FPGA Bridge including
the functions that operate on a list of Bridges.

Signed-off-by: Marco Pagani <[email protected]>
---
drivers/fpga/tests/fpga-bridge-test.c | 175 ++++++++++++++++++++++++++
1 file changed, 175 insertions(+)
create mode 100644 drivers/fpga/tests/fpga-bridge-test.c

diff --git a/drivers/fpga/tests/fpga-bridge-test.c b/drivers/fpga/tests/fpga-bridge-test.c
new file mode 100644
index 000000000000..fce67ac59a7c
--- /dev/null
+++ b/drivers/fpga/tests/fpga-bridge-test.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for the FPGA Bridge
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani <[email protected]>
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <kunit/test.h>
+#include <linux/fpga/fpga-bridge.h>
+
+struct bridge_stats {
+ bool enable;
+};
+
+struct bridge_ctx {
+ struct fpga_bridge *bridge;
+ struct platform_device *pdev;
+ struct bridge_stats stats;
+};
+
+static int op_enable_set(struct fpga_bridge *bridge, bool enable)
+{
+ struct bridge_stats *stats = bridge->priv;
+
+ stats->enable = enable;
+
+ return 0;
+}
+
+/*
+ * Fake Bridge that implements only the enable_set op to track
+ * the state.
+ */
+static const struct fpga_bridge_ops fake_bridge_ops = {
+ .enable_set = op_enable_set
+};
+
+/**
+ * register_test_bridge() - Register a fake Bridge for testing.
+ * @test: KUnit test context object.
+ *
+ * Return: Context of the newly registered Bridge.
+ */
+static struct bridge_ctx *register_test_bridge(struct kunit *test)
+{
+ struct bridge_ctx *ctx;
+
+ ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ ctx->pdev = platform_device_register_simple("bridge_pdev", PLATFORM_DEVID_AUTO, NULL, 0);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->pdev);
+
+ ctx->bridge = fpga_bridge_register(&ctx->pdev->dev, "Fake FPGA Bridge", &fake_bridge_ops,
+ &ctx->stats);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(ctx->bridge));
+
+ return ctx;
+}
+
+static void unregister_test_bridge(struct bridge_ctx *ctx)
+{
+ fpga_bridge_unregister(ctx->bridge);
+ platform_device_unregister(ctx->pdev);
+}
+
+static void fpga_bridge_test_get(struct kunit *test)
+{
+ struct bridge_ctx *ctx = test->priv;
+ struct fpga_bridge *bridge;
+
+ bridge = fpga_bridge_get(&ctx->pdev->dev, NULL);
+ KUNIT_EXPECT_PTR_EQ(test, bridge, ctx->bridge);
+
+ bridge = fpga_bridge_get(&ctx->pdev->dev, NULL);
+ KUNIT_EXPECT_EQ(test, PTR_ERR(bridge), -EBUSY);
+
+ fpga_bridge_put(ctx->bridge);
+}
+
+static void fpga_bridge_test_toggle(struct kunit *test)
+{
+ struct bridge_ctx *ctx = test->priv;
+ int ret;
+
+ ret = fpga_bridge_disable(ctx->bridge);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_FALSE(test, ctx->stats.enable);
+
+ ret = fpga_bridge_enable(ctx->bridge);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_TRUE(test, ctx->stats.enable);
+}
+
+/* Test the functions for getting and controlling a list of bridges */
+static void fpga_bridge_test_get_put_list(struct kunit *test)
+{
+ struct list_head bridge_list;
+ struct bridge_ctx *ctx_0, *ctx_1;
+ int ret;
+
+ ctx_0 = test->priv;
+ ctx_1 = register_test_bridge(test);
+
+ INIT_LIST_HEAD(&bridge_list);
+
+ /* Get bridge 0 and add it to the list */
+ ret = fpga_bridge_get_to_list(&ctx_0->pdev->dev, NULL, &bridge_list);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_PTR_EQ(test, ctx_0->bridge,
+ list_first_entry_or_null(&bridge_list, struct fpga_bridge, node));
+
+ /* Get bridge 1 and add it to the list */
+ ret = fpga_bridge_get_to_list(&ctx_1->pdev->dev, NULL, &bridge_list);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_PTR_EQ(test, ctx_1->bridge,
+ list_first_entry_or_null(&bridge_list, struct fpga_bridge, node));
+
+ /* Disable an then enable both bridges from the list */
+ ret = fpga_bridges_disable(&bridge_list);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_FALSE(test, ctx_0->stats.enable);
+ KUNIT_EXPECT_FALSE(test, ctx_1->stats.enable);
+
+ ret = fpga_bridges_enable(&bridge_list);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_TRUE(test, ctx_0->stats.enable);
+ KUNIT_EXPECT_TRUE(test, ctx_1->stats.enable);
+
+ /* Put and remove both bridges from the list */
+ fpga_bridges_put(&bridge_list);
+
+ KUNIT_EXPECT_TRUE(test, list_empty(&bridge_list));
+
+ unregister_test_bridge(ctx_1);
+}
+
+static int fpga_bridge_test_init(struct kunit *test)
+{
+ test->priv = register_test_bridge(test);
+
+ return 0;
+}
+
+static void fpga_bridge_test_exit(struct kunit *test)
+{
+ unregister_test_bridge(test->priv);
+}
+
+static struct kunit_case fpga_bridge_test_cases[] = {
+ KUNIT_CASE(fpga_bridge_test_get),
+ KUNIT_CASE(fpga_bridge_test_toggle),
+ KUNIT_CASE(fpga_bridge_test_get_put_list),
+ {}
+};
+
+static struct kunit_suite fpga_bridge_suite = {
+ .name = "fpga_bridge",
+ .init = fpga_bridge_test_init,
+ .exit = fpga_bridge_test_exit,
+ .test_cases = fpga_bridge_test_cases,
+};
+
+kunit_test_suite(fpga_bridge_suite);
+
+MODULE_LICENSE("GPL");
--
2.40.1


2023-06-16 16:15:45

by Marco Pagani

[permalink] [raw]
Subject: [PATCH v7 4/4] fpga: add configuration for the FPGA KUnit test suites.

Add configuration for the KUnit test suites for the core components
of the FPGA subsystem.

Signed-off-by: Marco Pagani <[email protected]>
---
drivers/fpga/Kconfig | 2 ++
drivers/fpga/Makefile | 3 +++
drivers/fpga/tests/.kunitconfig | 5 +++++
drivers/fpga/tests/Kconfig | 11 +++++++++++
drivers/fpga/tests/Makefile | 5 +++++
5 files changed, 26 insertions(+)
create mode 100644 drivers/fpga/tests/.kunitconfig
create mode 100644 drivers/fpga/tests/Kconfig
create mode 100644 drivers/fpga/tests/Makefile

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 0a00763b9f28..2f689ac4ba3a 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -276,4 +276,6 @@ config FPGA_MGR_LATTICE_SYSCONFIG_SPI
FPGA manager driver support for Lattice FPGAs programming over slave
SPI sysCONFIG interface.

+source "drivers/fpga/tests/Kconfig"
+
endif # FPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 72e554b4d2f7..352a2612623e 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -55,3 +55,6 @@ obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000) += dfl-n3000-nios.o

# Drivers for FPGAs which implement DFL
obj-$(CONFIG_FPGA_DFL_PCI) += dfl-pci.o
+
+# KUnit tests
+obj-$(CONFIG_FPGA_KUNIT_TESTS) += tests/
diff --git a/drivers/fpga/tests/.kunitconfig b/drivers/fpga/tests/.kunitconfig
new file mode 100644
index 000000000000..a1c2a2974c39
--- /dev/null
+++ b/drivers/fpga/tests/.kunitconfig
@@ -0,0 +1,5 @@
+CONFIG_KUNIT=y
+CONFIG_FPGA=y
+CONFIG_FPGA_REGION=y
+CONFIG_FPGA_BRIDGE=y
+CONFIG_FPGA_KUNIT_TESTS=y
diff --git a/drivers/fpga/tests/Kconfig b/drivers/fpga/tests/Kconfig
new file mode 100644
index 000000000000..e4a64815f16d
--- /dev/null
+++ b/drivers/fpga/tests/Kconfig
@@ -0,0 +1,11 @@
+config FPGA_KUNIT_TESTS
+ tristate "KUnit test for the FPGA subsystem" if !KUNIT_ALL_TESTS
+ depends on FPGA && FPGA_REGION && FPGA_BRIDGE && KUNIT=y
+ default KUNIT_ALL_TESTS
+ help
+ This builds unit tests for the FPGA subsystem
+
+ For more information on KUnit and unit tests in general,
+ please refer to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
diff --git a/drivers/fpga/tests/Makefile b/drivers/fpga/tests/Makefile
new file mode 100644
index 000000000000..faa5fa230ab0
--- /dev/null
+++ b/drivers/fpga/tests/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_FPGA_KUNIT_TESTS) += fpga-mgr-test.o
+obj-$(CONFIG_FPGA_KUNIT_TESTS) += fpga-bridge-test.o
+obj-$(CONFIG_FPGA_KUNIT_TESTS) += fpga-region-test.o
--
2.40.1


2023-06-25 07:30:26

by Xu Yilun

[permalink] [raw]
Subject: Re: [PATCH v7 1/4] fpga: add an initial KUnit suite for the FPGA Manager

On 2023-06-16 at 17:44:02 +0200, Marco Pagani wrote:
> The suite tests the basic behaviors of the FPGA Manager including
> programming using a single contiguous buffer and a scatter gather table.
>
> Signed-off-by: Marco Pagani <[email protected]>
> ---
> drivers/fpga/tests/fpga-mgr-test.c | 302 +++++++++++++++++++++++++++++
> 1 file changed, 302 insertions(+)
> create mode 100644 drivers/fpga/tests/fpga-mgr-test.c
>
> diff --git a/drivers/fpga/tests/fpga-mgr-test.c b/drivers/fpga/tests/fpga-mgr-test.c
> new file mode 100644
> index 000000000000..70e897dad3b6
> --- /dev/null
> +++ b/drivers/fpga/tests/fpga-mgr-test.c
> @@ -0,0 +1,302 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the FPGA Manager
> + *
> + * Copyright (C) 2023 Red Hat, Inc.
> + *
> + * Author: Marco Pagani <[email protected]>
> + */
> +
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/scatterlist.h>
> +#include <kunit/test.h>
> +#include <linux/fpga/fpga-mgr.h>

alphabetical order please

> +
> +#define HEADER_FILL 'H'
> +#define IMAGE_FILL 'P'
> +#define IMAGE_BLOCK 1024
> +
> +#define HEADER_SIZE IMAGE_BLOCK
> +#define IMAGE_SIZE (IMAGE_BLOCK * 4)
> +
> +struct mgr_stats {
> + bool header_match;
> + bool image_match;
> + u32 seq_num;
> + u32 op_parse_header_seq;
> + u32 op_write_init_seq;
> + u32 op_write_seq;
> + u32 op_write_sg_seq;
> + u32 op_write_complete_seq;
> + enum fpga_mgr_states op_parse_header_state;
> + enum fpga_mgr_states op_write_init_state;
> + enum fpga_mgr_states op_write_state;
> + enum fpga_mgr_states op_write_sg_state;
> + enum fpga_mgr_states op_write_complete_state;
> +};
> +
> +struct mgr_ctx {
> + struct fpga_image_info *img_info;
> + struct fpga_manager *mgr;
> + struct platform_device *pdev;
> + struct mgr_stats stats;
> +};
> +
> +/**
> + * init_test_buffer() - Allocate and initialize a test image in a buffer.
> + * @test: KUnit test context object.
> + * @count: image size in bytes.
> + *
> + * Return: pointer to the newly allocated image.
> + */
> +static char *init_test_buffer(struct kunit *test, size_t count)
> +{
> + char *buf;
> + size_t i;
> +
> + buf = kunit_kzalloc(test, count, GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
> +
> + for (i = 0; i < count; i++)
> + buf[i] = i < HEADER_SIZE ? HEADER_FILL : IMAGE_FILL;

memset?

> +
> + return buf;
> +}
> +
> +static int op_parse_header(struct fpga_manager *mgr, struct fpga_image_info *info,
> + const char *buf, size_t count)
> +{
> + struct mgr_stats *stats = mgr->priv;
> + size_t i;
> +
> + /* Set header_size and data_size for later */
> + info->header_size = HEADER_SIZE;
> + info->data_size = info->count - HEADER_SIZE;
> +
> + stats->header_match = true;
> +
> + /* Check header */
> + for (i = 0; i < info->header_size; i++)
> + if (buf[i] != HEADER_FILL)
> + stats->header_match = false;
> +
> + stats->op_parse_header_state = mgr->state;
> + stats->op_parse_header_seq = stats->seq_num++;
> +
> + return 0;
> +}
> +
> +static int op_write_init(struct fpga_manager *mgr, struct fpga_image_info *info,
> + const char *buf, size_t count)
> +{
> + struct mgr_stats *stats = mgr->priv;
> +
> + stats->op_write_init_state = mgr->state;
> + stats->op_write_init_seq = stats->seq_num++;
> +
> + return 0;
> +}
> +
> +static int op_write(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> + struct mgr_stats *stats = mgr->priv;
> + size_t i;
> +
> + /* Check image */
> + stats->image_match = true;
> + for (i = 0; i < count; i++)
> + if (buf[i] != IMAGE_FILL)
> + stats->image_match = false;
> +
> + stats->op_write_state = mgr->state;
> + stats->op_write_seq = stats->seq_num++;
> +
> + return 0;
> +}
> +
> +static int op_write_sg(struct fpga_manager *mgr, struct sg_table *sgt)
> +{
> + struct mgr_stats *stats = mgr->priv;
> + struct scatterlist *sg;
> + char *img;
> + unsigned int si, i, j = 0;
> +
> + stats->image_match = true;
> +
> + /* Check image (write_sg will still get whole image in sg_table) */
> + for_each_sgtable_sg(sgt, sg, si) {
> + img = sg_virt(sg);
> + for (i = 0; i < sg->length; i++) {
> + if (i + j > HEADER_SIZE && img[i] != IMAGE_FILL)

Is it possible just use sg_miter_skip() to skip the image header?

> + stats->image_match = false;
> + }
> + j += i;
> + }
> +
> + stats->op_write_sg_state = mgr->state;
> + stats->op_write_sg_seq = stats->seq_num++;
> +
> + return 0;
> +}
> +
> +static int op_write_complete(struct fpga_manager *mgr, struct fpga_image_info *info)
> +{
> + struct mgr_stats *stats = mgr->priv;
> +
> + stats->op_write_complete_state = mgr->state;
> + stats->op_write_complete_seq = stats->seq_num++;
> +
> + return 0;
> +}
> +
> +/*
> + * Fake Manager that implements all ops required to check the programming
> + * sequence using a single contiguous buffer and a scatter gather table.
> + */
> +static const struct fpga_manager_ops fake_mgr_ops = {
> + .skip_header = true,
> + .parse_header = op_parse_header,
> + .write_init = op_write_init,
> + .write = op_write,
> + .write_sg = op_write_sg,
> + .write_complete = op_write_complete,
> +};
> +
> +static void fpga_mgr_test_get(struct kunit *test)
> +{
> + struct mgr_ctx *ctx = test->priv;
> + struct fpga_manager *mgr;
> +
> + mgr = fpga_mgr_get(&ctx->pdev->dev);
> + KUNIT_EXPECT_PTR_EQ(test, mgr, ctx->mgr);
> +
> + fpga_mgr_put(ctx->mgr);
> +}
> +
> +static void fpga_mgr_test_lock(struct kunit *test)
> +{
> + struct mgr_ctx *ctx = test->priv;
> + int ret;
> +
> + ret = fpga_mgr_lock(ctx->mgr);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + ret = fpga_mgr_lock(ctx->mgr);
> + KUNIT_EXPECT_EQ(test, ret, -EBUSY);
> +
> + fpga_mgr_unlock(ctx->mgr);
> +}
> +
> +/* Check the programming sequence using an image in a buffer */
> +static void fpga_mgr_test_img_load_buf(struct kunit *test)
> +{
> + struct mgr_ctx *ctx = test->priv;
> + char *img_buf;
> + int ret;
> +
> + img_buf = init_test_buffer(test, IMAGE_SIZE);
> +
> + ctx->img_info->count = IMAGE_SIZE;
> + ctx->img_info->buf = img_buf;
> +
> + ret = fpga_mgr_load(ctx->mgr, ctx->img_info);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + KUNIT_EXPECT_TRUE(test, ctx->stats.header_match);
> + KUNIT_EXPECT_TRUE(test, ctx->stats.image_match);
> +
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_parse_header_state, FPGA_MGR_STATE_PARSE_HEADER);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_state, FPGA_MGR_STATE_WRITE_INIT);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_state, FPGA_MGR_STATE_WRITE);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_state, FPGA_MGR_STATE_WRITE_COMPLETE);
> +
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_seq, ctx->stats.op_parse_header_seq + 1);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_seq, ctx->stats.op_parse_header_seq + 2);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_seq, ctx->stats.op_parse_header_seq + 3);
> +}
> +
> +/* Check the programming sequence using an image in a scatter gather table */
> +static void fpga_mgr_test_img_load_sgt(struct kunit *test)
> +{
> + struct mgr_ctx *ctx = test->priv;
> + struct sg_table *sgt;
> + char *img_buf;
> + int ret;
> +
> + img_buf = init_test_buffer(test, IMAGE_SIZE);
> +
> + sgt = kunit_kzalloc(test, sizeof(*sgt), GFP_KERNEL);
> + ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> + sg_init_one(sgt->sgl, img_buf, IMAGE_SIZE);
> +
> + ctx->img_info->sgt = sgt;
> +
> + ret = fpga_mgr_load(ctx->mgr, ctx->img_info);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + KUNIT_EXPECT_TRUE(test, ctx->stats.header_match);
> + KUNIT_EXPECT_TRUE(test, ctx->stats.image_match);
> +
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_parse_header_state, FPGA_MGR_STATE_PARSE_HEADER);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_state, FPGA_MGR_STATE_WRITE_INIT);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_sg_state, FPGA_MGR_STATE_WRITE);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_state, FPGA_MGR_STATE_WRITE_COMPLETE);
> +
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_seq, ctx->stats.op_parse_header_seq + 1);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_sg_seq, ctx->stats.op_parse_header_seq + 2);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_seq, ctx->stats.op_parse_header_seq + 3);
> +
> + sg_free_table(ctx->img_info->sgt);
> +}
> +
> +static int fpga_mgr_test_init(struct kunit *test)
> +{
> + struct mgr_ctx *ctx;
> +
> + ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> + ctx->pdev = platform_device_register_simple("mgr_pdev", PLATFORM_DEVID_AUTO, NULL, 0);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->pdev);
> +
> + ctx->mgr = devm_fpga_mgr_register(&ctx->pdev->dev, "Fake FPGA Manager", &fake_mgr_ops,
> + &ctx->stats);
> + KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(ctx->mgr));
> +
> + ctx->img_info = fpga_image_info_alloc(&ctx->pdev->dev);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->img_info);
> +
> + test->priv = ctx;
> +
> + return 0;
> +}
> +
> +static void fpga_mgr_test_exit(struct kunit *test)
> +{
> + struct mgr_ctx *ctx = test->priv;
> +
> + fpga_image_info_free(ctx->img_info);
> + platform_device_unregister(ctx->pdev);
> +}
> +
> +static struct kunit_case fpga_mgr_test_cases[] = {
> + KUNIT_CASE(fpga_mgr_test_get),
> + KUNIT_CASE(fpga_mgr_test_lock),
> + KUNIT_CASE(fpga_mgr_test_img_load_buf),
> + KUNIT_CASE(fpga_mgr_test_img_load_sgt),
> + {}
> +};
> +
> +static struct kunit_suite fpga_mgr_suite = {
> + .name = "fpga_mgr",
> + .init = fpga_mgr_test_init,
> + .exit = fpga_mgr_test_exit,
> + .test_cases = fpga_mgr_test_cases,
> +};
> +
> +kunit_test_suite(fpga_mgr_suite);
> +
> +MODULE_LICENSE("GPL");
> --
> 2.40.1
>

2023-06-25 07:43:27

by Xu Yilun

[permalink] [raw]
Subject: Re: [PATCH v7 2/4] fpga: add an initial KUnit suite for the FPGA Bridge

On 2023-06-16 at 17:44:03 +0200, Marco Pagani wrote:
> The suite tests the basic behaviors of the FPGA Bridge including
> the functions that operate on a list of Bridges.
^
why uppercase?

>
> Signed-off-by: Marco Pagani <[email protected]>
> ---
> drivers/fpga/tests/fpga-bridge-test.c | 175 ++++++++++++++++++++++++++
> 1 file changed, 175 insertions(+)
> create mode 100644 drivers/fpga/tests/fpga-bridge-test.c
>
> diff --git a/drivers/fpga/tests/fpga-bridge-test.c b/drivers/fpga/tests/fpga-bridge-test.c
> new file mode 100644
> index 000000000000..fce67ac59a7c
> --- /dev/null
> +++ b/drivers/fpga/tests/fpga-bridge-test.c
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the FPGA Bridge
> + *
> + * Copyright (C) 2023 Red Hat, Inc.
> + *
> + * Author: Marco Pagani <[email protected]>
> + */
> +
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <kunit/test.h>
> +#include <linux/fpga/fpga-bridge.h>

alphabetical order please.

Others LGTM.
Acked-by: Xu Yilun <[email protected]>

2023-06-25 08:03:14

by Xu Yilun

[permalink] [raw]
Subject: Re: [PATCH v7 3/4] fpga: add an initial KUnit suite for the FPGA Region

On 2023-06-16 at 17:44:04 +0200, Marco Pagani wrote:
> The suite tests the basic behaviors of the FPGA Region including
> the programming and the function for finding a specific Region.
>
> Signed-off-by: Marco Pagani <[email protected]>
> ---
> drivers/fpga/tests/fpga-region-test.c | 211 ++++++++++++++++++++++++++
> 1 file changed, 211 insertions(+)
> create mode 100644 drivers/fpga/tests/fpga-region-test.c
>
> diff --git a/drivers/fpga/tests/fpga-region-test.c b/drivers/fpga/tests/fpga-region-test.c
> new file mode 100644
> index 000000000000..a502f3f2560d
> --- /dev/null
> +++ b/drivers/fpga/tests/fpga-region-test.c
> @@ -0,0 +1,211 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the FPGA Region
> + *
> + * Copyright (C) 2023 Red Hat, Inc.
> + *
> + * Author: Marco Pagani <[email protected]>
> + */
> +
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <kunit/test.h>
> +#include <linux/platform_device.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/fpga/fpga-bridge.h>
> +#include <linux/fpga/fpga-region.h>

Ditto

> +
> +struct mgr_stats {
> + u32 write_count;
> +};
> +
> +struct bridge_stats {
> + bool enable;
> + u32 cycles_count;
> +};
> +
> +struct test_ctx {
> + struct fpga_manager *mgr;
> + struct platform_device *mgr_pdev;
> + struct fpga_bridge *bridge;
> + struct platform_device *bridge_pdev;
> + struct fpga_region *region;
> + struct platform_device *region_pdev;
> + struct bridge_stats bridge_stats;
> + struct mgr_stats mgr_stats;
> +};
> +
> +static int op_write(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> + struct mgr_stats *stats = mgr->priv;
> +
> + stats->write_count++;
> +
> + return 0;
> +}
> +
> +/*
> + * Fake Manager that implements only the write op to count the number of
> + * programming cycles. The internals of the programming sequence are
> + * tested in the Manager suite since they are outside the responsibility
> + * of the Region.
> + */
> +static const struct fpga_manager_ops fake_mgr_ops = {
> + .write = op_write,
> +};
> +
> +static int op_enable_set(struct fpga_bridge *bridge, bool enable)
> +{
> + struct bridge_stats *stats = bridge->priv;
> +
> + if (!stats->enable && enable)
> + stats->cycles_count++;
> +
> + stats->enable = enable;
> +
> + return 0;
> +}
> +
> +/*
> + * Fake Bridge that implements only enable_set op to count the number of
> + * activation cycles.
> + */
> +static const struct fpga_bridge_ops fake_bridge_ops = {
> + .enable_set = op_enable_set
^
I prefer to add the comma

Others LGTM.
Acked-by: Xu Yilun <[email protected]>

2023-06-25 08:31:42

by Xu Yilun

[permalink] [raw]
Subject: Re: [PATCH v7 4/4] fpga: add configuration for the FPGA KUnit test suites.

On 2023-06-16 at 17:44:05 +0200, Marco Pagani wrote:
> Add configuration for the KUnit test suites for the core components
> of the FPGA subsystem.
>
> Signed-off-by: Marco Pagani <[email protected]>
> ---
> drivers/fpga/Kconfig | 2 ++
> drivers/fpga/Makefile | 3 +++
> drivers/fpga/tests/.kunitconfig | 5 +++++
> drivers/fpga/tests/Kconfig | 11 +++++++++++
> drivers/fpga/tests/Makefile | 5 +++++
> 5 files changed, 26 insertions(+)
> create mode 100644 drivers/fpga/tests/.kunitconfig
> create mode 100644 drivers/fpga/tests/Kconfig
> create mode 100644 drivers/fpga/tests/Makefile
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 0a00763b9f28..2f689ac4ba3a 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -276,4 +276,6 @@ config FPGA_MGR_LATTICE_SYSCONFIG_SPI
> FPGA manager driver support for Lattice FPGAs programming over slave
> SPI sysCONFIG interface.
>
> +source "drivers/fpga/tests/Kconfig"
> +
> endif # FPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 72e554b4d2f7..352a2612623e 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -55,3 +55,6 @@ obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000) += dfl-n3000-nios.o
>
> # Drivers for FPGAs which implement DFL
> obj-$(CONFIG_FPGA_DFL_PCI) += dfl-pci.o
> +
> +# KUnit tests
> +obj-$(CONFIG_FPGA_KUNIT_TESTS) += tests/
> diff --git a/drivers/fpga/tests/.kunitconfig b/drivers/fpga/tests/.kunitconfig
> new file mode 100644
> index 000000000000..a1c2a2974c39
> --- /dev/null
> +++ b/drivers/fpga/tests/.kunitconfig
> @@ -0,0 +1,5 @@
> +CONFIG_KUNIT=y
> +CONFIG_FPGA=y
> +CONFIG_FPGA_REGION=y
> +CONFIG_FPGA_BRIDGE=y
> +CONFIG_FPGA_KUNIT_TESTS=y
> diff --git a/drivers/fpga/tests/Kconfig b/drivers/fpga/tests/Kconfig
> new file mode 100644
> index 000000000000..e4a64815f16d
> --- /dev/null
> +++ b/drivers/fpga/tests/Kconfig
> @@ -0,0 +1,11 @@
> +config FPGA_KUNIT_TESTS
> + tristate "KUnit test for the FPGA subsystem" if !KUNIT_ALL_TESTS
> + depends on FPGA && FPGA_REGION && FPGA_BRIDGE && KUNIT=y

Why KUNIT shouldn't be 'm'

> + default KUNIT_ALL_TESTS
> + help
> + This builds unit tests for the FPGA subsystem
> +
> + For more information on KUnit and unit tests in general,
> + please refer to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> + If unsure, say N.
> diff --git a/drivers/fpga/tests/Makefile b/drivers/fpga/tests/Makefile
> new file mode 100644
> index 000000000000..faa5fa230ab0
> --- /dev/null
> +++ b/drivers/fpga/tests/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_FPGA_KUNIT_TESTS) += fpga-mgr-test.o
> +obj-$(CONFIG_FPGA_KUNIT_TESTS) += fpga-bridge-test.o
> +obj-$(CONFIG_FPGA_KUNIT_TESTS) += fpga-region-test.o

Could be integrated in one line?

Thanks,
Yilun

> --
> 2.40.1
>

2023-06-27 13:40:53

by Marco Pagani

[permalink] [raw]
Subject: Re: [PATCH v7 1/4] fpga: add an initial KUnit suite for the FPGA Manager



On 2023-06-25 09:11, Xu Yilun wrote:
> On 2023-06-16 at 17:44:02 +0200, Marco Pagani wrote:
>> The suite tests the basic behaviors of the FPGA Manager including
>> programming using a single contiguous buffer and a scatter gather table.
>>
>> Signed-off-by: Marco Pagani <[email protected]>
>> ---
>> drivers/fpga/tests/fpga-mgr-test.c | 302 +++++++++++++++++++++++++++++
>> 1 file changed, 302 insertions(+)
>> create mode 100644 drivers/fpga/tests/fpga-mgr-test.c
>>
>> diff --git a/drivers/fpga/tests/fpga-mgr-test.c b/drivers/fpga/tests/fpga-mgr-test.c
>> new file mode 100644
>> index 000000000000..70e897dad3b6
>> --- /dev/null
>> +++ b/drivers/fpga/tests/fpga-mgr-test.c
>> @@ -0,0 +1,302 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * KUnit test for the FPGA Manager
>> + *
>> + * Copyright (C) 2023 Red Hat, Inc.
>> + *
>> + * Author: Marco Pagani <[email protected]>
>> + */
>> +
>> +#include <linux/types.h>
>> +#include <linux/module.h>
>> +#include <linux/device.h>
>> +#include <linux/scatterlist.h>
>> +#include <kunit/test.h>
>> +#include <linux/fpga/fpga-mgr.h>
>
> alphabetical order please
>

I'll sort all includes alphabetically in the next version.

>> +
>> +#define HEADER_FILL 'H'
>> +#define IMAGE_FILL 'P'
>> +#define IMAGE_BLOCK 1024
>> +
>> +#define HEADER_SIZE IMAGE_BLOCK
>> +#define IMAGE_SIZE (IMAGE_BLOCK * 4)
>> +
>> +struct mgr_stats {
>> + bool header_match;
>> + bool image_match;
>> + u32 seq_num;
>> + u32 op_parse_header_seq;
>> + u32 op_write_init_seq;
>> + u32 op_write_seq;
>> + u32 op_write_sg_seq;
>> + u32 op_write_complete_seq;
>> + enum fpga_mgr_states op_parse_header_state;
>> + enum fpga_mgr_states op_write_init_state;
>> + enum fpga_mgr_states op_write_state;
>> + enum fpga_mgr_states op_write_sg_state;
>> + enum fpga_mgr_states op_write_complete_state;
>> +};
>> +
>> +struct mgr_ctx {
>> + struct fpga_image_info *img_info;
>> + struct fpga_manager *mgr;
>> + struct platform_device *pdev;
>> + struct mgr_stats stats;
>> +};
>> +
>> +/**
>> + * init_test_buffer() - Allocate and initialize a test image in a buffer.
>> + * @test: KUnit test context object.
>> + * @count: image size in bytes.
>> + *
>> + * Return: pointer to the newly allocated image.
>> + */
>> +static char *init_test_buffer(struct kunit *test, size_t count)
>> +{
>> + char *buf;
>> + size_t i;
>> +
>> + buf = kunit_kzalloc(test, count, GFP_KERNEL);
>> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
>> +
>> + for (i = 0; i < count; i++)
>> + buf[i] = i < HEADER_SIZE ? HEADER_FILL : IMAGE_FILL;
>
> memset?
>

Okay, I'll use memset in the next version.

>> +
>> + return buf;
>> +}
>> +
>> +static int op_parse_header(struct fpga_manager *mgr, struct fpga_image_info *info,
>> + const char *buf, size_t count)
>> +{
>> + struct mgr_stats *stats = mgr->priv;
>> + size_t i;
>> +
>> + /* Set header_size and data_size for later */
>> + info->header_size = HEADER_SIZE;
>> + info->data_size = info->count - HEADER_SIZE;
>> +
>> + stats->header_match = true;
>> +
>> + /* Check header */
>> + for (i = 0; i < info->header_size; i++)
>> + if (buf[i] != HEADER_FILL)
>> + stats->header_match = false;
>> +
>> + stats->op_parse_header_state = mgr->state;
>> + stats->op_parse_header_seq = stats->seq_num++;
>> +
>> + return 0;
>> +}
>> +
>> +static int op_write_init(struct fpga_manager *mgr, struct fpga_image_info *info,
>> + const char *buf, size_t count)
>> +{
>> + struct mgr_stats *stats = mgr->priv;
>> +
>> + stats->op_write_init_state = mgr->state;
>> + stats->op_write_init_seq = stats->seq_num++;
>> +
>> + return 0;
>> +}
>> +
>> +static int op_write(struct fpga_manager *mgr, const char *buf, size_t count)
>> +{
>> + struct mgr_stats *stats = mgr->priv;
>> + size_t i;
>> +
>> + /* Check image */
>> + stats->image_match = true;
>> + for (i = 0; i < count; i++)
>> + if (buf[i] != IMAGE_FILL)
>> + stats->image_match = false;
>> +
>> + stats->op_write_state = mgr->state;
>> + stats->op_write_seq = stats->seq_num++;
>> +
>> + return 0;
>> +}
>> +
>> +static int op_write_sg(struct fpga_manager *mgr, struct sg_table *sgt)
>> +{
>> + struct mgr_stats *stats = mgr->priv;
>> + struct scatterlist *sg;
>> + char *img;
>> + unsigned int si, i, j = 0;
>> +
>> + stats->image_match = true;
>> +
>> + /* Check image (write_sg will still get whole image in sg_table) */
>> + for_each_sgtable_sg(sgt, sg, si) {
>> + img = sg_virt(sg);
>> + for (i = 0; i < sg->length; i++) {
>> + if (i + j > HEADER_SIZE && img[i] != IMAGE_FILL)
>
> Is it possible just use sg_miter_skip() to skip the image header?
>

I'll use sg_miter_* functions in the next version.

>> + stats->image_match = false;
>> + }
>> + j += i;
>> + }
>> +
>> + stats->op_write_sg_state = mgr->state;
>> + stats->op_write_sg_seq = stats->seq_num++;
>> +
>> + return 0;
>> +}
>> +
>> +static int op_write_complete(struct fpga_manager *mgr, struct fpga_image_info *info)
>> +{
>> + struct mgr_stats *stats = mgr->priv;
>> +
>> + stats->op_write_complete_state = mgr->state;
>> + stats->op_write_complete_seq = stats->seq_num++;
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Fake Manager that implements all ops required to check the programming
>> + * sequence using a single contiguous buffer and a scatter gather table.
>> + */
>> +static const struct fpga_manager_ops fake_mgr_ops = {
>> + .skip_header = true,
>> + .parse_header = op_parse_header,
>> + .write_init = op_write_init,
>> + .write = op_write,
>> + .write_sg = op_write_sg,
>> + .write_complete = op_write_complete,
>> +};
>> +
>> +static void fpga_mgr_test_get(struct kunit *test)
>> +{
>> + struct mgr_ctx *ctx = test->priv;
>> + struct fpga_manager *mgr;
>> +
>> + mgr = fpga_mgr_get(&ctx->pdev->dev);
>> + KUNIT_EXPECT_PTR_EQ(test, mgr, ctx->mgr);
>> +
>> + fpga_mgr_put(ctx->mgr);
>> +}
>> +
>> +static void fpga_mgr_test_lock(struct kunit *test)
>> +{
>> + struct mgr_ctx *ctx = test->priv;
>> + int ret;
>> +
>> + ret = fpga_mgr_lock(ctx->mgr);
>> + KUNIT_EXPECT_EQ(test, ret, 0);
>> +
>> + ret = fpga_mgr_lock(ctx->mgr);
>> + KUNIT_EXPECT_EQ(test, ret, -EBUSY);
>> +
>> + fpga_mgr_unlock(ctx->mgr);
>> +}
>> +
>> +/* Check the programming sequence using an image in a buffer */
>> +static void fpga_mgr_test_img_load_buf(struct kunit *test)
>> +{
>> + struct mgr_ctx *ctx = test->priv;
>> + char *img_buf;
>> + int ret;
>> +
>> + img_buf = init_test_buffer(test, IMAGE_SIZE);
>> +
>> + ctx->img_info->count = IMAGE_SIZE;
>> + ctx->img_info->buf = img_buf;
>> +
>> + ret = fpga_mgr_load(ctx->mgr, ctx->img_info);
>> + KUNIT_EXPECT_EQ(test, ret, 0);
>> +
>> + KUNIT_EXPECT_TRUE(test, ctx->stats.header_match);
>> + KUNIT_EXPECT_TRUE(test, ctx->stats.image_match);
>> +
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_parse_header_state, FPGA_MGR_STATE_PARSE_HEADER);
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_state, FPGA_MGR_STATE_WRITE_INIT);
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_state, FPGA_MGR_STATE_WRITE);
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_state, FPGA_MGR_STATE_WRITE_COMPLETE);
>> +
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_seq, ctx->stats.op_parse_header_seq + 1);
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_seq, ctx->stats.op_parse_header_seq + 2);
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_seq, ctx->stats.op_parse_header_seq + 3);
>> +}
>> +
>> +/* Check the programming sequence using an image in a scatter gather table */
>> +static void fpga_mgr_test_img_load_sgt(struct kunit *test)
>> +{
>> + struct mgr_ctx *ctx = test->priv;
>> + struct sg_table *sgt;
>> + char *img_buf;
>> + int ret;
>> +
>> + img_buf = init_test_buffer(test, IMAGE_SIZE);
>> +
>> + sgt = kunit_kzalloc(test, sizeof(*sgt), GFP_KERNEL);
>> + ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
>> + KUNIT_ASSERT_EQ(test, ret, 0);
>> + sg_init_one(sgt->sgl, img_buf, IMAGE_SIZE);
>> +
>> + ctx->img_info->sgt = sgt;
>> +
>> + ret = fpga_mgr_load(ctx->mgr, ctx->img_info);
>> + KUNIT_EXPECT_EQ(test, ret, 0);
>> +
>> + KUNIT_EXPECT_TRUE(test, ctx->stats.header_match);
>> + KUNIT_EXPECT_TRUE(test, ctx->stats.image_match);
>> +
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_parse_header_state, FPGA_MGR_STATE_PARSE_HEADER);
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_state, FPGA_MGR_STATE_WRITE_INIT);
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_sg_state, FPGA_MGR_STATE_WRITE);
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_state, FPGA_MGR_STATE_WRITE_COMPLETE);
>> +
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_seq, ctx->stats.op_parse_header_seq + 1);
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_sg_seq, ctx->stats.op_parse_header_seq + 2);
>> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_seq, ctx->stats.op_parse_header_seq + 3);
>> +
>> + sg_free_table(ctx->img_info->sgt);
>> +}
>> +
>> +static int fpga_mgr_test_init(struct kunit *test)
>> +{
>> + struct mgr_ctx *ctx;
>> +
>> + ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
>> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
>> +
>> + ctx->pdev = platform_device_register_simple("mgr_pdev", PLATFORM_DEVID_AUTO, NULL, 0);
>> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->pdev);
>> +
>> + ctx->mgr = devm_fpga_mgr_register(&ctx->pdev->dev, "Fake FPGA Manager", &fake_mgr_ops,
>> + &ctx->stats);
>> + KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(ctx->mgr));
>> +
>> + ctx->img_info = fpga_image_info_alloc(&ctx->pdev->dev);
>> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->img_info);
>> +
>> + test->priv = ctx;
>> +
>> + return 0;
>> +}
>> +
>> +static void fpga_mgr_test_exit(struct kunit *test)
>> +{
>> + struct mgr_ctx *ctx = test->priv;
>> +
>> + fpga_image_info_free(ctx->img_info);
>> + platform_device_unregister(ctx->pdev);
>> +}
>> +
>> +static struct kunit_case fpga_mgr_test_cases[] = {
>> + KUNIT_CASE(fpga_mgr_test_get),
>> + KUNIT_CASE(fpga_mgr_test_lock),
>> + KUNIT_CASE(fpga_mgr_test_img_load_buf),
>> + KUNIT_CASE(fpga_mgr_test_img_load_sgt),
>> + {}
>> +};
>> +
>> +static struct kunit_suite fpga_mgr_suite = {
>> + .name = "fpga_mgr",
>> + .init = fpga_mgr_test_init,
>> + .exit = fpga_mgr_test_exit,
>> + .test_cases = fpga_mgr_test_cases,
>> +};
>> +
>> +kunit_test_suite(fpga_mgr_suite);
>> +
>> +MODULE_LICENSE("GPL");
>> --
>> 2.40.1
>>
>


2023-06-27 13:54:49

by Marco Pagani

[permalink] [raw]
Subject: Re: [PATCH v7 2/4] fpga: add an initial KUnit suite for the FPGA Bridge



On 2023-06-25 09:33, Xu Yilun wrote:
> On 2023-06-16 at 17:44:03 +0200, Marco Pagani wrote:
>> The suite tests the basic behaviors of the FPGA Bridge including
>> the functions that operate on a list of Bridges.
> ^
> why uppercase?
>

It's a typo. I'll fix the description in the next version.

>>
>> Signed-off-by: Marco Pagani <[email protected]>
>> ---
>> drivers/fpga/tests/fpga-bridge-test.c | 175 ++++++++++++++++++++++++++
>> 1 file changed, 175 insertions(+)
>> create mode 100644 drivers/fpga/tests/fpga-bridge-test.c
>>
>> diff --git a/drivers/fpga/tests/fpga-bridge-test.c b/drivers/fpga/tests/fpga-bridge-test.c
>> new file mode 100644
>> index 000000000000..fce67ac59a7c
>> --- /dev/null
>> +++ b/drivers/fpga/tests/fpga-bridge-test.c
>> @@ -0,0 +1,175 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * KUnit test for the FPGA Bridge
>> + *
>> + * Copyright (C) 2023 Red Hat, Inc.
>> + *
>> + * Author: Marco Pagani <[email protected]>
>> + */
>> +
>> +#include <linux/types.h>
>> +#include <linux/module.h>
>> +#include <linux/device.h>
>> +#include <kunit/test.h>
>> +#include <linux/fpga/fpga-bridge.h>
>
> alphabetical order please.
>

I'll sort all includes alphabetically in the next version.

> Others LGTM.
> Acked-by: Xu Yilun <[email protected]>
>


2023-06-27 16:57:48

by Marco Pagani

[permalink] [raw]
Subject: Re: [PATCH v7 4/4] fpga: add configuration for the FPGA KUnit test suites.



On 2023-06-25 10:23, Xu Yilun wrote:
> On 2023-06-16 at 17:44:05 +0200, Marco Pagani wrote:
>> Add configuration for the KUnit test suites for the core components
>> of the FPGA subsystem.
>>
>> Signed-off-by: Marco Pagani <[email protected]>
>> ---
>> drivers/fpga/Kconfig | 2 ++
>> drivers/fpga/Makefile | 3 +++
>> drivers/fpga/tests/.kunitconfig | 5 +++++
>> drivers/fpga/tests/Kconfig | 11 +++++++++++
>> drivers/fpga/tests/Makefile | 5 +++++
>> 5 files changed, 26 insertions(+)
>> create mode 100644 drivers/fpga/tests/.kunitconfig
>> create mode 100644 drivers/fpga/tests/Kconfig
>> create mode 100644 drivers/fpga/tests/Makefile
>>
>> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
>> index 0a00763b9f28..2f689ac4ba3a 100644
>> --- a/drivers/fpga/Kconfig
>> +++ b/drivers/fpga/Kconfig
>> @@ -276,4 +276,6 @@ config FPGA_MGR_LATTICE_SYSCONFIG_SPI
>> FPGA manager driver support for Lattice FPGAs programming over slave
>> SPI sysCONFIG interface.
>>
>> +source "drivers/fpga/tests/Kconfig"
>> +
>> endif # FPGA
>> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
>> index 72e554b4d2f7..352a2612623e 100644
>> --- a/drivers/fpga/Makefile
>> +++ b/drivers/fpga/Makefile
>> @@ -55,3 +55,6 @@ obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000) += dfl-n3000-nios.o
>>
>> # Drivers for FPGAs which implement DFL
>> obj-$(CONFIG_FPGA_DFL_PCI) += dfl-pci.o
>> +
>> +# KUnit tests
>> +obj-$(CONFIG_FPGA_KUNIT_TESTS) += tests/
>> diff --git a/drivers/fpga/tests/.kunitconfig b/drivers/fpga/tests/.kunitconfig
>> new file mode 100644
>> index 000000000000..a1c2a2974c39
>> --- /dev/null
>> +++ b/drivers/fpga/tests/.kunitconfig
>> @@ -0,0 +1,5 @@
>> +CONFIG_KUNIT=y
>> +CONFIG_FPGA=y
>> +CONFIG_FPGA_REGION=y
>> +CONFIG_FPGA_BRIDGE=y
>> +CONFIG_FPGA_KUNIT_TESTS=y
>> diff --git a/drivers/fpga/tests/Kconfig b/drivers/fpga/tests/Kconfig
>> new file mode 100644
>> index 000000000000..e4a64815f16d
>> --- /dev/null
>> +++ b/drivers/fpga/tests/Kconfig
>> @@ -0,0 +1,11 @@
>> +config FPGA_KUNIT_TESTS
>> + tristate "KUnit test for the FPGA subsystem" if !KUNIT_ALL_TESTS
>> + depends on FPGA && FPGA_REGION && FPGA_BRIDGE && KUNIT=y
>
> Why KUNIT shouldn't be 'm'
>

The "running tips" section of KUnit documentation suggests that tests should
ideally depend on KUNIT=y in their Kconfigs because some features will not work
when KUNIT=m


>> + default KUNIT_ALL_TESTS
>> + help
>> + This builds unit tests for the FPGA subsystem
>> +
>> + For more information on KUnit and unit tests in general,
>> + please refer to the KUnit documentation in Documentation/dev-tools/kunit/.
>> +
>> + If unsure, say N.
>> diff --git a/drivers/fpga/tests/Makefile b/drivers/fpga/tests/Makefile
>> new file mode 100644
>> index 000000000000..faa5fa230ab0
>> --- /dev/null
>> +++ b/drivers/fpga/tests/Makefile
>> @@ -0,0 +1,5 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +obj-$(CONFIG_FPGA_KUNIT_TESTS) += fpga-mgr-test.o
>> +obj-$(CONFIG_FPGA_KUNIT_TESTS) += fpga-bridge-test.o
>> +obj-$(CONFIG_FPGA_KUNIT_TESTS) += fpga-region-test.o
>
> Could be integrated in one line?

Yes.

>
> Thanks,
> Yilun
>
>> --
>> 2.40.1
>>
>