RE: [PATCH net-next] [RFC] dpaa2-eth: Move DPAA2 Ethernet driver from staging to drivers/net

-----Original Message-----
From: Andrew Lunn [mailto:andrew@lunn.ch]
Sent: Friday, August 3, 2018 7:43 PM
To: Ioana Ciocoi Radulescu <redacted>
Cc: netdev@vger.kernel.org; davem@davemloft.net;
gregkh@linuxfoundation.org; devel@driverdev.osuosl.org; linux-
kernel@vger.kernel.org; Ioana Ciornei [off-list ref]; Laurentiu
Tudor [off-list ref]; Madalin-cristian Bucur
[off-list ref]; Horia Geanta [off-list ref]
Subject: Re: [PATCH net-next] [RFC] dpaa2-eth: Move DPAA2 Ethernet driver
from staging to drivers/net

quoted

+static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
+{
+	struct device *dev;
+	struct net_device *net_dev = NULL;
+	struct dpaa2_eth_priv *priv = NULL;
+	int err = 0;
+
+	dev = &dpni_dev->dev;
+
+	/* Net device */
+	net_dev = alloc_etherdev_mq(sizeof(*priv),

DPAA2_ETH_MAX_TX_QUEUES);

quoted

+	if (!net_dev) {
+		dev_err(dev, "alloc_etherdev_mq() failed\n");
+		return -ENOMEM;
+	}
+
+	SET_NETDEV_DEV(net_dev, dev);
+	dev_set_drvdata(dev, net_dev);
+
+	priv = netdev_priv(net_dev);
+	priv->net_dev = net_dev;
+
+	priv->iommu_domain = iommu_get_domain_for_dev(dev);
+
+	/* Obtain a MC portal */
+	err = fsl_mc_portal_allocate(dpni_dev,

FSL_MC_IO_ATOMIC_CONTEXT_PORTAL,

quoted

+				     &priv->mc_io);
+	if (err) {
+		if (err == -ENXIO)
+			err = -EPROBE_DEFER;
+		else
+			dev_err(dev, "MC portal allocation failed\n");
+		goto err_portal_alloc;
+	}
+
+	/* MC objects initialization and configuration */
+	err = setup_dpni(dpni_dev);
+	if (err)
+		goto err_dpni_setup;
+
+	err = setup_dpio(priv);
+	if (err)
+		goto err_dpio_setup;
+
+	setup_fqs(priv);
+
+	err = setup_dpbp(priv);
+	if (err)
+		goto err_dpbp_setup;
+
+	err = bind_dpni(priv);
+	if (err)
+		goto err_bind;
+
+	/* Add a NAPI context for each channel */
+	add_ch_napi(priv);
+
+	/* Percpu statistics */
+	priv->percpu_stats = alloc_percpu(*priv->percpu_stats);
+	if (!priv->percpu_stats) {
+		dev_err(dev, "alloc_percpu(percpu_stats) failed\n");
+		err = -ENOMEM;
+		goto err_alloc_percpu_stats;
+	}
+	priv->percpu_extras = alloc_percpu(*priv->percpu_extras);
+	if (!priv->percpu_extras) {
+		dev_err(dev, "alloc_percpu(percpu_extras) failed\n");
+		err = -ENOMEM;
+		goto err_alloc_percpu_extras;
+	}
+
+	err = netdev_init(net_dev);
+	if (err)
+		goto err_netdev_init;

At the end of netdev_init() you call netdev_register(). From that
point on, you device is live. Its .ndo's can be called....

quoted

+
+	/* Configure checksum offload based on current interface flags */
+	err = set_rx_csum(priv, !!(net_dev->features & NETIF_F_RXCSUM));
+	if (err)
+		goto err_csum;
+
+	err = set_tx_csum(priv, !!(net_dev->features &
+				   (NETIF_F_IP_CSUM |

NETIF_F_IPV6_CSUM)));

quoted

+	if (err)
+		goto err_csum;
+
+	err = alloc_rings(priv);
+	if (err)
+		goto err_alloc_rings;

How well does the device work if it has not allocated the rings yet,
when it is asked to do something?

quoted

+
+	net_dev->ethtool_ops = &dpaa2_ethtool_ops;
+
+	err = setup_irqs(dpni_dev);

How well do it work without interrupts being set up?

quoted

+	if (err) {
+		netdev_warn(net_dev, "Failed to set link interrupt, fall back

to polling\n");

quoted

+		priv->poll_thread = kthread_run(poll_link_state, priv,
+						"%s_poll_link", net_dev-
name);
+		if (IS_ERR(priv->poll_thread)) {
+			netdev_err(net_dev, "Error starting polling

thread\n");

quoted

+			goto err_poll_thread;
+		}
+		priv->do_link_poll = true;
+	}

Probably the correct place to register the netdev is here.

Good point, I'll fix it in v2.

Thanks a lot,
Ioana