Messages in this thread | | | Date | Wed, 12 Jul 2017 17:29:33 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH net 01/20] net/hinic: Initialize hw interface |
| |
> + > +#define HINIC_DRV_NAME "HiNIC" > +#define HINIC_DRV_VERSION "1.0"
Hi Aviad
Please don't add a driver version. There was a discussion about this recently, how pointless it is.
> +/** > + * hinic_init_hwdev - Initialize the NIC HW > + * @hwdev: the NIC HW device that is returned from the initialization > + * @pdev: the NIC pci device > + * > + * Return 0 - Success, negative - Failure > + * > + * Initialize the NIC HW device and return a pointer to it in the first arg > + **/ > +int hinic_init_hwdev(struct hinic_hwdev **hwdev, struct pci_dev *pdev) > +{ > + struct hinic_pfhwdev *pfhwdev; > + struct hinic_hwif *hwif; > + int err; > + > + hwif = kzalloc(sizeof(*hwif), GFP_KERNEL);
Using the devm_ functions makes your cleanup code simpler when handling memory.
> +/** > + * nic_dev_init - Initialize the NIC device > + * @pdev: the NIC pci device > + * > + * Return 0 - Success, negative - Failure > + **/ > +static int nic_dev_init(struct pci_dev *pdev) > +{ > + struct hinic_dev *nic_dev; > + struct net_device *netdev; > + struct hinic_hwdev *hwdev; > + int err, num_qps; > + > + err = hinic_init_hwdev(&hwdev, pdev); > + if (err) { > + dev_err(&pdev->dev, "Failed to initialize HW device\n"); > + return err; > + } > + > + num_qps = hinic_hwdev_num_qps(hwdev); > + if (num_qps <= 0) { > + dev_err(&pdev->dev, "Invalid number of QPS\n"); > + err = -EINVAL; > + goto num_qps_err; > + } > + > + netdev = alloc_etherdev_mq(sizeof(*nic_dev), num_qps); > + if (!netdev) { > + pr_err("Failed to allocate Ethernet device\n");
Above you used dev_err, here you used pr_err(). Please be consistent.
> + err = -ENOMEM; > + goto alloc_etherdev_err; > + } > + > + netdev->netdev_ops = &hinic_netdev_ops; > + > + nic_dev = (struct hinic_dev *)netdev_priv(netdev); > + nic_dev->hwdev = hwdev; > + nic_dev->netdev = netdev; > + nic_dev->msg_enable = MSG_ENABLE_DEFAULT; > + > + pci_set_drvdata(pdev, netdev); > + > + netif_carrier_off(netdev); > + > + err = register_netdev(netdev); > + if (err) { > + netif_err(nic_dev, probe, netdev, "Failed to register netdev\n");
probably not a good idea to use netif_err, if register_netdev just failed. dev_err() would be better.
| |