dmaengine Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] dmaengine: ti: k3-udma: Fixes for alloc_chan_resources
@ 2020-05-27  7:06 Peter Ujfalusi
  2020-05-27  7:06 ` [PATCH 1/2] dmaengine: ti: k3-udma: Fix cleanup code " Peter Ujfalusi
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Peter Ujfalusi @ 2020-05-27  7:06 UTC (permalink / raw)
  To: vkoul; +Cc: dmaengine, dan.j.williams

Hi,

It turned out that udma_stop() can not be used to stop the channel which was
left enabled during boot (missing cleanup in early boot) since it would initiate
teardown. This is not supported on non configured channels.
Simply reset the running channel instead fixes the issue.

While looking at this issue I have noticed that the cleanup path misses
resources if the error happens early.

Regards,
Peter
---
Peter Ujfalusi (2):
  dmaengine: ti: k3-udma: Fix cleanup code for alloc_chan_resources
  dmaengine: ti: k3-udma: Fix the running channel handling in
    alloc_chan_resources

 drivers/dma/ti/k3-udma.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] dmaengine: ti: k3-udma: Fix cleanup code for alloc_chan_resources
  2020-05-27  7:06 [PATCH 0/2] dmaengine: ti: k3-udma: Fixes for alloc_chan_resources Peter Ujfalusi
@ 2020-05-27  7:06 ` Peter Ujfalusi
  2020-05-27  7:06 ` [PATCH 2/2] dmaengine: ti: k3-udma: Fix the running channel handling in alloc_chan_resources Peter Ujfalusi
  2020-06-16 15:56 ` [PATCH 0/2] dmaengine: ti: k3-udma: Fixes for alloc_chan_resources Vinod Koul
  2 siblings, 0 replies; 4+ messages in thread
From: Peter Ujfalusi @ 2020-05-27  7:06 UTC (permalink / raw)
  To: vkoul; +Cc: dmaengine, dan.j.williams

Some of the earlier errors should be sent to the error cleanup path to
make sure that the uchan struct is reset, the dma_pool (if allocated) is
released and memcpy channel pairs are released in a correct way.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/ti/k3-udma.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
index 4bc470eb4143..502d7f2edd8a 100644
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -1753,7 +1753,8 @@ static int udma_alloc_chan_resources(struct dma_chan *chan)
 			dev_err(ud->ddev.dev,
 				"Descriptor pool allocation failed\n");
 			uc->use_dma_pool = false;
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto err_cleanup;
 		}
 	}
 
@@ -1773,16 +1774,18 @@ static int udma_alloc_chan_resources(struct dma_chan *chan)
 
 		ret = udma_get_chan_pair(uc);
 		if (ret)
-			return ret;
+			goto err_cleanup;
 
 		ret = udma_alloc_tx_resources(uc);
-		if (ret)
-			return ret;
+		if (ret) {
+			udma_put_rchan(uc);
+			goto err_cleanup;
+		}
 
 		ret = udma_alloc_rx_resources(uc);
 		if (ret) {
 			udma_free_tx_resources(uc);
-			return ret;
+			goto err_cleanup;
 		}
 
 		uc->config.src_thread = ud->psil_base + uc->tchan->id;
@@ -1800,10 +1803,8 @@ static int udma_alloc_chan_resources(struct dma_chan *chan)
 			uc->id);
 
 		ret = udma_alloc_tx_resources(uc);
-		if (ret) {
-			uc->config.remote_thread_id = -1;
-			return ret;
-		}
+		if (ret)
+			goto err_cleanup;
 
 		uc->config.src_thread = ud->psil_base + uc->tchan->id;
 		uc->config.dst_thread = uc->config.remote_thread_id;
@@ -1820,10 +1821,8 @@ static int udma_alloc_chan_resources(struct dma_chan *chan)
 			uc->id);
 
 		ret = udma_alloc_rx_resources(uc);
-		if (ret) {
-			uc->config.remote_thread_id = -1;
-			return ret;
-		}
+		if (ret)
+			goto err_cleanup;
 
 		uc->config.src_thread = uc->config.remote_thread_id;
 		uc->config.dst_thread = (ud->psil_base + uc->rchan->id) |
@@ -1838,7 +1837,9 @@ static int udma_alloc_chan_resources(struct dma_chan *chan)
 		/* Can not happen */
 		dev_err(uc->ud->dev, "%s: chan%d invalid direction (%u)\n",
 			__func__, uc->id, uc->config.dir);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_cleanup;
+
 	}
 
 	/* check if the channel configuration was successful */
@@ -1919,7 +1920,7 @@ static int udma_alloc_chan_resources(struct dma_chan *chan)
 err_res_free:
 	udma_free_tx_resources(uc);
 	udma_free_rx_resources(uc);
-
+err_cleanup:
 	udma_reset_uchan(uc);
 
 	if (uc->use_dma_pool) {
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] dmaengine: ti: k3-udma: Fix the running channel handling in alloc_chan_resources
  2020-05-27  7:06 [PATCH 0/2] dmaengine: ti: k3-udma: Fixes for alloc_chan_resources Peter Ujfalusi
  2020-05-27  7:06 ` [PATCH 1/2] dmaengine: ti: k3-udma: Fix cleanup code " Peter Ujfalusi
@ 2020-05-27  7:06 ` Peter Ujfalusi
  2020-06-16 15:56 ` [PATCH 0/2] dmaengine: ti: k3-udma: Fixes for alloc_chan_resources Vinod Koul
  2 siblings, 0 replies; 4+ messages in thread
From: Peter Ujfalusi @ 2020-05-27  7:06 UTC (permalink / raw)
  To: vkoul; +Cc: dmaengine, dan.j.williams

In the unlikely case when the channel is running (RT enabled) during
alloc_chan_resources then we should use udma_reset_chan() and not
udma_stop() as the later is trying to initiate a teardown on the channel,
which is not valid at this point.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/ti/k3-udma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
index 502d7f2edd8a..0d5fb154b8e2 100644
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -1848,7 +1848,7 @@ static int udma_alloc_chan_resources(struct dma_chan *chan)
 
 	if (udma_is_chan_running(uc)) {
 		dev_warn(ud->dev, "chan%d: is running!\n", uc->id);
-		udma_stop(uc);
+		udma_reset_chan(uc, false);
 		if (udma_is_chan_running(uc)) {
 			dev_err(ud->dev, "chan%d: won't stop!\n", uc->id);
 			ret = -EBUSY;
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 0/2] dmaengine: ti: k3-udma: Fixes for alloc_chan_resources
  2020-05-27  7:06 [PATCH 0/2] dmaengine: ti: k3-udma: Fixes for alloc_chan_resources Peter Ujfalusi
  2020-05-27  7:06 ` [PATCH 1/2] dmaengine: ti: k3-udma: Fix cleanup code " Peter Ujfalusi
  2020-05-27  7:06 ` [PATCH 2/2] dmaengine: ti: k3-udma: Fix the running channel handling in alloc_chan_resources Peter Ujfalusi
@ 2020-06-16 15:56 ` Vinod Koul
  2 siblings, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2020-06-16 15:56 UTC (permalink / raw)
  To: Peter Ujfalusi; +Cc: dmaengine, dan.j.williams

On 27-05-20, 10:06, Peter Ujfalusi wrote:
> Hi,
> 
> It turned out that udma_stop() can not be used to stop the channel which was
> left enabled during boot (missing cleanup in early boot) since it would initiate
> teardown. This is not supported on non configured channels.
> Simply reset the running channel instead fixes the issue.
> 
> While looking at this issue I have noticed that the cleanup path misses
> resources if the error happens early.

Applied, thanks

-- 
~Vinod

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-27  7:06 [PATCH 0/2] dmaengine: ti: k3-udma: Fixes for alloc_chan_resources Peter Ujfalusi
2020-05-27  7:06 ` [PATCH 1/2] dmaengine: ti: k3-udma: Fix cleanup code " Peter Ujfalusi
2020-05-27  7:06 ` [PATCH 2/2] dmaengine: ti: k3-udma: Fix the running channel handling in alloc_chan_resources Peter Ujfalusi
2020-06-16 15:56 ` [PATCH 0/2] dmaengine: ti: k3-udma: Fixes for alloc_chan_resources Vinod Koul

dmaengine Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dmaengine/0 dmaengine/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dmaengine dmaengine/ https://lore.kernel.org/dmaengine \
		dmaengine@vger.kernel.org
	public-inbox-index dmaengine

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.dmaengine


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git