Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Fri, 22 Apr 2022 22:19:43 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     Maxime Chevallier <maxime.chevallier@bootlin.com>
Cc:     davem@davemloft.net, Rob Herring <robh+dt@kernel.org>,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, thomas.petazzoni@bootlin.com,
        Florian Fainelli <f.fainelli@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        linux-arm-kernel@lists.infradead.org,
        Vladimir Oltean <vladimir.oltean@nxp.com>,
        Luka Perkov <luka.perkov@sartura.hr>,
        Robert Marko <robert.marko@sartura.hr>
Subject: Re: [PATCH net-next 1/5] net: ipqess: introduce the Qualcomm IPQESS
 driver
Message-ID: <YmMN37VjQNwhLDuX@lunn.ch>
References: <20220422180305.301882-1-maxime.chevallier@bootlin.com>
 <20220422180305.301882-2-maxime.chevallier@bootlin.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220422180305.301882-2-maxime.chevallier@bootlin.com>
Precedence: bulk

> +static int ipqess_axi_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct net_device *netdev;
> +	phy_interface_t phy_mode;
> +	struct resource *res;
> +	struct ipqess *ess;
> +	int i, err = 0;
> +
> +	netdev = devm_alloc_etherdev_mqs(&pdev->dev, sizeof(struct ipqess),
> +					 IPQESS_NETDEV_QUEUES,
> +					 IPQESS_NETDEV_QUEUES);
> +	if (!netdev)
> +		return -ENOMEM;
> +
> +	ess = netdev_priv(netdev);
> +	ess->netdev = netdev;
> +	ess->pdev = pdev;
> +	spin_lock_init(&ess->stats_lock);
> +	SET_NETDEV_DEV(netdev, &pdev->dev);
> +	platform_set_drvdata(pdev, netdev);

....

> +
> +	ipqess_set_ethtool_ops(netdev);
> +
> +	err = register_netdev(netdev);
> +	if (err)
> +		goto err_out;

Before register_netdev() even returns, your devices can be in use, the
open callback called and packets sent. This is particularly true for
NFS root. Which means any setup done after this is probably wrong.

> +
> +	err = ipqess_hw_init(ess);
> +	if (err)
> +		goto err_out;
> +
> +	for (i = 0; i < IPQESS_NETDEV_QUEUES; i++) {
> +		int qid;
> +
> +		netif_tx_napi_add(netdev, &ess->tx_ring[i].napi_tx,
> +				  ipqess_tx_napi, 64);
> +		netif_napi_add(netdev,
> +			       &ess->rx_ring[i].napi_rx,
> +			       ipqess_rx_napi, 64);
> +
> +		qid = ess->tx_ring[i].idx;
> +		err = devm_request_irq(&ess->netdev->dev, ess->tx_irq[qid],
> +				       ipqess_interrupt_tx, 0,
> +				       ess->tx_irq_names[qid],
> +				       &ess->tx_ring[i]);
> +		if (err)
> +			goto err_out;
> +
> +		qid = ess->rx_ring[i].idx;
> +		err = devm_request_irq(&ess->netdev->dev, ess->rx_irq[qid],
> +				       ipqess_interrupt_rx, 0,
> +				       ess->rx_irq_names[qid],
> +				       &ess->rx_ring[i]);
> +		if (err)
> +			goto err_out;
> +	}

All this should probably go before netdev_register().

> +static int ipqess_get_strset_count(struct net_device *netdev, int sset)
> +{
> +	switch (sset) {
> +	case ETH_SS_STATS:
> +		return ARRAY_SIZE(ipqess_stats);
> +	default:
> +		netdev_dbg(netdev, "%s: Invalid string set", __func__);

Unsupported would be better than invalid.

> +		return -EOPNOTSUPP;
> +	}
> +}

  Andrew