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

Simon Horman
2026-04-03T12:38:26Z

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
> >