From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751336AbbKNKB7 (ORCPT ); Sat, 14 Nov 2015 05:01:59 -0500 Received: from mail-lb0-f182.google.com ([209.85.217.182]:34638 "EHLO mail-lb0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751068AbbKNKB4 (ORCPT ); Sat, 14 Nov 2015 05:01:56 -0500 Subject: Re: [PATCH v3 2/7] spi: imx: replace fixed timeout with calculated one To: Sascha Hauer References: <1446388901-6073-1-git-send-email-anton.bondarenko.sama@gmail.com> <1446388901-6073-3-git-send-email-anton.bondarenko.sama@gmail.com> <20151105084723.GI8526@pengutronix.de> <56425176.9060805@gmail.com> <20151111081215.GQ8526@pengutronix.de> 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 From: Anton Bondarenko Message-ID: <56470691.4020607@gmail.com> Date: Sat, 14 Nov 2015 11:01:53 +0100 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: <20151111081215.GQ8526@pengutronix.de> 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 11.11.2015 09:12, Sascha Hauer wrote: > On Tue, Nov 10, 2015 at 09:20:06PM +0100, Anton Bondarenko wrote: >> >> >> On 05.11.2015 09:47, Sascha Hauer wrote: >>>> @@ -890,12 +891,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); >>>> +} >>> >>> I think you can simplify this function to: >>> >>> timeout = size * 8 / spi_imx->spi_bus_clk; >>> timeout += 2; >>> >>> return msecs_to_jiffies(timeout * MSEC_PER_SEC); >>> >>> The rationale is that when size * 8 / spi_imx->spi_bus_clk is 0 then we >>> can add another second and be fine. No need to calculate this more >>> accurate, in the end it's important to catch the timeout. If we do this >>> one or two seconds too late doesn't matter. >>> >>> Sascha >>> >> >> Sascha, >> >> What about something like this: >> timeout = size * (8 + 4) / spi_imx->spi_bus_clk; >> timeout += 2; >> >> return msecs_to_jiffies(2 * timeout * MSEC_PER_SEC); >> >> I think we still need to take into account 4 CLKs between each burst. > > Fine with me. > > Sascha > > Tested new implementation successfully. Will be V4 patchset. Does anyone has other comments regarding this commit? Regards, Anton