[10/21] net/ixgbe/base: move increments after evaluations
Checks
Commit Message
The retry variable was being incremented before it was evaluated by the
subsequent conditional against the maximum retries to figure out which
message to print. So we'll move the increment op to the end.
Signed-off-by: Jeb Cramer <jeb.j.cramer@intel.com>
Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
---
drivers/net/ixgbe/base/ixgbe_phy.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
Comments
On 6/12/2020 4:23 AM, Guinan Sun wrote:
> The retry variable was being incremented before it was evaluated by the
> subsequent conditional against the maximum retries to figure out which
> message to print. So we'll move the increment op to the end.
>
> Signed-off-by: Jeb Cramer <jeb.j.cramer@intel.com>
> Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> ---
> drivers/net/ixgbe/base/ixgbe_phy.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
> index 13f00ac67..9bb24f1ef 100644
> --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> @@ -138,11 +138,11 @@ s32 ixgbe_read_i2c_combined_generic_int(struct ixgbe_hw *hw, u8 addr, u16 reg,
> ixgbe_i2c_bus_clear(hw);
> if (lock)
> hw->mac.ops.release_swfw_sync(hw, swfw_mask);
> - retry++;
> if (retry < max_retry)
> DEBUGOUT("I2C byte read combined error - Retrying.\n");
> else
> DEBUGOUT("I2C byte read combined error.\n");
> + retry++;
> } while (retry < max_retry);
>
Isn't this wrong?
With this update "DEBUGOUT("I2C byte read combined error.\n");" log won't be
printed at all.
And instead why not keep "Retrying" log in the loop without check and move
"error" log out of loop, that looks easier to me...
Hi Ferruh
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 7:59 PM
> To: Sun, GuinanX <guinanx.sun@intel.com>; dev@dpdk.org
> Cc: Cramer, Jeb J <jeb.j.cramer@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 10/21] net/ixgbe/base: move increments after
> evaluations
>
> On 6/12/2020 4:23 AM, Guinan Sun wrote:
> > The retry variable was being incremented before it was evaluated by
> > the subsequent conditional against the maximum retries to figure out
> > which message to print. So we'll move the increment op to the end.
> >
> > Signed-off-by: Jeb Cramer <jeb.j.cramer@intel.com>
> > Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> > ---
> > drivers/net/ixgbe/base/ixgbe_phy.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c
> > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > index 13f00ac67..9bb24f1ef 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > @@ -138,11 +138,11 @@ s32 ixgbe_read_i2c_combined_generic_int(struct
> ixgbe_hw *hw, u8 addr, u16 reg,
> > ixgbe_i2c_bus_clear(hw);
> > if (lock)
> > hw->mac.ops.release_swfw_sync(hw, swfw_mask);
> > - retry++;
> > if (retry < max_retry)
> > DEBUGOUT("I2C byte read combined error -
> Retrying.\n");
> > else
> > DEBUGOUT("I2C byte read combined error.\n");
> > + retry++;
> > } while (retry < max_retry);
> >
>
> Isn't this wrong?
> With this update "DEBUGOUT("I2C byte read combined error.\n");" log won't
> be printed at all.
> And instead why not keep "Retrying" log in the loop without check and move
> "error" log out of loop, that looks easier to me...
The next patch will be combined with the current patch later, which will fix the problem.
@@ -138,11 +138,11 @@ s32 ixgbe_read_i2c_combined_generic_int(struct ixgbe_hw *hw, u8 addr, u16 reg,
ixgbe_i2c_bus_clear(hw);
if (lock)
hw->mac.ops.release_swfw_sync(hw, swfw_mask);
- retry++;
if (retry < max_retry)
DEBUGOUT("I2C byte read combined error - Retrying.\n");
else
DEBUGOUT("I2C byte read combined error.\n");
+ retry++;
} while (retry < max_retry);
return IXGBE_ERR_I2C;
@@ -203,11 +203,11 @@ s32 ixgbe_write_i2c_combined_generic_int(struct ixgbe_hw *hw, u8 addr, u16 reg,
ixgbe_i2c_bus_clear(hw);
if (lock)
hw->mac.ops.release_swfw_sync(hw, swfw_mask);
- retry++;
if (retry < max_retry)
DEBUGOUT("I2C byte write combined error - Retrying.\n");
else
DEBUGOUT("I2C byte write combined error.\n");
+ retry++;
} while (retry < max_retry);
return IXGBE_ERR_I2C;
@@ -2057,12 +2057,11 @@ STATIC s32 ixgbe_read_i2c_byte_generic_int(struct ixgbe_hw *hw, u8 byte_offset,
hw->mac.ops.release_swfw_sync(hw, swfw_mask);
msec_delay(100);
}
- retry++;
if (retry < max_retry)
DEBUGOUT("I2C byte read error - Retrying.\n");
else
DEBUGOUT("I2C byte read error.\n");
-
+ retry++;
} while (retry < max_retry);
return status;
@@ -2161,11 +2160,11 @@ STATIC s32 ixgbe_write_i2c_byte_generic_int(struct ixgbe_hw *hw, u8 byte_offset,
fail:
ixgbe_i2c_bus_clear(hw);
- retry++;
if (retry < max_retry)
DEBUGOUT("I2C byte write error - Retrying.\n");
else
DEBUGOUT("I2C byte write error.\n");
+ retry++;
} while (retry < max_retry);
if (lock)