[PATCH linux dev-4.10] i2c: fsi: Fix unused variable warning

[PATCH linux dev-4.10] i2c: fsi: Fix unused variable warning

[Joel Stanley]

When there is no status bit to handle we return the uninitialised rc
variable, causing this warning:

drivers/i2c/busses/i2c-fsi.c: In function ‘fsi_i2c_xfer’:
drivers/i2c/busses/i2c-fsi.c:410:7: warning: ‘rc’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
    if (rc < 0)
       ^
drivers/i2c/busses/i2c-fsi.c:358:6: note: ‘rc’ was declared here
  int rc;
      ^~

Instead return zero, but also print a warning as this looks like an
error, as  we have checked the I2C_FSI_STAT register for
I2C_STAT_ANY_RESP, but none of the status bits were set.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/i2c/busses/i2c-fsi.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
index 4689479acf5e..b4eb1028a2ae 100644
--- a/drivers/i2c/busses/i2c-fsi.c
+++ b/drivers/i2c/busses/i2c-fsi.c
@@ -355,12 +355,12 @@ static int fsi_i2c_read_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
 static int fsi_i2c_handle_status(struct fsi_i2c_port *port,
 				 struct i2c_msg *msg, u32 status)
 {
-	int rc;
-	u8 fifo_count;
 	struct fsi_i2c_master *i2c = port->master;
-	u32 dummy = 0;
+	u8 fifo_count;
+	int rc;
 
 	if (status & I2C_STAT_ERR) {
+		u32 dummy = 0;
 		rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_ERR, &dummy);
 		if (rc)
 			return rc;
@@ -387,9 +387,12 @@ static int fsi_i2c_handle_status(struct fsi_i2c_port *port,
 			rc = -ENODATA;
 		else
 			rc = msg->len;
+		return rc;
 	}
 
-	return rc;
+	dev_warn(&port->adapter.dev, "no status to handle\n");
+
+	return 0;
 }
 
 static int fsi_i2c_wait(struct fsi_i2c_port *port, struct i2c_msg *msg,
-- 
2.11.0

Re: [PATCH linux dev-4.10] i2c: fsi: Fix unused variable warning

[Andrew Jeffery]

On Thu, Jun 1, 2017, at 16:07, Joel Stanley wrote:
> When there is no status bit to handle we return the uninitialised rc
> variable, causing this warning:
> 
> drivers/i2c/busses/i2c-fsi.c: In function ‘fsi_i2c_xfer’:
> drivers/i2c/busses/i2c-fsi.c:410:7: warning: ‘rc’ may be used
> uninitialized in
> this function [-Wmaybe-uninitialized]
>     if (rc < 0)
>        ^
> drivers/i2c/busses/i2c-fsi.c:358:6: note: ‘rc’ was declared here
>   int rc;
>       ^~
> 
> Instead return zero, but also print a warning as this looks like an
> error, as  we have checked the I2C_FSI_STAT register for
> I2C_STAT_ANY_RESP, but none of the status bits were set.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  drivers/i2c/busses/i2c-fsi.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
> index 4689479acf5e..b4eb1028a2ae 100644
> --- a/drivers/i2c/busses/i2c-fsi.c
> +++ b/drivers/i2c/busses/i2c-fsi.c
> @@ -355,12 +355,12 @@ static int fsi_i2c_read_fifo(struct fsi_i2c_port
> *port, struct i2c_msg *msg,
>  static int fsi_i2c_handle_status(struct fsi_i2c_port *port,
>  				 struct i2c_msg *msg, u32 status)
>  {
> -       int rc;
> -       u8 fifo_count;
>  	struct fsi_i2c_master *i2c = port->master;
> -       u32 dummy = 0;
> +       u8 fifo_count;
> +       int rc;
>  
>  	if (status & I2C_STAT_ERR) {
> +               u32 dummy = 0;
>  		rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_ERR, &dummy);
>  		if (rc)
>  			return rc;
> @@ -387,9 +387,12 @@ static int fsi_i2c_handle_status(struct fsi_i2c_port
> *port,
>  			rc = -ENODATA;
>  		else
>  			rc = msg->len;
> +               return rc;
>  	}
>  
> -       return rc;
> +       dev_warn(&port->adapter.dev, "no status to handle\n");
> +
> +       return 0;
>  }
>  
>  static int fsi_i2c_wait(struct fsi_i2c_port *port, struct i2c_msg *msg,
> -- 
> 2.11.0
> 

Re: [PATCH linux dev-4.10] i2c: fsi: Fix unused variable warning

[Eddie James]

On 06/01/2017 01:37 AM, Joel Stanley wrote:
> When there is no status bit to handle we return the uninitialised rc
> variable, causing this warning:
>
> drivers/i2c/busses/i2c-fsi.c: In function ‘fsi_i2c_xfer’:
> drivers/i2c/busses/i2c-fsi.c:410:7: warning: ‘rc’ may be used uninitialized in
> this function [-Wmaybe-uninitialized]
>      if (rc < 0)
>         ^
> drivers/i2c/busses/i2c-fsi.c:358:6: note: ‘rc’ was declared here
>    int rc;

Good catch, thanks.

>        ^~
>
> Instead return zero, but also print a warning as this looks like an
> error, as  we have checked the I2C_FSI_STAT register for
> I2C_STAT_ANY_RESP, but none of the status bits were set.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   drivers/i2c/busses/i2c-fsi.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
> index 4689479acf5e..b4eb1028a2ae 100644
> --- a/drivers/i2c/busses/i2c-fsi.c
> +++ b/drivers/i2c/busses/i2c-fsi.c
> @@ -355,12 +355,12 @@ static int fsi_i2c_read_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
>   static int fsi_i2c_handle_status(struct fsi_i2c_port *port,
>   				 struct i2c_msg *msg, u32 status)
>   {
> -	int rc;
> -	u8 fifo_count;
>   	struct fsi_i2c_master *i2c = port->master;
> -	u32 dummy = 0;
> +	u8 fifo_count;
> +	int rc;
>
>   	if (status & I2C_STAT_ERR) {
> +		u32 dummy = 0;
>   		rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_ERR, &dummy);
>   		if (rc)
>   			return rc;
> @@ -387,9 +387,12 @@ static int fsi_i2c_handle_status(struct fsi_i2c_port *port,
>   			rc = -ENODATA;
>   		else
>   			rc = msg->len;
> +		return rc;
>   	}
>
> -	return rc;
> +	dev_warn(&port->adapter.dev, "no status to handle\n");

This is fine with me, though it is currently impossible to hit the 
warning as we only call this function if a combined mask of the above 
checks (I2C_STAT_ERR, I2C_STAT_DAT_REQ, or I2C_STAT_CMD_COMP) is set. 
That's not very clear in the code, I admit.

Thanks.

Reviewed-by: Eddie James <eajames@linux.vnet.ibm.com>

> +
> +	return 0;
>   }
>
>   static int fsi_i2c_wait(struct fsi_i2c_port *port, struct i2c_msg *msg,