[11/21] net/ixgbe/base: modify loop accounting for retries
Checks
Commit Message
The condition for comparing retry against max_retry was flawed in the
do-while loops. For the case where retry was initialized to 0 and
max_retry was initialized to 1, we'd break out of the loop at the
condition when the intent is to retry the code at least once.
Otherwise, the loop is unnecessary. The other places have a larger
max_retry so code would get run multiple times (if necessary), but not
to the intended extent.
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 | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Comments
On 6/12/2020 4:24 AM, Guinan Sun wrote:
> The condition for comparing retry against max_retry was flawed in the
> do-while loops. For the case where retry was initialized to 0 and
> max_retry was initialized to 1, we'd break out of the loop at the
> condition when the intent is to retry the code at least once.
> Otherwise, the loop is unnecessary. The other places have a larger
> max_retry so code would get run multiple times (if necessary), but not
> to the intended extent.
>
> 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 | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
> index 9bb24f1ef..823cf161e 100644
> --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> @@ -143,7 +143,7 @@ s32 ixgbe_read_i2c_combined_generic_int(struct ixgbe_hw *hw, u8 addr, u16 reg,
> else
> DEBUGOUT("I2C byte read combined error.\n");
> retry++;
> - } while (retry < max_retry);
> + } while (retry <= max_retry);
>
> return IXGBE_ERR_I2C;
Ahh, previous patch becomes correct with this change, can you please combine
them? No need to break first and fix later.
Hi Ferruh
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 8:00 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 11/21] net/ixgbe/base: modify loop accounting
> for retries
>
> On 6/12/2020 4:24 AM, Guinan Sun wrote:
> > The condition for comparing retry against max_retry was flawed in the
> > do-while loops. For the case where retry was initialized to 0 and
> > max_retry was initialized to 1, we'd break out of the loop at the
> > condition when the intent is to retry the code at least once.
> > Otherwise, the loop is unnecessary. The other places have a larger
> > max_retry so code would get run multiple times (if necessary), but not
> > to the intended extent.
> >
> > 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 | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c
> > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > index 9bb24f1ef..823cf161e 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > @@ -143,7 +143,7 @@ s32 ixgbe_read_i2c_combined_generic_int(struct
> ixgbe_hw *hw, u8 addr, u16 reg,
> > else
> > DEBUGOUT("I2C byte read combined error.\n");
> > retry++;
> > - } while (retry < max_retry);
> > + } while (retry <= max_retry);
> >
> > return IXGBE_ERR_I2C;
>
> Ahh, previous patch becomes correct with this change, can you please combine
> them? No need to break first and fix later.
Patch V2 will fix it.
@@ -143,7 +143,7 @@ s32 ixgbe_read_i2c_combined_generic_int(struct ixgbe_hw *hw, u8 addr, u16 reg,
else
DEBUGOUT("I2C byte read combined error.\n");
retry++;
- } while (retry < max_retry);
+ } while (retry <= max_retry);
return IXGBE_ERR_I2C;
}
@@ -208,7 +208,7 @@ s32 ixgbe_write_i2c_combined_generic_int(struct ixgbe_hw *hw, u8 addr, u16 reg,
else
DEBUGOUT("I2C byte write combined error.\n");
retry++;
- } while (retry < max_retry);
+ } while (retry <= max_retry);
return IXGBE_ERR_I2C;
}
@@ -2062,7 +2062,7 @@ STATIC s32 ixgbe_read_i2c_byte_generic_int(struct ixgbe_hw *hw, u8 byte_offset,
else
DEBUGOUT("I2C byte read error.\n");
retry++;
- } while (retry < max_retry);
+ } while (retry <= max_retry);
return status;
}
@@ -2165,7 +2165,7 @@ STATIC s32 ixgbe_write_i2c_byte_generic_int(struct ixgbe_hw *hw, u8 byte_offset,
else
DEBUGOUT("I2C byte write error.\n");
retry++;
- } while (retry < max_retry);
+ } while (retry <= max_retry);
if (lock)
hw->mac.ops.release_swfw_sync(hw, swfw_mask);