[PATCH for-next 0/5] RDMA/hns: Cleanup for removing redundant statements and judging return values

[PATCH for-next 0/5] RDMA/hns: Cleanup for removing redundant statements and judging return values

[Wenpeng Liang]

Most static warnings are detected by tools, mainly about:

(1) #1~3: Remove redundant statements.
(2) #4~5: About the return value of the function.

Chengchang Tang (1):
  RDMA/hns: Remove unnecessary check for the sgid_attr when modifying QP

Guofeng Yue (1):
  RDMA/hns: Remove redundant variable "ret"

Haoyue Xu (1):
  RDMA/hns: Init the variable at the suitable place

Wenpeng Liang (1):
  RDMA/hns: Add judgment on the execution result of CMDQ that free vf
    resource

Yixing Liu (1):
  RDMA/hns: Remove unused function to_hns_roce_state()

 drivers/infiniband/hw/hns/hns_roce_device.h | 11 --------
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 29 ++++++++++++---------
 drivers/infiniband/hw/hns/hns_roce_qp.c     | 20 --------------
 3 files changed, 17 insertions(+), 43 deletions(-)

--
2.33.0

[PATCH for-next 1/5] RDMA/hns: Remove unnecessary check for the sgid_attr when modifying QP

[Wenpeng Liang]

From: Chengchang Tang <tangchengchang@huawei.com>

The sgid_attr cannot be null in this scenario. This judgment is redundant.

Fixes: 606bf89e98ef ("RDMA/hns: Refactor for hns_roce_v2_modify_qp function")
Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Wenpeng Liang <liangwenpeng@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 2b0cef17ad45..cd87e6e86720 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -4966,9 +4966,7 @@ static int hns_roce_v2_set_path(struct ib_qp *ibqp,
 		if (ret)
 			return ret;
 
-		if (gid_attr)
-			is_udp = (gid_attr->gid_type ==
-				 IB_GID_TYPE_ROCE_UDP_ENCAP);
+		is_udp = (gid_attr->gid_type == IB_GID_TYPE_ROCE_UDP_ENCAP);
 	}
 
 	/* Only HIP08 needs to set the vlan_en bits in QPC */
-- 
2.33.0

[PATCH for-next 2/5] RDMA/hns: Remove unused function to_hns_roce_state()

[Wenpeng Liang]

From: Yixing Liu <liuyixing1@huawei.com>

This function is only used in HIP06, which has been removed. So remove it.

Signed-off-by: Yixing Liu <liuyixing1@huawei.com>
Signed-off-by: Wenpeng Liang <liangwenpeng@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_device.h | 11 -----------
 drivers/infiniband/hw/hns/hns_roce_qp.c     | 20 --------------------
 2 files changed, 31 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 3083d6db1d68..bc9f25e79c87 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -106,16 +106,6 @@ enum {
 	SERV_TYPE_XRC = 5,
 };
 
-enum hns_roce_qp_state {
-	HNS_ROCE_QP_STATE_RST,
-	HNS_ROCE_QP_STATE_INIT,
-	HNS_ROCE_QP_STATE_RTR,
-	HNS_ROCE_QP_STATE_RTS,
-	HNS_ROCE_QP_STATE_SQD,
-	HNS_ROCE_QP_STATE_ERR,
-	HNS_ROCE_QP_NUM_STATE,
-};
-
 enum hns_roce_event {
 	HNS_ROCE_EVENT_TYPE_PATH_MIG                  = 0x01,
 	HNS_ROCE_EVENT_TYPE_PATH_MIG_FAILED           = 0x02,
@@ -1191,7 +1181,6 @@ void *hns_roce_get_send_wqe(struct hns_roce_qp *hr_qp, unsigned int n);
 void *hns_roce_get_extend_sge(struct hns_roce_qp *hr_qp, unsigned int n);
 bool hns_roce_wq_overflow(struct hns_roce_wq *hr_wq, u32 nreq,
 			  struct ib_cq *ib_cq);
-enum hns_roce_qp_state to_hns_roce_state(enum ib_qp_state state);
 void hns_roce_lock_cqs(struct hns_roce_cq *send_cq,
 		       struct hns_roce_cq *recv_cq);
 void hns_roce_unlock_cqs(struct hns_roce_cq *send_cq,
diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c
index d78373e10aab..48d3616a6d71 100644
--- a/drivers/infiniband/hw/hns/hns_roce_qp.c
+++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
@@ -243,26 +243,6 @@ static int alloc_qpn(struct hns_roce_dev *hr_dev, struct hns_roce_qp *hr_qp)
 	return 0;
 }
 
-enum hns_roce_qp_state to_hns_roce_state(enum ib_qp_state state)
-{
-	switch (state) {
-	case IB_QPS_RESET:
-		return HNS_ROCE_QP_STATE_RST;
-	case IB_QPS_INIT:
-		return HNS_ROCE_QP_STATE_INIT;
-	case IB_QPS_RTR:
-		return HNS_ROCE_QP_STATE_RTR;
-	case IB_QPS_RTS:
-		return HNS_ROCE_QP_STATE_RTS;
-	case IB_QPS_SQD:
-		return HNS_ROCE_QP_STATE_SQD;
-	case IB_QPS_ERR:
-		return HNS_ROCE_QP_STATE_ERR;
-	default:
-		return HNS_ROCE_QP_NUM_STATE;
-	}
-}
-
 static void add_qp_to_list(struct hns_roce_dev *hr_dev,
 			   struct hns_roce_qp *hr_qp,
 			   struct ib_cq *send_cq, struct ib_cq *recv_cq)
-- 
2.33.0

[PATCH for-next 3/5] RDMA/hns: Remove redundant variable "ret"

[Wenpeng Liang]

From: Guofeng Yue <yueguofeng@hisilicon.com>

It is completely redundant for this function to use "ret" to store the
return value of the subfunction.

Signed-off-by: Guofeng Yue <yueguofeng@hisilicon.com>
Signed-off-by: Wenpeng Liang <liangwenpeng@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index cd87e6e86720..aa3eca16e04a 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -3257,7 +3257,6 @@ static int hns_roce_v2_write_mtpt(struct hns_roce_dev *hr_dev,
 				  void *mb_buf, struct hns_roce_mr *mr)
 {
 	struct hns_roce_v2_mpt_entry *mpt_entry;
-	int ret;
 
 	mpt_entry = mb_buf;
 	memset(mpt_entry, 0, sizeof(*mpt_entry));
@@ -3296,9 +3295,7 @@ static int hns_roce_v2_write_mtpt(struct hns_roce_dev *hr_dev,
 		     to_hr_hw_page_shift(mr->pbl_mtr.hem_cfg.ba_pg_shift));
 	hr_reg_enable(mpt_entry, MPT_INNER_PA_VLD);
 
-	ret = set_mtpt_pbl(hr_dev, mpt_entry, mr);
-
-	return ret;
+	return set_mtpt_pbl(hr_dev, mpt_entry, mr);
 }
 
 static int hns_roce_v2_rereg_write_mtpt(struct hns_roce_dev *hr_dev,
-- 
2.33.0

[PATCH for-next 4/5] RDMA/hns: Add judgment on the execution result of CMDQ that free vf resource

[Wenpeng Liang]

CDMQ may fail to execute, so its return value should not be ignored.

Signed-off-by: Wenpeng Liang <liangwenpeng@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index aa3eca16e04a..5d6da396586d 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -1510,7 +1510,7 @@ static void __hns_roce_function_clear(struct hns_roce_dev *hr_dev, int vf_id)
 	hns_roce_func_clr_rst_proc(hr_dev, ret, fclr_write_fail_flag);
 }
 
-static void hns_roce_free_vf_resource(struct hns_roce_dev *hr_dev, int vf_id)
+static int hns_roce_free_vf_resource(struct hns_roce_dev *hr_dev, int vf_id)
 {
 	enum hns_roce_opcode_type opcode = HNS_ROCE_OPC_ALLOC_VF_RES;
 	struct hns_roce_cmq_desc desc[2];
@@ -1521,17 +1521,26 @@ static void hns_roce_free_vf_resource(struct hns_roce_dev *hr_dev, int vf_id)
 	desc[0].flag |= cpu_to_le16(HNS_ROCE_CMD_FLAG_NEXT);
 	hns_roce_cmq_setup_basic_desc(&desc[1], opcode, false);
 	hr_reg_write(req_a, FUNC_RES_A_VF_ID, vf_id);
-	hns_roce_cmq_send(hr_dev, desc, 2);
+
+	return hns_roce_cmq_send(hr_dev, desc, 2);
 }
 
 static void hns_roce_function_clear(struct hns_roce_dev *hr_dev)
 {
+	int ret;
 	int i;
 
 	for (i = hr_dev->func_num - 1; i >= 0; i--) {
 		__hns_roce_function_clear(hr_dev, i);
-		if (i != 0)
-			hns_roce_free_vf_resource(hr_dev, i);
+
+		if (i == 0)
+			continue;
+
+		ret = hns_roce_free_vf_resource(hr_dev, i);
+		if (ret)
+			ibdev_err(&hr_dev->ib_dev,
+				  "failed to free vf resource, vf_id = %d, ret = %d.\n",
+				  i, ret);
 	}
 }
 
-- 
2.33.0

[PATCH for-next 5/5] RDMA/hns: Init the variable at the suitable place

[Wenpeng Liang]

From: Haoyue Xu <xuhaoyue1@hisilicon.com>

Assigning a value to ret in the init statement of a for-loop makes the code
less readable.

Signed-off-by: Haoyue Xu <xuhaoyue1@hisilicon.com>
Signed-off-by: Wenpeng Liang <liangwenpeng@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 5d6da396586d..2abed0e3dfd8 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -1296,7 +1296,8 @@ static int __hns_roce_cmq_send(struct hns_roce_dev *hr_dev,
 	} while (++timeout < priv->cmq.tx_timeout);
 
 	if (hns_roce_cmq_csq_done(hr_dev)) {
-		for (ret = 0, i = 0; i < num; i++) {
+		ret = 0;
+		for (i = 0; i < num; i++) {
 			/* check the result of hardware write back */
 			desc[i] = csq->desc[tail++];
 			if (tail == csq->desc_num)
-- 
2.33.0

Re: [PATCH for-next 0/5] RDMA/hns: Cleanup for removing redundant statements and judging return values

[Jason Gunthorpe]

On Sat, Apr 09, 2022 at 04:32:49PM +0800, Wenpeng Liang wrote:
> Most static warnings are detected by tools, mainly about:
> 
> (1) #1~3: Remove redundant statements.
> (2) #4~5: About the return value of the function.
> 
> Chengchang Tang (1):
>   RDMA/hns: Remove unnecessary check for the sgid_attr when modifying QP
> 
> Guofeng Yue (1):
>   RDMA/hns: Remove redundant variable "ret"
> 
> Haoyue Xu (1):
>   RDMA/hns: Init the variable at the suitable place
> 
> Wenpeng Liang (1):
>   RDMA/hns: Add judgment on the execution result of CMDQ that free vf
>     resource
> 
> Yixing Liu (1):
>   RDMA/hns: Remove unused function to_hns_roce_state()
> 
>  drivers/infiniband/hw/hns/hns_roce_device.h | 11 --------
>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 29 ++++++++++++---------
>  drivers/infiniband/hw/hns/hns_roce_qp.c     | 20 --------------
>  3 files changed, 17 insertions(+), 43 deletions(-)

Applied to for-next, thanks

Jason