Re: [net-next,v28,3/8] eea: probe the netdevice and create adminq

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
eea: probe the netdevice and create adminq

Add basic driver framework for the Alibaba Elastic Ethernet Adapter(EEA).
This commit creates and registers the netdevice after PCI probe, and
initializes the admin queue to send commands to the device.

> diff --git a/drivers/net/ethernet/alibaba/eea/eea_net.c b/drivers/net/ethernet/alibaba/eea/eea_net.c
> new file mode 100644
> index 000000000000..31cb9ca5b408
> --- /dev/null
> +++ b/drivers/net/ethernet/alibaba/eea/eea_net.c
> @@ -0,0 +1,202 @@

[ ... ]

> +static int eea_netdev_init_features(struct net_device *netdev,
> +				    struct eea_net *enet,
> +				    struct eea_device *edev)
> +{

[ ... ]

> +	netif_carrier_on(netdev);
> +
> +err_free:
> +	kfree(cfg);
> +	return err;
> +}

[ ... ]

> +int eea_net_probe(struct eea_device *edev)
> +{
> +	struct eea_net *enet;
> +	int err = -ENOMEM;
> +
> +	enet = eea_netdev_alloc(edev, edev->rx_num);
> +	if (!enet)
> +		return -ENOMEM;
> +
> +	err = eea_create_adminq(enet, edev->rx_num + edev->tx_num);
> +	if (err)
> +		goto err_free_netdev;
> +
> +	err = eea_adminq_config_host_info(enet);
> +	if (err)
> +		goto err_reset_dev;
> +
> +	err = eea_netdev_init_features(enet->netdev, enet, edev);
> +	if (err)
> +		goto err_reset_dev;
> +
> +	err = register_netdev(enet->netdev);
> +	if (err)
> +		goto err_reset_dev;
> +
> +	netif_carrier_off(enet->netdev);

Looking at the initialization sequence, eea_netdev_init_features() calls
netif_carrier_on() before the device is registered, then eea_net_probe()
calls netif_carrier_off() immediately after register_netdev().

This creates a brief window after registration where userspace can observe
the carrier as UP before it's set to OFF. Is the netif_carrier_on() call
in eea_netdev_init_features() necessary? The standard driver pattern is to
call netif_carrier_off() before or immediately after register_netdev(), not
to set carrier on first and then override it.
-- 
pw-bot: cr