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

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

[Vasanthakumar Thiagarajan]

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 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

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

[Vasanthakumar Thiagarajan]

To enable firmware debug messages through uart interface,

modprobe ath6kl_sdio uart_debug=1.

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 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

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

[Vasanthakumar Thiagarajan]

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 <vthiagar@qca.qualcomm.com>
---
 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

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

[Kalle Valo]

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

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

[Kalle Valo]

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

Kalle

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

[Vasanthakumar Thiagarajan]

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

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

[Johannes Berg]

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

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

[Vasanthakumar Thiagarajan]

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