Thread (27 messages) flat view 27 messages, 3 authors, 2017-07-13

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help