From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751542AbdBMCE4 (ORCPT ); Sun, 12 Feb 2017 21:04:56 -0500 Received: from mga01.intel.com ([192.55.52.88]:36717 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751366AbdBMCEy (ORCPT ); Sun, 12 Feb 2017 21:04:54 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,155,1484035200"; d="scan'208";a="933119717" Date: Mon, 13 Feb 2017 07:35:30 +0530 From: Vinod Koul To: jiada_wang@mentor.com Cc: dan.j.williams@intel.com, dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/1] dma: imx-sdma: add 1ms delay to ensure SDMA channel is stopped Message-ID: <20170213020530.GI2843@localhost> References: <1486738005-4297-1-git-send-email-jiada_wang@mentor.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1486738005-4297-1-git-send-email-jiada_wang@mentor.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 Fri, Feb 10, 2017 at 06:46:45AM -0800, jiada_wang@mentor.com wrote: > From: Jiada Wang > > sdma_disable_channel() cannot ensure dma is stopped to access > module's FIFOs. Maybe SDMA core is running and accessing BD when > disable of corresponding channel, this may cause sometimes even > after call of .sdma_disable_channel(), SDMA core still be running > and accessing module's FIFOs. > > We should add delay of one BD SDMA cost time, the maximum is 1ms. > So that SDMA clients by calling .device_terminate_all can > ensure SDMA core has really been stopped. > > Signed-off-by: Jiada Wang > --- > drivers/dma/imx-sdma.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c > index d1651a5..7332c40 100644 > --- a/drivers/dma/imx-sdma.c > +++ b/drivers/dma/imx-sdma.c > @@ -937,6 +937,14 @@ static int sdma_disable_channel(struct dma_chan *chan) > return 0; > } > > +static int sdma_disable_channel_with_delay(struct dma_chan *chan) > +{ > + sdma_disable_channel(chan); > + mdelay(1); what is the gaurantee that 1ms is fine? Shouldn't you poll the bit to see channel is disabled properly.. > + > + return 0; > +} > + > static void sdma_set_watermarklevel_for_p2p(struct sdma_channel *sdmac) > { > struct sdma_engine *sdma = sdmac->sdma; > @@ -1828,7 +1836,7 @@ static int sdma_probe(struct platform_device *pdev) > sdma->dma_device.device_prep_slave_sg = sdma_prep_slave_sg; > sdma->dma_device.device_prep_dma_cyclic = sdma_prep_dma_cyclic; > sdma->dma_device.device_config = sdma_config; > - sdma->dma_device.device_terminate_all = sdma_disable_channel; > + sdma->dma_device.device_terminate_all = sdma_disable_channel_with_delay; > sdma->dma_device.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES); > sdma->dma_device.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES); > sdma->dma_device.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV); > -- > 2.7.4 > > -- ~Vinod