On Tue, 7 May 2019 at 13:34, Colin King <colin.king@xxxxxxxxxxxxx> wrote:
>
> From: Colin Ian King <colin.king@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxx>
> ---
>
> 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@xxxxxxxxx>
-Vladimir
