Messages in this thread | | | From | Vladimir Oltean <> | Date | Tue, 7 May 2019 16:59:10 +0300 | Subject | Re: [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
| |