The dmatest module parameter 'timeout' is documented as accepting a
-1 to mean "infinite timeout". Change the parameter to to signed
integer, and check the value to call the appropriate wait_event()
function.
Signed-off-by: Gary R Hook <[email protected]>
---
drivers/dma/dmatest.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index b96814a7dceb..28a237686578 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -63,7 +63,7 @@ MODULE_PARM_DESC(pq_sources,
"Number of p+q source buffers (default: 3)");
static int timeout = 3000;
-module_param(timeout, uint, S_IRUGO | S_IWUSR);
+module_param(timeout, int, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(timeout, "Transfer Timeout in msec (default: 3000), "
"Pass -1 for infinite timeout");
@@ -795,8 +795,13 @@ static int dmatest_func(void *data)
}
dma_async_issue_pending(chan);
- wait_event_freezable_timeout(thread->done_wait, done->done,
- msecs_to_jiffies(params->timeout));
+ /* A timeout value of -1 means infinite wait */
+ if (timeout == -1)
+ wait_event_freezable(thread->done_wait, done->done);
+ else
+ wait_event_freezable_timeout(thread->done_wait,
+ done->done,
+ msecs_to_jiffies(params->timeout));
status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
On 31-05-19, 19:43, Hook, Gary wrote:
> The dmatest module parameter 'timeout' is documented as accepting a
> -1 to mean "infinite timeout". Change the parameter to to signed
> integer, and check the value to call the appropriate wait_event()
> function.
>
> Signed-off-by: Gary R Hook <[email protected]>
> ---
> drivers/dma/dmatest.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index b96814a7dceb..28a237686578 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -63,7 +63,7 @@ MODULE_PARM_DESC(pq_sources,
> "Number of p+q source buffers (default: 3)");
>
> static int timeout = 3000;
> -module_param(timeout, uint, S_IRUGO | S_IWUSR);
> +module_param(timeout, int, S_IRUGO | S_IWUSR);
> MODULE_PARM_DESC(timeout, "Transfer Timeout in msec (default: 3000), "
> "Pass -1 for infinite timeout");
>
> @@ -795,8 +795,13 @@ static int dmatest_func(void *data)
> }
> dma_async_issue_pending(chan);
>
> - wait_event_freezable_timeout(thread->done_wait, done->done,
> - msecs_to_jiffies(params->timeout));
> + /* A timeout value of -1 means infinite wait */
> + if (timeout == -1)
> + wait_event_freezable(thread->done_wait, done->done);
well i am not too happy that we have a infinite wait and no way to
cancel, maybe remove this case?
> + else
> + wait_event_freezable_timeout(thread->done_wait,
> + done->done,
> + msecs_to_jiffies(params->timeout));
>
> status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
>
>
--
~Vinod
On 6/4/19 7:23 AM, Vinod Koul wrote:
> [CAUTION: External Email]
>
> On 31-05-19, 19:43, Hook, Gary wrote:
>> The dmatest module parameter 'timeout' is documented as accepting a
>> -1 to mean "infinite timeout". Change the parameter to to signed
>> integer, and check the value to call the appropriate wait_event()
>> function.
>>
>> Signed-off-by: Gary R Hook <[email protected]>
>> ---
>> drivers/dma/dmatest.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
>> index b96814a7dceb..28a237686578 100644
>> --- a/drivers/dma/dmatest.c
>> +++ b/drivers/dma/dmatest.c
>> @@ -63,7 +63,7 @@ MODULE_PARM_DESC(pq_sources,
>> "Number of p+q source buffers (default: 3)");
>>
>> static int timeout = 3000;
>> -module_param(timeout, uint, S_IRUGO | S_IWUSR);
>> +module_param(timeout, int, S_IRUGO | S_IWUSR);
>> MODULE_PARM_DESC(timeout, "Transfer Timeout in msec (default: 3000), "
>> "Pass -1 for infinite timeout");
>>
>> @@ -795,8 +795,13 @@ static int dmatest_func(void *data)
>> }
>> dma_async_issue_pending(chan);
>>
>> - wait_event_freezable_timeout(thread->done_wait, done->done,
>> - msecs_to_jiffies(params->timeout));
>> + /* A timeout value of -1 means infinite wait */
>> + if (timeout == -1)
>> + wait_event_freezable(thread->done_wait, done->done);
>
> well i am not too happy that we have a infinite wait and no way to
> cancel, maybe remove this case?
Well, I was uncomfortable with documentation that didn't match behavior.
I see two choices (and I just chose one to start a conversation):
1) Accept this patch, with an infinite timeout, or
2) Leave the data type alone, but change the description to state that
timeout values up to hex 0xFFFFFFFF / decimal 4294967295 can be used,
emulating an "infinite" wait. A very long wait that eventually pops a
timer is probably preferable. I don't think there are any conversion
issues since the jiffy parameter to wait_event_freezable_timeout() is
converted to a long. I could be wrong about that.
I'm happy to go with option (2). Please suggest a course of action.
grh
>
>> + else
>> + wait_event_freezable_timeout(thread->done_wait,
>> + done->done,
>> + msecs_to_jiffies(params->timeout));
>>
>> status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
On 04-06-19, 16:47, Gary R Hook wrote:
> Well, I was uncomfortable with documentation that didn't match behavior.
That is the right way :)
>
> I see two choices (and I just chose one to start a conversation):
>
> 1) Accept this patch, with an infinite timeout, or
> 2) Leave the data type alone, but change the description to state that
> timeout values up to hex 0xFFFFFFFF / decimal 4294967295 can be used,
> emulating an "infinite" wait. A very long wait that eventually pops a
> timer is probably preferable. I don't think there are any conversion
> issues since the jiffy parameter to wait_event_freezable_timeout() is
> converted to a long. I could be wrong about that.
>
> I'm happy to go with option (2). Please suggest a course of action.
That looks sensible to me as well
Thanks
--
~Vinod