diff mbox

Adding a parameter to set a minimal length for dmatest

Message ID 87y49aplrf.fsf@free-electrons.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Gregory CLEMENT March 22, 2016, 3:36 p.m. UTC
Hi,

while using the dmatest module to test the mv_xor.c driver, I got
failures. I finally found that when the "noverify" parameter is not set,
then the buffer length used is totally random (modulo the maximum size
of the buffer). But in the mv_xor_prep_dma_xor there is a test about the
minimal size the dmaengine can use. So when the length was under this
minimum size then I got some test failures like the following: "dmatest:
dma1chan0-copy0: result #11: 'prep error' with src_off=0x29c9
dst_off=0x1c51 len=0x33 (0)".

Unless the drivers are supposed to accept all buffer size I think it is
not an error, and I would like to be able to use this dmatest for
automatic test and not to have to check if the error is a real one or
not.

I think that the following patch could improve the dmatest module.

What do you think about it?

Did I miss something or is a good ieda to send a proper patch?

Thanks!

Gregory

Comments

Dan Williams March 22, 2016, 3:41 p.m. UTC | #1
On Tue, Mar 22, 2016 at 8:36 AM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
>
> Hi,
>
> while using the dmatest module to test the mv_xor.c driver, I got
> failures. I finally found that when the "noverify" parameter is not set,
> then the buffer length used is totally random (modulo the maximum size
> of the buffer). But in the mv_xor_prep_dma_xor there is a test about the
> minimal size the dmaengine can use. So when the length was under this
> minimum size then I got some test failures like the following: "dmatest:
> dma1chan0-copy0: result #11: 'prep error' with src_off=0x29c9
> dst_off=0x1c51 len=0x33 (0)".
>
> Unless the drivers are supposed to accept all buffer size I think it is
> not an error, and I would like to be able to use this dmatest for
> automatic test and not to have to check if the error is a real one or
> not.
>
> I think that the following patch could improve the dmatest module.
>
> What do you think about it?
>
> Did I miss something or is a good ieda to send a proper patch?

Looks like a useful patch to me.
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gregory CLEMENT March 22, 2016, 3:57 p.m. UTC | #2
Hi Dan,
 
 On mar., mars 22 2016, Dan Williams <dan.j.williams@intel.com> wrote:

> On Tue, Mar 22, 2016 at 8:36 AM, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
>>
>> Hi,
>>
>> while using the dmatest module to test the mv_xor.c driver, I got
>> failures. I finally found that when the "noverify" parameter is not set,
>> then the buffer length used is totally random (modulo the maximum size
>> of the buffer). But in the mv_xor_prep_dma_xor there is a test about the
>> minimal size the dmaengine can use. So when the length was under this
>> minimum size then I got some test failures like the following: "dmatest:
>> dma1chan0-copy0: result #11: 'prep error' with src_off=0x29c9
>> dst_off=0x1c51 len=0x33 (0)".
>>
>> Unless the drivers are supposed to accept all buffer size I think it is
>> not an error, and I would like to be able to use this dmatest for
>> automatic test and not to have to check if the error is a real one or
>> not.
>>
>> I think that the following patch could improve the dmatest module.
>>
>> What do you think about it?
>>
>> Did I miss something or is a good ieda to send a proper patch?
>
> Looks like a useful patch to me.

Thanks for your prompt answer, so I am going to send patch on the ML to
have a real review.

Thanks,

Gregory
Gregory CLEMENT March 22, 2016, 5:55 p.m. UTC | #3
Hi,
 
 On mar., mars 22 2016, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote:

> Hi Dan,
>  
>  On mar., mars 22 2016, Dan Williams <dan.j.williams@intel.com> wrote:
>
>> On Tue, Mar 22, 2016 at 8:36 AM, Gregory CLEMENT
>> <gregory.clement@free-electrons.com> wrote:
>>>
>>> Hi,
>>>
>>> while using the dmatest module to test the mv_xor.c driver, I got
>>> failures. I finally found that when the "noverify" parameter is not set,
>>> then the buffer length used is totally random (modulo the maximum size
>>> of the buffer). But in the mv_xor_prep_dma_xor there is a test about the
>>> minimal size the dmaengine can use. So when the length was under this
>>> minimum size then I got some test failures like the following: "dmatest:
>>> dma1chan0-copy0: result #11: 'prep error' with src_off=0x29c9
>>> dst_off=0x1c51 len=0x33 (0)".
>>>
>>> Unless the drivers are supposed to accept all buffer size I think it is
>>> not an error, and I would like to be able to use this dmatest for
>>> automatic test and not to have to check if the error is a real one or
>>> not.
>>>
>>> I think that the following patch could improve the dmatest module.
>>>
>>> What do you think about it?
>>>
>>> Did I miss something or is a good ieda to send a proper patch?
>>
>> Looks like a useful patch to me.
>
> Thanks for your prompt answer, so I am going to send patch on the ML to
> have a real review.

In the meantime, my colleague Thomas Petazzoni found that it would make
more sens to make the framework aware of this limitation instead of
having to know the minimum size for each dma test. We could do it in the
same way that it was done for the alignment constraint.

What do you think of it?

Gregory

>
> Thanks,
>
> Gregory
>
> -- 
> Gregory Clement, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Williams March 22, 2016, 6:14 p.m. UTC | #4
On Tue, Mar 22, 2016 at 10:55 AM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> Hi,
>
>  On mar., mars 22 2016, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote:
>
>> Hi Dan,
>>
>>  On mar., mars 22 2016, Dan Williams <dan.j.williams@intel.com> wrote:
>>
>>> On Tue, Mar 22, 2016 at 8:36 AM, Gregory CLEMENT
>>> <gregory.clement@free-electrons.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> while using the dmatest module to test the mv_xor.c driver, I got
>>>> failures. I finally found that when the "noverify" parameter is not set,
>>>> then the buffer length used is totally random (modulo the maximum size
>>>> of the buffer). But in the mv_xor_prep_dma_xor there is a test about the
>>>> minimal size the dmaengine can use. So when the length was under this
>>>> minimum size then I got some test failures like the following: "dmatest:
>>>> dma1chan0-copy0: result #11: 'prep error' with src_off=0x29c9
>>>> dst_off=0x1c51 len=0x33 (0)".
>>>>
>>>> Unless the drivers are supposed to accept all buffer size I think it is
>>>> not an error, and I would like to be able to use this dmatest for
>>>> automatic test and not to have to check if the error is a real one or
>>>> not.
>>>>
>>>> I think that the following patch could improve the dmatest module.
>>>>
>>>> What do you think about it?
>>>>
>>>> Did I miss something or is a good ieda to send a proper patch?
>>>
>>> Looks like a useful patch to me.
>>
>> Thanks for your prompt answer, so I am going to send patch on the ML to
>> have a real review.
>
> In the meantime, my colleague Thomas Petazzoni found that it would make
> more sens to make the framework aware of this limitation instead of
> having to know the minimum size for each dma test. We could do it in the
> same way that it was done for the alignment constraint.
>
> What do you think of it?
>

I think we should just enforce the minimum alignment as the minimum
transfer size.  I don't think we have a need to support unaligned
transfer sizes.
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index b8576fd6bd0e..81b8cd893621 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -74,6 +74,10 @@  static bool verbose;
 module_param(verbose, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(verbose, "Enable \"success\" result messages (default: off)");
 
+static unsigned int min_size = 1;
+module_param(min_size, uint, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(min_size, "Minimal size of the buffer (default: 1)");
+
 /**
  * struct dmatest_params - test parameters.
  * @buf_size:          size of the memcpy test buffer
@@ -97,6 +101,7 @@  struct dmatest_params {
        unsigned int    pq_sources;
        int             timeout;
        bool            noverify;
+       unsigned int    min_size;
 };
 
 /**
@@ -505,7 +510,8 @@  static int dmatest_func(void *data)
                if (params->noverify)
                        len = params->buf_size;
                else
-                       len = dmatest_random() % params->buf_size + 1;
+                       len = dmatest_random() % (params->buf_size - params->min_size)
+                               + params->min_size;
 
                len = (len >> align) << align;
                if (!len)
@@ -864,6 +870,8 @@  static void run_threaded_test(struct dmatest_info *info)
        struct dmatest_params *params = &info->params;
 
        /* Copy test parameters */
+       if (test_buf_size < min_size)
+               test_buf_size = min_size;
        params->buf_size = test_buf_size;
        strlcpy(params->channel, strim(test_channel), sizeof(params->channel));
        strlcpy(params->device, strim(test_device), sizeof(params->device));
@@ -874,6 +882,7 @@  static void run_threaded_test(struct dmatest_info *info)
        params->pq_sources = pq_sources;
        params->timeout = timeout;
        params->noverify = noverify;
+       params->min_size = min_size;
 
        request_channels(info, DMA_MEMCPY);
        request_channels(info, DMA_XOR);