2018-02-26 16:35:46

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 0/8] staging: wilc1000: fix coding style & checkpatch reported issues

Cleanup patch series to fix checkpatch.pl reported issue & code
modification to follow linux coding style.

Ajay Singh (8):
staging: wilc1000: remove unnecessary while(0) in
wilc_wlan_handle_txq()
staging: wilc1000: rename label _end_ in wilc_wlan_handle_txq()
staging: wilc1000: fix line over 80 char in wilc_wlan_handle_txq()
staging: wilc1000: move multiple definition of same macro to common
header
staging: wilc1000: rename WILC_WFI_mgmt_rx() to avoid camelCase
staging: wilc1000: fix line over 80 char in wilc_wlan_handle_rxq()
staging: wilc1000: fix line over 80 char in wilc_wlan_cfg_set()
staging: wilc1000: fix open parenthesis mismatch issue in
wilc_wlan_cfg_set()

drivers/staging/wilc1000/linux_mon.c | 3 -
drivers/staging/wilc1000/linux_wlan.c | 2 +-
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 -
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 2 +-
drivers/staging/wilc1000/wilc_wlan.c | 361 +++++++++++-----------
drivers/staging/wilc1000/wilc_wlan.h | 5 +
6 files changed, 183 insertions(+), 193 deletions(-)

--
2.7.4


2018-02-26 16:35:53

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 2/8] staging: wilc1000: rename label _end_ in wilc_wlan_handle_txq()

Rename label name starting with '_' to follow as per linux coding style.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wlan.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 37d6d87..5422359 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -648,7 +648,7 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)
} while (!wilc->quit);

if (!ret)
- goto _end_;
+ goto out_release_bus;

timeout = 200;
do {
@@ -695,11 +695,11 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)
} while (1);

if (!ret)
- goto _end_;
+ goto out_release_bus;

if (entries == 0) {
ret = WILC_TX_ERR_NO_BUF;
- goto _end_;
+ goto out_release_bus;
}

release_bus(wilc, RELEASE_ALLOW_SLEEP);
@@ -756,11 +756,11 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)

ret = func->hif_clear_int_ext(wilc, ENABLE_TX_VMM);
if (!ret)
- goto _end_;
+ goto out_release_bus;

ret = func->hif_block_tx_ext(wilc, 0, txb, offset);

-_end_:
+out_release_bus:
release_bus(wilc, RELEASE_ALLOW_SLEEP);

out:
--
2.7.4

2018-02-27 13:23:49

by Ajay Singh

[permalink] [raw]
Subject: Re: [PATCH 8/8] staging: wilc1000: fix open parenthesis mismatch issue in wilc_wlan_cfg_set()

Hi Dan,

On Tue, 27 Feb 2018 12:41:40 +0300
Dan Carpenter <[email protected]> wrote:

> The first 5 patches are good, but the last 3 are not OK.

Thanks for your review comments.
I will resubmit the patch series by only including first 5 patches. I
will recheck the last 3 patches and submit them separately.

>
> Normally "tmp" is used as an iterator pointer or something along those
> lines. For example, here is a good use of "tmp".
>
> tmp = left;
> left = right;
> right = tmp;
>
> In this example, you want to store a pointer temporarily, so what else
> are you going to call it besides "tmp"? The name "tmp" doesn't mean
> "I want a short name and I'm too lazy to think of one".
>

Sure, I will take care of this point in subsequent patches.

Regards,
Ajay

2018-02-27 13:32:00

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 8/8] staging: wilc1000: fix open parenthesis mismatch issue in wilc_wlan_cfg_set()

On Tue, Feb 27, 2018 at 06:53:40PM +0530, Ajay Singh wrote:
> Hi Dan,
>
> On Tue, 27 Feb 2018 12:41:40 +0300
> Dan Carpenter <[email protected]> wrote:
>
> > The first 5 patches are good, but the last 3 are not OK.
>
> Thanks for your review comments.
> I will resubmit the patch series by only including first 5 patches. I
> will recheck the last 3 patches and submit them separately.
>

Greg can probably just apply the first 5 as-is. No need to resend.

regards,
dan carpenter

2018-02-26 16:36:14

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 8/8] staging: wilc1000: fix open parenthesis mismatch issue in wilc_wlan_cfg_set()

Fix 'Alignment should match open parenthesis' issue found by
checkpatch.pl script.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wlan.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index acf7591..971e61d2 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -1273,13 +1273,14 @@ int wilc_wlan_cfg_get(struct wilc_vif *vif, int start, u16 wid, int commit,
wilc->cfg_frame_offset = offset;

if (commit) {
+ unsigned long tmp = msecs_to_jiffies(CFG_PKTS_TIMEOUT);
+
wilc->cfg_frame_in_use = 1;

if (wilc_wlan_cfg_commit(vif, WILC_CFG_QUERY, drv_handler))
ret_size = 0;

- if (!wait_for_completion_timeout(&wilc->cfg_event,
- msecs_to_jiffies(CFG_PKTS_TIMEOUT))) {
+ if (!wait_for_completion_timeout(&wilc->cfg_event, tmp)) {
netdev_dbg(vif->ndev, "Get Timed Out\n");
ret_size = 0;
}
--
2.7.4

2018-02-26 16:36:07

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 6/8] staging: wilc1000: fix line over 80 char in wilc_wlan_handle_rxq()

Fix 'line over 80 character' issue found by checkpatch.pl script.
Refactor wilc_wlan_handle_rxq() code to remove the checkpatch.pl
warnings.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wlan.c | 46 ++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 74b80ad..223bf8b 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -798,6 +798,7 @@ static void wilc_wlan_handle_rxq(struct wilc *wilc)
u32 header;
u32 pkt_len, pkt_offset, tp_len;
int is_cfg_packet;
+ int tmp;

memcpy(&header, &buffer[offset], 4);
header = cpu_to_le32(header);
@@ -815,28 +816,31 @@ static void wilc_wlan_handle_rxq(struct wilc *wilc)
IS_MANAGMEMENT_CALLBACK |
IS_MGMT_STATUS_SUCCES);

- wilc_wfi_mgmt_rx(wilc, &buffer[offset + HOST_HDR_OFFSET], pkt_len);
+ tmp = offset + HOST_HDR_OFFSET;
+ wilc_wfi_mgmt_rx(wilc, &buffer[tmp], pkt_len);
+ } else if (!is_cfg_packet) {
+ if (pkt_len > 0) {
+ wilc_frmw_to_linux(wilc,
+ &buffer[offset],
+ pkt_len,
+ pkt_offset);
+ }
} else {
- if (!is_cfg_packet) {
- if (pkt_len > 0) {
- wilc_frmw_to_linux(wilc,
- &buffer[offset],
- pkt_len,
- pkt_offset);
- }
- } else {
- struct wilc_cfg_rsp rsp;
-
- wilc_wlan_cfg_indicate_rx(wilc, &buffer[pkt_offset + offset], pkt_len, &rsp);
- if (rsp.type == WILC_CFG_RSP) {
- if (wilc->cfg_seq_no == rsp.seq_no)
- complete(&wilc->cfg_event);
- } else if (rsp.type == WILC_CFG_RSP_STATUS) {
- wilc_mac_indicate(wilc, WILC_MAC_INDICATE_STATUS);
-
- } else if (rsp.type == WILC_CFG_RSP_SCAN) {
- wilc_mac_indicate(wilc, WILC_MAC_INDICATE_SCAN);
- }
+ struct wilc_cfg_rsp rsp;
+
+ tmp = pkt_offset + offset;
+ wilc_wlan_cfg_indicate_rx(wilc, &buffer[tmp],
+ pkt_len, &rsp);
+ if (rsp.type == WILC_CFG_RSP) {
+ if (wilc->cfg_seq_no == rsp.seq_no)
+ complete(&wilc->cfg_event);
+ } else if (rsp.type == WILC_CFG_RSP_STATUS) {
+ tmp = WILC_MAC_INDICATE_STATUS;
+ wilc_mac_indicate(wilc, tmp);
+
+ } else if (rsp.type == WILC_CFG_RSP_SCAN) {
+ tmp = WILC_MAC_INDICATE_SCAN;
+ wilc_mac_indicate(wilc, tmp);
}
}
offset += tp_len;
--
2.7.4

2018-02-26 16:35:50

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 1/8] staging: wilc1000: remove unnecessary while(0) in wilc_wlan_handle_txq()

Refactor wilc_wlan_handle_txq() by removing unnecessary while(0)
loop. "Line over 80 char" issues in wilc_wlan_handle_txq() are fix by
reducing extra leading tab.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wlan.c | 295 +++++++++++++++++------------------
1 file changed, 141 insertions(+), 154 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 1a9ef1a..37d6d87 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -583,200 +583,187 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)

txb = wilc->tx_buffer;
wilc->txq_exit = 0;
+
+ if (wilc->quit)
+ goto out;
+
+ mutex_lock(&wilc->txq_add_to_head_cs);
+ wilc_wlan_txq_filter_dup_tcp_ack(dev);
+ tqe = wilc_wlan_txq_get_first(wilc);
+ i = 0;
+ sum = 0;
do {
- if (wilc->quit)
- break;
+ if (tqe && (i < (WILC_VMM_TBL_SIZE - 1))) {
+ if (tqe->type == WILC_CFG_PKT)
+ vmm_sz = ETH_CONFIG_PKT_HDR_OFFSET;

- mutex_lock(&wilc->txq_add_to_head_cs);
- wilc_wlan_txq_filter_dup_tcp_ack(dev);
- tqe = wilc_wlan_txq_get_first(wilc);
- i = 0;
- sum = 0;
- do {
- if (tqe && (i < (WILC_VMM_TBL_SIZE - 1))) {
- if (tqe->type == WILC_CFG_PKT)
- vmm_sz = ETH_CONFIG_PKT_HDR_OFFSET;
+ else if (tqe->type == WILC_NET_PKT)
+ vmm_sz = ETH_ETHERNET_HDR_OFFSET;

- else if (tqe->type == WILC_NET_PKT)
- vmm_sz = ETH_ETHERNET_HDR_OFFSET;
+ else
+ vmm_sz = HOST_HDR_OFFSET;

- else
- vmm_sz = HOST_HDR_OFFSET;
+ vmm_sz += tqe->buffer_size;

- vmm_sz += tqe->buffer_size;
+ if (vmm_sz & 0x3)
+ vmm_sz = (vmm_sz + 4) & ~0x3;

- if (vmm_sz & 0x3)
- vmm_sz = (vmm_sz + 4) & ~0x3;
+ if ((sum + vmm_sz) > LINUX_TX_SIZE)
+ break;

- if ((sum + vmm_sz) > LINUX_TX_SIZE)
- break;
+ vmm_table[i] = vmm_sz / 4;
+ if (tqe->type == WILC_CFG_PKT)
+ vmm_table[i] |= BIT(10);
+ vmm_table[i] = cpu_to_le32(vmm_table[i]);

- vmm_table[i] = vmm_sz / 4;
- if (tqe->type == WILC_CFG_PKT)
- vmm_table[i] |= BIT(10);
- vmm_table[i] = cpu_to_le32(vmm_table[i]);
+ i++;
+ sum += vmm_sz;
+ tqe = wilc_wlan_txq_get_next(wilc, tqe);
+ } else {
+ break;
+ }
+ } while (1);

- i++;
- sum += vmm_sz;
- tqe = wilc_wlan_txq_get_next(wilc, tqe);
- } else {
- break;
- }
- } while (1);
+ if (i == 0)
+ goto out;
+ vmm_table[i] = 0x0;

- if (i == 0)
+ acquire_bus(wilc, ACQUIRE_AND_WAKEUP);
+ counter = 0;
+ func = wilc->hif_func;
+ do {
+ ret = func->hif_read_reg(wilc, WILC_HOST_TX_CTRL, &reg);
+ if (!ret)
break;
- vmm_table[i] = 0x0;

- acquire_bus(wilc, ACQUIRE_AND_WAKEUP);
- counter = 0;
- func = wilc->hif_func;
- do {
- ret = func->hif_read_reg(wilc, WILC_HOST_TX_CTRL, &reg);
- if (!ret)
- break;
+ if ((reg & 0x1) == 0)
+ break;

- if ((reg & 0x1) == 0)
- break;
+ counter++;
+ if (counter > 200) {
+ counter = 0;
+ ret = func->hif_write_reg(wilc, WILC_HOST_TX_CTRL, 0);
+ break;
+ }
+ } while (!wilc->quit);

- counter++;
- if (counter > 200) {
- counter = 0;
- ret = func->hif_write_reg(wilc,
- WILC_HOST_TX_CTRL, 0);
- break;
- }
- } while (!wilc->quit);
+ if (!ret)
+ goto _end_;

+ timeout = 200;
+ do {
+ ret = func->hif_block_tx(wilc,
+ WILC_VMM_TBL_RX_SHADOW_BASE,
+ (u8 *)vmm_table,
+ ((i + 1) * 4));
if (!ret)
- goto _end_;
+ break;

- timeout = 200;
- do {
- ret = func->hif_block_tx(wilc,
- WILC_VMM_TBL_RX_SHADOW_BASE,
- (u8 *)vmm_table,
- ((i + 1) * 4));
- if (!ret)
- break;
+ ret = func->hif_write_reg(wilc, WILC_HOST_VMM_CTL, 0x2);
+ if (!ret)
+ break;

- ret = func->hif_write_reg(wilc, WILC_HOST_VMM_CTL, 0x2);
+ do {
+ ret = func->hif_read_reg(wilc, WILC_HOST_VMM_CTL, &reg);
if (!ret)
break;
-
- do {
- ret = func->hif_read_reg(wilc,
- WILC_HOST_VMM_CTL,
- &reg);
- if (!ret)
- break;
- if ((reg >> 2) & 0x1) {
- entries = ((reg >> 3) & 0x3f);
- break;
- }
- release_bus(wilc, RELEASE_ALLOW_SLEEP);
- } while (--timeout);
- if (timeout <= 0) {
- ret = func->hif_write_reg(wilc,
- WILC_HOST_VMM_CTL,
- 0x0);
+ if ((reg >> 2) & 0x1) {
+ entries = ((reg >> 3) & 0x3f);
break;
}
+ release_bus(wilc, RELEASE_ALLOW_SLEEP);
+ } while (--timeout);
+ if (timeout <= 0) {
+ ret = func->hif_write_reg(wilc, WILC_HOST_VMM_CTL, 0x0);
+ break;
+ }
+
+ if (!ret)
+ break;

+ if (entries == 0) {
+ ret = func->hif_read_reg(wilc, WILC_HOST_TX_CTRL, &reg);
if (!ret)
break;
-
- if (entries == 0) {
- ret = func->hif_read_reg(wilc,
- WILC_HOST_TX_CTRL,
- &reg);
- if (!ret)
- break;
- reg &= ~BIT(0);
- ret = func->hif_write_reg(wilc,
- WILC_HOST_TX_CTRL,
- reg);
- if (!ret)
- break;
+ reg &= ~BIT(0);
+ ret = func->hif_write_reg(wilc, WILC_HOST_TX_CTRL, reg);
+ if (!ret)
break;
- }
break;
- } while (1);
+ }
+ break;
+ } while (1);

- if (!ret)
- goto _end_;
+ if (!ret)
+ goto _end_;

- if (entries == 0) {
- ret = WILC_TX_ERR_NO_BUF;
- goto _end_;
- }
+ if (entries == 0) {
+ ret = WILC_TX_ERR_NO_BUF;
+ goto _end_;
+ }

- release_bus(wilc, RELEASE_ALLOW_SLEEP);
+ release_bus(wilc, RELEASE_ALLOW_SLEEP);

- offset = 0;
- i = 0;
- do {
- tqe = wilc_wlan_txq_remove_from_head(dev);
- if (tqe && vmm_table[i] != 0) {
- u32 header, buffer_offset;
-
- vmm_table[i] = cpu_to_le32(vmm_table[i]);
- vmm_sz = (vmm_table[i] & 0x3ff);
- vmm_sz *= 4;
- header = (tqe->type << 31) |
- (tqe->buffer_size << 15) |
- vmm_sz;
- if (tqe->type == WILC_MGMT_PKT)
- header |= BIT(30);
- else
- header &= ~BIT(30);
-
- header = cpu_to_le32(header);
- memcpy(&txb[offset], &header, 4);
- if (tqe->type == WILC_CFG_PKT) {
- buffer_offset = ETH_CONFIG_PKT_HDR_OFFSET;
- } else if (tqe->type == WILC_NET_PKT) {
- char *bssid = ((struct tx_complete_data *)(tqe->priv))->bssid;
-
- buffer_offset = ETH_ETHERNET_HDR_OFFSET;
- memcpy(&txb[offset + 8], bssid, 6);
- } else {
- buffer_offset = HOST_HDR_OFFSET;
- }
+ offset = 0;
+ i = 0;
+ do {
+ tqe = wilc_wlan_txq_remove_from_head(dev);
+ if (tqe && vmm_table[i] != 0) {
+ u32 header, buffer_offset;
+
+ vmm_table[i] = cpu_to_le32(vmm_table[i]);
+ vmm_sz = (vmm_table[i] & 0x3ff);
+ vmm_sz *= 4;
+ header = (tqe->type << 31) |
+ (tqe->buffer_size << 15) |
+ vmm_sz;
+ if (tqe->type == WILC_MGMT_PKT)
+ header |= BIT(30);
+ else
+ header &= ~BIT(30);

- memcpy(&txb[offset + buffer_offset],
- tqe->buffer, tqe->buffer_size);
- offset += vmm_sz;
- i++;
- tqe->status = 1;
- if (tqe->tx_complete_func)
- tqe->tx_complete_func(tqe->priv,
- tqe->status);
- if (tqe->tcp_pending_ack_idx != NOT_TCP_ACK &&
- tqe->tcp_pending_ack_idx < MAX_PENDING_ACKS)
- pending_acks_info[tqe->tcp_pending_ack_idx].txqe = NULL;
- kfree(tqe);
+ header = cpu_to_le32(header);
+ memcpy(&txb[offset], &header, 4);
+ if (tqe->type == WILC_CFG_PKT) {
+ buffer_offset = ETH_CONFIG_PKT_HDR_OFFSET;
+ } else if (tqe->type == WILC_NET_PKT) {
+ char *bssid = ((struct tx_complete_data *)(tqe->priv))->bssid;
+
+ buffer_offset = ETH_ETHERNET_HDR_OFFSET;
+ memcpy(&txb[offset + 8], bssid, 6);
} else {
- break;
+ buffer_offset = HOST_HDR_OFFSET;
}
- } while (--entries);

- acquire_bus(wilc, ACQUIRE_AND_WAKEUP);
+ memcpy(&txb[offset + buffer_offset],
+ tqe->buffer, tqe->buffer_size);
+ offset += vmm_sz;
+ i++;
+ tqe->status = 1;
+ if (tqe->tx_complete_func)
+ tqe->tx_complete_func(tqe->priv,
+ tqe->status);
+ if (tqe->tcp_pending_ack_idx != NOT_TCP_ACK &&
+ tqe->tcp_pending_ack_idx < MAX_PENDING_ACKS)
+ pending_acks_info[tqe->tcp_pending_ack_idx].txqe = NULL;
+ kfree(tqe);
+ } else {
+ break;
+ }
+ } while (--entries);

- ret = func->hif_clear_int_ext(wilc, ENABLE_TX_VMM);
- if (!ret)
- goto _end_;
+ acquire_bus(wilc, ACQUIRE_AND_WAKEUP);

- ret = func->hif_block_tx_ext(wilc, 0, txb, offset);
- if (!ret)
- goto _end_;
+ ret = func->hif_clear_int_ext(wilc, ENABLE_TX_VMM);
+ if (!ret)
+ goto _end_;
+
+ ret = func->hif_block_tx_ext(wilc, 0, txb, offset);

_end_:
+ release_bus(wilc, RELEASE_ALLOW_SLEEP);

- release_bus(wilc, RELEASE_ALLOW_SLEEP);
- if (ret != 1)
- break;
- } while (0);
+out:
mutex_unlock(&wilc->txq_add_to_head_cs);

wilc->txq_exit = 1;
--
2.7.4

2018-02-27 09:35:25

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 6/8] staging: wilc1000: fix line over 80 char in wilc_wlan_handle_rxq()

On Mon, Feb 26, 2018 at 10:02:00PM +0530, Ajay Singh wrote:
> Fix 'line over 80 character' issue found by checkpatch.pl script.
> Refactor wilc_wlan_handle_rxq() code to remove the checkpatch.pl
> warnings.
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/wilc_wlan.c | 46 ++++++++++++++++++++----------------
> 1 file changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
> index 74b80ad..223bf8b 100644
> --- a/drivers/staging/wilc1000/wilc_wlan.c
> +++ b/drivers/staging/wilc1000/wilc_wlan.c
> @@ -798,6 +798,7 @@ static void wilc_wlan_handle_rxq(struct wilc *wilc)
> u32 header;
> u32 pkt_len, pkt_offset, tp_len;
> int is_cfg_packet;
> + int tmp;
>

Heh. Nope. Don't do this. You've just create a "tmp" variable to hold
many different types of random long values... It makes the code less
readable.

Just break it up into separate functions.

regards,
dan carpenter

2018-02-27 09:41:59

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 8/8] staging: wilc1000: fix open parenthesis mismatch issue in wilc_wlan_cfg_set()

The first 5 patches are good, but the last 3 are not OK.

Normally "tmp" is used as an iterator pointer or something along those
lines. For example, here is a good use of "tmp".

tmp = left;
left = right;
right = tmp;

In this example, you want to store a pointer temporarily, so what else
are you going to call it besides "tmp"? The name "tmp" doesn't mean
"I want a short name and I'm too lazy to think of one".

regards,
dan carpenter

2018-02-26 16:35:57

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 3/8] staging: wilc1000: fix line over 80 char in wilc_wlan_handle_txq()

Refactor wilc_wlan_handle_txq() to fix 'line over 80 char' issue found
by checkpatch.pl script.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wlan.c | 76 ++++++++++++++++++------------------
1 file changed, 39 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 5422359..ee7102d 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -707,49 +707,51 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)
offset = 0;
i = 0;
do {
+ u32 header, buffer_offset;
+ char *bssid;
+
tqe = wilc_wlan_txq_remove_from_head(dev);
- if (tqe && vmm_table[i] != 0) {
- u32 header, buffer_offset;
+ if (!tqe)
+ break;

- vmm_table[i] = cpu_to_le32(vmm_table[i]);
- vmm_sz = (vmm_table[i] & 0x3ff);
- vmm_sz *= 4;
- header = (tqe->type << 31) |
- (tqe->buffer_size << 15) |
- vmm_sz;
- if (tqe->type == WILC_MGMT_PKT)
- header |= BIT(30);
- else
- header &= ~BIT(30);
+ if (vmm_table[i] == 0)
+ break;

- header = cpu_to_le32(header);
- memcpy(&txb[offset], &header, 4);
- if (tqe->type == WILC_CFG_PKT) {
- buffer_offset = ETH_CONFIG_PKT_HDR_OFFSET;
- } else if (tqe->type == WILC_NET_PKT) {
- char *bssid = ((struct tx_complete_data *)(tqe->priv))->bssid;
-
- buffer_offset = ETH_ETHERNET_HDR_OFFSET;
- memcpy(&txb[offset + 8], bssid, 6);
- } else {
- buffer_offset = HOST_HDR_OFFSET;
- }
+ vmm_table[i] = cpu_to_le32(vmm_table[i]);
+ vmm_sz = (vmm_table[i] & 0x3ff);
+ vmm_sz *= 4;
+ header = (tqe->type << 31) |
+ (tqe->buffer_size << 15) |
+ vmm_sz;
+ if (tqe->type == WILC_MGMT_PKT)
+ header |= BIT(30);
+ else
+ header &= ~BIT(30);

- memcpy(&txb[offset + buffer_offset],
- tqe->buffer, tqe->buffer_size);
- offset += vmm_sz;
- i++;
- tqe->status = 1;
- if (tqe->tx_complete_func)
- tqe->tx_complete_func(tqe->priv,
- tqe->status);
- if (tqe->tcp_pending_ack_idx != NOT_TCP_ACK &&
- tqe->tcp_pending_ack_idx < MAX_PENDING_ACKS)
- pending_acks_info[tqe->tcp_pending_ack_idx].txqe = NULL;
- kfree(tqe);
+ header = cpu_to_le32(header);
+ memcpy(&txb[offset], &header, 4);
+ if (tqe->type == WILC_CFG_PKT) {
+ buffer_offset = ETH_CONFIG_PKT_HDR_OFFSET;
+ } else if (tqe->type == WILC_NET_PKT) {
+ bssid = ((struct tx_complete_data *)(tqe->priv))->bssid;
+
+ buffer_offset = ETH_ETHERNET_HDR_OFFSET;
+ memcpy(&txb[offset + 8], bssid, 6);
} else {
- break;
+ buffer_offset = HOST_HDR_OFFSET;
}
+
+ memcpy(&txb[offset + buffer_offset],
+ tqe->buffer, tqe->buffer_size);
+ offset += vmm_sz;
+ i++;
+ tqe->status = 1;
+ if (tqe->tx_complete_func)
+ tqe->tx_complete_func(tqe->priv, tqe->status);
+ if (tqe->tcp_pending_ack_idx != NOT_TCP_ACK &&
+ tqe->tcp_pending_ack_idx < MAX_PENDING_ACKS)
+ pending_acks_info[tqe->tcp_pending_ack_idx].txqe = NULL;
+ kfree(tqe);
} while (--entries);

acquire_bus(wilc, ACQUIRE_AND_WAKEUP);
--
2.7.4

2018-02-27 09:36:31

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 7/8] staging: wilc1000: fix line over 80 char in wilc_wlan_cfg_set()

On Mon, Feb 26, 2018 at 10:02:01PM +0530, Ajay Singh wrote:
> Fix 'line over 80 character' issue found by checkpatch.pl script.
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/wilc_wlan.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
> index 223bf8b..acf7591 100644
> --- a/drivers/staging/wilc1000/wilc_wlan.c
> +++ b/drivers/staging/wilc1000/wilc_wlan.c
> @@ -1230,6 +1230,8 @@ int wilc_wlan_cfg_set(struct wilc_vif *vif, int start, u16 wid, u8 *buffer,
> wilc->cfg_frame_offset = offset;
>
> if (commit) {
> + unsigned long tmp = msecs_to_jiffies(CFG_PKTS_TIMEOUT);

Again. This "tmp" is a bad name here. "tmp" can be a good name, and
I use it all the time, but *here* it is a bad name.

regards,
dan carpenter

2018-02-26 16:36:00

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 4/8] staging: wilc1000: move multiple definition of same macro to common header

Move the same #define from multiple '.c' files to common header file.
Instead of having same macro in different '.c' files, now kept in
common '.h' file.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/linux_mon.c | 3 ---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 ---
drivers/staging/wilc1000/wilc_wlan.c | 4 ----
drivers/staging/wilc1000/wilc_wlan.h | 5 +++++
4 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_mon.c b/drivers/staging/wilc1000/linux_mon.c
index 91d49c4..bbdfc7a 100644
--- a/drivers/staging/wilc1000/linux_mon.c
+++ b/drivers/staging/wilc1000/linux_mon.c
@@ -40,9 +40,6 @@ static u8 broadcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};

#define IEEE80211_RADIOTAP_F_TX_RTS 0x0004 /* used rts/cts handshake */
#define IEEE80211_RADIOTAP_F_TX_FAIL 0x0001 /* failed due to excessive*/
-#define IS_MANAGMEMENT 0x100
-#define IS_MANAGMEMENT_CALLBACK 0x080
-#define IS_MGMT_STATUS_SUCCES 0x040
#define GET_PKT_OFFSET(a) (((a) >> 22) & 0x1ff)

void WILC_WFI_monitor_rx(u8 *buff, u32 size)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 621810d..3f78abc 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -85,9 +85,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
#define TCP_ACK_FILTER_LINK_SPEED_THRESH 54
#define DEFAULT_LINK_SPEED 72

-#define IS_MANAGMEMENT 0x100
-#define IS_MANAGMEMENT_CALLBACK 0x080
-#define IS_MGMT_STATUS_SUCCES 0x040
#define GET_PKT_OFFSET(a) (((a) >> 22) & 0x1ff)

static struct network_info last_scanned_shadow[MAX_NUM_SCANNED_NETWORKS_SHADOW];
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index ee7102d..f762785 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -810,10 +810,6 @@ static void wilc_wlan_handle_rxq(struct wilc *wilc)
if (pkt_len == 0 || tp_len == 0)
break;

- #define IS_MANAGMEMENT 0x100
- #define IS_MANAGMEMENT_CALLBACK 0x080
- #define IS_MGMT_STATUS_SUCCES 0x040
-
if (pkt_offset & IS_MANAGMEMENT) {
pkt_offset &= ~(IS_MANAGMEMENT |
IS_MANAGMEMENT_CALLBACK |
diff --git a/drivers/staging/wilc1000/wilc_wlan.h b/drivers/staging/wilc1000/wilc_wlan.h
index da71731..fa157a6 100644
--- a/drivers/staging/wilc1000/wilc_wlan.h
+++ b/drivers/staging/wilc1000/wilc_wlan.h
@@ -195,6 +195,11 @@
#define ENABLE_TX_VMM (SEL_VMM_TBL0 | EN_VMM)
/*time for expiring the completion of cfg packets*/
#define CFG_PKTS_TIMEOUT 2000
+
+#define IS_MANAGMEMENT 0x100
+#define IS_MANAGMEMENT_CALLBACK 0x080
+#define IS_MGMT_STATUS_SUCCES 0x040
+
/********************************************
*
* Debug Type
--
2.7.4

2018-02-27 09:51:12

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 7/8] staging: wilc1000: fix line over 80 char in wilc_wlan_cfg_set()

On Mon, Feb 26, 2018 at 10:02:01PM +0530, Ajay Singh wrote:
> Fix 'line over 80 character' issue found by checkpatch.pl script.
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/wilc_wlan.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
> index 223bf8b..acf7591 100644
> --- a/drivers/staging/wilc1000/wilc_wlan.c
> +++ b/drivers/staging/wilc1000/wilc_wlan.c
> @@ -1230,6 +1230,8 @@ int wilc_wlan_cfg_set(struct wilc_vif *vif, int start, u16 wid, u8 *buffer,
> wilc->cfg_frame_offset = offset;
>
> if (commit) {
> + unsigned long tmp = msecs_to_jiffies(CFG_PKTS_TIMEOUT);
> +
> netdev_dbg(vif->ndev,
> "[WILC]PACKET Commit with sequence number %d\n",
> wilc->cfg_seq_no);
> @@ -1239,8 +1241,7 @@ int wilc_wlan_cfg_set(struct wilc_vif *vif, int start, u16 wid, u8 *buffer,
> if (wilc_wlan_cfg_commit(vif, WILC_CFG_SET, drv_handler))
> ret_size = 0;
>
> - if (!wait_for_completion_timeout(&wilc->cfg_event,
> - msecs_to_jiffies(CFG_PKTS_TIMEOUT))) {
> + if (!wait_for_completion_timeout(&wilc->cfg_event, tmp)) {


Also, it's not just the variable name I have an issue with. I like that
I can see CFG_PKTS_TIMEOUT directly instead of having to look for it a
few lines back. Don't add unecessary indirection.

So just leave this one as-is. Or flip the "if (commit) " condition
around and do:

if (!commit)
return ret_size;

regards,
dan carpenter

2018-02-26 16:36:04

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 5/8] staging: wilc1000: rename WILC_WFI_mgmt_rx() to avoid camelCase

Fix "Avoid camelCase" issue found by checkpatch.pl script.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/linux_wlan.c | 2 +-
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 2 +-
drivers/staging/wilc1000/wilc_wlan.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index cf746f2..fe19bf3 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -1149,7 +1149,7 @@ void wilc_frmw_to_linux(struct wilc *wilc, u8 *buff, u32 size, u32 pkt_offset)
}
}

-void WILC_WFI_mgmt_rx(struct wilc *wilc, u8 *buff, u32 size)
+void wilc_wfi_mgmt_rx(struct wilc *wilc, u8 *buff, u32 size)
{
int i = 0;
struct wilc_vif *vif;
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 3337fb2..d62c4f1 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -230,7 +230,7 @@ void wilc_netdev_cleanup(struct wilc *wilc);
int wilc_netdev_init(struct wilc **wilc, struct device *dev, int io_type,
int gpio, const struct wilc_hif_func *ops);
void wilc1000_wlan_deinit(struct net_device *dev);
-void WILC_WFI_mgmt_rx(struct wilc *wilc, u8 *buff, u32 size);
+void wilc_wfi_mgmt_rx(struct wilc *wilc, u8 *buff, u32 size);
int wilc_wlan_get_firmware(struct net_device *dev);
int wilc_wlan_set_bssid(struct net_device *wilc_netdev, u8 *bssid, u8 mode);

diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index f762785..74b80ad 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -815,7 +815,7 @@ static void wilc_wlan_handle_rxq(struct wilc *wilc)
IS_MANAGMEMENT_CALLBACK |
IS_MGMT_STATUS_SUCCES);

- WILC_WFI_mgmt_rx(wilc, &buffer[offset + HOST_HDR_OFFSET], pkt_len);
+ wilc_wfi_mgmt_rx(wilc, &buffer[offset + HOST_HDR_OFFSET], pkt_len);
} else {
if (!is_cfg_packet) {
if (pkt_len > 0) {
--
2.7.4

2018-02-26 16:36:11

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 7/8] staging: wilc1000: fix line over 80 char in wilc_wlan_cfg_set()

Fix 'line over 80 character' issue found by checkpatch.pl script.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wlan.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 223bf8b..acf7591 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -1230,6 +1230,8 @@ int wilc_wlan_cfg_set(struct wilc_vif *vif, int start, u16 wid, u8 *buffer,
wilc->cfg_frame_offset = offset;

if (commit) {
+ unsigned long tmp = msecs_to_jiffies(CFG_PKTS_TIMEOUT);
+
netdev_dbg(vif->ndev,
"[WILC]PACKET Commit with sequence number %d\n",
wilc->cfg_seq_no);
@@ -1239,8 +1241,7 @@ int wilc_wlan_cfg_set(struct wilc_vif *vif, int start, u16 wid, u8 *buffer,
if (wilc_wlan_cfg_commit(vif, WILC_CFG_SET, drv_handler))
ret_size = 0;

- if (!wait_for_completion_timeout(&wilc->cfg_event,
- msecs_to_jiffies(CFG_PKTS_TIMEOUT))) {
+ if (!wait_for_completion_timeout(&wilc->cfg_event, tmp)) {
netdev_dbg(vif->ndev, "Set Timed Out\n");
ret_size = 0;
}
--
2.7.4