From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756870AbcBWDFz (ORCPT ); Mon, 22 Feb 2016 22:05:55 -0500 Received: from mga02.intel.com ([134.134.136.20]:56502 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754795AbcBWDFy (ORCPT ); Mon, 22 Feb 2016 22:05:54 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,487,1449561600"; d="scan'208";a="918916335" Date: Tue, 23 Feb 2016 08:39:37 +0530 From: Vinod Koul To: Kedareswara rao Appana Cc: dan.j.williams@intel.com, michal.simek@xilinx.com, soren.brinkmann@xilinx.com, appanad@xilinx.com, moritz.fischer@ettus.com, laurent.pinchart@ideasonboard.com, luis@debethencourt.com, anirudh@xilinx.com, dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/4] dmaengine: xilinx_vdma: Simplify spin lock handling Message-ID: <20160223030937.GH17690@localhost> References: <1456120477-32134-1-git-send-email-appanad@xilinx.com> <1456120477-32134-2-git-send-email-appanad@xilinx.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1456120477-32134-2-git-send-email-appanad@xilinx.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? > > Signed-off-by: Kedareswara rao Appana > --- > 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... > 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 > > 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- ~Vinod