lkml.org 
[lkml]   [2019]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH][next] net: dsa: sja1105: fix check on while loop exit
On Tue, 7 May 2019 at 13:34, Colin King <colin.king@canonical.com> wrote:
>
> From: Colin Ian King <colin.king@canonical.com>
>
> The while-loop exit condition check is not correct; the
> loop should continue if the returns from the function calls are
> negative or the CRC status returns are invalid. Currently it
> is ignoring the returns from the function calls. Fix this by
> removing the status return checks and only break from the loop
> at the very end when we know that all the success condtions have
> been met.
>
> Kudos to Dan Carpenter for describing the correct fix.
>
> Addresses-Coverity: ("Uninitialized scalar variable")
> Fixes: 8aa9ebccae87 ("net: dsa: Introduce driver for NXP SJA1105 5-port L2 switch")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>
> V2: Discard my broken origina fix. Use correct fix as described by
> Dan Carpenter.
> ---
> drivers/net/dsa/sja1105/sja1105_spi.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
> index 244a94ccfc18..40ac696adf63 100644
> --- a/drivers/net/dsa/sja1105/sja1105_spi.c
> +++ b/drivers/net/dsa/sja1105/sja1105_spi.c
> @@ -465,9 +465,11 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
> dev_err(dev, "Switch reported that configuration is "
> "invalid, retrying...\n");
> continue;
> +
> }
> - } while (--retries && (status.crcchkl == 1 || status.crcchkg == 1 ||
> - status.configs == 0 || status.ids == 1));
> + /* Success! */
> + break;
> + } while (--retries);
>
> if (!retries) {
> rc = -EIO;
> --
> 2.20.1
>

Hi Colin, Dan,

Thanks for the fix.
This portion below needs to be changed as well: "else if (retries !=
RETRIES - 1)" should become "else if (retries != RETRIES)" because on
success, it now does not decrement retries due to the early break.
Otherwise this message gets printed: "Succeeded after 0 tried" which
was not the original intention.
Please also remove the extraneous newline introduced at 468.

Tested-by: Vladimir Oltean <olteanv@gmail.com>

-Vladimir

\
 
 \ /
  Last update: 2019-05-07 16:00    [W:0.039 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site