From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755734AbbLGJmc (ORCPT ); Mon, 7 Dec 2015 04:42:32 -0500 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:40837 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755464AbbLGJm3 (ORCPT ); Mon, 7 Dec 2015 04:42:29 -0500 Date: Mon, 7 Dec 2015 10:42:24 +0100 From: Sascha Hauer To: Anton Bondarenko Cc: broonie@kernel.org, b38343@freescale.com, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, vladimir_zapolskiy@mentor.com, jiada_wang@mentor.com Subject: Re: [PATCH v5 08/11] spi: imx: allow only WML aligned transfers to use DMA Message-ID: <20151207094224.GC11966@pengutronix.de> References: <1449334629-4715-1-git-send-email-anton.bondarenko.sama@gmail.com> <1449334629-4715-9-git-send-email-anton.bondarenko.sama@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1449334629-4715-9-git-send-email-anton.bondarenko.sama@gmail.com> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 10:34:44 up 50 days, 18:13, 96 users, load average: 0.01, 0.21, 0.30 User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: sha@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Dec 05, 2015 at 05:57:06PM +0100, Anton Bondarenko wrote: > RX DMA tail data handling doesn't work correctly in many cases with current > implementation. It happens because SPI core was setup to generates both RX > and RX TAIL events. And RX TAIL event does not work correctly. > This can be easily verified by sending SPI transaction with size modulus > WML(32 in our case) not equal 0. > > Also removing change introduced in f6ee9b582d2db652497b73c1f117591dfb6d3a90 > since this change only fix usecases with transfer size from 33 to 128 bytes > and does not fix 129 bytes etc. > > This is output from transaction with len 138 bytes in loopback mode at 10Mhz: > TX0000: a3 97 a2 55 53 be f1 fc f9 79 6b 52 14 13 e9 e2 > TX0010: 2d 51 8e 1f 56 08 57 27 a7 05 d4 d0 52 82 77 75 > TX0020: 1b 99 4a ed 58 3d 6a 52 36 d5 24 4a 68 8e ad 95 > TX0030: 5f 3c 35 b5 c4 8c dd 6c 11 32 3d e2 b4 b4 59 cf > TX0040: ce 23 3d 27 df a7 f9 96 fc 1e e0 66 2c 0e 7b 8c > TX0050: ca 30 42 8f bc 9f 7b ce d1 b8 b1 87 ec 8a d6 bb > TX0060: 2e 15 63 0e 3c dc a4 3a 7a 06 20 a7 93 1b 34 dd > TX0070: 4c f5 ec 88 96 68 d6 68 a0 09 6f 8e 93 47 c9 41 > TX0080: db ac cf 97 89 f3 51 05 79 71 > > RX0000: a3 97 a2 55 53 be f1 fc f9 79 6b 52 14 13 e9 e2 > RX0010: 2d 51 8e 1f 56 08 57 27 a7 05 d4 d0 52 82 77 75 > RX0020: 1b 99 4a ed 58 3d 6a 52 36 d5 24 4a 68 8e ad 95 > RX0030: 5f 3c 35 00 00 b5 00 00 00 c4 00 00 8c 00 00 dd > RX0040: 6c 11 32 3d e2 b4 b4 59 cf ce 23 3d 27 df a7 f9 > RX0050: 96 fc 1e e0 66 2c 0e 7b 8c ca 30 42 8f 1f 1f bc > RX0060: 9f 7b ce d1 b8 b1 87 ec 8a d6 bb 2e 15 63 0e ed > RX0070: ed 3c 58 58 58 dc 3d 3d a4 6a 6a 3a 52 52 7a 36 > RX0080: 06 20 a7 93 1b 34 dd 4c f5 ec > > Zeros at offset 33 and 34 caused by reading empty RX FIFO which not possible > if DMA RX read was triggered by RX event. This mean DMA was triggered > by RX TAIL event. > > Signed-off-by: Anton Bondarenko > --- > drivers/spi/spi-imx.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c > index fa24637..7a68c62 100644 > --- a/drivers/spi/spi-imx.c > +++ b/drivers/spi/spi-imx.c > @@ -204,8 +204,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->wml * sizeof(u32)) > + if (spi_imx->dma_is_inited && transfer->len > spi_imx->wml && > + (transfer->len % spi_imx->wml) == 0) > return true; Must transfer->len really be bigger than spi_imx->wml? I would assume it should be >= instead. And where is the * sizeof(u32) gone? If that's unnecessary I heven't understood why. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |