diff mbox series

[27/37] net/ice/base: resolve static analysis reported issues

Message ID 20190228055650.25237-28-qi.z.zhang@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series share code update. |

Checks

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

Commit Message

Qi Zhang Feb. 28, 2019, 5:56 a.m. UTC 
  Resolve static analysis reported issue in
ice_get_itr_intrl_gran and ice_ptg_find_ptype.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/net/ice/base/ice_common.c    | 12 ++----------
 drivers/net/ice/base/ice_flex_pipe.c |  7 +------
 2 files changed, 3 insertions(+), 16 deletions(-)

Comments

Ferruh Yigit March 1, 2019, 10:36 a.m. UTC | #1 
On 2/28/2019 5:56 AM, Qi Zhang wrote:
> Resolve static analysis reported issue in
> ice_get_itr_intrl_gran and ice_ptg_find_ptype.

Same comment with previous patch, related to the commit log.

Commit log says "static analysis reported issues" are solved, what are they
really? If we know them we can know your intention and say something about the
code below, otherwise is there a way to figure out if something wrong below?

And "impact" part, "what is the impact of the change?" Will those issues create
definite memory corruption? Or protection for possible issue. Knowing this helps
maintainers and LTS maintainers to priorities the patch accordingly.

Also please use "fix" instead of "resolve", fix is kind of keyword we tend to
use, and provide a fixes line for whatever fixed.

> 
> Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
Qi Zhang March 4, 2019, 1:54 a.m. UTC | #2 
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, March 1, 2019 6:37 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Yang, Qiming <qiming.yang@intel.com>
> Cc: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; dev@dpdk.org; Allan,
> Bruce W <bruce.w.allan@intel.com>
> Subject: Re: [PATCH 27/37] net/ice/base: resolve static analysis reported issues
> 
> On 2/28/2019 5:56 AM, Qi Zhang wrote:
> > Resolve static analysis reported issue in ice_get_itr_intrl_gran and
> > ice_ptg_find_ptype.
> 
> Same comment with previous patch, related to the commit log.
> 
> Commit log says "static analysis reported issues" are solved, what are they really?
> If we know them we can know your intention and say something about the code
> below, otherwise is there a way to figure out if something wrong below?

OK, I think we can add more explanation here.
> 
> And "impact" part, "what is the impact of the change?" Will those issues create
> definite memory corruption? Or protection for possible issue. Knowing this helps
> maintainers and LTS maintainers to priorities the patch accordingly.
> 

The ice driver is claimed as an experimental release in 19.02, only basic function is enabled and the quality is not guaranteed, 
We assume user already know this and expected to meet some issue, 
Also 19.02 is not LTS release, so maintainer no need to worry about fix back port.
so from my view, the "impact" part is not necessary to be highlighted for this patch but a nice to have, am I right?

> Also please use "fix" instead of "resolve", fix is kind of keyword we tend to use,
> and provide a fixes line for whatever fixed.

OK, will add fix line for this.

Thanks
Qi

> 
> >
> > Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> > Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>
diff mbox series

Patch

diff --git a/drivers/net/ice/base/ice_common.c b/drivers/net/ice/base/ice_common.c
index ae0e7fc5f..2362dd774 100644
--- a/drivers/net/ice/base/ice_common.c
+++ b/drivers/net/ice/base/ice_common.c
@@ -741,7 +741,7 @@  void ice_output_fw_log(struct ice_hw *hw, struct ice_aq_desc *desc, void *buf)
  * Determines the itr/intrl granularities based on the maximum aggregate
  * bandwidth according to the device's configuration during power-on.
  */
-static enum ice_status ice_get_itr_intrl_gran(struct ice_hw *hw)
+static void ice_get_itr_intrl_gran(struct ice_hw *hw)
 {
 	u8 max_agg_bw = (rd32(hw, GL_PWR_MODE_CTL) &
 			 GL_PWR_MODE_CTL_CAR_MAX_BW_M) >>
@@ -758,13 +758,7 @@  static enum ice_status ice_get_itr_intrl_gran(struct ice_hw *hw)
 		hw->itr_gran = ICE_ITR_GRAN_MAX_25;
 		hw->intrl_gran = ICE_INTRL_GRAN_MAX_25;
 		break;
-	default:
-		ice_debug(hw, ICE_DBG_INIT,
-			  "Failed to determine itr/intrl granularity\n");
-		return ICE_ERR_CFG;
 	}
-
-	return ICE_SUCCESS;
 }
 
 /**
@@ -795,9 +789,7 @@  enum ice_status ice_init_hw(struct ice_hw *hw)
 	if (status)
 		return status;
 
-	status = ice_get_itr_intrl_gran(hw);
-	if (status)
-		return status;
+	ice_get_itr_intrl_gran(hw);
 
 
 	status = ice_init_all_ctrlq(hw);
diff --git a/drivers/net/ice/base/ice_flex_pipe.c b/drivers/net/ice/base/ice_flex_pipe.c
index 1dd121b28..525378079 100644
--- a/drivers/net/ice/base/ice_flex_pipe.c
+++ b/drivers/net/ice/base/ice_flex_pipe.c
@@ -4288,17 +4288,12 @@  ice_add_prof_to_lst(struct ice_hw *hw, enum ice_block blk,
 	p->tcam_count = map->ptype_count;
 
 	for (i = 0; i < map->ptype_count; i++) {
-		enum ice_status status;
 		u8 ptg;
 
 		p->tcam[i].prof_id = map->prof_id;
 		p->tcam[i].tcam_idx = ICE_INVALID_TCAM;
 
-		status = ice_ptg_find_ptype(hw, blk, map->ptype[i], &ptg);
-		if (status) {
-			ice_free(hw, p);
-			return status;
-		}
+		ice_ptg_find_ptype(hw, blk, map->ptype[i], &ptg);
 
 		p->tcam[i].ptg = ptg;
 	}