Re: [External] : Re: [PATCH] ixgbe: Fix typos and clarify comments in X550 driver code

[ALOK TIWARI <alok.a.tiwari@oracle.com>]

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.

------------------
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<horms@kernel.org>


Thanks,
Alok