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

[Maxime Coquelin <maxime.coquelin@redhat.com>]

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

>   
>   
>   
>