[v2,12/20] net/ixgbe/base: modify coding style
diff mbox series

Message ID 20200702031329.4495-13-guinanx.sun@intel.com
State Superseded, archived
Delegated to: Qi Zhang
Headers show
Series
  • update ixgbe base code
Related show

Checks

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

Commit Message

Guinan Sun July 2, 2020, 3:13 a.m. UTC
Fix unchecked return value.
Add cast for type mismatch.

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 July 8, 2020, 3:26 p.m. UTC | #1
On 7/2/2020 4:13 AM, Guinan Sun wrote:
> Fix unchecked return value.
> Add cast for type mismatch.
> 
> 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(-)
> 
> 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);

Hi Guinan, Qi,

These are not coding style changes, as commit log says they are fixes in the code.

Can you please properly separate the patch based on the fix type and provide
relevant patch title?
Guinan Sun July 9, 2020, 7:58 a.m. UTC | #2
Hi Ferruh

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, July 8, 2020 11:26 PM
> To: Sun, GuinanX <guinanx.sun@intel.com>; dev@dpdk.org; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: Guo, Jia <jia.guo@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>;
> Chylkowski, JakubX <jakubx.chylkowski@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 12/20] net/ixgbe/base: modify coding style
> 
> On 7/2/2020 4:13 AM, Guinan Sun wrote:
> > Fix unchecked return value.
> > Add cast for type mismatch.
> >
> > 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(-)
> >
> > 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);
> 
> Hi Guinan, Qi,
> 
> These are not coding style changes, as commit log says they are fixes in the
> code.
> 
> Can you please properly separate the patch based on the fix type and provide
> relevant patch title?

First, we will make changes to the commit log.
The reason is that this is not a fix, but some unnecessary return value check is deleted.
Secondly, we split this patch into two. This will make the expression clearer.

Patch
diff mbox series

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 dc7b905c5..b17c02c35 100644
--- a/drivers/net/ixgbe/base/ixgbe_dcb_82598.c
+++ b/drivers/net/ixgbe/base/ixgbe_dcb_82598.c
@@ -159,7 +159,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;