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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250523071909.GO365796@horms.kernel.org>
Date: Fri, 23 May 2025 08:19:09 +0100
From: Simon Horman <horms@...nel.org>
To: ALOK TIWARI <alok.a.tiwari@...cle.com>
Cc: anthony.l.nguyen@...el.com, przemyslaw.kitszel@...el.com,
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com,
	intel-wired-lan@...ts.osuosl.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, darren.kenny@...cle.com
Subject: Re: [External] : Re: [PATCH] ixgbe: Fix typos and clarify comments
 in X550 driver code

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<horms@...nel.org>
> 
> 
> Thanks,
> Alok
>