On Thu, May 22, 2025 at 11:41:00PM +0530, ALOK TIWARI wrote:
> Hi Simon,
>
> Thanks for Your review.
>
> On 22-05-2025 22:51, Simon Horman wrote:
> > > @@ -1754,7 +1754,7 @@ ixgbe_setup_mac_link_sfp_n(struct ixgbe_hw *hw,
> > > ixgbe_link_speed speed,
> > > ret_val = ixgbe_supported_sfp_modules_X550em(hw, &setup_linear);
> > > /* If no SFP module present, then return success. Return
> > > success since
> > > - * SFP not present error is not excepted in the setup MAC link flow.
> > > + * SFP not present error is not accepted in the setup MAC link flow.
> > I wonder if "excepted" was supposed to be "expected".
>
>
> Yes, "expected" definitely reads more naturally. However, I noticed that in
> one place, the comment uses "accepted" instead — perhaps to imply a policy
> or behavior enforcement.
Understood. I did hesitate in writing my previous email as I'm not entirely
sure what the intention was. I do agree that accepted makes sense.
And I'm happy to keep that in the absence of more information.
>
> ------------------
> static int
> ixgbe_setup_mac_link_sfp_x550em(struct ixgbe_hw *hw,
> ixgbe_link_speed speed,
> __always_unused bool
> autoneg_wait_to_complete)
> {
> bool setup_linear = false;
> u16 reg_slice, reg_val;
> int status;
>
> /* Check if SFP module is supported and linear */
> status = ixgbe_supported_sfp_modules_X550em(hw, &setup_linear);
>
> /* If no SFP module present, then return success. Return success
> since
> * there is no reason to configure CS4227 and SFP not present error
> is
> * not accepted in the setup MAC link flow.
> */
> if (status == -ENOENT)
> --------------------
>
> >
> > > */
> > > if (ret_val == -ENOENT)
> > > return 0;
> > > @@ -1804,7 +1804,7 @@ ixgbe_setup_mac_link_sfp_x550a(struct ixgbe_hw *hw,
> > > ixgbe_link_speed speed,
> > > ret_val = ixgbe_supported_sfp_modules_X550em(hw, &setup_linear);
> > > /* If no SFP module present, then return success. Return
> > > success since
> > > - * SFP not present error is not excepted in the setup MAC link flow.
> > > + * SFP not present error is not accepted in the setup MAC link flow.
> > Ditto.
> >
> > > */
> > > if (ret_val == -ENOENT)
> > > return 0;
> > The above notwithstanding, this looks good to me.
> >
> > Reviewed-by: Simon Horman<[email protected]>
>
>
> Thanks,
> Alok
>
