Linux-FPGA Archive mirror
 help / color / mirror / Atom feed
From: Xu Yilun <yilun.xu@intel.com>
To: Marco Pagani <marpagan@redhat.com>
Cc: Moritz Fischer <mdf@kernel.org>, Wu Hao <hao.wu@intel.com>,
	Tom Rix <trix@redhat.com>,
	linux-kernel@vger.kernel.org, linux-fpga@vger.kernel.org
Subject: Re: [PATCH v8 1/4] fpga: add an initial KUnit suite for the FPGA Manager
Date: Wed, 12 Jul 2023 10:00:16 +0800	[thread overview]
Message-ID: <ZK4JMDZfdVyFkN92@yilunxu-OptiPlex-7050> (raw)
In-Reply-To: <1d03a695-d603-a4d5-b726-4b0ff6bc33f8@redhat.com>

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 <marpagan@redhat.com>
> >> ---
> >>  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 <marpagan@redhat.com>
> >> + */
> >> +
> 
> [...]
> 
> >> +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;
> >> +}
> > 
> 

  reply	other threads:[~2023-07-12  2:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-30 15:25 [PATCH v8 0/4] fpga: add initial KUnit tests for the subsystem Marco Pagani
2023-06-30 15:25 ` [PATCH v8 1/4] fpga: add an initial KUnit suite for the FPGA Manager Marco Pagani
2023-07-10  4:44   ` Xu Yilun
2023-07-11 14:02     ` Marco Pagani
2023-07-12  2:00       ` Xu Yilun [this message]
2023-06-30 15:25 ` [PATCH v8 2/4] fpga: add an initial KUnit suite for the FPGA Bridge Marco Pagani
2023-06-30 15:25 ` [PATCH v8 3/4] fpga: add an initial KUnit suite for the FPGA Region Marco Pagani
2023-06-30 15:25 ` [PATCH v8 4/4] fpga: add configuration for the FPGA KUnit test suites Marco Pagani
2023-07-10  4:47   ` Xu Yilun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZK4JMDZfdVyFkN92@yilunxu-OptiPlex-7050 \
    --to=yilun.xu@intel.com \
    --cc=hao.wu@intel.com \
    --cc=linux-fpga@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marpagan@redhat.com \
    --cc=mdf@kernel.org \
    --cc=trix@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).