> -----Original Message----- > From: je...@silicom-usa.com <je...@silicom-usa.com> > Sent: Tuesday, March 8, 2022 06:34 > To: dev@dpdk.org > Cc: Stephen Douthit <steph...@silicom-usa.com>; Daly, Jeff > <je...@silicom-usa.com>; Wang, Haiyue > <haiyue.w...@intel.com> > Subject: [PATCH] net/ixgbe: Retry SFP ID read field to handle misbehaving SFPs > > From: Stephen Douthit <steph...@silicom-usa.com> > > Some XGS-PON SFPs have been observed ACKing I2C reads and returning > uninitialized garbage while their uC boots. This can lead to the SFP ID > code marking an otherwise working SFP module as unsupported if a bogus > ID value is read while its internal PHY/microcontroller is still > booting. > > Retry the ID read several times looking not just for NAK, but also for a > valid ID field. > > Since the device isn't NAKing the trasanction the existing longer retry > code in ixgbe_read_i2c_byte_generic_int() doesn't apply here. > > Signed-off-by: Stephen Douthit <steph...@silicom-usa.com> > Signed-off-by: Jeff Daly <je...@silicom-usa.com> > --- > drivers/net/ixgbe/base/ixgbe_phy.c | 31 ++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c > b/drivers/net/ixgbe/base/ixgbe_phy.c > index d8d51d2c3f..27bce066a1 100644 > --- a/drivers/net/ixgbe/base/ixgbe_phy.c > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c > @@ -1275,6 +1275,7 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw > *hw)
> + for (id_reads = 0; id_reads < 5; id_reads++) { > + status = hw->phy.ops.read_i2c_eeprom(hw, > + IXGBE_SFF_IDENTIFIER, > + &identifier); > > - if (status != IXGBE_SUCCESS) > + DEBUGOUT("status %d, id %d\n", status, identifier); DEBUGOUT("status %d, SFF identifier 0x%x\n", status, identifier); Since "#define IXGBE_SFF_IDENTIFIER_SFP 0x3" is hex value, and the ID is too simple. > + if (!status && > + identifier == IXGBE_SFF_IDENTIFIER_SFP) > + break; > + } > + > + if (status != IXGBE_SUCCESS) { > + DEBUGOUT("Failed SFF ID read (%d attempts)\n", id_reads); > goto err_read_i2c_eeprom; > + } This DEBUGOUT can be removed, it's redundant, every call for hw->phy.ops.read_i2c_eeprom has added the DEBUOUT. > > if (identifier != IXGBE_SFF_IDENTIFIER_SFP) { > hw->phy.type = ixgbe_phy_sfp_unsupported; > -- > 2.25.1