Re: [net-next v3 05/15] iecm: Add basic netdevice functionality
From: Joe Perches <joe@perches.com>
Date: 2020-06-26 02:39:10
On Thu, 2020-06-25 at 19:07 -0700, Jeff Kirsher wrote:
From: Alice Michael <redacted> This implements probe, interface up/down, and netdev_ops.
trivial notes:
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/intel/iecm/iecm_lib.c b/drivers/net/ethernet/intel/iecm/iecm_lib.c
[]
quoted hunk ↗ jump to hunk
@@ -194,7 +298,24 @@ static int iecm_vport_rel(struct iecm_vport *vport) */ static void iecm_vport_rel_all(struct iecm_adapter *adapter) { - /* stub */ + int err, i; + + if (!adapter->vports) + return; + + for (i = 0; i < adapter->num_alloc_vport; i++) { + if (!adapter->vports[i]) + continue; + + err = iecm_vport_rel(adapter->vports[i]); + if (err) + dev_dbg(&adapter->pdev->dev, + "Failed to release adapter->vport[%d], err %d,\n",
odd comma
+ i, err); + else + adapter->vports[i] = NULL; + } + adapter->num_alloc_vport = 0;
If one of these fails to release, why always set num_alloc_vport to 0?
quoted hunk ↗ jump to hunk
@@ -273,7 +483,40 @@ static void iecm_init_task(struct work_struct *work) */ static int iecm_api_init(struct iecm_adapter *adapter) { - /* stub */ + struct iecm_reg_ops *reg_ops = &adapter->dev_ops.reg_ops; + struct pci_dev *pdev = adapter->pdev; + + if (!adapter->dev_ops.reg_ops_init) { + dev_err(&pdev->dev, "Invalid device, register API init not defined.\n");
inconsistent uses of periods after logging messages.
+ return -EINVAL;
+ }
+ adapter->dev_ops.reg_ops_init(adapter);
+ if (!(reg_ops->ctlq_reg_init && reg_ops->vportq_reg_init &&
+ reg_ops->intr_reg_init && reg_ops->mb_intr_reg_init &&
+ reg_ops->reset_reg_init && reg_ops->trigger_reset)) {
+ dev_err(&pdev->dev, "Invalid device, missing one or more register functions\n");Most are without period.
+ return -EINVAL;
+ }
+
+ if (adapter->dev_ops.vc_ops_init) {
+ struct iecm_virtchnl_ops *vc_ops;
+
+ adapter->dev_ops.vc_ops_init(adapter);
+ vc_ops = &adapter->dev_ops.vc_ops;
+ if (!(vc_ops->core_init && vc_ops->vport_init &&
+ vc_ops->vport_queue_ids_init && vc_ops->get_caps &&
+ vc_ops->config_queues && vc_ops->enable_queues &&
+ vc_ops->disable_queues && vc_ops->irq_map_unmap &&
+ vc_ops->get_set_rss_lut && vc_ops->get_set_rss_hash &&
+ vc_ops->adjust_qs && vc_ops->get_ptype)) {style trivia: Sometimes it's clearer for human readers if all the tests are separated on individual lines.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/intel/iecm/iecm_virtchnl.c b/drivers/net/ethernet/intel/iecm/iecm_virtchnl.c
[]
quoted hunk ↗ jump to hunk
@@ -594,6 +642,25 @@ static bool iecm_is_capability_ena(struct iecm_adapter *adapter, u64 flag) */ void iecm_vc_ops_init(struct iecm_adapter *adapter) { - /* stub */
Maybe add a temporary for adapter->dev_ops.vc_ops to reduce visual clutter?
+ adapter->dev_ops.vc_ops.core_init = iecm_vc_core_init; + adapter->dev_ops.vc_ops.vport_init = iecm_vport_init; + adapter->dev_ops.vc_ops.vport_queue_ids_init = + iecm_vport_queue_ids_init; + adapter->dev_ops.vc_ops.get_caps = iecm_send_get_caps_msg; + adapter->dev_ops.vc_ops.is_cap_ena = iecm_is_capability_ena;