| Message ID | 20200202222854.18409-13-digetx@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | NVIDIA Tegra APB DMA driver fixes and improvements | expand |
On 02/02/2020 22:28, Dmitry Osipenko wrote: > The pending_sg_req list can't ever be empty because: > > 1. If it was empty, then handle_cont_sngl_cycle_dma_done() shall crash > before of handle_continuous_head_request() invocation. > > 2. The handle_cont_sngl_cycle_dma_done() can't happen after stopping DMA. By this you mean calling terminate_all? > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/dma/tegra20-apb-dma.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c > index 62d181bd5e62..c7dc27ef1856 100644 > --- a/drivers/dma/tegra20-apb-dma.c > +++ b/drivers/dma/tegra20-apb-dma.c > @@ -564,12 +564,6 @@ static bool handle_continuous_head_request(struct tegra_dma_channel *tdc, > { > struct tegra_dma_sg_req *hsgreq; > > - if (list_empty(&tdc->pending_sg_req)) { > - dev_err(tdc2dev(tdc), "DMA is running without req\n"); > - tegra_dma_stop(tdc); > - return false; > - } > - > /* > * Check that head req on list should be in flight. > * If it is not in flight then abort transfer as > There is also a list_empty() check in tdc_configure_next_head_desc() which is also redundant and could be removed here as well. Jon
04.02.2020 14:52, Jon Hunter пишет: > > On 02/02/2020 22:28, Dmitry Osipenko wrote: >> The pending_sg_req list can't ever be empty because: >> >> 1. If it was empty, then handle_cont_sngl_cycle_dma_done() shall crash >> before of handle_continuous_head_request() invocation. >> >> 2. The handle_cont_sngl_cycle_dma_done() can't happen after stopping DMA. > > By this you mean calling terminate_all? Yes, and also the handle_continuous_head_request() itself because it stops DMA on error, which clears interrupt status, and thus, ISR handle returns IRQ_NONE without handling next interrupt. >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> drivers/dma/tegra20-apb-dma.c | 6 ------ >> 1 file changed, 6 deletions(-) >> >> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c >> index 62d181bd5e62..c7dc27ef1856 100644 >> --- a/drivers/dma/tegra20-apb-dma.c >> +++ b/drivers/dma/tegra20-apb-dma.c >> @@ -564,12 +564,6 @@ static bool handle_continuous_head_request(struct tegra_dma_channel *tdc, >> { >> struct tegra_dma_sg_req *hsgreq; >> >> - if (list_empty(&tdc->pending_sg_req)) { >> - dev_err(tdc2dev(tdc), "DMA is running without req\n"); >> - tegra_dma_stop(tdc); >> - return false; >> - } >> - >> /* >> * Check that head req on list should be in flight. >> * If it is not in flight then abort transfer as >> > > There is also a list_empty() check in tdc_configure_next_head_desc() > which is also redundant and could be removed here as well. Good catch :) I'll squash all these list_empty() removals into a single patch.
diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c index 62d181bd5e62..c7dc27ef1856 100644 --- a/drivers/dma/tegra20-apb-dma.c +++ b/drivers/dma/tegra20-apb-dma.c @@ -564,12 +564,6 @@ static bool handle_continuous_head_request(struct tegra_dma_channel *tdc, { struct tegra_dma_sg_req *hsgreq; - if (list_empty(&tdc->pending_sg_req)) { - dev_err(tdc2dev(tdc), "DMA is running without req\n"); - tegra_dma_stop(tdc); - return false; - } - /* * Check that head req on list should be in flight. * If it is not in flight then abort transfer as
The pending_sg_req list can't ever be empty because: 1. If it was empty, then handle_cont_sngl_cycle_dma_done() shall crash before of handle_continuous_head_request() invocation. 2. The handle_cont_sngl_cycle_dma_done() can't happen after stopping DMA. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/dma/tegra20-apb-dma.c | 6 ------ 1 file changed, 6 deletions(-)