[PATCH 1/4] silicom: checkpatch: assignments in if conditions

[PATCH 1/4] silicom: checkpatch: assignments in if conditions

[Lorenz Haspel]

Fixes checkpatch error:
There were assignments in if conditions, so I extracted them.

Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
---
 drivers/staging/silicom/bpctl_mod.c |  172 ++++++++++++++++++++---------------
 1 file changed, 98 insertions(+), 74 deletions(-)

diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
index a3b974b..d2debe8 100644
--- a/drivers/staging/silicom/bpctl_mod.c
+++ b/drivers/staging/silicom/bpctl_mod.c
@@ -245,9 +245,11 @@ static int bp_device_event(struct notifier_block *unused,
 	case NETDEV_CHANGE:{
 			if (netif_carrier_ok(dev))
 				return NOTIFY_DONE;
-
-			if (((dev_num = get_dev_idx(dev->ifindex)) == -1) ||
-			    (!(pbpctl_dev = &bpctl_dev_arr[dev_num])))
+			dev_num = get_dev_idx(dev->ifindex);
+			if (dev_num == -1)
+				return NOTIFY_DONE;
+			pbpctl_dev = &bpctl_dev_arr[dev_num];
+			if (!pbpctl_dev)
 				return NOTIFY_DONE;
 
 			if ((is_bypass_fn(pbpctl_dev)) == 1)
@@ -306,7 +308,8 @@ static void write_pulse(bpctl_dev_t *pbpctl_dev,
 		ctrl = BP10G_READ_REG(pbpctl_dev, ESDP);
 
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return;
 		ctrl = BP10G_READ_REG(pbpctl_dev_c, ESDP);
 	}
@@ -606,7 +609,8 @@ static int read_pulse(bpctl_dev_t *pbpctl_dev, unsigned int ctrl_ext,
 	if (pbpctl_dev->bp_540)
 		ctrl = BP10G_READ_REG(pbpctl_dev, ESDP);
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return -1;
 		ctrl = BP10G_READ_REG(pbpctl_dev_c, ESDP);
 	}
@@ -775,7 +779,8 @@ static void write_reg(bpctl_dev_t *pbpctl_dev, unsigned char value,
 	bpctl_dev_t *pbpctl_dev_c = NULL;
 	unsigned long flags;
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return;
 	}
 	if ((pbpctl_dev->wdt_status == WDT_STATUS_EN) &&
@@ -953,7 +958,8 @@ static int read_reg(bpctl_dev_t *pbpctl_dev, unsigned char addr)
 	atomic_set(&pbpctl_dev->wdt_busy, 1);
 #endif
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return -1;
 	}
 
@@ -1224,7 +1230,8 @@ static int wdt_pulse(bpctl_dev_t *pbpctl_dev)
 		return -1;
 #endif
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return -1;
 	}
 
@@ -1743,8 +1750,8 @@ static void write_data_port_int(bpctl_dev_t *pbpctl_dev,
 static int write_data_int(bpctl_dev_t *pbpctl_dev, unsigned char value)
 {
 	bpctl_dev_t *pbpctl_dev_b = NULL;
-
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+	if (!pbpctl_dev_b)
 		return -1;
 	atomic_set(&pbpctl_dev->wdt_busy, 1);
 	write_data_port_int(pbpctl_dev, value & 0x3);
@@ -1965,7 +1972,8 @@ int tpl_hw_on(bpctl_dev_t *pbpctl_dev)
 	int ret = 0, ctrl = 0;
 	bpctl_dev_t *pbpctl_dev_b = NULL;
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+	if (!pbpctl_dev_b)
 		return BP_NOT_CAP;
 
 	if (pbpctl_dev->bp_caps_ex & TPL2_CAP_EX) {
@@ -1991,8 +1999,8 @@ int tpl_hw_off(bpctl_dev_t *pbpctl_dev)
 {
 	int ret = 0, ctrl = 0;
 	bpctl_dev_t *pbpctl_dev_b = NULL;
-
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+	if (!pbpctl_dev_b)
 		return BP_NOT_CAP;
 	if (pbpctl_dev->bp_caps_ex & TPL2_CAP_EX) {
 		cmnd_on(pbpctl_dev);
@@ -2373,11 +2381,10 @@ static int set_tx(bpctl_dev_t *pbpctl_dev, int tx_state)
 		if (PEG5_IF_SERIES(pbpctl_dev->subdevice)) {
 			if (tx_state) {
 				uint16_t mii_reg;
-				if (!
-				    (ret =
-				     bp75_read_phy_reg(pbpctl_dev,
+				ret = bp75_read_phy_reg(pbpctl_dev,
 						       BPCTLI_PHY_CONTROL,
-						       &mii_reg))) {
+						       &mii_reg);
+				if (!ret) {
 					if (mii_reg & BPCTLI_MII_CR_POWER_DOWN) {
 						ret =
 						    bp75_write_phy_reg
@@ -2389,12 +2396,10 @@ static int set_tx(bpctl_dev_t *pbpctl_dev, int tx_state)
 				}
 			} else {
 				uint16_t mii_reg;
-				if (!
-				    (ret =
-				     bp75_read_phy_reg(pbpctl_dev,
+				ret = bp75_read_phy_reg(pbpctl_dev,
 						       BPCTLI_PHY_CONTROL,
-						       &mii_reg))) {
-
+						       &mii_reg);
+				if (!ret) {
 					mii_reg |= BPCTLI_MII_CR_POWER_DOWN;
 					ret =
 					    bp75_write_phy_reg(pbpctl_dev,
@@ -3237,30 +3242,33 @@ int bypass_from_last_read(bpctl_dev_t *pbpctl_dev)
 	uint32_t ctrl_ext = 0;
 	bpctl_dev_t *pbpctl_dev_b = NULL;
 
-	if ((pbpctl_dev->bp_caps & SW_CTL_CAP)
-	    && (pbpctl_dev_b = get_status_port_fn(pbpctl_dev))) {
-		ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
-		BPCTL_BP_WRITE_REG(pbpctl_dev_b, CTRL_EXT,
+	if (pbpctl_dev->bp_caps & SW_CTL_CAP) {
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (pbpctl_dev_b) {
+			ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
+			BPCTL_BP_WRITE_REG(pbpctl_dev_b, CTRL_EXT,
 				   (ctrl_ext & ~BPCTLI_CTRL_EXT_SDP7_DIR));
-		ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
-		if (ctrl_ext & BPCTLI_CTRL_EXT_SDP7_DATA)
-			return 0;
-		return 1;
-	} else
-		return BP_NOT_CAP;
+			ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
+			if (ctrl_ext & BPCTLI_CTRL_EXT_SDP7_DATA)
+				return 0;
+			return 1;
+		}
+	}
+	return BP_NOT_CAP;
 }
 
 int bypass_status_clear(bpctl_dev_t *pbpctl_dev)
 {
 	bpctl_dev_t *pbpctl_dev_b = NULL;
 
-	if ((pbpctl_dev->bp_caps & SW_CTL_CAP)
-	    && (pbpctl_dev_b = get_status_port_fn(pbpctl_dev))) {
-
-		send_bypass_clear_pulse(pbpctl_dev_b, 1);
-		return 0;
-	} else
-		return BP_NOT_CAP;
+	if (pbpctl_dev->bp_caps & SW_CTL_CAP) {
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (pbpctl_dev_b) {
+			send_bypass_clear_pulse(pbpctl_dev_b, 1);
+			return 0;
+		}
+	}
+	return BP_NOT_CAP;
 }
 
 int bypass_flag_status(bpctl_dev_t *pbpctl_dev)
@@ -3328,8 +3336,8 @@ static int bypass_status(bpctl_dev_t *pbpctl_dev)
 	if (pbpctl_dev->bp_caps & BP_CAP) {
 
 		bpctl_dev_t *pbpctl_dev_b = NULL;
-
-		if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_b)
 			return BP_NOT_CAP;
 
 		if (INTEL_IF_SERIES(pbpctl_dev->subdevice)) {
@@ -3616,8 +3624,8 @@ int tap_status(bpctl_dev_t *pbpctl_dev)
 
 	if (pbpctl_dev->bp_caps & TAP_CAP) {
 		bpctl_dev_t *pbpctl_dev_b = NULL;
-
-		if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_b)
 			return BP_NOT_CAP;
 
 		if (pbpctl_dev->bp_ext_ver >= 0x8) {
@@ -3713,7 +3721,8 @@ int disc_off_status(bpctl_dev_t *pbpctl_dev)
 	u32 ctrl_ext = 0;
 
 	if (pbpctl_dev->bp_caps & DISC_CAP) {
-		if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_b)
 			return BP_NOT_CAP;
 		if (DISCF_IF_SERIES(pbpctl_dev->subdevice))
 			return ((((read_reg(pbpctl_dev, STATUS_DISC_REG_ADDR)) &
@@ -3794,8 +3803,8 @@ static int disc_status(bpctl_dev_t *pbpctl_dev)
 {
 	int ctrl = 0;
 	if (pbpctl_dev->bp_caps & DISC_CAP) {
-
-		if ((ctrl = disc_off_status(pbpctl_dev)) < 0)
+		ctrl = disc_off_status(pbpctl_dev);
+		if (ctrl < 0)
 			return ctrl;
 		return ((ctrl == 0) ? 1 : 0);
 
@@ -3910,8 +3919,8 @@ int tpl2_flag_status(bpctl_dev_t *pbpctl_dev)
 int tpl_hw_status(bpctl_dev_t *pbpctl_dev)
 {
 	bpctl_dev_t *pbpctl_dev_b = NULL;
-
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+	if (!pbpctl_dev_b)
 		return BP_NOT_CAP;
 
 	if (TPL_IF_SERIES(pbpctl_dev->subdevice))
@@ -4216,8 +4225,8 @@ void bypass_caps_init(bpctl_dev_t *pbpctl_dev)
 int bypass_off_init(bpctl_dev_t *pbpctl_dev)
 {
 	int ret = 0;
-
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (INTEL_IF_SERIES(pbpctl_dev->subdevice))
 		return dis_bypass_cap(pbpctl_dev);
@@ -4403,7 +4412,8 @@ int set_bypass_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
 
 	if (!(pbpctl_dev->bp_caps & BP_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (!bypass_mode)
 		ret = bypass_off(pbpctl_dev);
@@ -4435,7 +4445,8 @@ int set_dis_bypass_fn(bpctl_dev_t *pbpctl_dev, int dis_param)
 
 	if (!(pbpctl_dev->bp_caps & BP_DIS_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (dis_param)
 		ret = dis_bypass_cap(pbpctl_dev);
@@ -4461,11 +4472,13 @@ int set_bypass_pwoff_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
 
 	if (!(pbpctl_dev->bp_caps & BP_PWOFF_CTL_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (bypass_mode)
 		ret = bypass_state_pwroff(pbpctl_dev);
-	else
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		ret = normal_state_pwroff(pbpctl_dev);
 	cmnd_off(pbpctl_dev);
 	return ret;
@@ -4487,7 +4500,8 @@ int set_bypass_pwup_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
 
 	if (!(pbpctl_dev->bp_caps & BP_PWUP_CTL_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (bypass_mode)
 		ret = bypass_state_pwron(pbpctl_dev);
@@ -4514,7 +4528,8 @@ int set_bypass_wd_fn(bpctl_dev_t *pbpctl_dev, int timeout)
 	if (!(pbpctl_dev->bp_caps & WD_CTL_CAP))
 		return BP_NOT_CAP;
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (!timeout)
 		ret = wdt_off(pbpctl_dev);
@@ -4583,7 +4598,8 @@ int set_std_nic_fn(bpctl_dev_t *pbpctl_dev, int nic_mode)
 	if (!(pbpctl_dev->bp_caps & STD_NIC_CAP))
 		return BP_NOT_CAP;
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (nic_mode)
 		ret = std_nic_on(pbpctl_dev);
@@ -4648,8 +4664,8 @@ int get_tap_pwup_fn(bpctl_dev_t *pbpctl_dev)
 	int ret = 0;
 	if (!pbpctl_dev)
 		return -1;
-
-	if ((ret = default_pwron_tap_status(pbpctl_dev)) < 0)
+	ret = default_pwron_tap_status(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	return ((ret == 0) ? 1 : 0);
 }
@@ -4823,8 +4839,8 @@ int get_disc_port_pwup_fn(bpctl_dev_t *pbpctl_dev)
 	int ret = 0;
 	if (!pbpctl_dev)
 		return -1;
-
-	if ((ret = default_pwron_disc_port_status(pbpctl_dev)) < 0)
+	ret = default_pwron_disc_port_status(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	return ((ret == 0) ? 1 : 0);
 }
@@ -4851,7 +4867,8 @@ int reset_cont_fn(bpctl_dev_t *pbpctl_dev)
 	if (!pbpctl_dev)
 		return -1;
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	return reset_cont(pbpctl_dev);
 }
@@ -4867,10 +4884,12 @@ int set_tx_fn(bpctl_dev_t *pbpctl_dev, int tx_state)
 	    (pbpctl_dev->bp_caps & SW_CTL_CAP)) {
 		if ((pbpctl_dev->bp_tpl_flag))
 			return BP_NOT_CAP;
-	} else if ((pbpctl_dev_b = get_master_port_fn(pbpctl_dev))) {
-		if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
-		    (pbpctl_dev_b->bp_tpl_flag))
-			return BP_NOT_CAP;
+	} else {
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (pbpctl_dev_b)
+			if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
+			    (pbpctl_dev_b->bp_tpl_flag))
+				return BP_NOT_CAP;
 	}
 	return set_tx(pbpctl_dev, tx_state);
 }
@@ -4984,10 +5003,13 @@ int get_tx_fn(bpctl_dev_t *pbpctl_dev)
 	    (pbpctl_dev->bp_caps & SW_CTL_CAP)) {
 		if ((pbpctl_dev->bp_tpl_flag))
 			return BP_NOT_CAP;
-	} else if ((pbpctl_dev_b = get_master_port_fn(pbpctl_dev))) {
-		if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
-		    (pbpctl_dev_b->bp_tpl_flag))
-			return BP_NOT_CAP;
+	} else{
+		pbpctl_dev_b = get_master_port_fn(pbpctl_dev);
+		if (pbpctl_dev_b) {
+			if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
+			    (pbpctl_dev_b->bp_tpl_flag))
+				return BP_NOT_CAP;
+		}
 	}
 	return tx_status(pbpctl_dev);
 }
@@ -5023,8 +5045,8 @@ static void bp_tpl_timer_fn(unsigned long param)
 	bpctl_dev_t *pbpctl_dev = (bpctl_dev_t *) param;
 	uint32_t link1, link2;
 	bpctl_dev_t *pbpctl_dev_b = NULL;
-
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+	if (!pbpctl_dev_b)
 		return;
 
 	if (!pbpctl_dev->bp_tpl_flag) {
@@ -5128,7 +5150,8 @@ int set_tpl_fn(bpctl_dev_t *pbpctl_dev, int tpl_mode)
 
 	if (pbpctl_dev->bp_caps & TPL_CAP) {
 		if (tpl_mode) {
-			if ((pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+			pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+			if (pbpctl_dev_b)
 				set_tx(pbpctl_dev_b, 1);
 			set_tx(pbpctl_dev, 1);
 		}
@@ -6708,7 +6731,8 @@ static int init_one(bpctl_dev_t *dev, bpmod_info_t *info, struct pci_dev *pdev1)
 			reset_cont(dev);
 	}
 #ifdef BP_SELF_TEST
-	if ((dev->bp_tx_data = kzalloc(BPTEST_DATA_LEN, GFP_KERNEL))) {
+	dev->bp_tx_data = kzalloc(BPTEST_DATA_LEN, GFP_KERNEL);
+	if (dev->bp_tx_data) {
 		memset(dev->bp_tx_data, 0xff, 6);
 		memset(dev->bp_tx_data + 6, 0x0, 1);
 		memset(dev->bp_tx_data + 7, 0xaa, 5);
-- 
1.7.10.4

[PATCH 2/4] silicom: checkpatch: fixed whitespace errors

[Lorenz Haspel]

started cleanfile
also fixed some other whitespace errors cleanfile didn't find

Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
---
 drivers/staging/silicom/bpctl_mod.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
index d2debe8..1461130 100644
--- a/drivers/staging/silicom/bpctl_mod.c
+++ b/drivers/staging/silicom/bpctl_mod.c
@@ -1421,8 +1421,8 @@ static int wdt_pulse(bpctl_dev_t *pbpctl_dev)
 				(ctrl_ext &
 				 ~(BP10G_MCLK_DATA_OUT | BP10G_MDIO_DATA_OUT)));
 	}
-	if ((pbpctl_dev->wdt_status == WDT_STATUS_EN)	/*&&
-							   (pbpctl_dev->bp_ext_ver<PXG4BPFI_VER) */)
+	if ((pbpctl_dev->wdt_status == WDT_STATUS_EN))
+		/*&& (pbpctl_dev->bp_ext_ver<PXG4BPFI_VER) */
 		pbpctl_dev->bypass_wdt_on_time = jiffies;
 #ifdef BP_SYNC_FLAG
 	spin_unlock_irqrestore(&pbpctl_dev->bypass_wr_lock, flags);
@@ -6652,7 +6652,7 @@ static void find_fw(bpctl_dev_t *dev)
 			    ioremap(mmio_start, mmio_len);
 
 			dev->bp_fw_ver = bypass_fw_ver(dev);
-			if (dev-> bp_fw_ver == 0xa8)
+			if (dev->bp_fw_ver == 0xa8)
 				break;
 		}
 	}
-- 
1.7.10.4

[PATCH 3/4] silicom: checkpatch: trailing statements

[Lorenz Haspel]

fixed checkpatch error:
trailing statements that should be on next line

Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
---
 drivers/staging/silicom/bpctl_mod.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
index 1461130..b1d6041 100644
--- a/drivers/staging/silicom/bpctl_mod.c
+++ b/drivers/staging/silicom/bpctl_mod.c
@@ -165,7 +165,8 @@ static int bp_device_event(struct notifier_block *unused,
 			memcpy(&cbuf, drvinfo.bus_info, 32);
 			buf = &cbuf[0];
 
-			while (*buf++ != ':');
+			while (*buf++ != ':')
+				;
 			for (i = 0; i < 10; i++, buf++) {
 				if (*buf == ':')
 					break;
@@ -2158,12 +2159,14 @@ static void bp75_release_phy(bpctl_dev_t *pbpctl_dev)
 {
 	u16 mask = BPCTLI_SWFW_PHY0_SM;
 	u32 swfw_sync;
+	s32 ret_val;
 
 	if ((pbpctl_dev->func == 1) || (pbpctl_dev->func == 3))
 		mask = BPCTLI_SWFW_PHY1_SM;
 
-	while (bp75_get_hw_semaphore_generic(pbpctl_dev) != 0);
-	/* Empty */
+	do
+		ret_val = bp75_get_hw_semaphore_generic(pbpctl_dev);
+	while (ret_val != 0);
 
 	swfw_sync = BPCTL_READ_REG(pbpctl_dev, SW_FW_SYNC);
 	swfw_sync &= ~mask;
@@ -5368,7 +5371,8 @@ static void if_scan_init(void)
 		memcpy(&cbuf, drvinfo.bus_info, 32);
 		buf = &cbuf[0];
 
-		while (*buf++ != ':');
+		while (*buf++ != ':')
+			;
 		for (i = 0; i < 10; i++, buf++) {
 			if (*buf == ':')
 				break;
-- 
1.7.10.4

[PATCH 4/4] solicom: checkpatch: added parenthesis to macros

[Lorenz Haspel]

fixed checkpatch error:
added parenthesis around complex macros.

Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
---
 drivers/staging/silicom/bpctl_mod.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
index b1d6041..c08aa9f 100644
--- a/drivers/staging/silicom/bpctl_mod.c
+++ b/drivers/staging/silicom/bpctl_mod.c
@@ -44,9 +44,11 @@ MODULE_VERSION(BP_MOD_VER);
 spinlock_t bpvm_lock;
 
 #define lock_bpctl()					\
-if (down_interruptible(&bpctl_sema)) {			\
-	return -ERESTARTSYS;				\
-}							\
+do {							\
+	if (down_interruptible(&bpctl_sema)) {		\
+		return -ERESTARTSYS;			\
+	}						\
+} while (0)
 
 #define unlock_bpctl()					\
 	up(&bpctl_sema);
@@ -7879,7 +7881,8 @@ int bypass_proc_create_dev_sd(bpctl_dev_t *pbp_device_block)
 	}
 	current_pfs->bypass_entry = procfs_dir;
 
-#define ENTRY(x) ret |= procfs_add(#x, &x##_ops, pbp_device_block)
+#define ENTRY(x) (ret |= procfs_add(#x, &x##_ops, pbp_device_block))
+
 	ENTRY(bypass_info);
 	if (pbp_device_block->bp_caps & SW_CTL_CAP) {
 		/* Create set param proc's */
-- 
1.7.10.4

Re: [PATCH 4/4] solicom: checkpatch: added parenthesis to macros

[Joe Perches]

On Mon, 2013-06-17 at 18:26 +0200, Lorenz Haspel wrote:
> fixed checkpatch error:
> added parenthesis around complex macros.
[]
> diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
[]
> @@ -44,9 +44,11 @@ MODULE_VERSION(BP_MOD_VER);
>  spinlock_t bpvm_lock;
>  
>  #define lock_bpctl()					\
> -if (down_interruptible(&bpctl_sema)) {			\
> -	return -ERESTARTSYS;				\
> -}							\
> +do {							\
> +	if (down_interruptible(&bpctl_sema)) {		\
> +		return -ERESTARTSYS;			\
> +	}						\
> +} while (0)

Macros with goto or return are also poor style.
Probably better to expand these in-place instead.

Re: [PATCH 1/4] silicom: checkpatch: assignments in if conditions

[Dan Carpenter]

On Mon, Jun 17, 2013 at 06:26:23PM +0200, Lorenz Haspel wrote:
> Fixes checkpatch error:
> There were assignments in if conditions, so I extracted them.
> 
> Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
> Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>

Signed-off-by is a legal thing like signing a document.  It could be
that Michael actually signed off on these, and that's fine.  Anyway,
I'm curious now, why is Michael signing off on these?

regards,
dan carpenter

Re: [PATCH 1/4] silicom: checkpatch: assignments in if conditions

[Dan Carpenter]

This will need to be redone because there were some buggy extra
lines added toward the end of the patch.

On Mon, Jun 17, 2013 at 06:26:23PM +0200, Lorenz Haspel wrote:
> @@ -1743,8 +1750,8 @@ static void write_data_port_int(bpctl_dev_t *pbpctl_dev,
>  static int write_data_int(bpctl_dev_t *pbpctl_dev, unsigned char value)
>  {
>  	bpctl_dev_t *pbpctl_dev_b = NULL;
> -

This blank line is needed.  (Put a blank line between declarations
and code).

> -	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
> +	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
> +	if (!pbpctl_dev_b)
>  		return -1;
>  	atomic_set(&pbpctl_dev->wdt_busy, 1);
>  	write_data_port_int(pbpctl_dev, value & 0x3);
> @@ -1965,7 +1972,8 @@ int tpl_hw_on(bpctl_dev_t *pbpctl_dev)
>  	int ret = 0, ctrl = 0;
>  	bpctl_dev_t *pbpctl_dev_b = NULL;
>  
> -	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
> +	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
> +	if (!pbpctl_dev_b)
>  		return BP_NOT_CAP;
>  
>  	if (pbpctl_dev->bp_caps_ex & TPL2_CAP_EX) {

> @@ -1991,8 +1999,8 @@ int tpl_hw_off(bpctl_dev_t *pbpctl_dev)
>  {
>  	int ret = 0, ctrl = 0;
>  	bpctl_dev_t *pbpctl_dev_b = NULL;
> -

Same for this blank line and all the following instances.

> -	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
> +	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
> +	if (!pbpctl_dev_b)
>  		return BP_NOT_CAP;
>  	if (pbpctl_dev->bp_caps_ex & TPL2_CAP_EX) {
>  		cmnd_on(pbpctl_dev);

> @@ -4461,11 +4472,13 @@ int set_bypass_pwoff_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
>  
>  	if (!(pbpctl_dev->bp_caps & BP_PWOFF_CTL_CAP))
>  		return BP_NOT_CAP;
> -	if ((ret = cmnd_on(pbpctl_dev)) < 0)
> +	ret = cmnd_on(pbpctl_dev);
> +	if (ret < 0)
>  		return ret;
>  	if (bypass_mode)
>  		ret = bypass_state_pwroff(pbpctl_dev);
> -	else
> +	ret = cmnd_on(pbpctl_dev);
> +	if (ret < 0)
>  		ret = normal_state_pwroff(pbpctl_dev);

These added lines do not belong.  The patch will need to be redone
to fix this bug.

>  	cmnd_off(pbpctl_dev);
>  	return ret;

> @@ -4867,10 +4884,12 @@ int set_tx_fn(bpctl_dev_t *pbpctl_dev, int tx_state)
>  	    (pbpctl_dev->bp_caps & SW_CTL_CAP)) {
>  		if ((pbpctl_dev->bp_tpl_flag))
>  			return BP_NOT_CAP;
> -	} else if ((pbpctl_dev_b = get_master_port_fn(pbpctl_dev))) {
> -		if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
> -		    (pbpctl_dev_b->bp_tpl_flag))
> -			return BP_NOT_CAP;
> +	} else {
> +		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
> +		if (pbpctl_dev_b)
> +			if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
> +			    (pbpctl_dev_b->bp_tpl_flag))
> +				return BP_NOT_CAP;

Please put curly brace {} around multi-line indents.  Even though
they are not needed for semantic reasons they make the code more
readable.

>  	}
>  	return set_tx(pbpctl_dev, tx_state);
>  }

regards,
dan carpenter

Re: [PATCH 1/4] silicom: checkpatch: assignments in if conditions

[Dan Carpenter]

On Mon, Jun 17, 2013 at 07:17:29PM +0200, Michael Banken wrote:
> Lorenz and I are working on these patches as a team in the same room.
> That is why our patches are signed off by both of us.

Good deal.

regards,
dan carpenter

Re: [PATCH 1/4] silicom: checkpatch: assignments in if conditions

[Dan Carpenter]

Btw, when you redo this one, you can probably just send this one by
itself as a reply to the original email without resending the other
3.  There is an in-reply-to option in git send.  I use mutt so I
don't know the details.

1)  Subject should be:

Subject: [PATCH 1/4 v2] silicom: checkpatch: assignments in if conditions

2) After the signed-off-by lines put:

signed-off-by: you
---
v2: removed some buggy extra lines and fixed white space issues

regards,
dan carpenter

Re: [PATCH 1/4] silicom: checkpatch: assignments in if conditions

[Joe Perches]

On Mon, 2013-06-17 at 20:22 +0300, Dan Carpenter wrote:
> This will need to be redone because there were some buggy extra
> lines added toward the end of the patch.

[]

> > @@ -4867,10 +4884,12 @@ int set_tx_fn(bpctl_dev_t *pbpctl_dev, int tx_state)
> >  	    (pbpctl_dev->bp_caps & SW_CTL_CAP)) {
> >  		if ((pbpctl_dev->bp_tpl_flag))
> >  			return BP_NOT_CAP;
> > -	} else if ((pbpctl_dev_b = get_master_port_fn(pbpctl_dev))) {
> > -		if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
> > -		    (pbpctl_dev_b->bp_tpl_flag))
> > -			return BP_NOT_CAP;
> > +	} else {
> > +		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
> > +		if (pbpctl_dev_b)
> > +			if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
> > +			    (pbpctl_dev_b->bp_tpl_flag))
> > +				return BP_NOT_CAP;
> 
> Please put curly brace {} around multi-line indents.  Even though
> they are not needed for semantic reasons they make the code more
> readable.

Better still would be to combine the multi-statement ifs
into a single test and avoid the braces altogether.

		if (pbpctl_dev_b &&
		    pbpctl_dev_b->bp_caps & TPL_CAP &&
		    pbpctl_dev_b->bp_tpl_flag)

[PATCH 1/4 v2] silicom: checkpatch: assignments in if conditions

[Lorenz Haspel]

Fixes checkpatch error:
There were assignments in if conditions, so I extracted them.

Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
---
v2: removed some buggy extra lines and fixed white space issues
---
 drivers/staging/silicom/bpctl_mod.c |  182 +++++++++++++++++++++++------------
 1 file changed, 119 insertions(+), 63 deletions(-)

diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
index a3b974b..82b4dbb 100644
--- a/drivers/staging/silicom/bpctl_mod.c
+++ b/drivers/staging/silicom/bpctl_mod.c
@@ -245,9 +245,14 @@ static int bp_device_event(struct notifier_block *unused,
 	case NETDEV_CHANGE:{
 			if (netif_carrier_ok(dev))
 				return NOTIFY_DONE;
+			dev_num = get_dev_idx(dev->ifindex);
 
-			if (((dev_num = get_dev_idx(dev->ifindex)) == -1) ||
-			    (!(pbpctl_dev = &bpctl_dev_arr[dev_num])))
+			if (dev_num == -1)
+				return NOTIFY_DONE;
+
+			pbpctl_dev = &bpctl_dev_arr[dev_num];
+
+			if (!pbpctl_dev)
 				return NOTIFY_DONE;
 
 			if ((is_bypass_fn(pbpctl_dev)) == 1)
@@ -306,7 +311,9 @@ static void write_pulse(bpctl_dev_t *pbpctl_dev,
 		ctrl = BP10G_READ_REG(pbpctl_dev, ESDP);
 
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+
+		if (!pbpctl_dev_c)
 			return;
 		ctrl = BP10G_READ_REG(pbpctl_dev_c, ESDP);
 	}
@@ -606,7 +613,9 @@ static int read_pulse(bpctl_dev_t *pbpctl_dev, unsigned int ctrl_ext,
 	if (pbpctl_dev->bp_540)
 		ctrl = BP10G_READ_REG(pbpctl_dev, ESDP);
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+
+		if (!pbpctl_dev_c)
 			return -1;
 		ctrl = BP10G_READ_REG(pbpctl_dev_c, ESDP);
 	}
@@ -775,7 +784,9 @@ static void write_reg(bpctl_dev_t *pbpctl_dev, unsigned char value,
 	bpctl_dev_t *pbpctl_dev_c = NULL;
 	unsigned long flags;
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+
+		if (!pbpctl_dev_c)
 			return;
 	}
 	if ((pbpctl_dev->wdt_status == WDT_STATUS_EN) &&
@@ -953,7 +964,9 @@ static int read_reg(bpctl_dev_t *pbpctl_dev, unsigned char addr)
 	atomic_set(&pbpctl_dev->wdt_busy, 1);
 #endif
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+
+		if (!pbpctl_dev_c)
 			return -1;
 	}
 
@@ -1224,7 +1237,9 @@ static int wdt_pulse(bpctl_dev_t *pbpctl_dev)
 		return -1;
 #endif
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+
+		if (!pbpctl_dev_c)
 			return -1;
 	}
 
@@ -1742,9 +1757,9 @@ static void write_data_port_int(bpctl_dev_t *pbpctl_dev,
 
 static int write_data_int(bpctl_dev_t *pbpctl_dev, unsigned char value)
 {
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return -1;
 	atomic_set(&pbpctl_dev->wdt_busy, 1);
 	write_data_port_int(pbpctl_dev, value & 0x3);
@@ -1963,9 +1978,9 @@ int disc_port_off(bpctl_dev_t *pbpctl_dev)
 int tpl_hw_on(bpctl_dev_t *pbpctl_dev)
 {
 	int ret = 0, ctrl = 0;
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return BP_NOT_CAP;
 
 	if (pbpctl_dev->bp_caps_ex & TPL2_CAP_EX) {
@@ -1990,9 +2005,9 @@ int tpl_hw_on(bpctl_dev_t *pbpctl_dev)
 int tpl_hw_off(bpctl_dev_t *pbpctl_dev)
 {
 	int ret = 0, ctrl = 0;
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return BP_NOT_CAP;
 	if (pbpctl_dev->bp_caps_ex & TPL2_CAP_EX) {
 		cmnd_on(pbpctl_dev);
@@ -2373,11 +2388,11 @@ static int set_tx(bpctl_dev_t *pbpctl_dev, int tx_state)
 		if (PEG5_IF_SERIES(pbpctl_dev->subdevice)) {
 			if (tx_state) {
 				uint16_t mii_reg;
-				if (!
-				    (ret =
-				     bp75_read_phy_reg(pbpctl_dev,
+				ret = bp75_read_phy_reg(pbpctl_dev,
 						       BPCTLI_PHY_CONTROL,
-						       &mii_reg))) {
+						       &mii_reg);
+
+				if (!ret) {
 					if (mii_reg & BPCTLI_MII_CR_POWER_DOWN) {
 						ret =
 						    bp75_write_phy_reg
@@ -2389,12 +2404,11 @@ static int set_tx(bpctl_dev_t *pbpctl_dev, int tx_state)
 				}
 			} else {
 				uint16_t mii_reg;
-				if (!
-				    (ret =
-				     bp75_read_phy_reg(pbpctl_dev,
+				ret = bp75_read_phy_reg(pbpctl_dev,
 						       BPCTLI_PHY_CONTROL,
-						       &mii_reg))) {
+						       &mii_reg);
 
+				if (!ret) {
 					mii_reg |= BPCTLI_MII_CR_POWER_DOWN;
 					ret =
 					    bp75_write_phy_reg(pbpctl_dev,
@@ -3237,30 +3251,36 @@ int bypass_from_last_read(bpctl_dev_t *pbpctl_dev)
 	uint32_t ctrl_ext = 0;
 	bpctl_dev_t *pbpctl_dev_b = NULL;
 
-	if ((pbpctl_dev->bp_caps & SW_CTL_CAP)
-	    && (pbpctl_dev_b = get_status_port_fn(pbpctl_dev))) {
-		ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
-		BPCTL_BP_WRITE_REG(pbpctl_dev_b, CTRL_EXT,
+	if (pbpctl_dev->bp_caps & SW_CTL_CAP) {
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+
+		if (pbpctl_dev_b) {
+			ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
+			BPCTL_BP_WRITE_REG(pbpctl_dev_b, CTRL_EXT,
 				   (ctrl_ext & ~BPCTLI_CTRL_EXT_SDP7_DIR));
-		ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
-		if (ctrl_ext & BPCTLI_CTRL_EXT_SDP7_DATA)
-			return 0;
-		return 1;
-	} else
-		return BP_NOT_CAP;
+			ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
+
+			if (ctrl_ext & BPCTLI_CTRL_EXT_SDP7_DATA)
+				return 0;
+			return 1;
+		}
+	}
+	return BP_NOT_CAP;
 }
 
 int bypass_status_clear(bpctl_dev_t *pbpctl_dev)
 {
 	bpctl_dev_t *pbpctl_dev_b = NULL;
 
-	if ((pbpctl_dev->bp_caps & SW_CTL_CAP)
-	    && (pbpctl_dev_b = get_status_port_fn(pbpctl_dev))) {
+	if (pbpctl_dev->bp_caps & SW_CTL_CAP) {
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-		send_bypass_clear_pulse(pbpctl_dev_b, 1);
-		return 0;
-	} else
-		return BP_NOT_CAP;
+		if (pbpctl_dev_b) {
+			send_bypass_clear_pulse(pbpctl_dev_b, 1);
+			return 0;
+		}
+	}
+	return BP_NOT_CAP;
 }
 
 int bypass_flag_status(bpctl_dev_t *pbpctl_dev)
@@ -3327,9 +3347,9 @@ static int bypass_status(bpctl_dev_t *pbpctl_dev)
 	u32 ctrl_ext = 0;
 	if (pbpctl_dev->bp_caps & BP_CAP) {
 
-		bpctl_dev_t *pbpctl_dev_b = NULL;
+		bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-		if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+		if (!pbpctl_dev_b)
 			return BP_NOT_CAP;
 
 		if (INTEL_IF_SERIES(pbpctl_dev->subdevice)) {
@@ -3615,9 +3635,9 @@ int tap_status(bpctl_dev_t *pbpctl_dev)
 	u32 ctrl_ext = 0;
 
 	if (pbpctl_dev->bp_caps & TAP_CAP) {
-		bpctl_dev_t *pbpctl_dev_b = NULL;
+		bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-		if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+		if (!pbpctl_dev_b)
 			return BP_NOT_CAP;
 
 		if (pbpctl_dev->bp_ext_ver >= 0x8) {
@@ -3713,7 +3733,9 @@ int disc_off_status(bpctl_dev_t *pbpctl_dev)
 	u32 ctrl_ext = 0;
 
 	if (pbpctl_dev->bp_caps & DISC_CAP) {
-		if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+
+		if (!pbpctl_dev_b)
 			return BP_NOT_CAP;
 		if (DISCF_IF_SERIES(pbpctl_dev->subdevice))
 			return ((((read_reg(pbpctl_dev, STATUS_DISC_REG_ADDR)) &
@@ -3794,8 +3816,9 @@ static int disc_status(bpctl_dev_t *pbpctl_dev)
 {
 	int ctrl = 0;
 	if (pbpctl_dev->bp_caps & DISC_CAP) {
+		ctrl = disc_off_status(pbpctl_dev);
 
-		if ((ctrl = disc_off_status(pbpctl_dev)) < 0)
+		if (ctrl < 0)
 			return ctrl;
 		return ((ctrl == 0) ? 1 : 0);
 
@@ -3909,9 +3932,9 @@ int tpl2_flag_status(bpctl_dev_t *pbpctl_dev)
 
 int tpl_hw_status(bpctl_dev_t *pbpctl_dev)
 {
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return BP_NOT_CAP;
 
 	if (TPL_IF_SERIES(pbpctl_dev->subdevice))
@@ -4215,9 +4238,9 @@ void bypass_caps_init(bpctl_dev_t *pbpctl_dev)
 
 int bypass_off_init(bpctl_dev_t *pbpctl_dev)
 {
-	int ret = 0;
+	int ret = cmnd_on(pbpctl_dev);
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	if (ret < 0)
 		return ret;
 	if (INTEL_IF_SERIES(pbpctl_dev->subdevice))
 		return dis_bypass_cap(pbpctl_dev);
@@ -4403,7 +4426,10 @@ int set_bypass_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
 
 	if (!(pbpctl_dev->bp_caps & BP_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+
+	ret = cmnd_on(pbpctl_dev);
+
+	if (ret < 0)
 		return ret;
 	if (!bypass_mode)
 		ret = bypass_off(pbpctl_dev);
@@ -4435,7 +4461,10 @@ int set_dis_bypass_fn(bpctl_dev_t *pbpctl_dev, int dis_param)
 
 	if (!(pbpctl_dev->bp_caps & BP_DIS_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+
+	ret = cmnd_on(pbpctl_dev);
+
+	if (ret < 0)
 		return ret;
 	if (dis_param)
 		ret = dis_bypass_cap(pbpctl_dev);
@@ -4461,7 +4490,10 @@ int set_bypass_pwoff_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
 
 	if (!(pbpctl_dev->bp_caps & BP_PWOFF_CTL_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+
+	ret = cmnd_on(pbpctl_dev);
+
+	if (ret < 0)
 		return ret;
 	if (bypass_mode)
 		ret = bypass_state_pwroff(pbpctl_dev);
@@ -4487,7 +4519,10 @@ int set_bypass_pwup_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
 
 	if (!(pbpctl_dev->bp_caps & BP_PWUP_CTL_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+
+	ret = cmnd_on(pbpctl_dev);
+
+	if (ret < 0)
 		return ret;
 	if (bypass_mode)
 		ret = bypass_state_pwron(pbpctl_dev);
@@ -4514,7 +4549,9 @@ int set_bypass_wd_fn(bpctl_dev_t *pbpctl_dev, int timeout)
 	if (!(pbpctl_dev->bp_caps & WD_CTL_CAP))
 		return BP_NOT_CAP;
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+
+	if (ret < 0)
 		return ret;
 	if (!timeout)
 		ret = wdt_off(pbpctl_dev);
@@ -4583,7 +4620,9 @@ int set_std_nic_fn(bpctl_dev_t *pbpctl_dev, int nic_mode)
 	if (!(pbpctl_dev->bp_caps & STD_NIC_CAP))
 		return BP_NOT_CAP;
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+
+	if (ret < 0)
 		return ret;
 	if (nic_mode)
 		ret = std_nic_on(pbpctl_dev);
@@ -4649,7 +4688,9 @@ int get_tap_pwup_fn(bpctl_dev_t *pbpctl_dev)
 	if (!pbpctl_dev)
 		return -1;
 
-	if ((ret = default_pwron_tap_status(pbpctl_dev)) < 0)
+	ret = default_pwron_tap_status(pbpctl_dev);
+
+	if (ret < 0)
 		return ret;
 	return ((ret == 0) ? 1 : 0);
 }
@@ -4824,7 +4865,9 @@ int get_disc_port_pwup_fn(bpctl_dev_t *pbpctl_dev)
 	if (!pbpctl_dev)
 		return -1;
 
-	if ((ret = default_pwron_disc_port_status(pbpctl_dev)) < 0)
+	ret = default_pwron_disc_port_status(pbpctl_dev);
+
+	if (ret < 0)
 		return ret;
 	return ((ret == 0) ? 1 : 0);
 }
@@ -4851,7 +4894,9 @@ int reset_cont_fn(bpctl_dev_t *pbpctl_dev)
 	if (!pbpctl_dev)
 		return -1;
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+
+	if (ret < 0)
 		return ret;
 	return reset_cont(pbpctl_dev);
 }
@@ -4867,8 +4912,11 @@ int set_tx_fn(bpctl_dev_t *pbpctl_dev, int tx_state)
 	    (pbpctl_dev->bp_caps & SW_CTL_CAP)) {
 		if ((pbpctl_dev->bp_tpl_flag))
 			return BP_NOT_CAP;
-	} else if ((pbpctl_dev_b = get_master_port_fn(pbpctl_dev))) {
-		if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
+	} else {
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+
+		if ((pbpctl_dev_b) &&
+		    (pbpctl_dev_b->bp_caps & TPL_CAP) &&
 		    (pbpctl_dev_b->bp_tpl_flag))
 			return BP_NOT_CAP;
 	}
@@ -4984,8 +5032,11 @@ int get_tx_fn(bpctl_dev_t *pbpctl_dev)
 	    (pbpctl_dev->bp_caps & SW_CTL_CAP)) {
 		if ((pbpctl_dev->bp_tpl_flag))
 			return BP_NOT_CAP;
-	} else if ((pbpctl_dev_b = get_master_port_fn(pbpctl_dev))) {
-		if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
+	} else{
+		pbpctl_dev_b = get_master_port_fn(pbpctl_dev);
+
+		if ((pbpctl_dev_b) &&
+		    (pbpctl_dev_b->bp_caps & TPL_CAP) &&
 		    (pbpctl_dev_b->bp_tpl_flag))
 			return BP_NOT_CAP;
 	}
@@ -5023,8 +5074,9 @@ static void bp_tpl_timer_fn(unsigned long param)
 	bpctl_dev_t *pbpctl_dev = (bpctl_dev_t *) param;
 	uint32_t link1, link2;
 	bpctl_dev_t *pbpctl_dev_b = NULL;
+	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return;
 
 	if (!pbpctl_dev->bp_tpl_flag) {
@@ -5128,7 +5180,9 @@ int set_tpl_fn(bpctl_dev_t *pbpctl_dev, int tpl_mode)
 
 	if (pbpctl_dev->bp_caps & TPL_CAP) {
 		if (tpl_mode) {
-			if ((pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+			pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+
+			if (pbpctl_dev_b)
 				set_tx(pbpctl_dev_b, 1);
 			set_tx(pbpctl_dev, 1);
 		}
@@ -6708,7 +6762,9 @@ static int init_one(bpctl_dev_t *dev, bpmod_info_t *info, struct pci_dev *pdev1)
 			reset_cont(dev);
 	}
 #ifdef BP_SELF_TEST
-	if ((dev->bp_tx_data = kzalloc(BPTEST_DATA_LEN, GFP_KERNEL))) {
+	dev->bp_tx_data = kzalloc(BPTEST_DATA_LEN, GFP_KERNEL);
+
+	if (dev->bp_tx_data) {
 		memset(dev->bp_tx_data, 0xff, 6);
 		memset(dev->bp_tx_data + 6, 0x0, 1);
 		memset(dev->bp_tx_data + 7, 0xaa, 5);
-- 
1.7.10.4

[PATCH 4/4 v2] silicom: checkpatch: errors caused by macros

[Lorenz Haspel]

fixed checkpatch error:
added parenthesis around complex macro.

Macro with return was only used once in the code,
so I expandet it in-place.

Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
---
v2: expanded macro in-place
---
 drivers/staging/silicom/bpctl_mod.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
index 5511f5e..d2b8004 100644
--- a/drivers/staging/silicom/bpctl_mod.c
+++ b/drivers/staging/silicom/bpctl_mod.c
@@ -43,11 +43,6 @@ MODULE_DESCRIPTION(BP_MOD_DESCR);
 MODULE_VERSION(BP_MOD_VER);
 spinlock_t bpvm_lock;
 
-#define lock_bpctl()					\
-if (down_interruptible(&bpctl_sema)) {			\
-	return -ERESTARTSYS;				\
-}							\
-
 #define unlock_bpctl()					\
 	up(&bpctl_sema);
 
@@ -5452,7 +5447,8 @@ static long device_ioctl(struct file *file,	/* see include/linux/fs.h */
 	static bpctl_dev_t *pbpctl_dev;
 
 	/* lock_kernel(); */
-	lock_bpctl();
+	if (down_interruptible(&bpctl_sema))
+		return -ERESTARTSYS;
 	/* local_irq_save(flags); */
 	/* if(!spin_trylock_irqsave(&bpvm_lock)){
 	   local_irq_restore(flags);
@@ -7911,7 +7907,8 @@ int bypass_proc_create_dev_sd(bpctl_dev_t *pbp_device_block)
 	}
 	current_pfs->bypass_entry = procfs_dir;
 
-#define ENTRY(x) ret |= procfs_add(#x, &x##_ops, pbp_device_block)
+#define ENTRY(x) (ret |= procfs_add(#x, &x##_ops, pbp_device_block))
+
 	ENTRY(bypass_info);
 	if (pbp_device_block->bp_caps & SW_CTL_CAP) {
 		/* Create set param proc's */
-- 
1.7.10.4

Re: [PATCH 4/4 v2] silicom: checkpatch: errors caused by macros

[Joe Perches]

On Mon, 2013-06-17 at 21:20 +0200, Lorenz Haspel wrote:
> fixed checkpatch error:
> added parenthesis around complex macro.
> 
> Macro with return was only used once in the code,
> so I expandet it in-place.
[]
> diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
[]
> -#define lock_bpctl()					\
> -if (down_interruptible(&bpctl_sema)) {			\
> -	return -ERESTARTSYS;				\
> -}							\
> -
>  #define unlock_bpctl()					\
>  	up(&bpctl_sema);

Symmetry please.

Most likely, this unlock_bpctl macro is only used once too.
I suggest removing it as well.

Re: [PATCH 4/4 v2] silicom: checkpatch: errors caused by macros

[Dan Carpenter]

On Mon, Jun 17, 2013 at 12:42:12PM -0700, Joe Perches wrote:
> On Mon, 2013-06-17 at 21:20 +0200, Lorenz Haspel wrote:
> > fixed checkpatch error:
> > added parenthesis around complex macro.
> > 
> > Macro with return was only used once in the code,
> > so I expandet it in-place.
> []
> > diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
> []
> > -#define lock_bpctl()					\
> > -if (down_interruptible(&bpctl_sema)) {			\
> > -	return -ERESTARTSYS;				\
> > -}							\
> > -
> >  #define unlock_bpctl()					\
> >  	up(&bpctl_sema);
> 
> Symmetry please.
> 
> Most likely, this unlock_bpctl macro is only used once too.
> I suggest removing it as well.
> 

Joe is right, of course, but this could be fixed in a later patch.

regards,
dan carpenter

Re: [PATCH 4/4 v2] silicom: checkpatch: errors caused by macros

[Joe Perches]

On Mon, 2013-06-17 at 23:49 +0300, Dan Carpenter wrote:
> On Mon, Jun 17, 2013 at 12:42:12PM -0700, Joe Perches wrote:
> > On Mon, 2013-06-17 at 21:20 +0200, Lorenz Haspel wrote:
> > > fixed checkpatch error:
> > > added parenthesis around complex macro.
> > > 
> > > Macro with return was only used once in the code,
> > > so I expandet it in-place.
> > []
> > > diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
> > []
> > > -#define lock_bpctl()					\
> > > -if (down_interruptible(&bpctl_sema)) {			\
> > > -	return -ERESTARTSYS;				\
> > > -}							\
> > > -
> > >  #define unlock_bpctl()					\
> > >  	up(&bpctl_sema);
> > 
> > Symmetry please.
> > 
> > Most likely, this unlock_bpctl macro is only used once too.
> > I suggest removing it as well.
> > 
> 
> Joe is right, of course, but this could be fixed in a later patch.

Generally I think it's better that new submitters patches
should go through more strict reviews and be as correct
as possible.  I think this is especially true for patches
that are just checkpatch driven.

Re: [PATCH 4/4 v2] silicom: checkpatch: errors caused by macros

[Greg KH]

On Mon, Jun 17, 2013 at 02:03:43PM -0700, Joe Perches wrote:
> On Mon, 2013-06-17 at 23:49 +0300, Dan Carpenter wrote:
> > On Mon, Jun 17, 2013 at 12:42:12PM -0700, Joe Perches wrote:
> > > On Mon, 2013-06-17 at 21:20 +0200, Lorenz Haspel wrote:
> > > > fixed checkpatch error:
> > > > added parenthesis around complex macro.
> > > > 
> > > > Macro with return was only used once in the code,
> > > > so I expandet it in-place.
> > > []
> > > > diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
> > > []
> > > > -#define lock_bpctl()					\
> > > > -if (down_interruptible(&bpctl_sema)) {			\
> > > > -	return -ERESTARTSYS;				\
> > > > -}							\
> > > > -
> > > >  #define unlock_bpctl()					\
> > > >  	up(&bpctl_sema);
> > > 
> > > Symmetry please.
> > > 
> > > Most likely, this unlock_bpctl macro is only used once too.
> > > I suggest removing it as well.
> > > 
> > 
> > Joe is right, of course, but this could be fixed in a later patch.
> 
> Generally I think it's better that new submitters patches
> should go through more strict reviews and be as correct
> as possible.  I think this is especially true for patches
> that are just checkpatch driven.

I totally disagree, sorry.

Re: [PATCH 1/4 v2] silicom: checkpatch: assignments in if conditions

[Dan Carpenter]

On Mon, Jun 17, 2013 at 09:15:39PM +0200, Lorenz Haspel wrote:
> Fixes checkpatch error:
> There were assignments in if conditions, so I extracted them.
> 
> Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
> Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
> ---
> v2: removed some buggy extra lines and fixed white space issues

Gar....  This isn't right either.  Now it has *too many* blank
lines.  It's only between declarations and code that I was
complaining about.  You've added them between assignments and error
checks.


> @@ -1224,7 +1237,9 @@ static int wdt_pulse(bpctl_dev_t *pbpctl_dev)
>  		return -1;
>  #endif
>  	if (pbpctl_dev->bp_10g9) {
> -		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
> +		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
> +

This blank line is harmful.

> +		if (!pbpctl_dev_c)
>  			return -1;
>  	}
>  
> @@ -1742,9 +1757,9 @@ static void write_data_port_int(bpctl_dev_t *pbpctl_dev,
>  
>  static int write_data_int(bpctl_dev_t *pbpctl_dev, unsigned char value)
>  {
> -	bpctl_dev_t *pbpctl_dev_b = NULL;
> +	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
>  

This blank line is required.

So what you have here is fine, but if you wanted you could re-write
this like:
{
	bpctl_dev_t *pbpctl_dev_b;

	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
	if (!pbpctl_dev_b)
		return -1;

Generally, you shouldn't put anything complicated in the initializer
statement.  People don't read that code as thouroughly and
initializers are sometimes a source of bugs.  But what you have here
is also perfectly acceptable.

> -	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
> +	if (!pbpctl_dev_b)
>  		return -1;
>  	atomic_set(&pbpctl_dev->wdt_busy, 1);
>  	write_data_port_int(pbpctl_dev, value & 0x3);

rergards,
dan carpenter

Re: [PATCH 4/4 v2] silicom: checkpatch: errors caused by macros

[Joe Perches]

On Mon, 2013-06-17 at 14:14 -0700, Greg KH wrote:
> On Mon, Jun 17, 2013 at 02:03:43PM -0700, Joe Perches wrote:
> > Generally I think it's better that new submitters patches
> > should go through more strict reviews and be as correct
> > as possible.  I think this is especially true for patches
> > that are just checkpatch driven.
> 
> I totally disagree, sorry.

I'm unsurprised.  We have different tastes.

While whitespace only cleanup patches have some use,
for these types of patches, I'm more interested in
educating others what sorts of patches have higher
value.

o defects
o style/readability
o whitespace

As always, ymmv.

[PATCH 1/4 v3] silicom: checkpatch: assignments in if conditions

[Lorenz Haspel]

Fixes checkpatch error:
There were assignments in if conditions, so I extracted them.

Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
---
v2: removed some buggy extra lines and fixed white space issues
v3: fixed some more extra lines
---
 drivers/staging/silicom/bpctl_mod.c |  179 +++++++++++++++++++----------------
 1 file changed, 98 insertions(+), 81 deletions(-)

diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
index a3b974b..88ece4a 100644
--- a/drivers/staging/silicom/bpctl_mod.c
+++ b/drivers/staging/silicom/bpctl_mod.c
@@ -245,9 +245,11 @@ static int bp_device_event(struct notifier_block *unused,
 	case NETDEV_CHANGE:{
 			if (netif_carrier_ok(dev))
 				return NOTIFY_DONE;
-
-			if (((dev_num = get_dev_idx(dev->ifindex)) == -1) ||
-			    (!(pbpctl_dev = &bpctl_dev_arr[dev_num])))
+			dev_num = get_dev_idx(dev->ifindex);
+			if (dev_num == -1)
+				return NOTIFY_DONE;
+			pbpctl_dev = &bpctl_dev_arr[dev_num];
+			if (!pbpctl_dev)
 				return NOTIFY_DONE;
 
 			if ((is_bypass_fn(pbpctl_dev)) == 1)
@@ -306,7 +308,8 @@ static void write_pulse(bpctl_dev_t *pbpctl_dev,
 		ctrl = BP10G_READ_REG(pbpctl_dev, ESDP);
 
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return;
 		ctrl = BP10G_READ_REG(pbpctl_dev_c, ESDP);
 	}
@@ -606,7 +609,8 @@ static int read_pulse(bpctl_dev_t *pbpctl_dev, unsigned int ctrl_ext,
 	if (pbpctl_dev->bp_540)
 		ctrl = BP10G_READ_REG(pbpctl_dev, ESDP);
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return -1;
 		ctrl = BP10G_READ_REG(pbpctl_dev_c, ESDP);
 	}
@@ -775,7 +779,8 @@ static void write_reg(bpctl_dev_t *pbpctl_dev, unsigned char value,
 	bpctl_dev_t *pbpctl_dev_c = NULL;
 	unsigned long flags;
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return;
 	}
 	if ((pbpctl_dev->wdt_status == WDT_STATUS_EN) &&
@@ -953,7 +958,8 @@ static int read_reg(bpctl_dev_t *pbpctl_dev, unsigned char addr)
 	atomic_set(&pbpctl_dev->wdt_busy, 1);
 #endif
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return -1;
 	}
 
@@ -1224,7 +1230,8 @@ static int wdt_pulse(bpctl_dev_t *pbpctl_dev)
 		return -1;
 #endif
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return -1;
 	}
 
@@ -1742,9 +1749,9 @@ static void write_data_port_int(bpctl_dev_t *pbpctl_dev,
 
 static int write_data_int(bpctl_dev_t *pbpctl_dev, unsigned char value)
 {
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return -1;
 	atomic_set(&pbpctl_dev->wdt_busy, 1);
 	write_data_port_int(pbpctl_dev, value & 0x3);
@@ -1963,9 +1970,9 @@ int disc_port_off(bpctl_dev_t *pbpctl_dev)
 int tpl_hw_on(bpctl_dev_t *pbpctl_dev)
 {
 	int ret = 0, ctrl = 0;
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return BP_NOT_CAP;
 
 	if (pbpctl_dev->bp_caps_ex & TPL2_CAP_EX) {
@@ -1990,9 +1997,9 @@ int tpl_hw_on(bpctl_dev_t *pbpctl_dev)
 int tpl_hw_off(bpctl_dev_t *pbpctl_dev)
 {
 	int ret = 0, ctrl = 0;
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return BP_NOT_CAP;
 	if (pbpctl_dev->bp_caps_ex & TPL2_CAP_EX) {
 		cmnd_on(pbpctl_dev);
@@ -2373,28 +2380,24 @@ static int set_tx(bpctl_dev_t *pbpctl_dev, int tx_state)
 		if (PEG5_IF_SERIES(pbpctl_dev->subdevice)) {
 			if (tx_state) {
 				uint16_t mii_reg;
-				if (!
-				    (ret =
-				     bp75_read_phy_reg(pbpctl_dev,
+				ret = bp75_read_phy_reg(pbpctl_dev,
 						       BPCTLI_PHY_CONTROL,
-						       &mii_reg))) {
-					if (mii_reg & BPCTLI_MII_CR_POWER_DOWN) {
-						ret =
-						    bp75_write_phy_reg
-						    (pbpctl_dev,
-						     BPCTLI_PHY_CONTROL,
-						     mii_reg &
-						     ~BPCTLI_MII_CR_POWER_DOWN);
-					}
+						       &mii_reg);
+				if ((!ret) &&
+				    (mii_reg & BPCTLI_MII_CR_POWER_DOWN)) {
+					ret =
+					    bp75_write_phy_reg
+					    (pbpctl_dev,
+					     BPCTLI_PHY_CONTROL,
+					     mii_reg &
+					     ~BPCTLI_MII_CR_POWER_DOWN);
 				}
 			} else {
 				uint16_t mii_reg;
-				if (!
-				    (ret =
-				     bp75_read_phy_reg(pbpctl_dev,
+				ret = bp75_read_phy_reg(pbpctl_dev,
 						       BPCTLI_PHY_CONTROL,
-						       &mii_reg))) {
-
+						       &mii_reg);
+				if (!ret) {
 					mii_reg |= BPCTLI_MII_CR_POWER_DOWN;
 					ret =
 					    bp75_write_phy_reg(pbpctl_dev,
@@ -3237,30 +3240,33 @@ int bypass_from_last_read(bpctl_dev_t *pbpctl_dev)
 	uint32_t ctrl_ext = 0;
 	bpctl_dev_t *pbpctl_dev_b = NULL;
 
-	if ((pbpctl_dev->bp_caps & SW_CTL_CAP)
-	    && (pbpctl_dev_b = get_status_port_fn(pbpctl_dev))) {
-		ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
-		BPCTL_BP_WRITE_REG(pbpctl_dev_b, CTRL_EXT,
+	if (pbpctl_dev->bp_caps & SW_CTL_CAP) {
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (pbpctl_dev_b) {
+			ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
+			BPCTL_BP_WRITE_REG(pbpctl_dev_b, CTRL_EXT,
 				   (ctrl_ext & ~BPCTLI_CTRL_EXT_SDP7_DIR));
-		ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
-		if (ctrl_ext & BPCTLI_CTRL_EXT_SDP7_DATA)
-			return 0;
-		return 1;
-	} else
-		return BP_NOT_CAP;
+			ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
+			if (ctrl_ext & BPCTLI_CTRL_EXT_SDP7_DATA)
+				return 0;
+			return 1;
+		}
+	}
+	return BP_NOT_CAP;
 }
 
 int bypass_status_clear(bpctl_dev_t *pbpctl_dev)
 {
 	bpctl_dev_t *pbpctl_dev_b = NULL;
 
-	if ((pbpctl_dev->bp_caps & SW_CTL_CAP)
-	    && (pbpctl_dev_b = get_status_port_fn(pbpctl_dev))) {
-
-		send_bypass_clear_pulse(pbpctl_dev_b, 1);
-		return 0;
-	} else
-		return BP_NOT_CAP;
+	if (pbpctl_dev->bp_caps & SW_CTL_CAP) {
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (pbpctl_dev_b) {
+			send_bypass_clear_pulse(pbpctl_dev_b, 1);
+			return 0;
+		}
+	}
+	return BP_NOT_CAP;
 }
 
 int bypass_flag_status(bpctl_dev_t *pbpctl_dev)
@@ -3326,10 +3332,8 @@ static int bypass_status(bpctl_dev_t *pbpctl_dev)
 {
 	u32 ctrl_ext = 0;
 	if (pbpctl_dev->bp_caps & BP_CAP) {
-
-		bpctl_dev_t *pbpctl_dev_b = NULL;
-
-		if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+		bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_b)
 			return BP_NOT_CAP;
 
 		if (INTEL_IF_SERIES(pbpctl_dev->subdevice)) {
@@ -3615,9 +3619,8 @@ int tap_status(bpctl_dev_t *pbpctl_dev)
 	u32 ctrl_ext = 0;
 
 	if (pbpctl_dev->bp_caps & TAP_CAP) {
-		bpctl_dev_t *pbpctl_dev_b = NULL;
-
-		if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+		bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_b)
 			return BP_NOT_CAP;
 
 		if (pbpctl_dev->bp_ext_ver >= 0x8) {
@@ -3713,7 +3716,8 @@ int disc_off_status(bpctl_dev_t *pbpctl_dev)
 	u32 ctrl_ext = 0;
 
 	if (pbpctl_dev->bp_caps & DISC_CAP) {
-		if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_b)
 			return BP_NOT_CAP;
 		if (DISCF_IF_SERIES(pbpctl_dev->subdevice))
 			return ((((read_reg(pbpctl_dev, STATUS_DISC_REG_ADDR)) &
@@ -3794,8 +3798,8 @@ static int disc_status(bpctl_dev_t *pbpctl_dev)
 {
 	int ctrl = 0;
 	if (pbpctl_dev->bp_caps & DISC_CAP) {
-
-		if ((ctrl = disc_off_status(pbpctl_dev)) < 0)
+		ctrl = disc_off_status(pbpctl_dev);
+		if (ctrl < 0)
 			return ctrl;
 		return ((ctrl == 0) ? 1 : 0);
 
@@ -3909,9 +3913,9 @@ int tpl2_flag_status(bpctl_dev_t *pbpctl_dev)
 
 int tpl_hw_status(bpctl_dev_t *pbpctl_dev)
 {
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return BP_NOT_CAP;
 
 	if (TPL_IF_SERIES(pbpctl_dev->subdevice))
@@ -4215,9 +4219,9 @@ void bypass_caps_init(bpctl_dev_t *pbpctl_dev)
 
 int bypass_off_init(bpctl_dev_t *pbpctl_dev)
 {
-	int ret = 0;
+	int ret = cmnd_on(pbpctl_dev);
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	if (ret < 0)
 		return ret;
 	if (INTEL_IF_SERIES(pbpctl_dev->subdevice))
 		return dis_bypass_cap(pbpctl_dev);
@@ -4403,7 +4407,8 @@ int set_bypass_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
 
 	if (!(pbpctl_dev->bp_caps & BP_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (!bypass_mode)
 		ret = bypass_off(pbpctl_dev);
@@ -4435,7 +4440,8 @@ int set_dis_bypass_fn(bpctl_dev_t *pbpctl_dev, int dis_param)
 
 	if (!(pbpctl_dev->bp_caps & BP_DIS_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (dis_param)
 		ret = dis_bypass_cap(pbpctl_dev);
@@ -4461,7 +4467,8 @@ int set_bypass_pwoff_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
 
 	if (!(pbpctl_dev->bp_caps & BP_PWOFF_CTL_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (bypass_mode)
 		ret = bypass_state_pwroff(pbpctl_dev);
@@ -4487,7 +4494,8 @@ int set_bypass_pwup_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
 
 	if (!(pbpctl_dev->bp_caps & BP_PWUP_CTL_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (bypass_mode)
 		ret = bypass_state_pwron(pbpctl_dev);
@@ -4514,7 +4522,8 @@ int set_bypass_wd_fn(bpctl_dev_t *pbpctl_dev, int timeout)
 	if (!(pbpctl_dev->bp_caps & WD_CTL_CAP))
 		return BP_NOT_CAP;
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (!timeout)
 		ret = wdt_off(pbpctl_dev);
@@ -4583,7 +4592,8 @@ int set_std_nic_fn(bpctl_dev_t *pbpctl_dev, int nic_mode)
 	if (!(pbpctl_dev->bp_caps & STD_NIC_CAP))
 		return BP_NOT_CAP;
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (nic_mode)
 		ret = std_nic_on(pbpctl_dev);
@@ -4648,8 +4658,8 @@ int get_tap_pwup_fn(bpctl_dev_t *pbpctl_dev)
 	int ret = 0;
 	if (!pbpctl_dev)
 		return -1;
-
-	if ((ret = default_pwron_tap_status(pbpctl_dev)) < 0)
+	ret = default_pwron_tap_status(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	return ((ret == 0) ? 1 : 0);
 }
@@ -4823,8 +4833,8 @@ int get_disc_port_pwup_fn(bpctl_dev_t *pbpctl_dev)
 	int ret = 0;
 	if (!pbpctl_dev)
 		return -1;
-
-	if ((ret = default_pwron_disc_port_status(pbpctl_dev)) < 0)
+	ret = default_pwron_disc_port_status(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	return ((ret == 0) ? 1 : 0);
 }
@@ -4851,7 +4861,8 @@ int reset_cont_fn(bpctl_dev_t *pbpctl_dev)
 	if (!pbpctl_dev)
 		return -1;
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	return reset_cont(pbpctl_dev);
 }
@@ -4867,8 +4878,10 @@ int set_tx_fn(bpctl_dev_t *pbpctl_dev, int tx_state)
 	    (pbpctl_dev->bp_caps & SW_CTL_CAP)) {
 		if ((pbpctl_dev->bp_tpl_flag))
 			return BP_NOT_CAP;
-	} else if ((pbpctl_dev_b = get_master_port_fn(pbpctl_dev))) {
-		if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
+	} else {
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if ((pbpctl_dev_b) &&
+		    (pbpctl_dev_b->bp_caps & TPL_CAP) &&
 		    (pbpctl_dev_b->bp_tpl_flag))
 			return BP_NOT_CAP;
 	}
@@ -4984,8 +4997,10 @@ int get_tx_fn(bpctl_dev_t *pbpctl_dev)
 	    (pbpctl_dev->bp_caps & SW_CTL_CAP)) {
 		if ((pbpctl_dev->bp_tpl_flag))
 			return BP_NOT_CAP;
-	} else if ((pbpctl_dev_b = get_master_port_fn(pbpctl_dev))) {
-		if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
+	} else{
+		pbpctl_dev_b = get_master_port_fn(pbpctl_dev);
+		if ((pbpctl_dev_b) &&
+		    (pbpctl_dev_b->bp_caps & TPL_CAP) &&
 		    (pbpctl_dev_b->bp_tpl_flag))
 			return BP_NOT_CAP;
 	}
@@ -5022,9 +5037,9 @@ static void bp_tpl_timer_fn(unsigned long param)
 {
 	bpctl_dev_t *pbpctl_dev = (bpctl_dev_t *) param;
 	uint32_t link1, link2;
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return;
 
 	if (!pbpctl_dev->bp_tpl_flag) {
@@ -5128,7 +5143,8 @@ int set_tpl_fn(bpctl_dev_t *pbpctl_dev, int tpl_mode)
 
 	if (pbpctl_dev->bp_caps & TPL_CAP) {
 		if (tpl_mode) {
-			if ((pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+			pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+			if (pbpctl_dev_b)
 				set_tx(pbpctl_dev_b, 1);
 			set_tx(pbpctl_dev, 1);
 		}
@@ -6708,7 +6724,8 @@ static int init_one(bpctl_dev_t *dev, bpmod_info_t *info, struct pci_dev *pdev1)
 			reset_cont(dev);
 	}
 #ifdef BP_SELF_TEST
-	if ((dev->bp_tx_data = kzalloc(BPTEST_DATA_LEN, GFP_KERNEL))) {
+	dev->bp_tx_data = kzalloc(BPTEST_DATA_LEN, GFP_KERNEL);
+	if (dev->bp_tx_data) {
 		memset(dev->bp_tx_data, 0xff, 6);
 		memset(dev->bp_tx_data + 6, 0x0, 1);
 		memset(dev->bp_tx_data + 7, 0xaa, 5);
-- 
1.7.10.4

Re: [PATCH 1/4 v3] silicom: checkpatch: assignments in if conditions

[Dan Carpenter]

On Tue, Jun 18, 2013 at 06:28:38PM +0200, Lorenz Haspel wrote:
> Fixes checkpatch error:
> There were assignments in if conditions, so I extracted them.
> 
> Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
> Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>

Looks good, thanks.

regards,
dan carpenter

Re: [PATCH 1/4 v3] silicom: checkpatch: assignments in if conditions

[Greg KH]

On Tue, Jun 18, 2013 at 06:28:38PM +0200, Lorenz Haspel wrote:
> Fixes checkpatch error:
> There were assignments in if conditions, so I extracted them.
> 
> Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
> Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
> ---
> v2: removed some buggy extra lines and fixed white space issues
> v3: fixed some more extra lines
> ---
>  drivers/staging/silicom/bpctl_mod.c |  179 +++++++++++++++++++----------------
>  1 file changed, 98 insertions(+), 81 deletions(-)

I'm getting confused.  I've applied some of the patches in this series,
so are you going to resend all 4 patches?

How about just resending what I haven't applied yet, as a whole new
series, that would help out on my end a lot.

thanks,

greg k-h

[PATCH 1/2 v2] silicom: checkpatch: errors caused by macros

[Lorenz Haspel]

fixed checkpatch error:
added parenthesis around complex macro.

Macro with return was only used once in the code,
so I expandet it in-place.

Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
---
v2: expanded macro in-place
---
 drivers/staging/silicom/bpctl_mod.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
index a0ea241..75672af 100644
--- a/drivers/staging/silicom/bpctl_mod.c
+++ b/drivers/staging/silicom/bpctl_mod.c
@@ -43,11 +43,6 @@ MODULE_DESCRIPTION(BP_MOD_DESCR);
 MODULE_VERSION(BP_MOD_VER);
 spinlock_t bpvm_lock;
 
-#define lock_bpctl()					\
-if (down_interruptible(&bpctl_sema)) {			\
-	return -ERESTARTSYS;				\
-}							\
-
 #define unlock_bpctl()					\
 	up(&bpctl_sema);
 
@@ -5398,7 +5393,8 @@ static long device_ioctl(struct file *file,	/* see include/linux/fs.h */
 	static bpctl_dev_t *pbpctl_dev;
 
 	/* lock_kernel(); */
-	lock_bpctl();
+	if (down_interruptible(&bpctl_sema))
+		return -ERESTARTSYS;
 	/* local_irq_save(flags); */
 	/* if(!spin_trylock_irqsave(&bpvm_lock)){
 	   local_irq_restore(flags);
@@ -7855,7 +7851,8 @@ int bypass_proc_create_dev_sd(bpctl_dev_t *pbp_device_block)
 	}
 	current_pfs->bypass_entry = procfs_dir;
 
-#define ENTRY(x) ret |= procfs_add(#x, &x##_ops, pbp_device_block)
+#define ENTRY(x) (ret |= procfs_add(#x, &x##_ops, pbp_device_block))
+
 	ENTRY(bypass_info);
 	if (pbp_device_block->bp_caps & SW_CTL_CAP) {
 		/* Create set param proc's */
-- 
1.7.10.4

[PATCH 2/2 v3] silicom: checkpatch: assignments in if conditions

[Lorenz Haspel]

Fixes checkpatch error:
There were assignments in if conditions, so I extracted them.

Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
---
v2: removed some buggy extra lines and fixed white space issues
v3: fixed some more extra lines
---
 drivers/staging/silicom/bpctl_mod.c |  179 +++++++++++++++++++----------------
 1 file changed, 98 insertions(+), 81 deletions(-)

diff --git a/drivers/staging/silicom/bpctl_mod.c b/drivers/staging/silicom/bpctl_mod.c
index 75672af..88e8bcb 100644
--- a/drivers/staging/silicom/bpctl_mod.c
+++ b/drivers/staging/silicom/bpctl_mod.c
@@ -241,9 +241,11 @@ static int bp_device_event(struct notifier_block *unused,
 	case NETDEV_CHANGE:{
 			if (netif_carrier_ok(dev))
 				return NOTIFY_DONE;
-
-			if (((dev_num = get_dev_idx(dev->ifindex)) == -1) ||
-			    (!(pbpctl_dev = &bpctl_dev_arr[dev_num])))
+			dev_num = get_dev_idx(dev->ifindex);
+			if (dev_num == -1)
+				return NOTIFY_DONE;
+			pbpctl_dev = &bpctl_dev_arr[dev_num];
+			if (!pbpctl_dev)
 				return NOTIFY_DONE;
 
 			if ((is_bypass_fn(pbpctl_dev)) == 1)
@@ -302,7 +304,8 @@ static void write_pulse(bpctl_dev_t *pbpctl_dev,
 		ctrl = BP10G_READ_REG(pbpctl_dev, ESDP);
 
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return;
 		ctrl = BP10G_READ_REG(pbpctl_dev_c, ESDP);
 	}
@@ -602,7 +605,8 @@ static int read_pulse(bpctl_dev_t *pbpctl_dev, unsigned int ctrl_ext,
 	if (pbpctl_dev->bp_540)
 		ctrl = BP10G_READ_REG(pbpctl_dev, ESDP);
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return -1;
 		ctrl = BP10G_READ_REG(pbpctl_dev_c, ESDP);
 	}
@@ -771,7 +775,8 @@ static void write_reg(bpctl_dev_t *pbpctl_dev, unsigned char value,
 	bpctl_dev_t *pbpctl_dev_c = NULL;
 	unsigned long flags;
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return;
 	}
 	if ((pbpctl_dev->wdt_status == WDT_STATUS_EN) &&
@@ -949,7 +954,8 @@ static int read_reg(bpctl_dev_t *pbpctl_dev, unsigned char addr)
 	atomic_set(&pbpctl_dev->wdt_busy, 1);
 #endif
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return -1;
 	}
 
@@ -1220,7 +1226,8 @@ static int wdt_pulse(bpctl_dev_t *pbpctl_dev)
 		return -1;
 #endif
 	if (pbpctl_dev->bp_10g9) {
-		if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_c)
 			return -1;
 	}
 
@@ -1738,9 +1745,9 @@ static void write_data_port_int(bpctl_dev_t *pbpctl_dev,
 
 static int write_data_int(bpctl_dev_t *pbpctl_dev, unsigned char value)
 {
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return -1;
 	atomic_set(&pbpctl_dev->wdt_busy, 1);
 	write_data_port_int(pbpctl_dev, value & 0x3);
@@ -1959,9 +1966,9 @@ int disc_port_off(bpctl_dev_t *pbpctl_dev)
 int tpl_hw_on(bpctl_dev_t *pbpctl_dev)
 {
 	int ret = 0, ctrl = 0;
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return BP_NOT_CAP;
 
 	if (pbpctl_dev->bp_caps_ex & TPL2_CAP_EX) {
@@ -1986,9 +1993,9 @@ int tpl_hw_on(bpctl_dev_t *pbpctl_dev)
 int tpl_hw_off(bpctl_dev_t *pbpctl_dev)
 {
 	int ret = 0, ctrl = 0;
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return BP_NOT_CAP;
 	if (pbpctl_dev->bp_caps_ex & TPL2_CAP_EX) {
 		cmnd_on(pbpctl_dev);
@@ -2371,28 +2378,24 @@ static int set_tx(bpctl_dev_t *pbpctl_dev, int tx_state)
 		if (PEG5_IF_SERIES(pbpctl_dev->subdevice)) {
 			if (tx_state) {
 				uint16_t mii_reg;
-				if (!
-				    (ret =
-				     bp75_read_phy_reg(pbpctl_dev,
+				ret = bp75_read_phy_reg(pbpctl_dev,
 						       BPCTLI_PHY_CONTROL,
-						       &mii_reg))) {
-					if (mii_reg & BPCTLI_MII_CR_POWER_DOWN) {
-						ret =
-						    bp75_write_phy_reg
-						    (pbpctl_dev,
-						     BPCTLI_PHY_CONTROL,
-						     mii_reg &
-						     ~BPCTLI_MII_CR_POWER_DOWN);
-					}
+						       &mii_reg);
+				if ((!ret) &&
+				    (mii_reg & BPCTLI_MII_CR_POWER_DOWN)) {
+					ret =
+					    bp75_write_phy_reg
+					    (pbpctl_dev,
+					     BPCTLI_PHY_CONTROL,
+					     mii_reg &
+					     ~BPCTLI_MII_CR_POWER_DOWN);
 				}
 			} else {
 				uint16_t mii_reg;
-				if (!
-				    (ret =
-				     bp75_read_phy_reg(pbpctl_dev,
+				ret = bp75_read_phy_reg(pbpctl_dev,
 						       BPCTLI_PHY_CONTROL,
-						       &mii_reg))) {
-
+						       &mii_reg);
+				if (!ret) {
 					mii_reg |= BPCTLI_MII_CR_POWER_DOWN;
 					ret =
 					    bp75_write_phy_reg(pbpctl_dev,
@@ -3235,30 +3238,33 @@ int bypass_from_last_read(bpctl_dev_t *pbpctl_dev)
 	uint32_t ctrl_ext = 0;
 	bpctl_dev_t *pbpctl_dev_b = NULL;
 
-	if ((pbpctl_dev->bp_caps & SW_CTL_CAP)
-	    && (pbpctl_dev_b = get_status_port_fn(pbpctl_dev))) {
-		ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
-		BPCTL_BP_WRITE_REG(pbpctl_dev_b, CTRL_EXT,
+	if (pbpctl_dev->bp_caps & SW_CTL_CAP) {
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (pbpctl_dev_b) {
+			ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
+			BPCTL_BP_WRITE_REG(pbpctl_dev_b, CTRL_EXT,
 				   (ctrl_ext & ~BPCTLI_CTRL_EXT_SDP7_DIR));
-		ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
-		if (ctrl_ext & BPCTLI_CTRL_EXT_SDP7_DATA)
-			return 0;
-		return 1;
-	} else
-		return BP_NOT_CAP;
+			ctrl_ext = BPCTL_READ_REG(pbpctl_dev_b, CTRL_EXT);
+			if (ctrl_ext & BPCTLI_CTRL_EXT_SDP7_DATA)
+				return 0;
+			return 1;
+		}
+	}
+	return BP_NOT_CAP;
 }
 
 int bypass_status_clear(bpctl_dev_t *pbpctl_dev)
 {
 	bpctl_dev_t *pbpctl_dev_b = NULL;
 
-	if ((pbpctl_dev->bp_caps & SW_CTL_CAP)
-	    && (pbpctl_dev_b = get_status_port_fn(pbpctl_dev))) {
-
-		send_bypass_clear_pulse(pbpctl_dev_b, 1);
-		return 0;
-	} else
-		return BP_NOT_CAP;
+	if (pbpctl_dev->bp_caps & SW_CTL_CAP) {
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (pbpctl_dev_b) {
+			send_bypass_clear_pulse(pbpctl_dev_b, 1);
+			return 0;
+		}
+	}
+	return BP_NOT_CAP;
 }
 
 int bypass_flag_status(bpctl_dev_t *pbpctl_dev)
@@ -3324,10 +3330,8 @@ static int bypass_status(bpctl_dev_t *pbpctl_dev)
 {
 	u32 ctrl_ext = 0;
 	if (pbpctl_dev->bp_caps & BP_CAP) {
-
-		bpctl_dev_t *pbpctl_dev_b = NULL;
-
-		if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+		bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_b)
 			return BP_NOT_CAP;
 
 		if (INTEL_IF_SERIES(pbpctl_dev->subdevice)) {
@@ -3613,9 +3617,8 @@ int tap_status(bpctl_dev_t *pbpctl_dev)
 	u32 ctrl_ext = 0;
 
 	if (pbpctl_dev->bp_caps & TAP_CAP) {
-		bpctl_dev_t *pbpctl_dev_b = NULL;
-
-		if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+		bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_b)
 			return BP_NOT_CAP;
 
 		if (pbpctl_dev->bp_ext_ver >= 0x8) {
@@ -3711,7 +3714,8 @@ int disc_off_status(bpctl_dev_t *pbpctl_dev)
 	u32 ctrl_ext = 0;
 
 	if (pbpctl_dev->bp_caps & DISC_CAP) {
-		if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if (!pbpctl_dev_b)
 			return BP_NOT_CAP;
 		if (DISCF_IF_SERIES(pbpctl_dev->subdevice))
 			return ((((read_reg(pbpctl_dev, STATUS_DISC_REG_ADDR)) &
@@ -3792,8 +3796,8 @@ static int disc_status(bpctl_dev_t *pbpctl_dev)
 {
 	int ctrl = 0;
 	if (pbpctl_dev->bp_caps & DISC_CAP) {
-
-		if ((ctrl = disc_off_status(pbpctl_dev)) < 0)
+		ctrl = disc_off_status(pbpctl_dev);
+		if (ctrl < 0)
 			return ctrl;
 		return ((ctrl == 0) ? 1 : 0);
 
@@ -3907,9 +3911,9 @@ int tpl2_flag_status(bpctl_dev_t *pbpctl_dev)
 
 int tpl_hw_status(bpctl_dev_t *pbpctl_dev)
 {
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return BP_NOT_CAP;
 
 	if (TPL_IF_SERIES(pbpctl_dev->subdevice))
@@ -4213,9 +4217,9 @@ void bypass_caps_init(bpctl_dev_t *pbpctl_dev)
 
 int bypass_off_init(bpctl_dev_t *pbpctl_dev)
 {
-	int ret = 0;
+	int ret = cmnd_on(pbpctl_dev);
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	if (ret < 0)
 		return ret;
 	if (INTEL_IF_SERIES(pbpctl_dev->subdevice))
 		return dis_bypass_cap(pbpctl_dev);
@@ -4401,7 +4405,8 @@ int set_bypass_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
 
 	if (!(pbpctl_dev->bp_caps & BP_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (!bypass_mode)
 		ret = bypass_off(pbpctl_dev);
@@ -4433,7 +4438,8 @@ int set_dis_bypass_fn(bpctl_dev_t *pbpctl_dev, int dis_param)
 
 	if (!(pbpctl_dev->bp_caps & BP_DIS_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (dis_param)
 		ret = dis_bypass_cap(pbpctl_dev);
@@ -4459,7 +4465,8 @@ int set_bypass_pwoff_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
 
 	if (!(pbpctl_dev->bp_caps & BP_PWOFF_CTL_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (bypass_mode)
 		ret = bypass_state_pwroff(pbpctl_dev);
@@ -4485,7 +4492,8 @@ int set_bypass_pwup_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
 
 	if (!(pbpctl_dev->bp_caps & BP_PWUP_CTL_CAP))
 		return BP_NOT_CAP;
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (bypass_mode)
 		ret = bypass_state_pwron(pbpctl_dev);
@@ -4512,7 +4520,8 @@ int set_bypass_wd_fn(bpctl_dev_t *pbpctl_dev, int timeout)
 	if (!(pbpctl_dev->bp_caps & WD_CTL_CAP))
 		return BP_NOT_CAP;
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (!timeout)
 		ret = wdt_off(pbpctl_dev);
@@ -4581,7 +4590,8 @@ int set_std_nic_fn(bpctl_dev_t *pbpctl_dev, int nic_mode)
 	if (!(pbpctl_dev->bp_caps & STD_NIC_CAP))
 		return BP_NOT_CAP;
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	if (nic_mode)
 		ret = std_nic_on(pbpctl_dev);
@@ -4646,8 +4656,8 @@ int get_tap_pwup_fn(bpctl_dev_t *pbpctl_dev)
 	int ret = 0;
 	if (!pbpctl_dev)
 		return -1;
-
-	if ((ret = default_pwron_tap_status(pbpctl_dev)) < 0)
+	ret = default_pwron_tap_status(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	return ((ret == 0) ? 1 : 0);
 }
@@ -4821,8 +4831,8 @@ int get_disc_port_pwup_fn(bpctl_dev_t *pbpctl_dev)
 	int ret = 0;
 	if (!pbpctl_dev)
 		return -1;
-
-	if ((ret = default_pwron_disc_port_status(pbpctl_dev)) < 0)
+	ret = default_pwron_disc_port_status(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	return ((ret == 0) ? 1 : 0);
 }
@@ -4849,7 +4859,8 @@ int reset_cont_fn(bpctl_dev_t *pbpctl_dev)
 	if (!pbpctl_dev)
 		return -1;
 
-	if ((ret = cmnd_on(pbpctl_dev)) < 0)
+	ret = cmnd_on(pbpctl_dev);
+	if (ret < 0)
 		return ret;
 	return reset_cont(pbpctl_dev);
 }
@@ -4865,8 +4876,10 @@ int set_tx_fn(bpctl_dev_t *pbpctl_dev, int tx_state)
 	    (pbpctl_dev->bp_caps & SW_CTL_CAP)) {
 		if ((pbpctl_dev->bp_tpl_flag))
 			return BP_NOT_CAP;
-	} else if ((pbpctl_dev_b = get_master_port_fn(pbpctl_dev))) {
-		if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
+	} else {
+		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+		if ((pbpctl_dev_b) &&
+		    (pbpctl_dev_b->bp_caps & TPL_CAP) &&
 		    (pbpctl_dev_b->bp_tpl_flag))
 			return BP_NOT_CAP;
 	}
@@ -4982,8 +4995,10 @@ int get_tx_fn(bpctl_dev_t *pbpctl_dev)
 	    (pbpctl_dev->bp_caps & SW_CTL_CAP)) {
 		if ((pbpctl_dev->bp_tpl_flag))
 			return BP_NOT_CAP;
-	} else if ((pbpctl_dev_b = get_master_port_fn(pbpctl_dev))) {
-		if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
+	} else{
+		pbpctl_dev_b = get_master_port_fn(pbpctl_dev);
+		if ((pbpctl_dev_b) &&
+		    (pbpctl_dev_b->bp_caps & TPL_CAP) &&
 		    (pbpctl_dev_b->bp_tpl_flag))
 			return BP_NOT_CAP;
 	}
@@ -5020,9 +5035,9 @@ static void bp_tpl_timer_fn(unsigned long param)
 {
 	bpctl_dev_t *pbpctl_dev = (bpctl_dev_t *) param;
 	uint32_t link1, link2;
-	bpctl_dev_t *pbpctl_dev_b = NULL;
+	bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
 
-	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+	if (!pbpctl_dev_b)
 		return;
 
 	if (!pbpctl_dev->bp_tpl_flag) {
@@ -5126,7 +5141,8 @@ int set_tpl_fn(bpctl_dev_t *pbpctl_dev, int tpl_mode)
 
 	if (pbpctl_dev->bp_caps & TPL_CAP) {
 		if (tpl_mode) {
-			if ((pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
+			pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
+			if (pbpctl_dev_b)
 				set_tx(pbpctl_dev_b, 1);
 			set_tx(pbpctl_dev, 1);
 		}
@@ -6708,7 +6724,8 @@ static int init_one(bpctl_dev_t *dev, bpmod_info_t *info, struct pci_dev *pdev1)
 			reset_cont(dev);
 	}
 #ifdef BP_SELF_TEST
-	if ((dev->bp_tx_data = kzalloc(BPTEST_DATA_LEN, GFP_KERNEL))) {
+	dev->bp_tx_data = kzalloc(BPTEST_DATA_LEN, GFP_KERNEL);
+	if (dev->bp_tx_data) {
 		memset(dev->bp_tx_data, 0xff, 6);
 		memset(dev->bp_tx_data + 6, 0x0, 1);
 		memset(dev->bp_tx_data + 7, 0xaa, 5);
-- 
1.7.10.4

Re: [PATCH 2/2 v3] silicom: checkpatch: assignments in if conditions

[Greg KH]

On Wed, Jun 19, 2013 at 10:44:04AM +0200, Lorenz Haspel wrote:
> Fixes checkpatch error:
> There were assignments in if conditions, so I extracted them.
> 
> Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
> Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
> ---
> v2: removed some buggy extra lines and fixed white space issues
> v3: fixed some more extra lines

This patch doesn't apply at all to my tree :(

greg k-h

Re: [PATCH 2/2 v3] silicom: checkpatch: assignments in if conditions

[Lorenz Kernel]

Quoting Greg KH <gregkh@linuxfoundation.org>:

> On Wed, Jun 19, 2013 at 10:44:04AM +0200, Lorenz Haspel wrote:
>> Fixes checkpatch error:
>> There were assignments in if conditions, so I extracted them.
>>
>> Signed-off-by: Lorenz Haspel <lorenz@badgers.com>
>> Signed-off-by: Michael Banken <michael.banken@mathe.stud.uni-erlangen.de>
>> ---
>> v2: removed some buggy extra lines and fixed white space issues
>> v3: fixed some more extra lines
>
> This patch doesn't apply at all to my tree :(
>
> greg k-h
>

This patch doesn't apply, because Chad Williamson already sent a patch  
fixing the problem.
His patch is already in next-tree, so my patch is not longer necessary.

Lorenz Haspel



----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program,
part of the Horde framework and provided by HomeFreeMail.com.