Received-SPF: pass (google.com: domain of linux-kernel+bounces-162565-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Jernej =?utf-8?B?xaBrcmFiZWM=?= <jernej.skrabec@gmail.com>
To: linux-spi@vger.kernel.org, Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>,
 Mark Brown <broonie@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
 Samuel Holland <samuel@sholland.org>, linux-arm-kernel@lists.infradead.org,
 linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org
Subject:
 Re: [PATCH 7/8] spi: sun6i: use 'time_left' variable with
 wait_for_completion_timeout()
Date: Mon, 29 Apr 2024 17:20:14 +0200
Message-ID: <4885599.GXAFRqVoOG@jernej-laptop>
In-Reply-To: <20240429112843.67628-8-wsa+renesas@sang-engineering.com>
References:
 <20240429112843.67628-1-wsa+renesas@sang-engineering.com>
 <20240429112843.67628-8-wsa+renesas@sang-engineering.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="utf-8"

Dne ponedeljek, 29. april 2024 ob 13:28:40 GMT +2 je Wolfram Sang napisal(a):
> There is a confusing pattern in the kernel to use a variable named 'timeout' to
> store the result of wait_for_completion_timeout() causing patterns like:
> 
> 	timeout = wait_for_completion_timeout(...)
> 	if (!timeout) return -ETIMEDOUT;
> 
> with all kinds of permutations. Use 'time_left' as a variable to make the code
> self explaining.
> 
> Fix to the proper variable type 'unsigned long' while here.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

> ---
>  drivers/spi/spi-sun6i.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
> index cd018ea1abf1..7bbe7ea0d66a 100644
> --- a/drivers/spi/spi-sun6i.c
> +++ b/drivers/spi/spi-sun6i.c
> @@ -277,7 +277,8 @@ static int sun6i_spi_transfer_one(struct spi_controller *host,
>  				  struct spi_transfer *tfr)
>  {
>  	struct sun6i_spi *sspi = spi_controller_get_devdata(host);
> -	unsigned int div, div_cdr1, div_cdr2, timeout;
> +	unsigned int div, div_cdr1, div_cdr2;
> +	unsigned long time_left;
>  	unsigned int start, end, tx_time;
>  	unsigned int trig_level;
>  	unsigned int tx_len = 0, rx_len = 0, nbits = 0;
> @@ -488,26 +489,26 @@ static int sun6i_spi_transfer_one(struct spi_controller *host,
>  
>  	tx_time = spi_controller_xfer_timeout(host, tfr);
>  	start = jiffies;
> -	timeout = wait_for_completion_timeout(&sspi->done,
> -					      msecs_to_jiffies(tx_time));
> +	time_left = wait_for_completion_timeout(&sspi->done,
> +						msecs_to_jiffies(tx_time));
>  
>  	if (!use_dma) {
>  		sun6i_spi_drain_fifo(sspi);
>  	} else {
> -		if (timeout && rx_len) {
> +		if (time_left && rx_len) {
>  			/*
>  			 * Even though RX on the peripheral side has finished
>  			 * RX DMA might still be in flight
>  			 */
> -			timeout = wait_for_completion_timeout(&sspi->dma_rx_done,
> -							      timeout);
> -			if (!timeout)
> +			time_left = wait_for_completion_timeout(&sspi->dma_rx_done,
> +							      time_left);
> +			if (!time_left)
>  				dev_warn(&host->dev, "RX DMA timeout\n");
>  		}
>  	}
>  
>  	end = jiffies;
> -	if (!timeout) {
> +	if (!time_left) {
>  		dev_warn(&host->dev,
>  			 "%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
>  			 dev_name(&spi->dev), tfr->len, tfr->speed_hz,
>