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

[Jakub Kicinski <kuba@kernel.org>]

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.
-- 
pw-bot: cr