From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752583AbbJTX2m (ORCPT ); Tue, 20 Oct 2015 19:28:42 -0400 Received: from mail-lf0-f50.google.com ([209.85.215.50]:33695 "EHLO mail-lf0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751741AbbJTX2l (ORCPT ); Tue, 20 Oct 2015 19:28:41 -0400 Subject: Re: [PATCH v2 3/8] spi: imx: add support for all SPI word width for DMA transfer To: Robin Gong References: <20150930083539.GD2709@shlinux2> <04376A3D786BD947B28569C998A374A62E280E21@NA-MBX-02.mgc.mentorg.com> <20151008095102.GC6101@shlinux2> Cc: "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: <5626CE25.8040706@gmail.com> Date: Wed, 21 Oct 2015 01:28:37 +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: <20151008095102.GC6101@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:51, Robin Gong wrote: > On Thu, Oct 01, 2015 at 12:34:54AM +0000, Bondarenko, Anton wrote: >> On 30.09.2015 10:35, Robin Gong wrote: >>> On Fri, Sep 25, 2015 at 07:57:10PM +0200, Anton Bondarenko wrote: >>>> @@ -91,11 +91,15 @@ struct spi_imx_data { >>>> >>>> struct completion xfer_done; >>>> void __iomem *base; >>>> + unsigned long base_phys; >>>> + >>>> struct clk *clk_per; >>>> struct clk *clk_ipg; >>>> unsigned long spi_clk; >>>> unsigned int spi_bus_clk; >>>> >>>> + unsigned int bpw_w; >>>> + >>> It's better to change bytes_per_word for clear understanding,since bpw in spi means >>> bits_per_word... >> Agree. Fixed in V3 >>>> unsigned int count; >>>> void (*tx)(struct spi_imx_data *); >>>> void (*rx)(struct spi_imx_data *); >>>> @@ -201,9 +205,13 @@ static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi, >>>> struct spi_transfer *transfer) >>>> { >>>> struct spi_imx_data *spi_imx = spi_master_get_devdata(master); >>>> + unsigned int bpw_w = transfer->bits_per_word; >>>> >>>> - if (spi_imx->dma_is_inited && >>>> - (transfer->len > spi_imx->wml * sizeof(u32))) >>>> + if (!bpw_w) >>>> + bpw_w = spi->bits_per_word; >>>> + >>>> + bpw_w = DIV_ROUND_UP(bpw_w, 8); >>>> + if (spi_imx->dma_is_inited && (transfer->len > spi_imx->wml * bpw_w)) >>> Please remove bpw_w here as I talked in the first patch. >> As explained in patch1 we need to use SPI word size in calculation to decide >> if we want to go with DMA or PIO mode. Just a short example: >> We need to transfer 24 SPI words with BPW == 32. This will take ~ 24 PIO writes >> to FIFO (and same for reads). But transfer->len will be 24*4=96 bytes. >> WML is 32 SPI words. The decision will be incorrect if we do not take into account >> SPI bits per word. > Yes, you are right, but I'm thinking so many 'DIV_ROUND_UP()'in your patch to > get bpw, can you centralize it or just use the 'bpw_w' in 'struct spi_imx_data'? We can't use bpw_w from spi_imx_data because this is external callback for SPI core and driver does not know the purpose of this call. We also can't update current bpw_w for the same reason. I'll add a function like this: int spi_imx_get_bytes_per_word(int bpw) { return DIV_ROUND_UP(bpw, BITS_PER_BYTE); } >>>> return true; >>>> return false; >>>> } Regards, Anton