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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux