diff mbox

[v3,4/4] dmaengine: xilinx_vdma: Use readl_poll_timeout instead of do while loop's

Message ID 1456495434-11722-4-git-send-email-appanad@xilinx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Appana Durga Kedareswara rao Feb. 26, 2016, 2:03 p.m. UTC
It is sometimes necessary to poll a memory-mapped register until its
value satisfies some condition use convenience macros
that do this instead of do while loop's.

This patch updates the same in the driver.

Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v3:
---> removed patch dmaengine: xilinx_vdma: Improve channel idle checking
from the series as it is not a valid patch.
---> Added this patch.
Changes for v2:
---> splitted the changes into multiple patches.

 drivers/dma/xilinx/xilinx_vdma.c | 46 +++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

Comments

Vinod Koul March 3, 2016, 3:29 p.m. UTC | #1
On Fri, Feb 26, 2016 at 07:33:54PM +0530, Kedareswara rao Appana wrote:

>  static void xilinx_vdma_halt(struct xilinx_vdma_chan *chan)
>  {
> -	int loop = XILINX_VDMA_LOOP_COUNT;
> +	int err = 0;
> +	u32 val;
>  
>  	vdma_ctrl_clr(chan, XILINX_VDMA_REG_DMACR, XILINX_VDMA_DMACR_RUNSTOP);
>  
>  	/* Wait for the hardware to halt */
> -	do {
> -		if (vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR) &
> -		    XILINX_VDMA_DMASR_HALTED)
> -			break;
> -	} while (loop--);
> +	err = xilinx_vdma_poll_timeout(chan, XILINX_VDMA_REG_DMASR, val,
> +				      (val & XILINX_VDMA_DMASR_HALTED), 0,
> +				      XILINX_VDMA_LOOP_COUNT);
>  
> -	if (!loop) {
> +	if (err) {
>  		dev_err(chan->dev, "Cannot stop channel %p: %x\n",
>  			chan, vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR));
>  		chan->err = true;
> @@ -576,18 +579,17 @@ static void xilinx_vdma_halt(struct xilinx_vdma_chan *chan)
>   */
>  static void xilinx_vdma_start(struct xilinx_vdma_chan *chan)
>  {
> -	int loop = XILINX_VDMA_LOOP_COUNT;
> +	int err = 0;

why is this initialization required here and other places?

> +	u32 val;
>  
>  	vdma_ctrl_set(chan, XILINX_VDMA_REG_DMACR, XILINX_VDMA_DMACR_RUNSTOP);
>  
>  	/* Wait for the hardware to start */
> -	do {
> -		if (!(vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR) &
> -		      XILINX_VDMA_DMASR_HALTED))
> -			break;
> -	} while (loop--);
> +	err = xilinx_vdma_poll_timeout(chan, XILINX_VDMA_REG_DMASR, val,
> +				      !(val & XILINX_VDMA_DMASR_HALTED), 0,
> +				      XILINX_VDMA_LOOP_COUNT);
>  
> -	if (!loop) {
> +	if (err) {
>  		dev_err(chan->dev, "Cannot start channel %p: %x\n",
>  			chan, vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR));
>  
> @@ -754,21 +756,17 @@ static void xilinx_vdma_complete_descriptor(struct xilinx_vdma_chan *chan)
>   */
>  static int xilinx_vdma_reset(struct xilinx_vdma_chan *chan)
>  {
> -	int loop = XILINX_VDMA_LOOP_COUNT;
> +	int err = 0;
>  	u32 tmp;
>  
>  	vdma_ctrl_set(chan, XILINX_VDMA_REG_DMACR, XILINX_VDMA_DMACR_RESET);
>  
> -	tmp = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR) &
> -		XILINX_VDMA_DMACR_RESET;
> -
>  	/* Wait for the hardware to finish reset */
> -	do {
> -		tmp = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR) &
> -			XILINX_VDMA_DMACR_RESET;
> -	} while (loop-- && tmp);
> +	err = xilinx_vdma_poll_timeout(chan, XILINX_VDMA_REG_DMACR, tmp,
> +				      !(tmp & XILINX_VDMA_DMACR_RESET), 0,
> +				      XILINX_VDMA_LOOP_COUNT);
>  
> -	if (!loop) {
> +	if (err) {
Appana Durga Kedareswara rao March 3, 2016, 5:12 p.m. UTC | #2
Hi Vinod,


> -----Original Message-----
> From: dmaengine-owner@vger.kernel.org [mailto:dmaengine-
> owner@vger.kernel.org] On Behalf Of Vinod Koul
> Sent: Thursday, March 03, 2016 9:00 PM
> To: Appana Durga Kedareswara Rao
> Cc: dan.j.williams@intel.com; Michal Simek; Soren Brinkmann; Appana Durga
> Kedareswara Rao; moritz.fischer@ettus.com;
> laurent.pinchart@ideasonboard.com; luis@debethencourt.com; Anirudha
> Sarangi; dmaengine@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 4/4] dmaengine: xilinx_vdma: Use readl_poll_timeout
> instead of do while loop's
> 
> On Fri, Feb 26, 2016 at 07:33:54PM +0530, Kedareswara rao Appana wrote:
> 
> >  static void xilinx_vdma_halt(struct xilinx_vdma_chan *chan)  {
> > -	int loop = XILINX_VDMA_LOOP_COUNT;
> > +	int err = 0;
> > +	u32 val;
> >
> >  	vdma_ctrl_clr(chan, XILINX_VDMA_REG_DMACR,
> > XILINX_VDMA_DMACR_RUNSTOP);
> >
> >  	/* Wait for the hardware to halt */
> > -	do {
> > -		if (vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR) &
> > -		    XILINX_VDMA_DMASR_HALTED)
> > -			break;
> > -	} while (loop--);
> > +	err = xilinx_vdma_poll_timeout(chan, XILINX_VDMA_REG_DMASR, val,
> > +				      (val & XILINX_VDMA_DMASR_HALTED), 0,
> > +				      XILINX_VDMA_LOOP_COUNT);
> >
> > -	if (!loop) {
> > +	if (err) {
> >  		dev_err(chan->dev, "Cannot stop channel %p: %x\n",
> >  			chan, vdma_ctrl_read(chan,
> XILINX_VDMA_REG_DMASR));
> >  		chan->err = true;
> > @@ -576,18 +579,17 @@ static void xilinx_vdma_halt(struct
> xilinx_vdma_chan *chan)
> >   */
> >  static void xilinx_vdma_start(struct xilinx_vdma_chan *chan)  {
> > -	int loop = XILINX_VDMA_LOOP_COUNT;
> > +	int err = 0;
> 
> why is this initialization required here and other places?

Yes initialization is not required on the other mail you said you already applied this patch series.
Will send a separate patch to fix it.

Regards,
Kedar.

> 
> > +	u32 val;
> >
> >  	vdma_ctrl_set(chan, XILINX_VDMA_REG_DMACR,
> > XILINX_VDMA_DMACR_RUNSTOP);
> >
> >  	/* Wait for the hardware to start */
> > -	do {
> > -		if (!(vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR) &
> > -		      XILINX_VDMA_DMASR_HALTED))
> > -			break;
> > -	} while (loop--);
> > +	err = xilinx_vdma_poll_timeout(chan, XILINX_VDMA_REG_DMASR, val,
> > +				      !(val & XILINX_VDMA_DMASR_HALTED), 0,
> > +				      XILINX_VDMA_LOOP_COUNT);
> >
> > -	if (!loop) {
> > +	if (err) {
> >  		dev_err(chan->dev, "Cannot start channel %p: %x\n",
> >  			chan, vdma_ctrl_read(chan,
> XILINX_VDMA_REG_DMASR));
> >
> > @@ -754,21 +756,17 @@ static void xilinx_vdma_complete_descriptor(struct
> xilinx_vdma_chan *chan)
> >   */
> >  static int xilinx_vdma_reset(struct xilinx_vdma_chan *chan)  {
> > -	int loop = XILINX_VDMA_LOOP_COUNT;
> > +	int err = 0;
> >  	u32 tmp;
> >
> >  	vdma_ctrl_set(chan, XILINX_VDMA_REG_DMACR,
> XILINX_VDMA_DMACR_RESET);
> >
> > -	tmp = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR) &
> > -		XILINX_VDMA_DMACR_RESET;
> > -
> >  	/* Wait for the hardware to finish reset */
> > -	do {
> > -		tmp = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR) &
> > -			XILINX_VDMA_DMACR_RESET;
> > -	} while (loop-- && tmp);
> > +	err = xilinx_vdma_poll_timeout(chan, XILINX_VDMA_REG_DMACR, tmp,
> > +				      !(tmp & XILINX_VDMA_DMACR_RESET), 0,
> > +				      XILINX_VDMA_LOOP_COUNT);
> >
> > -	if (!loop) {
> > +	if (err) {
> 
> 
> 
> --
> ~Vinod
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body
> of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/dma/xilinx/xilinx_vdma.c b/drivers/dma/xilinx/xilinx_vdma.c
index 70b2b32..bc2ca45 100644
--- a/drivers/dma/xilinx/xilinx_vdma.c
+++ b/drivers/dma/xilinx/xilinx_vdma.c
@@ -28,6 +28,7 @@ 
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of_dma.h>
@@ -254,6 +255,9 @@  struct xilinx_vdma_device {
 	container_of(chan, struct xilinx_vdma_chan, common)
 #define to_vdma_tx_descriptor(tx) \
 	container_of(tx, struct xilinx_vdma_tx_descriptor, async_tx)
+#define xilinx_vdma_poll_timeout(chan, reg, val, cond, delay_us, timeout_us) \
+	readl_poll_timeout(chan->xdev->regs + chan->ctrl_offset + reg, val, \
+			   cond, delay_us, timeout_us)
 
 /* IO accessors */
 static inline u32 vdma_read(struct xilinx_vdma_chan *chan, u32 reg)
@@ -550,18 +554,17 @@  static bool xilinx_vdma_is_idle(struct xilinx_vdma_chan *chan)
  */
 static void xilinx_vdma_halt(struct xilinx_vdma_chan *chan)
 {
-	int loop = XILINX_VDMA_LOOP_COUNT;
+	int err = 0;
+	u32 val;
 
 	vdma_ctrl_clr(chan, XILINX_VDMA_REG_DMACR, XILINX_VDMA_DMACR_RUNSTOP);
 
 	/* Wait for the hardware to halt */
-	do {
-		if (vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR) &
-		    XILINX_VDMA_DMASR_HALTED)
-			break;
-	} while (loop--);
+	err = xilinx_vdma_poll_timeout(chan, XILINX_VDMA_REG_DMASR, val,
+				      (val & XILINX_VDMA_DMASR_HALTED), 0,
+				      XILINX_VDMA_LOOP_COUNT);
 
-	if (!loop) {
+	if (err) {
 		dev_err(chan->dev, "Cannot stop channel %p: %x\n",
 			chan, vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR));
 		chan->err = true;
@@ -576,18 +579,17 @@  static void xilinx_vdma_halt(struct xilinx_vdma_chan *chan)
  */
 static void xilinx_vdma_start(struct xilinx_vdma_chan *chan)
 {
-	int loop = XILINX_VDMA_LOOP_COUNT;
+	int err = 0;
+	u32 val;
 
 	vdma_ctrl_set(chan, XILINX_VDMA_REG_DMACR, XILINX_VDMA_DMACR_RUNSTOP);
 
 	/* Wait for the hardware to start */
-	do {
-		if (!(vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR) &
-		      XILINX_VDMA_DMASR_HALTED))
-			break;
-	} while (loop--);
+	err = xilinx_vdma_poll_timeout(chan, XILINX_VDMA_REG_DMASR, val,
+				      !(val & XILINX_VDMA_DMASR_HALTED), 0,
+				      XILINX_VDMA_LOOP_COUNT);
 
-	if (!loop) {
+	if (err) {
 		dev_err(chan->dev, "Cannot start channel %p: %x\n",
 			chan, vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR));
 
@@ -754,21 +756,17 @@  static void xilinx_vdma_complete_descriptor(struct xilinx_vdma_chan *chan)
  */
 static int xilinx_vdma_reset(struct xilinx_vdma_chan *chan)
 {
-	int loop = XILINX_VDMA_LOOP_COUNT;
+	int err = 0;
 	u32 tmp;
 
 	vdma_ctrl_set(chan, XILINX_VDMA_REG_DMACR, XILINX_VDMA_DMACR_RESET);
 
-	tmp = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR) &
-		XILINX_VDMA_DMACR_RESET;
-
 	/* Wait for the hardware to finish reset */
-	do {
-		tmp = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR) &
-			XILINX_VDMA_DMACR_RESET;
-	} while (loop-- && tmp);
+	err = xilinx_vdma_poll_timeout(chan, XILINX_VDMA_REG_DMACR, tmp,
+				      !(tmp & XILINX_VDMA_DMACR_RESET), 0,
+				      XILINX_VDMA_LOOP_COUNT);
 
-	if (!loop) {
+	if (err) {
 		dev_err(chan->dev, "reset timeout, cr %x, sr %x\n",
 			vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR),
 			vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR));
@@ -777,7 +775,7 @@  static int xilinx_vdma_reset(struct xilinx_vdma_chan *chan)
 
 	chan->err = false;
 
-	return 0;
+	return err;
 }
 
 /**