diff mbox series

[13/21] net/ixgbe/base: modify Klocwork hits for DDK 7.0

Message ID 20200612032410.20864-14-guinanx.sun@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers show
Series update ixgbe base code | expand

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch warning coding style issues

Commit Message

Guinan Sun June 12, 2020, 3:24 a.m. UTC
Fix issues found by Klocwork for DDK 7.0

Signed-off-by: Jakub Chylkowski <jakubx.chylkowski@intel.com>
Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_82599.c     | 12 +++++-------
 drivers/net/ixgbe/base/ixgbe_common.c    |  6 ++----
 drivers/net/ixgbe/base/ixgbe_common.h    |  2 +-
 drivers/net/ixgbe/base/ixgbe_dcb_82598.c |  2 +-
 drivers/net/ixgbe/base/ixgbe_dcb_82599.c |  2 +-
 drivers/net/ixgbe/base/ixgbe_phy.c       | 25 +++++++-----------------
 drivers/net/ixgbe/base/ixgbe_x540.c      |  2 +-
 drivers/net/ixgbe/base/ixgbe_x550.c      |  2 +-
 8 files changed, 19 insertions(+), 34 deletions(-)

Comments

Ferruh Yigit June 22, 2020, noon UTC | #1
On 6/12/2020 4:24 AM, Guinan Sun wrote:
> Fix issues found by Klocwork for DDK 7.0

This doesn't say much on its own, can you please document the actual issues
fixed, like "fix unchecked return value" and I think can drop the Klocwork
reference, and even not asking what DDK is ...

Feel free to split patch into multiple patches if there are multiple types of
issues have been fixed.

> 
> Signed-off-by: Jakub Chylkowski <jakubx.chylkowski@intel.com>
> Signed-off-by: Guinan Sun <guinanx.sun@intel.com>

<...>
Guinan Sun July 1, 2020, 4:52 a.m. UTC | #2
Hi Ferruh

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 8:00 PM
> To: Sun, GuinanX <guinanx.sun@intel.com>; dev@dpdk.org
> Cc: Chylkowski, JakubX <jakubx.chylkowski@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 13/21] net/ixgbe/base: modify Klocwork hits
> for DDK 7.0
> 
> On 6/12/2020 4:24 AM, Guinan Sun wrote:
> > Fix issues found by Klocwork for DDK 7.0
> 
> This doesn't say much on its own, can you please document the actual issues
> fixed, like "fix unchecked return value" and I think can drop the Klocwork
> reference, and even not asking what DDK is ...
> 

We classified the inspection results and divided them into the following two types.
The first one is the return value check.
The second is type conversion.

In addition, regarding this part of the content, we think it is to improve the code style.
If it is a fix, the modified content is more scattered, and it is not easy to collect the fix version.
So, the title is tentatively 'modify coding style'.

> Feel free to split patch into multiple patches if there are multiple types of issues
> have been fixed.

First of all, I agree with you, but for the improvement of coding style, it is not necessary to split into multiple patches, 
because this has nothing to do with the function, it is just an improvement point.
If the function is different, it will be split into different patches.

> 
> >
> > Signed-off-by: Jakub Chylkowski <jakubx.chylkowski@intel.com>
> > Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> 
> <...>
diff mbox series

Patch

diff --git a/drivers/net/ixgbe/base/ixgbe_82599.c b/drivers/net/ixgbe/base/ixgbe_82599.c
index 193233746..e425f28af 100644
--- a/drivers/net/ixgbe/base/ixgbe_82599.c
+++ b/drivers/net/ixgbe/base/ixgbe_82599.c
@@ -1547,7 +1547,7 @@  void ixgbe_fdir_add_signature_filter_82599(struct ixgbe_hw *hw,
 	 * is for FDIRCMD.  Then do a 64-bit register write from FDIRHASH.
 	 */
 	fdirhashcmd = (u64)fdircmd << 32;
-	fdirhashcmd |= ixgbe_atr_compute_sig_hash_82599(input, common);
+	fdirhashcmd |= (u64)ixgbe_atr_compute_sig_hash_82599(input, common);
 	IXGBE_WRITE_REG64(hw, IXGBE_FDIRHASH, fdirhashcmd);
 
 	DEBUGOUT2("Tx Queue=%x hash=%x\n", queue, (u32)fdirhashcmd);
@@ -1636,7 +1636,7 @@  STATIC u32 ixgbe_get_fdirtcpm_82599(union ixgbe_atr_input *input_mask)
 {
 	u32 mask = IXGBE_NTOHS(input_mask->formatted.dst_port);
 	mask <<= IXGBE_FDIRTCPM_DPORTM_SHIFT;
-	mask |= IXGBE_NTOHS(input_mask->formatted.src_port);
+	mask |= (u32)IXGBE_NTOHS(input_mask->formatted.src_port);
 	mask = ((mask & 0x55555555) << 1) | ((mask & 0xAAAAAAAA) >> 1);
 	mask = ((mask & 0x33333333) << 2) | ((mask & 0xCCCCCCCC) >> 2);
 	mask = ((mask & 0x0F0F0F0F) << 4) | ((mask & 0xF0F0F0F0) >> 4);
@@ -1868,14 +1868,14 @@  s32 ixgbe_fdir_write_perfect_filter_82599(struct ixgbe_hw *hw,
 		/* record source and destination port (little-endian)*/
 		fdirport = IXGBE_NTOHS(input->formatted.dst_port);
 		fdirport <<= IXGBE_FDIRPORT_DESTINATION_SHIFT;
-		fdirport |= IXGBE_NTOHS(input->formatted.src_port);
+		fdirport |= (u32)IXGBE_NTOHS(input->formatted.src_port);
 		IXGBE_WRITE_REG(hw, IXGBE_FDIRPORT, fdirport);
 	}
 
 	/* record VLAN (little-endian) and flex_bytes(big-endian) */
 	fdirvlan = IXGBE_STORE_AS_BE16(input->formatted.flex_bytes);
 	fdirvlan <<= IXGBE_FDIRVLAN_FLEX_SHIFT;
-	fdirvlan |= IXGBE_NTOHS(input->formatted.vlan_id);
+	fdirvlan |= (u32)IXGBE_NTOHS(input->formatted.vlan_id);
 	IXGBE_WRITE_REG(hw, IXGBE_FDIRVLAN, fdirvlan);
 
 	if (cloud_mode) {
@@ -2093,9 +2093,7 @@  s32 ixgbe_start_hw_82599(struct ixgbe_hw *hw)
 	if (ret_val != IXGBE_SUCCESS)
 		goto out;
 
-	ret_val = ixgbe_start_hw_gen2(hw);
-	if (ret_val != IXGBE_SUCCESS)
-		goto out;
+	ixgbe_start_hw_gen2(hw);
 
 	/* We need to run link autotry after the driver loads */
 	hw->mac.autotry_restart = true;
diff --git a/drivers/net/ixgbe/base/ixgbe_common.c b/drivers/net/ixgbe/base/ixgbe_common.c
index c24127b03..c4e99d876 100644
--- a/drivers/net/ixgbe/base/ixgbe_common.c
+++ b/drivers/net/ixgbe/base/ixgbe_common.c
@@ -422,7 +422,7 @@  s32 ixgbe_start_hw_generic(struct ixgbe_hw *hw)
  *    82599
  *    X540
  **/
-s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
+void ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
 {
 	u32 i;
 	u32 regval;
@@ -447,8 +447,6 @@  s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
 			    IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
 		IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
 	}
-
-	return IXGBE_SUCCESS;
 }
 
 /**
@@ -739,7 +737,7 @@  s32 ixgbe_read_pba_num_generic(struct ixgbe_hw *hw, u32 *pba_num)
 		DEBUGOUT("NVM Read Error\n");
 		return ret_val;
 	}
-	*pba_num |= data;
+	*pba_num |= (u32)data;
 
 	return IXGBE_SUCCESS;
 }
diff --git a/drivers/net/ixgbe/base/ixgbe_common.h b/drivers/net/ixgbe/base/ixgbe_common.h
index 1d4839a62..e753d613c 100644
--- a/drivers/net/ixgbe/base/ixgbe_common.h
+++ b/drivers/net/ixgbe/base/ixgbe_common.h
@@ -23,7 +23,7 @@  u16 ixgbe_get_pcie_msix_count_generic(struct ixgbe_hw *hw);
 s32 ixgbe_init_ops_generic(struct ixgbe_hw *hw);
 s32 ixgbe_init_hw_generic(struct ixgbe_hw *hw);
 s32 ixgbe_start_hw_generic(struct ixgbe_hw *hw);
-s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw);
+void ixgbe_start_hw_gen2(struct ixgbe_hw *hw);
 s32 ixgbe_clear_hw_cntrs_generic(struct ixgbe_hw *hw);
 s32 ixgbe_read_pba_num_generic(struct ixgbe_hw *hw, u32 *pba_num);
 s32 ixgbe_read_pba_string_generic(struct ixgbe_hw *hw, u8 *pba_num,
diff --git a/drivers/net/ixgbe/base/ixgbe_dcb_82598.c b/drivers/net/ixgbe/base/ixgbe_dcb_82598.c
index bb309e28f..3a049b749 100644
--- a/drivers/net/ixgbe/base/ixgbe_dcb_82598.c
+++ b/drivers/net/ixgbe/base/ixgbe_dcb_82598.c
@@ -167,7 +167,7 @@  s32 ixgbe_dcb_config_tx_desc_arbiter_82598(struct ixgbe_hw *hw,
 	for (i = 0; i < IXGBE_DCB_MAX_TRAFFIC_CLASS; i++) {
 		max_credits = max[i];
 		reg = max_credits << IXGBE_TDTQ2TCCR_MCL_SHIFT;
-		reg |= refill[i];
+		reg |= (u32)(refill[i]);
 		reg |= (u32)(bwg_id[i]) << IXGBE_TDTQ2TCCR_BWG_SHIFT;
 
 		if (tsa[i] == ixgbe_dcb_tsa_group_strict_cee)
diff --git a/drivers/net/ixgbe/base/ixgbe_dcb_82599.c b/drivers/net/ixgbe/base/ixgbe_dcb_82599.c
index 04e0d1fb7..c26aaab91 100644
--- a/drivers/net/ixgbe/base/ixgbe_dcb_82599.c
+++ b/drivers/net/ixgbe/base/ixgbe_dcb_82599.c
@@ -166,7 +166,7 @@  s32 ixgbe_dcb_config_tx_desc_arbiter_82599(struct ixgbe_hw *hw, u16 *refill,
 	for (i = 0; i < IXGBE_DCB_MAX_TRAFFIC_CLASS; i++) {
 		max_credits = max[i];
 		reg = max_credits << IXGBE_RTTDT2C_MCL_SHIFT;
-		reg |= refill[i];
+		reg |= (u32)(refill[i]);
 		reg |= (u32)(bwg_id[i]) << IXGBE_RTTDT2C_BWG_SHIFT;
 
 		if (tsa[i] == ixgbe_dcb_tsa_group_strict_cee)
diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index 620154a41..f859b152e 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -8,10 +8,10 @@ 
 
 STATIC void ixgbe_i2c_start(struct ixgbe_hw *hw);
 STATIC void ixgbe_i2c_stop(struct ixgbe_hw *hw);
-STATIC s32 ixgbe_clock_in_i2c_byte(struct ixgbe_hw *hw, u8 *data);
+STATIC void ixgbe_clock_in_i2c_byte(struct ixgbe_hw *hw, u8 *data);
 STATIC s32 ixgbe_clock_out_i2c_byte(struct ixgbe_hw *hw, u8 data);
 STATIC s32 ixgbe_get_i2c_ack(struct ixgbe_hw *hw);
-STATIC s32 ixgbe_clock_in_i2c_bit(struct ixgbe_hw *hw, bool *data);
+STATIC void ixgbe_clock_in_i2c_bit(struct ixgbe_hw *hw, bool *data);
 STATIC s32 ixgbe_clock_out_i2c_bit(struct ixgbe_hw *hw, bool data);
 STATIC void ixgbe_raise_i2c_clk(struct ixgbe_hw *hw, u32 *i2cctl);
 STATIC void ixgbe_lower_i2c_clk(struct ixgbe_hw *hw, u32 *i2cctl);
@@ -46,11 +46,7 @@  STATIC s32 ixgbe_out_i2c_byte_ack(struct ixgbe_hw *hw, u8 byte)
  */
 STATIC s32 ixgbe_in_i2c_byte_ack(struct ixgbe_hw *hw, u8 *byte)
 {
-	s32 status;
-
-	status = ixgbe_clock_in_i2c_byte(hw, byte);
-	if (status)
-		return status;
+	ixgbe_clock_in_i2c_byte(hw, byte);
 	/* ACK */
 	return ixgbe_clock_out_i2c_bit(hw, false);
 }
@@ -123,8 +119,7 @@  s32 ixgbe_read_i2c_combined_generic_int(struct ixgbe_hw *hw, u8 addr, u16 reg,
 		if (ixgbe_in_i2c_byte_ack(hw, &low_bits))
 			goto fail;
 		/* Get csum */
-		if (ixgbe_clock_in_i2c_byte(hw, &csum_byte))
-			goto fail;
+		ixgbe_clock_in_i2c_byte(hw, &csum_byte);
 		/* NACK */
 		if (ixgbe_clock_out_i2c_bit(hw, false))
 			goto fail;
@@ -2034,9 +2029,7 @@  STATIC s32 ixgbe_read_i2c_byte_generic_int(struct ixgbe_hw *hw, u8 byte_offset,
 		if (status != IXGBE_SUCCESS)
 			goto fail;
 
-		status = ixgbe_clock_in_i2c_byte(hw, data);
-		if (status != IXGBE_SUCCESS)
-			goto fail;
+		ixgbe_clock_in_i2c_byte(hw, data);
 
 		status = ixgbe_clock_out_i2c_bit(hw, nack);
 		if (status != IXGBE_SUCCESS)
@@ -2281,7 +2274,7 @@  STATIC void ixgbe_i2c_stop(struct ixgbe_hw *hw)
  *
  * Clocks in one byte data via I2C data/clock
  **/
-STATIC s32 ixgbe_clock_in_i2c_byte(struct ixgbe_hw *hw, u8 *data)
+STATIC void ixgbe_clock_in_i2c_byte(struct ixgbe_hw *hw, u8 *data)
 {
 	s32 i;
 	bool bit = 0;
@@ -2293,8 +2286,6 @@  STATIC s32 ixgbe_clock_in_i2c_byte(struct ixgbe_hw *hw, u8 *data)
 		ixgbe_clock_in_i2c_bit(hw, &bit);
 		*data |= bit << i;
 	}
-
-	return IXGBE_SUCCESS;
 }
 
 /**
@@ -2390,7 +2381,7 @@  STATIC s32 ixgbe_get_i2c_ack(struct ixgbe_hw *hw)
  *
  * Clocks in one bit via I2C data/clock
  **/
-STATIC s32 ixgbe_clock_in_i2c_bit(struct ixgbe_hw *hw, bool *data)
+STATIC void ixgbe_clock_in_i2c_bit(struct ixgbe_hw *hw, bool *data)
 {
 	u32 i2cctl = IXGBE_READ_REG(hw, IXGBE_I2CCTL_BY_MAC(hw));
 	u32 data_oe_bit = IXGBE_I2C_DATA_OE_N_EN_BY_MAC(hw);
@@ -2415,8 +2406,6 @@  STATIC s32 ixgbe_clock_in_i2c_bit(struct ixgbe_hw *hw, bool *data)
 
 	/* Minimum low period of clock is 4.7 us */
 	usec_delay(IXGBE_I2C_T_LOW);
-
-	return IXGBE_SUCCESS;
 }
 
 /**
diff --git a/drivers/net/ixgbe/base/ixgbe_x540.c b/drivers/net/ixgbe/base/ixgbe_x540.c
index 838bfc8b5..4cea16a81 100644
--- a/drivers/net/ixgbe/base/ixgbe_x540.c
+++ b/drivers/net/ixgbe/base/ixgbe_x540.c
@@ -288,7 +288,7 @@  s32 ixgbe_start_hw_X540(struct ixgbe_hw *hw)
 	if (ret_val != IXGBE_SUCCESS)
 		goto out;
 
-	ret_val = ixgbe_start_hw_gen2(hw);
+	ixgbe_start_hw_gen2(hw);
 
 out:
 	return ret_val;
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index 5750f79eb..d3363ff1d 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -699,7 +699,7 @@  static s32 ixgbe_setup_fw_link(struct ixgbe_hw *hw)
 
 	for (i = 0; i < sizeof(ixgbe_fw_map) / sizeof(ixgbe_fw_map[0]); ++i) {
 		if (hw->phy.autoneg_advertised & ixgbe_fw_map[i].phy_speed)
-			setup[0] |= ixgbe_fw_map[i].fw_speed;
+			setup[0] |= (u32)(ixgbe_fw_map[i].fw_speed);
 	}
 	setup[0] |= FW_PHY_ACT_SETUP_LINK_HP | FW_PHY_ACT_SETUP_LINK_AN;