From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751898AbdCAIQs (ORCPT ); Wed, 1 Mar 2017 03:16:48 -0500 Received: from relay1.mentorg.com ([192.94.38.131]:60988 "EHLO relay1.mentorg.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751835AbdCAIQr (ORCPT ); Wed, 1 Mar 2017 03:16:47 -0500 Subject: Re: [PATCH 1/1] dma: imx-sdma: add 1ms delay to ensure SDMA channel is stopped To: Vinod Koul References: <1486738005-4297-1-git-send-email-jiada_wang@mentor.com> <20170213020530.GI2843@localhost> <20170213102254.GK2843@localhost> CC: , , From: Jiada Wang Message-ID: <06384fdf-64b6-c796-a4ef-676c7d14f0e7@mentor.com> Date: Wed, 1 Mar 2017 17:14:47 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20170213102254.GK2843@localhost> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Vinod On 02/13/2017 07:22 PM, Vinod Koul wrote: > On Mon, Feb 13, 2017 at 03:30:19PM +0900, Jiada Wang wrote: >>>> +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.. >>> >> I got the information from NXP (freescale) R&D team, >> according to them, by write '1' to SDMA_H_STATSTOP, only disables >> the related sdma channel (so poll HE bit will indicates the channel >> has been disabled), >> but it cannot ensure SDMA core stop to access modules' FIFO, >> SDMA core may still is running, this is a bug in HW. > > Okay b ut you are not doing the HE bit here..?? by calling sdma_disable_channel(chan) here, sdma driver clears corresponding HE bit, thus disables the channel. But it can't ensure SDMA core is stopped by this operation. >> >> regarding if the '1ms' is enough to ensure SDMA core has stopped, >> NXP R&D team mentioned: >> "we should add some delay of one BD SDMA cost time after disable the >> channel bit, the maximum is 1ms" >> so I assume 1ms should work for all cases > > At least please document this in changelog and comments in code. > I will update my patch to add comments once all concerns are addressed. Thanks, Jiada