Subject: [PATCH 1/3] ath6kl: Remove few unnecessary spin_locks around set_bit()

Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
drivers/net/wireless/ath/ath6kl/txrx.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/txrx.c b/drivers/net/wireless/ath/ath6kl/txrx.c
index 506a303..7645eac 100644
--- a/drivers/net/wireless/ath/ath6kl/txrx.c
+++ b/drivers/net/wireless/ath/ath6kl/txrx.c
@@ -449,9 +449,7 @@ enum htc_send_full_action ath6kl_tx_queue_full(struct htc_target *target,
* WMI queue with too many commands the only exception to
* this is during testing using endpointping.
*/
- spin_lock_bh(&ar->lock);
set_bit(WMI_CTRL_EP_FULL, &ar->flag);
- spin_unlock_bh(&ar->lock);
ath6kl_err("wmi ctrl ep is full\n");
return action;
}
@@ -479,9 +477,7 @@ enum htc_send_full_action ath6kl_tx_queue_full(struct htc_target *target,
action != HTC_SEND_FULL_DROP) {
spin_unlock_bh(&ar->list_lock);

- spin_lock_bh(&vif->if_lock);
set_bit(NETQ_STOPPED, &vif->flags);
- spin_unlock_bh(&vif->if_lock);
netif_stop_queue(vif->ndev);

return action;
--
1.7.0.4



Subject: [PATCH 3/3] ath6kl: Remove few deadcode in main.c

1. In ath6kl_add_new_sta(), if (ielen <= ATH6KL_MAX_IE) is going to be
always true due to ielen being u8 and is checked against 256.

2. In ath6kl_reset_device(), since control can never reach switch..case
when the target_type is neither TARGET_TYPE_AR6003 nor TARGET_TYPE_AR6004,
remove the default option of switch statement.

Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
drivers/net/wireless/ath/ath6kl/main.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/main.c b/drivers/net/wireless/ath/ath6kl/main.c
index eea3c74..ff447b0 100644
--- a/drivers/net/wireless/ath/ath6kl/main.c
+++ b/drivers/net/wireless/ath/ath6kl/main.c
@@ -62,8 +62,7 @@ static void ath6kl_add_new_sta(struct ath6kl *ar, u8 *mac, u16 aid, u8 *wpaie,

sta = &ar->sta_list[free_slot];
memcpy(sta->mac, mac, ETH_ALEN);
- if (ielen <= ATH6KL_MAX_IE)
- memcpy(sta->wpa_ie, wpaie, ielen);
+ memcpy(sta->wpa_ie, wpaie, ielen);
sta->aid = aid;
sta->keymgmt = keymgmt;
sta->ucipher = ucipher;
@@ -347,9 +346,6 @@ void ath6kl_reset_device(struct ath6kl *ar, u32 target_type,
case TARGET_TYPE_AR6004:
address = AR6004_RESET_CONTROL_ADDRESS;
break;
- default:
- address = AR6003_RESET_CONTROL_ADDRESS;
- break;
}

status = ath6kl_diag_write32(ar, address, data);
--
1.7.0.4


Subject: [PATCH 2/3] ath6kl: Add a module parameter to enable uart debug

To enable firmware debug messages through uart interface,

modprobe ath6kl_sdio uart_debug=1.

Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
drivers/net/wireless/ath/ath6kl/init.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
index 57e0312..fcac538 100644
--- a/drivers/net/wireless/ath/ath6kl/init.c
+++ b/drivers/net/wireless/ath/ath6kl/init.c
@@ -28,10 +28,12 @@
unsigned int debug_mask;
static unsigned int testmode;
static bool suspend_cutpower;
+static unsigned int uart_debug;

module_param(debug_mask, uint, 0644);
module_param(testmode, uint, 0644);
module_param(suspend_cutpower, bool, 0444);
+module_param(uart_debug, uint, 0644);

static const struct ath6kl_hw hw_list[] = {
{
@@ -464,6 +466,13 @@ int ath6kl_configure_target(struct ath6kl *ar)
u8 fw_iftype, fw_mode = 0, fw_submode = 0;
int i, status;

+ param = uart_debug;
+ if (ath6kl_bmi_write(ar, ath6kl_get_hi_item_addr(ar,
+ HI_ITEM(hi_serial_enable)), (u8 *)&param, 4)) {
+ ath6kl_err("bmi_write_memory for uart debug failed\n");
+ return -EIO;
+ }
+
/*
* Note: Even though the firmware interface type is
* chosen as BSS_STA for all three interfaces, can
--
1.7.0.4


2012-01-02 17:49:46

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 3/3] ath6kl: Remove few deadcode in main.c

On 12/29/2011 12:35 PM, Vasanthakumar Thiagarajan wrote:
> 1. In ath6kl_add_new_sta(), if (ielen <= ATH6KL_MAX_IE) is going to be
> always true due to ielen being u8 and is checked against 256.
>
> 2. In ath6kl_reset_device(), since control can never reach switch..case
> when the target_type is neither TARGET_TYPE_AR6003 nor TARGET_TYPE_AR6004,
> remove the default option of switch statement.

Two separate patches, please.

> @@ -62,8 +62,7 @@ static void ath6kl_add_new_sta(struct ath6kl *ar, u8 *mac, u16 aid, u8 *wpaie,
>
> sta = &ar->sta_list[free_slot];
> memcpy(sta->mac, mac, ETH_ALEN);
> - if (ielen <= ATH6KL_MAX_IE)
> - memcpy(sta->wpa_ie, wpaie, ielen);
> + memcpy(sta->wpa_ie, wpaie, ielen);

And then someone changes the u8 ielen to something else and accidentally
adds a bug. I would prefer to have an explicit check for the ielen to
make it obvious what's the maximum length.

I don't really like using u8 for ielen either. IMHO size_t or similar
would be better.

Kalle

2012-01-03 08:12:53

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 3/3] ath6kl: Remove few deadcode in main.c

On Tue, 2012-01-03 at 11:10 +0530, Vasanthakumar Thiagarajan wrote:

> > > @@ -62,8 +62,7 @@ static void ath6kl_add_new_sta(struct ath6kl *ar, u8 *mac, u16 aid, u8 *wpaie,
> > >
> > > sta = &ar->sta_list[free_slot];
> > > memcpy(sta->mac, mac, ETH_ALEN);
> > > - if (ielen <= ATH6KL_MAX_IE)
> > > - memcpy(sta->wpa_ie, wpaie, ielen);
> > > + memcpy(sta->wpa_ie, wpaie, ielen);
>
> >
> > And then someone changes the u8 ielen to something else and accidentally
> > adds a bug. I would prefer to have an explicit check for the ielen to
> > make it obvious what's the maximum length.
> >
> > I don't really like using u8 for ielen either. IMHO size_t or similar
> > would be better.
>
> The length of any IE is 1 byte, u8 is the appropriate one.

No -- this can contain multiple IEs.

johannes


Subject: Re: [PATCH 3/3] ath6kl: Remove few deadcode in main.c

On Tue, Jan 03, 2012 at 09:12:50AM +0100, Johannes Berg wrote:
> On Tue, 2012-01-03 at 11:10 +0530, Vasanthakumar Thiagarajan wrote:
>
> > > > @@ -62,8 +62,7 @@ static void ath6kl_add_new_sta(struct ath6kl *ar, u8 *mac, u16 aid, u8 *wpaie,
> > > >
> > > > sta = &ar->sta_list[free_slot];
> > > > memcpy(sta->mac, mac, ETH_ALEN);
> > > > - if (ielen <= ATH6KL_MAX_IE)
> > > > - memcpy(sta->wpa_ie, wpaie, ielen);
> > > > + memcpy(sta->wpa_ie, wpaie, ielen);
> >
> > >
> > > And then someone changes the u8 ielen to something else and accidentally
> > > adds a bug. I would prefer to have an explicit check for the ielen to
> > > make it obvious what's the maximum length.
> > >
> > > I don't really like using u8 for ielen either. IMHO size_t or similar
> > > would be better.
> >
> > The length of any IE is 1 byte, u8 is the appropriate one.
>
> No -- this can contain multiple IEs.

Right, it can, but as of now this is any one these ies, wpa, wapi, wpa/wpa2.
Ok, i don't see any disadvantage in converting this u8 to size_t,
i'll send the next version.

Vasanth

2012-01-02 17:47:35

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/3] ath6kl: Remove few unnecessary spin_locks around set_bit()

Patches 1 and 2 applied. For patch 3 I sent comments.

Kalle

Subject: Re: [PATCH 3/3] ath6kl: Remove few deadcode in main.c

On Mon, Jan 02, 2012 at 06:33:15PM +0200, Kalle Valo wrote:
> On 12/29/2011 12:35 PM, Vasanthakumar Thiagarajan wrote:
> > 1. In ath6kl_add_new_sta(), if (ielen <= ATH6KL_MAX_IE) is going to be
> > always true due to ielen being u8 and is checked against 256.
> >
> > 2. In ath6kl_reset_device(), since control can never reach switch..case
> > when the target_type is neither TARGET_TYPE_AR6003 nor TARGET_TYPE_AR6004,
> > remove the default option of switch statement.
>
> Two separate patches, please.

Ok, I split the patch.
>
> > @@ -62,8 +62,7 @@ static void ath6kl_add_new_sta(struct ath6kl *ar, u8 *mac, u16 aid, u8 *wpaie,
> >
> > sta = &ar->sta_list[free_slot];
> > memcpy(sta->mac, mac, ETH_ALEN);
> > - if (ielen <= ATH6KL_MAX_IE)
> > - memcpy(sta->wpa_ie, wpaie, ielen);
> > + memcpy(sta->wpa_ie, wpaie, ielen);

>
> And then someone changes the u8 ielen to something else and accidentally
> adds a bug. I would prefer to have an explicit check for the ielen to
> make it obvious what's the maximum length.
>
> I don't really like using u8 for ielen either. IMHO size_t or similar
> would be better.

The length of any IE is 1 byte, u8 is the appropriate one.

Vasanth