Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751731Ab3FZUrL (ORCPT ); Wed, 26 Jun 2013 16:47:11 -0400 Received: from mail-lb0-f173.google.com ([209.85.217.173]:34005 "EHLO mail-lb0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750843Ab3FZUrJ (ORCPT ); Wed, 26 Jun 2013 16:47:09 -0400 MIME-Version: 1.0 In-Reply-To: <20130621234647.GA3763@intel.com> References: <20130621234647.GA3763@intel.com> Date: Wed, 26 Jun 2013 13:47:07 -0700 X-Google-Sender-Auth: e-aGSgApecYNHcSMdGMD6GzJUUQ Message-ID: Subject: Re: [PATCH v2] dmatest: masking tests for channel capabilities From: Dan Williams To: Jubin Mehta Cc: Vinod Koul , Linux Kernel Mailing List , Andy Shevchenko , Dave Jiang , Jon Mason Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5824 Lines: 156 On Fri, Jun 21, 2013 at 4:46 PM, Jubin Mehta 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 [..] > 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 > #include > #include > +#include > > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/