All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tudor Ambarus <tudor.ambarus@microchip.com>
To: <vkoul@kernel.org>, <peda@axentia.se>, <du@axentia.se>
Cc: <maciej.sosnowski@intel.com>, <nicolas.ferre@microchip.com>,
	<mripard@kernel.org>, <torfl6749@gmail.com>,
	<linux-kernel@vger.kernel.org>, <dmaengine@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	"Tudor Ambarus" <tudor.ambarus@microchip.com>
Subject: [PATCH v2 19/32] dmaengine: at_hdmac: Pass residue by address to avoid unnecessary implicit casts
Date: Tue, 25 Oct 2022 12:02:53 +0300	[thread overview]
Message-ID: <20221025090306.297886-20-tudor.ambarus@microchip.com> (raw)
In-Reply-To: <20221025090306.297886-1-tudor.ambarus@microchip.com>

struct dma_tx_state defines residue as u32. atc_get_bytes_left() returned
an int which could be either an error or the value of the residue. This
could cause problems if the controller supported a u32 buffer transfer size
and the u32 value was past the max int can hold. Our controller does not
support u32 buffer transfer size, but even so, improve the code and pass
the residue by address to avoid unnecessary implicit casts and make
atc_get_bytes_left() return 0 on success or -errno on errors.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/dma/at_hdmac.c | 54 +++++++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index fbfb207104e9..e2c46f32b284 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -293,7 +293,7 @@ static struct at_desc *atc_get_desc_by_cookie(struct at_dma_chan *atchan,
  * @current_len: the number of bytes left before reading CTRLA
  * @ctrla: the value of CTRLA
  */
-static inline int atc_calc_bytes_left(int current_len, u32 ctrla)
+static inline u32 atc_calc_bytes_left(u32 current_len, u32 ctrla)
 {
 	u32 btsize = (ctrla & ATC_BTSIZE_MAX);
 	u32 src_width = ATC_REG_TO_SRC_WIDTH(ctrla);
@@ -308,17 +308,20 @@ static inline int atc_calc_bytes_left(int current_len, u32 ctrla)
 }
 
 /**
- * atc_get_bytes_left - get the number of bytes residue for a cookie
+ * atc_get_bytes_left - get the number of bytes residue for a cookie.
+ * The residue is passed by address and updated on success.
  * @chan: DMA channel
  * @cookie: transaction identifier to check status of
+ * @residue: residue to be updated.
+ * Return 0 on success, -errono otherwise.
  */
-static int atc_get_bytes_left(struct dma_chan *chan, dma_cookie_t cookie)
+static int atc_get_bytes_left(struct dma_chan *chan, dma_cookie_t cookie,
+			      u32 *residue)
 {
 	struct at_dma_chan      *atchan = to_at_dma_chan(chan);
 	struct at_desc *desc_first = atc_first_active(atchan);
 	struct at_desc *desc;
-	int ret;
-	u32 ctrla, dscr;
+	u32 len, ctrla, dscr;
 	unsigned int i;
 
 	/*
@@ -333,7 +336,7 @@ static int atc_get_bytes_left(struct dma_chan *chan, dma_cookie_t cookie)
 		return desc->total_len;
 
 	/* cookie matches to the currently running transfer */
-	ret = desc_first->total_len;
+	len = desc_first->total_len;
 
 	if (desc_first->lli.dscr) {
 		/* hardware linked list transfer */
@@ -419,29 +422,31 @@ static int atc_get_bytes_left(struct dma_chan *chan, dma_cookie_t cookie)
 			return -ETIMEDOUT;
 
 		/* for the first descriptor we can be more accurate */
-		if (desc_first->lli.dscr == dscr)
-			return atc_calc_bytes_left(ret, ctrla);
+		if (desc_first->lli.dscr == dscr) {
+			*residue = atc_calc_bytes_left(len, ctrla);
+			return 0;
+		}
 
-		ret -= desc_first->len;
+		len -= desc_first->len;
 		list_for_each_entry(desc, &desc_first->tx_list, desc_node) {
 			if (desc->lli.dscr == dscr)
 				break;
 
-			ret -= desc->len;
+			len -= desc->len;
 		}
 
 		/*
 		 * For the current descriptor in the chain we can calculate
 		 * the remaining bytes using the channel's register.
 		 */
-		ret = atc_calc_bytes_left(ret, ctrla);
+		*residue = atc_calc_bytes_left(len, ctrla);
 	} else {
 		/* single transfer */
 		ctrla = channel_readl(atchan, CTRLA);
-		ret = atc_calc_bytes_left(ret, ctrla);
+		*residue = atc_calc_bytes_left(len, ctrla);
 	}
 
-	return ret;
+	return 0;
 }
 
 /**
@@ -1457,31 +1462,32 @@ atc_tx_status(struct dma_chan *chan,
 {
 	struct at_dma_chan	*atchan = to_at_dma_chan(chan);
 	unsigned long		flags;
-	enum dma_status		ret;
-	int bytes = 0;
+	enum dma_status		dma_status;
+	u32 residue;
+	int ret;
 
-	ret = dma_cookie_status(chan, cookie, txstate);
-	if (ret == DMA_COMPLETE || !txstate)
-		return ret;
+	dma_status = dma_cookie_status(chan, cookie, txstate);
+	if (dma_status == DMA_COMPLETE || !txstate)
+		return dma_status;
 
 	spin_lock_irqsave(&atchan->lock, flags);
 
 	/*  Get number of bytes left in the active transactions */
-	bytes = atc_get_bytes_left(chan, cookie);
+	ret = atc_get_bytes_left(chan, cookie, &residue);
 
 	spin_unlock_irqrestore(&atchan->lock, flags);
 
-	if (unlikely(bytes < 0)) {
+	if (unlikely(ret < 0)) {
 		dev_vdbg(chan2dev(chan), "get residual bytes error\n");
 		return DMA_ERROR;
 	} else {
-		dma_set_residue(txstate, bytes);
+		dma_set_residue(txstate, residue);
 	}
 
-	dev_vdbg(chan2dev(chan), "tx_status %d: cookie = %d residue = %d\n",
-		 ret, cookie, bytes);
+	dev_vdbg(chan2dev(chan), "tx_status %d: cookie = %d residue = %u\n",
+		 dma_status, cookie, residue);
 
-	return ret;
+	return dma_status;
 }
 
 /**
-- 
2.25.1


WARNING: multiple messages have this Message-ID (diff)
From: Tudor Ambarus <tudor.ambarus@microchip.com>
To: <vkoul@kernel.org>, <peda@axentia.se>, <du@axentia.se>
Cc: Tudor Ambarus <tudor.ambarus@microchip.com>,
	maciej.sosnowski@intel.com, linux-kernel@vger.kernel.org,
	mripard@kernel.org, linux-arm-kernel@lists.infradead.org,
	dmaengine@vger.kernel.org, torfl6749@gmail.com
Subject: [PATCH v2 19/32] dmaengine: at_hdmac: Pass residue by address to avoid unnecessary implicit casts
Date: Tue, 25 Oct 2022 12:02:53 +0300	[thread overview]
Message-ID: <20221025090306.297886-20-tudor.ambarus@microchip.com> (raw)
In-Reply-To: <20221025090306.297886-1-tudor.ambarus@microchip.com>

struct dma_tx_state defines residue as u32. atc_get_bytes_left() returned
an int which could be either an error or the value of the residue. This
could cause problems if the controller supported a u32 buffer transfer size
and the u32 value was past the max int can hold. Our controller does not
support u32 buffer transfer size, but even so, improve the code and pass
the residue by address to avoid unnecessary implicit casts and make
atc_get_bytes_left() return 0 on success or -errno on errors.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/dma/at_hdmac.c | 54 +++++++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index fbfb207104e9..e2c46f32b284 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -293,7 +293,7 @@ static struct at_desc *atc_get_desc_by_cookie(struct at_dma_chan *atchan,
  * @current_len: the number of bytes left before reading CTRLA
  * @ctrla: the value of CTRLA
  */
-static inline int atc_calc_bytes_left(int current_len, u32 ctrla)
+static inline u32 atc_calc_bytes_left(u32 current_len, u32 ctrla)
 {
 	u32 btsize = (ctrla & ATC_BTSIZE_MAX);
 	u32 src_width = ATC_REG_TO_SRC_WIDTH(ctrla);
@@ -308,17 +308,20 @@ static inline int atc_calc_bytes_left(int current_len, u32 ctrla)
 }
 
 /**
- * atc_get_bytes_left - get the number of bytes residue for a cookie
+ * atc_get_bytes_left - get the number of bytes residue for a cookie.
+ * The residue is passed by address and updated on success.
  * @chan: DMA channel
  * @cookie: transaction identifier to check status of
+ * @residue: residue to be updated.
+ * Return 0 on success, -errono otherwise.
  */
-static int atc_get_bytes_left(struct dma_chan *chan, dma_cookie_t cookie)
+static int atc_get_bytes_left(struct dma_chan *chan, dma_cookie_t cookie,
+			      u32 *residue)
 {
 	struct at_dma_chan      *atchan = to_at_dma_chan(chan);
 	struct at_desc *desc_first = atc_first_active(atchan);
 	struct at_desc *desc;
-	int ret;
-	u32 ctrla, dscr;
+	u32 len, ctrla, dscr;
 	unsigned int i;
 
 	/*
@@ -333,7 +336,7 @@ static int atc_get_bytes_left(struct dma_chan *chan, dma_cookie_t cookie)
 		return desc->total_len;
 
 	/* cookie matches to the currently running transfer */
-	ret = desc_first->total_len;
+	len = desc_first->total_len;
 
 	if (desc_first->lli.dscr) {
 		/* hardware linked list transfer */
@@ -419,29 +422,31 @@ static int atc_get_bytes_left(struct dma_chan *chan, dma_cookie_t cookie)
 			return -ETIMEDOUT;
 
 		/* for the first descriptor we can be more accurate */
-		if (desc_first->lli.dscr == dscr)
-			return atc_calc_bytes_left(ret, ctrla);
+		if (desc_first->lli.dscr == dscr) {
+			*residue = atc_calc_bytes_left(len, ctrla);
+			return 0;
+		}
 
-		ret -= desc_first->len;
+		len -= desc_first->len;
 		list_for_each_entry(desc, &desc_first->tx_list, desc_node) {
 			if (desc->lli.dscr == dscr)
 				break;
 
-			ret -= desc->len;
+			len -= desc->len;
 		}
 
 		/*
 		 * For the current descriptor in the chain we can calculate
 		 * the remaining bytes using the channel's register.
 		 */
-		ret = atc_calc_bytes_left(ret, ctrla);
+		*residue = atc_calc_bytes_left(len, ctrla);
 	} else {
 		/* single transfer */
 		ctrla = channel_readl(atchan, CTRLA);
-		ret = atc_calc_bytes_left(ret, ctrla);
+		*residue = atc_calc_bytes_left(len, ctrla);
 	}
 
-	return ret;
+	return 0;
 }
 
 /**
@@ -1457,31 +1462,32 @@ atc_tx_status(struct dma_chan *chan,
 {
 	struct at_dma_chan	*atchan = to_at_dma_chan(chan);
 	unsigned long		flags;
-	enum dma_status		ret;
-	int bytes = 0;
+	enum dma_status		dma_status;
+	u32 residue;
+	int ret;
 
-	ret = dma_cookie_status(chan, cookie, txstate);
-	if (ret == DMA_COMPLETE || !txstate)
-		return ret;
+	dma_status = dma_cookie_status(chan, cookie, txstate);
+	if (dma_status == DMA_COMPLETE || !txstate)
+		return dma_status;
 
 	spin_lock_irqsave(&atchan->lock, flags);
 
 	/*  Get number of bytes left in the active transactions */
-	bytes = atc_get_bytes_left(chan, cookie);
+	ret = atc_get_bytes_left(chan, cookie, &residue);
 
 	spin_unlock_irqrestore(&atchan->lock, flags);
 
-	if (unlikely(bytes < 0)) {
+	if (unlikely(ret < 0)) {
 		dev_vdbg(chan2dev(chan), "get residual bytes error\n");
 		return DMA_ERROR;
 	} else {
-		dma_set_residue(txstate, bytes);
+		dma_set_residue(txstate, residue);
 	}
 
-	dev_vdbg(chan2dev(chan), "tx_status %d: cookie = %d residue = %d\n",
-		 ret, cookie, bytes);
+	dev_vdbg(chan2dev(chan), "tx_status %d: cookie = %d residue = %u\n",
+		 dma_status, cookie, residue);
 
-	return ret;
+	return dma_status;
 }
 
 /**
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2022-10-25  9:06 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-25  9:02 [PATCH v2 00/32] dmaengine: at_hdmac: Fix concurrency bugs and then convert to virt-dma Tudor Ambarus
2022-10-25  9:02 ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 01/32] dmaengine: at_hdmac: Fix at_lli struct definition Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 02/32] dmaengine: at_hdmac: Don't start transactions at tx_submit level Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 03/32] dmaengine: at_hdmac: Start transfer for cyclic channels in issue_pending Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 04/32] dmaengine: at_hdmac: Fix premature completion of desc " Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 05/32] dmaengine: at_hdmac: Do not call the complete callback on device_terminate_all Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 06/32] dmaengine: at_hdmac: Protect atchan->status with the channel lock Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 07/32] dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all() Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 08/32] dmaengine: at_hdmac: Fix concurrency over descriptor Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 09/32] dmaengine: at_hdmac: Free the memset buf without holding the chan lock Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 10/32] dmaengine: at_hdmac: Fix concurrency over the active list Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 11/32] dmaengine: at_hdmac: Fix descriptor handling when issuing it to hardware Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 12/32] dmaengine: at_hdmac: Fix completion of unissued descriptor in case of errors Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 13/32] dmaengine: at_hdmac: Don't allow CPU to reorder channel enable Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 14/32] dmaengine: at_hdmac: Fix impossible condition Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 15/32] dmaengine: at_hdmac: Check return code of dma_async_device_register Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 16/32] dmaengine: at_hdmac: Do not print messages on console while holding the lock Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 17/32] dmaengine: at_hdmac: Return dma_cookie_status()'s ret code when txstate is NULL Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 18/32] dmaengine: at_hdmac: Remove superfluous cast Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` Tudor Ambarus [this message]
2022-10-25  9:02   ` [PATCH v2 19/32] dmaengine: at_hdmac: Pass residue by address to avoid unnecessary implicit casts Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 20/32] dmaengine: at_hdmac: s/atc_get_bytes_left/atc_get_residue Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 21/32] dmaengine: at_hdmac: Introduce atc_get_llis_residue() Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 22/32] dmaengine: at_hdmac: Use devm_kzalloc() and struct_size() Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 23/32] dmaengine: at_hdmac: Use devm_platform_ioremap_resource Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 24/32] dmaengine: at_hdmac: Use devm_clk_get() Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:02 ` [PATCH v2 25/32] dmaengine: at_hdmac: Use pm_ptr() Tudor Ambarus
2022-10-25  9:02   ` Tudor Ambarus
2022-10-25  9:03 ` [PATCH v2 26/32] dmaengine: at_hdmac: Set include entries in alphabetic order Tudor Ambarus
2022-10-25  9:03   ` Tudor Ambarus
2022-10-25  9:03 ` [PATCH v2 27/32] dmaengine: at_hdmac: Keep register definitions and structures private to at_hdmac.c Tudor Ambarus
2022-10-25  9:03   ` Tudor Ambarus
2022-10-25  9:03 ` [PATCH v2 28/32] dmaengine: at_hdmac: Use bitfield access macros Tudor Ambarus
2022-10-25  9:03   ` Tudor Ambarus
2022-10-25  9:03 ` [PATCH v2 29/32] dmaengine: at_hdmac: Rename "dma_common" to "dma_device" Tudor Ambarus
2022-10-25  9:03   ` Tudor Ambarus
2022-10-25  9:03 ` [PATCH v2 30/32] dmaengine: at_hdmac: Rename "chan_common" to "dma_chan" Tudor Ambarus
2022-10-25  9:03   ` Tudor Ambarus
2022-10-25  9:03 ` [PATCH v2 31/32] dmaengine: at_hdmac: Remove unused member of at_dma_chan Tudor Ambarus
2022-10-25  9:03   ` Tudor Ambarus
2022-10-25  9:03 ` [PATCH v2 32/32] dmaengine: at_hdmac: Convert driver to use virt-dma Tudor Ambarus
2022-10-25  9:03   ` Tudor Ambarus
2022-10-28 12:55   ` Nicolas Ferre
2022-10-28 12:55     ` Nicolas Ferre
2022-10-25  9:06 ` [PATCH v2 00/32] dmaengine: at_hdmac: Fix concurrency bugs and then convert to virt-dma Tudor.Ambarus
2022-10-25  9:06   ` Tudor.Ambarus
2022-10-28 13:00 ` Nicolas Ferre
2022-10-28 13:00   ` Nicolas Ferre
2022-11-04 13:22 ` Vinod Koul
2022-11-04 13:22   ` Vinod Koul
2022-11-04 15:14   ` Tudor.Ambarus
2022-11-04 15:14     ` Tudor.Ambarus
2022-11-04 15:21     ` Tudor.Ambarus
2022-11-04 15:21       ` Tudor.Ambarus
2022-11-04 17:59       ` Vinod Koul
2022-11-04 17:59         ` Vinod Koul
2022-11-07  4:33         ` Vinod Koul
2022-11-07  4:33           ` Vinod Koul
2022-11-07  9:00           ` Tudor.Ambarus
2022-11-07  9:00             ` Tudor.Ambarus

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=20221025090306.297886-20-tudor.ambarus@microchip.com \
    --to=tudor.ambarus@microchip.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=du@axentia.se \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maciej.sosnowski@intel.com \
    --cc=mripard@kernel.org \
    --cc=nicolas.ferre@microchip.com \
    --cc=peda@axentia.se \
    --cc=torfl6749@gmail.com \
    --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.