[v4,02/14] dmaengine: tegra-apb: Implement synchronization callback
diff mbox series

Message ID 20200112173006.29863-3-digetx@gmail.com
State Changes Requested
Headers show
Series
  • NVIDIA Tegra APB DMA driver fixes and improvements
Related show

Commit Message

Dmitry Osipenko Jan. 12, 2020, 5:29 p.m. UTC
The ISR tasklet could be kept scheduled after DMA transfer termination,
let's add synchronization callback which blocks until tasklet is finished.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/dma/tegra20-apb-dma.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Jon Hunter Jan. 14, 2020, 3:15 p.m. UTC | #1
On 12/01/2020 17:29, Dmitry Osipenko wrote:
> The ISR tasklet could be kept scheduled after DMA transfer termination,
> let's add synchronization callback which blocks until tasklet is finished.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/dma/tegra20-apb-dma.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index 319f31d27014..664e9c5df3ba 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -798,6 +798,13 @@ static int tegra_dma_terminate_all(struct dma_chan *dc)
>  	return 0;
>  }
>  
> +static void tegra_dma_synchronize(struct dma_chan *dc)
> +{
> +	struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
> +
> +	tasklet_kill(&tdc->tasklet);
> +}
> +

Wouldn't there need to be some clean-up here? If the tasklet is
scheduled, seems that there would be some other house-keeping that needs
to be done after killing it.

Jon
Dmitry Osipenko Jan. 14, 2020, 9:02 p.m. UTC | #2
14.01.2020 18:15, Jon Hunter пишет:
> 
> On 12/01/2020 17:29, Dmitry Osipenko wrote:
>> The ISR tasklet could be kept scheduled after DMA transfer termination,
>> let's add synchronization callback which blocks until tasklet is finished.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/dma/tegra20-apb-dma.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
>> index 319f31d27014..664e9c5df3ba 100644
>> --- a/drivers/dma/tegra20-apb-dma.c
>> +++ b/drivers/dma/tegra20-apb-dma.c
>> @@ -798,6 +798,13 @@ static int tegra_dma_terminate_all(struct dma_chan *dc)
>>  	return 0;
>>  }
>>  
>> +static void tegra_dma_synchronize(struct dma_chan *dc)
>> +{
>> +	struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
>> +
>> +	tasklet_kill(&tdc->tasklet);
>> +}
>> +
> 
> Wouldn't there need to be some clean-up here? If the tasklet is
> scheduled, seems that there would be some other house-keeping that needs
> to be done after killing it.

I'm not seeing anything to clean-up, could you please clarify?
Jon Hunter Jan. 15, 2020, 9:18 a.m. UTC | #3
On 14/01/2020 21:02, Dmitry Osipenko wrote:
> 14.01.2020 18:15, Jon Hunter пишет:
>>
>> On 12/01/2020 17:29, Dmitry Osipenko wrote:
>>> The ISR tasklet could be kept scheduled after DMA transfer termination,
>>> let's add synchronization callback which blocks until tasklet is finished.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  drivers/dma/tegra20-apb-dma.c | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
>>> index 319f31d27014..664e9c5df3ba 100644
>>> --- a/drivers/dma/tegra20-apb-dma.c
>>> +++ b/drivers/dma/tegra20-apb-dma.c
>>> @@ -798,6 +798,13 @@ static int tegra_dma_terminate_all(struct dma_chan *dc)
>>>  	return 0;
>>>  }
>>>  
>>> +static void tegra_dma_synchronize(struct dma_chan *dc)
>>> +{
>>> +	struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
>>> +
>>> +	tasklet_kill(&tdc->tasklet);
>>> +}
>>> +
>>
>> Wouldn't there need to be some clean-up here? If the tasklet is
>> scheduled, seems that there would be some other house-keeping that needs
>> to be done after killing it.
> 
> I'm not seeing anything to clean-up, could you please clarify?

Clean-up with regard to the descriptors. I was concerned if you will the
tasklet the necessary clean-up of the descriptors is not handled.

Jon
Jon Hunter Jan. 15, 2020, 10:25 a.m. UTC | #4
On 15/01/2020 09:18, Jon Hunter wrote:
> 
> On 14/01/2020 21:02, Dmitry Osipenko wrote:
>> 14.01.2020 18:15, Jon Hunter пишет:
>>>
>>> On 12/01/2020 17:29, Dmitry Osipenko wrote:
>>>> The ISR tasklet could be kept scheduled after DMA transfer termination,
>>>> let's add synchronization callback which blocks until tasklet is finished.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>>  drivers/dma/tegra20-apb-dma.c | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
>>>> index 319f31d27014..664e9c5df3ba 100644
>>>> --- a/drivers/dma/tegra20-apb-dma.c
>>>> +++ b/drivers/dma/tegra20-apb-dma.c
>>>> @@ -798,6 +798,13 @@ static int tegra_dma_terminate_all(struct dma_chan *dc)
>>>>  	return 0;
>>>>  }
>>>>  
>>>> +static void tegra_dma_synchronize(struct dma_chan *dc)
>>>> +{
>>>> +	struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
>>>> +
>>>> +	tasklet_kill(&tdc->tasklet);
>>>> +}
>>>> +
>>>
>>> Wouldn't there need to be some clean-up here? If the tasklet is
>>> scheduled, seems that there would be some other house-keeping that needs
>>> to be done after killing it.
>>
>> I'm not seeing anything to clean-up, could you please clarify?
> 
> Clean-up with regard to the descriptors. I was concerned if you will the
> tasklet the necessary clean-up of the descriptors is not handled.

Ah I see that tasklet_kill, unlike tasklet_kill_immediate, does wait for
the tasklet to run if scheduled. OK, then this should be fine.

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

Patch
diff mbox series

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 319f31d27014..664e9c5df3ba 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -798,6 +798,13 @@  static int tegra_dma_terminate_all(struct dma_chan *dc)
 	return 0;
 }
 
+static void tegra_dma_synchronize(struct dma_chan *dc)
+{
+	struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
+
+	tasklet_kill(&tdc->tasklet);
+}
+
 static unsigned int tegra_dma_sg_bytes_xferred(struct tegra_dma_channel *tdc,
 					       struct tegra_dma_sg_req *sg_req)
 {
@@ -1506,6 +1513,7 @@  static int tegra_dma_probe(struct platform_device *pdev)
 	tdma->dma_dev.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
 	tdma->dma_dev.device_config = tegra_dma_slave_config;
 	tdma->dma_dev.device_terminate_all = tegra_dma_terminate_all;
+	tdma->dma_dev.device_synchronize = tegra_dma_synchronize;
 	tdma->dma_dev.device_tx_status = tegra_dma_tx_status;
 	tdma->dma_dev.device_issue_pending = tegra_dma_issue_pending;