From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753096AbbJTXEA (ORCPT ); Tue, 20 Oct 2015 19:04:00 -0400 Received: from mail-lf0-f47.google.com ([209.85.215.47]:35145 "EHLO mail-lf0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750904AbbJTXD6 (ORCPT ); Tue, 20 Oct 2015 19:03:58 -0400 Subject: Re: [PATCH v2 1/8] spi: imx: Fix DMA transfer To: Robin Gong References: <20150930082341.GB2709@shlinux2> <04376A3D786BD947B28569C998A374A62E280DD6@NA-MBX-02.mgc.mentorg.com> <20151008091951.GA6101@shlinux2> Cc: "s.hauer@pengutronix.de" , "broonie@kernel.org" , "linux-kernel@vger.kernel.org" , "linux-spi@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "Wang, Jiada (ESD)" , "Zapolskiy, Vladimir" From: Anton Bondarenko Message-ID: <5626C85B.2090707@gmail.com> Date: Wed, 21 Oct 2015 01:03:55 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151008091951.GA6101@shlinux2> 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 On 08.10.2015 11:19, Robin Gong wrote: > On Thu, Oct 01, 2015 at 12:02:41AM +0000, Bondarenko, Anton wrote: >>>> @@ -201,9 +202,8 @@ static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi, >>>> { >>>> struct spi_imx_data *spi_imx = spi_master_get_devdata(master); >>>> >>>> - if (spi_imx->dma_is_inited >>>> - && transfer->len > spi_imx->rx_wml * sizeof(u32) >>>> - && transfer->len > spi_imx->tx_wml * sizeof(u32)) >>>> + if (spi_imx->dma_is_inited && >>>> + (transfer->len > spi_imx->wml * sizeof(u32))) >>> Add Sascha in the loop. I don't think "* sizeof(u32)", since even 1 byte data >>> will consume one position of 32bit FIFO Thus if here >>> spi_imx->wml = spi_imx_get_fifosize(spi_imx) / 2 = 32, the threshold value >>> which judge DMA mode used or not should be 32 not 32 * 4. >>> Of course, it will not cause any function break since both DMA and PIO can work >>> ,but I think we'd better correct it. >> I agree, in case of 1 byte SPI word we do not need to multiply by 4. >> But for 16 bit and 32 bit SPI words it's necessary. This part is >> addressed in patch 3. >> I could remove "* sizeof(u32)" for now. > I still think don't need *sizeof(u32) even for 16bit and 32bit, whatever bits > used as one spi word(<32bits), one spi word consume one position of SPI FIFO > (32bit). Will be removed in V3 for this patch. >>>> return true; >>>> return false; >>>> } Regards, Anton