Skip to contentSkip to conversation

igb: cleanup igb_enable_mas() a bit

PATCH In review Dan Carpenter 2 participants 2 messages Mar 19, 2015

Subsystems (3)
Network drivers Kernel janitors Intel Ethernet
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 igb: cleanup igb_enable_mas() a bit

Static checkers complain about this because we do:

	if (!(connsw & E1000_CONNSW_SERDESD)) {
		...
	} else if (connsw & E1000_CONNSW_SERDESD) {
		...
	} else {
		...
	}

Once you eliminate that E1000_CONNSW_SERDESD is set and not set then there aren't any other possibilities so the else statement is dead code.

This function always returns zero so if you delete the "ret_val" variable, the code is shorter and more clear.

Signed-off-by Dan Carpenter <[email protected]>
drivers/net/ethernet/intel/igb/igb_main.c +3 -11
index f366b3b..8dd581f 100644
@@ -1840,11 +1840,10 @@ static s32 igb_enable_mas(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	u32 connsw;
-	s32 ret_val = 0;
 
 	connsw = rd32(E1000_CONNSW);
-	if (!(hw->phy.media_type = e1000_media_type_copper))
-		return ret_val;
+	if (hw->phy.media_type != e1000_media_type_copper)
+		return 0;
 
 	/* configure for SerDes media detect */
 	if (!(connsw & E1000_CONNSW_SERDESD)) {

@@ -1852,15 +1851,8 @@ static s32 igb_enable_mas(struct igb_adapter *adapter)
 		connsw |= E1000_CONNSW_AUTOSENSE_EN;
 		wr32(E1000_CONNSW, connsw);
 		wrfl();
-	} else if (connsw & E1000_CONNSW_SERDESD) {
-		/* already SerDes, no need to enable anything */
-		return ret_val;
-	} else {
-		netdev_info(adapter->netdev,
-			"MAS: Unable to configure feature, disabling..\n");
-		adapter->flags &= ~IGB_FLAG_MAS_ENABLE;
 	}
-	return ret_val;
+	return 0;
 }
 
 void igb_reset(struct igb_adapter *adapter)
#1 lore raw
Quoted 10 lines · Dan Carpenter

Static checkers complain about this because we do:

        if (!(connsw & E1000_CONNSW_SERDESD)) {
                ...
        } else if (connsw & E1000_CONNSW_SERDESD) {
                ...
        } else {
                ...
        }
Quoted 8 lines

Once you eliminate that E1000_CONNSW_SERDESD is set and not set then there aren't any other possibilities so the else statement is dead code.

This function always returns zero so if you delete the "ret_val" variable, the code is shorter and more clear.

Signed-off-by Dan Carpenter <[email protected]>

Thanks Dan, surprised we did not see this earlier. I have added your patch to my queue.

Signature · 2 lines
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git
dev-queue