linux-kernel - Re: [PATCH 11/15] i2c: jz4780: use 'time_left' variable with wait_for_completion

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1a72ad2d6f72805de2c99db8ba8ea984711da81b.camel@crapouillou.net>
Date: Tue, 30 Apr 2024 14:49:50 +0200
From: Paul Cercueil <paul@...pouillou.net>
To: Wolfram Sang <wsa+renesas@...g-engineering.com>, 
	linux-i2c@...r.kernel.org
Cc: Andi Shyti <andi.shyti@...nel.org>, linux-mips@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 11/15] i2c: jz4780: use 'time_left' variable with
 wait_for_completion_timeout()

Le samedi 27 avril 2024 à 22:36 +0200, Wolfram Sang a écrit :
> 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@...g-engineering.com>

Acked-by: Paul Cercueil <paul@...pouillou.net>

Cheers,
-Paul

> ---
>  drivers/i2c/busses/i2c-jz4780.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-jz4780.c
> b/drivers/i2c/busses/i2c-jz4780.c
> index 55035cca0ae5..7951891d6b97 100644
> --- a/drivers/i2c/busses/i2c-jz4780.c
> +++ b/drivers/i2c/busses/i2c-jz4780.c
> @@ -565,7 +565,7 @@ static inline int jz4780_i2c_xfer_read(struct
> jz4780_i2c *i2c,
>  				       int idx)
>  {
>  	int ret = 0;
> -	long timeout;
> +	unsigned long time_left;
>  	int wait_time = JZ4780_I2C_TIMEOUT * (len + 5);
>  	unsigned short tmp;
>  	unsigned long flags;
> @@ -600,10 +600,10 @@ static inline int jz4780_i2c_xfer_read(struct
> jz4780_i2c *i2c,
>  
>  	spin_unlock_irqrestore(&i2c->lock, flags);
>  
> -	timeout = wait_for_completion_timeout(&i2c->trans_waitq,
> -					     
> msecs_to_jiffies(wait_time));
> +	time_left = wait_for_completion_timeout(&i2c->trans_waitq,
> +						msecs_to_jiffies(wai
> t_time));
>  
> -	if (!timeout) {
> +	if (!time_left) {
>  		dev_err(&i2c->adap.dev, "irq read timeout\n");
>  		dev_dbg(&i2c->adap.dev, "send cmd count:%d  %d\n",
>  			i2c->cmd, i2c->cmd_buf[i2c->cmd]);
> @@ -627,7 +627,7 @@ static inline int jz4780_i2c_xfer_write(struct
> jz4780_i2c *i2c,
>  {
>  	int ret = 0;
>  	int wait_time = JZ4780_I2C_TIMEOUT * (len + 5);
> -	long timeout;
> +	unsigned long time_left;
>  	unsigned short tmp;
>  	unsigned long flags;
>  
> @@ -655,14 +655,14 @@ static inline int jz4780_i2c_xfer_write(struct
> jz4780_i2c *i2c,
>  
>  	spin_unlock_irqrestore(&i2c->lock, flags);
>  
> -	timeout = wait_for_completion_timeout(&i2c->trans_waitq,
> -					     
> msecs_to_jiffies(wait_time));
> -	if (timeout && !i2c->stop_hold) {
> +	time_left = wait_for_completion_timeout(&i2c->trans_waitq,
> +						msecs_to_jiffies(wai
> t_time));
> +	if (time_left && !i2c->stop_hold) {
>  		unsigned short i2c_sta;
>  		int write_in_process;
>  
> -		timeout = JZ4780_I2C_TIMEOUT * 100;
> -		for (; timeout > 0; timeout--) {
> +		time_left = JZ4780_I2C_TIMEOUT * 100;
> +		for (; time_left > 0; time_left--) {
>  			i2c_sta = jz4780_i2c_readw(i2c,
> JZ4780_I2C_STA);
>  
>  			write_in_process = (i2c_sta &
> JZ4780_I2C_STA_MSTACT) ||
> @@ -673,7 +673,7 @@ static inline int jz4780_i2c_xfer_write(struct
> jz4780_i2c *i2c,
>  		}
>  	}
>  
> -	if (!timeout) {
> +	if (!time_left) {
>  		dev_err(&i2c->adap.dev, "write wait timeout\n");
>  		ret = -EIO;
>  	}