On Thu, 2020-06-25 at 19:07 -0700, Jeff Kirsher wrote:
> From: Alice Michael <alice.mich...@intel.com>
>
> This implements probe, interface up/down, and netdev_ops.
trivial notes:
> diff --git a/drivers/net/ethernet/intel/iecm/iecm_lib.c
> b/drivers/net/ethernet/intel/iecm/iecm_lib.c
[]
> @@ -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?
> @@ -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.
> diff --git a/drivers/net/ethernet/intel/iecm/iecm_virtchnl.c
> b/drivers/net/ethernet/intel/iecm/iecm_virtchnl.c
[]
> @@ -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;
