Hi Vinod, > -----Original Message----- > From: Vinod Koul [mailto:vinod.koul@xxxxxxxxx] > Sent: Tuesday, February 23, 2016 8:40 AM > To: Appana Durga Kedareswara Rao > Cc: dan.j.williams@xxxxxxxxx; Michal Simek; Soren Brinkmann; Appana Durga > Kedareswara Rao; moritz.fischer@xxxxxxxxx; > laurent.pinchart@xxxxxxxxxxxxxxxx; luis@xxxxxxxxxxxxxxxxx; Anirudha > Sarangi; dmaengine@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v2 2/4] dmaengine: xilinx_vdma: Simplify spin lock handling > > On Mon, Feb 22, 2016 at 11:24:35AM +0530, Kedareswara rao Appana wrote: > > This patch simplifies the spin lock handling in the driver. > > But sadly doesn't describe how? Ok sure will improve commit message in the next version. > > > > > Signed-off-by: Kedareswara rao Appana <appanad@xxxxxxxxxx> > > --- > > Changes for v2: > > ---> splitted the changes into multiple patches. > > > > drivers/dma/xilinx/xilinx_vdma.c | 27 ++++++++++----------------- > > 1 file changed, 10 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/dma/xilinx/xilinx_vdma.c > > b/drivers/dma/xilinx/xilinx_vdma.c > > index 06bffec..d646218 100644 > > --- a/drivers/dma/xilinx/xilinx_vdma.c > > +++ b/drivers/dma/xilinx/xilinx_vdma.c > > @@ -605,17 +605,14 @@ static void xilinx_vdma_start_transfer(struct > > xilinx_vdma_chan *chan) { > > struct xilinx_vdma_config *config = &chan->config; > > struct xilinx_vdma_tx_descriptor *desc, *tail_desc; > > - unsigned long flags; > > u32 reg; > > struct xilinx_vdma_tx_segment *tail_segment; > > > > if (chan->err) > > return; > > > > - spin_lock_irqsave(&chan->lock, flags); > > - > > It would help if you add a comment to this function taht we need to invoke this > with lock held... Ok sure will add comment.. > > > if (list_empty(&chan->pending_list)) > > - goto out_unlock; > > + return; > > > > desc = list_first_entry(&chan->pending_list, > > struct xilinx_vdma_tx_descriptor, node); @@ - > 629,7 +626,7 @@ > > static void xilinx_vdma_start_transfer(struct xilinx_vdma_chan *chan) > > if (chan->has_sg && xilinx_vdma_is_running(chan) && > > !xilinx_vdma_is_idle(chan)) { > > dev_dbg(chan->dev, "DMA controller still busy\n"); > > - goto out_unlock; > > + return; > > } > > > > /* > > @@ -676,7 +673,7 @@ static void xilinx_vdma_start_transfer(struct > xilinx_vdma_chan *chan) > > xilinx_vdma_start(chan); > > > > if (chan->err) > > - goto out_unlock; > > + return; > > > > /* Start the transfer */ > > if (chan->has_sg) { > > @@ -696,7 +693,7 @@ static void xilinx_vdma_start_transfer(struct > xilinx_vdma_chan *chan) > > } > > > > if (!last) > > - goto out_unlock; > > + return; > > > > /* HW expects these parameters to be same for one > transaction */ > > vdma_desc_write(chan, XILINX_VDMA_REG_HSIZE, last- > >hw.hsize); @@ > > -707,9 +704,6 @@ static void xilinx_vdma_start_transfer(struct > > xilinx_vdma_chan *chan) > > > > list_splice_tail_init(&chan->pending_list, &chan->active_list); > > chan->desc_pendingcount = 0; > > - > > -out_unlock: > > - spin_unlock_irqrestore(&chan->lock, flags); > > } > > > > /** > > @@ -719,8 +713,11 @@ out_unlock: > > static void xilinx_vdma_issue_pending(struct dma_chan *dchan) { > > struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan); > > + unsigned long flags; > > > > + spin_lock_irqsave(&chan->lock, flags); > > xilinx_vdma_start_transfer(chan); > > + spin_unlock_irqrestore(&chan->lock, flags); > > } > > > > /** > > @@ -732,21 +729,15 @@ static void xilinx_vdma_issue_pending(struct > > dma_chan *dchan) static void xilinx_vdma_complete_descriptor(struct > > xilinx_vdma_chan *chan) { > > struct xilinx_vdma_tx_descriptor *desc, *next; > > - unsigned long flags; > > - > > - spin_lock_irqsave(&chan->lock, flags); > > this one as well Ok sure will add comment.. Regards, Kedar. > > > > > if (list_empty(&chan->active_list)) > > - goto out_unlock; > > + return; > > > > list_for_each_entry_safe(desc, next, &chan->active_list, node) { > > list_del(&desc->node); > > dma_cookie_complete(&desc->async_tx); > > list_add_tail(&desc->node, &chan->done_list); > > } > > - > > -out_unlock: > > - spin_unlock_irqrestore(&chan->lock, flags); > > } > > > > /** > > @@ -857,8 +848,10 @@ static irqreturn_t xilinx_vdma_irq_handler(int irq, > void *data) > > } > > > > if (status & XILINX_VDMA_DMASR_FRM_CNT_IRQ) { > > + spin_lock(&chan->lock); > > xilinx_vdma_complete_descriptor(chan); > > xilinx_vdma_start_transfer(chan); > > + spin_unlock(&chan->lock); > > } > > > > tasklet_schedule(&chan->tasklet); > > -- > > 2.1.2 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe dmaengine" > > in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo > > info at http://vger.kernel.org/majordomo-info.html > > -- > ~Vinod -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
