Message ID | 1456495434-11722-4-git-send-email-appanad@xilinx.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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) {
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 --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; } /**
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(-)