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 <[email protected]>
---
Documentation/dmatest.txt | 33 ++++++++++++++++
drivers/dma/dmatest.c | 92 +++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 122 insertions(+), 3 deletions(-)
diff --git a/Documentation/dmatest.txt b/Documentation/dmatest.txt
index 279ac0a..d092646 100644
--- a/Documentation/dmatest.txt
+++ b/Documentation/dmatest.txt
@@ -79,3 +79,36 @@ Comparison between buffers is stored to the dedicated structure.
Note that the verify result is now accessible only via file 'results' in the
debugfs.
+
+ Part 5 - Masking tests for channel capabilities
+
+By default, the dmatest module concurrently tests all the hardware capabilities
+(MEMCPY, XOR and PQ) supported by a particular DMA channel. In order to disable
+any capability/capabilities from being tested, the user needs to modify the
+file 'cap_mask' in debugfs.
+
+Method to enable the capabilities:
+
+PATH = /sys/kernel/debug/dmatest/cap_mask
+echo copy,xor,pq > $PATH # Enables all the capability tests (DEFAULT)
+echo xor > $PATH # Enables XOR capability test
+
+Note: No whitespaces between the capabilities.
+ Only COMMA (,) to be used for separation.
+
+Example to perform only MEMCPY and PQ mode tests:
+
+ % modprobe dmatest
+ % echo dma0chan0 > /sys/kernel/debug/dmatest/channel
+ % echo copy,pq > /sys/kernel/debug/dmatest/cap_mask
+ % echo 1 > /sys/kernel/debug/dmatest/iterations
+ % echo 1 > /sys/kernel/debug/dmatest/run
+
+ Output:
+ % cat /sys/kernel/debug/dmatest/results
+
+ dma0chan0-copy0: #1: No errors with src_off=0xXX dst_off=0xXX len=0xXX
+ dma0chan0-pq0: #1: No errors with src_off=0xXX dst_off=0xXX len=0xXX
+
+Note: The result is available only for the enabled capabilities of
+ particular channel.
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);
+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];
};
/**
@@ -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;
+}
+
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;
+ }
+
+ /* 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;
}
@@ -1183,6 +1237,31 @@ static const struct file_operations dtf_results_fops = {
.release = single_release,
};
+static ssize_t dtf_read_cap_mask(struct file *file, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct dmatest_info *info = file->private_data;
+ return simple_read_from_buffer(buf, count, ppos,
+ info->dbgfs_params.cap_mask,
+ strlen(info->dbgfs_params.cap_mask));
+}
+
+static ssize_t dtf_write_cap_mask(struct file *file, const char __user *buf,
+ size_t size, loff_t *ppos)
+{
+ struct dmatest_info *info = file->private_data;
+ return dtf_write_string(info->dbgfs_params.cap_mask,
+ sizeof(info->dbgfs_params.cap_mask),
+ ppos, buf, size);
+}
+
+static const struct file_operations dtf_cap_mask_fops = {
+ .read = dtf_read_cap_mask,
+ .write = dtf_write_cap_mask,
+ .open = simple_open,
+ .llseek = default_llseek,
+};
+
static int dmatest_register_dbgfs(struct dmatest_info *info)
{
struct dentry *d;
@@ -1247,6 +1326,12 @@ static int dmatest_register_dbgfs(struct dmatest_info *info)
if (IS_ERR_OR_NULL(d))
goto err_node;
+ /* Mask bits for hardware capabilities tests */
+ d = debugfs_create_file("cap_mask", S_IRUGO | S_IWUSR, info->root,
+ info, &dtf_cap_mask_fops);
+ if (IS_ERR_OR_NULL(d))
+ goto err_node;
+
/* Run or stop threaded test */
d = debugfs_create_file("run", S_IWUSR | S_IRUGO, info->root,
info, &dtf_run_fops);
@@ -1292,6 +1377,7 @@ static int __init dmatest_init(void)
params->xor_sources = xor_sources;
params->pq_sources = pq_sources;
params->timeout = timeout;
+ strlcpy(params->cap_mask, test_cap_mask, sizeof(params->cap_mask));
ret = dmatest_register_dbgfs(info);
if (ret)
--
1.7.9.5
On Fri, Jun 21, 2013 at 04:46:59PM -0700, 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 <[email protected]>
> ---
> Documentation/dmatest.txt | 33 ++++++++++++++++
> drivers/dma/dmatest.c | 92 +++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 122 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/dmatest.txt b/Documentation/dmatest.txt
> index 279ac0a..d092646 100644
> --- a/Documentation/dmatest.txt
> +++ b/Documentation/dmatest.txt
> @@ -79,3 +79,36 @@ Comparison between buffers is stored to the dedicated structure.
>
> Note that the verify result is now accessible only via file 'results' in the
> debugfs.
> +
> + Part 5 - Masking tests for channel capabilities
> +
> +By default, the dmatest module concurrently tests all the hardware capabilities
> +(MEMCPY, XOR and PQ) supported by a particular DMA channel. In order to disable
> +any capability/capabilities from being tested, the user needs to modify the
> +file 'cap_mask' in debugfs.
> +
> +Method to enable the capabilities:
> +
> +PATH = /sys/kernel/debug/dmatest/cap_mask
> +echo copy,xor,pq > $PATH # Enables all the capability tests (DEFAULT)
> +echo xor > $PATH # Enables XOR capability test
> +
> +Note: No whitespaces between the capabilities.
> + Only COMMA (,) to be used for separation.
> +
> +Example to perform only MEMCPY and PQ mode tests:
> +
> + % modprobe dmatest
> + % echo dma0chan0 > /sys/kernel/debug/dmatest/channel
> + % echo copy,pq > /sys/kernel/debug/dmatest/cap_mask
> + % echo 1 > /sys/kernel/debug/dmatest/iterations
> + % echo 1 > /sys/kernel/debug/dmatest/run
> +
> + Output:
> + % cat /sys/kernel/debug/dmatest/results
> +
> + dma0chan0-copy0: #1: No errors with src_off=0xXX dst_off=0xXX len=0xXX
> + dma0chan0-pq0: #1: No errors with src_off=0xXX dst_off=0xXX len=0xXX
> +
> +Note: The result is available only for the enabled capabilities of
> + particular channel.
> 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);
> +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];
> };
>
> /**
> @@ -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);
It might be a bit nit picky, but "memcpy" instead of "copy" would
better match the docs and what users think.
The rest looks fine to me.
Thanks,
Jon
> + 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;
> +}
> +
> 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;
> + }
> +
> + /* 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;
> }
> @@ -1183,6 +1237,31 @@ static const struct file_operations dtf_results_fops = {
> .release = single_release,
> };
>
> +static ssize_t dtf_read_cap_mask(struct file *file, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct dmatest_info *info = file->private_data;
> + return simple_read_from_buffer(buf, count, ppos,
> + info->dbgfs_params.cap_mask,
> + strlen(info->dbgfs_params.cap_mask));
> +}
> +
> +static ssize_t dtf_write_cap_mask(struct file *file, const char __user *buf,
> + size_t size, loff_t *ppos)
> +{
> + struct dmatest_info *info = file->private_data;
> + return dtf_write_string(info->dbgfs_params.cap_mask,
> + sizeof(info->dbgfs_params.cap_mask),
> + ppos, buf, size);
> +}
> +
> +static const struct file_operations dtf_cap_mask_fops = {
> + .read = dtf_read_cap_mask,
> + .write = dtf_write_cap_mask,
> + .open = simple_open,
> + .llseek = default_llseek,
> +};
> +
> static int dmatest_register_dbgfs(struct dmatest_info *info)
> {
> struct dentry *d;
> @@ -1247,6 +1326,12 @@ static int dmatest_register_dbgfs(struct dmatest_info *info)
> if (IS_ERR_OR_NULL(d))
> goto err_node;
>
> + /* Mask bits for hardware capabilities tests */
> + d = debugfs_create_file("cap_mask", S_IRUGO | S_IWUSR, info->root,
> + info, &dtf_cap_mask_fops);
> + if (IS_ERR_OR_NULL(d))
> + goto err_node;
> +
> /* Run or stop threaded test */
> d = debugfs_create_file("run", S_IWUSR | S_IRUGO, info->root,
> info, &dtf_run_fops);
> @@ -1292,6 +1377,7 @@ static int __init dmatest_init(void)
> params->xor_sources = xor_sources;
> params->pq_sources = pq_sources;
> params->timeout = timeout;
> + strlcpy(params->cap_mask, test_cap_mask, sizeof(params->cap_mask));
>
> ret = dmatest_register_dbgfs(info);
> if (ret)
> --
> 1.7.9.5
>
On Fri, Jun 21, 2013 at 4:46 PM, Jubin Mehta <[email protected]> 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 <[email protected]>
[..]
> 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
On Wed, Jun 26, 2013 at 01:47:07PM -0700, Dan Williams wrote:
> On Fri, Jun 21, 2013 at 4:46 PM, Jubin Mehta <[email protected]> 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 <[email protected]>
> [..]
> > 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.
>
I shall do that. But, instead of having just the cap_mask parameter as
a writable module parameter, it would be great if we could have all
the module parameters writable and just have run file (to trigger the
test) and results file (to display the results of the test) in debugfs.
> The 'get' routine can also display the available testable operation
> types.
>
The testable operation types depend on the dma channel under test.
Thus, a 'get' routine cannot be used to obtain a default value for
the module parameter 'cap_mask'.
--
Jubin
> > +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