* [V2,3/5] dmaengine: dmatest: Add alignment parameter
From: Seraj Alijan @ 2018-08-31 11:03 UTC (permalink / raw)
To: vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake,
Seraj Alijan
Add parameter "alignment" to allow setting the address alignment
manually. Having the ability to configure address alignment from
user space adds new testing capabilities where different alignments can
be configured for testing without having to modify the dma device
alignment properties.
If configured, the alignment value will override the device alignment
property of the target device.
Signed-off-by: Seraj Alijan <seraj.alijan@sondrel.com>
---
drivers/dma/dmatest.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 3c55190..c980865 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -79,6 +79,10 @@ static bool verbose;
module_param(verbose, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(verbose, "Enable \"success\" result messages (default: off)");
+static int alignment = -1;
+module_param(alignment, int, 0644);
+MODULE_PARM_DESC(alignment, "Custom data address alignment taken as 2^(alignment) (default: not used (-1))");
+
/**
* struct dmatest_params - test parameters.
* @buf_size: size of the memcpy test buffer
@@ -103,6 +107,7 @@ struct dmatest_params {
int timeout;
bool noverify;
bool norandom;
+ int alignment;
};
/**
@@ -519,22 +524,26 @@ static int dmatest_func(void *data)
chan = thread->chan;
dev = chan->device;
if (thread->type == DMA_MEMCPY) {
- align = dev->copy_align;
+ align = params->alignment < 0 ? dev->copy_align :
+ params->alignment;
src_cnt = dst_cnt = 1;
} else if (thread->type == DMA_MEMSET) {
- align = dev->fill_align;
+ align = params->alignment < 0 ? dev->fill_align :
+ params->alignment;
src_cnt = dst_cnt = 1;
is_memset = true;
} else if (thread->type == DMA_XOR) {
/* force odd to ensure dst = src */
src_cnt = min_odd(params->xor_sources | 1, dev->max_xor);
dst_cnt = 1;
- align = dev->xor_align;
+ align = params->alignment < 0 ? dev->xor_align :
+ params->alignment;
} else if (thread->type == DMA_PQ) {
/* force odd to ensure dst = src */
src_cnt = min_odd(params->pq_sources | 1, dma_maxpq(dev, 0));
dst_cnt = 2;
- align = dev->pq_align;
+ align = params->alignment < 0 ? dev->pq_align :
+ params->alignment;
pq_coefs = kmalloc(params->pq_sources + 1, GFP_KERNEL);
if (!pq_coefs)
@@ -1032,6 +1041,7 @@ static void add_threaded_test(struct dmatest_info *info)
params->timeout = timeout;
params->noverify = noverify;
params->norandom = norandom;
+ params->alignment = alignment;
request_channels(info, DMA_MEMCPY);
request_channels(info, DMA_MEMSET);
^ permalink raw reply related
* [V2,4/5] dmaengine: dmatest: Add transfer_size parameter
From: Seraj Alijan @ 2018-08-31 11:03 UTC (permalink / raw)
To: vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake,
Seraj Alijan
Existing transfer size "len" is either generated randomly or set to the
size of test_buf_size. In some cases we need to explicitly specify a
transfer size that is different from the buffer size and non aligned to
test the target device's ability to handle unaligned transfers.
This patch adds optional parameter "transfer_size" to allow setting
explicit transfer size for dma transfers.
Signed-off-by: Seraj Alijan <seraj.alijan@sondrel.com>
---
drivers/dma/dmatest.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index c980865..937acb0 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -83,6 +83,10 @@ static int alignment = -1;
module_param(alignment, int, 0644);
MODULE_PARM_DESC(alignment, "Custom data address alignment taken as 2^(alignment) (default: not used (-1))");
+static unsigned int transfer_size;
+module_param(transfer_size, uint, 0644);
+MODULE_PARM_DESC(transfer_size, "Optional custom transfer size in bytes (default: not used (0))");
+
/**
* struct dmatest_params - test parameters.
* @buf_size: size of the memcpy test buffer
@@ -108,6 +112,7 @@ struct dmatest_params {
bool noverify;
bool norandom;
int alignment;
+ unsigned int transfer_size;
};
/**
@@ -636,15 +641,25 @@ static int dmatest_func(void *data)
break;
}
- if (params->norandom)
+ if (params->transfer_size) {
+ if (params->transfer_size >= params->buf_size) {
+ pr_err("%u-byte transfer size must be lower than %u-buffer size\n",
+ params->transfer_size, params->buf_size);
+ break;
+ }
+ len = params->transfer_size;
+ } else if (params->norandom) {
len = params->buf_size;
- else
+ } else {
len = dmatest_random() % params->buf_size + 1;
+ }
- len = (len >> align) << align;
- if (!len)
- len = 1 << align;
-
+ /* Do not alter transfer size explicitly defined by user */
+ if (!params->transfer_size) {
+ len = (len >> align) << align;
+ if (!len)
+ len = 1 << align;
+ }
total_len += len;
if (params->norandom) {
@@ -1042,6 +1057,7 @@ static void add_threaded_test(struct dmatest_info *info)
params->noverify = noverify;
params->norandom = norandom;
params->alignment = alignment;
+ params->transfer_size = transfer_size;
request_channels(info, DMA_MEMCPY);
request_channels(info, DMA_MEMSET);
^ permalink raw reply related
* [V2,5/5] dmaengine: Documentation: Add documentation for multi chan testing
From: Seraj Alijan @ 2018-08-31 11:03 UTC (permalink / raw)
To: vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake,
Seraj Alijan
Modify documentation to add multi channel testing support.
Signed-off-by: Seraj Alijan <seraj.alijan@sondrel.com>
---
Documentation/driver-api/dmaengine/dmatest.rst | 27 ++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/Documentation/driver-api/dmaengine/dmatest.rst b/Documentation/driver-api/dmaengine/dmatest.rst
index 7ce5e71..0a7e4c3 100644
--- a/Documentation/driver-api/dmaengine/dmatest.rst
+++ b/Documentation/driver-api/dmaengine/dmatest.rst
@@ -26,28 +26,43 @@ Part 2 - When dmatest is built as a module
Example of usage::
- % modprobe dmatest channel=dma0chan0 timeout=2000 iterations=1 run=1
+ % modprobe dmatest timeout=2000 iterations=1 channel=dma0chan0 run=1
...or::
% modprobe dmatest
- % echo dma0chan0 > /sys/module/dmatest/parameters/channel
% echo 2000 > /sys/module/dmatest/parameters/timeout
% echo 1 > /sys/module/dmatest/parameters/iterations
+ % echo dma0chan0 > /sys/module/dmatest/parameters/channel
% echo 1 > /sys/module/dmatest/parameters/run
...or on the kernel command line::
- dmatest.channel=dma0chan0 dmatest.timeout=2000 dmatest.iterations=1 dmatest.run=1
+ dmatest.timeout=2000 dmatest.iterations=1 dmatest.channel=dma0chan0 dmatest.run=1
+
+Example of multi-channel test usage:
+ % modprobe dmatest
+ % echo 2000 > /sys/module/dmatest/parameters/timeout
+ % echo 1 > /sys/module/dmatest/parameters/iterations
+ % echo dma0chan0 > /sys/module/dmatest/parameters/channel
+ % echo dma0chan1 > /sys/module/dmatest/parameters/channel
+ % echo dma0chan2 > /sys/module/dmatest/parameters/channel
+ % echo 1 > /sys/module/dmatest/parameters/run
+Note: the channel parameter should always be the last parameter set prior to
+running the test (setting run=1), this is because upon setting the channel
+parameter, that specific channel is requested using the dmaengine and a thread
+is created with the existing parameters. This thread is set as pending
+and will be executed once run is set to 1. Any parameters set after the thread
+is created are not applied.
.. hint::
available channel list could be extracted by running the following command::
% ls -1 /sys/class/dma/
-Once started a message like "dmatest: Started 1 threads using dma0chan0" is
-emitted. After that only test failure messages are reported until the test
-stops.
+Once started a message like " dmatest: Added 1 threads using dma0chan0" is
+emitted. A thread for that specific channel is created and is now pending, the
+pending thread is started once run is to 1.
Note that running a new test will not stop any in progress test.
^ permalink raw reply related
* [V2,5/5] dmaengine: Documentation: Add documentation for multi chan testing
From: Peter Ujfalusi @ 2018-08-31 11:17 UTC (permalink / raw)
To: Seraj Alijan, vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake
Hi,
On 2018-08-31 14:03, Seraj Alijan wrote:
> Modify documentation to add multi channel testing support.
>
> Signed-off-by: Seraj Alijan <seraj.alijan@sondrel.com>
> ---
> Documentation/driver-api/dmaengine/dmatest.rst | 27 ++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/driver-api/dmaengine/dmatest.rst b/Documentation/driver-api/dmaengine/dmatest.rst
> index 7ce5e71..0a7e4c3 100644
> --- a/Documentation/driver-api/dmaengine/dmatest.rst
> +++ b/Documentation/driver-api/dmaengine/dmatest.rst
> @@ -26,28 +26,43 @@ Part 2 - When dmatest is built as a module
>
> Example of usage::
>
> - % modprobe dmatest channel=dma0chan0 timeout=2000 iterations=1 run=1
> + % modprobe dmatest timeout=2000 iterations=1 channel=dma0chan0 run=1
>
> ...or::
>
> % modprobe dmatest
> - % echo dma0chan0 > /sys/module/dmatest/parameters/channel
> % echo 2000 > /sys/module/dmatest/parameters/timeout
> % echo 1 > /sys/module/dmatest/parameters/iterations
> + % echo dma0chan0 > /sys/module/dmatest/parameters/channel
> % echo 1 > /sys/module/dmatest/parameters/run
>
> ...or on the kernel command line::
>
> - dmatest.channel=dma0chan0 dmatest.timeout=2000 dmatest.iterations=1 dmatest.run=1
> + dmatest.timeout=2000 dmatest.iterations=1 dmatest.channel=dma0chan0 dmatest.run=1
> +
> +Example of multi-channel test usage:
> + % modprobe dmatest
> + % echo 2000 > /sys/module/dmatest/parameters/timeout
> + % echo 1 > /sys/module/dmatest/parameters/iterations
> + % echo dma0chan0 > /sys/module/dmatest/parameters/channel
> + % echo dma0chan1 > /sys/module/dmatest/parameters/channel
> + % echo dma0chan2 > /sys/module/dmatest/parameters/channel
Can I still use:
echo "" > /sys/module/dmatest/parameters/channel
to stop dmatest to run on the specific channel(s) or do I need to reboot
the machine to do so?
I normally test multi channel memcpy with:
echo 8000000 > /sys/module/dmatest/parameters/test_buf_size
echo 2000 > /sys/module/dmatest/parameters/timeout
echo 20 > /sys/module/dmatest/parameters/iterations
echo 20 > /sys/module/dmatest/parameters/max_channels
echo 1 > /sys/module/dmatest/parameters/run
So it will start running memtest on 20 channels.
If I do a test on specific channel (different DMA controller) I set the
channel run the test and then with "" I can revert back to normal
torture test.
> + % echo 1 > /sys/module/dmatest/parameters/run
>
> +Note: the channel parameter should always be the last parameter set prior to
> +running the test (setting run=1), this is because upon setting the channel
> +parameter, that specific channel is requested using the dmaengine and a thread
> +is created with the existing parameters. This thread is set as pending
> +and will be executed once run is set to 1. Any parameters set after the thread
> +is created are not applied.
> .. hint::
> available channel list could be extracted by running the following command::
>
> % ls -1 /sys/class/dma/
>
> -Once started a message like "dmatest: Started 1 threads using dma0chan0" is
> -emitted. After that only test failure messages are reported until the test
> -stops.
> +Once started a message like " dmatest: Added 1 threads using dma0chan0" is
> +emitted. A thread for that specific channel is created and is now pending, the
> +pending thread is started once run is to 1.
>
> Note that running a new test will not stop any in progress test.
>
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* [V2,5/5] dmaengine: Documentation: Add documentation for multi chan testing
From: Peter Ujfalusi @ 2018-08-31 11:29 UTC (permalink / raw)
To: Seraj Alijan, vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake
Hi,
On 2018-08-31 14:03, Seraj Alijan wrote:
> Modify documentation to add multi channel testing support.
>
> Signed-off-by: Seraj Alijan <seraj.alijan@sondrel.com>
> ---
> Documentation/driver-api/dmaengine/dmatest.rst | 27 ++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/driver-api/dmaengine/dmatest.rst b/Documentation/driver-api/dmaengine/dmatest.rst
> index 7ce5e71..0a7e4c3 100644
> --- a/Documentation/driver-api/dmaengine/dmatest.rst
> +++ b/Documentation/driver-api/dmaengine/dmatest.rst
> @@ -26,28 +26,43 @@ Part 2 - When dmatest is built as a module
>
> Example of usage::
>
> - % modprobe dmatest channel=dma0chan0 timeout=2000 iterations=1 run=1
> + % modprobe dmatest timeout=2000 iterations=1 channel=dma0chan0 run=1
>
> ...or::
>
> % modprobe dmatest
> - % echo dma0chan0 > /sys/module/dmatest/parameters/channel
> % echo 2000 > /sys/module/dmatest/parameters/timeout
> % echo 1 > /sys/module/dmatest/parameters/iterations
> + % echo dma0chan0 > /sys/module/dmatest/parameters/channel
> % echo 1 > /sys/module/dmatest/parameters/run
>
> ...or on the kernel command line::
>
> - dmatest.channel=dma0chan0 dmatest.timeout=2000 dmatest.iterations=1 dmatest.run=1
> + dmatest.timeout=2000 dmatest.iterations=1 dmatest.channel=dma0chan0 dmatest.run=1
> +
> +Example of multi-channel test usage:
> + % modprobe dmatest
> + % echo 2000 > /sys/module/dmatest/parameters/timeout
> + % echo 1 > /sys/module/dmatest/parameters/iterations
> + % echo dma0chan0 > /sys/module/dmatest/parameters/channel
> + % echo dma0chan1 > /sys/module/dmatest/parameters/channel
> + % echo dma0chan2 > /sys/module/dmatest/parameters/channel
> + % echo 1 > /sys/module/dmatest/parameters/run
>
> +Note: the channel parameter should always be the last parameter set prior to
> +running the test (setting run=1), this is because upon setting the channel
> +parameter, that specific channel is requested using the dmaengine and a thread
> +is created with the existing parameters.
When the channel is going to be released after this change?
It is a bit awkward currently that after the test finished the
channel(s) are still requested and it can be released by
cat /sys/module/dmatest/parameters/run
or to run the test again and before the next run the channel(s) are
going to be released just to be requested again.
> This thread is set as pending
> +and will be executed once run is set to 1. Any parameters set after the thread
> +is created are not applied.
> .. hint::
> available channel list could be extracted by running the following command::
>
> % ls -1 /sys/class/dma/
>
> -Once started a message like "dmatest: Started 1 threads using dma0chan0" is
> -emitted. After that only test failure messages are reported until the test
> -stops.
> +Once started a message like " dmatest: Added 1 threads using dma0chan0" is
> +emitted. A thread for that specific channel is created and is now pending, the
> +pending thread is started once run is to 1.
>
> Note that running a new test will not stop any in progress test.
>
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* [4/6] dt-bindings: dmaengine: usb-dmac: Add binding for r8a774a1
From: Rob Herring @ 2018-08-31 12:20 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Vinod Koul, Mark Rutland, Biju Das, Greg Kroah-Hartman, dmaengine,
devicetree, Simon Horman, Geert Uytterhoeven, Chris Paterson,
linux-renesas-soc, linux-kernel
On Fri, 24 Aug 2018 08:56:13 +0100, Fabrizio Castro wrote:
> From: Biju Das <biju.das@bp.renesas.com>
>
> This patch adds binding for r8a774a1 (RZ/G2M).
>
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> Reviewed-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
> Documentation/devicetree/bindings/dma/renesas,usb-dmac.txt | 1 +
> 1 file changed, 1 insertion(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [V2,5/5] dmaengine: Documentation: Add documentation for multi chan testing
From: Seraj Mohammed @ 2018-08-31 13:41 UTC (permalink / raw)
To: Peter Ujfalusi, Seraj Alijan, vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake
Hello,
On 31/08/18 12:17, Peter Ujfalusi wrote:
> Hi,
>
> On 2018-08-31 14:03, Seraj Alijan wrote:
>> Modify documentation to add multi channel testing support.
>>
>> Signed-off-by: Seraj Alijan <seraj.alijan@sondrel.com>
>> ---
>> Documentation/driver-api/dmaengine/dmatest.rst | 27 ++++++++++++++++++++------
>> 1 file changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/driver-api/dmaengine/dmatest.rst b/Documentation/driver-api/dmaengine/dmatest.rst
>> index 7ce5e71..0a7e4c3 100644
>> --- a/Documentation/driver-api/dmaengine/dmatest.rst
>> +++ b/Documentation/driver-api/dmaengine/dmatest.rst
>> @@ -26,28 +26,43 @@ Part 2 - When dmatest is built as a module
>>
>> Example of usage::
>>
>> - % modprobe dmatest channel=dma0chan0 timeout=2000 iterations=1 run=1
>> + % modprobe dmatest timeout=2000 iterations=1 channel=dma0chan0 run=1
>>
>> ...or::
>>
>> % modprobe dmatest
>> - % echo dma0chan0 > /sys/module/dmatest/parameters/channel
>> % echo 2000 > /sys/module/dmatest/parameters/timeout
>> % echo 1 > /sys/module/dmatest/parameters/iterations
>> + % echo dma0chan0 > /sys/module/dmatest/parameters/channel
>> % echo 1 > /sys/module/dmatest/parameters/run
>>
>> ...or on the kernel command line::
>>
>> - dmatest.channel=dma0chan0 dmatest.timeout=2000 dmatest.iterations=1 dmatest.run=1
>> + dmatest.timeout=2000 dmatest.iterations=1 dmatest.channel=dma0chan0 dmatest.run=1
>> +
>> +Example of multi-channel test usage:
>> + % modprobe dmatest
>> + % echo 2000 > /sys/module/dmatest/parameters/timeout
>> + % echo 1 > /sys/module/dmatest/parameters/iterations
>> + % echo dma0chan0 > /sys/module/dmatest/parameters/channel
>> + % echo dma0chan1 > /sys/module/dmatest/parameters/channel
>> + % echo dma0chan2 > /sys/module/dmatest/parameters/channel
>
> Can I still use:
> echo "" > /sys/module/dmatest/parameters/channel
>
> to stop dmatest to run on the specific channel(s) or do I need to reboot
> the machine to do so?
is this behavior documented somewhere? or did you discover this from your
experience with using the module?
The short answer to your question is no, with this change you cannot stop
a test by echoing an empty string to the channel parameter, you will either
have to run the test, or reboot the machine.
Setting the channel parameter to an empty string will request all
channels up to "max_channels" parameter value. This is because with this
change, editing the channel parameter will invoke the process of allocating
a thread for the specified channel and storing that thread in a thread
list, after that, setting run to 1 will simply iterate through the thread
list and start all pending threads.
> I normally test multi channel memcpy with:
> echo 8000000 > /sys/module/dmatest/parameters/test_buf_size
> echo 2000 > /sys/module/dmatest/parameters/timeout
> echo 20 > /sys/module/dmatest/parameters/iterations
> echo 20 > /sys/module/dmatest/parameters/max_channels
> echo 1 > /sys/module/dmatest/parameters/run
>
> So it will start running memtest on 20 channels.
>
> If I do a test on specific channel (different DMA controller) I set the
> channel run the test and then with "" I can revert back to normal
> torture test.
Yes, but the problem is that you either run a test on all channels, or
a single channel. This change makes it possible to specify a list of
channels to test, for example we can now launch a test on channels 0,3,5,6
by doing
echo dma0chan0 > /sys/module/dmatest/parameters/channel
echo dma0chan3 > /sys/module/dmatest/parameters/channel
echo dma0chan5 > /sys/module/dmatest/parameters/channel
echo dma0chan6 > /sys/module/dmatest/parameters/channel
echo 1 > /sys/module/dmatest/parameters/run
as far as I'm aware this functionality was not possible before.
You can still run your torture test with a simple modification:
echo 8000000 > /sys/module/dmatest/parameters/test_buf_size
echo 2000 > /sys/module/dmatest/parameters/timeout
echo 20 > /sys/module/dmatest/parameters/iterations
echo 20 > /sys/module/dmatest/parameters/max_channels
echo "" > /sys/module/dmatest/parameters/channel << allocates 20 channels
echo 1 > /sys/module/dmatest/parameters/run
>> + % echo 1 > /sys/module/dmatest/parameters/run
>>
>> +Note: the channel parameter should always be the last parameter set prior to
>> +running the test (setting run=1), this is because upon setting the channel
>> +parameter, that specific channel is requested using the dmaengine and a thread
>> +is created with the existing parameters. This thread is set as pending
>> +and will be executed once run is set to 1. Any parameters set after the thread
>> +is created are not applied.
>> .. hint::
>> available channel list could be extracted by running the following command::
>>
>> % ls -1 /sys/class/dma/
>>
>> -Once started a message like "dmatest: Started 1 threads using dma0chan0" is
>> -emitted. After that only test failure messages are reported until the test
>> -stops.
>> +Once started a message like " dmatest: Added 1 threads using dma0chan0" is
>> +emitted. A thread for that specific channel is created and is now pending, the
>> +pending thread is started once run is to 1.
>>
>> Note that running a new test will not stop any in progress test.
>>
>>
>
> - Péter
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
- Seraj
^ permalink raw reply
* [V2,5/5] dmaengine: Documentation: Add documentation for multi chan testing
From: Peter Ujfalusi @ 2018-08-31 14:02 UTC (permalink / raw)
To: Seraj Mohammed, Seraj Alijan, vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake
Hi Seraj,
On 2018-08-31 16:41, Seraj Mohammed wrote:
> Hello,
>
> On 31/08/18 12:17, Peter Ujfalusi wrote:
>> Hi,
>>
>> On 2018-08-31 14:03, Seraj Alijan wrote:
>>> Modify documentation to add multi channel testing support.
>>>
>>> Signed-off-by: Seraj Alijan <seraj.alijan@sondrel.com>
>>> ---
>>> Documentation/driver-api/dmaengine/dmatest.rst | 27 ++++++++++++++++++++------
>>> 1 file changed, 21 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/driver-api/dmaengine/dmatest.rst b/Documentation/driver-api/dmaengine/dmatest.rst
>>> index 7ce5e71..0a7e4c3 100644
>>> --- a/Documentation/driver-api/dmaengine/dmatest.rst
>>> +++ b/Documentation/driver-api/dmaengine/dmatest.rst
>>> @@ -26,28 +26,43 @@ Part 2 - When dmatest is built as a module
>>>
>>> Example of usage::
>>>
>>> - % modprobe dmatest channel=dma0chan0 timeout=2000 iterations=1 run=1
>>> + % modprobe dmatest timeout=2000 iterations=1 channel=dma0chan0 run=1
>>>
>>> ...or::
>>>
>>> % modprobe dmatest
>>> - % echo dma0chan0 > /sys/module/dmatest/parameters/channel
>>> % echo 2000 > /sys/module/dmatest/parameters/timeout
>>> % echo 1 > /sys/module/dmatest/parameters/iterations
>>> + % echo dma0chan0 > /sys/module/dmatest/parameters/channel
>>> % echo 1 > /sys/module/dmatest/parameters/run
>>>
>>> ...or on the kernel command line::
>>>
>>> - dmatest.channel=dma0chan0 dmatest.timeout=2000 dmatest.iterations=1 dmatest.run=1
>>> + dmatest.timeout=2000 dmatest.iterations=1 dmatest.channel=dma0chan0 dmatest.run=1
>>> +
>>> +Example of multi-channel test usage:
>>> + % modprobe dmatest
>>> + % echo 2000 > /sys/module/dmatest/parameters/timeout
>>> + % echo 1 > /sys/module/dmatest/parameters/iterations
>>> + % echo dma0chan0 > /sys/module/dmatest/parameters/channel
>>> + % echo dma0chan1 > /sys/module/dmatest/parameters/channel
>>> + % echo dma0chan2 > /sys/module/dmatest/parameters/channel
>>
>> Can I still use:
>> echo "" > /sys/module/dmatest/parameters/channel
>>
>> to stop dmatest to run on the specific channel(s) or do I need to reboot
>> the machine to do so?
>
> is this behavior documented somewhere? or did you discover this from your
> experience with using the module?
> The short answer to your question is no, with this change you cannot stop
> a test by echoing an empty string to the channel parameter, you will either
> have to run the test, or reboot the machine.
>
> Setting the channel parameter to an empty string will request all
> channels up to "max_channels" parameter value. This is because with this
> change, editing the channel parameter will invoke the process of allocating
> a thread for the specified channel and storing that thread in a thread
> list, after that, setting run to 1 will simply iterate through the thread
> list and start all pending threads.
>
>> I normally test multi channel memcpy with:
>> echo 8000000 > /sys/module/dmatest/parameters/test_buf_size
>> echo 2000 > /sys/module/dmatest/parameters/timeout
>> echo 20 > /sys/module/dmatest/parameters/iterations
>> echo 20 > /sys/module/dmatest/parameters/max_channels
>> echo 1 > /sys/module/dmatest/parameters/run
>>
>> So it will start running memtest on 20 channels.
>>
>> If I do a test on specific channel (different DMA controller) I set the
>> channel run the test and then with "" I can revert back to normal
>> torture test.
>
> Yes, but the problem is that you either run a test on all channels, or
> a single channel. This change makes it possible to specify a list of
> channels to test, for example we can now launch a test on channels 0,3,5,6
> by doing
>
> echo dma0chan0 > /sys/module/dmatest/parameters/channel
> echo dma0chan3 > /sys/module/dmatest/parameters/channel
> echo dma0chan5 > /sys/module/dmatest/parameters/channel
> echo dma0chan6 > /sys/module/dmatest/parameters/channel
> echo 1 > /sys/module/dmatest/parameters/run
>
> as far as I'm aware this functionality was not possible before.
True and it is something which would be nice to have. I could construct
single test run for channels on different DMA controller. That is pretty
cool.
However, if I got it right I need to reboot the machine if I want to
change any of the parameters for a re-run.
Let's say I want to run tests on 6 channel with 50K buffers, 100K 1M, 10M.
I can not do this without rebooting, right?
Would not be possible to:
echo "dma0chan0 dma0chan3 dma0chan5 dma0chan6" >
/sys/module/dmatest/parameters/channel
and use the same logic already existing for max_channels to start off
the tests on different channels?
> You can still run your torture test with a simple modification:
>
> echo 8000000 > /sys/module/dmatest/parameters/test_buf_size
> echo 2000 > /sys/module/dmatest/parameters/timeout
> echo 20 > /sys/module/dmatest/parameters/iterations
> echo 20 > /sys/module/dmatest/parameters/max_channels
> echo "" > /sys/module/dmatest/parameters/channel << allocates 20 channels
> echo 1 > /sys/module/dmatest/parameters/run
Sure, but if I test something on a specific channels I need to reboot.
Or if I want to change parameters for a next run...
>
>>> + % echo 1 > /sys/module/dmatest/parameters/run
>>>
>>> +Note: the channel parameter should always be the last parameter set prior to
>>> +running the test (setting run=1), this is because upon setting the channel
>>> +parameter, that specific channel is requested using the dmaengine and a thread
>>> +is created with the existing parameters. This thread is set as pending
>>> +and will be executed once run is set to 1. Any parameters set after the thread
>>> +is created are not applied.
>>> .. hint::
>>> available channel list could be extracted by running the following command::
>>>
>>> % ls -1 /sys/class/dma/
>>>
>>> -Once started a message like "dmatest: Started 1 threads using dma0chan0" is
>>> -emitted. After that only test failure messages are reported until the test
>>> -stops.
>>> +Once started a message like " dmatest: Added 1 threads using dma0chan0" is
>>> +emitted. A thread for that specific channel is created and is now pending, the
>>> +pending thread is started once run is to 1.
>>>
>>> Note that running a new test will not stop any in progress test.
>>>
>>>
>>
>> - Péter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>
>
> - Seraj
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* [V2,5/5] dmaengine: Documentation: Add documentation for multi chan testing
From: Peter Ujfalusi @ 2018-08-31 14:22 UTC (permalink / raw)
To: Seraj Mohammed, Seraj Alijan, vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake
Hi,
On 2018-08-31 16:41, Seraj Mohammed wrote:
>> Can I still use:
>> echo "" > /sys/module/dmatest/parameters/channel
>>
>> to stop dmatest to run on the specific channel(s) or do I need to reboot
>> the machine to do so?
>
> is this behavior documented somewhere? or did you discover this from your
> experience with using the module?
If the channels is "" then it is not set. We can change the channels
currently and there is no difference between selecting a channel or
selecting none.
> The short answer to your question is no, with this change you cannot stop
> a test by echoing an empty string to the channel parameter, you will either
> have to run the test, or reboot the machine.
>
> Setting the channel parameter to an empty string will request all
> channels up to "max_channels" parameter value.
Yes, and it is very useful. What would be even more useful is if via the
channel I could select the starting channel from where the max_channels
will start...
> This is because with this
> change, editing the channel parameter will invoke the process of allocating
> a thread for the specified channel and storing that thread in a thread
> list, after that, setting run to 1 will simply iterate through the thread
> list and start all pending threads.
We have automated regression tests via dmatest's memcpy, it does changes
parameters between runs to gather the data for different transfer sizes,
runs in selected channels and runs also with different max_channels.
It is a regression if we need to reboot between every test to change
parameters, channels, etc.
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* [V2,5/5] dmaengine: Documentation: Add documentation for multi chan testing
From: Seraj Mohammed @ 2018-08-31 15:01 UTC (permalink / raw)
To: Peter Ujfalusi, Seraj Alijan, vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake
On 31/08/18 15:02, Peter Ujfalusi wrote:
> Hi Seraj,
>
> On 2018-08-31 16:41, Seraj Mohammed wrote:
>> Hello,
>>
>> On 31/08/18 12:17, Peter Ujfalusi wrote:
>>> Hi,
>>>
>>> On 2018-08-31 14:03, Seraj Alijan wrote:
>>>> Modify documentation to add multi channel testing support.
>>>>
>>>> Signed-off-by: Seraj Alijan <seraj.alijan@sondrel.com>
>>>> ---
>>>> Documentation/driver-api/dmaengine/dmatest.rst | 27 ++++++++++++++++++++------
>>>> 1 file changed, 21 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/driver-api/dmaengine/dmatest.rst b/Documentation/driver-api/dmaengine/dmatest.rst
>>>> index 7ce5e71..0a7e4c3 100644
>>>> --- a/Documentation/driver-api/dmaengine/dmatest.rst
>>>> +++ b/Documentation/driver-api/dmaengine/dmatest.rst
>>>> @@ -26,28 +26,43 @@ Part 2 - When dmatest is built as a module
>>>>
>>>> Example of usage::
>>>>
>>>> - % modprobe dmatest channel=dma0chan0 timeout=2000 iterations=1 run=1
>>>> + % modprobe dmatest timeout=2000 iterations=1 channel=dma0chan0 run=1
>>>>
>>>> ...or::
>>>>
>>>> % modprobe dmatest
>>>> - % echo dma0chan0 > /sys/module/dmatest/parameters/channel
>>>> % echo 2000 > /sys/module/dmatest/parameters/timeout
>>>> % echo 1 > /sys/module/dmatest/parameters/iterations
>>>> + % echo dma0chan0 > /sys/module/dmatest/parameters/channel
>>>> % echo 1 > /sys/module/dmatest/parameters/run
>>>>
>>>> ...or on the kernel command line::
>>>>
>>>> - dmatest.channel=dma0chan0 dmatest.timeout=2000 dmatest.iterations=1 dmatest.run=1
>>>> + dmatest.timeout=2000 dmatest.iterations=1 dmatest.channel=dma0chan0 dmatest.run=1
>>>> +
>>>> +Example of multi-channel test usage:
>>>> + % modprobe dmatest
>>>> + % echo 2000 > /sys/module/dmatest/parameters/timeout
>>>> + % echo 1 > /sys/module/dmatest/parameters/iterations
>>>> + % echo dma0chan0 > /sys/module/dmatest/parameters/channel
>>>> + % echo dma0chan1 > /sys/module/dmatest/parameters/channel
>>>> + % echo dma0chan2 > /sys/module/dmatest/parameters/channel
>>>
>>> Can I still use:
>>> echo "" > /sys/module/dmatest/parameters/channel
>>>
>>> to stop dmatest to run on the specific channel(s) or do I need to reboot
>>> the machine to do so?
>>
>> is this behavior documented somewhere? or did you discover this from your
>> experience with using the module?
>> The short answer to your question is no, with this change you cannot stop
>> a test by echoing an empty string to the channel parameter, you will either
>> have to run the test, or reboot the machine.
>>
>> Setting the channel parameter to an empty string will request all
>> channels up to "max_channels" parameter value. This is because with this
>> change, editing the channel parameter will invoke the process of allocating
>> a thread for the specified channel and storing that thread in a thread
>> list, after that, setting run to 1 will simply iterate through the thread
>> list and start all pending threads.
>>
>>> I normally test multi channel memcpy with:
>>> echo 8000000 > /sys/module/dmatest/parameters/test_buf_size
>>> echo 2000 > /sys/module/dmatest/parameters/timeout
>>> echo 20 > /sys/module/dmatest/parameters/iterations
>>> echo 20 > /sys/module/dmatest/parameters/max_channels
>>> echo 1 > /sys/module/dmatest/parameters/run
>>>
>>> So it will start running memtest on 20 channels.
>>>
>>> If I do a test on specific channel (different DMA controller) I set the
>>> channel run the test and then with "" I can revert back to normal
>>> torture test.
>>
>> Yes, but the problem is that you either run a test on all channels, or
>> a single channel. This change makes it possible to specify a list of
>> channels to test, for example we can now launch a test on channels 0,3,5,6
>> by doing
>>
>> echo dma0chan0 > /sys/module/dmatest/parameters/channel
>> echo dma0chan3 > /sys/module/dmatest/parameters/channel
>> echo dma0chan5 > /sys/module/dmatest/parameters/channel
>> echo dma0chan6 > /sys/module/dmatest/parameters/channel
>> echo 1 > /sys/module/dmatest/parameters/run
>>
>> as far as I'm aware this functionality was not possible before.
>
> True and it is something which would be nice to have. I could construct
> single test run for channels on different DMA controller. That is pretty
> cool.
>
> However, if I got it right I need to reboot the machine if I want to
> change any of the parameters for a re-run.
> Let's say I want to run tests on 6 channel with 50K buffers, 100K 1M, 10M.
> I can not do this without rebooting, right?
You only need to reboot the machine if you want to change paameters after
allocating the channels.
Example:
echo 4096 > /sys/module/dmatest/parameters/test_buf_size
echo 1 > /sys/module/dmatest/parameters/iterations
echo 6 > /sys/module/dmatest/parameters/alignment
echo 0 > /sys/module/dmatest/parameters/noverify
echo 0 > /sys/module/dmatest/parameters/norandom
echo 1 > /sys/module/dmatest/parameters/threads_per_chan
echo 20000 > /sys/module/dmatest/parameters/timeout
At the is point, you can still change any of the parameters above without
rebooting
echo dma0chan0 > /sys/module/dmatest/parameters/channel
[ 31.465240] dmatest: Added 1 threads using dma0chan0
echo dma0chan1 > /sys/module/dmatest/parameters/channel
[ 33.142020] dmatest: Added 1 threads using dma0chan1
echo dma0chan2 > /sys/module/dmatest/parameters/channel
[ 34.402123] dmatest: Added 1 threads using dma0chan2
As you can see now channels 0,1, 2 have already been allocated and their
respective threads are pending. At this point if you want to change
any parameters you have to reboot, because the channels have already been
requested with the properties set above.
echo 1 > /sys/module/dmatest/parameters/run
dmatest: dma0chan0-copy0: summary 1 tests, 0 failures 285.71 iops 571
KB/s (0)
dmatest: dma0chan2-copy0: summary 1 tests, 0 failures 142.85 iops 285
KB/s (0)
dmatest: dma0chan1-copy0: summary 1 tests, 0 failures 142.85 iops 428
KB/s (0)
At this point the test is finished and you can run a new test with
completely different parameters without having to reboot.
> Would not be possible to:
>
> echo "dma0chan0 dma0chan3 dma0chan5 dma0chan6" >
> /sys/module/dmatest/parameters/channel
>
> and use the same logic already existing for max_channels to start off
> the tests on different channels?
It is possible, but i wouldn't make it possible to start all threads at
once, which is what i want to do. If i do it this way, the test would have
have extract the channels one by one and allocate and start them
sequentially. My patch ensures that when I set run to 1, all channels fire
off at the same time, or as close as possible without having the overhead
of allocating channels inbetween transfers.
>> You can still run your torture test with a simple modification:
>>
>> echo 8000000 > /sys/module/dmatest/parameters/test_buf_size
>> echo 2000 > /sys/module/dmatest/parameters/timeout
>> echo 20 > /sys/module/dmatest/parameters/iterations
>> echo 20 > /sys/module/dmatest/parameters/max_channels
>> echo "" > /sys/module/dmatest/parameters/channel << allocates 20 channels
>> echo 1 > /sys/module/dmatest/parameters/run
>
> Sure, but if I test something on a specific channels I need to reboot.
> Or if I want to change parameters for a next run...
>
>>
>>>> + % echo 1 > /sys/module/dmatest/parameters/run
>>>>
>>>> +Note: the channel parameter should always be the last parameter set prior to
>>>> +running the test (setting run=1), this is because upon setting the channel
>>>> +parameter, that specific channel is requested using the dmaengine and a thread
>>>> +is created with the existing parameters. This thread is set as pending
>>>> +and will be executed once run is set to 1. Any parameters set after the thread
>>>> +is created are not applied.
>>>> .. hint::
>>>> available channel list could be extracted by running the following command::
>>>>
>>>> % ls -1 /sys/class/dma/
>>>>
>>>> -Once started a message like "dmatest: Started 1 threads using dma0chan0" is
>>>> -emitted. After that only test failure messages are reported until the test
>>>> -stops.
>>>> +Once started a message like " dmatest: Added 1 threads using dma0chan0" is
>>>> +emitted. A thread for that specific channel is created and is now pending, the
>>>> +pending thread is started once run is to 1.
>>>>
>>>> Note that running a new test will not stop any in progress test.
>>>>
>>>>
>>>
>>> - Péter
>>>
>>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>>
>>
>> - Seraj
>>
>
> - Péter
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
^ permalink raw reply
* [V2,5/5] dmaengine: Documentation: Add documentation for multi chan testing
From: Seraj Mohammed @ 2018-08-31 15:42 UTC (permalink / raw)
To: Peter Ujfalusi, Seraj Alijan, vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake
On 31/08/18 12:29, Peter Ujfalusi wrote:
> Hi,
>
> On 2018-08-31 14:03, Seraj Alijan wrote:
>> Modify documentation to add multi channel testing support.
>>
>> Signed-off-by: Seraj Alijan <seraj.alijan@sondrel.com>
>> ---
>> Documentation/driver-api/dmaengine/dmatest.rst | 27 ++++++++++++++++++++------
>> 1 file changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/driver-api/dmaengine/dmatest.rst b/Documentation/driver-api/dmaengine/dmatest.rst
>> index 7ce5e71..0a7e4c3 100644
>> --- a/Documentation/driver-api/dmaengine/dmatest.rst
>> +++ b/Documentation/driver-api/dmaengine/dmatest.rst
>> @@ -26,28 +26,43 @@ Part 2 - When dmatest is built as a module
>>
>> Example of usage::
>>
>> - % modprobe dmatest channel=dma0chan0 timeout=2000 iterations=1 run=1
>> + % modprobe dmatest timeout=2000 iterations=1 channel=dma0chan0 run=1
>>
>> ...or::
>>
>> % modprobe dmatest
>> - % echo dma0chan0 > /sys/module/dmatest/parameters/channel
>> % echo 2000 > /sys/module/dmatest/parameters/timeout
>> % echo 1 > /sys/module/dmatest/parameters/iterations
>> + % echo dma0chan0 > /sys/module/dmatest/parameters/channel
>> % echo 1 > /sys/module/dmatest/parameters/run
>>
>> ...or on the kernel command line::
>>
>> - dmatest.channel=dma0chan0 dmatest.timeout=2000 dmatest.iterations=1 dmatest.run=1
>> + dmatest.timeout=2000 dmatest.iterations=1 dmatest.channel=dma0chan0 dmatest.run=1
>> +
>> +Example of multi-channel test usage:
>> + % modprobe dmatest
>> + % echo 2000 > /sys/module/dmatest/parameters/timeout
>> + % echo 1 > /sys/module/dmatest/parameters/iterations
>> + % echo dma0chan0 > /sys/module/dmatest/parameters/channel
>> + % echo dma0chan1 > /sys/module/dmatest/parameters/channel
>> + % echo dma0chan2 > /sys/module/dmatest/parameters/channel
>> + % echo 1 > /sys/module/dmatest/parameters/run
>>
>> +Note: the channel parameter should always be the last parameter set prior to
>> +running the test (setting run=1), this is because upon setting the channel
>> +parameter, that specific channel is requested using the dmaengine and a thread
>> +is created with the existing parameters.
>
> When the channel is going to be released after this change?
>
> It is a bit awkward currently that after the test finished the
> channel(s) are still requested and it can be released by
> cat /sys/module/dmatest/parameters/run
This patch does not tackle this issue, after the test is run these
channels remain requested, you can still clear them the same way.
> or to run the test again and before the next run the channel(s) are
> going to be released just to be requested again.
With this change, after you run your test, the channels remain requested
but their respective threads are no longer considered pending. So when
you issue:
echo 1 > /sys/module/dmatest/parameters/run
The test will check if it has any pending threads, if it sees that the
thread list is populated, but non of the threads are pending, it will
clear them all. You would have to requested the channels again before
attempting to start another run.
This same behavior is applied when you request a channel after running a
test, it will clear all previous threads before requesting new channels.
example scenario 1: issuing run command after completing test
echo 2000 > /sys/module/dmatest/parameters/timeout
echo 1 > /sys/module/dmatest/parameters/iterations
echo dma0chan0 > /sys/module/dmatest/parameters/channel
[ 1295.534205] dmatest: Added 1 threads using dma0chan0
echo dma0chan1 > /sys/module/dmatest/parameters/channel
[ 1295.543496] dmatest: Added 1 threads using dma0chan1
echo dma0chan2 > /sys/module/dmatest/parameters/channel
[ 1295.549113] dmatest: Added 1 threads using dma0chan2
echo 1 > /sys/module/dmatest/parameters/run
[ 1296.436301] dmatest: dma0chan1-copy0: summary 1 tests, 0 failures
400.00 iops 0 KB/s (0)
[ 1296.441081] dmatest: dma0chan2-copy0: summary 1 tests, 0 failures
181.81 iops 363 KB/s (0)
[ 1296.441590] dmatest: dma0chan0-copy0: summary 1 tests, 0 failures
222.22 iops 666 KB/s (0)
echo 1 > /sys/module/dmatest/parameters/run
As you can see, issuing the run command again does nothing, it simply
checked that it had no pending threads, so it cleared all channels.
example scenarios 2: requesting channel after completing test
echo dma0chan0 > /sys/module/dmatest/parameters/channel
[ 1394.279145] dmatest: Added 1 threads using dma0chan0
echo dma0chan1 > /sys/module/dmatest/parameters/channel
[ 1395.877829] dmatest: Added 1 threads using dma0chan1
echo dma0chan2 > /sys/module/dmatest/parameters/channel
[ 1396.776893] dmatest: Added 1 threads using dma0chan2
echo 1 > /sys/module/dmatest/parameters/run
[ 1400.798124] dmatest: dma0chan1-copy0: summary 1 tests, 0 failures
345.33 iops 758 KB/s (0)
[ 1400.799443] dmatest: dma0chan0-copy0: summary 1 tests, 0 failures
333.33 iops 1000 KB/s (0)
[ 1400.799688] dmatest: dma0chan2-copy0: summary 1 tests, 0 failures
333.33 iops 666 KB/s (0)
echo dma0chan1 > /sys/module/dmatest/parameters/channel
[ 1405.373603] dmatest: Added 1 threads using dma0chan1
echo 1 > /sys/module/dmatest/parameters/run
[ 1406.512495] dmatest: dma0chan1-copy0: summary 1 tests, 0 failures
200.00 iops 400 KB/s (0)
In this example you can see that you can request a new channel after
completing a test, the test has automatically identified that it has a
list of non pending threads from the previous run, it cleared them, and
allocated the new channel.
>> This thread is set as pending
>> +and will be executed once run is set to 1. Any parameters set after the thread
>> +is created are not applied.
>> .. hint::
>> available channel list could be extracted by running the following command::
>>
>> % ls -1 /sys/class/dma/
>>
>> -Once started a message like "dmatest: Started 1 threads using dma0chan0" is
>> -emitted. After that only test failure messages are reported until the test
>> -stops.
>> +Once started a message like " dmatest: Added 1 threads using dma0chan0" is
>> +emitted. A thread for that specific channel is created and is now pending, the
>> +pending thread is started once run is to 1.
>>
>> Note that running a new test will not stop any in progress test.
>>
>>
>
> - Péter
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
^ permalink raw reply
* [V2,5/5] dmaengine: Documentation: Add documentation for multi chan testing
From: Seraj Mohammed @ 2018-08-31 15:48 UTC (permalink / raw)
To: Peter Ujfalusi, Seraj Alijan, vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake
Hi,
On 31/08/18 15:22, Peter Ujfalusi wrote:
> Hi,
>
> On 2018-08-31 16:41, Seraj Mohammed wrote:
>>> Can I still use:
>>> echo "" > /sys/module/dmatest/parameters/channel
>>>
>>> to stop dmatest to run on the specific channel(s) or do I need to reboot
>>> the machine to do so?
>>
>> is this behavior documented somewhere? or did you discover this from your
>> experience with using the module?
>
> If the channels is "" then it is not set. We can change the channels
> currently and there is no difference between selecting a channel or
> selecting none.
>
>> The short answer to your question is no, with this change you cannot stop
>> a test by echoing an empty string to the channel parameter, you will either
>> have to run the test, or reboot the machine.
>>
>> Setting the channel parameter to an empty string will request all
>> channels up to "max_channels" parameter value.
>
> Yes, and it is very useful. What would be even more useful is if via the
> channel I could select the starting channel from where the max_channels
> will start...
Since this patch already allows you to specifically select which
channels to run on, i don't see the benefit in allocating channels using
ranges.
>> This is because with this
>> change, editing the channel parameter will invoke the process of allocating
>> a thread for the specified channel and storing that thread in a thread
>> list, after that, setting run to 1 will simply iterate through the thread
>> list and start all pending threads.
>
> We have automated regression tests via dmatest's memcpy, it does changes
> parameters between runs to gather the data for different transfer sizes,
> runs in selected channels and runs also with different max_channels.
>
> It is a regression if we need to reboot between every test to change
> parameters, channels, etc.
Once you have successfully completed a test, you don't need to reboot to
start a new one with different parameters.
> - Péter
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
^ permalink raw reply
* [1/3] arm64: dts: actions: s900: Enable Tx DMA for UART5
From: Manivannan Sadhasivam @ 2018-09-01 16:42 UTC (permalink / raw)
To: vkoul, dan.j.williams, afaerber, robh+dt, gregkh, jslaby
Cc: linux-serial, dmaengine, liuwei, 96boards, devicetree,
daniel.thompson, amit.kucheria, linux-arm-kernel, linux-kernel,
hzhang, bdong, manivannanece23, thomas.liau, jeff.chen, pn,
edgar.righi, Manivannan Sadhasivam
Enable Tx DMA for UART5 in Actions Semi S900 SoC.
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
arch/arm64/boot/dts/actions/s900.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/actions/s900.dtsi b/arch/arm64/boot/dts/actions/s900.dtsi
index eceba914762c..39af1236f611 100644
--- a/arch/arm64/boot/dts/actions/s900.dtsi
+++ b/arch/arm64/boot/dts/actions/s900.dtsi
@@ -156,6 +156,8 @@
compatible = "actions,s900-uart", "actions,owl-uart";
reg = <0x0 0xe012a000 0x0 0x2000>;
interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
+ dma-names = "tx";
+ dmas = <&dma 26>;
status = "disabled";
};
^ permalink raw reply related
* [2/3] dmaengine: Add Slave and Cyclic mode support for Actions Semi Owl S900 SoC
From: Manivannan Sadhasivam @ 2018-09-01 16:42 UTC (permalink / raw)
To: vkoul, dan.j.williams, afaerber, robh+dt, gregkh, jslaby
Cc: linux-serial, dmaengine, liuwei, 96boards, devicetree,
daniel.thompson, amit.kucheria, linux-arm-kernel, linux-kernel,
hzhang, bdong, manivannanece23, thomas.liau, jeff.chen, pn,
edgar.righi, Manivannan Sadhasivam
Add Slave and Cyclic mode support for Actions Semi Owl S900 SoC. The slave
mode supports bus width of 4 bytes common for all peripherals and 1 byte
specific for UART.
The cyclic mode supports only block mode transfer.
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
drivers/dma/owl-dma.c | 273 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 266 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/owl-dma.c b/drivers/dma/owl-dma.c
index 7812a6338acd..7f7b3e76bcf7 100644
--- a/drivers/dma/owl-dma.c
+++ b/drivers/dma/owl-dma.c
@@ -21,6 +21,7 @@
#include <linux/mm.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/of_dma.h>
#include <linux/slab.h>
#include "virt-dma.h"
@@ -165,6 +166,7 @@ struct owl_dma_lli {
struct owl_dma_txd {
struct virt_dma_desc vd;
struct list_head lli_list;
+ bool cyclic;
};
/**
@@ -191,6 +193,8 @@ struct owl_dma_vchan {
struct virt_dma_chan vc;
struct owl_dma_pchan *pchan;
struct owl_dma_txd *txd;
+ struct dma_slave_config cfg;
+ u8 drq;
};
/**
@@ -336,9 +340,11 @@ static struct owl_dma_lli *owl_dma_alloc_lli(struct owl_dma *od)
static struct owl_dma_lli *owl_dma_add_lli(struct owl_dma_txd *txd,
struct owl_dma_lli *prev,
- struct owl_dma_lli *next)
+ struct owl_dma_lli *next,
+ bool is_cyclic)
{
- list_add_tail(&next->node, &txd->lli_list);
+ if (!is_cyclic)
+ list_add_tail(&next->node, &txd->lli_list);
if (prev) {
prev->hw.next_lli = next->phys;
@@ -351,7 +357,9 @@ static struct owl_dma_lli *owl_dma_add_lli(struct owl_dma_txd *txd,
static inline int owl_dma_cfg_lli(struct owl_dma_vchan *vchan,
struct owl_dma_lli *lli,
dma_addr_t src, dma_addr_t dst,
- u32 len, enum dma_transfer_direction dir)
+ u32 len, enum dma_transfer_direction dir,
+ struct dma_slave_config *sconfig,
+ bool is_cyclic)
{
struct owl_dma_lli_hw *hw = &lli->hw;
u32 mode;
@@ -364,6 +372,26 @@ static inline int owl_dma_cfg_lli(struct owl_dma_vchan *vchan,
OWL_DMA_MODE_DT_DCU | OWL_DMA_MODE_SAM_INC |
OWL_DMA_MODE_DAM_INC;
+ break;
+ case DMA_MEM_TO_DEV:
+ mode |= OWL_DMA_MODE_TS(vchan->drq)
+ | OWL_DMA_MODE_ST_DCU | OWL_DMA_MODE_DT_DEV
+ | OWL_DMA_MODE_SAM_INC | OWL_DMA_MODE_DAM_CONST;
+
+ /* Handle bus width for UART */
+ if (sconfig->dst_addr_width == DMA_SLAVE_BUSWIDTH_1_BYTE)
+ mode |= OWL_DMA_MODE_NDDBW_8BIT;
+
+ break;
+ case DMA_DEV_TO_MEM:
+ mode |= OWL_DMA_MODE_TS(vchan->drq)
+ | OWL_DMA_MODE_ST_DEV | OWL_DMA_MODE_DT_DCU
+ | OWL_DMA_MODE_SAM_CONST | OWL_DMA_MODE_DAM_INC;
+
+ /* Handle bus width for UART */
+ if (sconfig->src_addr_width == DMA_SLAVE_BUSWIDTH_1_BYTE)
+ mode |= OWL_DMA_MODE_NDDBW_8BIT;
+
break;
default:
return -EINVAL;
@@ -381,7 +409,10 @@ static inline int owl_dma_cfg_lli(struct owl_dma_vchan *vchan,
OWL_DMA_LLC_SAV_LOAD_NEXT |
OWL_DMA_LLC_DAV_LOAD_NEXT);
- hw->ctrlb = llc_hw_ctrlb(OWL_DMA_INTCTL_SUPER_BLOCK);
+ if (is_cyclic)
+ hw->ctrlb = llc_hw_ctrlb(OWL_DMA_INTCTL_BLOCK);
+ else
+ hw->ctrlb = llc_hw_ctrlb(OWL_DMA_INTCTL_SUPER_BLOCK);
return 0;
}
@@ -443,6 +474,16 @@ static void owl_dma_terminate_pchan(struct owl_dma *od,
spin_unlock_irqrestore(&od->lock, flags);
}
+static void owl_dma_pause_pchan(struct owl_dma_pchan *pchan)
+{
+ pchan_writel(pchan, 1, OWL_DMAX_PAUSE);
+}
+
+static void owl_dma_resume_pchan(struct owl_dma_pchan *pchan)
+{
+ pchan_writel(pchan, 0, OWL_DMAX_PAUSE);
+}
+
static int owl_dma_start_next_txd(struct owl_dma_vchan *vchan)
{
struct owl_dma *od = to_owl_dma(vchan->vc.chan.device);
@@ -464,7 +505,10 @@ static int owl_dma_start_next_txd(struct owl_dma_vchan *vchan)
lli = list_first_entry(&txd->lli_list,
struct owl_dma_lli, node);
- int_ctl = OWL_DMA_INTCTL_SUPER_BLOCK;
+ if (txd->cyclic)
+ int_ctl = OWL_DMA_INTCTL_BLOCK;
+ else
+ int_ctl = OWL_DMA_INTCTL_SUPER_BLOCK;
pchan_writel(pchan, OWL_DMAX_MODE, OWL_DMA_MODE_LME);
pchan_writel(pchan, OWL_DMAX_LINKLIST_CTL,
@@ -627,6 +671,54 @@ static int owl_dma_terminate_all(struct dma_chan *chan)
return 0;
}
+static int owl_dma_config(struct dma_chan *chan,
+ struct dma_slave_config *config)
+{
+ struct owl_dma_vchan *vchan = to_owl_vchan(chan);
+
+ /* Reject definitely invalid configurations */
+ if (config->src_addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES ||
+ config->dst_addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES)
+ return -EINVAL;
+
+ memcpy(&vchan->cfg, config, sizeof(struct dma_slave_config));
+
+ return 0;
+}
+
+static int owl_dma_pause(struct dma_chan *chan)
+{
+ struct owl_dma_vchan *vchan = to_owl_vchan(chan);
+ unsigned long flags;
+
+ spin_lock_irqsave(&vchan->vc.lock, flags);
+
+ owl_dma_pause_pchan(vchan->pchan);
+
+ spin_unlock_irqrestore(&vchan->vc.lock, flags);
+
+ return 0;
+}
+
+static int owl_dma_resume(struct dma_chan *chan)
+{
+ struct owl_dma_vchan *vchan = to_owl_vchan(chan);
+ unsigned long flags;
+
+ if (!vchan->pchan && !vchan->txd)
+ return 0;
+
+ dev_dbg(chan2dev(chan), "vchan %p: resume\n", &vchan->vc);
+
+ spin_lock_irqsave(&vchan->vc.lock, flags);
+
+ owl_dma_resume_pchan(vchan->pchan);
+
+ spin_unlock_irqrestore(&vchan->vc.lock, flags);
+
+ return 0;
+}
+
static u32 owl_dma_getbytes_chan(struct owl_dma_vchan *vchan)
{
struct owl_dma_pchan *pchan;
@@ -754,13 +846,14 @@ static struct dma_async_tx_descriptor
bytes = min_t(size_t, (len - offset), OWL_DMA_FRAME_MAX_LENGTH);
ret = owl_dma_cfg_lli(vchan, lli, src + offset, dst + offset,
- bytes, DMA_MEM_TO_MEM);
+ bytes, DMA_MEM_TO_MEM,
+ &vchan->cfg, txd->cyclic);
if (ret) {
dev_warn(chan2dev(chan), "failed to config lli\n");
goto err_txd_free;
}
- prev = owl_dma_add_lli(txd, prev, lli);
+ prev = owl_dma_add_lli(txd, prev, lli, false);
}
return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
@@ -770,6 +863,133 @@ static struct dma_async_tx_descriptor
return NULL;
}
+static struct dma_async_tx_descriptor
+ *owl_dma_prep_slave_sg(struct dma_chan *chan,
+ struct scatterlist *sgl,
+ unsigned int sg_len,
+ enum dma_transfer_direction dir,
+ unsigned long flags, void *context)
+{
+ struct owl_dma *od = to_owl_dma(chan->device);
+ struct owl_dma_vchan *vchan = to_owl_vchan(chan);
+ struct dma_slave_config *sconfig = &vchan->cfg;
+ struct owl_dma_txd *txd;
+ struct owl_dma_lli *lli, *prev = NULL;
+ struct scatterlist *sg;
+ dma_addr_t addr, src = 0, dst = 0;
+ size_t len;
+ int ret, i;
+
+ txd = kzalloc(sizeof(*txd), GFP_NOWAIT);
+ if (!txd)
+ return NULL;
+
+ INIT_LIST_HEAD(&txd->lli_list);
+
+ for_each_sg(sgl, sg, sg_len, i) {
+ addr = sg_dma_address(sg);
+ len = sg_dma_len(sg);
+
+ if (len > OWL_DMA_FRAME_MAX_LENGTH) {
+ dev_err(od->dma.dev,
+ "frame length exceeds max supported length");
+ goto err_txd_free;
+ }
+
+ lli = owl_dma_alloc_lli(od);
+ if (!lli) {
+ dev_err(chan2dev(chan), "failed to allocate lli");
+ goto err_txd_free;
+ }
+
+ if (dir == DMA_MEM_TO_DEV) {
+ src = addr;
+ dst = sconfig->dst_addr;
+ } else {
+ src = sconfig->src_addr;
+ dst = addr;
+ }
+
+ ret = owl_dma_cfg_lli(vchan, lli, src, dst, len, dir, sconfig,
+ txd->cyclic);
+ if (ret) {
+ dev_warn(chan2dev(chan), "failed to config lli");
+ goto err_txd_free;
+ }
+
+ prev = owl_dma_add_lli(txd, prev, lli, false);
+ }
+
+ return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
+
+err_txd_free:
+ owl_dma_free_txd(od, txd);
+
+ return NULL;
+}
+
+static struct dma_async_tx_descriptor
+ *owl_prep_dma_cyclic(struct dma_chan *chan,
+ dma_addr_t buf_addr, size_t buf_len,
+ size_t period_len,
+ enum dma_transfer_direction dir,
+ unsigned long flags)
+{
+ struct owl_dma *od = to_owl_dma(chan->device);
+ struct owl_dma_vchan *vchan = to_owl_vchan(chan);
+ struct dma_slave_config *sconfig = &vchan->cfg;
+ struct owl_dma_txd *txd;
+ struct owl_dma_lli *lli, *prev = NULL, *first = NULL;
+ dma_addr_t src = 0, dst = 0;
+ unsigned int periods = buf_len / period_len;
+ int ret, i;
+
+ txd = kzalloc(sizeof(*txd), GFP_NOWAIT);
+ if (!txd)
+ return NULL;
+
+ INIT_LIST_HEAD(&txd->lli_list);
+ txd->cyclic = true;
+
+ for (i = 0; i < periods; i++) {
+ lli = owl_dma_alloc_lli(od);
+ if (!lli) {
+ dev_warn(chan2dev(chan), "failed to allocate lli");
+ goto err_txd_free;
+ }
+
+ if (dir == DMA_MEM_TO_DEV) {
+ src = buf_addr + (period_len * i);
+ dst = sconfig->dst_addr;
+ } else if (dir == DMA_DEV_TO_MEM) {
+ src = sconfig->src_addr;
+ dst = buf_addr + (period_len * i);
+ }
+
+ ret = owl_dma_cfg_lli(vchan, lli, src, dst, period_len,
+ dir, sconfig, txd->cyclic);
+ if (ret) {
+ dev_warn(chan2dev(chan), "failed to config lli");
+ goto err_txd_free;
+ }
+
+ if (!first)
+ first = lli;
+
+ prev = owl_dma_add_lli(txd, prev, lli, false);
+ }
+
+ /* close the cyclic list */
+ owl_dma_add_lli(txd, prev, first, true);
+
+ return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
+
+err_txd_free:
+ owl_dma_free_txd(od, txd);
+
+ return NULL;
+}
+
static void owl_dma_free_chan_resources(struct dma_chan *chan)
{
struct owl_dma_vchan *vchan = to_owl_vchan(chan);
@@ -790,6 +1010,27 @@ static inline void owl_dma_free(struct owl_dma *od)
}
}
+static struct dma_chan *owl_dma_of_xlate(struct of_phandle_args *dma_spec,
+ struct of_dma *ofdma)
+{
+ struct owl_dma *od = ofdma->of_dma_data;
+ struct owl_dma_vchan *vchan;
+ struct dma_chan *chan;
+ u8 drq = dma_spec->args[0];
+
+ if (drq > od->nr_vchans)
+ return NULL;
+
+ chan = dma_get_any_slave_channel(&od->dma);
+ if (!chan)
+ return NULL;
+
+ vchan = to_owl_vchan(chan);
+ vchan->drq = drq;
+
+ return chan;
+}
+
static int owl_dma_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -833,12 +1074,19 @@ static int owl_dma_probe(struct platform_device *pdev)
spin_lock_init(&od->lock);
dma_cap_set(DMA_MEMCPY, od->dma.cap_mask);
+ dma_cap_set(DMA_SLAVE, od->dma.cap_mask);
+ dma_cap_set(DMA_CYCLIC, od->dma.cap_mask);
od->dma.dev = &pdev->dev;
od->dma.device_free_chan_resources = owl_dma_free_chan_resources;
od->dma.device_tx_status = owl_dma_tx_status;
od->dma.device_issue_pending = owl_dma_issue_pending;
od->dma.device_prep_dma_memcpy = owl_dma_prep_memcpy;
+ od->dma.device_prep_slave_sg = owl_dma_prep_slave_sg;
+ od->dma.device_prep_dma_cyclic = owl_prep_dma_cyclic;
+ od->dma.device_config = owl_dma_config;
+ od->dma.device_pause = owl_dma_pause;
+ od->dma.device_resume = owl_dma_resume;
od->dma.device_terminate_all = owl_dma_terminate_all;
od->dma.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
od->dma.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
@@ -910,8 +1158,18 @@ static int owl_dma_probe(struct platform_device *pdev)
goto err_pool_free;
}
+ /* Device-tree DMA controller registration */
+ ret = of_dma_controller_register(pdev->dev.of_node,
+ owl_dma_of_xlate, od);
+ if (ret) {
+ dev_err(&pdev->dev, "of_dma_controller_register failed\n");
+ goto err_dma_unregister;
+ }
+
return 0;
+err_dma_unregister:
+ dma_async_device_unregister(&od->dma);
err_pool_free:
clk_disable_unprepare(od->clk);
dma_pool_destroy(od->lli_pool);
@@ -923,6 +1181,7 @@ static int owl_dma_remove(struct platform_device *pdev)
{
struct owl_dma *od = platform_get_drvdata(pdev);
+ of_dma_controller_free(pdev->dev.of_node);
dma_async_device_unregister(&od->dma);
/* Mask all interrupts for this execution environment */
^ permalink raw reply related
* [3/3] tty: serial: Add Tx DMA support for UART in Actions Semi Owl SoCs
From: Manivannan Sadhasivam @ 2018-09-01 16:42 UTC (permalink / raw)
To: vkoul, dan.j.williams, afaerber, robh+dt, gregkh, jslaby
Cc: linux-serial, dmaengine, liuwei, 96boards, devicetree,
daniel.thompson, amit.kucheria, linux-arm-kernel, linux-kernel,
hzhang, bdong, manivannanece23, thomas.liau, jeff.chen, pn,
edgar.righi, Manivannan Sadhasivam
Add Tx DMA support for Actions Semi Owl SoCs. If there is no DMA
property specified in DT, it will fallback to default interrupt mode.
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
drivers/tty/serial/owl-uart.c | 172 +++++++++++++++++++++++++++++++++-
1 file changed, 171 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c
index 29a6dc6a8d23..1b3016db7ae2 100644
--- a/drivers/tty/serial/owl-uart.c
+++ b/drivers/tty/serial/owl-uart.c
@@ -11,6 +11,8 @@
#include <linux/clk.h>
#include <linux/console.h>
#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of.h>
@@ -48,6 +50,8 @@
#define OWL_UART_CTL_RXIE BIT(18)
#define OWL_UART_CTL_TXIE BIT(19)
#define OWL_UART_CTL_LBEN BIT(20)
+#define OWL_UART_CTL_DRCR BIT(21)
+#define OWL_UART_CTL_DTCR BIT(22)
#define OWL_UART_STAT_RIP BIT(0)
#define OWL_UART_STAT_TIP BIT(1)
@@ -71,12 +75,21 @@ struct owl_uart_info {
struct owl_uart_port {
struct uart_port port;
struct clk *clk;
+
+ struct dma_chan *tx_ch;
+ dma_addr_t tx_dma_buf;
+ dma_cookie_t dma_tx_cookie;
+ u32 tx_size;
+ bool tx_dma;
+ bool dma_tx_running;
};
#define to_owl_uart_port(prt) container_of(prt, struct owl_uart_port, prt)
static struct owl_uart_port *owl_uart_ports[OWL_UART_PORT_NUM];
+static void owl_uart_dma_start_tx(struct owl_uart_port *owl_port);
+
static inline void owl_uart_write(struct uart_port *port, u32 val, unsigned int off)
{
writel(val, port->membase + off);
@@ -115,6 +128,83 @@ static unsigned int owl_uart_get_mctrl(struct uart_port *port)
return mctrl;
}
+static void owl_uart_dma_tx_callback(void *data)
+{
+ struct owl_uart_port *owl_port = data;
+ struct uart_port *port = &owl_port->port;
+ struct circ_buf *xmit = &port->state->xmit;
+ unsigned long flags;
+ u32 val;
+
+ dma_sync_single_for_cpu(port->dev, owl_port->tx_dma_buf,
+ UART_XMIT_SIZE, DMA_TO_DEVICE);
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ owl_port->dma_tx_running = 0;
+
+ xmit->tail += owl_port->tx_size;
+ xmit->tail &= UART_XMIT_SIZE - 1;
+ port->icount.tx += owl_port->tx_size;
+
+ if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
+
+ /* Disable Tx DRQ */
+ val = owl_uart_read(port, OWL_UART_CTL);
+ val &= ~OWL_UART_CTL_TXDE;
+ owl_uart_write(port, val, OWL_UART_CTL);
+
+ /* Clear pending Tx IRQ */
+ val = owl_uart_read(port, OWL_UART_STAT);
+ val |= OWL_UART_STAT_TIP;
+ owl_uart_write(port, val, OWL_UART_STAT);
+
+ if (!uart_circ_empty(xmit) && !uart_tx_stopped(port))
+ owl_uart_dma_start_tx(owl_port);
+
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static void owl_uart_dma_start_tx(struct owl_uart_port *owl_port)
+{
+ struct uart_port *port = &owl_port->port;
+ struct circ_buf *xmit = &port->state->xmit;
+ struct dma_async_tx_descriptor *desc;
+ u32 val;
+
+ if (uart_tx_stopped(port) || uart_circ_empty(xmit) ||
+ owl_port->dma_tx_running)
+ return;
+
+ dma_sync_single_for_device(port->dev, owl_port->tx_dma_buf,
+ UART_XMIT_SIZE, DMA_TO_DEVICE);
+
+ owl_port->tx_size = CIRC_CNT_TO_END(xmit->head, xmit->tail,
+ UART_XMIT_SIZE);
+
+ desc = dmaengine_prep_slave_single(owl_port->tx_ch,
+ owl_port->tx_dma_buf + xmit->tail,
+ owl_port->tx_size, DMA_MEM_TO_DEV,
+ DMA_PREP_INTERRUPT);
+ if (!desc)
+ return;
+
+ desc->callback = owl_uart_dma_tx_callback;
+ desc->callback_param = owl_port;
+
+ /* Enable Tx DRQ */
+ val = owl_uart_read(port, OWL_UART_CTL);
+ val &= ~OWL_UART_CTL_TXIE;
+ val |= OWL_UART_CTL_TXDE | OWL_UART_CTL_DTCR;
+ owl_uart_write(port, val, OWL_UART_CTL);
+
+ /* Start Tx DMA transfer */
+ owl_port->dma_tx_running = true;
+ owl_port->dma_tx_cookie = dmaengine_submit(desc);
+ dma_async_issue_pending(owl_port->tx_ch);
+}
+
static unsigned int owl_uart_tx_empty(struct uart_port *port)
{
unsigned long flags;
@@ -159,6 +249,7 @@ static void owl_uart_stop_tx(struct uart_port *port)
static void owl_uart_start_tx(struct uart_port *port)
{
+ struct owl_uart_port *owl_port = to_owl_uart_port(port);
u32 val;
if (uart_tx_stopped(port)) {
@@ -166,6 +257,11 @@ static void owl_uart_start_tx(struct uart_port *port)
return;
}
+ if (owl_port->tx_dma) {
+ owl_uart_dma_start_tx(owl_port);
+ return;
+ }
+
val = owl_uart_read(port, OWL_UART_STAT);
val |= OWL_UART_STAT_TIP;
owl_uart_write(port, val, OWL_UART_STAT);
@@ -273,13 +369,27 @@ static irqreturn_t owl_uart_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static void owl_dma_channel_free(struct owl_uart_port *owl_port)
+{
+ dmaengine_terminate_all(owl_port->tx_ch);
+ dma_release_channel(owl_port->tx_ch);
+ dma_unmap_single(owl_port->port.dev, owl_port->tx_dma_buf,
+ UART_XMIT_SIZE, DMA_TO_DEVICE);
+ owl_port->dma_tx_running = false;
+ owl_port->tx_ch = NULL;
+}
+
static void owl_uart_shutdown(struct uart_port *port)
{
- u32 val;
+ struct owl_uart_port *owl_port = to_owl_uart_port(port);
unsigned long flags;
+ u32 val;
spin_lock_irqsave(&port->lock, flags);
+ if (owl_port->tx_dma)
+ owl_dma_channel_free(owl_port);
+
val = owl_uart_read(port, OWL_UART_CTL);
val &= ~(OWL_UART_CTL_TXIE | OWL_UART_CTL_RXIE
| OWL_UART_CTL_TXDE | OWL_UART_CTL_RXDE | OWL_UART_CTL_EN);
@@ -290,6 +400,62 @@ static void owl_uart_shutdown(struct uart_port *port)
free_irq(port->irq, port);
}
+static int owl_uart_dma_tx_init(struct uart_port *port)
+{
+ struct owl_uart_port *owl_port = to_owl_uart_port(port);
+ struct device *dev = port->dev;
+ struct dma_slave_config slave_config;
+ int ret;
+
+ owl_port->tx_dma = false;
+
+ /* Request DMA TX channel */
+ owl_port->tx_ch = dma_request_slave_channel(dev, "tx");
+ if (!owl_port->tx_ch) {
+ dev_info(dev, "tx dma alloc failed\n");
+ return -ENODEV;
+ }
+
+ owl_port->tx_dma_buf = dma_map_single(dev,
+ owl_port->port.state->xmit.buf,
+ UART_XMIT_SIZE, DMA_TO_DEVICE);
+ if (dma_mapping_error(dev, owl_port->tx_dma_buf)) {
+ ret = -ENOMEM;
+ goto alloc_err;
+ }
+
+ /* Configure DMA channel */
+ memset(&slave_config, 0, sizeof(slave_config));
+ slave_config.direction = DMA_MEM_TO_DEV;
+ slave_config.dst_addr = port->mapbase + OWL_UART_TXDAT;
+ slave_config.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+
+ ret = dmaengine_slave_config(owl_port->tx_ch, &slave_config);
+ if (ret < 0) {
+ dev_err(dev, "tx dma channel config failed\n");
+ ret = -ENODEV;
+ goto map_err;
+ }
+
+ /* Use DMA buffer size as the FIFO size */
+ port->fifosize = UART_XMIT_SIZE;
+
+ /* Set DMA flag */
+ owl_port->tx_dma = true;
+ owl_port->dma_tx_running = false;
+
+ return 0;
+
+map_err:
+ dma_unmap_single(dev, owl_port->tx_dma_buf, UART_XMIT_SIZE,
+ DMA_TO_DEVICE);
+alloc_err:
+ dma_release_channel(owl_port->tx_ch);
+ owl_port->tx_ch = NULL;
+
+ return ret;
+}
+
static int owl_uart_startup(struct uart_port *port)
{
u32 val;
@@ -301,6 +467,10 @@ static int owl_uart_startup(struct uart_port *port)
if (ret)
return ret;
+ ret = owl_uart_dma_tx_init(port);
+ if (!ret)
+ dev_info(port->dev, "using DMA for tx\n");
+
spin_lock_irqsave(&port->lock, flags);
val = owl_uart_read(port, OWL_UART_STAT);
^ permalink raw reply related
* [V2,5/5] dmaengine: Documentation: Add documentation for multi chan testing
From: Peter Ujfalusi @ 2018-09-03 8:19 UTC (permalink / raw)
To: Seraj Mohammed, Seraj Alijan, vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake
Hi,
On 2018-08-31 18:01, Seraj Mohammed wrote:
>> However, if I got it right I need to reboot the machine if I want to
>> change any of the parameters for a re-run.
>> Let's say I want to run tests on 6 channel with 50K buffers, 100K 1M, 10M.
>> I can not do this without rebooting, right?
>
> You only need to reboot the machine if you want to change paameters after
> allocating the channels.
Instead of reboot running the test is fine to clear things up.
> Example:
>
> echo 4096 > /sys/module/dmatest/parameters/test_buf_size
> echo 1 > /sys/module/dmatest/parameters/iterations
> echo 6 > /sys/module/dmatest/parameters/alignment
> echo 0 > /sys/module/dmatest/parameters/noverify
> echo 0 > /sys/module/dmatest/parameters/norandom
> echo 1 > /sys/module/dmatest/parameters/threads_per_chan
> echo 20000 > /sys/module/dmatest/parameters/timeout
>
> At the is point, you can still change any of the parameters above without
> rebooting
But I can not do:
echo 8000000 > /sys/module/dmatest/parameters/test_buf_size
echo 2000 > /sys/module/dmatest/parameters/timeout
echo 20 > /sys/module/dmatest/parameters/iterations
echo 20 > /sys/module/dmatest/parameters/max_channels
echo 1 > /sys/module/dmatest/parameters/run
it will _not_ start 20 iteration tests on 20 channels.
>
> echo dma0chan0 > /sys/module/dmatest/parameters/channel
> [ 31.465240] dmatest: Added 1 threads using dma0chan0
> echo dma0chan1 > /sys/module/dmatest/parameters/channel
> [ 33.142020] dmatest: Added 1 threads using dma0chan1
> echo dma0chan2 > /sys/module/dmatest/parameters/channel
> [ 34.402123] dmatest: Added 1 threads using dma0chan2
I need to:
echo dma0chan10 > /sys/module/dmatest/parameters/channel
cat /sys/module/dmatest/parameters/channel
dma0chan10
echo dma0chan11 > /sys/module/dmatest/parameters/channel
cat /sys/module/dmatest/parameters/channel
dma0chan11
^ it tells me that only dma0chan11 is in channels while in fact I have
dma0chan10 and dma0chan11 selected.
then I run the test:
echo 1 > /sys/module/dmatest/parameters/run
[ 514.370959] dmatest: dma0chan10-copy: summary 1 tests, 0 failures
7692 iops 15384 KB/s (0)
[ 514.371099] dmatest: dma0chan11-copy: summary 1 tests, 0 failures
11111 iops 22222 KB/s (0)
After this:
cat /sys/module/dmatest/parameters/channel returns nothing, which I
believe is correct. So I could be able to run the 20 iteration on 20
channels, but I can not:
echo 20 > /sys/module/dmatest/parameters/iterations
echo 20 > /sys/module/dmatest/parameters/max_channels
echo 1 > /sys/module/dmatest/parameters/run
The patch effectively breaks the batch mode via max_channels.
> As you can see now channels 0,1, 2 have already been allocated and their
> respective threads are pending. At this point if you want to change
> any parameters you have to reboot, because the channels have already been
> requested with the properties set above.
>
> echo 1 > /sys/module/dmatest/parameters/run
> dmatest: dma0chan0-copy0: summary 1 tests, 0 failures 285.71 iops 571
> KB/s (0)
> dmatest: dma0chan2-copy0: summary 1 tests, 0 failures 142.85 iops 285
> KB/s (0)
> dmatest: dma0chan1-copy0: summary 1 tests, 0 failures 142.85 iops 428
> KB/s (0)
>
> At this point the test is finished and you can run a new test with
> completely different parameters without having to reboot.
That's fine.
If I want to just repeat the same test then I would put the same
channels back in a loop and run it again to run them overnight for example.
But it is still unclear how the channels will be released..
cat /sys/class/dma/dma0chan10/in_use
0
echo dma0chan10 > /sys/module/dmatest/parameters/channel
cat /sys/class/dma/dma0chan10/in_use
1
echo 1 > /sys/module/dmatest/parameters/run
[ 690.326039] dmatest: dma0chan10-copy: summary 20 tests, 0 failures 71
iops 290511 KB/s (0)
cat /sys/class/dma/dma0chan10/in_use
1
echo 1 > /sys/module/dmatest/parameters/run
# will not run any tests
cat /sys/class/dma/dma0chan10/in_use
1
echo dma0chan12 > /sys/module/dmatest/parameters/channel
cat /sys/class/dma/dma0chan10/in_use
0
cat /sys/class/dma/dma0chan12/in_use
1
So it appears that the channel is going to be released when a new
channel is added, right?
Which brought up some interesting thing:
echo "" > /sys/module/dmatest/parameters/channel
Will add the 20 channels to the test :o
One thing would be really nice is to print something when we hit run and
the dmatest actually going to do something or if the dmatest is not
configured properly and will not going to run any test.
>
>> Would not be possible to:
>>
>> echo "dma0chan0 dma0chan3 dma0chan5 dma0chan6" >
>> /sys/module/dmatest/parameters/channel
>>
>> and use the same logic already existing for max_channels to start off
>> the tests on different channels?
>
> It is possible, but i wouldn't make it possible to start all threads at
> once, which is what i want to do. If i do it this way, the test would have
> have extract the channels one by one and allocate and start them
> sequentially. My patch ensures that when I set run to 1, all channels fire
> off at the same time, or as close as possible without having the overhead
> of allocating channels inbetween transfers.
>
>>> You can still run your torture test with a simple modification:
>>>
>>> echo 8000000 > /sys/module/dmatest/parameters/test_buf_size
>>> echo 2000 > /sys/module/dmatest/parameters/timeout
>>> echo 20 > /sys/module/dmatest/parameters/iterations
>>> echo 20 > /sys/module/dmatest/parameters/max_channels
>>> echo "" > /sys/module/dmatest/parameters/channel << allocates 20 channels
>>> echo 1 > /sys/module/dmatest/parameters/run
>>
>> Sure, but if I test something on a specific channels I need to reboot.
>> Or if I want to change parameters for a next run...
>>
>>>
>>>>> + % echo 1 > /sys/module/dmatest/parameters/run
>>>>>
>>>>> +Note: the channel parameter should always be the last parameter set prior to
>>>>> +running the test (setting run=1), this is because upon setting the channel
>>>>> +parameter, that specific channel is requested using the dmaengine and a thread
>>>>> +is created with the existing parameters. This thread is set as pending
>>>>> +and will be executed once run is set to 1. Any parameters set after the thread
>>>>> +is created are not applied.
>>>>> .. hint::
>>>>> available channel list could be extracted by running the following command::
>>>>>
>>>>> % ls -1 /sys/class/dma/
>>>>>
>>>>> -Once started a message like "dmatest: Started 1 threads using dma0chan0" is
>>>>> -emitted. After that only test failure messages are reported until the test
>>>>> -stops.
>>>>> +Once started a message like " dmatest: Added 1 threads using dma0chan0" is
>>>>> +emitted. A thread for that specific channel is created and is now pending, the
>>>>> +pending thread is started once run is to 1.
>>>>>
>>>>> Note that running a new test will not stop any in progress test.
>>>>>
>>>>>
>>>>
>>>> - Péter
>>>>
>>>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>>>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>>>
>>>
>>> - Seraj
>>>
>>
>> - Péter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* [3/4] dmaengine: imx-sdma: implement channel termination via worker
From: Lucas Stach @ 2018-09-03 8:41 UTC (permalink / raw)
To: Robin Gong, Vinod Koul
Cc: dmaengine@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
patchwork-lst@pengutronix.de
Hi Robin,
Am Freitag, den 31.08.2018, 09:49 +0000 schrieb Robin Gong:
> Hi Lucas,
> Seems I miss your previous mail. Thanks for your patch, but if move most jobs
> of sdma_disable_channel_with_delay() into worker, that will bring another race condition
> that upper driver such as Audio terminate channel and free resource of dma channel without
> really channel stop, if dma transfer done interrupt come after that, oops or kernel cash may
> be caught. Leave 'sdmac->desc = NULL' in the sdma_disable_channel_with_delay() may fix
> such potential issue.
No, there is no such issue. The audio channel terminate will call
dmaengine_terminate_sync(), which internally calls
dmaengine_terminate_async() and then does a dmaengine_synchronize(). As this patchset implements the device_synchronize function in the sdma driver, this will wait for the worker to finish its execution, so there is no race condition to worry about here.
Regards,
Lucas
> > -----Original Message-----
> > > > From: Lucas Stach <l.stach@pengutronix.de>
> > Sent: 2018年8月30日 21:22
> > > > To: Vinod Koul <vkoul@kernel.org>
> > > > > > Cc: Robin Gong <yibin.gong@nxp.com>; dmaengine@vger.kernel.org;
> > > > > > dl-linux-imx <linux-imx@nxp.com>; kernel@pengutronix.de;
> > patchwork-lst@pengutronix.de
> > Subject: [PATCH 3/4] dmaengine: imx-sdma: implement channel termination via
> > worker
> >
> > The dmaengine documentation states that device_terminate_all may be
> > asynchronous and need not wait for the active transfers to stop.
> >
> > This allows us to move most of the functionality currently implemented in the
> > sdma channel termination function to run in a worker, outside of any atomic
> > context. Moving this out of atomic context has two
> > benefits: we can now sleep while waiting for the channel to terminate, instead
> > of busy waiting and the freeing of the dma descriptors happens with IRQs
> > enabled, getting rid of a warning in the dma mapping code.
> >
> > As the termination is now async, we need to implement the device_synchronize
> > dma engine function which simply waits for the worker to finish its execution.
> >
> > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> > drivers/dma/imx-sdma.c | 40 +++++++++++++++++++++++++++++-----------
> > 1 file changed, 29 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index
> > 8d2fec8b16cc..a3ac216ede37 100644
> > --- a/drivers/dma/imx-sdma.c
> > +++ b/drivers/dma/imx-sdma.c
> > @@ -32,6 +32,7 @@
> > #include <linux/of_address.h>
> > #include <linux/of_device.h>
> > #include <linux/of_dma.h>
> > +#include <linux/workqueue.h>
> >
> > #include <asm/irq.h>
> > #include <linux/platform_data/dma-imx-sdma.h>
> > @@ -375,6 +376,7 @@ struct sdma_channel {
> > > > > > u32 shp_addr, per_addr;
> > > > > > enum dma_status status;
> > > > > > struct imx_dma_data data;
> > > > > > + struct work_struct terminate_worker;
> > };
> >
> > > > #define IMX_DMA_SG_LOOP BIT(0)
> > @@ -1025,31 +1027,45 @@ static int sdma_disable_channel(struct dma_chan
> > *chan)
> >
> > > > return 0;
> > }
> > -
> > -static int sdma_disable_channel_with_delay(struct dma_chan *chan)
> > +static void sdma_channel_terminate(struct work_struct *work)
> > {
> > > > - struct sdma_channel *sdmac = to_sdma_chan(chan);
> > > > + struct sdma_channel *sdmac = container_of(work, struct sdma_channel,
> > > > + terminate_worker);
> > > > unsigned long flags;
> > > > LIST_HEAD(head);
> >
> > > > - sdma_disable_channel(chan);
> > > > - spin_lock_irqsave(&sdmac->vc.lock, flags);
> > > > - vchan_get_all_descriptors(&sdmac->vc, &head);
> > > > - sdmac->desc = NULL;
> > > > - spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> > > > - vchan_dma_desc_free_list(&sdmac->vc, &head);
> > -
> > > > /*
> > > > * According to NXP R&D team a delay of one BD SDMA cost time
> > > > * (maximum is 1ms) should be added after disable of the channel
> > > > * bit, to ensure SDMA core has really been stopped after SDMA
> > > > * clients call .device_terminate_all.
> > > > */
> > > > - mdelay(1);
> > > > + usleep_range(1000, 2000);
> > +
> > > > + spin_lock_irqsave(&sdmac->vc.lock, flags);
> > > > + vchan_get_all_descriptors(&sdmac->vc, &head);
> > > > + sdmac->desc = NULL;
> > > > + spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> > > > + vchan_dma_desc_free_list(&sdmac->vc, &head); }
> > +
> > +static int sdma_disable_channel_with_delay(struct dma_chan *chan) {
> > > > + struct sdma_channel *sdmac = to_sdma_chan(chan);
> > +
> > > > + sdma_disable_channel(chan);
> > > > + schedule_work(&sdmac->terminate_worker);
> >
> > > > return 0;
> > }
> >
> > +static void sdma_channel_synchronize(struct dma_chan *chan) {
> > > > + struct sdma_channel *sdmac = to_sdma_chan(chan);
> > +
> > > > + flush_work(&sdmac->terminate_worker);
> > +}
> > +
> > static void sdma_set_watermarklevel_for_p2p(struct sdma_channel *sdmac)
> > {
> > > > struct sdma_engine *sdma = sdmac->sdma; @@ -1993,6 +2009,7 @@
> > static int sdma_probe(struct platform_device *pdev)
> >
> > > > sdmac->channel = i;
> > > > sdmac->vc.desc_free = sdma_desc_free;
> > > > + INIT_WORK(&sdmac->terminate_worker,
> > sdma_channel_terminate);
> > > > /*
> > > > * Add the channel to the DMAC list. Do not add channel 0 though
> > > > * because we need it internally in the SDMA driver. This also means
> > @@ -2045,6 +2062,7 @@ static int sdma_probe(struct platform_device
> > *pdev)
> > > > sdma->dma_device.device_prep_dma_cyclic = sdma_prep_dma_cyclic;
> > > > sdma->dma_device.device_config = sdma_config;
> > > > sdma->dma_device.device_terminate_all =
> > sdma_disable_channel_with_delay;
> > > > + sdma->dma_device.device_synchronize = sdma_channel_synchronize;
> > > > sdma->dma_device.src_addr_widths = SDMA_DMA_BUSWIDTHS;
> > > > sdma->dma_device.dst_addr_widths = SDMA_DMA_BUSWIDTHS;
> > > > sdma->dma_device.directions = SDMA_DMA_DIRECTIONS;
> > --
> > 2.18.0
>
>
^ permalink raw reply
* [v4,2/7] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors
From: Andrea Merello @ 2018-09-03 8:46 UTC (permalink / raw)
To: Vinod
Cc: dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
linux-kernel, Rob Herring, Mark Rutland, devicetree,
Radhey Shyam Pandey
On Thu, Aug 30, 2018 at 3:27 PM Vinod <vkoul@kernel.org> wrote:
>
> On 30-08-18, 10:11, Andrea Merello wrote:
> > On Wed, Aug 29, 2018 at 10:12 AM Andrea Merello
> > <andrea.merello@gmail.com> wrote:
> > >
> > > On Mon, Aug 27, 2018 at 7:30 AM Vinod <vkoul@kernel.org> wrote:
> > > >
> > > > On 02-08-18, 16:10, Andrea Merello wrote:
> > > >
> > > > s/cylic/cyclic in patch title
> > >
> > > OK
> > >
> > > > > Whenever a single or cyclic transaction is prepared, the driver
> > > > > could eventually split it over several SG descriptors in order
> > > > > to deal with the HW maximum transfer length.
> > > > >
> > > > > This could end up in DMA operations starting from a misaligned
> > > > > address. This seems fatal for the HW if DRE is not enabled.
> > > >
> > > > DRE?
> > >
> > > Stands for "Data Realignment Engine". I will add this string nearby
> > > the acronym..
> > >
> > > > >
> > > > > This patch eventually adjusts the transfer size in order to make sure
> > > > > all operations start from an aligned address.
> > > > >
> > > > > Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > > > > Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> > > > > Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > > > > ---
> > > > > Changes in v2:
> > > > > - don't introduce copy_mask field, rather rely on already-esistent
> > > > > copy_align field. Suggested by Radhey Shyam Pandey
> > > > > - reword title
> > > > > Changes in v3:
> > > > > - fix bug introduced in v2: wrong copy size when DRE is enabled
> > > > > - use implementation suggested by Radhey Shyam Pandey
> > > > > Changes in v4:
> > > > > - rework on the top of 1/6
> > > > > ---
> > > > > drivers/dma/xilinx/xilinx_dma.c | 22 ++++++++++++++++++----
> > > > > 1 file changed, 18 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> > > > > index a3aaa0e34cc7..aaa6de8a70e4 100644
> > > > > --- a/drivers/dma/xilinx/xilinx_dma.c
> > > > > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > > > > @@ -954,15 +954,28 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
> > > > >
> > > > > /**
> > > > > * xilinx_dma_calc_copysize - Calculate the amount of data to copy
> > > > > + * @chan: Driver specific DMA channel
> > > > > * @size: Total data that needs to be copied
> > > > > * @done: Amount of data that has been already copied
> > > > > *
> > > > > * Return: Amount of data that has to be copied
> > > > > */
> > > > > -static int xilinx_dma_calc_copysize(int size, int done)
> > > > > +static int xilinx_dma_calc_copysize(struct xilinx_dma_chan *chan,
> > > > > + int size, int done)
> > > >
> > > > please align with opening brace
> > >
> > > OK
> >
> > Sorry for getting back on this.
> > I've checked it, but it seems already aligned with opening brace in
> > the original e-mail text I've sent. (4 tabs + 4 spaces).
>
> Okay, please see that code looks fine, I will check after I apply
Yes, I confirm that here the code does look fine: the 2nd line is
aligned with opening brace indeed.
Do you want I produce now a v5 with all the other fixes you asked for
(basically commit message fixes), or you are going to apply/check this
one and should I wait for that?
> --
> ~Vinod
^ permalink raw reply
* [3/4] dmaengine: imx-sdma: implement channel termination via worker
From: Robin Gong @ 2018-09-03 8:59 UTC (permalink / raw)
To: Lucas Stach, Vinod Koul
Cc: dmaengine@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
patchwork-lst@pengutronix.de
> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: 2018年9月3日 16:41
> To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kernel.org>
> Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> kernel@pengutronix.de; patchwork-lst@pengutronix.de
> Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> termination via worker
>
> Hi Robin,
>
> Am Freitag, den 31.08.2018, 09:49 +0000 schrieb Robin Gong:
> > Hi Lucas,
> > Seems I miss your previous mail. Thanks for your patch, but if move
> > most jobs of sdma_disable_channel_with_delay() into worker, that will
> > bring another race condition that upper driver such as Audio terminate
> > channel and free resource of dma channel without really channel stop,
> > if dma transfer done interrupt come after that, oops or kernel cash
> > may be caught. Leave 'sdmac->desc = NULL' in the
> sdma_disable_channel_with_delay() may fix such potential issue.
>
> No, there is no such issue. The audio channel terminate will call
> dmaengine_terminate_sync(), which internally calls
> dmaengine_terminate_async() and then does a dmaengine_synchronize(). As
> this patchset implements the device_synchronize function in the sdma driver,
> this will wait for the worker to finish its execution, so there is no race condition
> to worry about here.
>
> Regards,
> Lucas
Yes, but how about other drivers which not call dmaengine_terminate_sync()?
>
>
> > > -----Original Message-----
> > > > > From: Lucas Stach <l.stach@pengutronix.de>
> > > Sent: 2018年8月30日 21:22
> > > > > To: Vinod Koul <vkoul@kernel.org>
> > > > > > > Cc: Robin Gong <yibin.gong@nxp.com>;
> > > > > > > dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > > > > > > kernel@pengutronix.de;
> > > patchwork-lst@pengutronix.de
> > > Subject: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> > > termination via worker
> > >
> > > The dmaengine documentation states that device_terminate_all may be
> > > asynchronous and need not wait for the active transfers to stop.
> > >
> > > This allows us to move most of the functionality currently
> > > implemented in the sdma channel termination function to run in a
> > > worker, outside of any atomic context. Moving this out of atomic
> > > context has two
> > > benefits: we can now sleep while waiting for the channel to
> > > terminate, instead of busy waiting and the freeing of the dma
> > > descriptors happens with IRQs enabled, getting rid of a warning in the dma
> mapping code.
> > >
> > > As the termination is now async, we need to implement the
> > > device_synchronize dma engine function which simply waits for the worker
> to finish its execution.
> > >
> > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > ---
> > > drivers/dma/imx-sdma.c | 40
> > > +++++++++++++++++++++++++++++-----------
> > > 1 file changed, 29 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index
> > > 8d2fec8b16cc..a3ac216ede37 100644
> > > --- a/drivers/dma/imx-sdma.c
> > > +++ b/drivers/dma/imx-sdma.c
> > > @@ -32,6 +32,7 @@
> > > #include <linux/of_address.h>
> > > #include <linux/of_device.h>
> > > #include <linux/of_dma.h>
> > > +#include <linux/workqueue.h>
> > >
> > > #include <asm/irq.h>
> > > #include <linux/platform_data/dma-imx-sdma.h>
> > > @@ -375,6 +376,7 @@ struct sdma_channel {
> > > > > > > u32 shp_addr, per_addr;
> > > > > > > enum dma_status status;
> > > > > > > struct imx_dma_data data;
> > > > > > > + struct work_struct terminate_worker;
> > > };
> > >
> > > > > #define IMX_DMA_SG_LOOP BIT(0)
> > > @@ -1025,31 +1027,45 @@ static int sdma_disable_channel(struct
> > > dma_chan
> > > *chan)
> > >
> > > > > return 0;
> > > }
> > > -
> > > -static int sdma_disable_channel_with_delay(struct dma_chan *chan)
> > > +static void sdma_channel_terminate(struct work_struct *work)
> > > {
> > > > > - struct sdma_channel *sdmac = to_sdma_chan(chan);
> > > > > + struct sdma_channel *sdmac = container_of(work, struct
> sdma_channel,
> > > > > + terminate_worker);
> > > > > unsigned long flags;
> > > > > LIST_HEAD(head);
> > >
> > > > > - sdma_disable_channel(chan);
> > > > > - spin_lock_irqsave(&sdmac->vc.lock, flags);
> > > > > - vchan_get_all_descriptors(&sdmac->vc, &head);
> > > > > - sdmac->desc = NULL;
> > > > > - spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> > > > > - vchan_dma_desc_free_list(&sdmac->vc, &head);
> > > -
> > > > > /*
> > > > > * According to NXP R&D team a delay of one BD SDMA cost time
> > > > > * (maximum is 1ms) should be added after disable of the channel
> > > > > * bit, to ensure SDMA core has really been stopped after SDMA
> > > > > * clients call .device_terminate_all.
> > > > > */
> > > > > - mdelay(1);
> > > > > + usleep_range(1000, 2000);
> > > +
> > > > > + spin_lock_irqsave(&sdmac->vc.lock, flags);
> > > > > + vchan_get_all_descriptors(&sdmac->vc, &head);
> > > > > + sdmac->desc = NULL;
> > > > > + spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> > > > > + vchan_dma_desc_free_list(&sdmac->vc, &head); }
> > > +
> > > +static int sdma_disable_channel_with_delay(struct dma_chan *chan) {
> > > > > + struct sdma_channel *sdmac = to_sdma_chan(chan);
> > > +
> > > > > + sdma_disable_channel(chan);
> > > > > + schedule_work(&sdmac->terminate_worker);
> > >
> > > > > return 0;
> > > }
> > >
> > > +static void sdma_channel_synchronize(struct dma_chan *chan) {
> > > > > + struct sdma_channel *sdmac = to_sdma_chan(chan);
> > > +
> > > > > + flush_work(&sdmac->terminate_worker);
> > > +}
> > > +
> > > static void sdma_set_watermarklevel_for_p2p(struct sdma_channel
> > > *sdmac) {
> > > > > struct sdma_engine *sdma = sdmac->sdma; @@ -1993,6 +2009,7
> @@
> > > static int sdma_probe(struct platform_device *pdev)
> > >
> > > > > sdmac->channel = i;
> > > > > sdmac->vc.desc_free = sdma_desc_free;
> > > > > + INIT_WORK(&sdmac->terminate_worker,
> > > sdma_channel_terminate);
> > > > > /*
> > > > > * Add the channel to the DMAC list. Do not add channel 0
> though
> > > > > * because we need it internally in the SDMA driver. This
> > > > > also means
> > > @@ -2045,6 +2062,7 @@ static int sdma_probe(struct platform_device
> > > *pdev)
> > > > > sdma->dma_device.device_prep_dma_cyclic =
> sdma_prep_dma_cyclic;
> > > > > sdma->dma_device.device_config = sdma_config;
> > > > > sdma->dma_device.device_terminate_all =
> > > sdma_disable_channel_with_delay;
> > > > > + sdma->dma_device.device_synchronize =
> > > > > +sdma_channel_synchronize;
> > > > > sdma->dma_device.src_addr_widths = SDMA_DMA_BUSWIDTHS;
> > > > > sdma->dma_device.dst_addr_widths = SDMA_DMA_BUSWIDTHS;
> > > > > sdma->dma_device.directions = SDMA_DMA_DIRECTIONS;
> > > --
> > > 2.18.0
> >
> >
^ permalink raw reply
* [v4,2/7] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors
From: Vinod Koul @ 2018-09-03 10:49 UTC (permalink / raw)
To: Andrea Merello
Cc: dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
linux-kernel, Rob Herring, Mark Rutland, devicetree,
Radhey Shyam Pandey
On 03-09-18, 10:46, Andrea Merello wrote:
> Yes, I confirm that here the code does look fine: the 2nd line is
> aligned with opening brace indeed.
>
> Do you want I produce now a v5 with all the other fixes you asked for
> (basically commit message fixes), or you are going to apply/check this
> one and should I wait for that?
v5 please
^ permalink raw reply
* [1/3] dmaengine: fsldma: move spin_lock_bh to spin_lock in tasklet
From: Vinod Koul @ 2018-09-03 10:55 UTC (permalink / raw)
To: Barry Song
Cc: leoyang.li, zw, dan.j.williams, linuxppc-dev, dmaengine,
linux-kernel
On 17-08-18, 06:00, Barry Song wrote:
> as you are already in a tasklet, it is unnecessary to call spin_lock_bh.
Applied all, thanks
You may want to check you send-mail options, they patches were all over
my inbox and npot threaded
^ permalink raw reply
* [RESEND,v2] dmaengine: sprd: Support DMA link-list mode
From: Vinod Koul @ 2018-09-03 11:30 UTC (permalink / raw)
To: Baolin Wang; +Cc: dan.j.williams, eric.long, broonie, dmaengine, linux-kernel
On 28-08-18, 19:09, Baolin Wang wrote:
> From: Eric Long <eric.long@spreadtrum.com>
>
> The Spreadtrum DMA can support the link-list transaction mode, which means
> DMA controller can do transaction one by one automatically once we linked
> these transaction by link-list register.
Applied, thanks
^ permalink raw reply
* [3/4] dmaengine: imx-sdma: implement channel termination via worker
From: Lucas Stach @ 2018-09-03 13:11 UTC (permalink / raw)
To: Robin Gong, Vinod Koul
Cc: dmaengine@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
patchwork-lst@pengutronix.de
Am Montag, den 03.09.2018, 08:59 +0000 schrieb Robin Gong:
> > -----Original Message-----
> > From: Lucas Stach <l.stach@pengutronix.de>
> > Sent: 2018年9月3日 16:41
> > To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kernel.org>
> > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > kernel@pengutronix.de; patchwork-lst@pengutronix.de
> > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> > termination via worker
> >
> > Hi Robin,
> >
> > Am Freitag, den 31.08.2018, 09:49 +0000 schrieb Robin Gong:
> > > Hi Lucas,
> > > Seems I miss your previous mail. Thanks for your patch, but if
> > > move
> > > most jobs of sdma_disable_channel_with_delay() into worker, that
> > > will
> > > bring another race condition that upper driver such as Audio
> > > terminate
> > > channel and free resource of dma channel without really channel
> > > stop,
> > > if dma transfer done interrupt come after that, oops or kernel
> > > cash
> > > may be caught. Leave 'sdmac->desc = NULL' in the
> >
> > sdma_disable_channel_with_delay() may fix such potential issue.
> >
> > No, there is no such issue. The audio channel terminate will call
> > dmaengine_terminate_sync(), which internally calls
> > dmaengine_terminate_async() and then does a
> > dmaengine_synchronize(). As
> > this patchset implements the device_synchronize function in the
> > sdma driver,
> > this will wait for the worker to finish its execution, so there is
> > no race condition
> > to worry about here.
> >
> > Regards,
> > Lucas
>
> Yes, but how about other drivers which not call
> dmaengine_terminate_sync()?
Please read the dmaengine documentation. device_terminate_all has no
requirement that the transfer is actually canceled when the call
returns. If the caller needs a guarantee that the channel is stopped it
_must_ call device_synchronize.
For your convenience I'm copying the relevant part of the docs below
(from dmaengine_terminate_async(), which is what calls
device_terminate_all():
"Calling this function will terminate all active and pending
descriptors that have previously been submitted to the channel. It is
not guaranteed though that the transfer for the active descriptor has
stopped when the function returns. Furthermore it is possible the
complete callback of a submitted transfer is still running when this
function returns.
dmaengine_synchronize() needs to be called before it is safe to free
any memory that is accessed by previously submitted descriptors or
before freeing any resources accessed from within the completion
callback of any previously submitted descriptors."
Regards,
Lucas
^ permalink raw reply
* [V2,5/5] dmaengine: Documentation: Add documentation for multi chan testing
From: Seraj Mohammed @ 2018-09-03 14:49 UTC (permalink / raw)
To: Peter Ujfalusi, Seraj Alijan, vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake
Hi,
On 03/09/18 09:19, Peter Ujfalusi wrote:
> Hi,
>
> On 2018-08-31 18:01, Seraj Mohammed wrote:
>>> However, if I got it right I need to reboot the machine if I want to
>>> change any of the parameters for a re-run.
>>> Let's say I want to run tests on 6 channel with 50K buffers, 100K 1M, 10M.
>>> I can not do this without rebooting, right?
>>
>> You only need to reboot the machine if you want to change paameters after
>> allocating the channels.
>
> Instead of reboot running the test is fine to clear things up.
>
>> Example:
>>
>> echo 4096 > /sys/module/dmatest/parameters/test_buf_size
>> echo 1 > /sys/module/dmatest/parameters/iterations
>> echo 6 > /sys/module/dmatest/parameters/alignment
>> echo 0 > /sys/module/dmatest/parameters/noverify
>> echo 0 > /sys/module/dmatest/parameters/norandom
>> echo 1 > /sys/module/dmatest/parameters/threads_per_chan
>> echo 20000 > /sys/module/dmatest/parameters/timeout
>>
>> At the is point, you can still change any of the parameters above without
>> rebooting
>
> But I can not do:
> echo 8000000 > /sys/module/dmatest/parameters/test_buf_size
> echo 2000 > /sys/module/dmatest/parameters/timeout
> echo 20 > /sys/module/dmatest/parameters/iterations
> echo 20 > /sys/module/dmatest/parameters/max_channels
> echo 1 > /sys/module/dmatest/parameters/run
>
> it will _not_ start 20 iteration tests on 20 channels.
This is correct behavior, because issuing
echo 1 > /sys/module/dmatest/parameters/run
will no longer handle channel allocation with this patch, all
functionality dealing with allocating channels has been stripped off
from the run callback and put into a new callback that is triggered by
the channel parameter.
So under the new change, you can do the same by:
echo 8000000 > /sys/module/dmatest/parameters/test_buf_size
echo 2000 > /sys/module/dmatest/parameters/timeout
echo 20 > /sys/module/dmatest/parameters/iterations
echo 20 > /sys/module/dmatest/parameters/max_channels
echo "" > /sys/module/dmatest/parameters/channel << Will trigger call
back to allocate up to max_channels if invoked with empty string
echo 1 > /sys/module/dmatest/parameters/run
With this change setting run to 1 will only launch threads that have
already been allocated by invoking the channel parameter by either
echoing an empty string, or a specific channel. If no channels are
allocated, setting run to 1 will not do anything.
>>
>> echo dma0chan0 > /sys/module/dmatest/parameters/channel
>> [ 31.465240] dmatest: Added 1 threads using dma0chan0
>> echo dma0chan1 > /sys/module/dmatest/parameters/channel
>> [ 33.142020] dmatest: Added 1 threads using dma0chan1
>> echo dma0chan2 > /sys/module/dmatest/parameters/channel
>> [ 34.402123] dmatest: Added 1 threads using dma0chan2
>
> I need to:
> echo dma0chan10 > /sys/module/dmatest/parameters/channel
> cat /sys/module/dmatest/parameters/channel
> dma0chan10
>
> echo dma0chan11 > /sys/module/dmatest/parameters/channel
> cat /sys/module/dmatest/parameters/channel
> dma0chan11
>
> ^ it tells me that only dma0chan11 is in channels while in fact I have
> dma0chan10 and dma0chan11 selected.
Correct, reading the channel parameter will report back the name of the
LAST channel that was allocated, not a string containing all channels.
> then I run the test:
> echo 1 > /sys/module/dmatest/parameters/run
> [ 514.370959] dmatest: dma0chan10-copy: summary 1 tests, 0 failures
> 7692 iops 15384 KB/s (0)
> [ 514.371099] dmatest: dma0chan11-copy: summary 1 tests, 0 failures
> 11111 iops 22222 KB/s (0)
>
> After this:
> cat /sys/module/dmatest/parameters/channel returns nothing, which I
> believe is correct. So I could be able to run the 20 iteration on 20
> channels, but I can not:
> echo 20 > /sys/module/dmatest/parameters/iterations
> echo 20 > /sys/module/dmatest/parameters/max_channels
> echo 1 > /sys/module/dmatest/parameters/run
>
> The patch effectively breaks the batch mode via max_channels.
It doesn't matter if the channel string is empty, you need to invoke the
channel parameter to allocate channels prior to running the test.
This should work
echo 20 > /sys/module/dmatest/parameters/iterations
echo 20 > /sys/module/dmatest/parameters/max_channels
echo "" > /sys/module/dmatest/parameters/channel
echo 1 > /sys/module/dmatest/parameters/run
>> As you can see now channels 0,1, 2 have already been allocated and their
>> respective threads are pending. At this point if you want to change
>> any parameters you have to reboot, because the channels have already been
>> requested with the properties set above.
>>
>> echo 1 > /sys/module/dmatest/parameters/run
>> dmatest: dma0chan0-copy0: summary 1 tests, 0 failures 285.71 iops 571
>> KB/s (0)
>> dmatest: dma0chan2-copy0: summary 1 tests, 0 failures 142.85 iops 285
>> KB/s (0)
>> dmatest: dma0chan1-copy0: summary 1 tests, 0 failures 142.85 iops 428
>> KB/s (0)
>>
>> At this point the test is finished and you can run a new test with
>> completely different parameters without having to reboot.
>
> That's fine.
>
> If I want to just repeat the same test then I would put the same
> channels back in a loop and run it again to run them overnight for example.
>
> But it is still unclear how the channels will be released..
> cat /sys/class/dma/dma0chan10/in_use
> 0
> echo dma0chan10 > /sys/module/dmatest/parameters/channel
> cat /sys/class/dma/dma0chan10/in_use
> 1
> echo 1 > /sys/module/dmatest/parameters/run
> [ 690.326039] dmatest: dma0chan10-copy: summary 20 tests, 0 failures 71
> iops 290511 KB/s (0)
> cat /sys/class/dma/dma0chan10/in_use
> 1
> echo 1 > /sys/module/dmatest/parameters/run
> # will not run any tests
> cat /sys/class/dma/dma0chan10/in_use
> 1
>
> echo dma0chan12 > /sys/module/dmatest/parameters/channel
> cat /sys/class/dma/dma0chan10/in_use
> 0
> cat /sys/class/dma/dma0chan12/in_use
> 1
>
>
> So it appears that the channel is going to be released when a new
> channel is added, right?
Correct, after completing a test, adding a new channel will clear all
previously allocated channels to ensure the new test is not polluted
with residual parameters from prior test runs.
Another way of clearing channels from previous test run is to query the
run parameter:
cat /sys/module/dmatest/parameters/run
should clear all channels allocated by previous test run.
> Which brought up some interesting thing:
> echo "" > /sys/module/dmatest/parameters/channel
>
> Will add the 20 channels to the test :o
>
> One thing would be really nice is to print something when we hit run and
> the dmatest actually going to do something or if the dmatest is not
> configured properly and will not going to run any test.
Good idea, adding a message informing user should clear up any confusion.
>
>>
>>> Would not be possible to:
>>>
>>> echo "dma0chan0 dma0chan3 dma0chan5 dma0chan6" >
>>> /sys/module/dmatest/parameters/channel
>>>
>>> and use the same logic already existing for max_channels to start off
>>> the tests on different channels?
>>
>> It is possible, but i wouldn't make it possible to start all threads at
>> once, which is what i want to do. If i do it this way, the test would have
>> have extract the channels one by one and allocate and start them
>> sequentially. My patch ensures that when I set run to 1, all channels fire
>> off at the same time, or as close as possible without having the overhead
>> of allocating channels inbetween transfers.
>>
>>>> You can still run your torture test with a simple modification:
>>>>
>>>> echo 8000000 > /sys/module/dmatest/parameters/test_buf_size
>>>> echo 2000 > /sys/module/dmatest/parameters/timeout
>>>> echo 20 > /sys/module/dmatest/parameters/iterations
>>>> echo 20 > /sys/module/dmatest/parameters/max_channels
>>>> echo "" > /sys/module/dmatest/parameters/channel << allocates 20 channels
>>>> echo 1 > /sys/module/dmatest/parameters/run
>>>
>>> Sure, but if I test something on a specific channels I need to reboot.
>>> Or if I want to change parameters for a next run...
>>>
>>>>
>>>>>> + % echo 1 > /sys/module/dmatest/parameters/run
>>>>>>
>>>>>> +Note: the channel parameter should always be the last parameter set prior to
>>>>>> +running the test (setting run=1), this is because upon setting the channel
>>>>>> +parameter, that specific channel is requested using the dmaengine and a thread
>>>>>> +is created with the existing parameters. This thread is set as pending
>>>>>> +and will be executed once run is set to 1. Any parameters set after the thread
>>>>>> +is created are not applied.
>>>>>> .. hint::
>>>>>> available channel list could be extracted by running the following command::
>>>>>>
>>>>>> % ls -1 /sys/class/dma/
>>>>>>
>>>>>> -Once started a message like "dmatest: Started 1 threads using dma0chan0" is
>>>>>> -emitted. After that only test failure messages are reported until the test
>>>>>> -stops.
>>>>>> +Once started a message like " dmatest: Added 1 threads using dma0chan0" is
>>>>>> +emitted. A thread for that specific channel is created and is now pending, the
>>>>>> +pending thread is started once run is to 1.
>>>>>>
>>>>>> Note that running a new test will not stop any in progress test.
>>>>>>
>>>>>>
>>>>>
>>>>> - Péter
>>>>>
>>>>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>>>>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>>>>
>>>>
>>>> - Seraj
>>>>
>>>
>>> - Péter
>>>
>>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>>
>
> - Péter
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
- Seraj
^ permalink raw reply
* [3/4] dmaengine: imx-sdma: implement channel termination via worker
From: Robin Gong @ 2018-09-04 2:36 UTC (permalink / raw)
To: Lucas Stach, Vinod Koul
Cc: dmaengine@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
patchwork-lst@pengutronix.de
> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: 2018年9月3日 21:12
> To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kernel.org>
> Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> kernel@pengutronix.de; patchwork-lst@pengutronix.de
> Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> termination via worker
>
> Am Montag, den 03.09.2018, 08:59 +0000 schrieb Robin Gong:
> > > -----Original Message-----
> > > From: Lucas Stach <l.stach@pengutronix.de>
> > > Sent: 2018年9月3日 16:41
> > > To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kernel.org>
> > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > > kernel@pengutronix.de; patchwork-lst@pengutronix.de
> > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> > > termination via worker
> > >
> > > Hi Robin,
> > >
> > > Am Freitag, den 31.08.2018, 09:49 +0000 schrieb Robin Gong:
> > > > Hi Lucas,
> > > > Seems I miss your previous mail. Thanks for your patch, but if
> > > > move most jobs of sdma_disable_channel_with_delay() into worker,
> > > > that will bring another race condition that upper driver such as
> > > > Audio terminate channel and free resource of dma channel without
> > > > really channel stop, if dma transfer done interrupt come after
> > > > that, oops or kernel cash may be caught. Leave 'sdmac->desc =
> > > > NULL' in the
> > >
> > > sdma_disable_channel_with_delay() may fix such potential issue.
> > >
> > > No, there is no such issue. The audio channel terminate will call
> > > dmaengine_terminate_sync(), which internally calls
> > > dmaengine_terminate_async() and then does a dmaengine_synchronize().
> > > As this patchset implements the device_synchronize function in the
> > > sdma driver, this will wait for the worker to finish its execution,
> > > so there is no race condition to worry about here.
> > >
> > > Regards,
> > > Lucas
> >
> > Yes, but how about other drivers which not call
> > dmaengine_terminate_sync()?
>
> Please read the dmaengine documentation. device_terminate_all has no
> requirement that the transfer is actually canceled when the call returns. If the
> caller needs a guarantee that the channel is stopped it _must_ call
> device_synchronize.
I know that, but the fact is some driver still use dmaengine_terminate_all() such as
Spi/uart driver. My concern is how to avoid to break their function.
>
> For your convenience I'm copying the relevant part of the docs below (from
> dmaengine_terminate_async(), which is what calls
> device_terminate_all():
>
> "Calling this function will terminate all active and pending descriptors that have
> previously been submitted to the channel. It is not guaranteed though that the
> transfer for the active descriptor has stopped when the function returns.
> Furthermore it is possible the complete callback of a submitted transfer is still
> running when this function returns.
>
> dmaengine_synchronize() needs to be called before it is safe to free any
> memory that is accessed by previously submitted descriptors or before freeing
> any resources accessed from within the completion callback of any previously
> submitted descriptors."
>
> Regards,
> Lucas
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox