> -----Original Message----- > From: Jeff Daly <je...@silicom-usa.com> > Sent: Monday, May 30, 2022 9:47 PM > To: Zhang, Qi Z <qi.z.zh...@intel.com>; dev@dpdk.org; Yang, Qiming > <qiming.y...@intel.com>; Wu, Wenjun1 <wenjun1...@intel.com> > Cc: Stephen Douthit <steph...@silicom-usa.com> > Subject: RE: [PATCH 1/3] ixgbe: make link update thread periodic > > > > > -----Original Message----- > > From: Zhang, Qi Z <qi.z.zh...@intel.com> > > Sent: Sunday, May 29, 2022 7:25 PM > > To: Jeff Daly <je...@silicom-usa.com>; dev@dpdk.org; Yang, Qiming > > <qiming.y...@intel.com>; Wu, Wenjun1 <wenjun1...@intel.com> > > Cc: Stephen Douthit <steph...@silicom-usa.com> > > Subject: RE: [PATCH 1/3] ixgbe: make link update thread periodic > > > > Caution: This is an external email. Please take care when clicking > > links or opening attachments. > > > > > > > -----Original Message----- > > > From: Jeff Daly <je...@silicom-usa.com> > > > Sent: Friday, May 20, 2022 2:03 AM > > > To: dev@dpdk.org; Yang, Qiming <qiming.y...@intel.com>; Wu, Wenjun1 > > > <wenjun1...@intel.com> > > > Cc: Stephen Douthit <steph...@silicom-usa.com> > > > Subject: [PATCH 1/3] ixgbe: make link update thread periodic > > > > > > Rather than run-to-completion, allow the link update thread to be > > periodic. > > > This will set the stage for properly handling hot-plugging. > > > > Could you explain more about what's the hot-plugging issue with > > run-to- completion you try to fix? > > > > it doesn't work right when you have SFPs. (at least not on our platform or on > an > 82599 dual SFP add-in card we have). run-to-completion only works 1x. if > you remove and plug in a different SFP it doesn't work. This patch series > should have been taking in context with the original SFP hotplug patch but > apparently since I can't ever seem to get the patch submission threading to do > what I mean perhaps some context has gone missing. the SFP hotplug fix has > been in the queue since Dec 2021, has been reworked several times, has gone > through a change in Intel maintainership. this patch series makes the SFP hot > plugging work like the Intel kernel driver does. > > > > > > > Signed-off-by: Jeff Daly <je...@silicom-usa.com> > > > Inspired-by: Stephen Douthit <steph...@silicom-usa.com> > > > --- > > > drivers/net/ixgbe/base/ixgbe_common.c | 4 +- > > > drivers/net/ixgbe/ixgbe_ethdev.c | 180 ++++++++++---------------- > > > 2 files changed, 71 insertions(+), 113 deletions(-) > > > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_common.c > > > b/drivers/net/ixgbe/base/ixgbe_common.c > > > index aa843bd5c4a5..712062306491 100644 > > > --- a/drivers/net/ixgbe/base/ixgbe_common.c > > > +++ b/drivers/net/ixgbe/base/ixgbe_common.c > > > @@ -4154,8 +4154,8 @@ s32 ixgbe_check_mac_link_generic(struct > > > ixgbe_hw *hw, ixgbe_link_speed *speed, > > > break; > > > case ixgbe_mac_X550EM_x: > > > case ixgbe_mac_X550EM_a: > > > - sfp_cage_full = IXGBE_READ_REG(hw, IXGBE_ESDP) > > > & > > > - IXGBE_ESDP_SDP0; > > > + sfp_cage_full = !(IXGBE_READ_REG(hw, > > > + IXGBE_ESDP) > > > & > > > + IXGBE_ESDP_SDP0); > > > break; > > > default: > > > /* sanity check - No SFP+ devices here */ diff > > > --git > > > > Looks like you change the behavior of link status check for x550. > > I'm not an ixgbe expert, but I know this is not kernel driver's > > implementation. > > > > sigh. this was supposed to be part of a different patch which also had some > question about functionality. the SDP0 bit check doesn't specifically need to > be a check for a '1', since the bit reflects the state of the pin on the > platform. > Intel's platform implementations have an inverter on the board to switch the > state. > MOD_ABS from an SFP will be '0' when an SFP is plugged in. with an inverter > in the platform the signal will be '1' when an SFP is plugged in. there's no > guidance from Intel's platform design guide that an inverter needs to be > between the SFP and the NIC SDP pin so having it only follow Intel's platform > implementations is hard to justify.
OK, I assume the existing code should be proved for the normal scenario (remove and plug in with the same SFP) So how can we guarantee this change will not break something? Could you help me to understand why we should ignore the difference when SDP0 is 1 in normal scenario? Before change we will continue to read the link register and return the link speed. after change we return SPEED_UNKNOWN immediately . > > > So do you think this is a fix for both DPDK and kernel driver? if it > > is, please move this change into a separate patch and we need to > > reach the right expert to approve this. > > > > > > no, as explained above.
