On Wed, Jun 28, 2017 at 3:46 AM, Sinan Kaya <okaya@xxxxxxxxxxxxxx> wrote:
> Introducing memset test into dmatest. This change allows us to test
> memset capable HW using the dmatest test procedure. The new dmatest
> value for memset is 2 and it is not the default value.
>
> Memset support patch shares the same code path as the other dmatest
> code to reuse as much as we can.
>
> The first value inside the source buffer is used as a pattern
> to fill in the destination buffer space.
>
> Prior to running the test, source/destination buffers are initialized
> in the current code.
>
> "The remaining bits are the inverse of a counter which increments by
> one for each byte address."
>
> Memset test will fill in the upper bits of pattern with the inverse of
> fixed counter value 1 as opposed to an incrementing value in a loop.
>
> An example run is as follows:
>
> echo dma0chan0 > /sys/module/dmatest/parameters/channel
> echo 2 > /sys/module/dmatest/parameters/dmatest
> echo 2000 > /sys/module/dmatest/parameters/timeout
> echo 10 > /sys/module/dmatest/parameters/iterations
> echo 1 > /sys/module/dmatest/parameters/run
Good someone is developing tests!
My comments below.
> static void dmatest_init_srcs(u8 **bufs, unsigned int start, unsigned int len,
> + unsigned int buf_size, bool is_memset)
> {
> unsigned int i;
> u8 *buf;
>
> for (; (buf = *bufs); bufs++) {
> + for (i = 0; i < start; i++) {
> + unsigned int val = is_memset ? PATTERN_MEMSET_IDX : i;
> +
> + buf[i] = PATTERN_SRC | (~val & PATTERN_COUNT_MASK);
> + unsigned int val = is_memset ? PATTERN_MEMSET_IDX : i;
> +
> buf[i] = PATTERN_SRC | PATTERN_COPY
> + | (~val & PATTERN_COUNT_MASK);
> + unsigned int val = is_memset ? PATTERN_MEMSET_IDX : i;
> +
> + buf[i] = PATTERN_SRC | (~val & PATTERN_COUNT_MASK);
Looks like a candidate now for a helper function
static inline u8 gen_src_value(u8 index, bool is_memset)
{
u8 val = is_memset ? PATTERN_MEMSET_IDX : i;
return PATTERN_SRC | (~val & PATTERN_COUNT_MASK);
}
buf[i] = gen_src_value(i, is_memset);
buf[i] = gen_src_value(i, is_memset) | PATTERN_COPY;
buf[i] = gen_src_value(i, is_memset);
> + unsigned int val = is_memset ? PATTERN_MEMSET_IDX : i;
> +
> + buf[i] = PATTERN_DST | (~val & PATTERN_COUNT_MASK);
> + unsigned int val = is_memset ? PATTERN_MEMSET_IDX : i;
> +
> buf[i] = PATTERN_DST | PATTERN_OVERWRITE
> + | (~val & PATTERN_COUNT_MASK);
> + unsigned int val = is_memset ? PATTERN_MEMSET_IDX : i;
> +
> + buf[i] = PATTERN_DST | (~val & PATTERN_COUNT_MASK);
Ditto for DST.
> + unsigned int counter, bool is_srcbuf, bool is_memset)
This is a signal to replace it with
unsigned int flags
#define DMATEST_FL_SRCBUF BIT(0)
#define DMATEST_FL_MEMSET BIT(1)
or alike.
I'm not insisting, just consider an opportunity.
> + bool is_srcbuf, bool is_memset)
Ditto.
> start = ktime_get();
> +
> pr_debug("%s: verifying source buffer...\n", current->comm);
The change doesn't belong to the patch.
> +
> error_count += dmatest_verify(thread->dsts, dst_off + len,
Ditto.
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
