Re: [dpdk-dev] [PATCH 40/49] net/ice/base: code clean up

["Rong, Leyi" <leyi.rong@intel.com>]

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Wednesday, June 5, 2019 8:07 PM
> To: Rong, Leyi <leyi.rong@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 40/49] net/ice/base: code clean up
> 
> 
> 
> On 6/4/19 7:42 AM, Leyi Rong wrote:
> > Cleanup the useless code.
> >
> > Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> > Signed-off-by: Leyi Rong <leyi.rong@intel.com>
> > ---
> >   drivers/net/ice/base/ice_controlq.c  | 62 +---------------------------
> >   drivers/net/ice/base/ice_fdir.h      |  1 -
> >   drivers/net/ice/base/ice_flex_pipe.c |  5 ++-
> >   drivers/net/ice/base/ice_sched.c     |  4 +-
> >   drivers/net/ice/base/ice_type.h      |  3 ++
> >   5 files changed, 10 insertions(+), 65 deletions(-)
> >
> > diff --git a/drivers/net/ice/base/ice_controlq.c
> > b/drivers/net/ice/base/ice_controlq.c
> > index 4cb6df113..3ef07e094 100644
> > --- a/drivers/net/ice/base/ice_controlq.c
> > +++ b/drivers/net/ice/base/ice_controlq.c
> > @@ -262,7 +262,7 @@ ice_cfg_sq_regs(struct ice_hw *hw, struct ice_ctl_q_info *cq)
> >    * @hw: pointer to the hardware structure
> >    * @cq: pointer to the specific Control queue
> >    *
> > - * Configure base address and length registers for the receive (event
> > q)
> > + * Configure base address and length registers for the receive (event
> > + queue)
> >    */
> >   static enum ice_status
> >   ice_cfg_rq_regs(struct ice_hw *hw, struct ice_ctl_q_info *cq) @@
> > -772,9 +772,6 @@ static u16 ice_clean_sq(struct ice_hw *hw, struct ice_ctl_q_info *cq)
> >   	struct ice_ctl_q_ring *sq = &cq->sq;
> >   	u16 ntc = sq->next_to_clean;
> >   	struct ice_sq_cd *details;
> > -#if 0
> > -	struct ice_aq_desc desc_cb;
> > -#endif
> >   	struct ice_aq_desc *desc;
> >
> >   	desc = ICE_CTL_Q_DESC(*sq, ntc);
> > @@ -783,15 +780,6 @@ static u16 ice_clean_sq(struct ice_hw *hw, struct ice_ctl_q_info *cq)
> >   	while (rd32(hw, cq->sq.head) != ntc) {
> >   		ice_debug(hw, ICE_DBG_AQ_MSG,
> >   			  "ntc %d head %d.\n", ntc, rd32(hw, cq->sq.head)); -#if 0
> > -		if (details->callback) {
> > -			ICE_CTL_Q_CALLBACK cb_func =
> > -				(ICE_CTL_Q_CALLBACK)details->callback;
> > -			ice_memcpy(&desc_cb, desc, sizeof(desc_cb),
> > -				   ICE_DMA_TO_DMA);
> > -			cb_func(hw, &desc_cb);
> > -		}
> > -#endif
> >   		ice_memset(desc, 0, sizeof(*desc), ICE_DMA_MEM);
> >   		ice_memset(details, 0, sizeof(*details), ICE_NONDMA_MEM);
> >   		ntc++;
> > @@ -941,38 +929,8 @@ ice_sq_send_cmd(struct ice_hw *hw, struct ice_ctl_q_info *cq,
> >   	details = ICE_CTL_Q_DETAILS(cq->sq, cq->sq.next_to_use);
> >   	if (cd)
> >   		*details = *cd;
> > -#if 0
> > -		/* FIXME: if/when this block gets enabled (when the #if 0
> > -		 * is removed), add braces to both branches of the surrounding
> > -		 * conditional expression. The braces have been removed to
> > -		 * prevent checkpatch complaining.
> > -		 */
> > -
> > -		/* If the command details are defined copy the cookie. The
> > -		 * CPU_TO_LE32 is not needed here because the data is ignored
> > -		 * by the FW, only used by the driver
> > -		 */
> > -		if (details->cookie) {
> > -			desc->cookie_high =
> > -				CPU_TO_LE32(ICE_HI_DWORD(details->cookie));
> > -			desc->cookie_low =
> > -				CPU_TO_LE32(ICE_LO_DWORD(details->cookie));
> > -		}
> > -#endif
> >   	else
> >   		ice_memset(details, 0, sizeof(*details), ICE_NONDMA_MEM); -#if 0
> > -	/* clear requested flags and then set additional flags if defined */
> > -	desc->flags &= ~CPU_TO_LE16(details->flags_dis);
> > -	desc->flags |= CPU_TO_LE16(details->flags_ena);
> > -
> > -	if (details->postpone && !details->async) {
> > -		ice_debug(hw, ICE_DBG_AQ_MSG,
> > -			  "Async flag not set along with postpone flag\n");
> > -		status = ICE_ERR_PARAM;
> > -		goto sq_send_command_error;
> > -	}
> > -#endif
> >
> >   	/* Call clean and check queue available function to reclaim the
> >   	 * descriptors that were processed by FW/MBX; the function returns
> > the @@ -1019,20 +977,8 @@ ice_sq_send_cmd(struct ice_hw *hw, struct ice_ctl_q_info *cq,
> >   	(cq->sq.next_to_use)++;
> >   	if (cq->sq.next_to_use == cq->sq.count)
> >   		cq->sq.next_to_use = 0;
> > -#if 0
> > -	/* FIXME - handle this case? */
> > -	if (!details->postpone)
> > -#endif
> >   	wr32(hw, cq->sq.tail, cq->sq.next_to_use);
> >
> > -#if 0
> > -	/* if command details are not defined or async flag is not set,
> > -	 * we need to wait for desc write back
> > -	 */
> > -	if (!details->async && !details->postpone) {
> > -		/* FIXME - handle this case? */
> > -	}
> > -#endif
> >   	do {
> >   		if (ice_sq_done(hw, cq))
> >   			break;
> > @@ -1087,9 +1033,6 @@ ice_sq_send_cmd(struct ice_hw *hw, struct
> > ice_ctl_q_info *cq,
> >
> >   	/* update the error if time out occurred */
> >   	if (!cmd_completed) {
> > -#if 0
> > -	    (!details->async && !details->postpone)) {
> > -#endif
> >   		ice_debug(hw, ICE_DBG_AQ_MSG,
> >   			  "Control Send Queue Writeback timeout.\n");
> >   		status = ICE_ERR_AQ_TIMEOUT;
> > @@ -1208,9 +1151,6 @@ ice_clean_rq_elem(struct ice_hw *hw, struct ice_ctl_q_info *cq,
> >   	cq->rq.next_to_clean = ntc;
> >   	cq->rq.next_to_use = ntu;
> >
> > -#if 0
> > -	ice_nvmupd_check_wait_event(hw, LE16_TO_CPU(e->desc.opcode));
> > -#endif
> >   clean_rq_elem_out:
> >   	/* Set pending if needed, unlock and return */
> >   	if (pending) {
> 
> 
> Starting from here, the rest looks unrelated to the commit subject.
> 
> > diff --git a/drivers/net/ice/base/ice_fdir.h
> > b/drivers/net/ice/base/ice_fdir.h index 2ecb147f1..f8f06658c 100644
> > --- a/drivers/net/ice/base/ice_fdir.h
> > +++ b/drivers/net/ice/base/ice_fdir.h
> > @@ -173,7 +173,6 @@ struct ice_fdir_fltr {
> >   	u32 fltr_id;
> >   };
> >
> > -
> >   /* Dummy packet filter definition structure. */
> >   struct ice_fdir_base_pkt {
> >   	enum ice_fltr_ptype flow;
> > diff --git a/drivers/net/ice/base/ice_flex_pipe.c
> > b/drivers/net/ice/base/ice_flex_pipe.c
> > index 2a310b6e1..46234c014 100644
> > --- a/drivers/net/ice/base/ice_flex_pipe.c
> > +++ b/drivers/net/ice/base/ice_flex_pipe.c
> > @@ -398,7 +398,7 @@ ice_find_boost_entry(struct ice_seg *ice_seg, u16 addr,
> >    * Handles enumeration of individual label entries.
> >    */
> >   static void *
> > -ice_label_enum_handler(u32 __always_unused sect_type, void *section,
> > u32 index,
> > +ice_label_enum_handler(u32 __ALWAYS_UNUSED sect_type, void *section,
> > +u32 index,
> >   		       u32 *offset)
> >   {
> >   	struct ice_label_section *labels;
> > @@ -640,7 +640,7 @@ static bool ice_bits_max_set(const u8 *mask, u16 size, u16 max)
> >    * @size: the size of the complete key in bytes (must be even)
> >    * @val: array of 8-bit values that makes up the value portion of the key
> >    * @upd: array of 8-bit masks that determine what key portion to
> > update
> > - * @dc: array of 8-bit masks that make up the dont' care mask
> > + * @dc: array of 8-bit masks that make up the don't care mask
> >    * @nm: array of 8-bit masks that make up the never match mask
> >    * @off: the offset of the first byte in the key to update
> >    * @len: the number of bytes in the key update @@ -4544,6 +4544,7 @@
> > ice_move_vsi(struct ice_hw *hw, enum ice_block blk, u16 vsi, u16 vsig,
> >   	status = ice_vsig_find_vsi(hw, blk, vsi, &orig_vsig);
> >   	if (!status)
> >   		status = ice_vsig_add_mv_vsi(hw, blk, vsi, vsig);
> > +
> >   	if (status) {
> >   		ice_free(hw, p);
> >   		return status;
> > diff --git a/drivers/net/ice/base/ice_sched.c
> > b/drivers/net/ice/base/ice_sched.c
> > index a72e72982..fa3158a7b 100644
> > --- a/drivers/net/ice/base/ice_sched.c
> > +++ b/drivers/net/ice/base/ice_sched.c
> > @@ -1233,7 +1233,7 @@ enum ice_status ice_sched_init_port(struct ice_port_info *pi)
> >   		goto err_init_port;
> >   	}
> >
> > -	/* If the last node is a leaf node then the index of the Q group
> > +	/* If the last node is a leaf node then the index of the queue group
> >   	 * layer is two less than the number of elements.
> >   	 */
> >   	if (num_elems > 2 && buf[0].generic[num_elems - 1].data.elem_type
> > == @@ -3529,9 +3529,11 @@ ice_cfg_agg_vsi_priority_per_tc(struct ice_port_info *pi, u32 agg_id,
> >   		LIST_FOR_EACH_ENTRY(agg_vsi_info, &agg_info->agg_vsi_list,
> >   				    ice_sched_agg_vsi_info, list_entry)
> >   			if (agg_vsi_info->vsi_handle == vsi_handle) {
> > +				/* cppcheck-suppress unreadVariable */
> >   				vsi_handle_valid = true;
> >   				break;
> >   			}
> > +
> >   		if (!vsi_handle_valid)
> >   			goto exit_agg_priority_per_tc;
> >
> > diff --git a/drivers/net/ice/base/ice_type.h
> > b/drivers/net/ice/base/ice_type.h index f4e151c55..f76be2b58 100644
> > --- a/drivers/net/ice/base/ice_type.h
> > +++ b/drivers/net/ice/base/ice_type.h
> > @@ -114,6 +114,9 @@ static inline u32 ice_round_to_num(u32 N, u32 R)
> >   #define ICE_DBG_USER		BIT_ULL(31)
> >   #define ICE_DBG_ALL		0xFFFFFFFFFFFFFFFFULL
> >
> > +#ifndef __ALWAYS_UNUSED
> > +#define __ALWAYS_UNUSED
> > +#endif
> 
> That does not look related
> 

Hi Maxime,

For this commit, I did modifications that will not impact any functionalities to aligned to the ND
released shared code. Since the modified lines may scattered in many different commits in the
original shared code repo, so I just put them into one.

Thanks,
Leyi