linux-kernel - Re: [PATCH 1/1] dma: imx-sdma: add 1ms delay to ensure SDMA channel is stopped

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <06384fdf-64b6-c796-a4ef-676c7d14f0e7@mentor.com>
Date:   Wed, 1 Mar 2017 17:14:47 +0900
From:   Jiada Wang <jiada_wang@...tor.com>
To:     Vinod Koul <vinod.koul@...el.com>
CC:     <dan.j.williams@...el.com>, <dmaengine@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] dma: imx-sdma: add 1ms delay to ensure SDMA channel
 is stopped

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