> -----Original Message----- > From: Luca Boccassi [mailto:[email protected]] > Sent: Friday, November 2, 2018 10:21 AM > To: Zhang, Qi Z <[email protected]>; [email protected] > Cc: Lu, Wenzhuo <[email protected]>; Ananyev, Konstantin > <[email protected]>; [email protected] > Subject: Re: [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified PHY > > On Fri, 2018-11-02 at 14:11 +0000, Zhang, Qi Z wrote: > > > -----Original Message----- > > > From: Luca Boccassi [mailto:[email protected]] > > > Sent: Thursday, November 1, 2018 9:04 AM > > > To: [email protected] > > > Cc: Lu, Wenzhuo <[email protected]>; Ananyev, Konstantin > > > <[email protected]>; Zhang, Qi Z <[email protected]>; > > > Luca Boccassi <[email protected]>; [email protected] > > > Subject: [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified > > > PHY > > > > > > ixgbe_identify_phy_x550em() was missing the code to handle > > > unidentified PHY that has been there in 82599 so it was not able to > > > complete initialization of ixgbe sequence if no sfp plugged in. > > > Port it over to return an appropriate type and complete init > > > sequence properly. > > > > > > Fixes: d2e72774e58c ("ixgbe/base: support X550") > > > Cc: [email protected] > > > > > > Signed-off-by: Luca Boccassi <[email protected]> > > > --- > > > v2: refresh to remove merge conflict with master > > > > > > drivers/net/ixgbe/base/ixgbe_x550.c | 19 ++++++++++++++++--- > > > 1 file changed, 16 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c > > > b/drivers/net/ixgbe/base/ixgbe_x550.c > > > index f7b98af52..83b394861 100644 > > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c > > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c > > > @@ -315,13 +315,21 @@ STATIC void ixgbe_setup_mux_ctl(struct > > > ixgbe_hw > > > *hw) > > > */ > > > STATIC s32 ixgbe_identify_phy_x550em(struct ixgbe_hw *hw) { > > > + s32 status; > > > + > > > hw->mac.ops.set_lan_id(hw); > > > > > > ixgbe_read_mng_if_sel_x550em(hw); > > > > > > switch (hw->device_id) { > > > case IXGBE_DEV_ID_X550EM_A_SFP: > > > - return ixgbe_identify_sfp_module_X550em(hw); > > > + status = ixgbe_identify_sfp_module_X550em(hw); > > > + /* Set PHY type none if no PHY detected */ > > > + if (hw->phy.type == ixgbe_phy_unknown) { > > > + hw->phy.type = ixgbe_phy_none; > > > + return IXGBE_SUCCESS; > > > + } > > > > Why this can't be handled at caller, why we replace phy_unknown by > > phy_none only for x550? > > Hi, thanks for the review. > > I've moved the code into the caller in v3.
Sorry, this is not what I suggested. I'm not sure for X550EM_A_SFP, it is the case that the device does not have PHY so we should correct it (by replace it with no_phy) or it is the case that we can't identify PHY so we just replace it by no_phy to bypass the check for workaround? If the second case, is it possible not to do replacement and handle it at upper layer (caller or caller's caller ... ) and keep the information more accurate? > > It's done for x550 simply because that's the hardware that we have and that > this was tested with, I didn't want to take extra risks. It's also the > hardware > that our customer reported the issue with. > > -- > Kind regards, > Luca Boccassi

