[PATCH 4.4 6/9] net/ncsi: Avoid if statements in ncsi_suspend_dev()

Gavin Shan gwshan at linux.vnet.ibm.com
Fri Oct 21 11:47:43 AEDT 2016


There are several if/else statements in the state machine implemented
by switch/case in ncsi_suspend_dev() to avoid duplicated code. It
makes the code a bit hard to be understood.

This drops if/else statements in ncsi_suspend_dev() to improve the
code readability as Joel Stanley suggested. Also, it becomes easy to
add more states in the state machine without affecting current code.
No logical changes introduced by this.

Comparing to the original patch, this also drops the tag "done" in
ncsi_suspend_dev(). The code path covered by the tag is replaced
by newly introduced tag "error".

Suggested-by: Joel Stanley <joel at jms.id.au>
Signed-off-by: Gavin Shan <gwshan at linux.vnet.ibm.com>
---
 net/ncsi/ncsi-manage.c | 83 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 32 deletions(-)

diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index e3fe191..99045ed 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -695,8 +695,8 @@ error:
 static void ncsi_dev_suspend(struct ncsi_dev_priv *ndp)
 {
 	struct ncsi_dev *nd = &ndp->ndp_ndev;
-	struct ncsi_package *np;
-	struct ncsi_channel *nc;
+	struct ncsi_package *np = ndp->ndp_active_package;
+	struct ncsi_channel *nc = ndp->ndp_active_channel;
 	struct ncsi_cmd_arg nca;
 	int ret;
 
@@ -705,51 +705,66 @@ static void ncsi_dev_suspend(struct ncsi_dev_priv *ndp)
 	switch (nd->nd_state) {
 	case ncsi_dev_state_suspend:
 		/* If there're no active channel, we're done */
-		if (!ndp->ndp_active_channel) {
-			nd->nd_state = ncsi_dev_state_suspend_done;
-			goto done;
-		}
+		if (!ndp->ndp_active_channel)
+			goto error;
 
 		nd->nd_state = ncsi_dev_state_suspend_select;
 		/* Fall through */
 	case ncsi_dev_state_suspend_select:
+		atomic_set(&ndp->ndp_pending_reqs, 1);
+
+		nca.nca_type = NCSI_PKT_CMD_SP;
+		nca.nca_package = np->np_id;
+		nca.nca_channel = NCSI_RESERVED_CHANNEL;
+		nca.nca_bytes[0] = 1;
+
+		nd->nd_state = ncsi_dev_state_suspend_dcnt;
+		ret = ncsi_xmit_cmd(&nca);
+		if (ret)
+			goto error;
+
+		break;
 	case ncsi_dev_state_suspend_dcnt:
+		atomic_set(&ndp->ndp_pending_reqs, 1);
+
+		nca.nca_type = NCSI_PKT_CMD_DCNT;
+		nca.nca_package = np->np_id;
+		nca.nca_channel = nc->nc_id;
+
+		nd->nd_state = ncsi_dev_state_suspend_dc;
+		ret = ncsi_xmit_cmd(&nca);
+		if (ret)
+			goto error;
+
+		break;
 	case ncsi_dev_state_suspend_dc:
+		atomic_set(&ndp->ndp_pending_reqs, 1);
+
+		nca.nca_type = NCSI_PKT_CMD_DC;
+		nca.nca_package = np->np_id;
+		nca.nca_channel = nc->nc_id;
+		nca.nca_bytes[0] = 1;
+
+		nd->nd_state = ncsi_dev_state_suspend_deselect;
+		ret = ncsi_xmit_cmd(&nca);
+		if (ret)
+			goto error;
+
+		break;
 	case ncsi_dev_state_suspend_deselect:
 		atomic_set(&ndp->ndp_pending_reqs, 1);
 
-		np = ndp->ndp_active_package;
-		nc = ndp->ndp_active_channel;
+		nca.nca_type = NCSI_PKT_CMD_DP;
 		nca.nca_package = np->np_id;
-		if (nd->nd_state == ncsi_dev_state_suspend_select) {
-			nca.nca_type = NCSI_PKT_CMD_SP;
-			nca.nca_channel = NCSI_RESERVED_CHANNEL;
-			nca.nca_bytes[0] = 1;
-			nd->nd_state = ncsi_dev_state_suspend_dcnt;
-		} else if (nd->nd_state == ncsi_dev_state_suspend_dcnt) {
-			nca.nca_type = NCSI_PKT_CMD_DCNT;
-			nca.nca_channel = nc->nc_id;
-			nd->nd_state = ncsi_dev_state_suspend_dc;
-		} else if (nd->nd_state == ncsi_dev_state_suspend_dc) {
-			nca.nca_type = NCSI_PKT_CMD_DC;
-			nca.nca_channel = nc->nc_id;
-			nca.nca_bytes[0] = 1;
-			nd->nd_state = ncsi_dev_state_suspend_deselect;
-		} else if (nd->nd_state == ncsi_dev_state_suspend_deselect) {
-			nca.nca_type = NCSI_PKT_CMD_DP;
-			nca.nca_channel = NCSI_RESERVED_CHANNEL;
-			nd->nd_state = ncsi_dev_state_suspend_done;
-		}
+		nca.nca_channel = NCSI_RESERVED_CHANNEL;
 
+		nd->nd_state = ncsi_dev_state_suspend_done;
 		ret = ncsi_xmit_cmd(&nca);
-		if (ret) {
-			nd->nd_state = ncsi_dev_state_suspend_done;
-			goto done;
-		}
+		if (ret)
+			goto error;
 
 		break;
 	case ncsi_dev_state_suspend_done:
-done:
 		if (ndp->ndp_flags & NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE)
 			ncsi_choose_active_channel(ndp);
 
@@ -767,6 +782,10 @@ done:
 		pr_warn("%s: Unsupported NCSI dev state 0x%x\n",
 			__func__, nd->nd_state);
 	}
+
+	return;
+error:
+	nd->nd_state = ncsi_dev_state_functional;
 }
 
 static void ncsi_dev_work(struct work_struct *work)
-- 
2.1.0



More information about the openbmc mailing list