Quantcast

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

classic Classic list List threaded Threaded
25 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[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 <[hidden email]>
Signed-off-by: Michael Banken <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[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 <[hidden email]>
Signed-off-by: Michael Banken <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Lorenz Haspel
In reply to this post by Lorenz Haspel
fixed checkpatch error:
trailing statements that should be on next line

Signed-off-by: Lorenz Haspel <[hidden email]>
Signed-off-by: Michael Banken <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Lorenz Haspel
In reply to this post by Lorenz Haspel
fixed checkpatch error:
added parenthesis around complex macros.

Signed-off-by: Lorenz Haspel <[hidden email]>
Signed-off-by: Michael Banken <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Dan Carpenter
In reply to this post by Lorenz Haspel
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 <[hidden email]>
> Signed-off-by: Michael Banken <[hidden email]>

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Dan Carpenter
In reply to this post by Lorenz Haspel
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Dan Carpenter
In reply to this post by 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Dan Carpenter
In reply to this post by 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Joe Perches
In reply to this post by Dan Carpenter
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)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Lorenz Haspel
In reply to this post by Lorenz Haspel
Fixes checkpatch error:
There were assignments in if conditions, so I extracted them.

Signed-off-by: Lorenz Haspel <[hidden email]>
Signed-off-by: Michael Banken <[hidden email]>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Lorenz Haspel
In reply to this post by 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 <[hidden email]>
Signed-off-by: Michael Banken <[hidden email]>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Greg Kroah-Hartman
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Dan Carpenter
In reply to this post by Lorenz Haspel
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 <[hidden email]>
> Signed-off-by: Michael Banken <[hidden email]>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Joe Perches
In reply to this post by Greg Kroah-Hartman
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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Lorenz Haspel
In reply to this post by Lorenz Haspel
Fixes checkpatch error:
There were assignments in if conditions, so I extracted them.

Signed-off-by: Lorenz Haspel <[hidden email]>
Signed-off-by: Michael Banken <[hidden email]>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 <[hidden email]>
> Signed-off-by: Michael Banken <[hidden email]>

Looks good, thanks.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
12
Loading...