Re: [dpdk-dev] [PATCH 8/8] ionic: nits - whitespace, logging, helper variables

[Ferruh Yigit]

On 11/2/2020 6:35 PM, Andrew Boyer wrote:
These are minor cleanups which did not deserve their own patches.
Signed-off-by: Andrew Boyer <abo...@pensando.io>
---
  drivers/net/ionic/ionic_ethdev.c | 10 ++++------
  drivers/net/ionic/ionic_lif.c    | 22 ++++++++++-----------
  drivers/net/ionic/ionic_main.c   |  4 +---
  drivers/net/ionic/ionic_rxtx.c   | 33 ++++++++++++++------------------
  4 files changed, 29 insertions(+), 40 deletions(-)

diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index ff5c2e51a..04d4c989c 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -571,7 +571,7 @@ ionic_dev_rss_reta_update(struct rte_eth_dev *eth_dev,
  
  	if (reta_size != ident->lif.eth.rss_ind_tbl_sz) {
                IONIC_PRINT(ERR, "The size of hash lookup table configured "
-                       "(%d) doesn't match the number hardware can supported "
+                       "(%d) doesn't match the number hardware can support "
                        "(%d)",
                        reta_size, ident->lif.eth.rss_ind_tbl_sz);
                return -EINVAL;
@@ -605,7 +605,7 @@ ionic_dev_rss_reta_query(struct rte_eth_dev *eth_dev,
  
  	if (reta_size != ident->lif.eth.rss_ind_tbl_sz) {
                IONIC_PRINT(ERR, "The size of hash lookup table configured "
-                       "(%d) doesn't match the number hardware can supported "
+                       "(%d) doesn't match the number hardware can support "
                        "(%d)",
                        reta_size, ident->lif.eth.rss_ind_tbl_sz);
                return -EINVAL;
@@ -901,7 +901,7 @@ ionic_dev_start(struct rte_eth_dev *eth_dev)
        struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
        struct ionic_adapter *adapter = lif->adapter;
        struct ionic_dev *idev = &adapter->idev;
-       uint32_t allowed_speeds;
+       uint32_t speed, allowed_speeds;
        int err;
  
  	IONIC_PRINT_CALL();
@@ -929,8 +929,7 @@ ionic_dev_start(struct rte_eth_dev *eth_dev)
        }
  
  	if (eth_dev->data->dev_conf.link_speeds & ETH_LINK_SPEED_FIXED) {
-               uint32_t speed = ionic_parse_link_speeds(dev_conf->link_speeds);
-
+               speed = ionic_parse_link_speeds(dev_conf->link_speeds);
                if (speed)
                        ionic_dev_cmd_port_speed(idev, speed);
        }

I guess this is personal choice, but I suggest reducing the whitespace change as 
much as possible, if there is nothing to fix or refactor.

@@ -1264,7 +1263,6 @@ eth_ionic_pci_probe(struct rte_pci_driver *pci_drv 
__rte_unused,
        }
  
  	err = ionic_configure_intr(adapter);
-
        if (err) {
                IONIC_PRINT(ERR, "Failed to configure interrupts");
                goto err_free_adapter;
diff --git a/drivers/net/ionic/ionic_lif.c b/drivers/net/ionic/ionic_lif.c
index 2e33fb8d9..018103c5b 100644
--- a/drivers/net/ionic/ionic_lif.c
+++ b/drivers/net/ionic/ionic_lif.c
@@ -85,7 +85,8 @@ ionic_lif_reset(struct ionic_lif *lif)
  }
  
  static void
-ionic_lif_get_abs_stats(const struct ionic_lif *lif, struct rte_eth_stats 
*stats)
+ionic_lif_get_abs_stats(const struct ionic_lif *lif,
+               struct rte_eth_stats *stats)
  {
        struct ionic_lif_stats *ls = &lif->info->stats;
        uint32_t i;
@@ -305,10 +306,11 @@ ionic_dev_add_mac(struct rte_eth_dev *eth_dev,
  }
  
  void
-ionic_dev_remove_mac(struct rte_eth_dev *eth_dev, uint32_t index __rte_unused)
+ionic_dev_remove_mac(struct rte_eth_dev *eth_dev, uint32_t index)
  {
        struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
        struct ionic_adapter *adapter = lif->adapter;
+       struct rte_ether_addr *mac_addr;
  
  	IONIC_PRINT_CALL();
  
@@ -319,11 +321,12 @@ ionic_dev_remove_mac(struct rte_eth_dev *eth_dev, uint32_t index __rte_unused)
                return;
        }
  
-	if (!rte_is_valid_assigned_ether_addr(&eth_dev->data->mac_addrs[index]))
+       mac_addr = &eth_dev->data->mac_addrs[index];
+
+       if (!rte_is_valid_assigned_ether_addr(mac_addr))
                return;
  
-	ionic_lif_addr_del(lif, (const uint8_t *)
-               &eth_dev->data->mac_addrs[index]);
+       ionic_lif_addr_del(lif, (const uint8_t *)mac_addr);
  }
  
  int
@@ -658,7 +661,6 @@ ionic_qcq_alloc(struct ionic_lif *lif, uint8_t type,
        new->base_z = rte_eth_dma_zone_reserve(lif->eth_dev,
                base /* name */, index /* queue_idx */,
                total_size, IONIC_ALIGN, socket_id);
-
        if (!new->base_z) {
                IONIC_PRINT(ERR, "Cannot reserve queue DMA memory");
                err = -ENOMEM;
@@ -682,8 +684,8 @@ ionic_qcq_alloc(struct ionic_lif *lif, uint8_t type,
                ionic_q_sg_map(&new->q, sg_base, sg_base_pa);
        }
  
-	IONIC_PRINT(DEBUG, "Q-Base-PA = %ju CQ-Base-PA = %ju "
-               "SG-base-PA = %ju",
+       IONIC_PRINT(DEBUG, "Q-Base-PA = %#lx CQ-Base-PA = %#lx "
+               "SG-base-PA = %#lx",
                q_base_pa, cq_base_pa, sg_base_pa);

As far as I remember '%j' used intentionally, otherwise this will break the 
32bit build.

  
  	ionic_q_map(&new->q, q_base, q_base_pa);
@@ -839,7 +841,6 @@ ionic_lif_alloc(struct ionic_lif *lif)
  
  	lif->txqcqs = rte_zmalloc("ionic", sizeof(*lif->txqcqs) *
                adapter->max_ntxqs_per_lif, 0);
-
        if (!lif->txqcqs) {
                IONIC_PRINT(ERR, "Cannot allocate tx queues array");
                return -ENOMEM;

As far as I can see this patchset has,
1- minor fixes
2- some code refactoring
3- Whitespace updates

Would you mind whitespace changes for now, and separate patch for 1 & 2?