All of lore.kernel.org
 help / color / mirror / Atom feed
From: <Tudor.Ambarus@microchip.com>
To: <Ludovic.Desroches@microchip.com>, <dan.j.williams@intel.com>,
	<vkoul@kernel.org>
Cc: <dmaengine@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<Tudor.Ambarus@microchip.com>
Subject: [PATCH 03/10] dmaengine: at_hdmac: Return err in case the chan is not free at alloc res time
Date: Thu, 23 Jan 2020 14:03:06 +0000	[thread overview]
Message-ID: <20200123140237.125799-3-tudor.ambarus@microchip.com> (raw)
In-Reply-To: <20200123140237.125799-1-tudor.ambarus@microchip.com>

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Having a list of descriptors allocated for the channel at
device_alloc_chan_resources() time is a sign for bad free usage.
Return err and add a debug message in case the channel is not
free from a previous use.

atchan->descs_allocated becomes useless, get rid of it. More,
drop the error message in atc_desc_get() because now it would
introduce an extra if statement. The callers of atc_desc_get()
already print error messages in case the callee fails, no one
is hurt.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/dma/at_hdmac.c      | 31 ++++++++-----------------------
 drivers/dma/at_hdmac_regs.h |  2 --
 2 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 301bae45cf8d..e17b75075904 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -146,17 +146,8 @@ static struct at_desc *atc_desc_get(struct at_dma_chan *atchan)
 		"scanned %u descriptors on freelist\n", i);
 
 	/* no more descriptor available in initial pool: create one more */
-	if (!ret) {
+	if (!ret)
 		ret = atc_alloc_descriptor(&atchan->chan_common, GFP_ATOMIC);
-		if (ret) {
-			spin_lock_irqsave(&atchan->lock, flags);
-			atchan->descs_allocated++;
-			spin_unlock_irqrestore(&atchan->lock, flags);
-		} else {
-			dev_err(chan2dev(&atchan->chan_common),
-					"not enough descriptors available\n");
-		}
-	}
 
 	return ret;
 }
@@ -1553,6 +1544,11 @@ static int atc_alloc_chan_resources(struct dma_chan *chan)
 		return -EIO;
 	}
 
+	if (!list_empty(&atchan->free_list)) {
+		dev_dbg(chan2dev(chan), "can't allocate channel resources (channel not freed from a previous use)\n");
+		return -EIO;
+	}
+
 	cfg = ATC_DEFAULT_CFG;
 
 	atslave = chan->private;
@@ -1568,11 +1564,6 @@ static int atc_alloc_chan_resources(struct dma_chan *chan)
 			cfg = atslave->cfg;
 	}
 
-	/* have we already been set up?
-	 * reconfigure channel but no need to reallocate descriptors */
-	if (!list_empty(&atchan->free_list))
-		return atchan->descs_allocated;
-
 	/* Allocate initial pool of descriptors */
 	for (i = 0; i < init_nr_desc_per_channel; i++) {
 		desc = atc_alloc_descriptor(chan, GFP_KERNEL);
@@ -1584,17 +1575,15 @@ static int atc_alloc_chan_resources(struct dma_chan *chan)
 		list_add_tail(&desc->desc_node, &atchan->free_list);
 	}
 
-	atchan->descs_allocated = i;
 	dma_cookie_init(chan);
 
 	/* channel parameters */
 	channel_writel(atchan, CFG, cfg);
 
 	dev_dbg(chan2dev(chan),
-		"alloc_chan_resources: allocated %d descriptors\n",
-		atchan->descs_allocated);
+		"alloc_chan_resources: allocated %d descriptors\n", i);
 
-	return atchan->descs_allocated;
+	return i;
 }
 
 /**
@@ -1608,9 +1597,6 @@ static void atc_free_chan_resources(struct dma_chan *chan)
 	struct at_desc		*desc, *_desc;
 	LIST_HEAD(list);
 
-	dev_dbg(chan2dev(chan), "free_chan_resources: (descs allocated=%u)\n",
-		atchan->descs_allocated);
-
 	/* ASSERT:  channel is idle */
 	BUG_ON(!list_empty(&atchan->active_list));
 	BUG_ON(!list_empty(&atchan->queue));
@@ -1623,7 +1609,6 @@ static void atc_free_chan_resources(struct dma_chan *chan)
 		dma_pool_free(atdma->dma_desc_pool, desc, desc->txd.phys);
 	}
 	list_splice_init(&atchan->free_list, &list);
-	atchan->descs_allocated = 0;
 	atchan->status = 0;
 
 	/*
diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
index fe8a5853ec49..397692e937b3 100644
--- a/drivers/dma/at_hdmac_regs.h
+++ b/drivers/dma/at_hdmac_regs.h
@@ -243,7 +243,6 @@ enum atc_status {
  * @active_list: list of descriptors dmaengine is being running on
  * @queue: list of descriptors ready to be submitted to engine
  * @free_list: list of descriptors usable by the channel
- * @descs_allocated: records the actual size of the descriptor pool
  */
 struct at_dma_chan {
 	struct dma_chan		chan_common;
@@ -264,7 +263,6 @@ struct at_dma_chan {
 	struct list_head	active_list;
 	struct list_head	queue;
 	struct list_head	free_list;
-	unsigned int		descs_allocated;
 };
 
 #define	channel_readl(atchan, name) \
-- 
2.23.0

  parent reply	other threads:[~2020-01-23 14:03 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-23 14:03 [PATCH 01/10] dmaengine: at_hdmac: Substitute kzalloc with kmalloc Tudor.Ambarus
2020-01-23 14:03 ` [PATCH 02/10] dmaengine: at_hdmac: Drop locking in at_hdmac_alloc_chan_resources() Tudor.Ambarus
2020-01-23 14:03 ` Tudor.Ambarus [this message]
2020-01-23 14:03 ` [PATCH 04/10] dmaengine: at_hdmac: Drop description for a not defined parameter Tudor.Ambarus
2020-01-23 14:03 ` [PATCH 05/10] dmaengine: at_hdmac: Switch atomic allocations to GFP_NOWAIT Tudor.Ambarus
2020-01-23 14:03 ` [PATCH 06/10] dmaengine: at_hdmac: Fix deadlocks Tudor.Ambarus
2020-01-23 14:03 ` [PATCH 07/10] dmaengine: at_xdmac: Drop always true check Tudor.Ambarus
2020-01-23 14:03 ` [PATCH 08/10] dmaengine: at_xdmac: Drop locking in at_xdmac_alloc_chan_resources() Tudor.Ambarus
2020-01-23 14:03 ` [PATCH 09/10] dmaengine: at_xdmac: GFP_KERNEL for user that can sleep Tudor.Ambarus
2020-01-23 14:03 ` [PATCH 10/10] dmaengine: at_xdmac: Fix locking in tasklet Tudor.Ambarus
2020-02-03  8:24 ` [PATCH 01/10] dmaengine: at_hdmac: Substitute kzalloc with kmalloc Ludovic Desroches
2020-02-25  5:58 ` Vinod Koul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200123140237.125799-3-tudor.ambarus@microchip.com \
    --to=tudor.ambarus@microchip.com \
    --cc=Ludovic.Desroches@microchip.com \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.