Re: [PATCH net 01/20] net/hinic: Initialize hw interface
From: Andrew Lunn <andrew@lunn.ch>
Date: 2017-07-12 15:29:41
Also in:
lkml
+ +#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.