RE: [net-next,v1,3/7] net: wangxun: move reusable PCI driver ops functions into libwx

["Jiawen Wu" <jiawenwu@trustnetic.com>]

On Wed, Feb 4, 2026 10:23 AM, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email says "Sending this out without checking since you have to
> repost. Please note that the AI code review may be wrong especially
> around defensive programming (which we recommend against)."
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: wangxun: move reusable PCI driver ops functions into libwx
> 
> This commit refactors PCI driver ops functions (suspend, resume, shutdown)
> from ngbe_main.c into the shared libwx library, adding function pointers
> wx->close_suspend() and adjusting wx->do_reset() to support this.
> 
> > diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> > --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> > +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> 
> [ ... ]
> 
> > +int wx_resume(struct pci_dev *pdev)
> > +{
> > +	struct net_device *netdev;
> > +	struct wx *wx;
> > +	u32 err;
> > +
> > +	wx = pci_get_drvdata(pdev);
> > +	netdev = wx->netdev;
> > +
> > +	err = pci_enable_device_mem(pdev);
> > +	if (err) {
> > +		wx_err(wx, "Cannot enable PCI device from suspend\n");
> > +		return err;
> > +	}
> > +	pci_set_master(pdev);
> > +	device_wakeup_disable(&pdev->dev);
> > +
> > +	wx->do_reset(netdev, false);
> > +	rtnl_lock();
> > +
> > +	err = wx_init_interrupt_scheme(wx);
> > +	if (!err && netif_running(netdev))
> > +		err = netdev->netdev_ops->ndo_open(netdev);
> > +	if (!err)
> > +		netif_device_attach(netdev);
> > +	rtnl_unlock();
> > +
> > +	return 0;
> > +}
> 
> Does wx_resume() need to return the error value stored in err?
> 
> The function assigns errors from wx_init_interrupt_scheme() and ndo_open()
> to the err variable, and correctly skips netif_device_attach() when errors
> occur, but then unconditionally returns 0 instead of returning err.
> 
> The early error handling for pci_enable_device_mem() correctly returns err,
> but errors from the later calls are lost. This would tell the PM core that
> resume succeeded when it actually failed.

OK. I'll correct it.