[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA9_cmePsWFfqmnEGB6XBJj7dsY7dSJtSLvy+s3NzVkttviRWQ@mail.gmail.com>
Date: Wed, 26 Jun 2013 13:47:07 -0700
From: Dan Williams <djbw@...com>
To: Jubin Mehta <jubin.mehta@...el.com>
Cc: Vinod Koul <vinod.koul@...el.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Dave Jiang <dave.jiang@...el.com>,
Jon Mason <jon.mason@...el.com>
Subject: Re: [PATCH v2] dmatest: masking tests for channel capabilities
On Fri, Jun 21, 2013 at 4:46 PM, Jubin Mehta <jubin.mehta@...el.com> wrote:
> The current dmatest module tests all the hardware capabilities (MEMCPY, XOR
> and PQ) supported by a particular DMA channel and these tests are performed
> concurrently by default. This patch allows the user to enable or disable the
> test performed for any particular capability. The mask bits for enabling the
> tests are set using the debugfs.
>
> Signed-off-by: Jubin Mehta <jubin.mehta@...el.com>
[..]
> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index d8ce4ec..c152755 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -23,6 +23,7 @@
> #include <linux/debugfs.h>
> #include <linux/uaccess.h>
> #include <linux/seq_file.h>
> +#include <linux/string.h>
>
> static unsigned int test_buf_size = 16384;
> module_param(test_buf_size, uint, S_IRUGO);
> @@ -66,6 +67,11 @@ module_param(timeout, uint, S_IRUGO);
> MODULE_PARM_DESC(timeout, "Transfer Timeout in msec (default: 3000), "
> "Pass -1 for infinite timeout");
>
> +static char test_cap_mask[20] = "copy,xor,pq";
> +module_param_string(cap_mask, test_cap_mask, sizeof(test_cap_mask), S_IRUGO);
Let's just make this module parameter writable so we don't need to add
a new debugfs file.
The 'get' routine can also display the available testable operation types.
> +MODULE_PARM_DESC(cap_mask, "Mask bits for different capability test "
> + "(default: copy,xor,pq)");
> +
> /* Maximum amount of mismatched bytes in buffer to print */
> #define MAX_ERROR_COUNT 32
>
> @@ -164,6 +170,7 @@ struct dmatest_chan {
> * @xor_sources: number of xor source buffers
> * @pq_sources: number of p+q source buffers
> * @timeout: transfer timeout in msec, -1 for infinite timeout
> + * @cap_mask: mask for different hardware capabilities tests
> */
> struct dmatest_params {
> unsigned int buf_size;
> @@ -175,6 +182,7 @@ struct dmatest_params {
> unsigned int xor_sources;
> unsigned int pq_sources;
> int timeout;
> + char cap_mask[128];
This can be the capability mask directly... see below:
> };
>
> /**
> @@ -883,11 +891,44 @@ static int dmatest_add_threads(struct dmatest_info *info,
> return i;
> }
>
> +static int dmatest_cap_check(struct dmatest_info *info,
> + dma_cap_mask_t *test_cap_mask)
> +{
> + struct dmatest_params *params;
> + char *cap;
> + unsigned char c;
> + int i;
> +
> + params = &info->params;
> + cap = (params->cap_mask);
> +
> + for (i = 0; i < 3; i++) {
> + if (!strncmp(cap, "copy", 4)) {
> + dma_cap_set(DMA_MEMCPY, *test_cap_mask);
> + cap += 4;
> + } else if (!strncmp(cap, "xor", 3)) {
> + dma_cap_set(DMA_XOR, *test_cap_mask);
> + cap += 3;
> + } else if (!strncmp(cap, "pq", 2)) {
> + dma_cap_set(DMA_PQ, *test_cap_mask);
> + cap += 2;
> + } else {
> + return -1;
> + }
> + c = *cap++;
> + if (c != ',')
> + break;
> + }
> + return 0;
> +}
We can run this routine when the capabilities are set to return error
on invalid capability masks directly. Then we can set the
test_cap_mask before dmatest_add_channel().
> +
> static int dmatest_add_channel(struct dmatest_info *info,
> struct dma_chan *chan)
> {
> struct dmatest_chan *dtc;
> struct dma_device *dma_dev = chan->device;
> + struct dmatest_params *params;
> + dma_cap_mask_t test_cap_mask;
> unsigned int thread_count = 0;
> int cnt;
>
> @@ -900,15 +941,28 @@ static int dmatest_add_channel(struct dmatest_info *info,
> dtc->chan = chan;
> INIT_LIST_HEAD(&dtc->threads);
>
> - if (dma_has_cap(DMA_MEMCPY, dma_dev->cap_mask)) {
> + params = &info->params;
> +
> + dma_cap_zero(test_cap_mask);
> + cnt = dmatest_cap_check(info, &test_cap_mask);
> + if (cnt) {
> + pr_warn("dmatest: Invalid Capability Mask Paramters\n");
> + return -EINVAL;
> + }
As above I think this too late to be validating the parameters.
> +
> + /* Run dmatest threads for the supported and enabled capabilities */
> + if (dma_has_cap(DMA_MEMCPY, dma_dev->cap_mask)
> + && dma_has_cap(DMA_MEMCPY, test_cap_mask)) {
> cnt = dmatest_add_threads(info, dtc, DMA_MEMCPY);
> thread_count += cnt > 0 ? cnt : 0;
> }
> - if (dma_has_cap(DMA_XOR, dma_dev->cap_mask)) {
> + if (dma_has_cap(DMA_XOR, dma_dev->cap_mask)
> + && dma_has_cap(DMA_XOR, test_cap_mask)) {
> cnt = dmatest_add_threads(info, dtc, DMA_XOR);
> thread_count += cnt > 0 ? cnt : 0;
> }
> - if (dma_has_cap(DMA_PQ, dma_dev->cap_mask)) {
> + if (dma_has_cap(DMA_PQ, dma_dev->cap_mask)
> + && dma_has_cap(DMA_PQ, test_cap_mask)) {
> cnt = dmatest_add_threads(info, dtc, DMA_PQ);
> thread_count += cnt > 0 ? cnt : 0;
> }
Could above be written simpler with one bitmap_and(test_cap_mask,
dma_dev->cap_mask, test_cap_mask) at the start and then
for_each_set_bit(test_cap_mask)?
--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists