From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 31392A0521; Tue, 3 Nov 2020 14:06:28 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0BE62CA36; Tue, 3 Nov 2020 14:06:27 +0100 (CET) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 537A5CA2A for ; Tue, 3 Nov 2020 14:06:25 +0100 (CET) IronPort-SDR: gWASSpfrRRhe5mA9/DT9i8oUwwxJYFUBDHOELJJMQq7sb7M+IpghW6p8pr/qc5qZFo/ImUEen4 UzjpAvvy0sOQ== X-IronPort-AV: E=McAfee;i="6000,8403,9793"; a="233215635" X-IronPort-AV: E=Sophos;i="5.77,448,1596524400"; d="scan'208";a="233215635" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Nov 2020 05:06:22 -0800 IronPort-SDR: Jhzdg9ilgJbZ/8GychtEbTJvoBGfzlLlvvXmDVzjt8sWTK/N6FkM0qmw7TV2bAzc5TUSvLuRpa NRVFqxNaZxBQ== X-IronPort-AV: E=Sophos;i="5.77,448,1596524400"; d="scan'208";a="538485006" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.218.178]) ([10.213.218.178]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Nov 2020 05:06:19 -0800 To: Andrew Boyer , dev@dpdk.org Cc: cardigliano@ntop.org References: <20201102183527.69209-1-aboyer@pensando.io> <20201102183527.69209-9-aboyer@pensando.io> From: Ferruh Yigit Message-ID: Date: Tue, 3 Nov 2020 13:06:15 +0000 MIME-Version: 1.0 In-Reply-To: <20201102183527.69209-9-aboyer@pensando.io> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 8/8] ionic: nits - whitespace, logging, helper variables X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > 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(ð_dev->data->mac_addrs[index])) > + mac_addr = ð_dev->data->mac_addrs[index]; > + > + if (!rte_is_valid_assigned_ether_addr(mac_addr)) > return; > > - ionic_lif_addr_del(lif, (const uint8_t *) > - ð_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?