> -----Original Message-----
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Friday, August 3, 2018 7:43 PM
> To: Ioana Ciocoi Radulescu <ruxandra.radule...@nxp.com>
> Cc: net...@vger.kernel.org; da...@davemloft.net;
> gre...@linuxfoundation.org; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org; Ioana Ciornei <ioana.cior...@nxp.com>; Laurentiu
> Tudor <laurentiu.tu...@nxp.com>; Madalin-cristian Bucur
> <madalin.bu...@nxp.com>; Horia Geanta <horia.gea...@nxp.com>
> Subject: Re: [PATCH net-next] [RFC] dpaa2-eth: Move DPAA2 Ethernet driver
> from staging to drivers/net
> 
> > +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);
> > +   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,
> > +                                &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....
> 
> > +
> > +   /* 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)));
> > +   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?
> 
> > +
> > +   net_dev->ethtool_ops = &dpaa2_ethtool_ops;
> > +
> > +   err = setup_irqs(dpni_dev);
> 
> How well do it work without interrupts being set up?
> 
> > +   if (err) {
> > +           netdev_warn(net_dev, "Failed to set link interrupt, fall back
> to polling\n");
> > +           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");
> > +                   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
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to