From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755946AbbJHKFx (ORCPT ); Thu, 8 Oct 2015 06:05:53 -0400 Received: from mail-by2on0141.outbound.protection.outlook.com ([207.46.100.141]:32688 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751423AbbJHKFs (ORCPT ); Thu, 8 Oct 2015 06:05:48 -0400 Authentication-Results: spf=fail (sender IP is 192.88.158.2) smtp.mailfrom=freescale.com; mentor.com; dkim=none (message not signed) header.d=none;mentor.com; dmarc=none action=none header.from=freescale.com; Date: Thu, 8 Oct 2015 17:51:03 +0800 From: Robin Gong To: "Bondarenko, Anton" 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" Subject: Re: [PATCH v2 3/8] spi: imx: add support for all SPI word width for DMA transfer Message-ID: <20151008095102.GC6101@shlinux2> References: <20150930083539.GD2709@shlinux2> <04376A3D786BD947B28569C998A374A62E280E21@NA-MBX-02.mgc.mentorg.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <04376A3D786BD947B28569C998A374A62E280E21@NA-MBX-02.mgc.mentorg.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BN1AFFO11FD052;1:leyn7EizuOYh9qePj/CuQx40X82R244Pq3k4+2j960gGrnXUtmy/rmxCzmNRyKO3XRo7yRqrKTBMWzKc1TFZSaX0wcHR5KnE0C0SDdO7Jl46BrFUVyvVjS0C8f9BnsUHzgOSoDunuOwy8CB7qQqlpKZB4Oosscp69BxCQq69AE/vauwiZYMqM2lWKuZDxNsjfyaMIIPz6pem0jDMiyli7A10e50hflMv9XCpGddyL/2w3wItA9sCf6L8Unw+LE5ld+UsLjcGmnaY58IPsT24uqnm+2vQM2y/mucWGD+aZwD+FhBQUVFG9WPt6q5e2AqIDgpG/rVpJYwiixggQ+nDSA== X-Forefront-Antispam-Report: CIP:192.88.158.2;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(6009001)(2980300002)(1109001)(1110001)(339900001)(199003)(189002)(24454002)(5001960100002)(69596002)(46102003)(33656002)(105606002)(50466002)(85426001)(83506001)(110136002)(50986999)(64706001)(76176999)(189998001)(217423001)(47776003)(77096005)(87936001)(106466001)(5007970100001)(46406003)(2950100001)(97756001)(5001920100001)(54356999)(93886004)(4001350100001)(11100500001)(104016004)(81156007)(5008740100001)(23726002)(33716001)(6806005)(92566002)(97736004)(42262002);DIR:OUT;SFP:1102;SCL:1;SRVR:BL2PR03MB482;H:az84smr01.freescale.net;FPR:;SPF:Fail;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;BL2PR03MB482;2:RJ4jpVmCyMKxXpsLhwFdjpmcwRADTBHhDrWJq/uapkO1rct4YWbOqCXc0m9YvcGPc175Aopu7zTg3Zb8yIJzrGFPQyOzNZp5Zx7pMUgbvpvQQsCOp1Ua/ClOulyaPnc6UNPggEBttgZEmii+JFSTQmmHkmK4Iuz4rPmfCYN1I2M=;3:m8PR1diNcx9c1Imt9ws6X60nAm+dtEt7zhfVUu1dG8ivqJMHYTeb0DKd9K2reUSMXOVmPU2aVZePq4Vp8erdgpIDcLjTqpPNrMd9tX2oULGnDzUZZk0UI+dOZZQfrcrQ2T3QG6D+ujv5BwLhfDhYry4su1ghxXE7JxqSC46+sCM73cf8/O+SIIma30W0ToB6/1tHxmw/uL+3HCmR6vqSJQTfFwX2YoLd5y3+3vGUSqU=;25:a57cFWkqLIgMZr6qUl5DlZiUbtB1lCb5P7YdVhOLmNWKCF5BFY095hkYDMuiph/x/LVvAHUmf3+/tIqWa7TwvFp2QVGRfGqxJVvFFoerjLOlbDN3fBkq2BSXZAW2iGdTM08tFkTrO/KHT5mNWp3GCqb7BVg3nTjEHT/A8nl6jPuH8aQnGtB2aXS5ikhOrlWbzVWGNVt2YN1pKnbzu9Lq8hMHUZSTlJnpkjtwwc/372Z5o+BMWk/3eUsyXw0fKu3e8IKjpnseGrmnubHfJQszXQ== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BL2PR03MB482; X-Microsoft-Exchange-Diagnostics: 1;BL2PR03MB482;20:24NTigbZ6Kdj/rnnOMDnJK/AM5imBmoZGwP2UrU7wuUeyyWkteOSxh749thdDOE+M5vY45fUhK0WvOveSgON2l0niuWdZN2zJslD8NMdI8i5ZOh6BkAd7QJQvvfK9UINdpG2jjy+e6pMvjGMowAl2hxDgQejSmDOB0IlEoBsxqV+P+vuy8ZbGaJNzOvflbh33C13k8SX2l98uarPWKwC86yat4W9ATBkGP5sPWyq0TNDWLQOpYeuwC1r8K91JoBII+be2odJJHw0MzGqXxBCxk/iGqXgYMOIpQygyQXwFovbsDpqfIrpYvkQIc+foGyww39cVpv1xFp+61YJaIhWKN9KDRpkwpjq0bo+Ov0Rcxc=;4:vZiB0mUXJAPItuHLPwfBBVp6+ZlLTjb62crAzPdoyjcoEv0D5hlZ+y/w9Or/bmODMEOAOO6eZma8EDITt3APXEFUi1gV31asuG2g4Uzq4d88mtOCl7eh/xtg7UWjrJiOzrG87wbYGN7fySmH20Dn6Ig/g2UjWTjlN8ZAdZgRJIE2oaBOX329T480NWr+BJiyfkggov/pNqAAvvLnQ1VF5yS5NMYVmgkslmNdZXqc+fQM/QT8DPU+BoT47HTOlmi9260TcA69y3v0FaOO3e9dQ91K5RROMkKcKTI9r4oILahRpttLPTSJPHvw14rMw2vqeyRNFAIlGxme6lntSEg0Khj0anEeqfIKx2kpq1BFB04= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(520078)(5005006)(8121501046)(3002001);SRVR:BL2PR03MB482;BCL:0;PCL:0;RULEID:;SRVR:BL2PR03MB482; X-Forefront-PRVS: 0723A02764 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;BL2PR03MB482;23:v+MND8JHZitHcF6EIV6ojKQA7ND4a3hf4dogyZpf0f?= =?us-ascii?Q?t0WpETXEGzLD8zyLpid6N+5v5fBdoRiGkvgr0+PaZ+6XEnLBVYAv+1QWmWCy?= =?us-ascii?Q?HsTsVKOffU71fJSuxLYAXePmibZ7heBEl+ifnLHBB1BgiuggzzqTALNhdJNT?= =?us-ascii?Q?r5s1B0XfL6wekatdF7F3EAuphaCmohceHQTB097vjAVqv4nQiDegnfVNTBvH?= =?us-ascii?Q?g0tvxmjxr9usnrUJUvdX2GuzvtA5S0bCBhP9NJ+VRZlxK4F6/hC848+tt4Yc?= =?us-ascii?Q?tuhP9xRJdj1lq50UTg5PTzlaNWsnZlGzNQ6kn4GbbdRaaxq3xwWZiaT+VJqb?= =?us-ascii?Q?+g57LOEvuErpNhu0SHTiTSKZLKKq8MK1qL81QXFt9xweFLcP+zzEFDQe5uiA?= =?us-ascii?Q?8YUrgF5FiGN9+q6l6w5dtEsC8GBFFR9MZc2VyWtaOID/ZxM0iewj8BpXwkYQ?= =?us-ascii?Q?U7a3Gq9rq0xIV+cFG5sCm207toCnALNjhfKSGeA4L8AfgtfRvVL5FL6G9Lg6?= =?us-ascii?Q?OEzAzVkMWsYzfqyQuDDNOrI9jj21g1bpkNpcE+43yWJKtL87L+lBIbx+Zvq3?= =?us-ascii?Q?jIOfUxKfqoyd/qd+gfFvNyDbWvCSInccD4Rc54TnGRBR0bMMy/numBpRrab8?= =?us-ascii?Q?QrUAO+r6+PXDrG8LxwUIkWOrINeo6POVxLaVQtO2lC97Rz4GsnCkRYZtiV6j?= =?us-ascii?Q?nPVyPXVlwqAAENM3yGA3Tx90h/G8AfDznk8lcxdbX/wHa449vtQ+5mCJ0eAO?= =?us-ascii?Q?J7Su08bdDCpEwd/fHsGJN5f3B3+qzxEjxzpAvAcqwT5UBgadXelSwGGNmcWy?= =?us-ascii?Q?c6f8QKCiYa2MeI5WW3/cUCjNhzqexCAoflP9EWk5z233m9eTIOCFLF3dcNuD?= =?us-ascii?Q?0625RQWCP0mkPZ7XWILvFm2kFsoh97i54UdRwestMB14fVu/tIm9RccYKxIE?= =?us-ascii?Q?YKFqwwoV4MKG8RDHn/SfYyHN23pjnXU5myqSndAt6LfiHKoFipto1x5eftzN?= =?us-ascii?Q?fDq9/H50uJOVYmYeq4Fnyg3HPjaR6R/kvfm6A0SgHjBauufSPLBNqSjQrzIz?= =?us-ascii?Q?Sjsfy0Fkz5gardMDEYhqM4w1nlbSNfbyhAapf+C1ypDf6/5gUd4qa42r5TS2?= =?us-ascii?Q?r7J9YbrrEYODpkX7Clb0aBw5d+6CqLmwjzwIbJli7C7ifPqaW+Rg=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;BL2PR03MB482;5:9kjK9Mjm6roaX4b1dVIAX9EntdN89tCJs1kzAGopvXgxj9cF7rpJ36miVRoI/V0IOouT5kZrwaNWiJn/eDeSOWof3fmlKhe3mMuLC8SVfLoOYMRD8bdOJdXV5iBPsKvbdp1G07s8OEaPr0Riv8Ea8Q==;24:K73Env2bkFj4lILILbo0kPTHMnZzd/D36mi+qlukCtKEe/0nL6l59+ufYC1yrSmOyAxkW3T+xUbWu42bVnO5Poeq+JCr66hQ4TZYJ6aJWuo=;20:1Rm0zHGJubryoWNpCL6vxw/kQuzOWPB4iUDgi8FfVLCkUBUd4iin/JDk9SS/WHwbgpJmfwQRVY9Fe5nbgppotg== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Oct 2015 09:51:39.3738 (UTC) X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.158.2];Helo=[az84smr01.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL2PR03MB482 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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'? > >> return true; > >> return false; > >> } > >> @@ -1007,7 +1058,8 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, > >> DMA_FROM_DEVICE); > >> > >> spi_imx->rx_buf = pio_buffer; > >> - spi_imx->txfifo = left; > >> + spi_imx->txfifo = DIV_ROUND_UP(left, spi_imx->bpw_w); > >> + > > Looks not enough here, you have to set 'spi_imx->rx' per the right bpw. > What do you mean? We have had bpw_w == 1 before so > spi_imx->txfifo = DIV_ROUND_UP(left, spi_imx->bpw_w); > is equivalent to > spi_imx->txfifo = left; > Now we could have bpw_w equal to 2 or 4 also. txfifo is number of SPI words > and not number of bytes. Sorry, please ignore this comment, since 'spi_imx->rx' will be correctly set per the right bpw in spi_imx_setupxfer whatever in DMA or PIO mode. > >> reinit_completion(&spi_imx->xfer_done); > >> > >> spi_imx->devtype_data->intctrl(spi_imx, MXC_INT_TCEN);