From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754996AbbI3IsJ (ORCPT ); Wed, 30 Sep 2015 04:48:09 -0400 Received: from mail-bn1bon0117.outbound.protection.outlook.com ([157.56.111.117]:13749 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753115AbbI3Irw (ORCPT ); Wed, 30 Sep 2015 04:47:52 -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: Wed, 30 Sep 2015 16:31:36 +0800 From: Robin Gong To: Anton Bondarenko CC: , , , , , , Subject: Re: [PATCH v2 2/8] spi: imx: replace fixed timeout with calculated one Message-ID: <20150930083135.GC2709@shlinux2> References: <43a7e93d4d29253215a5590b09baeb11a7e6f6d8.1442923630.git.anton_bondarenko@mentor.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <43a7e93d4d29253215a5590b09baeb11a7e6f6d8.1442923630.git.anton_bondarenko@mentor.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BN1BFFO11FD005;1:YcYkycyxvibACIdzuUtyhfNyHr38Sdx07wvpAb2dOJSB2e9yvymLd85SjylymqBnDtvWGa0qf6XVatS5VbtN1Qya401vi6jumg+M3bMCnxr+39bXRjkJSYDlqOG8VEa9wIuMW8U49mmkMtnWaPONNEw/YzZOEdvIYKfDHO5pNshXA2GXMq2IDoqSzVBoRsbEIvMy75ptDBh0HXJO4te+6N6cIhE4WpDtKxKaIvyTydRPceSulKw90oup032DquBzVLDOsGf7XyhBuOnmuuboXWflLW121mUBMfxZlMxucbtpMVXBsi5PRhbMT34Ad1zy8JppDD0WrChSjsPqiweH8w== X-Forefront-Antispam-Report: CIP:192.88.158.2;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(6009001)(2980300002)(1110001)(1109001)(339900001)(24454002)(199003)(189002)(551934003)(83506001)(104016004)(33656002)(11100500001)(33716001)(69596002)(23726002)(5008740100001)(46406003)(5007970100001)(106466001)(77156002)(105606002)(97756001)(62966003)(46102003)(77096005)(87936001)(50986999)(68736005)(85426001)(6806005)(4001350100001)(5001830100001)(81156007)(4001540100001)(97736004)(2950100001)(5001860100001)(110136002)(189998001)(92566002)(76176999)(54356999)(5001960100002)(47776003)(19580395003)(19580405001)(64706001)(50466002)(42262002);DIR:OUT;SFP:1102;SCL:1;SRVR:BL2PR03MB483;H:az84smr01.freescale.net;FPR:;SPF:Fail;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;BL2PR03MB483;2:mrtyhTKTETlKaZFK+TQB0xzQ9yj2xr8lQiIp4VwTApQUSvNKRKga9v05RA5WpnGjBd7qg+gHwQ3GQpjo2F+ZHu755ypunvQ6StCgF6AeRzWJSbLUC9vhoGUGlB5A6y/9y051qJqn5xEeCIZn3LlKEj5OecHR/wPgPZStxVXGG/g=;3:EQ/WtIwbdVnZoP3qziQ5XKkfKRez/4jLV5nGckOb9SRSb1ywPkBjHfT8vlEKiNjWvJSzeFqSJqklKsH4VqLk1mt5/u0BkMsNCv3FO9txIdfs5JtyHW3fuJVkPR/W2SbmFcULFxBgHXQWlKGnf2XcVdS1G34o0x5D+nVc//vU5kpzTc3UWjWNu9cP7BqJKnVl0Elp4SWpi2bQGOs8z/NpbZG4vC8Q8zEuOAQKLfSOkHQ=;25:FoH9AoVjiTaa5tLr2+zXiaq2dojhW3j4pr/43zqnkvUqyAzLa4kQLA/2pituboRGTGF54UsI19g1NXQsl+rdPl9+lKNwj6vMJhLfidpGEri1uaYh8whoxRD/hgZ1e1Eqe/IEDIOMzKFcsTWDpGMWhVCijBFf48qbzm9iy9wBXZFTSeQHy6p/6udDu6YbjGBt/pErZUMR5Oo5mHdapcpaxL5WkuxOo3/MoJltySCrZt8xCS2y9+efo4wTMs7ObS8Yo3s6igc3pAnni0JgJhyn8A== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BL2PR03MB483; X-Microsoft-Exchange-Diagnostics: 1;BL2PR03MB483;20:R+OUshQxqVafIHnidcNPHEK9bylc0xEFdCaGN9A3/n6f85yidRu/Jpmqp0ggImRpUTrivdN6kyqDqCWYqSkOIuJx/T/nQ+JhEMgRkI32YVAQIzKpmCHKF6omHFVTrzb+i15GIKWZcpYSXHzW5FEz9mrt5do4kLcIPk6NR3E9o1dwmQhygJEWJ9laao3EL89Z/7/E7pD+T+lcnbAOlEBYLpSs8QDXEFupwijvlpfYcswwFfNsoww4vhT311uVDcKsHv09aHIPu4kgK2EP5KAju9dxQfD3YW6AwIRZRdcsFUf2To8bpEjlkWvzViu1Rm9qHm6bZ/mCmOntMU8h5mTNRG8YcOuike+i+o/31luGYUY=;4:VVSfF55U5tX4Pfw4y7rthHQr97F1mPal6wFDoo3szmo/IW90rPMUIFH6/0nb3yaBma6xNfoOB47UFJtEDfaac8RPS9u+2lWe0qzQRa5uqsjkmpVFSNb7xu3ARso6MIMs2anTcqWeCkVLKk0rTtXcmD6ObeBCMkcSIEGqkoKVssnEx0hBuFlAwE8MbmKEIFjqc+U98G0EAY3M+z4kmLd/EOjEUWY4q04BosVaSQUBhtW0hu3M8yTukH17D2WnB/4jQqDoG43rsEzi8jRQvN2d2WJ60ApO14sCPPK6/gm/XH3O3PhZa4mxS2JgBYPBeIPqP8LMrgsVq3BtImpmqr1QO/iX9twz0cb2SuNog3fN3mg= 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:BL2PR03MB483;BCL:0;PCL:0;RULEID:;SRVR:BL2PR03MB483; X-Forefront-PRVS: 071518EF63 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;BL2PR03MB483;23:qugdgJn8C3s7lmjkQtshCFn58MYUU3YKRxF0UyhGgE?= =?us-ascii?Q?jY0RBpkvEUiXvIwMlVKGqJe3akBP42H7PSib20CiN0MqqJgWNEFtt4qKjNgV?= =?us-ascii?Q?yPQcKCUSzoB9VzNL0PYGFvlWDYSyzyeYtfyUtnrh+SaTqMVruhHDn3ynVaNz?= =?us-ascii?Q?xj/FoWjR+26I8KZdaIBdQX5FdMNgXHDxYoGmrQoVo+mIFXw6PtWjqwOy9knx?= =?us-ascii?Q?VJZl3TmQsOcRsnCjJSD5tBsIFXQzXekTS0pQ5w1oTTTcghffjeMUba4fKJQk?= =?us-ascii?Q?XDZKFrXBfAcSmWxB+AuCKPSzFWilLWR4gVWdugC1TmAPqbxi7t4f+OEK93VM?= =?us-ascii?Q?vfa9hg84GQzDo1LKN9KDAIRXndgTeFAVAEmfD7MwXPyCOciES/rR+a0+176/?= =?us-ascii?Q?FrlOy7f1siStlQmJRi0OQAesXSBN8PCaWUu1tqLHZiX2qdGiy/5gLAoqhVG8?= =?us-ascii?Q?puATzNBtmlNQEbL7dghFOkwKRh+2DKMR3x4tFh9IYGWK2EvVUfCRzFxGR+lp?= =?us-ascii?Q?1U3gOe8IYhBUTwvuoUGazRbZg2Nec1n66LVBD4vwasOf/qPnrP4BUxcteOHQ?= =?us-ascii?Q?PpMf5KNKyPvLpg5x8VRzUzv317wki6QpVphBMWR9rxNlJkwISXQYSA6G4eei?= =?us-ascii?Q?IpHX5oubMyniMwsrZwKRWNOA/VpoDDkO1E0mTTxdBxE+HmSiUTtsRBmE41Ml?= =?us-ascii?Q?D/5ubYN2ulr52GlUb2tlPiPvtrZ6GGDevvbZHokqxzdr5sYeUJ+afkvtgAVz?= =?us-ascii?Q?ZJkkSQ5y5nOIAGx57NES/TsPOyVkYHs9rT8AnOi0m6fu50+fQ6+ACeSfKDis?= =?us-ascii?Q?xqgUuztSjOULZwcCpmZV6R6/OG4b1ys+fGEfLuRucg8SjNLYbw3oLagS/MwG?= =?us-ascii?Q?iYj8VSLqrHcSxS8zWXa8cuHNPSeJ1/bUlM+F546DcfAdAXl1dRHyTh2KaGqx?= =?us-ascii?Q?++//7jSrKitYZhYi7v2ExWsuKbCugIWrrGqpNvMNwLzdT1sFg7D9WDDzsZ13?= =?us-ascii?Q?0i38pFZ3HMtA156ttdiOyGDABxQwnqk1PniInzEZ43GQwPtIU7zO6jI0QZQ2?= =?us-ascii?Q?9zyp6jfVxi9bFGFWjPfy025PglPGZmDf0GkUbgGO/BDKqGtedq2eoEZeJ4oj?= =?us-ascii?Q?tpfy/WrjlhSE1/KHBu9k5enuq+NmrqGdHNCgbnJmf+U9M+002RKhVa5mKKQK?= =?us-ascii?Q?Sxf7Ae/GKrepzwCgt5NDAUFlc3U30j68sUpztpGA8TcaXk/JxOygN7D5V73t?= =?us-ascii?Q?QKCqCxonWSAZjYrz4H71cioX83Jose4jDrDWSu+ADPud0yEV9zE+Uq/Huweq?= =?us-ascii?Q?+uYmX3Ep6BMN3dHUwyXkKWC4blLfhAVyrOp9B0RG2N?= X-Microsoft-Exchange-Diagnostics: 1;BL2PR03MB483;5:ikDWhbSOMrpiZ0n354eFlyHEJH7df6G7IIeIzckt7AlGUw9PjbvCtQ5d4IgQXdWZrwbbL8paVjZTJJSTjooHozQ2KJZPDWBvTPS7zsZ6cgaTGody2iY7gfFCFqlDQmK7lwpLk6L9+Ran1mH9Ur6dYg==;24:P6uQU5aDic56wq/acGNNqE3baiaRpa+L0cxtJve+9cyOHQKvzvVov4cP63vxPBqTW/ITTrE3y92qu00LI5oQ4GUXrHE4RxSKQE0e90CuUj8=;20:KDrQJKQQ2QSnu/Zp0IjV8klNeW358acaEOGt8xj1BkWvkPsCbq+ialSq+l6rRk0JwZN+l6c9WNI5RmJFYsPQXA== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Sep 2015 08:32:08.0928 (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: BL2PR03MB483 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Sep 25, 2015 at 07:57:09PM +0200, Anton Bondarenko wrote: > Fixed timeout value can fire while transaction is ongoing. This may happen > because there are no strict requirements on SPI transaction duration. > Dynamic timeout value is generated based on SCLK and transaction size. > > There is also 4 * SCLK delay between TX bursts related to CS change. > > Signed-off-by: Anton Bondarenko > --- > drivers/spi/spi-imx.c | 49 +++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 41 insertions(+), 8 deletions(-) > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c > index 165bc2c..6c98eda 100644 > --- a/drivers/spi/spi-imx.c > +++ b/drivers/spi/spi-imx.c > @@ -59,7 +59,6 @@ > > /* The maximum bytes that a sdma BD can transfer.*/ > #define MAX_SDMA_BD_BYTES (1 << 15) > -#define IMX_DMA_TIMEOUT (msecs_to_jiffies(3000)) > struct spi_imx_config { > unsigned int speed_hz; > unsigned int bpw; > @@ -95,6 +94,7 @@ struct spi_imx_data { > struct clk *clk_per; > struct clk *clk_ipg; > unsigned long spi_clk; > + unsigned int spi_bus_clk; > > unsigned int count; > void (*tx)(struct spi_imx_data *); > @@ -317,8 +317,7 @@ static int __maybe_unused mx51_ecspi_config(struct spi_imx_data *spi_imx, > { > u32 ctrl = MX51_ECSPI_CTRL_ENABLE, dma = 0; > u32 cfg = readl(spi_imx->base + MX51_ECSPI_CONFIG); > - > - u32 clk = config->speed_hz, delay; > + u32 delay; > > /* > * The hardware seems to have a race condition when changing modes. The > @@ -330,7 +329,9 @@ static int __maybe_unused mx51_ecspi_config(struct spi_imx_data *spi_imx, > ctrl |= MX51_ECSPI_CTRL_MODE_MASK; > > /* set clock speed */ > - ctrl |= mx51_ecspi_clkdiv(spi_imx->spi_clk, config->speed_hz, &clk); > + spi_imx->spi_bus_clk = config->speed_hz; > + ctrl |= mx51_ecspi_clkdiv(spi_imx->spi_clk, config->speed_hz, > + &spi_imx->spi_bus_clk); > > /* set chip select to use */ > ctrl |= MX51_ECSPI_CTRL_CS(config->cs); > @@ -363,7 +364,7 @@ static int __maybe_unused mx51_ecspi_config(struct spi_imx_data *spi_imx, > * the SPI communication as the device on the other end would consider > * the change of SCLK polarity as a clock tick already. > */ > - delay = (2 * 1000000) / clk; > + delay = (2 * USEC_PER_SEC) / spi_imx->spi_bus_clk; > if (likely(delay < 10)) /* SCLK is faster than 100 kHz */ > udelay(delay); > else /* SCLK is _very_ slow */ > @@ -889,12 +890,40 @@ static void spi_imx_dma_tx_callback(void *cookie) > complete(&spi_imx->dma_tx_completion); > } > > +static int spi_imx_calculate_timeout(struct spi_imx_data *spi_imx, int size) > +{ > + unsigned long coef1 = 1; > + unsigned long coef2 = MSEC_PER_SEC; > + unsigned long timeout = 0; > + > + /* Swap coeficients to avoid div by 0 */ > + if (spi_imx->spi_bus_clk < MSEC_PER_SEC) { > + coef1 = MSEC_PER_SEC; > + coef2 = 1; > + } > + > + /* Time with actual data transfer */ > + timeout += DIV_ROUND_UP(8 * size * coef1, > + spi_imx->spi_bus_clk / coef2); > + > + /* Take CS change delay related to HW */ > + timeout += DIV_ROUND_UP((size - 1) * 4 * coef1, > + spi_imx->spi_bus_clk / coef2); > + > + /* Add extra second for scheduler related activities */ > + timeout += MSEC_PER_SEC; > + > + /* Double calculated timeout */ > + return msecs_to_jiffies(2 * timeout); > +} > + > static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, > struct spi_transfer *transfer) > { > struct dma_async_tx_descriptor *desc_tx = NULL, *desc_rx = NULL; > int ret; > unsigned long timeout; > + unsigned long transfer_timeout; > const int left = transfer->len % spi_imx->wml; > struct spi_master *master = spi_imx->bitbang.master; > struct sg_table *tx = &transfer->tx_sg, *rx = &transfer->rx_sg; > @@ -947,9 +976,11 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, > dma_async_issue_pending(master->dma_tx); > spi_imx->devtype_data->trigger(spi_imx); > > + transfer_timeout = spi_imx_calculate_timeout(spi_imx, transfer->len); > + > /* Wait SDMA to finish the data transfer.*/ > timeout = wait_for_completion_timeout(&spi_imx->dma_tx_completion, > - IMX_DMA_TIMEOUT); > + transfer_timeout); > if (!timeout) { > pr_warn("%s %s: I/O Error in DMA TX\n", > dev_driver_string(&master->dev), > @@ -957,8 +988,10 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, > dmaengine_terminate_all(master->dma_tx); > dmaengine_terminate_all(master->dma_rx); > } else { > + transfer_timeout = spi_imx_calculate_timeout(spi_imx, > + spi_imx->wml * 2); *2 again here? Although spi_imx_calculate_timeout has double it. > timeout = wait_for_completion_timeout( > - &spi_imx->dma_rx_completion, IMX_DMA_TIMEOUT); > + &spi_imx->dma_rx_completion, transfer_timeout); > if (!timeout) { > pr_warn("%s %s: I/O Error in DMA RX\n", > dev_driver_string(&master->dev), > @@ -980,7 +1013,7 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, > spi_imx->devtype_data->intctrl(spi_imx, MXC_INT_TCEN); > > timeout = wait_for_completion_timeout( > - &spi_imx->xfer_done, IMX_DMA_TIMEOUT); > + &spi_imx->xfer_done, transfer_timeout); > if (!timeout) { > pr_warn("%s %s: I/O Error in RX tail\n", > dev_driver_string(&master->dev), > -- > 2.5.2 >