[RFC PATCH v2 0/4] fpga: add initial KUnit tests for the subsystem

[RFC PATCH v2 0/4] fpga: add initial KUnit tests for the subsystem

[Marco Pagani]

This patch set introduces initial KUnit test suites for the FPGA subsystem.

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

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 fake FPGA manager
  fpga: add fake FPGA bridge
  fpga: add fake FPGA region
  fpga: add initial 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           |   6 +
 drivers/fpga/tests/fake-fpga-bridge.c | 228 ++++++++++++
 drivers/fpga/tests/fake-fpga-bridge.h |  36 ++
 drivers/fpga/tests/fake-fpga-mgr.c    | 369 +++++++++++++++++++
 drivers/fpga/tests/fake-fpga-mgr.h    |  42 +++
 drivers/fpga/tests/fake-fpga-region.c | 219 +++++++++++
 drivers/fpga/tests/fake-fpga-region.h |  38 ++
 drivers/fpga/tests/fpga-test.c        | 501 ++++++++++++++++++++++++++
 12 files changed, 1460 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/fake-fpga-bridge.c
 create mode 100644 drivers/fpga/tests/fake-fpga-bridge.h
 create mode 100644 drivers/fpga/tests/fake-fpga-mgr.c
 create mode 100644 drivers/fpga/tests/fake-fpga-mgr.h
 create mode 100644 drivers/fpga/tests/fake-fpga-region.c
 create mode 100644 drivers/fpga/tests/fake-fpga-region.h
 create mode 100644 drivers/fpga/tests/fpga-test.c

-- 
2.39.2

[RFC PATCH v2 1/4] fpga: add fake FPGA manager

[Marco Pagani]

Add fake FPGA manager platform driver with support functions.
The driver checks the programming sequence using KUnit expectations.
This module is part of the KUnit tests for the FPGA subsystem.

Signed-off-by: Marco Pagani <marpagan@redhat.com>
---
 drivers/fpga/tests/fake-fpga-mgr.c | 369 +++++++++++++++++++++++++++++
 drivers/fpga/tests/fake-fpga-mgr.h |  42 ++++
 2 files changed, 411 insertions(+)
 create mode 100644 drivers/fpga/tests/fake-fpga-mgr.c
 create mode 100644 drivers/fpga/tests/fake-fpga-mgr.h

diff --git a/drivers/fpga/tests/fake-fpga-mgr.c b/drivers/fpga/tests/fake-fpga-mgr.c
new file mode 100644
index 000000000000..1bca6baeb0e5
--- /dev/null
+++ b/drivers/fpga/tests/fake-fpga-mgr.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the fake FPGA manager
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani <marpagan@redhat.com>
+ */
+
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <kunit/test.h>
+
+#include "fake-fpga-mgr.h"
+
+#define FAKE_FPGA_MGR_DEV_NAME	"fake_fpga_mgr"
+
+#define FAKE_HEADER_BYTE	0x3f
+#define FAKE_HEADER_SIZE	FPGA_IMG_BLOCK
+
+struct fake_mgr_priv {
+	int rcfg_count;
+	bool op_parse_header;
+	bool op_write_init;
+	bool op_write;
+	bool op_write_sg;
+	bool op_write_complete;
+	struct kunit *test;
+};
+
+struct fake_mgr_data {
+	struct kunit *test;
+};
+
+static void check_header(struct kunit *test, const u8 *buf);
+
+static enum fpga_mgr_states op_state(struct fpga_manager *mgr)
+{
+	struct fake_mgr_priv *priv;
+
+	priv = mgr->priv;
+
+	if (priv->test)
+		kunit_info(priv->test, "Fake FPGA manager: state\n");
+
+	return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static u64 op_status(struct fpga_manager *mgr)
+{
+	struct fake_mgr_priv *priv;
+
+	priv = mgr->priv;
+
+	if (priv->test)
+		kunit_info(priv->test, "Fake FPGA manager: status\n");
+
+	return 0;
+}
+
+static int op_parse_header(struct fpga_manager *mgr, struct fpga_image_info *info,
+			   const char *buf, size_t count)
+{
+	struct fake_mgr_priv *priv;
+
+	priv = mgr->priv;
+
+	if (priv->test) {
+		kunit_info(priv->test, "Fake FPGA manager: parse_header\n");
+
+		KUNIT_EXPECT_EQ(priv->test, mgr->state,
+				FPGA_MGR_STATE_PARSE_HEADER);
+
+		check_header(priv->test, buf);
+	}
+
+	priv->op_parse_header = true;
+
+	return 0;
+}
+
+static int op_write_init(struct fpga_manager *mgr, struct fpga_image_info *info,
+			 const char *buf, size_t count)
+{
+	struct fake_mgr_priv *priv;
+
+	priv = mgr->priv;
+
+	if (priv->test) {
+		kunit_info(priv->test, "Fake FPGA manager: write_init\n");
+
+		KUNIT_EXPECT_EQ(priv->test, mgr->state,
+				FPGA_MGR_STATE_WRITE_INIT);
+	}
+
+	priv->op_write_init = true;
+
+	return 0;
+}
+
+static int op_write(struct fpga_manager *mgr, const char *buf, size_t count)
+{
+	struct fake_mgr_priv *priv;
+
+	priv = mgr->priv;
+
+	if (priv->test) {
+		kunit_info(priv->test, "Fake FPGA manager: write\n");
+
+		KUNIT_EXPECT_EQ(priv->test, mgr->state,
+				FPGA_MGR_STATE_WRITE);
+	}
+
+	priv->op_write = true;
+
+	return 0;
+}
+
+static int op_write_sg(struct fpga_manager *mgr, struct sg_table *sgt)
+{
+	struct fake_mgr_priv *priv;
+
+	priv = mgr->priv;
+
+	if (priv->test) {
+		kunit_info(priv->test, "Fake FPGA manager: write_sg\n");
+
+		KUNIT_EXPECT_EQ(priv->test, mgr->state,
+				FPGA_MGR_STATE_WRITE);
+	}
+
+	priv->op_write_sg = true;
+
+	return 0;
+}
+
+static int op_write_complete(struct fpga_manager *mgr, struct fpga_image_info *info)
+{
+	struct fake_mgr_priv *priv;
+
+	priv = mgr->priv;
+
+	if (priv->test) {
+		kunit_info(priv->test, "Fake FPGA manager: write_complete\n");
+
+		KUNIT_EXPECT_EQ(priv->test, mgr->state,
+				FPGA_MGR_STATE_WRITE_COMPLETE);
+	}
+
+	priv->op_write_complete = true;
+	priv->rcfg_count++;
+
+	return 0;
+}
+
+static void op_fpga_remove(struct fpga_manager *mgr)
+{
+	struct fake_mgr_priv *priv;
+
+	priv = mgr->priv;
+
+	if (priv->test)
+		kunit_info(priv->test, "Fake FPGA manager: remove\n");
+}
+
+static const struct fpga_manager_ops fake_fpga_mgr_ops = {
+	.initial_header_size = FAKE_HEADER_SIZE,
+	.skip_header = false,
+	.state = op_state,
+	.status = op_status,
+	.parse_header = op_parse_header,
+	.write_init = op_write_init,
+	.write = op_write,
+	.write_sg = op_write_sg,
+	.write_complete = op_write_complete,
+	.fpga_remove = op_fpga_remove,
+};
+
+/**
+ * fake_fpga_mgr_register() - register a fake FPGA manager.
+ * @mgr_ctx: fake FPGA manager context data structure.
+ * @test: KUnit test context object.
+ *
+ * Return: 0 if registration succeeded, an error code otherwise.
+ */
+int fake_fpga_mgr_register(struct fake_fpga_mgr *mgr_ctx, struct kunit *test)
+{
+	struct fake_mgr_data pdata;
+	int ret;
+
+	pdata.test = test;
+
+	mgr_ctx->pdev = platform_device_alloc(FAKE_FPGA_MGR_DEV_NAME,
+					      PLATFORM_DEVID_AUTO);
+	if (IS_ERR_OR_NULL(mgr_ctx->pdev)) {
+		pr_err("Fake FPGA manager device allocation failed\n");
+		return -ENOMEM;
+	}
+
+	platform_device_add_data(mgr_ctx->pdev, &pdata, sizeof(pdata));
+
+	ret = platform_device_add(mgr_ctx->pdev);
+	if (ret) {
+		pr_err("Fake FPGA manager device add failed\n");
+		platform_device_put(mgr_ctx->pdev);
+		return ret;
+	}
+
+	mgr_ctx->mgr = platform_get_drvdata(mgr_ctx->pdev);
+
+	if (test)
+		kunit_info(test, "Fake FPGA manager registered\n");
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fake_fpga_mgr_register);
+
+/**
+ * fake_fpga_mgr_unregister() - unregister a fake FPGA manager.
+ * @mgr_ctx: fake FPGA manager context data structure.
+ */
+void fake_fpga_mgr_unregister(struct fake_fpga_mgr *mgr_ctx)
+{
+	struct fake_mgr_priv *priv;
+	struct kunit *test;
+
+	if (!mgr_ctx)
+		return;
+
+	priv = mgr_ctx->mgr->priv;
+	test = priv->test;
+
+	if (mgr_ctx->pdev) {
+		platform_device_unregister(mgr_ctx->pdev);
+		if (test)
+			kunit_info(test, "Fake FPGA manager unregistered\n");
+	}
+}
+EXPORT_SYMBOL_GPL(fake_fpga_mgr_unregister);
+
+/**
+ * fake_fpga_mgr_get_rcfg_count() - get the number of reconfigurations.
+ * @mgr_ctx: fake FPGA manager context data structure.
+ *
+ * Return: number of reconfigurations.
+ */
+int fake_fpga_mgr_get_rcfg_count(const struct fake_fpga_mgr *mgr_ctx)
+{
+	struct fake_mgr_priv *priv;
+
+	priv = mgr_ctx->mgr->priv;
+
+	return priv->rcfg_count;
+}
+EXPORT_SYMBOL_GPL(fake_fpga_mgr_get_rcfg_count);
+
+/**
+ * fake_fpga_mgr_fill_header() - fill an image buffer with the test header.
+ * @buf: image buffer.
+ */
+void fake_fpga_mgr_fill_header(u8 *buf)
+{
+	int i;
+
+	for (i = 0; i < FAKE_HEADER_SIZE; i++)
+		buf[i] = FAKE_HEADER_BYTE;
+}
+EXPORT_SYMBOL_GPL(fake_fpga_mgr_fill_header);
+
+static void check_header(struct kunit *test, const u8 *buf)
+{
+	int i;
+
+	for (i = 0; i < FAKE_HEADER_SIZE; i++)
+		KUNIT_EXPECT_EQ(test, buf[i], FAKE_HEADER_BYTE);
+}
+
+static void clear_op_flags(struct fake_mgr_priv *priv)
+{
+	priv->op_parse_header = false;
+	priv->op_write_init = false;
+	priv->op_write = false;
+	priv->op_write_sg = false;
+	priv->op_write_complete = false;
+}
+
+/**
+ * fake_fpga_mgr_check_write_buf() - check if programming using a buffer succeeded.
+ * @mgr_ctx: fake FPGA manager context data structure.
+ */
+void fake_fpga_mgr_check_write_buf(struct fake_fpga_mgr *mgr_ctx)
+{
+	struct fake_mgr_priv *priv;
+
+	priv = mgr_ctx->mgr->priv;
+
+	if (priv->test) {
+		KUNIT_EXPECT_EQ(priv->test, priv->op_parse_header, true);
+		KUNIT_EXPECT_EQ(priv->test, priv->op_write_init, true);
+		KUNIT_EXPECT_EQ(priv->test, priv->op_write, true);
+		KUNIT_EXPECT_EQ(priv->test, priv->op_write_complete, true);
+	}
+
+	clear_op_flags(priv);
+}
+EXPORT_SYMBOL_GPL(fake_fpga_mgr_check_write_buf);
+
+/**
+ * fake_fpga_mgr_check_write_sgt() - check if programming using a s.g. table succeeded.
+ * @mgr_ctx: fake FPGA manager context data structure.
+ */
+void fake_fpga_mgr_check_write_sgt(struct fake_fpga_mgr *mgr_ctx)
+{
+	struct fake_mgr_priv *priv;
+
+	priv = mgr_ctx->mgr->priv;
+
+	if (priv->test) {
+		KUNIT_EXPECT_EQ(priv->test, priv->op_parse_header, true);
+		KUNIT_EXPECT_EQ(priv->test, priv->op_write_init, true);
+		KUNIT_EXPECT_EQ(priv->test, priv->op_write_sg, true);
+		KUNIT_EXPECT_EQ(priv->test, priv->op_write_complete, true);
+	}
+
+	clear_op_flags(priv);
+}
+EXPORT_SYMBOL_GPL(fake_fpga_mgr_check_write_sgt);
+
+static int fake_fpga_mgr_probe(struct platform_device *pdev)
+{
+	struct device *dev;
+	struct fake_mgr_priv *priv;
+	struct fake_mgr_data *pdata;
+	struct fpga_manager *mgr;
+
+	dev = &pdev->dev;
+	pdata = dev_get_platdata(dev);
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	if (pdata)
+		priv->test = pdata->test;
+
+	mgr = devm_fpga_mgr_register(dev, "Fake FPGA Manager",
+				     &fake_fpga_mgr_ops, priv);
+	if (IS_ERR(mgr))
+		return PTR_ERR(mgr);
+
+	platform_set_drvdata(pdev, mgr);
+
+	return 0;
+}
+
+static struct platform_driver fake_fpga_mgr_drv = {
+	.driver = {
+		.name = FAKE_FPGA_MGR_DEV_NAME
+	},
+	.probe = fake_fpga_mgr_probe,
+};
+
+module_platform_driver(fake_fpga_mgr_drv);
+
+MODULE_AUTHOR("Marco Pagani <marpagan@redhat.com>");
+MODULE_DESCRIPTION("Fake FPGA Manager");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/fpga/tests/fake-fpga-mgr.h b/drivers/fpga/tests/fake-fpga-mgr.h
new file mode 100644
index 000000000000..3778cbf594bc
--- /dev/null
+++ b/drivers/fpga/tests/fake-fpga-mgr.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Header file for the fake FPGA manager
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani <marpagan@redhat.com>
+ */
+
+#ifndef __FPGA_FAKE_MGR_H
+#define __FPGA_FAKE_MGR_H
+
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/platform_device.h>
+#include <kunit/test.h>
+
+#define FPGA_IMG_BLOCK	1024
+
+/**
+ * struct fake_fpga_mgr - fake FPGA manager context data structure
+ *
+ * @mgr: FPGA manager.
+ * @pdev: platform device of the FPGA manager.
+ */
+struct fake_fpga_mgr {
+	struct fpga_manager *mgr;
+	struct platform_device *pdev;
+};
+
+int fake_fpga_mgr_register(struct fake_fpga_mgr *mgr_ctx, struct kunit *test);
+
+void fake_fpga_mgr_unregister(struct fake_fpga_mgr *mgr_ctx);
+
+int fake_fpga_mgr_get_rcfg_count(const struct fake_fpga_mgr *mgr_ctx);
+
+void fake_fpga_mgr_fill_header(u8 *buf);
+
+void fake_fpga_mgr_check_write_buf(struct fake_fpga_mgr *mgr_ctx);
+
+void fake_fpga_mgr_check_write_sgt(struct fake_fpga_mgr *mgr_ctx);
+
+#endif /* __FPGA_FAKE_MGR_H */
-- 
2.39.2

[RFC PATCH v2 2/4] fpga: add fake FPGA bridge

[Marco Pagani]

Add fake FPGA bridge driver with support functions. The driver includes
a counter for the number of switching cycles. This module is part of
the KUnit tests for the FPGA subsystem.

Signed-off-by: Marco Pagani <marpagan@redhat.com>
---
 drivers/fpga/tests/fake-fpga-bridge.c | 228 ++++++++++++++++++++++++++
 drivers/fpga/tests/fake-fpga-bridge.h |  36 ++++
 2 files changed, 264 insertions(+)
 create mode 100644 drivers/fpga/tests/fake-fpga-bridge.c
 create mode 100644 drivers/fpga/tests/fake-fpga-bridge.h

diff --git a/drivers/fpga/tests/fake-fpga-bridge.c b/drivers/fpga/tests/fake-fpga-bridge.c
new file mode 100644
index 000000000000..8a2f64fc1bbb
--- /dev/null
+++ b/drivers/fpga/tests/fake-fpga-bridge.c
@@ -0,0 +1,228 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the fake FPGA bridge
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani <marpagan@redhat.com>
+ */
+
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/fpga/fpga-bridge.h>
+#include <kunit/test.h>
+
+#include "fake-fpga-bridge.h"
+
+#define FAKE_FPGA_BRIDGE_DEV_NAME	"fake_fpga_bridge"
+
+struct fake_bridge_priv {
+	int id;
+	bool enable;
+	int cycles_count;
+	struct kunit *test;
+};
+
+struct fake_bridge_data {
+	struct kunit *test;
+};
+
+static int op_enable_show(struct fpga_bridge *bridge)
+{
+	struct fake_bridge_priv *priv;
+
+	priv = bridge->priv;
+
+	if (priv->test)
+		kunit_info(priv->test, "Fake FPGA bridge %d: enable_show\n",
+			   priv->id);
+
+	return priv->enable;
+}
+
+static int op_enable_set(struct fpga_bridge *bridge, bool enable)
+{
+	struct fake_bridge_priv *priv;
+
+	priv = bridge->priv;
+
+	if (enable && !priv->enable)
+		priv->cycles_count++;
+
+	priv->enable = enable;
+
+	if (priv->test)
+		kunit_info(priv->test, "Fake FPGA bridge %d: enable_set: %d\n",
+			   priv->id, enable);
+
+	return 0;
+}
+
+static void op_remove(struct fpga_bridge *bridge)
+{
+	struct fake_bridge_priv *priv;
+
+	priv = bridge->priv;
+
+	if (priv->test)
+		kunit_info(priv->test, "Fake FPGA bridge: remove\n");
+}
+
+static const struct fpga_bridge_ops fake_fpga_bridge_ops = {
+	.enable_show = op_enable_show,
+	.enable_set = op_enable_set,
+	.fpga_bridge_remove = op_remove,
+};
+
+/**
+ * fake_fpga_bridge_register() - register a fake FPGA bridge.
+ * @bridge_ctx: fake FPGA bridge context data structure.
+ * @parent: parent device.
+ * @test: KUnit test context object.
+ *
+ * Return: 0 if registration succeeded, an error code otherwise.
+ */
+int fake_fpga_bridge_register(struct fake_fpga_bridge *bridge_ctx,
+			      struct device *parent, struct kunit *test)
+{
+	struct fake_bridge_data pdata;
+	struct fake_bridge_priv *priv;
+	int ret;
+
+	pdata.test = test;
+
+	bridge_ctx->pdev = platform_device_alloc(FAKE_FPGA_BRIDGE_DEV_NAME,
+						 PLATFORM_DEVID_AUTO);
+	if (IS_ERR(bridge_ctx->pdev)) {
+		pr_err("Fake FPGA bridge device allocation failed\n");
+		return -ENOMEM;
+	}
+
+	bridge_ctx->pdev->dev.parent = parent;
+	platform_device_add_data(bridge_ctx->pdev, &pdata, sizeof(pdata));
+
+	ret = platform_device_add(bridge_ctx->pdev);
+	if (ret) {
+		pr_err("Fake FPGA bridge device add failed\n");
+		platform_device_put(bridge_ctx->pdev);
+		return ret;
+	}
+
+	bridge_ctx->bridge = platform_get_drvdata(bridge_ctx->pdev);
+
+	if (test) {
+		priv = bridge_ctx->bridge->priv;
+		kunit_info(test, "Fake FPGA bridge %d registered\n", priv->id);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fake_fpga_bridge_register);
+
+/**
+ * fake_fpga_bridge_unregister() - unregister a fake FPGA bridge.
+ * @bridge_ctx: fake FPGA bridge context data structure.
+ */
+void fake_fpga_bridge_unregister(struct fake_fpga_bridge *bridge_ctx)
+{
+	struct fake_bridge_priv *priv;
+	struct kunit *test;
+	int id;
+
+	if (!bridge_ctx)
+		return;
+
+	priv = bridge_ctx->bridge->priv;
+	test = priv->test;
+	id = priv->id;
+
+	if (bridge_ctx->pdev) {
+		platform_device_unregister(bridge_ctx->pdev);
+		if (test)
+			kunit_info(test, "Fake FPGA bridge %d unregistered\n", id);
+	}
+}
+EXPORT_SYMBOL_GPL(fake_fpga_bridge_unregister);
+
+/**
+ * fake_fpga_bridge_get_state() - get state of a fake FPGA bridge.
+ * @bridge_ctx: fake FPGA bridge context data structure.
+ *
+ * Return: 1 if the bridge is enabled, 0 if disabled.
+ */
+int fake_fpga_bridge_get_state(const struct fake_fpga_bridge *bridge_ctx)
+{
+	return bridge_ctx->bridge->br_ops->enable_show(bridge_ctx->bridge);
+}
+EXPORT_SYMBOL_GPL(fake_fpga_bridge_get_state);
+
+/**
+ * fake_fpga_bridge_get_cycles_count() - get the number of switching cycles.
+ * @bridge_ctx: fake FPGA bridge context data structure.
+ *
+ * Return: number of switching cycles.
+ */
+int fake_fpga_bridge_get_cycles_count(const struct fake_fpga_bridge *bridge_ctx)
+{
+	struct fake_bridge_priv *priv;
+
+	priv = bridge_ctx->bridge->priv;
+
+	return priv->cycles_count;
+}
+EXPORT_SYMBOL_GPL(fake_fpga_bridge_get_cycles_count);
+
+static int fake_fpga_bridge_probe(struct platform_device *pdev)
+{
+	struct device *dev;
+	struct fpga_bridge *bridge;
+	struct fake_bridge_data *pdata;
+	struct fake_bridge_priv *priv;
+	static int id_count;
+
+	dev = &pdev->dev;
+	pdata = dev_get_platdata(dev);
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->id = id_count++;
+	priv->enable = true;
+
+	if (pdata)
+		priv->test = pdata->test;
+
+	bridge = fpga_bridge_register(dev, "Fake FPGA Bridge",
+				      &fake_fpga_bridge_ops, priv);
+	if (IS_ERR(bridge))
+		return PTR_ERR(bridge);
+
+	platform_set_drvdata(pdev, bridge);
+
+	return 0;
+}
+
+static int fake_fpga_bridge_remove(struct platform_device *pdev)
+{
+	struct fpga_bridge *bridge = platform_get_drvdata(pdev);
+
+	fpga_bridge_unregister(bridge);
+
+	return 0;
+}
+
+static struct platform_driver fake_fpga_bridge_drv = {
+	.driver = {
+		.name = FAKE_FPGA_BRIDGE_DEV_NAME
+	},
+	.probe = fake_fpga_bridge_probe,
+	.remove = fake_fpga_bridge_remove,
+};
+
+module_platform_driver(fake_fpga_bridge_drv);
+
+MODULE_AUTHOR("Marco Pagani <marpagan@redhat.com>");
+MODULE_DESCRIPTION("Fake FPGA Bridge");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/fpga/tests/fake-fpga-bridge.h b/drivers/fpga/tests/fake-fpga-bridge.h
new file mode 100644
index 000000000000..ae224b13f284
--- /dev/null
+++ b/drivers/fpga/tests/fake-fpga-bridge.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Header file for the fake FPGA bridge
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani <marpagan@redhat.com>
+ */
+
+#ifndef __FPGA_FAKE_BRIDGE_H
+#define __FPGA_FAKE_BRIDGE_H
+
+#include <linux/platform_device.h>
+#include <kunit/test.h>
+
+/**
+ * struct fake_fpga_bridge - fake FPGA bridge context data structure
+ *
+ * @bridge: FPGA bridge.
+ * @pdev: platform device of the FPGA bridge.
+ */
+struct fake_fpga_bridge {
+	struct fpga_bridge *bridge;
+	struct platform_device *pdev;
+};
+
+int fake_fpga_bridge_register(struct fake_fpga_bridge *bridge_ctx,
+			      struct device *parent, struct kunit *test);
+
+void fake_fpga_bridge_unregister(struct fake_fpga_bridge *bridge_ctx);
+
+int fake_fpga_bridge_get_state(const struct fake_fpga_bridge *bridge_ctx);
+
+int fake_fpga_bridge_get_cycles_count(const struct fake_fpga_bridge *bridge_ctx);
+
+#endif /* __FPGA_FAKE_BRIDGE_H */
-- 
2.39.2

Re: [RFC PATCH v2 2/4] fpga: add fake FPGA bridge

[Xu Yilun]

On 2023-03-10 at 18:04:10 +0100, Marco Pagani wrote:
> Add fake FPGA bridge driver with support functions. The driver includes
> a counter for the number of switching cycles. This module is part of
> the KUnit tests for the FPGA subsystem.
> 
> Signed-off-by: Marco Pagani <marpagan@redhat.com>
> ---
>  drivers/fpga/tests/fake-fpga-bridge.c | 228 ++++++++++++++++++++++++++
>  drivers/fpga/tests/fake-fpga-bridge.h |  36 ++++
>  2 files changed, 264 insertions(+)
>  create mode 100644 drivers/fpga/tests/fake-fpga-bridge.c
>  create mode 100644 drivers/fpga/tests/fake-fpga-bridge.h
> 
> diff --git a/drivers/fpga/tests/fake-fpga-bridge.c b/drivers/fpga/tests/fake-fpga-bridge.c
> new file mode 100644
> index 000000000000..8a2f64fc1bbb
> --- /dev/null
> +++ b/drivers/fpga/tests/fake-fpga-bridge.c
> @@ -0,0 +1,228 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the fake FPGA bridge
> + *
> + * Copyright (C) 2023 Red Hat, Inc.
> + *
> + * Author: Marco Pagani <marpagan@redhat.com>
> + */
> +
> +#include <linux/types.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/fpga/fpga-bridge.h>
> +#include <kunit/test.h>
> +
> +#include "fake-fpga-bridge.h"
> +
> +#define FAKE_FPGA_BRIDGE_DEV_NAME	"fake_fpga_bridge"
> +
> +struct fake_bridge_priv {
> +	int id;
> +	bool enable;
> +	int cycles_count;
> +	struct kunit *test;
> +};
> +
> +struct fake_bridge_data {
> +	struct kunit *test;
> +};
> +
> +static int op_enable_show(struct fpga_bridge *bridge)
> +{
> +	struct fake_bridge_priv *priv;
> +
> +	priv = bridge->priv;
> +
> +	if (priv->test)
> +		kunit_info(priv->test, "Fake FPGA bridge %d: enable_show\n",
> +			   priv->id);

Why check the kunit pointer every time? I remember you mentioned that
the fake fpga modules are expected to be used out of Kunit test, so the
priv->test may be NULL? I suggest you work on these usecases in separate
patchsets. For now just check priv->test on probe is fine.

> +
> +	return priv->enable;
> +}
> +
> +static int op_enable_set(struct fpga_bridge *bridge, bool enable)
> +{
> +	struct fake_bridge_priv *priv;
> +
> +	priv = bridge->priv;
> +
> +	if (enable && !priv->enable)
> +		priv->cycles_count++;
> +
> +	priv->enable = enable;
> +
> +	if (priv->test)
> +		kunit_info(priv->test, "Fake FPGA bridge %d: enable_set: %d\n",
> +			   priv->id, enable);
> +
> +	return 0;
> +}
> +
> +static void op_remove(struct fpga_bridge *bridge)
> +{
> +	struct fake_bridge_priv *priv;
> +
> +	priv = bridge->priv;
> +
> +	if (priv->test)
> +		kunit_info(priv->test, "Fake FPGA bridge: remove\n");
> +}
> +
> +static const struct fpga_bridge_ops fake_fpga_bridge_ops = {
> +	.enable_show = op_enable_show,
> +	.enable_set = op_enable_set,
> +	.fpga_bridge_remove = op_remove,
> +};
> +
> +/**
> + * fake_fpga_bridge_register() - register a fake FPGA bridge.
> + * @bridge_ctx: fake FPGA bridge context data structure.
> + * @parent: parent device.
> + * @test: KUnit test context object.
> + *
> + * Return: 0 if registration succeeded, an error code otherwise.
> + */
> +int fake_fpga_bridge_register(struct fake_fpga_bridge *bridge_ctx,
> +			      struct device *parent, struct kunit *test)

struct fake_fpga_bridge *fake_fpga_bridge_register(struct device *parent, ...)

Is it better?

Thanks,
Yilun

> +{
> +	struct fake_bridge_data pdata;
> +	struct fake_bridge_priv *priv;
> +	int ret;
> +
> +	pdata.test = test;
> +
> +	bridge_ctx->pdev = platform_device_alloc(FAKE_FPGA_BRIDGE_DEV_NAME,
> +						 PLATFORM_DEVID_AUTO);
> +	if (IS_ERR(bridge_ctx->pdev)) {
> +		pr_err("Fake FPGA bridge device allocation failed\n");
> +		return -ENOMEM;
> +	}
> +
> +	bridge_ctx->pdev->dev.parent = parent;
> +	platform_device_add_data(bridge_ctx->pdev, &pdata, sizeof(pdata));
> +
> +	ret = platform_device_add(bridge_ctx->pdev);
> +	if (ret) {
> +		pr_err("Fake FPGA bridge device add failed\n");
> +		platform_device_put(bridge_ctx->pdev);
> +		return ret;
> +	}
> +
> +	bridge_ctx->bridge = platform_get_drvdata(bridge_ctx->pdev);
> +
> +	if (test) {
> +		priv = bridge_ctx->bridge->priv;
> +		kunit_info(test, "Fake FPGA bridge %d registered\n", priv->id);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(fake_fpga_bridge_register);
> +
> +/**
> + * fake_fpga_bridge_unregister() - unregister a fake FPGA bridge.
> + * @bridge_ctx: fake FPGA bridge context data structure.
> + */
> +void fake_fpga_bridge_unregister(struct fake_fpga_bridge *bridge_ctx)
> +{
> +	struct fake_bridge_priv *priv;
> +	struct kunit *test;
> +	int id;
> +
> +	if (!bridge_ctx)
> +		return;
> +
> +	priv = bridge_ctx->bridge->priv;
> +	test = priv->test;
> +	id = priv->id;
> +
> +	if (bridge_ctx->pdev) {
> +		platform_device_unregister(bridge_ctx->pdev);
> +		if (test)
> +			kunit_info(test, "Fake FPGA bridge %d unregistered\n", id);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(fake_fpga_bridge_unregister);
> +
> +/**
> + * fake_fpga_bridge_get_state() - get state of a fake FPGA bridge.
> + * @bridge_ctx: fake FPGA bridge context data structure.
> + *
> + * Return: 1 if the bridge is enabled, 0 if disabled.
> + */
> +int fake_fpga_bridge_get_state(const struct fake_fpga_bridge *bridge_ctx)
> +{
> +	return bridge_ctx->bridge->br_ops->enable_show(bridge_ctx->bridge);
> +}
> +EXPORT_SYMBOL_GPL(fake_fpga_bridge_get_state);
> +
> +/**
> + * fake_fpga_bridge_get_cycles_count() - get the number of switching cycles.
> + * @bridge_ctx: fake FPGA bridge context data structure.
> + *
> + * Return: number of switching cycles.
> + */
> +int fake_fpga_bridge_get_cycles_count(const struct fake_fpga_bridge *bridge_ctx)
> +{
> +	struct fake_bridge_priv *priv;
> +
> +	priv = bridge_ctx->bridge->priv;
> +
> +	return priv->cycles_count;
> +}
> +EXPORT_SYMBOL_GPL(fake_fpga_bridge_get_cycles_count);
> +
> +static int fake_fpga_bridge_probe(struct platform_device *pdev)
> +{
> +	struct device *dev;
> +	struct fpga_bridge *bridge;
> +	struct fake_bridge_data *pdata;
> +	struct fake_bridge_priv *priv;
> +	static int id_count;
> +
> +	dev = &pdev->dev;
> +	pdata = dev_get_platdata(dev);
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->id = id_count++;
> +	priv->enable = true;
> +
> +	if (pdata)
> +		priv->test = pdata->test;
> +
> +	bridge = fpga_bridge_register(dev, "Fake FPGA Bridge",
> +				      &fake_fpga_bridge_ops, priv);
> +	if (IS_ERR(bridge))
> +		return PTR_ERR(bridge);
> +
> +	platform_set_drvdata(pdev, bridge);
> +
> +	return 0;
> +}
> +
> +static int fake_fpga_bridge_remove(struct platform_device *pdev)
> +{
> +	struct fpga_bridge *bridge = platform_get_drvdata(pdev);
> +
> +	fpga_bridge_unregister(bridge);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver fake_fpga_bridge_drv = {
> +	.driver = {
> +		.name = FAKE_FPGA_BRIDGE_DEV_NAME
> +	},
> +	.probe = fake_fpga_bridge_probe,
> +	.remove = fake_fpga_bridge_remove,
> +};
> +
> +module_platform_driver(fake_fpga_bridge_drv);
> +
> +MODULE_AUTHOR("Marco Pagani <marpagan@redhat.com>");
> +MODULE_DESCRIPTION("Fake FPGA Bridge");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/fpga/tests/fake-fpga-bridge.h b/drivers/fpga/tests/fake-fpga-bridge.h
> new file mode 100644
> index 000000000000..ae224b13f284
> --- /dev/null
> +++ b/drivers/fpga/tests/fake-fpga-bridge.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Header file for the fake FPGA bridge
> + *
> + * Copyright (C) 2023 Red Hat, Inc.
> + *
> + * Author: Marco Pagani <marpagan@redhat.com>
> + */
> +
> +#ifndef __FPGA_FAKE_BRIDGE_H
> +#define __FPGA_FAKE_BRIDGE_H
> +
> +#include <linux/platform_device.h>
> +#include <kunit/test.h>
> +
> +/**
> + * struct fake_fpga_bridge - fake FPGA bridge context data structure
> + *
> + * @bridge: FPGA bridge.
> + * @pdev: platform device of the FPGA bridge.
> + */
> +struct fake_fpga_bridge {
> +	struct fpga_bridge *bridge;
> +	struct platform_device *pdev;
> +};
> +
> +int fake_fpga_bridge_register(struct fake_fpga_bridge *bridge_ctx,
> +			      struct device *parent, struct kunit *test);
> +
> +void fake_fpga_bridge_unregister(struct fake_fpga_bridge *bridge_ctx);
> +
> +int fake_fpga_bridge_get_state(const struct fake_fpga_bridge *bridge_ctx);
> +
> +int fake_fpga_bridge_get_cycles_count(const struct fake_fpga_bridge *bridge_ctx);
> +
> +#endif /* __FPGA_FAKE_BRIDGE_H */
> -- 
> 2.39.2
> 

Re: [RFC PATCH v2 2/4] fpga: add fake FPGA bridge

[Marco Pagani]

On 2023-03-17 09:32, Xu Yilun wrote:
> On 2023-03-10 at 18:04:10 +0100, Marco Pagani wrote:
>> Add fake FPGA bridge driver with support functions. The driver includes
>> a counter for the number of switching cycles. This module is part of
>> the KUnit tests for the FPGA subsystem.
>>
>> Signed-off-by: Marco Pagani <marpagan@redhat.com>
>> ---
>>  drivers/fpga/tests/fake-fpga-bridge.c | 228 ++++++++++++++++++++++++++
>>  drivers/fpga/tests/fake-fpga-bridge.h |  36 ++++
>>  2 files changed, 264 insertions(+)
>>  create mode 100644 drivers/fpga/tests/fake-fpga-bridge.c
>>  create mode 100644 drivers/fpga/tests/fake-fpga-bridge.h
>>
>> diff --git a/drivers/fpga/tests/fake-fpga-bridge.c b/drivers/fpga/tests/fake-fpga-bridge.c
>> new file mode 100644
>> index 000000000000..8a2f64fc1bbb
>> --- /dev/null
>> +++ b/drivers/fpga/tests/fake-fpga-bridge.c
>> @@ -0,0 +1,228 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for the fake FPGA bridge
>> + *
>> + * Copyright (C) 2023 Red Hat, Inc.
>> + *
>> + * Author: Marco Pagani <marpagan@redhat.com>
>> + */
>> +
>> +#include <linux/types.h>
>> +#include <linux/device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/fpga/fpga-bridge.h>
>> +#include <kunit/test.h>
>> +
>> +#include "fake-fpga-bridge.h"
>> +
>> +#define FAKE_FPGA_BRIDGE_DEV_NAME	"fake_fpga_bridge"
>> +
>> +struct fake_bridge_priv {
>> +	int id;
>> +	bool enable;
>> +	int cycles_count;
>> +	struct kunit *test;
>> +};
>> +
>> +struct fake_bridge_data {
>> +	struct kunit *test;
>> +};
>> +
>> +static int op_enable_show(struct fpga_bridge *bridge)
>> +{
>> +	struct fake_bridge_priv *priv;
>> +
>> +	priv = bridge->priv;
>> +
>> +	if (priv->test)
>> +		kunit_info(priv->test, "Fake FPGA bridge %d: enable_show\n",
>> +			   priv->id);
> 
> Why check the kunit pointer every time? I remember you mentioned that
> the fake fpga modules are expected to be used out of Kunit test, so the
> priv->test may be NULL? I suggest you work on these usecases in separate
> patchsets. For now just check priv->test on probe is fine.
> 

The idea was to provide additional info messages, tied with the test, if the
fake bridge is registered with a test instance. If you believe these prints
are unnecessary, I can remove them or replace them with generic dev_info().

>> +
>> +	return priv->enable;
>> +}
>> +
>> +static int op_enable_set(struct fpga_bridge *bridge, bool enable)
>> +{
>> +	struct fake_bridge_priv *priv;
>> +
>> +	priv = bridge->priv;
>> +
>> +	if (enable && !priv->enable)
>> +		priv->cycles_count++;
>> +
>> +	priv->enable = enable;
>> +
>> +	if (priv->test)
>> +		kunit_info(priv->test, "Fake FPGA bridge %d: enable_set: %d\n",
>> +			   priv->id, enable);
>> +
>> +	return 0;
>> +}
>> +
>> +static void op_remove(struct fpga_bridge *bridge)
>> +{
>> +	struct fake_bridge_priv *priv;
>> +
>> +	priv = bridge->priv;
>> +
>> +	if (priv->test)
>> +		kunit_info(priv->test, "Fake FPGA bridge: remove\n");
>> +}
>> +
>> +static const struct fpga_bridge_ops fake_fpga_bridge_ops = {
>> +	.enable_show = op_enable_show,
>> +	.enable_set = op_enable_set,
>> +	.fpga_bridge_remove = op_remove,
>> +};
>> +
>> +/**
>> + * fake_fpga_bridge_register() - register a fake FPGA bridge.
>> + * @bridge_ctx: fake FPGA bridge context data structure.
>> + * @parent: parent device.
>> + * @test: KUnit test context object.
>> + *
>> + * Return: 0 if registration succeeded, an error code otherwise.
>> + */
>> +int fake_fpga_bridge_register(struct fake_fpga_bridge *bridge_ctx,
>> +			      struct device *parent, struct kunit *test)
> 
> struct fake_fpga_bridge *fake_fpga_bridge_register(struct device *parent, ...)
> 
> Is it better?
> 
> Thanks,
> Yilun

Agreed, it is better. I'll change the registration functions for the fake
bridge, manager, and region in the next version.

> 
>> +{
>> +	struct fake_bridge_data pdata;
>> +	struct fake_bridge_priv *priv;
>> +	int ret;
>> +
>> +	pdata.test = test;
>> +
>> +	bridge_ctx->pdev = platform_device_alloc(FAKE_FPGA_BRIDGE_DEV_NAME,
>> +						 PLATFORM_DEVID_AUTO);
>> +	if (IS_ERR(bridge_ctx->pdev)) {
>> +		pr_err("Fake FPGA bridge device allocation failed\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	bridge_ctx->pdev->dev.parent = parent;
>> +	platform_device_add_data(bridge_ctx->pdev, &pdata, sizeof(pdata));
>> +
>> +	ret = platform_device_add(bridge_ctx->pdev);
>> +	if (ret) {
>> +		pr_err("Fake FPGA bridge device add failed\n");
>> +		platform_device_put(bridge_ctx->pdev);
>> +		return ret;
>> +	}
>> +
>> +	bridge_ctx->bridge = platform_get_drvdata(bridge_ctx->pdev);
>> +
>> +	if (test) {
>> +		priv = bridge_ctx->bridge->priv;
>> +		kunit_info(test, "Fake FPGA bridge %d registered\n", priv->id);
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(fake_fpga_bridge_register);
>> +
>> +/**
>> + * fake_fpga_bridge_unregister() - unregister a fake FPGA bridge.
>> + * @bridge_ctx: fake FPGA bridge context data structure.
>> + */
>> +void fake_fpga_bridge_unregister(struct fake_fpga_bridge *bridge_ctx)
>> +{
>> +	struct fake_bridge_priv *priv;
>> +	struct kunit *test;
>> +	int id;
>> +
>> +	if (!bridge_ctx)
>> +		return;
>> +
>> +	priv = bridge_ctx->bridge->priv;
>> +	test = priv->test;
>> +	id = priv->id;
>> +
>> +	if (bridge_ctx->pdev) {
>> +		platform_device_unregister(bridge_ctx->pdev);
>> +		if (test)
>> +			kunit_info(test, "Fake FPGA bridge %d unregistered\n", id);
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(fake_fpga_bridge_unregister);
>> +
>> +/**
>> + * fake_fpga_bridge_get_state() - get state of a fake FPGA bridge.
>> + * @bridge_ctx: fake FPGA bridge context data structure.
>> + *
>> + * Return: 1 if the bridge is enabled, 0 if disabled.
>> + */
>> +int fake_fpga_bridge_get_state(const struct fake_fpga_bridge *bridge_ctx)
>> +{
>> +	return bridge_ctx->bridge->br_ops->enable_show(bridge_ctx->bridge);
>> +}
>> +EXPORT_SYMBOL_GPL(fake_fpga_bridge_get_state);
>> +
>> +/**
>> + * fake_fpga_bridge_get_cycles_count() - get the number of switching cycles.
>> + * @bridge_ctx: fake FPGA bridge context data structure.
>> + *
>> + * Return: number of switching cycles.
>> + */
>> +int fake_fpga_bridge_get_cycles_count(const struct fake_fpga_bridge *bridge_ctx)
>> +{
>> +	struct fake_bridge_priv *priv;
>> +
>> +	priv = bridge_ctx->bridge->priv;
>> +
>> +	return priv->cycles_count;
>> +}
>> +EXPORT_SYMBOL_GPL(fake_fpga_bridge_get_cycles_count);
>> +
>> +static int fake_fpga_bridge_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev;
>> +	struct fpga_bridge *bridge;
>> +	struct fake_bridge_data *pdata;
>> +	struct fake_bridge_priv *priv;
>> +	static int id_count;
>> +
>> +	dev = &pdev->dev;
>> +	pdata = dev_get_platdata(dev);
>> +
>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	priv->id = id_count++;
>> +	priv->enable = true;
>> +
>> +	if (pdata)
>> +		priv->test = pdata->test;
>> +
>> +	bridge = fpga_bridge_register(dev, "Fake FPGA Bridge",
>> +				      &fake_fpga_bridge_ops, priv);
>> +	if (IS_ERR(bridge))
>> +		return PTR_ERR(bridge);
>> +
>> +	platform_set_drvdata(pdev, bridge);
>> +
>> +	return 0;
>> +}
>> +
>> +static int fake_fpga_bridge_remove(struct platform_device *pdev)
>> +{
>> +	struct fpga_bridge *bridge = platform_get_drvdata(pdev);
>> +
>> +	fpga_bridge_unregister(bridge);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver fake_fpga_bridge_drv = {
>> +	.driver = {
>> +		.name = FAKE_FPGA_BRIDGE_DEV_NAME
>> +	},
>> +	.probe = fake_fpga_bridge_probe,
>> +	.remove = fake_fpga_bridge_remove,
>> +};
>> +
>> +module_platform_driver(fake_fpga_bridge_drv);
>> +
>> +MODULE_AUTHOR("Marco Pagani <marpagan@redhat.com>");
>> +MODULE_DESCRIPTION("Fake FPGA Bridge");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/fpga/tests/fake-fpga-bridge.h b/drivers/fpga/tests/fake-fpga-bridge.h
>> new file mode 100644
>> index 000000000000..ae224b13f284
>> --- /dev/null
>> +++ b/drivers/fpga/tests/fake-fpga-bridge.h
>> @@ -0,0 +1,36 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Header file for the fake FPGA bridge
>> + *
>> + * Copyright (C) 2023 Red Hat, Inc.
>> + *
>> + * Author: Marco Pagani <marpagan@redhat.com>
>> + */
>> +
>> +#ifndef __FPGA_FAKE_BRIDGE_H
>> +#define __FPGA_FAKE_BRIDGE_H
>> +
>> +#include <linux/platform_device.h>
>> +#include <kunit/test.h>
>> +
>> +/**
>> + * struct fake_fpga_bridge - fake FPGA bridge context data structure
>> + *
>> + * @bridge: FPGA bridge.
>> + * @pdev: platform device of the FPGA bridge.
>> + */
>> +struct fake_fpga_bridge {
>> +	struct fpga_bridge *bridge;
>> +	struct platform_device *pdev;
>> +};
>> +
>> +int fake_fpga_bridge_register(struct fake_fpga_bridge *bridge_ctx,
>> +			      struct device *parent, struct kunit *test);
>> +
>> +void fake_fpga_bridge_unregister(struct fake_fpga_bridge *bridge_ctx);
>> +
>> +int fake_fpga_bridge_get_state(const struct fake_fpga_bridge *bridge_ctx);
>> +
>> +int fake_fpga_bridge_get_cycles_count(const struct fake_fpga_bridge *bridge_ctx);
>> +
>> +#endif /* __FPGA_FAKE_BRIDGE_H */
>> -- 
>> 2.39.2
>>

Thanks,
Marco

Re: [RFC PATCH v2 2/4] fpga: add fake FPGA bridge

[Xu Yilun]

On 2023-03-21 at 18:33:05 +0100, Marco Pagani wrote:
> 
> 
> On 2023-03-17 09:32, Xu Yilun wrote:
> > On 2023-03-10 at 18:04:10 +0100, Marco Pagani wrote:
> >> Add fake FPGA bridge driver with support functions. The driver includes
> >> a counter for the number of switching cycles. This module is part of
> >> the KUnit tests for the FPGA subsystem.
> >>
> >> Signed-off-by: Marco Pagani <marpagan@redhat.com>
> >> ---
> >>  drivers/fpga/tests/fake-fpga-bridge.c | 228 ++++++++++++++++++++++++++
> >>  drivers/fpga/tests/fake-fpga-bridge.h |  36 ++++
> >>  2 files changed, 264 insertions(+)
> >>  create mode 100644 drivers/fpga/tests/fake-fpga-bridge.c
> >>  create mode 100644 drivers/fpga/tests/fake-fpga-bridge.h
> >>
> >> diff --git a/drivers/fpga/tests/fake-fpga-bridge.c b/drivers/fpga/tests/fake-fpga-bridge.c
> >> new file mode 100644
> >> index 000000000000..8a2f64fc1bbb
> >> --- /dev/null
> >> +++ b/drivers/fpga/tests/fake-fpga-bridge.c
> >> @@ -0,0 +1,228 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Driver for the fake FPGA bridge
> >> + *
> >> + * Copyright (C) 2023 Red Hat, Inc.
> >> + *
> >> + * Author: Marco Pagani <marpagan@redhat.com>
> >> + */
> >> +
> >> +#include <linux/types.h>
> >> +#include <linux/device.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/fpga/fpga-bridge.h>
> >> +#include <kunit/test.h>
> >> +
> >> +#include "fake-fpga-bridge.h"
> >> +
> >> +#define FAKE_FPGA_BRIDGE_DEV_NAME	"fake_fpga_bridge"
> >> +
> >> +struct fake_bridge_priv {
> >> +	int id;
> >> +	bool enable;
> >> +	int cycles_count;
> >> +	struct kunit *test;
> >> +};
> >> +
> >> +struct fake_bridge_data {
> >> +	struct kunit *test;
> >> +};
> >> +
> >> +static int op_enable_show(struct fpga_bridge *bridge)
> >> +{
> >> +	struct fake_bridge_priv *priv;
> >> +
> >> +	priv = bridge->priv;
> >> +
> >> +	if (priv->test)
> >> +		kunit_info(priv->test, "Fake FPGA bridge %d: enable_show\n",
> >> +			   priv->id);
> > 
> > Why check the kunit pointer every time? I remember you mentioned that
> > the fake fpga modules are expected to be used out of Kunit test, so the
> > priv->test may be NULL? I suggest you work on these usecases in separate
> > patchsets. For now just check priv->test on probe is fine.
> > 
> 
> The idea was to provide additional info messages, tied with the test, if the
> fake bridge is registered with a test instance. If you believe these prints
> are unnecessary, I can remove them or replace them with generic dev_info().

OK, on second thought, it's good to me.

Thanks,
Yilun

[RFC PATCH v2 3/4] fpga: add fake FPGA region

[Marco Pagani]

Add fake FPGA region platform driver with support functions. This
module is part of the KUnit tests for the FPGA subsystem.

Signed-off-by: Marco Pagani <marpagan@redhat.com>
---
 drivers/fpga/tests/fake-fpga-region.c | 219 ++++++++++++++++++++++++++
 drivers/fpga/tests/fake-fpga-region.h |  38 +++++
 2 files changed, 257 insertions(+)
 create mode 100644 drivers/fpga/tests/fake-fpga-region.c
 create mode 100644 drivers/fpga/tests/fake-fpga-region.h

diff --git a/drivers/fpga/tests/fake-fpga-region.c b/drivers/fpga/tests/fake-fpga-region.c
new file mode 100644
index 000000000000..54d0e564728b
--- /dev/null
+++ b/drivers/fpga/tests/fake-fpga-region.c
@@ -0,0 +1,219 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the fake FPGA region
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani <marpagan@redhat.com>
+ */
+
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/platform_device.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/fpga/fpga-region.h>
+#include <linux/fpga/fpga-bridge.h>
+#include <kunit/test.h>
+
+#include "fake-fpga-region.h"
+
+#define FAKE_FPGA_REGION_DEV_NAME	"fake_fpga_region"
+
+struct fake_region_priv {
+	int id;
+	struct kunit *test;
+	struct list_head bridge_list;
+};
+
+struct fake_region_data {
+	struct fpga_manager *mgr;
+	struct kunit *test;
+};
+
+/**
+ * fake_fpga_region_register() - register a fake FPGA region.
+ * @region_ctx: fake FPGA region context data structure.
+ * @mgr: associated FPGA manager.
+ * @parent: parent device.
+ * @test: KUnit test context object.
+ *
+ * Return: 0 if registration succeeded, an error code otherwise.
+ */
+int fake_fpga_region_register(struct fake_fpga_region *region_ctx,
+			      struct fpga_manager *mgr, struct device *parent,
+			      struct kunit *test)
+{
+	struct fake_region_data pdata;
+	struct fake_region_priv *priv;
+	int ret;
+
+	pdata.mgr = mgr;
+	pdata.test = test;
+
+	region_ctx->pdev = platform_device_alloc(FAKE_FPGA_REGION_DEV_NAME,
+						 PLATFORM_DEVID_AUTO);
+	if (IS_ERR(region_ctx->pdev)) {
+		pr_err("Fake FPGA region device allocation failed\n");
+		return -ENOMEM;
+	}
+
+	region_ctx->pdev->dev.parent = parent;
+	platform_device_add_data(region_ctx->pdev, &pdata, sizeof(pdata));
+
+	ret = platform_device_add(region_ctx->pdev);
+	if (ret) {
+		pr_err("Fake FPGA region device add failed\n");
+		platform_device_put(region_ctx->pdev);
+		return ret;
+	}
+
+	region_ctx->region = platform_get_drvdata(region_ctx->pdev);
+
+	if (test) {
+		priv = region_ctx->region->priv;
+		kunit_info(test, "Fake FPGA region %d registered\n", priv->id);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fake_fpga_region_register);
+
+/**
+ * fake_fpga_region_unregister() - unregister a fake FPGA region.
+ * @region_ctx: fake FPGA region context data structure.
+ */
+void fake_fpga_region_unregister(struct fake_fpga_region *region_ctx)
+{
+	struct fake_region_priv *priv;
+	struct kunit *test;
+	int id;
+
+	if (!region_ctx)
+		return;
+
+	priv = region_ctx->region->priv;
+	test = priv->test;
+	id = priv->id;
+
+	if (region_ctx->pdev) {
+		platform_device_unregister(region_ctx->pdev);
+		if (test)
+			kunit_info(test, "Fake FPGA region %d unregistered\n", id);
+	}
+}
+EXPORT_SYMBOL_GPL(fake_fpga_region_unregister);
+
+/**
+ * fake_fpga_region_add_bridge() - add a bridge to a fake FPGA region.
+ * @region_ctx: fake FPGA region context data structure.
+ * @bridge: FPGA bridge.
+ *
+ * Return: 0 if registration succeeded, an error code otherwise.
+ */
+void fake_fpga_region_add_bridge(struct fake_fpga_region *region_ctx,
+				 struct fpga_bridge *bridge)
+{
+	struct fake_region_priv *priv;
+
+	priv = region_ctx->region->priv;
+
+	/* Add bridge to the list of bridges in the private context */
+	list_add(&bridge->node, &priv->bridge_list);
+
+	if (priv->test)
+		kunit_info(priv->test, "Bridge added to fake FPGA region %d\n",
+			   priv->id);
+}
+EXPORT_SYMBOL_GPL(fake_fpga_region_add_bridge);
+
+static int fake_region_get_bridges(struct fpga_region *region)
+{
+	struct fake_region_priv *priv;
+	struct fpga_bridge *bridge, *tmp;
+	int ret;
+
+	priv = region->priv;
+
+	list_for_each_entry_safe(bridge, tmp, &priv->bridge_list, node) {
+		list_del(&bridge->node);
+		ret = fpga_bridge_get_to_list(bridge->dev.parent,
+					      region->info,
+					      &region->bridge_list);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+static int fake_fpga_region_probe(struct platform_device *pdev)
+{
+	struct device *dev;
+	struct fpga_region *region;
+	struct fpga_manager *mgr;
+	struct fake_region_data *pdata;
+	struct fake_region_priv *priv;
+	struct fpga_region_info info;
+	static int id_count;
+
+	dev = &pdev->dev;
+	pdata = dev_get_platdata(dev);
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "Missing platform data\n");
+		return -EINVAL;
+	}
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mgr = fpga_mgr_get(pdata->mgr->dev.parent);
+	if (IS_ERR(mgr))
+		return PTR_ERR(mgr);
+
+	INIT_LIST_HEAD(&priv->bridge_list);
+	priv->id = id_count++;
+	priv->test = pdata->test;
+
+	memset(&info, 0, sizeof(info));
+	info.priv = priv;
+	info.mgr = mgr;
+	info.get_bridges = fake_region_get_bridges;
+
+	region = fpga_region_register_full(dev, &info);
+	if (IS_ERR(region)) {
+		fpga_mgr_put(mgr);
+		return PTR_ERR(region);
+	}
+
+	platform_set_drvdata(pdev, region);
+
+	return 0;
+}
+
+static int fake_fpga_region_remove(struct platform_device *pdev)
+{
+	struct fpga_region *region = platform_get_drvdata(pdev);
+	struct fpga_manager *mgr = region->mgr;
+
+	fpga_mgr_put(mgr);
+	fpga_bridges_put(&region->bridge_list);
+	fpga_region_unregister(region);
+
+	return 0;
+}
+
+static struct platform_driver fake_fpga_region_drv = {
+	.driver = {
+		.name = FAKE_FPGA_REGION_DEV_NAME
+	},
+	.probe = fake_fpga_region_probe,
+	.remove = fake_fpga_region_remove,
+};
+
+module_platform_driver(fake_fpga_region_drv);
+
+MODULE_AUTHOR("Marco Pagani <marpagan@redhat.com>");
+MODULE_DESCRIPTION("Fake FPGA Bridge");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/fpga/tests/fake-fpga-region.h b/drivers/fpga/tests/fake-fpga-region.h
new file mode 100644
index 000000000000..9268ca335662
--- /dev/null
+++ b/drivers/fpga/tests/fake-fpga-region.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Header file for the fake FPGA region
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani <marpagan@redhat.com>
+ */
+
+#ifndef __FPGA_FAKE_RGN_H
+#define __FPGA_FAKE_RGN_H
+
+#include <linux/platform_device.h>
+#include <kunit/test.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/fpga/fpga-bridge.h>
+
+/**
+ * struct fake_fpga_region - fake FPGA region context data structure
+ *
+ * @region: FPGA region.
+ * @pdev: platform device of the FPGA region.
+ */
+struct fake_fpga_region {
+	struct fpga_region *region;
+	struct platform_device *pdev;
+};
+
+int fake_fpga_region_register(struct fake_fpga_region *region_ctx,
+			      struct fpga_manager *mgr, struct device *parent,
+			      struct kunit *test);
+
+void fake_fpga_region_add_bridge(struct fake_fpga_region *region_ctx,
+				 struct fpga_bridge *bridge);
+
+void fake_fpga_region_unregister(struct fake_fpga_region *region_ctx);
+
+#endif /* __FPGA_FAKE_RGN_H */
-- 
2.39.2

Re: [RFC PATCH v2 3/4] fpga: add fake FPGA region

[Xu Yilun]

On 2023-03-10 at 18:04:11 +0100, Marco Pagani wrote:
> Add fake FPGA region platform driver with support functions. This
> module is part of the KUnit tests for the FPGA subsystem.
> 
> Signed-off-by: Marco Pagani <marpagan@redhat.com>
> ---
>  drivers/fpga/tests/fake-fpga-region.c | 219 ++++++++++++++++++++++++++
>  drivers/fpga/tests/fake-fpga-region.h |  38 +++++
>  2 files changed, 257 insertions(+)
>  create mode 100644 drivers/fpga/tests/fake-fpga-region.c
>  create mode 100644 drivers/fpga/tests/fake-fpga-region.h
> 
> diff --git a/drivers/fpga/tests/fake-fpga-region.c b/drivers/fpga/tests/fake-fpga-region.c
> new file mode 100644
> index 000000000000..54d0e564728b
> --- /dev/null
> +++ b/drivers/fpga/tests/fake-fpga-region.c
> @@ -0,0 +1,219 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the fake FPGA region
> + *
> + * Copyright (C) 2023 Red Hat, Inc.
> + *
> + * Author: Marco Pagani <marpagan@redhat.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/list.h>
> +#include <linux/platform_device.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/fpga/fpga-region.h>
> +#include <linux/fpga/fpga-bridge.h>
> +#include <kunit/test.h>
> +
> +#include "fake-fpga-region.h"
> +
> +#define FAKE_FPGA_REGION_DEV_NAME	"fake_fpga_region"
> +
> +struct fake_region_priv {
> +	int id;
> +	struct kunit *test;
> +	struct list_head bridge_list;
> +};
> +
> +struct fake_region_data {
> +	struct fpga_manager *mgr;
> +	struct kunit *test;
> +};
> +
> +/**
> + * fake_fpga_region_register() - register a fake FPGA region.
> + * @region_ctx: fake FPGA region context data structure.
> + * @mgr: associated FPGA manager.
> + * @parent: parent device.
> + * @test: KUnit test context object.
> + *
> + * Return: 0 if registration succeeded, an error code otherwise.
> + */
> +int fake_fpga_region_register(struct fake_fpga_region *region_ctx,
> +			      struct fpga_manager *mgr, struct device *parent,
> +			      struct kunit *test)
> +{
> +	struct fake_region_data pdata;
> +	struct fake_region_priv *priv;
> +	int ret;
> +
> +	pdata.mgr = mgr;
> +	pdata.test = test;
> +
> +	region_ctx->pdev = platform_device_alloc(FAKE_FPGA_REGION_DEV_NAME,
> +						 PLATFORM_DEVID_AUTO);
> +	if (IS_ERR(region_ctx->pdev)) {
> +		pr_err("Fake FPGA region device allocation failed\n");
> +		return -ENOMEM;
> +	}
> +
> +	region_ctx->pdev->dev.parent = parent;
> +	platform_device_add_data(region_ctx->pdev, &pdata, sizeof(pdata));
> +
> +	ret = platform_device_add(region_ctx->pdev);
> +	if (ret) {
> +		pr_err("Fake FPGA region device add failed\n");
> +		platform_device_put(region_ctx->pdev);
> +		return ret;
> +	}
> +
> +	region_ctx->region = platform_get_drvdata(region_ctx->pdev);
> +
> +	if (test) {
> +		priv = region_ctx->region->priv;
> +		kunit_info(test, "Fake FPGA region %d registered\n", priv->id);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(fake_fpga_region_register);
> +
> +/**
> + * fake_fpga_region_unregister() - unregister a fake FPGA region.
> + * @region_ctx: fake FPGA region context data structure.
> + */
> +void fake_fpga_region_unregister(struct fake_fpga_region *region_ctx)
> +{
> +	struct fake_region_priv *priv;
> +	struct kunit *test;
> +	int id;
> +
> +	if (!region_ctx)
> +		return;
> +
> +	priv = region_ctx->region->priv;
> +	test = priv->test;
> +	id = priv->id;
> +
> +	if (region_ctx->pdev) {
> +		platform_device_unregister(region_ctx->pdev);
> +		if (test)
> +			kunit_info(test, "Fake FPGA region %d unregistered\n", id);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(fake_fpga_region_unregister);
> +
> +/**
> + * fake_fpga_region_add_bridge() - add a bridge to a fake FPGA region.
> + * @region_ctx: fake FPGA region context data structure.
> + * @bridge: FPGA bridge.
> + *
> + * Return: 0 if registration succeeded, an error code otherwise.
> + */
> +void fake_fpga_region_add_bridge(struct fake_fpga_region *region_ctx,
> +				 struct fpga_bridge *bridge)
> +{
> +	struct fake_region_priv *priv;
> +
> +	priv = region_ctx->region->priv;
> +
> +	/* Add bridge to the list of bridges in the private context */
> +	list_add(&bridge->node, &priv->bridge_list);
> +
> +	if (priv->test)
> +		kunit_info(priv->test, "Bridge added to fake FPGA region %d\n",
> +			   priv->id);
> +}
> +EXPORT_SYMBOL_GPL(fake_fpga_region_add_bridge);
> +
> +static int fake_region_get_bridges(struct fpga_region *region)
> +{
> +	struct fake_region_priv *priv;
> +	struct fpga_bridge *bridge, *tmp;
> +	int ret;
> +
> +	priv = region->priv;
> +
> +	list_for_each_entry_safe(bridge, tmp, &priv->bridge_list, node) {
> +		list_del(&bridge->node);

I think the fake_fpga_region user just need to call
fake_fpga_region_add_bridge() once on init, and may call
fpga_bridges_put() at any time after fpga_region_program_fpga(), then
you may lose the track of the bridges, which breaks the next
fpga_region_program_fpga().

Thanks,
Yilun

> +		ret = fpga_bridge_get_to_list(bridge->dev.parent,
> +					      region->info,
> +					      &region->bridge_list);
> +		if (ret)
> +			break;
> +	}
> +
> +	return ret;
> +}
> +
> +static int fake_fpga_region_probe(struct platform_device *pdev)
> +{
> +	struct device *dev;
> +	struct fpga_region *region;
> +	struct fpga_manager *mgr;
> +	struct fake_region_data *pdata;
> +	struct fake_region_priv *priv;
> +	struct fpga_region_info info;
> +	static int id_count;
> +
> +	dev = &pdev->dev;
> +	pdata = dev_get_platdata(dev);
> +
> +	if (!pdata) {
> +		dev_err(&pdev->dev, "Missing platform data\n");
> +		return -EINVAL;
> +	}
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	mgr = fpga_mgr_get(pdata->mgr->dev.parent);
> +	if (IS_ERR(mgr))
> +		return PTR_ERR(mgr);
> +
> +	INIT_LIST_HEAD(&priv->bridge_list);
> +	priv->id = id_count++;
> +	priv->test = pdata->test;
> +
> +	memset(&info, 0, sizeof(info));
> +	info.priv = priv;
> +	info.mgr = mgr;
> +	info.get_bridges = fake_region_get_bridges;
> +
> +	region = fpga_region_register_full(dev, &info);
> +	if (IS_ERR(region)) {
> +		fpga_mgr_put(mgr);
> +		return PTR_ERR(region);
> +	}
> +
> +	platform_set_drvdata(pdev, region);
> +
> +	return 0;
> +}
> +
> +static int fake_fpga_region_remove(struct platform_device *pdev)
> +{
> +	struct fpga_region *region = platform_get_drvdata(pdev);
> +	struct fpga_manager *mgr = region->mgr;
> +
> +	fpga_mgr_put(mgr);
> +	fpga_bridges_put(&region->bridge_list);
> +	fpga_region_unregister(region);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver fake_fpga_region_drv = {
> +	.driver = {
> +		.name = FAKE_FPGA_REGION_DEV_NAME
> +	},
> +	.probe = fake_fpga_region_probe,
> +	.remove = fake_fpga_region_remove,
> +};
> +
> +module_platform_driver(fake_fpga_region_drv);
> +
> +MODULE_AUTHOR("Marco Pagani <marpagan@redhat.com>");
> +MODULE_DESCRIPTION("Fake FPGA Bridge");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/fpga/tests/fake-fpga-region.h b/drivers/fpga/tests/fake-fpga-region.h
> new file mode 100644
> index 000000000000..9268ca335662
> --- /dev/null
> +++ b/drivers/fpga/tests/fake-fpga-region.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Header file for the fake FPGA region
> + *
> + * Copyright (C) 2023 Red Hat, Inc.
> + *
> + * Author: Marco Pagani <marpagan@redhat.com>
> + */
> +
> +#ifndef __FPGA_FAKE_RGN_H
> +#define __FPGA_FAKE_RGN_H
> +
> +#include <linux/platform_device.h>
> +#include <kunit/test.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/fpga/fpga-bridge.h>
> +
> +/**
> + * struct fake_fpga_region - fake FPGA region context data structure
> + *
> + * @region: FPGA region.
> + * @pdev: platform device of the FPGA region.
> + */
> +struct fake_fpga_region {
> +	struct fpga_region *region;
> +	struct platform_device *pdev;
> +};
> +
> +int fake_fpga_region_register(struct fake_fpga_region *region_ctx,
> +			      struct fpga_manager *mgr, struct device *parent,
> +			      struct kunit *test);
> +
> +void fake_fpga_region_add_bridge(struct fake_fpga_region *region_ctx,
> +				 struct fpga_bridge *bridge);
> +
> +void fake_fpga_region_unregister(struct fake_fpga_region *region_ctx);
> +
> +#endif /* __FPGA_FAKE_RGN_H */
> -- 
> 2.39.2
> 

Re: [RFC PATCH v2 3/4] fpga: add fake FPGA region

[Marco Pagani]

On 2023-03-17 11:40, Xu Yilun wrote:
> On 2023-03-10 at 18:04:11 +0100, Marco Pagani wrote:
>> Add fake FPGA region platform driver with support functions. This
>> module is part of the KUnit tests for the FPGA subsystem.
>>
>> Signed-off-by: Marco Pagani <marpagan@redhat.com>
>> ---
>>  drivers/fpga/tests/fake-fpga-region.c | 219 ++++++++++++++++++++++++++
>>  drivers/fpga/tests/fake-fpga-region.h |  38 +++++
>>  2 files changed, 257 insertions(+)
>>  create mode 100644 drivers/fpga/tests/fake-fpga-region.c
>>  create mode 100644 drivers/fpga/tests/fake-fpga-region.h
>>
>> diff --git a/drivers/fpga/tests/fake-fpga-region.c b/drivers/fpga/tests/fake-fpga-region.c
>> new file mode 100644
>> index 000000000000..54d0e564728b
>> --- /dev/null
>> +++ b/drivers/fpga/tests/fake-fpga-region.c
>> @@ -0,0 +1,219 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for the fake FPGA region
>> + *
>> + * Copyright (C) 2023 Red Hat, Inc.
>> + *
>> + * Author: Marco Pagani <marpagan@redhat.com>
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/list.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/fpga/fpga-mgr.h>
>> +#include <linux/fpga/fpga-region.h>
>> +#include <linux/fpga/fpga-bridge.h>
>> +#include <kunit/test.h>
>> +
>> +#include "fake-fpga-region.h"
>> +
>> +#define FAKE_FPGA_REGION_DEV_NAME	"fake_fpga_region"
>> +
>> +struct fake_region_priv {
>> +	int id;
>> +	struct kunit *test;
>> +	struct list_head bridge_list;
>> +};
>> +
>> +struct fake_region_data {
>> +	struct fpga_manager *mgr;
>> +	struct kunit *test;
>> +};
>> +
>> +/**
>> + * fake_fpga_region_register() - register a fake FPGA region.
>> + * @region_ctx: fake FPGA region context data structure.
>> + * @mgr: associated FPGA manager.
>> + * @parent: parent device.
>> + * @test: KUnit test context object.
>> + *
>> + * Return: 0 if registration succeeded, an error code otherwise.
>> + */
>> +int fake_fpga_region_register(struct fake_fpga_region *region_ctx,
>> +			      struct fpga_manager *mgr, struct device *parent,
>> +			      struct kunit *test)
>> +{
>> +	struct fake_region_data pdata;
>> +	struct fake_region_priv *priv;
>> +	int ret;
>> +
>> +	pdata.mgr = mgr;
>> +	pdata.test = test;
>> +
>> +	region_ctx->pdev = platform_device_alloc(FAKE_FPGA_REGION_DEV_NAME,
>> +						 PLATFORM_DEVID_AUTO);
>> +	if (IS_ERR(region_ctx->pdev)) {
>> +		pr_err("Fake FPGA region device allocation failed\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	region_ctx->pdev->dev.parent = parent;
>> +	platform_device_add_data(region_ctx->pdev, &pdata, sizeof(pdata));
>> +
>> +	ret = platform_device_add(region_ctx->pdev);
>> +	if (ret) {
>> +		pr_err("Fake FPGA region device add failed\n");
>> +		platform_device_put(region_ctx->pdev);
>> +		return ret;
>> +	}
>> +
>> +	region_ctx->region = platform_get_drvdata(region_ctx->pdev);
>> +
>> +	if (test) {
>> +		priv = region_ctx->region->priv;
>> +		kunit_info(test, "Fake FPGA region %d registered\n", priv->id);
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(fake_fpga_region_register);
>> +
>> +/**
>> + * fake_fpga_region_unregister() - unregister a fake FPGA region.
>> + * @region_ctx: fake FPGA region context data structure.
>> + */
>> +void fake_fpga_region_unregister(struct fake_fpga_region *region_ctx)
>> +{
>> +	struct fake_region_priv *priv;
>> +	struct kunit *test;
>> +	int id;
>> +
>> +	if (!region_ctx)
>> +		return;
>> +
>> +	priv = region_ctx->region->priv;
>> +	test = priv->test;
>> +	id = priv->id;
>> +
>> +	if (region_ctx->pdev) {
>> +		platform_device_unregister(region_ctx->pdev);
>> +		if (test)
>> +			kunit_info(test, "Fake FPGA region %d unregistered\n", id);
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(fake_fpga_region_unregister);
>> +
>> +/**
>> + * fake_fpga_region_add_bridge() - add a bridge to a fake FPGA region.
>> + * @region_ctx: fake FPGA region context data structure.
>> + * @bridge: FPGA bridge.
>> + *
>> + * Return: 0 if registration succeeded, an error code otherwise.
>> + */
>> +void fake_fpga_region_add_bridge(struct fake_fpga_region *region_ctx,
>> +				 struct fpga_bridge *bridge)
>> +{
>> +	struct fake_region_priv *priv;
>> +
>> +	priv = region_ctx->region->priv;
>> +
>> +	/* Add bridge to the list of bridges in the private context */
>> +	list_add(&bridge->node, &priv->bridge_list);
>> +
>> +	if (priv->test)
>> +		kunit_info(priv->test, "Bridge added to fake FPGA region %d\n",
>> +			   priv->id);
>> +}
>> +EXPORT_SYMBOL_GPL(fake_fpga_region_add_bridge);
>> +
>> +static int fake_region_get_bridges(struct fpga_region *region)
>> +{
>> +	struct fake_region_priv *priv;
>> +	struct fpga_bridge *bridge, *tmp;
>> +	int ret;
>> +
>> +	priv = region->priv;
>> +
>> +	list_for_each_entry_safe(bridge, tmp, &priv->bridge_list, node) {
>> +		list_del(&bridge->node);
> 
> I think the fake_fpga_region user just need to call
> fake_fpga_region_add_bridge() once on init, and may call
> fpga_bridges_put() at any time after fpga_region_program_fpga(), then
> you may lose the track of the bridges, which breaks the next
> fpga_region_program_fpga().
> 
> Thanks,
> Yilun
>

That's right, fake_fpga_region_add_bridge() is intended to be called once
(for each bridge) just after registering the region. I should have added
this to the kernel-doc description of the function.

When a fake region is unregistered, its bridges are released using
fpga_bridges_put() by the remove method of the platform driver.

In the fpga_test_static_cfg test case, the base region is configured
twice. Then, when the base region is unregistered by the exit method of
the test suite, the base bridge is released.

Here's the raw test output with dev_dbg() prints of the FPGA bridge
enabled:

    # Subtest: fpga
    1..2
    # fpga_test_static_cfg: Fake FPGA manager: state
    # fpga_test_static_cfg: Fake FPGA manager registered
    # fpga_test_static_cfg: Fake FPGA bridge 3 registered
    # fpga_test_static_cfg: Fake FPGA region 0 registered
    # fpga_test_static_cfg: Bridge added to fake FPGA region 0
    # fpga_test_static_cfg: FPGA base system built
    # fpga_test_static_cfg: Fake FPGA bridge 3: enable_show
    # fpga_test_static_cfg: FPGA image allocated in a buffer, size: 16384
fpga_bridge br0: get <---
fpga_bridge br0: disable
    # fpga_test_static_cfg: Fake FPGA bridge 3: enable_set: 0
    # fpga_test_static_cfg: Fake FPGA manager: parse_header
    # fpga_test_static_cfg: Fake FPGA manager: write_init
    # fpga_test_static_cfg: Fake FPGA manager: write
    # fpga_test_static_cfg: Fake FPGA manager: write_complete
fpga_bridge br0: enable
    # fpga_test_static_cfg: Fake FPGA bridge 3: enable_set: 1
    # fpga_test_static_cfg: Fake FPGA bridge 3: enable_show
    # fpga_test_static_cfg: FPGA configuration completed using a buffer image
    # fpga_test_static_cfg: FPGA image allocated in a scatter gather table, size: 16384
fpga_bridge br0: disable
    # fpga_test_static_cfg: Fake FPGA bridge 3: enable_set: 0
    # fpga_test_static_cfg: Fake FPGA manager: parse_header
    # fpga_test_static_cfg: Fake FPGA manager: write_init
    # fpga_test_static_cfg: Fake FPGA manager: write_sg
    # fpga_test_static_cfg: Fake FPGA manager: write_complete
fpga_bridge br0: enable
    # fpga_test_static_cfg: Fake FPGA bridge 3: enable_set: 1
    # fpga_test_static_cfg: Fake FPGA bridge 3: enable_show
    # fpga_test_static_cfg: FPGA configuration completed using scatter gather table image
fpga_bridge br0: put <---
    # fpga_test_static_cfg: Fake FPGA region 0 unregistered
    # fpga_test_static_cfg: Fake FPGA bridge: remove
    # fpga_test_static_cfg: Fake FPGA bridge 3 unregistered
fpga_manager fpga0: fpga_mgr_unregister Fake FPGA Manager
    # fpga_test_static_cfg: Fake FPGA manager: remove
    # fpga_test_static_cfg: Fake FPGA manager unregistered
    ok 1 fpga_test_static_cfg

In my understanding, of-region uses a similar approach by releasing
bridges only when the overlay is removed.


>> +		ret = fpga_bridge_get_to_list(bridge->dev.parent,
>> +					      region->info,
>> +					      &region->bridge_list);
>> +		if (ret)
>> +			break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int fake_fpga_region_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev;
>> +	struct fpga_region *region;
>> +	struct fpga_manager *mgr;
>> +	struct fake_region_data *pdata;
>> +	struct fake_region_priv *priv;
>> +	struct fpga_region_info info;
>> +	static int id_count;
>> +
>> +	dev = &pdev->dev;
>> +	pdata = dev_get_platdata(dev);
>> +
>> +	if (!pdata) {
>> +		dev_err(&pdev->dev, "Missing platform data\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	mgr = fpga_mgr_get(pdata->mgr->dev.parent);
>> +	if (IS_ERR(mgr))
>> +		return PTR_ERR(mgr);
>> +
>> +	INIT_LIST_HEAD(&priv->bridge_list);
>> +	priv->id = id_count++;
>> +	priv->test = pdata->test;
>> +
>> +	memset(&info, 0, sizeof(info));
>> +	info.priv = priv;
>> +	info.mgr = mgr;
>> +	info.get_bridges = fake_region_get_bridges;
>> +
>> +	region = fpga_region_register_full(dev, &info);
>> +	if (IS_ERR(region)) {
>> +		fpga_mgr_put(mgr);
>> +		return PTR_ERR(region);
>> +	}
>> +
>> +	platform_set_drvdata(pdev, region);
>> +
>> +	return 0;
>> +}
>> +
>> +static int fake_fpga_region_remove(struct platform_device *pdev)
>> +{
>> +	struct fpga_region *region = platform_get_drvdata(pdev);
>> +	struct fpga_manager *mgr = region->mgr;
>> +
>> +	fpga_mgr_put(mgr);
>> +	fpga_bridges_put(&region->bridge_list);
>> +	fpga_region_unregister(region);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver fake_fpga_region_drv = {
>> +	.driver = {
>> +		.name = FAKE_FPGA_REGION_DEV_NAME
>> +	},
>> +	.probe = fake_fpga_region_probe,
>> +	.remove = fake_fpga_region_remove,
>> +};
>> +
>> +module_platform_driver(fake_fpga_region_drv);
>> +
>> +MODULE_AUTHOR("Marco Pagani <marpagan@redhat.com>");
>> +MODULE_DESCRIPTION("Fake FPGA Bridge");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/fpga/tests/fake-fpga-region.h b/drivers/fpga/tests/fake-fpga-region.h
>> new file mode 100644
>> index 000000000000..9268ca335662
>> --- /dev/null
>> +++ b/drivers/fpga/tests/fake-fpga-region.h
>> @@ -0,0 +1,38 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Header file for the fake FPGA region
>> + *
>> + * Copyright (C) 2023 Red Hat, Inc.
>> + *
>> + * Author: Marco Pagani <marpagan@redhat.com>
>> + */
>> +
>> +#ifndef __FPGA_FAKE_RGN_H
>> +#define __FPGA_FAKE_RGN_H
>> +
>> +#include <linux/platform_device.h>
>> +#include <kunit/test.h>
>> +#include <linux/fpga/fpga-mgr.h>
>> +#include <linux/fpga/fpga-bridge.h>
>> +
>> +/**
>> + * struct fake_fpga_region - fake FPGA region context data structure
>> + *
>> + * @region: FPGA region.
>> + * @pdev: platform device of the FPGA region.
>> + */
>> +struct fake_fpga_region {
>> +	struct fpga_region *region;
>> +	struct platform_device *pdev;
>> +};
>> +
>> +int fake_fpga_region_register(struct fake_fpga_region *region_ctx,
>> +			      struct fpga_manager *mgr, struct device *parent,
>> +			      struct kunit *test);
>> +
>> +void fake_fpga_region_add_bridge(struct fake_fpga_region *region_ctx,
>> +				 struct fpga_bridge *bridge);
>> +
>> +void fake_fpga_region_unregister(struct fake_fpga_region *region_ctx);
>> +
>> +#endif /* __FPGA_FAKE_RGN_H */
>> -- 
>> 2.39.2
>>
> 

Thanks,
Marco

Re: [RFC PATCH v2 3/4] fpga: add fake FPGA region

[Xu Yilun]

On 2023-03-21 at 21:07:43 +0100, Marco Pagani wrote:
> 
> 
> On 2023-03-17 11:40, Xu Yilun wrote:
> > On 2023-03-10 at 18:04:11 +0100, Marco Pagani wrote:
> >> Add fake FPGA region platform driver with support functions. This
> >> module is part of the KUnit tests for the FPGA subsystem.
> >>
> >> Signed-off-by: Marco Pagani <marpagan@redhat.com>
> >> ---
> >>  drivers/fpga/tests/fake-fpga-region.c | 219 ++++++++++++++++++++++++++
> >>  drivers/fpga/tests/fake-fpga-region.h |  38 +++++
> >>  2 files changed, 257 insertions(+)
> >>  create mode 100644 drivers/fpga/tests/fake-fpga-region.c
> >>  create mode 100644 drivers/fpga/tests/fake-fpga-region.h
> >>
> >> diff --git a/drivers/fpga/tests/fake-fpga-region.c b/drivers/fpga/tests/fake-fpga-region.c
> >> new file mode 100644
> >> index 000000000000..54d0e564728b
> >> --- /dev/null
> >> +++ b/drivers/fpga/tests/fake-fpga-region.c
> >> @@ -0,0 +1,219 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Driver for the fake FPGA region
> >> + *
> >> + * Copyright (C) 2023 Red Hat, Inc.
> >> + *
> >> + * Author: Marco Pagani <marpagan@redhat.com>
> >> + */
> >> +
> >> +#include <linux/device.h>
> >> +#include <linux/list.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/fpga/fpga-mgr.h>
> >> +#include <linux/fpga/fpga-region.h>
> >> +#include <linux/fpga/fpga-bridge.h>
> >> +#include <kunit/test.h>
> >> +
> >> +#include "fake-fpga-region.h"
> >> +
> >> +#define FAKE_FPGA_REGION_DEV_NAME	"fake_fpga_region"
> >> +
> >> +struct fake_region_priv {
> >> +	int id;
> >> +	struct kunit *test;
> >> +	struct list_head bridge_list;
> >> +};
> >> +
> >> +struct fake_region_data {
> >> +	struct fpga_manager *mgr;
> >> +	struct kunit *test;
> >> +};
> >> +
> >> +/**
> >> + * fake_fpga_region_register() - register a fake FPGA region.
> >> + * @region_ctx: fake FPGA region context data structure.
> >> + * @mgr: associated FPGA manager.
> >> + * @parent: parent device.
> >> + * @test: KUnit test context object.
> >> + *
> >> + * Return: 0 if registration succeeded, an error code otherwise.
> >> + */
> >> +int fake_fpga_region_register(struct fake_fpga_region *region_ctx,
> >> +			      struct fpga_manager *mgr, struct device *parent,
> >> +			      struct kunit *test)
> >> +{
> >> +	struct fake_region_data pdata;
> >> +	struct fake_region_priv *priv;
> >> +	int ret;
> >> +
> >> +	pdata.mgr = mgr;
> >> +	pdata.test = test;
> >> +
> >> +	region_ctx->pdev = platform_device_alloc(FAKE_FPGA_REGION_DEV_NAME,
> >> +						 PLATFORM_DEVID_AUTO);
> >> +	if (IS_ERR(region_ctx->pdev)) {
> >> +		pr_err("Fake FPGA region device allocation failed\n");
> >> +		return -ENOMEM;
> >> +	}
> >> +
> >> +	region_ctx->pdev->dev.parent = parent;
> >> +	platform_device_add_data(region_ctx->pdev, &pdata, sizeof(pdata));
> >> +
> >> +	ret = platform_device_add(region_ctx->pdev);
> >> +	if (ret) {
> >> +		pr_err("Fake FPGA region device add failed\n");
> >> +		platform_device_put(region_ctx->pdev);
> >> +		return ret;
> >> +	}
> >> +
> >> +	region_ctx->region = platform_get_drvdata(region_ctx->pdev);
> >> +
> >> +	if (test) {
> >> +		priv = region_ctx->region->priv;
> >> +		kunit_info(test, "Fake FPGA region %d registered\n", priv->id);
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(fake_fpga_region_register);
> >> +
> >> +/**
> >> + * fake_fpga_region_unregister() - unregister a fake FPGA region.
> >> + * @region_ctx: fake FPGA region context data structure.
> >> + */
> >> +void fake_fpga_region_unregister(struct fake_fpga_region *region_ctx)
> >> +{
> >> +	struct fake_region_priv *priv;
> >> +	struct kunit *test;
> >> +	int id;
> >> +
> >> +	if (!region_ctx)
> >> +		return;
> >> +
> >> +	priv = region_ctx->region->priv;
> >> +	test = priv->test;
> >> +	id = priv->id;
> >> +
> >> +	if (region_ctx->pdev) {
> >> +		platform_device_unregister(region_ctx->pdev);
> >> +		if (test)
> >> +			kunit_info(test, "Fake FPGA region %d unregistered\n", id);
> >> +	}
> >> +}
> >> +EXPORT_SYMBOL_GPL(fake_fpga_region_unregister);
> >> +
> >> +/**
> >> + * fake_fpga_region_add_bridge() - add a bridge to a fake FPGA region.
> >> + * @region_ctx: fake FPGA region context data structure.
> >> + * @bridge: FPGA bridge.
> >> + *
> >> + * Return: 0 if registration succeeded, an error code otherwise.
> >> + */
> >> +void fake_fpga_region_add_bridge(struct fake_fpga_region *region_ctx,
> >> +				 struct fpga_bridge *bridge)
> >> +{
> >> +	struct fake_region_priv *priv;
> >> +
> >> +	priv = region_ctx->region->priv;
> >> +
> >> +	/* Add bridge to the list of bridges in the private context */
> >> +	list_add(&bridge->node, &priv->bridge_list);
> >> +
> >> +	if (priv->test)
> >> +		kunit_info(priv->test, "Bridge added to fake FPGA region %d\n",
> >> +			   priv->id);
> >> +}
> >> +EXPORT_SYMBOL_GPL(fake_fpga_region_add_bridge);
> >> +
> >> +static int fake_region_get_bridges(struct fpga_region *region)
> >> +{
> >> +	struct fake_region_priv *priv;
> >> +	struct fpga_bridge *bridge, *tmp;
> >> +	int ret;
> >> +
> >> +	priv = region->priv;
> >> +
> >> +	list_for_each_entry_safe(bridge, tmp, &priv->bridge_list, node) {
> >> +		list_del(&bridge->node);
> > 
> > I think the fake_fpga_region user just need to call
> > fake_fpga_region_add_bridge() once on init, and may call
> > fpga_bridges_put() at any time after fpga_region_program_fpga(), then
> > you may lose the track of the bridges, which breaks the next
> > fpga_region_program_fpga().
> > 
> > Thanks,
> > Yilun
> >
> 
> That's right, fake_fpga_region_add_bridge() is intended to be called once
> (for each bridge) just after registering the region. I should have added
> this to the kernel-doc description of the function.
> 
> When a fake region is unregistered, its bridges are released using
> fpga_bridges_put() by the remove method of the platform driver.

Please check the kernel-doc:

/**
 * fpga_region_program_fpga - program FPGA
 *
 * @region: FPGA region
 *
 * Program an FPGA using fpga image info (region->info).
 * If the region has a get_bridges function, the exclusive reference for the
 * bridges will be held if programming succeeds.  This is intended to prevent
 * reprogramming the region until the caller considers it safe to do so.
 * The caller will need to call fpga_bridges_put() before attempting to
 * reprogram the region.
 *
 * Return 0 for success or negative error code.
 */
int fpga_region_program_fpga(struct fpga_region *region)

That means the fpga test should call fpga_bridges_put() before a second
fpga_region_program_fpga().

> 
> In the fpga_test_static_cfg test case, the base region is configured
> twice. Then, when the base region is unregistered by the exit method of
> the test suite, the base bridge is released.
> 
> Here's the raw test output with dev_dbg() prints of the FPGA bridge
> enabled:
> 
>     # Subtest: fpga
>     1..2
>     # fpga_test_static_cfg: Fake FPGA manager: state
>     # fpga_test_static_cfg: Fake FPGA manager registered
>     # fpga_test_static_cfg: Fake FPGA bridge 3 registered
>     # fpga_test_static_cfg: Fake FPGA region 0 registered
>     # fpga_test_static_cfg: Bridge added to fake FPGA region 0
>     # fpga_test_static_cfg: FPGA base system built
>     # fpga_test_static_cfg: Fake FPGA bridge 3: enable_show
>     # fpga_test_static_cfg: FPGA image allocated in a buffer, size: 16384
> fpga_bridge br0: get <---
> fpga_bridge br0: disable
>     # fpga_test_static_cfg: Fake FPGA bridge 3: enable_set: 0
>     # fpga_test_static_cfg: Fake FPGA manager: parse_header
>     # fpga_test_static_cfg: Fake FPGA manager: write_init
>     # fpga_test_static_cfg: Fake FPGA manager: write
>     # fpga_test_static_cfg: Fake FPGA manager: write_complete
> fpga_bridge br0: enable
>     # fpga_test_static_cfg: Fake FPGA bridge 3: enable_set: 1
>     # fpga_test_static_cfg: Fake FPGA bridge 3: enable_show
>     # fpga_test_static_cfg: FPGA configuration completed using a buffer image
>     # fpga_test_static_cfg: FPGA image allocated in a scatter gather table, size: 16384
> fpga_bridge br0: disable
>     # fpga_test_static_cfg: Fake FPGA bridge 3: enable_set: 0
>     # fpga_test_static_cfg: Fake FPGA manager: parse_header
>     # fpga_test_static_cfg: Fake FPGA manager: write_init
>     # fpga_test_static_cfg: Fake FPGA manager: write_sg
>     # fpga_test_static_cfg: Fake FPGA manager: write_complete
> fpga_bridge br0: enable
>     # fpga_test_static_cfg: Fake FPGA bridge 3: enable_set: 1
>     # fpga_test_static_cfg: Fake FPGA bridge 3: enable_show
>     # fpga_test_static_cfg: FPGA configuration completed using scatter gather table image
> fpga_bridge br0: put <---
>     # fpga_test_static_cfg: Fake FPGA region 0 unregistered
>     # fpga_test_static_cfg: Fake FPGA bridge: remove
>     # fpga_test_static_cfg: Fake FPGA bridge 3 unregistered
> fpga_manager fpga0: fpga_mgr_unregister Fake FPGA Manager
>     # fpga_test_static_cfg: Fake FPGA manager: remove
>     # fpga_test_static_cfg: Fake FPGA manager unregistered
>     ok 1 fpga_test_static_cfg
> 
> In my understanding, of-region uses a similar approach by releasing
> bridges only when the overlay is removed.

The overlay removal is not equal to the of-fpga-region removal. Actually
the overlay could be applied & removed multiple times, and
fpga_region_program_fpga & fpga_bridges_put() are called each time.


Thanks,
Yilun

> 
> 
> >> +		ret = fpga_bridge_get_to_list(bridge->dev.parent,
> >> +					      region->info,
> >> +					      &region->bridge_list);
> >> +		if (ret)
> >> +			break;
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int fake_fpga_region_probe(struct platform_device *pdev)
> >> +{
> >> +	struct device *dev;
> >> +	struct fpga_region *region;
> >> +	struct fpga_manager *mgr;
> >> +	struct fake_region_data *pdata;
> >> +	struct fake_region_priv *priv;
> >> +	struct fpga_region_info info;
> >> +	static int id_count;
> >> +
> >> +	dev = &pdev->dev;
> >> +	pdata = dev_get_platdata(dev);
> >> +
> >> +	if (!pdata) {
> >> +		dev_err(&pdev->dev, "Missing platform data\n");
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >> +	if (!priv)
> >> +		return -ENOMEM;
> >> +
> >> +	mgr = fpga_mgr_get(pdata->mgr->dev.parent);
> >> +	if (IS_ERR(mgr))
> >> +		return PTR_ERR(mgr);
> >> +
> >> +	INIT_LIST_HEAD(&priv->bridge_list);
> >> +	priv->id = id_count++;
> >> +	priv->test = pdata->test;
> >> +
> >> +	memset(&info, 0, sizeof(info));
> >> +	info.priv = priv;
> >> +	info.mgr = mgr;
> >> +	info.get_bridges = fake_region_get_bridges;
> >> +
> >> +	region = fpga_region_register_full(dev, &info);
> >> +	if (IS_ERR(region)) {
> >> +		fpga_mgr_put(mgr);
> >> +		return PTR_ERR(region);
> >> +	}
> >> +
> >> +	platform_set_drvdata(pdev, region);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int fake_fpga_region_remove(struct platform_device *pdev)
> >> +{
> >> +	struct fpga_region *region = platform_get_drvdata(pdev);
> >> +	struct fpga_manager *mgr = region->mgr;
> >> +
> >> +	fpga_mgr_put(mgr);
> >> +	fpga_bridges_put(&region->bridge_list);
> >> +	fpga_region_unregister(region);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static struct platform_driver fake_fpga_region_drv = {
> >> +	.driver = {
> >> +		.name = FAKE_FPGA_REGION_DEV_NAME
> >> +	},
> >> +	.probe = fake_fpga_region_probe,
> >> +	.remove = fake_fpga_region_remove,
> >> +};
> >> +
> >> +module_platform_driver(fake_fpga_region_drv);
> >> +
> >> +MODULE_AUTHOR("Marco Pagani <marpagan@redhat.com>");
> >> +MODULE_DESCRIPTION("Fake FPGA Bridge");
> >> +MODULE_LICENSE("GPL v2");
> >> diff --git a/drivers/fpga/tests/fake-fpga-region.h b/drivers/fpga/tests/fake-fpga-region.h
> >> new file mode 100644
> >> index 000000000000..9268ca335662
> >> --- /dev/null
> >> +++ b/drivers/fpga/tests/fake-fpga-region.h
> >> @@ -0,0 +1,38 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +/*
> >> + * Header file for the fake FPGA region
> >> + *
> >> + * Copyright (C) 2023 Red Hat, Inc.
> >> + *
> >> + * Author: Marco Pagani <marpagan@redhat.com>
> >> + */
> >> +
> >> +#ifndef __FPGA_FAKE_RGN_H
> >> +#define __FPGA_FAKE_RGN_H
> >> +
> >> +#include <linux/platform_device.h>
> >> +#include <kunit/test.h>
> >> +#include <linux/fpga/fpga-mgr.h>
> >> +#include <linux/fpga/fpga-bridge.h>
> >> +
> >> +/**
> >> + * struct fake_fpga_region - fake FPGA region context data structure
> >> + *
> >> + * @region: FPGA region.
> >> + * @pdev: platform device of the FPGA region.
> >> + */
> >> +struct fake_fpga_region {
> >> +	struct fpga_region *region;
> >> +	struct platform_device *pdev;
> >> +};
> >> +
> >> +int fake_fpga_region_register(struct fake_fpga_region *region_ctx,
> >> +			      struct fpga_manager *mgr, struct device *parent,
> >> +			      struct kunit *test);
> >> +
> >> +void fake_fpga_region_add_bridge(struct fake_fpga_region *region_ctx,
> >> +				 struct fpga_bridge *bridge);
> >> +
> >> +void fake_fpga_region_unregister(struct fake_fpga_region *region_ctx);
> >> +
> >> +#endif /* __FPGA_FAKE_RGN_H */
> >> -- 
> >> 2.39.2
> >>
> > 
> 
> Thanks,
> Marco
> 

[RFC PATCH v2 4/4] fpga: add initial KUnit test suites

[Marco Pagani]

Introduce initial KUnit tests for the FPGA subsystem. Tests are
organized into three test suites. The first suite tests the FPGA
Manager. The second suite tests the FPGA Bridge. Finally, the last
test suite models a complete FPGA platform and tests static and
partial reconfiguration.

Signed-off-by: Marco Pagani <marpagan@redhat.com>
---
 drivers/fpga/Kconfig            |   2 +
 drivers/fpga/Makefile           |   3 +
 drivers/fpga/tests/.kunitconfig |   5 +
 drivers/fpga/tests/Kconfig      |  11 +
 drivers/fpga/tests/Makefile     |   6 +
 drivers/fpga/tests/fpga-test.c  | 501 ++++++++++++++++++++++++++++++++
 6 files changed, 528 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-test.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 6ce143dafd04..469d5bdd1a05 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..1cbea75dd29b
--- /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
+	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..0b052570659b
--- /dev/null
+++ b/drivers/fpga/tests/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_FPGA_KUNIT_TESTS) += fake-fpga-mgr.o
+obj-$(CONFIG_FPGA_KUNIT_TESTS) += fake-fpga-region.o
+obj-$(CONFIG_FPGA_KUNIT_TESTS) += fake-fpga-bridge.o
+obj-$(CONFIG_FPGA_KUNIT_TESTS) += fpga-test.o
diff --git a/drivers/fpga/tests/fpga-test.c b/drivers/fpga/tests/fpga-test.c
new file mode 100644
index 000000000000..df5f48dc2c54
--- /dev/null
+++ b/drivers/fpga/tests/fpga-test.c
@@ -0,0 +1,501 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for the FPGA subsystem
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani <marpagan@redhat.com>
+ */
+
+#include <kunit/test.h>
+#include <linux/list.h>
+#include <linux/platform_device.h>
+#include <linux/scatterlist.h>
+
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/fpga/fpga-region.h>
+#include <linux/fpga/fpga-bridge.h>
+
+#include "fake-fpga-region.h"
+#include "fake-fpga-bridge.h"
+#include "fake-fpga-mgr.h"
+
+#define STATIC_IMG_BLOCKS	16
+#define STATIC_IMG_SIZE		(FPGA_IMG_BLOCK * STATIC_IMG_BLOCKS)
+
+#define PARTIAL_IMG_BLOCKS	4
+#define PARTIAL_IMG_SIZE	(FPGA_IMG_BLOCK * PARTIAL_IMG_BLOCKS)
+
+/**
+ * buf_img_alloc() - Allocate a fake FPGA image using a buffer.
+ * @test: KUnit test context object.
+ * @dev: owning device.
+ * @size: image size.
+ *
+ * Return: pointer to a struct fpga_image_info or NULL on failure.
+ */
+static struct fpga_image_info *buf_img_alloc(struct kunit *test, struct device *dev,
+					     size_t size)
+{
+	struct fpga_image_info *img_info;
+	char *img_buf;
+
+	img_buf = kunit_kzalloc(test, size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, img_buf);
+	fake_fpga_mgr_fill_header(img_buf);
+
+	img_info = fpga_image_info_alloc(dev);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, img_info);
+
+	img_info->count = size;
+	img_info->buf = img_buf;
+
+	kunit_info(test, "FPGA image allocated in a buffer, size: %zu\n", size);
+
+	return img_info;
+}
+
+/**
+ * sgt_img_alloc() - Allocate a fake FPGA image using a scatter gather table.
+ * @test: KUnit test context object.
+ * @dev: owning device.
+ * @size: image size.
+ *
+ * Return: pointer to a struct fpga_image_info or NULL on failure.
+ */
+static struct fpga_image_info *sgt_img_alloc(struct kunit *test, struct device *dev,
+					     size_t size)
+{
+	struct fpga_image_info *img_info;
+	char *img_buf;
+	struct sg_table *sgt;
+	int ret;
+
+	img_buf = kunit_kzalloc(test, size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, img_buf);
+	fake_fpga_mgr_fill_header(img_buf);
+
+	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, size);
+
+	img_info = fpga_image_info_alloc(dev);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, img_info);
+
+	img_info->sgt = sgt;
+
+	kunit_info(test, "FPGA image allocated in a scatter gather table, size: %zu\n",
+		   size);
+
+	return img_info;
+}
+
+/**
+ * img_free() - Free a fake FPGA image
+ * @img_info: fpga image information struct.
+ *
+ */
+static void img_free(struct fpga_image_info *img_info)
+{
+	if (!img_info)
+		return;
+
+	if (img_info->sgt)
+		sg_free_table(img_info->sgt);
+
+	fpga_image_info_free(img_info);
+}
+
+static int fpga_mgr_test_init(struct kunit *test)
+{
+	struct fake_fpga_mgr *mgr_ctx;
+	int ret;
+
+	mgr_ctx = kunit_kzalloc(test, sizeof(*mgr_ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mgr_ctx);
+
+	ret = fake_fpga_mgr_register(mgr_ctx, test);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	test->priv = mgr_ctx;
+
+	return 0;
+}
+
+static void fpga_mgr_test_img_load_buf(struct kunit *test)
+{
+	struct fake_fpga_mgr *mgr_ctx;
+	struct fpga_image_info *img_info;
+	int ret;
+
+	mgr_ctx = test->priv;
+
+	/* Allocate an FPGA image using a buffer */
+	img_info = buf_img_alloc(test, &mgr_ctx->pdev->dev, STATIC_IMG_SIZE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, img_info);
+
+	KUNIT_EXPECT_EQ(test, 0, fake_fpga_mgr_get_rcfg_count(mgr_ctx));
+
+	ret = fpga_mgr_load(mgr_ctx->mgr, img_info);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	fake_fpga_mgr_check_write_buf(mgr_ctx);
+
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_mgr_get_rcfg_count(mgr_ctx));
+
+	img_free(img_info);
+}
+
+static void fpga_mgr_test_img_load_sgt(struct kunit *test)
+{
+	struct fake_fpga_mgr *mgr_ctx;
+	struct fpga_image_info *img_info;
+	int ret;
+
+	mgr_ctx = test->priv;
+
+	/* Allocate an FPGA image using a scatter gather table */
+	img_info = sgt_img_alloc(test, &mgr_ctx->pdev->dev, STATIC_IMG_SIZE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, img_info);
+
+	KUNIT_EXPECT_EQ(test, 0, fake_fpga_mgr_get_rcfg_count(mgr_ctx));
+
+	ret = fpga_mgr_load(mgr_ctx->mgr, img_info);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	fake_fpga_mgr_check_write_sgt(mgr_ctx);
+
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_mgr_get_rcfg_count(mgr_ctx));
+
+	img_free(img_info);
+}
+
+static void fpga_mgr_test_exit(struct kunit *test)
+{
+	struct fake_fpga_mgr *mgr_ctx;
+
+	mgr_ctx = test->priv;
+
+	if (mgr_ctx)
+		fake_fpga_mgr_unregister(mgr_ctx);
+}
+
+static struct kunit_case fpga_mgr_test_cases[] = {
+	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,
+};
+
+static int fpga_bridge_test_init(struct kunit *test)
+{
+	struct fake_fpga_bridge *bridge_ctx;
+	int ret;
+
+	bridge_ctx = kunit_kzalloc(test, sizeof(*bridge_ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bridge_ctx);
+
+	ret = fake_fpga_bridge_register(bridge_ctx, NULL, test);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	test->priv = bridge_ctx;
+
+	return 0;
+}
+
+static void fpga_bridge_test_exit(struct kunit *test)
+{
+	struct fake_fpga_bridge *bridge_ctx;
+
+	bridge_ctx = test->priv;
+
+	if (bridge_ctx)
+		fake_fpga_bridge_unregister(bridge_ctx);
+}
+
+static void fpga_bridge_test_toggle(struct kunit *test)
+{
+	struct fake_fpga_bridge *bridge_ctx;
+
+	bridge_ctx = test->priv;
+
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_bridge_get_state(bridge_ctx));
+
+	fpga_bridge_disable(bridge_ctx->bridge);
+	KUNIT_EXPECT_EQ(test, 0, fake_fpga_bridge_get_state(bridge_ctx));
+
+	fpga_bridge_enable(bridge_ctx->bridge);
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_bridge_get_state(bridge_ctx));
+}
+
+static void fpga_bridge_test_get_put_list(struct kunit *test)
+{
+	struct list_head bridge_list;
+	struct fake_fpga_bridge *bridge_0_ctx, *bridge_1_ctx;
+	int ret;
+
+	bridge_0_ctx = test->priv;
+
+	/* Register another bridge for this test */
+	bridge_1_ctx = kunit_kzalloc(test, sizeof(*bridge_1_ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bridge_1_ctx);
+
+	ret = fake_fpga_bridge_register(bridge_1_ctx, NULL, test);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	INIT_LIST_HEAD(&bridge_list);
+
+	/* Get bridge_0 and add it to the list */
+	ret = fpga_bridge_get_to_list(bridge_1_ctx->bridge->dev.parent, NULL,
+				      &bridge_list);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+
+	KUNIT_EXPECT_PTR_EQ(test, bridge_1_ctx->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(bridge_0_ctx->bridge->dev.parent, NULL,
+				      &bridge_list);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+
+	KUNIT_EXPECT_PTR_EQ(test, bridge_0_ctx->bridge,
+			    list_first_entry_or_null(&bridge_list, struct fpga_bridge, node));
+
+	/* Put and remove both bridges from the list */
+	fpga_bridges_put(&bridge_list);
+
+	KUNIT_EXPECT_TRUE(test, list_empty(&bridge_list));
+
+	fake_fpga_bridge_unregister(bridge_1_ctx);
+}
+
+static struct kunit_case fpga_bridge_test_cases[] = {
+	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,
+};
+
+struct fpga_base_ctx {
+	/*
+	 * Base FPGA layout consisting of a single region
+	 * controlled by a bridge and the FPGA manager
+	 */
+	struct fake_fpga_mgr *mgr_ctx;
+	struct fake_fpga_bridge *bridge_ctx;
+	struct fake_fpga_region *region_ctx;
+};
+
+static int fpga_test_init(struct kunit *test)
+{
+	struct fpga_base_ctx *base_ctx;
+	int ret;
+
+	base_ctx = kunit_kzalloc(test, sizeof(*base_ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, base_ctx);
+	test->priv = base_ctx;
+
+	/* Build the base FPGA layout */
+	base_ctx->mgr_ctx = kunit_kzalloc(test, sizeof(*base_ctx->mgr_ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, base_ctx->mgr_ctx);
+	ret = fake_fpga_mgr_register(base_ctx->mgr_ctx, test);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	base_ctx->bridge_ctx = kunit_kzalloc(test, sizeof(*base_ctx->bridge_ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, base_ctx->bridge_ctx);
+	ret = fake_fpga_bridge_register(base_ctx->bridge_ctx, NULL, test);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	/* The base region a child of the base bridge */
+	base_ctx->region_ctx = kunit_kzalloc(test, sizeof(*base_ctx->region_ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, base_ctx->region_ctx);
+	ret = fake_fpga_region_register(base_ctx->region_ctx, base_ctx->mgr_ctx->mgr,
+					&base_ctx->bridge_ctx->bridge->dev, test);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	fake_fpga_region_add_bridge(base_ctx->region_ctx, base_ctx->bridge_ctx->bridge);
+
+	kunit_info(test, "FPGA base system built\n");
+
+	KUNIT_EXPECT_EQ(test, 0, fake_fpga_mgr_get_rcfg_count(base_ctx->mgr_ctx));
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_bridge_get_state(base_ctx->bridge_ctx));
+	KUNIT_EXPECT_EQ(test, 0, fake_fpga_bridge_get_cycles_count(base_ctx->bridge_ctx));
+
+	return 0;
+}
+
+static void fpga_test_exit(struct kunit *test)
+{
+	struct fpga_base_ctx *base_ctx;
+
+	base_ctx = test->priv;
+
+	if (!base_ctx)
+		return;
+
+	if (base_ctx->region_ctx)
+		fake_fpga_region_unregister(base_ctx->region_ctx);
+
+	if (base_ctx->bridge_ctx)
+		fake_fpga_bridge_unregister(base_ctx->bridge_ctx);
+
+	if (base_ctx->mgr_ctx)
+		fake_fpga_mgr_unregister(base_ctx->mgr_ctx);
+}
+
+static void fpga_test_static_cfg(struct kunit *test)
+{
+	struct fpga_base_ctx *base_ctx;
+	struct fpga_image_info *buf_img_info;
+	struct fpga_image_info *sgt_img_info;
+	int ret;
+
+	base_ctx = test->priv;
+
+	buf_img_info = buf_img_alloc(test, &base_ctx->mgr_ctx->pdev->dev, STATIC_IMG_SIZE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf_img_info);
+
+	/* Configure the FPGA using the image in a buffer */
+	base_ctx->region_ctx->region->info = buf_img_info;
+	ret = fpga_region_program_fpga(base_ctx->region_ctx->region);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	fake_fpga_mgr_check_write_buf(base_ctx->mgr_ctx);
+
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_mgr_get_rcfg_count(base_ctx->mgr_ctx));
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_bridge_get_state(base_ctx->bridge_ctx));
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_bridge_get_cycles_count(base_ctx->bridge_ctx));
+
+	kunit_info(test, "FPGA configuration completed using a buffer image\n");
+
+	sgt_img_info = sgt_img_alloc(test, &base_ctx->mgr_ctx->pdev->dev, STATIC_IMG_SIZE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sgt_img_info);
+
+	/* Re-configure the FPGA using the image in a scatter list */
+	base_ctx->region_ctx->region->info = sgt_img_info;
+	ret = fpga_region_program_fpga(base_ctx->region_ctx->region);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	fake_fpga_mgr_check_write_sgt(base_ctx->mgr_ctx);
+
+	KUNIT_EXPECT_EQ(test, 2, fake_fpga_mgr_get_rcfg_count(base_ctx->mgr_ctx));
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_bridge_get_state(base_ctx->bridge_ctx));
+	KUNIT_EXPECT_EQ(test, 2, fake_fpga_bridge_get_cycles_count(base_ctx->bridge_ctx));
+
+	kunit_info(test, "FPGA configuration completed using scatter gather table image\n");
+
+	img_free(sgt_img_info);
+}
+
+static void fpga_test_partial_rcfg(struct kunit *test)
+{
+	struct fpga_base_ctx *base_ctx;
+	struct fake_fpga_region *sub_region_0_ctx, *sub_region_1_ctx;
+	struct fake_fpga_bridge *sub_bridge_0_ctx, *sub_bridge_1_ctx;
+	struct fpga_image_info *partial_img_info;
+	int ret;
+
+	base_ctx = test->priv;
+
+	/*
+	 * Add two reconfigurable sub-regions, each controlled by a bridge. The
+	 * reconfigurable sub-region are children of their bridges which are,
+	 * in turn, children of the base region. For simplicity, the same image
+	 * is used to configure reconfigurable regions
+	 */
+	sub_bridge_0_ctx = kunit_kzalloc(test, sizeof(*sub_bridge_0_ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sub_bridge_0_ctx);
+	ret = fake_fpga_bridge_register(sub_bridge_0_ctx,
+					&base_ctx->region_ctx->region->dev, test);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	sub_region_0_ctx = kunit_kzalloc(test, sizeof(*sub_region_0_ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sub_region_0_ctx);
+	ret = fake_fpga_region_register(sub_region_0_ctx, base_ctx->mgr_ctx->mgr,
+					&sub_bridge_0_ctx->bridge->dev, test);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	fake_fpga_region_add_bridge(sub_region_0_ctx, sub_bridge_0_ctx->bridge);
+
+	sub_bridge_1_ctx = kunit_kzalloc(test, sizeof(*sub_bridge_1_ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sub_bridge_1_ctx);
+	ret = fake_fpga_bridge_register(sub_bridge_1_ctx,
+					&base_ctx->region_ctx->region->dev, test);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	sub_region_1_ctx = kunit_kzalloc(test, sizeof(*sub_region_1_ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sub_region_1_ctx);
+	ret = fake_fpga_region_register(sub_region_1_ctx, base_ctx->mgr_ctx->mgr,
+					&sub_bridge_1_ctx->bridge->dev, test);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	fake_fpga_region_add_bridge(sub_region_1_ctx, sub_bridge_1_ctx->bridge);
+
+	/* Allocate a partial image using a buffer */
+	partial_img_info = buf_img_alloc(test, &base_ctx->mgr_ctx->pdev->dev,
+					 PARTIAL_IMG_SIZE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, partial_img_info);
+	partial_img_info->flags = FPGA_MGR_PARTIAL_RECONFIG;
+
+	/* Re-configure sub-region 0 with the partial image */
+	sub_region_0_ctx->region->info = partial_img_info;
+	ret = fpga_region_program_fpga(sub_region_0_ctx->region);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	fake_fpga_mgr_check_write_buf(base_ctx->mgr_ctx);
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_mgr_get_rcfg_count(base_ctx->mgr_ctx));
+
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_bridge_get_state(sub_bridge_0_ctx));
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_bridge_get_cycles_count(sub_bridge_0_ctx));
+
+	/* Re-configure sub-region 1 with the partial image */
+	sub_region_1_ctx->region->info = partial_img_info;
+	ret = fpga_region_program_fpga(sub_region_1_ctx->region);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	fake_fpga_mgr_check_write_buf(base_ctx->mgr_ctx);
+	KUNIT_EXPECT_EQ(test, 2, fake_fpga_mgr_get_rcfg_count(base_ctx->mgr_ctx));
+
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_bridge_get_state(sub_bridge_1_ctx));
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_bridge_get_cycles_count(sub_bridge_1_ctx));
+
+	/* Check that the base bridge has not been disabled during reconfiguration */
+	KUNIT_EXPECT_EQ(test, 1, fake_fpga_bridge_get_state(base_ctx->bridge_ctx));
+	KUNIT_EXPECT_EQ(test, 0, fake_fpga_bridge_get_cycles_count(base_ctx->bridge_ctx));
+
+	img_free(partial_img_info);
+	fake_fpga_region_unregister(sub_region_0_ctx);
+	fake_fpga_bridge_unregister(sub_bridge_0_ctx);
+	fake_fpga_region_unregister(sub_region_1_ctx);
+	fake_fpga_bridge_unregister(sub_bridge_1_ctx);
+}
+
+static struct kunit_case fpga_test_cases[] = {
+	KUNIT_CASE(fpga_test_static_cfg),
+	KUNIT_CASE(fpga_test_partial_rcfg),
+	{}
+};
+
+static struct kunit_suite fpga_suite = {
+	.name = "fpga",
+	.init = fpga_test_init,
+	.exit = fpga_test_exit,
+	.test_cases = fpga_test_cases,
+};
+
+kunit_test_suites(&fpga_mgr_suite, &fpga_bridge_suite, &fpga_suite);
+
+MODULE_LICENSE("GPL v2");
-- 
2.39.2