Re: [PATCH net] ice: fix ice_init_link() error return preventing probe

[Simon Horman <horms@kernel.org>]

On Fri, Apr 03, 2026 at 01:36:14PM +0100, Simon Horman wrote:
> On Fri, Mar 27, 2026 at 08:23:29AM +0100, Aleksandr Loktionov wrote:
> > From: Paul Greenwalt <paul.greenwalt@intel.com>
> > 
> > ice_init_link() can return an error status from ice_update_link_info()
> > or ice_init_phy_user_cfg(), causing probe to fail.
> > 
> > An incorrect NVM update procedure can result in link/PHY errors, and
> > the recommended resolution is to update the NVM using the correct
> > procedure. If the driver fails probe due to link errors, the user
> > cannot update the NVM to recover. The link/PHY errors logged are
> > non-fatal: they are already annotated as 'not a fatal error if this
> > fails'.
> > 
> > Since none of the errors inside ice_init_link() should prevent probe
> > from completing, convert it to void and remove the error check in the
> > caller. All failures are already logged; callers have no meaningful
> > recovery path for link init errors.
> > 
> > Fixes: 5b246e533d01 ("ice: split probe into smaller functions")

Sorry, one more thing.

I don't think this problem pre-dated the cited commit.

> > Cc: stable@vger.kernel.org
> > Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
> > Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> 
> The nit below notwithstanding, this looks good to me.
> 
> Reviewed-by: Simon Horman <horms@kernel.org>
> 
> > ---
> > 
> >  drivers/net/ethernet/intel/ice/ice_main.c | 12 +++---------
> >  1 file changed, 3 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> 
> ...
> 
> > @@ -5043,9 +5039,7 @@ static int ice_init(struct ice_pf *pf)
> >  
> >  	ice_init_wakeup(pf);
> >  
> > -	err = ice_init_link(pf);
> > -	if (err)
> > -		goto err_init_link;
> 
> I think now would be a good time to rename the err_init_link label,
> after what it does, rather than where it is (no longer) jumped from.
> And perhaps others in this function too
> 
> > +	ice_init_link(pf);
> >  
> >  	err = ice_send_version(pf);
> >  	if (err)
> > -- 
> > 2.52.0
> >