2023-06-30 15:30:42

by Marco Pagani

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

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

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

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



2023-07-10 05:30:15

by Xu Yilun

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

On 2023-06-30 at 17:25:04 +0200, Marco Pagani wrote:
> The suite tests the basic behaviors of the FPGA Manager including
> programming using a single contiguous buffer and a scatter gather table.
>
> Signed-off-by: Marco Pagani <[email protected]>
> ---
> drivers/fpga/tests/fpga-mgr-test.c | 311 +++++++++++++++++++++++++++++
> 1 file changed, 311 insertions(+)
> create mode 100644 drivers/fpga/tests/fpga-mgr-test.c
>
> diff --git a/drivers/fpga/tests/fpga-mgr-test.c b/drivers/fpga/tests/fpga-mgr-test.c
> new file mode 100644
> index 000000000000..6fd2e235f195
> --- /dev/null
> +++ b/drivers/fpga/tests/fpga-mgr-test.c
> @@ -0,0 +1,311 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the FPGA Manager
> + *
> + * Copyright (C) 2023 Red Hat, Inc.
> + *
> + * Author: Marco Pagani <[email protected]>
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/device.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/module.h>
> +#include <linux/scatterlist.h>
> +#include <linux/types.h>
> +
> +#define HEADER_FILL 'H'
> +#define IMAGE_FILL 'P'
> +#define IMAGE_BLOCK 1024
> +
> +#define HEADER_SIZE IMAGE_BLOCK
> +#define IMAGE_SIZE (IMAGE_BLOCK * 4)
> +
> +struct mgr_stats {
> + bool header_match;
> + bool image_match;
> + u32 seq_num;
> + u32 op_parse_header_seq;
> + u32 op_write_init_seq;
> + u32 op_write_seq;
> + u32 op_write_sg_seq;
> + u32 op_write_complete_seq;
> + enum fpga_mgr_states op_parse_header_state;
> + enum fpga_mgr_states op_write_init_state;
> + enum fpga_mgr_states op_write_state;
> + enum fpga_mgr_states op_write_sg_state;
> + enum fpga_mgr_states op_write_complete_state;
> +};
> +
> +struct mgr_ctx {
> + struct fpga_image_info *img_info;
> + struct fpga_manager *mgr;
> + struct platform_device *pdev;
> + struct mgr_stats stats;
> +};
> +
> +/**
> + * init_test_buffer() - Allocate and initialize a test image in a buffer.
> + * @test: KUnit test context object.
> + * @count: image size in bytes.
> + *
> + * Return: pointer to the newly allocated image.
> + */
> +static char *init_test_buffer(struct kunit *test, size_t count)
> +{
> + char *buf;
> +
> + KUNIT_ASSERT_GE(test, count, HEADER_SIZE);
> +
> + buf = kunit_kzalloc(test, count, GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
> +
> + memset(buf, HEADER_FILL, HEADER_SIZE);
> + memset(buf + HEADER_SIZE, IMAGE_FILL, count - HEADER_SIZE);
> +
> + return buf;
> +}
> +
> +static int op_parse_header(struct fpga_manager *mgr, struct fpga_image_info *info,
> + const char *buf, size_t count)
> +{
> + struct mgr_stats *stats = mgr->priv;
> + size_t i;
> +
> + /* Set header_size and data_size for later */
> + info->header_size = HEADER_SIZE;
> + info->data_size = info->count - HEADER_SIZE;
> +
> + stats->header_match = true;
> +
> + /* Check header */
> + for (i = 0; i < info->header_size; i++)
> + if (buf[i] != HEADER_FILL)
> + stats->header_match = false;
> +
> + stats->op_parse_header_state = mgr->state;
> + stats->op_parse_header_seq = stats->seq_num++;
> +
> + return 0;
> +}
> +
> +static int op_write_init(struct fpga_manager *mgr, struct fpga_image_info *info,
> + const char *buf, size_t count)
> +{
> + struct mgr_stats *stats = mgr->priv;
> +
> + stats->op_write_init_state = mgr->state;
> + stats->op_write_init_seq = stats->seq_num++;
> +
> + return 0;
> +}
> +
> +static int op_write(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> + struct mgr_stats *stats = mgr->priv;
> + size_t i;
> +
> + /* Check the image */
> + stats->image_match = true;
> + for (i = 0; i < count; i++)
> + if (buf[i] != IMAGE_FILL)
> + stats->image_match = false;
> +
> + stats->op_write_state = mgr->state;
> + stats->op_write_seq = stats->seq_num++;
> +
> + return 0;
> +}
> +
> +static int op_write_sg(struct fpga_manager *mgr, struct sg_table *sgt)
> +{
> + struct mgr_stats *stats = mgr->priv;
> + struct sg_mapping_iter miter;
> + char *img;
> + size_t i;
> +
> + /*
> + * Check the image, but first skip the header since write_sg will get
> + * the whole image in sg_table.
> + */
> + stats->image_match = true;
> + sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
> +
> + if (!sg_miter_skip(&miter, HEADER_SIZE))
> + stats->image_match = false;

If this fails, should we continue?

> +
> + while (sg_miter_next(&miter)) {
> + img = miter.addr;
> + for (i = 0; i < miter.length; i++) {
> + if (img[i] != IMAGE_FILL)
> + stats->image_match = false;
> + }
> + }
> +
> + sg_miter_stop(&miter);
> +
> + stats->op_write_sg_state = mgr->state;
> + stats->op_write_sg_seq = stats->seq_num++;
> +
> + return 0;
> +}

2023-07-11 14:20:12

by Marco Pagani

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



On 2023-07-10 06:44, Xu Yilun wrote:
> On 2023-06-30 at 17:25:04 +0200, Marco Pagani wrote:
>> The suite tests the basic behaviors of the FPGA Manager including
>> programming using a single contiguous buffer and a scatter gather table.
>>
>> Signed-off-by: Marco Pagani <[email protected]>
>> ---
>> drivers/fpga/tests/fpga-mgr-test.c | 311 +++++++++++++++++++++++++++++
>> 1 file changed, 311 insertions(+)
>> create mode 100644 drivers/fpga/tests/fpga-mgr-test.c
>>
>> diff --git a/drivers/fpga/tests/fpga-mgr-test.c b/drivers/fpga/tests/fpga-mgr-test.c
>> new file mode 100644
>> index 000000000000..6fd2e235f195
>> --- /dev/null
>> +++ b/drivers/fpga/tests/fpga-mgr-test.c
>> @@ -0,0 +1,311 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * KUnit test for the FPGA Manager
>> + *
>> + * Copyright (C) 2023 Red Hat, Inc.
>> + *
>> + * Author: Marco Pagani <[email protected]>
>> + */
>> +

[...]

>> +static int op_write(struct fpga_manager *mgr, const char *buf, size_t count)
>> +{
>> + struct mgr_stats *stats = mgr->priv;
>> + size_t i;
>> +
>> + /* Check the image */
>> + stats->image_match = true;
>> + for (i = 0; i < count; i++)
>> + if (buf[i] != IMAGE_FILL)
>> + stats->image_match = false;
>> +
>> + stats->op_write_state = mgr->state;
>> + stats->op_write_seq = stats->seq_num++;
>> +
>> + return 0;
>> +}
>> +
>> +static int op_write_sg(struct fpga_manager *mgr, struct sg_table *sgt)
>> +{
>> + struct mgr_stats *stats = mgr->priv;
>> + struct sg_mapping_iter miter;
>> + char *img;
>> + size_t i;
>> +
>> + /*
>> + * Check the image, but first skip the header since write_sg will get
>> + * the whole image in sg_table.
>> + */
>> + stats->image_match = true;
>> + sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
>> +
>> + if (!sg_miter_skip(&miter, HEADER_SIZE))
>> + stats->image_match = false;
>
> If this fails, should we continue?

Would it be okay to set the image_match flag to false and then
return 0 if sg_miter_skip() fails?

I think returning an error code to the FPGA manager would not
be beneficial in this case since if an op fails, it is a failure
of the FPGA manager itself, not the low-level driver that tests
the FPGA manager.


>
>> +
>> + while (sg_miter_next(&miter)) {
>> + img = miter.addr;
>> + for (i = 0; i < miter.length; i++) {
>> + if (img[i] != IMAGE_FILL)
>> + stats->image_match = false;
>> + }
>> + }
>> +
>> + sg_miter_stop(&miter);
>> +
>> + stats->op_write_sg_state = mgr->state;
>> + stats->op_write_sg_seq = stats->seq_num++;
>> +
>> + return 0;
>> +}
>


2023-07-12 02:54:06

by Xu Yilun

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

On 2023-07-11 at 16:02:44 +0200, Marco Pagani wrote:
>
>
> On 2023-07-10 06:44, Xu Yilun wrote:
> > On 2023-06-30 at 17:25:04 +0200, Marco Pagani wrote:
> >> The suite tests the basic behaviors of the FPGA Manager including
> >> programming using a single contiguous buffer and a scatter gather table.
> >>
> >> Signed-off-by: Marco Pagani <[email protected]>
> >> ---
> >> drivers/fpga/tests/fpga-mgr-test.c | 311 +++++++++++++++++++++++++++++
> >> 1 file changed, 311 insertions(+)
> >> create mode 100644 drivers/fpga/tests/fpga-mgr-test.c
> >>
> >> diff --git a/drivers/fpga/tests/fpga-mgr-test.c b/drivers/fpga/tests/fpga-mgr-test.c
> >> new file mode 100644
> >> index 000000000000..6fd2e235f195
> >> --- /dev/null
> >> +++ b/drivers/fpga/tests/fpga-mgr-test.c
> >> @@ -0,0 +1,311 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * KUnit test for the FPGA Manager
> >> + *
> >> + * Copyright (C) 2023 Red Hat, Inc.
> >> + *
> >> + * Author: Marco Pagani <[email protected]>
> >> + */
> >> +
>
> [...]
>
> >> +static int op_write(struct fpga_manager *mgr, const char *buf, size_t count)
> >> +{
> >> + struct mgr_stats *stats = mgr->priv;
> >> + size_t i;
> >> +
> >> + /* Check the image */
> >> + stats->image_match = true;
> >> + for (i = 0; i < count; i++)
> >> + if (buf[i] != IMAGE_FILL)
> >> + stats->image_match = false;
> >> +
> >> + stats->op_write_state = mgr->state;
> >> + stats->op_write_seq = stats->seq_num++;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int op_write_sg(struct fpga_manager *mgr, struct sg_table *sgt)
> >> +{
> >> + struct mgr_stats *stats = mgr->priv;
> >> + struct sg_mapping_iter miter;
> >> + char *img;
> >> + size_t i;
> >> +
> >> + /*
> >> + * Check the image, but first skip the header since write_sg will get
> >> + * the whole image in sg_table.
> >> + */
> >> + stats->image_match = true;
> >> + sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
> >> +
> >> + if (!sg_miter_skip(&miter, HEADER_SIZE))
> >> + stats->image_match = false;
> >
> > If this fails, should we continue?
>
> Would it be okay to set the image_match flag to false and then
> return 0 if sg_miter_skip() fails?

That's good to me.

>
> I think returning an error code to the FPGA manager would not
> be beneficial in this case since if an op fails, it is a failure
> of the FPGA manager itself, not the low-level driver that tests
> the FPGA manager.

So the idea is we never return error code in any fake driver ops?
That's OK. Please add some comments in code for it and the reason.

Thanks,
Yilun

>
>
> >
> >> +
> >> + while (sg_miter_next(&miter)) {
> >> + img = miter.addr;
> >> + for (i = 0; i < miter.length; i++) {
> >> + if (img[i] != IMAGE_FILL)
> >> + stats->image_match = false;
> >> + }
> >> + }
> >> +
> >> + sg_miter_stop(&miter);
> >> +
> >> + stats->op_write_sg_state = mgr->state;
> >> + stats->op_write_sg_seq = stats->seq_num++;
> >> +
> >> + return 0;
> >> +}
> >
>