All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] ath6kl: Fix sparse warning "symbol 'conn' shadows an earlier one"
@ 2011-09-30 13:48 Vasanthakumar Thiagarajan
  2011-09-30 13:48 ` [PATCH 2/5] ath6kl: Replace spin_lock_irqsave with spin_lock_bh Vasanthakumar Thiagarajan
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Vasanthakumar Thiagarajan @ 2011-09-30 13:48 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/txrx.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/txrx.c b/drivers/net/wireless/ath/ath6kl/txrx.c
index a711707..bcf7b01 100644
--- a/drivers/net/wireless/ath/ath6kl/txrx.c
+++ b/drivers/net/wireless/ath/ath6kl/txrx.c
@@ -1235,7 +1235,6 @@ void ath6kl_rx(struct htc_target *target, struct htc_packet *packet)
 			 * frame to it on the air else send the
 			 * frame up the stack.
 			 */
-			struct ath6kl_sta *conn = NULL;
 			conn = ath6kl_find_sta(ar, datap->h_dest);
 
 			if (conn && ar->intra_bss) {
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/5] ath6kl: Replace spin_lock_irqsave with spin_lock_bh
  2011-09-30 13:48 [PATCH 1/5] ath6kl: Fix sparse warning "symbol 'conn' shadows an earlier one" Vasanthakumar Thiagarajan
@ 2011-09-30 13:48 ` Vasanthakumar Thiagarajan
  2011-09-30 13:48 ` [PATCH 3/5] ath6kl: Claim sdio function only at appropriate places Vasanthakumar Thiagarajan
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Vasanthakumar Thiagarajan @ 2011-09-30 13:48 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

It is not necessary to use spinlock primitive to
protect data which is accessed in hard irq context as
nothing is running in hard irq with this driver. The
spinlock primitive meant to protect data in softirq
context is more appropriate.

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/main.c |   17 ++++-------
 drivers/net/wireless/ath/ath6kl/sdio.c |   51 +++++++++++++------------------
 2 files changed, 27 insertions(+), 41 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/main.c b/drivers/net/wireless/ath/ath6kl/main.c
index 30b5a53..adb1635 100644
--- a/drivers/net/wireless/ath/ath6kl/main.c
+++ b/drivers/net/wireless/ath/ath6kl/main.c
@@ -1025,8 +1025,6 @@ void ath6kl_connect_event(struct ath6kl *ar, u16 channel, u8 *bssid,
 			  u8 assoc_req_len, u8 assoc_resp_len,
 			  u8 *assoc_info)
 {
-	unsigned long flags;
-
 	ath6kl_cfg80211_connect_event(ar, channel, bssid,
 				      listen_int, beacon_int,
 				      net_type, beacon_ie_len,
@@ -1043,11 +1041,11 @@ void ath6kl_connect_event(struct ath6kl *ar, u16 channel, u8 *bssid,
 	netif_wake_queue(ar->net_dev);
 
 	/* Update connect & link status atomically */
-	spin_lock_irqsave(&ar->lock, flags);
+	spin_lock_bh(&ar->lock);
 	set_bit(CONNECTED, &ar->flag);
 	clear_bit(CONNECT_PEND, &ar->flag);
 	netif_carrier_on(ar->net_dev);
-	spin_unlock_irqrestore(&ar->lock, flags);
+	spin_unlock_bh(&ar->lock);
 
 	aggr_reset_state(ar->aggr_cntxt);
 	ar->reconnect_flag = 0;
@@ -1330,8 +1328,6 @@ void ath6kl_disconnect_event(struct ath6kl *ar, u8 reason, u8 *bssid,
 			     u8 assoc_resp_len, u8 *assoc_info,
 			     u16 prot_reason_status)
 {
-	unsigned long flags;
-
 	if (ar->nw_type == AP_NETWORK) {
 		if (!ath6kl_remove_sta(ar, bssid, prot_reason_status))
 			return;
@@ -1390,10 +1386,10 @@ void ath6kl_disconnect_event(struct ath6kl *ar, u8 reason, u8 *bssid,
 	}
 
 	/* update connect & link status atomically */
-	spin_lock_irqsave(&ar->lock, flags);
+	spin_lock_bh(&ar->lock);
 	clear_bit(CONNECTED, &ar->flag);
 	netif_carrier_off(ar->net_dev);
-	spin_unlock_irqrestore(&ar->lock, flags);
+	spin_unlock_bh(&ar->lock);
 
 	if ((reason != CSERV_DISCONNECT) || (ar->reconnect_flag != 1))
 		ar->reconnect_flag = 0;
@@ -1411,9 +1407,8 @@ void ath6kl_disconnect_event(struct ath6kl *ar, u8 reason, u8 *bssid,
 static int ath6kl_open(struct net_device *dev)
 {
 	struct ath6kl *ar = ath6kl_priv(dev);
-	unsigned long flags;
 
-	spin_lock_irqsave(&ar->lock, flags);
+	spin_lock_bh(&ar->lock);
 
 	set_bit(WLAN_ENABLED, &ar->flag);
 
@@ -1423,7 +1418,7 @@ static int ath6kl_open(struct net_device *dev)
 	} else
 		netif_carrier_off(dev);
 
-	spin_unlock_irqrestore(&ar->lock, flags);
+	spin_unlock_bh(&ar->lock);
 
 	return 0;
 }
diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c
index f1dc311..2dd7a88 100644
--- a/drivers/net/wireless/ath/ath6kl/sdio.c
+++ b/drivers/net/wireless/ath/ath6kl/sdio.c
@@ -166,12 +166,11 @@ static int ath6kl_sdio_io(struct sdio_func *func, u32 request, u32 addr,
 static struct bus_request *ath6kl_sdio_alloc_busreq(struct ath6kl_sdio *ar_sdio)
 {
 	struct bus_request *bus_req;
-	unsigned long flag;
 
-	spin_lock_irqsave(&ar_sdio->lock, flag);
+	spin_lock_bh(&ar_sdio->lock);
 
 	if (list_empty(&ar_sdio->bus_req_freeq)) {
-		spin_unlock_irqrestore(&ar_sdio->lock, flag);
+		spin_unlock_bh(&ar_sdio->lock);
 		return NULL;
 	}
 
@@ -179,7 +178,7 @@ static struct bus_request *ath6kl_sdio_alloc_busreq(struct ath6kl_sdio *ar_sdio)
 				   struct bus_request, list);
 	list_del(&bus_req->list);
 
-	spin_unlock_irqrestore(&ar_sdio->lock, flag);
+	spin_unlock_bh(&ar_sdio->lock);
 	ath6kl_dbg(ATH6KL_DBG_SCATTER, "%s: bus request 0x%p\n",
 		   __func__, bus_req);
 
@@ -189,14 +188,12 @@ static struct bus_request *ath6kl_sdio_alloc_busreq(struct ath6kl_sdio *ar_sdio)
 static void ath6kl_sdio_free_bus_req(struct ath6kl_sdio *ar_sdio,
 				     struct bus_request *bus_req)
 {
-	unsigned long flag;
-
 	ath6kl_dbg(ATH6KL_DBG_SCATTER, "%s: bus request 0x%p\n",
 		   __func__, bus_req);
 
-	spin_lock_irqsave(&ar_sdio->lock, flag);
+	spin_lock_bh(&ar_sdio->lock);
 	list_add_tail(&bus_req->list, &ar_sdio->bus_req_freeq);
-	spin_unlock_irqrestore(&ar_sdio->lock, flag);
+	spin_unlock_bh(&ar_sdio->lock);
 }
 
 static void ath6kl_sdio_setup_scat_data(struct hif_scatter_req *scat_req,
@@ -424,20 +421,19 @@ static void __ath6kl_sdio_write_async(struct ath6kl_sdio *ar_sdio,
 static void ath6kl_sdio_write_async_work(struct work_struct *work)
 {
 	struct ath6kl_sdio *ar_sdio;
-	unsigned long flags;
 	struct bus_request *req, *tmp_req;
 
 	ar_sdio = container_of(work, struct ath6kl_sdio, wr_async_work);
 	sdio_claim_host(ar_sdio->func);
 
-	spin_lock_irqsave(&ar_sdio->wr_async_lock, flags);
+	spin_lock_bh(&ar_sdio->wr_async_lock);
 	list_for_each_entry_safe(req, tmp_req, &ar_sdio->wr_asyncq, list) {
 		list_del(&req->list);
-		spin_unlock_irqrestore(&ar_sdio->wr_async_lock, flags);
+		spin_unlock_bh(&ar_sdio->wr_async_lock);
 		__ath6kl_sdio_write_async(ar_sdio, req);
-		spin_lock_irqsave(&ar_sdio->wr_async_lock, flags);
+		spin_lock_bh(&ar_sdio->wr_async_lock);
 	}
-	spin_unlock_irqrestore(&ar_sdio->wr_async_lock, flags);
+	spin_unlock_bh(&ar_sdio->wr_async_lock);
 
 	sdio_release_host(ar_sdio->func);
 }
@@ -520,7 +516,6 @@ static int ath6kl_sdio_write_async(struct ath6kl *ar, u32 address, u8 *buffer,
 {
 	struct ath6kl_sdio *ar_sdio = ath6kl_sdio_priv(ar);
 	struct bus_request *bus_req;
-	unsigned long flags;
 
 	bus_req = ath6kl_sdio_alloc_busreq(ar_sdio);
 
@@ -533,9 +528,9 @@ static int ath6kl_sdio_write_async(struct ath6kl *ar, u32 address, u8 *buffer,
 	bus_req->request = request;
 	bus_req->packet = packet;
 
-	spin_lock_irqsave(&ar_sdio->wr_async_lock, flags);
+	spin_lock_bh(&ar_sdio->wr_async_lock);
 	list_add_tail(&bus_req->list, &ar_sdio->wr_asyncq);
-	spin_unlock_irqrestore(&ar_sdio->wr_async_lock, flags);
+	spin_unlock_bh(&ar_sdio->wr_async_lock);
 	queue_work(ar->ath6kl_wq, &ar_sdio->wr_async_work);
 
 	return 0;
@@ -581,9 +576,8 @@ static struct hif_scatter_req *ath6kl_sdio_scatter_req_get(struct ath6kl *ar)
 {
 	struct ath6kl_sdio *ar_sdio = ath6kl_sdio_priv(ar);
 	struct hif_scatter_req *node = NULL;
-	unsigned long flag;
 
-	spin_lock_irqsave(&ar_sdio->scat_lock, flag);
+	spin_lock_bh(&ar_sdio->scat_lock);
 
 	if (!list_empty(&ar_sdio->scat_req)) {
 		node = list_first_entry(&ar_sdio->scat_req,
@@ -591,7 +585,7 @@ static struct hif_scatter_req *ath6kl_sdio_scatter_req_get(struct ath6kl *ar)
 		list_del(&node->list);
 	}
 
-	spin_unlock_irqrestore(&ar_sdio->scat_lock, flag);
+	spin_unlock_bh(&ar_sdio->scat_lock);
 
 	return node;
 }
@@ -600,13 +594,12 @@ static void ath6kl_sdio_scatter_req_add(struct ath6kl *ar,
 					struct hif_scatter_req *s_req)
 {
 	struct ath6kl_sdio *ar_sdio = ath6kl_sdio_priv(ar);
-	unsigned long flag;
 
-	spin_lock_irqsave(&ar_sdio->scat_lock, flag);
+	spin_lock_bh(&ar_sdio->scat_lock);
 
 	list_add_tail(&s_req->list, &ar_sdio->scat_req);
 
-	spin_unlock_irqrestore(&ar_sdio->scat_lock, flag);
+	spin_unlock_bh(&ar_sdio->scat_lock);
 
 }
 
@@ -617,7 +610,6 @@ static int ath6kl_sdio_async_rw_scatter(struct ath6kl *ar,
 	struct ath6kl_sdio *ar_sdio = ath6kl_sdio_priv(ar);
 	u32 request = scat_req->req;
 	int status = 0;
-	unsigned long flags;
 
 	if (!scat_req->len)
 		return -EINVAL;
@@ -631,9 +623,9 @@ static int ath6kl_sdio_async_rw_scatter(struct ath6kl *ar,
 		status = ath6kl_sdio_scat_rw(ar_sdio, scat_req->busrequest);
 		sdio_release_host(ar_sdio->func);
 	} else {
-		spin_lock_irqsave(&ar_sdio->wr_async_lock, flags);
+		spin_lock_bh(&ar_sdio->wr_async_lock);
 		list_add_tail(&scat_req->busrequest->list, &ar_sdio->wr_asyncq);
-		spin_unlock_irqrestore(&ar_sdio->wr_async_lock, flags);
+		spin_unlock_bh(&ar_sdio->wr_async_lock);
 		queue_work(ar->ath6kl_wq, &ar_sdio->wr_async_work);
 	}
 
@@ -645,13 +637,12 @@ static void ath6kl_sdio_cleanup_scatter(struct ath6kl *ar)
 {
 	struct ath6kl_sdio *ar_sdio = ath6kl_sdio_priv(ar);
 	struct hif_scatter_req *s_req, *tmp_req;
-	unsigned long flag;
 
 	/* empty the free list */
-	spin_lock_irqsave(&ar_sdio->scat_lock, flag);
+	spin_lock_bh(&ar_sdio->scat_lock);
 	list_for_each_entry_safe(s_req, tmp_req, &ar_sdio->scat_req, list) {
 		list_del(&s_req->list);
-		spin_unlock_irqrestore(&ar_sdio->scat_lock, flag);
+		spin_unlock_bh(&ar_sdio->scat_lock);
 
 		if (s_req->busrequest)
 			ath6kl_sdio_free_bus_req(ar_sdio, s_req->busrequest);
@@ -659,9 +650,9 @@ static void ath6kl_sdio_cleanup_scatter(struct ath6kl *ar)
 		kfree(s_req->sgentries);
 		kfree(s_req);
 
-		spin_lock_irqsave(&ar_sdio->scat_lock, flag);
+		spin_lock_bh(&ar_sdio->scat_lock);
 	}
-	spin_unlock_irqrestore(&ar_sdio->scat_lock, flag);
+	spin_unlock_bh(&ar_sdio->scat_lock);
 }
 
 /* setup of HIF scatter resources */
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/5] ath6kl: Claim sdio function only at appropriate places
  2011-09-30 13:48 [PATCH 1/5] ath6kl: Fix sparse warning "symbol 'conn' shadows an earlier one" Vasanthakumar Thiagarajan
  2011-09-30 13:48 ` [PATCH 2/5] ath6kl: Replace spin_lock_irqsave with spin_lock_bh Vasanthakumar Thiagarajan
@ 2011-09-30 13:48 ` Vasanthakumar Thiagarajan
  2011-09-30 13:48 ` [PATCH 4/5] ath6kl: Fix possible race in accessing bounce buffer Vasanthakumar Thiagarajan
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Vasanthakumar Thiagarajan @ 2011-09-30 13:48 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

There are places where tx_complete callbacks are called with
claiming the sdio function. This is wrong. Claim the sdio function
only when doing actual sdio read/write.

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/sdio.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c
index 2dd7a88..7695c29 100644
--- a/drivers/net/wireless/ath/ath6kl/sdio.c
+++ b/drivers/net/wireless/ath/ath6kl/sdio.c
@@ -134,6 +134,8 @@ static int ath6kl_sdio_io(struct sdio_func *func, u32 request, u32 addr,
 {
 	int ret = 0;
 
+	sdio_claim_host(func);
+
 	if (request & HIF_WRITE) {
 		/* FIXME: looks like ugly workaround for something */
 		if (addr >= HIF_MBOX_BASE_ADDR &&
@@ -155,6 +157,8 @@ static int ath6kl_sdio_io(struct sdio_func *func, u32 request, u32 addr,
 			ret = sdio_memcpy_fromio(func, buf, addr, len);
 	}
 
+	sdio_release_host(func);
+
 	ath6kl_dbg(ATH6KL_DBG_SDIO, "%s addr 0x%x%s buf 0x%p len %d\n",
 		   request & HIF_WRITE ? "wr" : "rd", addr,
 		   request & HIF_FIXED_ADDRESS ? " (fixed)" : "", buf, len);
@@ -287,10 +291,14 @@ static int ath6kl_sdio_scat_rw(struct ath6kl_sdio *ar_sdio,
 	mmc_req.cmd = &cmd;
 	mmc_req.data = &data;
 
+	sdio_claim_host(ar_sdio->func);
+
 	mmc_set_data_timeout(&data, ar_sdio->func->card);
 	/* synchronous call to process request */
 	mmc_wait_for_req(ar_sdio->func->card->host, &mmc_req);
 
+	sdio_release_host(ar_sdio->func);
+
 	status = cmd.error ? cmd.error : data.error;
 
 scat_complete:
@@ -391,11 +399,9 @@ static int ath6kl_sdio_read_write_sync(struct ath6kl *ar, u32 addr, u8 *buf,
 	} else
 		tbuf = buf;
 
-	sdio_claim_host(ar_sdio->func);
 	ret = ath6kl_sdio_io(ar_sdio->func, request, addr, tbuf, len);
 	if ((request & HIF_READ) && bounced)
 		memcpy(buf, tbuf, len);
-	sdio_release_host(ar_sdio->func);
 
 	return ret;
 }
@@ -424,7 +430,6 @@ static void ath6kl_sdio_write_async_work(struct work_struct *work)
 	struct bus_request *req, *tmp_req;
 
 	ar_sdio = container_of(work, struct ath6kl_sdio, wr_async_work);
-	sdio_claim_host(ar_sdio->func);
 
 	spin_lock_bh(&ar_sdio->wr_async_lock);
 	list_for_each_entry_safe(req, tmp_req, &ar_sdio->wr_asyncq, list) {
@@ -434,8 +439,6 @@ static void ath6kl_sdio_write_async_work(struct work_struct *work)
 		spin_lock_bh(&ar_sdio->wr_async_lock);
 	}
 	spin_unlock_bh(&ar_sdio->wr_async_lock);
-
-	sdio_release_host(ar_sdio->func);
 }
 
 static void ath6kl_sdio_irq_handler(struct sdio_func *func)
@@ -618,11 +621,9 @@ static int ath6kl_sdio_async_rw_scatter(struct ath6kl *ar,
 		"hif-scatter: total len: %d scatter entries: %d\n",
 		scat_req->len, scat_req->scat_entries);
 
-	if (request & HIF_SYNCHRONOUS) {
-		sdio_claim_host(ar_sdio->func);
+	if (request & HIF_SYNCHRONOUS)
 		status = ath6kl_sdio_scat_rw(ar_sdio, scat_req->busrequest);
-		sdio_release_host(ar_sdio->func);
-	} else {
+	else {
 		spin_lock_bh(&ar_sdio->wr_async_lock);
 		list_add_tail(&scat_req->busrequest->list, &ar_sdio->wr_asyncq);
 		spin_unlock_bh(&ar_sdio->wr_async_lock);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/5] ath6kl: Fix possible race in accessing bounce buffer
  2011-09-30 13:48 [PATCH 1/5] ath6kl: Fix sparse warning "symbol 'conn' shadows an earlier one" Vasanthakumar Thiagarajan
  2011-09-30 13:48 ` [PATCH 2/5] ath6kl: Replace spin_lock_irqsave with spin_lock_bh Vasanthakumar Thiagarajan
  2011-09-30 13:48 ` [PATCH 3/5] ath6kl: Claim sdio function only at appropriate places Vasanthakumar Thiagarajan
@ 2011-09-30 13:48 ` Vasanthakumar Thiagarajan
  2011-10-03 15:52   ` Thiagarajan, Vasanthakumar
  2011-09-30 13:48 ` [PATCH 5/5] ath6kl: Remove unused ar_sdio->dma_buffer Vasanthakumar Thiagarajan
  2011-10-03 11:08 ` [PATCH 1/5] ath6kl: Fix sparse warning "symbol 'conn' shadows an earlier one" Kalle Valo
  4 siblings, 1 reply; 9+ messages in thread
From: Vasanthakumar Thiagarajan @ 2011-09-30 13:48 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

There is only one bounce buffer (ar_sdio->dma_buffer) which is used
for both read and write without any protection. Fix this race by
allocating bounce buffer every time when it is needed.

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/sdio.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c
index 7695c29..06e3f09 100644
--- a/drivers/net/wireless/ath/ath6kl/sdio.c
+++ b/drivers/net/wireless/ath/ath6kl/sdio.c
@@ -383,25 +383,29 @@ static int ath6kl_sdio_read_write_sync(struct ath6kl *ar, u32 addr, u8 *buf,
 				       u32 len, u32 request)
 {
 	struct ath6kl_sdio *ar_sdio = ath6kl_sdio_priv(ar);
-	u8  *tbuf = NULL;
 	int ret;
+	u8 *bounce_buf = NULL;
 	bool bounced = false;
 
 	if (request & HIF_BLOCK_BASIS)
 		len = round_down(len, HIF_MBOX_BLOCK_SIZE);
 
 	if (buf_needs_bounce(buf)) {
-		if (!ar_sdio->dma_buffer)
+		bounce_buf = kmalloc(len, GFP_ATOMIC);
+		if (!bounce_buf)
 			return -ENOMEM;
-		tbuf = ar_sdio->dma_buffer;
-		memcpy(tbuf, buf, len);
+		memcpy(bounce_buf, buf, len);
 		bounced = true;
 	} else
-		tbuf = buf;
+		bounce_buf = buf;
+
+	ret = ath6kl_sdio_io(ar_sdio->func, request, addr, bounce_buf, len);
 
-	ret = ath6kl_sdio_io(ar_sdio->func, request, addr, tbuf, len);
 	if ((request & HIF_READ) && bounced)
-		memcpy(buf, tbuf, len);
+		memcpy(buf, bounce_buf, len);
+
+	if (bounced)
+		kfree(bounce_buf);
 
 	return ret;
 }
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/5] ath6kl: Remove unused ar_sdio->dma_buffer
  2011-09-30 13:48 [PATCH 1/5] ath6kl: Fix sparse warning "symbol 'conn' shadows an earlier one" Vasanthakumar Thiagarajan
                   ` (2 preceding siblings ...)
  2011-09-30 13:48 ` [PATCH 4/5] ath6kl: Fix possible race in accessing bounce buffer Vasanthakumar Thiagarajan
@ 2011-09-30 13:48 ` Vasanthakumar Thiagarajan
  2011-10-03 11:08 ` [PATCH 1/5] ath6kl: Fix sparse warning "symbol 'conn' shadows an earlier one" Kalle Valo
  4 siblings, 0 replies; 9+ messages in thread
From: Vasanthakumar Thiagarajan @ 2011-09-30 13:48 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/sdio.c |   12 +-----------
 1 files changed, 1 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c
index 06e3f09..6bef420 100644
--- a/drivers/net/wireless/ath/ath6kl/sdio.c
+++ b/drivers/net/wireless/ath/ath6kl/sdio.c
@@ -39,7 +39,6 @@ struct ath6kl_sdio {
 	struct bus_request bus_req[BUS_REQUEST_MAX_NUM];
 
 	struct ath6kl *ar;
-	u8 *dma_buffer;
 
 	/* scatter request list head */
 	struct list_head scat_req;
@@ -777,12 +776,6 @@ static int ath6kl_sdio_probe(struct sdio_func *func,
 	if (!ar_sdio)
 		return -ENOMEM;
 
-	ar_sdio->dma_buffer = kzalloc(HIF_DMA_BUFFER_SIZE, GFP_KERNEL);
-	if (!ar_sdio->dma_buffer) {
-		ret = -ENOMEM;
-		goto err_hif;
-	}
-
 	ar_sdio->func = func;
 	sdio_set_drvdata(func, ar_sdio);
 
@@ -806,7 +799,7 @@ static int ath6kl_sdio_probe(struct sdio_func *func,
 	if (!ar) {
 		ath6kl_err("Failed to alloc ath6kl core\n");
 		ret = -ENOMEM;
-		goto err_dma;
+		goto err_hif;
 	}
 
 	ar_sdio->ar = ar;
@@ -866,8 +859,6 @@ err_off:
 	ath6kl_sdio_power_off(ar_sdio);
 err_cfg80211:
 	ath6kl_cfg80211_deinit(ar_sdio->ar);
-err_dma:
-	kfree(ar_sdio->dma_buffer);
 err_hif:
 	kfree(ar_sdio);
 
@@ -891,7 +882,6 @@ static void ath6kl_sdio_remove(struct sdio_func *func)
 
 	ath6kl_sdio_power_off(ar_sdio);
 
-	kfree(ar_sdio->dma_buffer);
 	kfree(ar_sdio);
 }
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] ath6kl: Fix sparse warning "symbol 'conn' shadows an earlier one"
  2011-09-30 13:48 [PATCH 1/5] ath6kl: Fix sparse warning "symbol 'conn' shadows an earlier one" Vasanthakumar Thiagarajan
                   ` (3 preceding siblings ...)
  2011-09-30 13:48 ` [PATCH 5/5] ath6kl: Remove unused ar_sdio->dma_buffer Vasanthakumar Thiagarajan
@ 2011-10-03 11:08 ` Kalle Valo
  4 siblings, 0 replies; 9+ messages in thread
From: Kalle Valo @ 2011-10-03 11:08 UTC (permalink / raw)
  To: Vasanthakumar Thiagarajan; +Cc: linux-wireless

Thanks, patches 1,2,4 and 5 applied. For the patch 3 I took V2 which you
sent separately.

Kalle

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 4/5] ath6kl: Fix possible race in accessing bounce buffer
  2011-09-30 13:48 ` [PATCH 4/5] ath6kl: Fix possible race in accessing bounce buffer Vasanthakumar Thiagarajan
@ 2011-10-03 15:52   ` Thiagarajan, Vasanthakumar
  2011-10-04  8:08     ` Kalle Valo
  0 siblings, 1 reply; 9+ messages in thread
From: Thiagarajan, Vasanthakumar @ 2011-10-03 15:52 UTC (permalink / raw)
  To: Valo, Kalle; +Cc: linux-wireless


________________________________________
From: Thiagarajan, Vasanthakumar
Sent: Friday, September 30, 2011 7:18 PM
To: Valo, Kalle
Cc: linux-wireless@vger.kernel.org
Subject: [PATCH 4/5] ath6kl: Fix possible race in accessing bounce buffer

There is only one bounce buffer (ar_sdio->dma_buffer) which is used
for both read and write without any protection. Fix this race by
allocating bounce buffer every time when it is needed.

As I have never seen any race in bounce buffer practically during my testing,
this patch can be dropped instead of making the code complex for no reason.

Vasanth 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/5] ath6kl: Fix possible race in accessing bounce buffer
  2011-10-03 15:52   ` Thiagarajan, Vasanthakumar
@ 2011-10-04  8:08     ` Kalle Valo
  2011-10-10  4:52       ` Vasanthakumar Thiagarajan
  0 siblings, 1 reply; 9+ messages in thread
From: Kalle Valo @ 2011-10-04  8:08 UTC (permalink / raw)
  To: Thiagarajan, Vasanthakumar; +Cc: linux-wireless

On 10/03/2011 06:52 PM, Thiagarajan, Vasanthakumar wrote:
> 
> ________________________________________
> From: Thiagarajan, Vasanthakumar
> Sent: Friday, September 30, 2011 7:18 PM
> To: Valo, Kalle
> Cc: linux-wireless@vger.kernel.org
> Subject: [PATCH 4/5] ath6kl: Fix possible race in accessing bounce buffer
> 
> There is only one bounce buffer (ar_sdio->dma_buffer) which is used
> for both read and write without any protection. Fix this race by
> allocating bounce buffer every time when it is needed.
> 
> As I have never seen any race in bounce buffer practically during my testing,
> this patch can be dropped instead of making the code complex for no reason.

Heh, we got lucky again. I had applied the patches but forgot to push
them to github. So I was able to drop patches 4 and 5.

Kalle

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/5] ath6kl: Fix possible race in accessing bounce buffer
  2011-10-04  8:08     ` Kalle Valo
@ 2011-10-10  4:52       ` Vasanthakumar Thiagarajan
  0 siblings, 0 replies; 9+ messages in thread
From: Vasanthakumar Thiagarajan @ 2011-10-10  4:52 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless

On Tue, Oct 04, 2011 at 11:08:50AM +0300, Kalle Valo wrote:
> On 10/03/2011 06:52 PM, Thiagarajan, Vasanthakumar wrote:
> > 
> > ________________________________________
> > From: Thiagarajan, Vasanthakumar
> > Sent: Friday, September 30, 2011 7:18 PM
> > To: Valo, Kalle
> > Cc: linux-wireless@vger.kernel.org
> > Subject: [PATCH 4/5] ath6kl: Fix possible race in accessing bounce buffer
> > 
> > There is only one bounce buffer (ar_sdio->dma_buffer) which is used
> > for both read and write without any protection. Fix this race by
> > allocating bounce buffer every time when it is needed.
> > 
> > As I have never seen any race in bounce buffer practically during my testing,
> > this patch can be dropped instead of making the code complex for no reason.
> 
> Heh, we got lucky again. I had applied the patches but forgot to push
> them to github. So I was able to drop patches 4 and 5.

Thanks a lot.

Vasanth

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-10-10  4:53 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-30 13:48 [PATCH 1/5] ath6kl: Fix sparse warning "symbol 'conn' shadows an earlier one" Vasanthakumar Thiagarajan
2011-09-30 13:48 ` [PATCH 2/5] ath6kl: Replace spin_lock_irqsave with spin_lock_bh Vasanthakumar Thiagarajan
2011-09-30 13:48 ` [PATCH 3/5] ath6kl: Claim sdio function only at appropriate places Vasanthakumar Thiagarajan
2011-09-30 13:48 ` [PATCH 4/5] ath6kl: Fix possible race in accessing bounce buffer Vasanthakumar Thiagarajan
2011-10-03 15:52   ` Thiagarajan, Vasanthakumar
2011-10-04  8:08     ` Kalle Valo
2011-10-10  4:52       ` Vasanthakumar Thiagarajan
2011-09-30 13:48 ` [PATCH 5/5] ath6kl: Remove unused ar_sdio->dma_buffer Vasanthakumar Thiagarajan
2011-10-03 11:08 ` [PATCH 1/5] ath6kl: Fix sparse warning "symbol 'conn' shadows an earlier one" Kalle Valo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.