2011-09-15 07:57:52

by Vivek Natarajan

[permalink] [raw]
Subject: [PATCH] ath6kl: Deinitialize wiphy on error.

This fixes the panic observed on card removal.

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

diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c
index 0cce801..18154ff 100644
--- a/drivers/net/wireless/ath/ath6kl/sdio.c
+++ b/drivers/net/wireless/ath/ath6kl/sdio.c
@@ -25,6 +25,7 @@
#include "hif-ops.h"
#include "target.h"
#include "debug.h"
+#include "cfg80211.h"

struct ath6kl_sdio {
struct sdio_func *func;
@@ -816,7 +817,7 @@ static int ath6kl_sdio_probe(struct sdio_func *func,
ath6kl_err("Failed to enable 4-bit async irq mode %d\n",
ret);
sdio_release_host(func);
- goto err_dma;
+ goto err_sdio;
}

ath6kl_dbg(ATH6KL_DBG_TRC, "4-bit async irq mode enabled\n");
@@ -829,7 +830,7 @@ static int ath6kl_sdio_probe(struct sdio_func *func,

ret = ath6kl_sdio_power_on(ar_sdio);
if (ret)
- goto err_dma;
+ goto err_sdio;

sdio_claim_host(func);

@@ -853,6 +854,8 @@ static int ath6kl_sdio_probe(struct sdio_func *func,

err_off:
ath6kl_sdio_power_off(ar_sdio);
+err_sdio:
+ ath6kl_cfg80211_deinit(ar_sdio->ar);
err_dma:
kfree(ar_sdio->dma_buffer);
err_hif:
--
1.7.1



2011-09-15 14:02:45

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath6kl: Deinitialize wiphy on error.

Preferable style for the subject is:

ath6kl: deinitialise wiphy on error

On 09/15/2011 10:57 AM, Vivek Natarajan wrote:
> This fixes the panic observed on card removal.

Please add information about the crash. For example, you could copy the
stack trace.

> @@ -853,6 +854,8 @@ static int ath6kl_sdio_probe(struct sdio_func *func,
>
> err_off:
> ath6kl_sdio_power_off(ar_sdio);
> +err_sdio:
> + ath6kl_cfg80211_deinit(ar_sdio->ar);
> err_dma:
> kfree(ar_sdio->dma_buffer);
> err_hif:

err_cfg80211, or something like that, would be a better name for the label.

But this change actually has an architectural issue. The idea is that
sdio.c only calls functions either from hif.h or core.h. It should not
use any functions from cfg80211.c.

A better fix would be to add new set of functions:

ath6kl_core_free() which can be called after ath6kl_core_alloc() is executed

ath6kl_core_cleanup() which can be called after ath6kl_core_init() is
executed. This would replace the current ath6kl_destroy() function.

But I can do the architecture change because it's more work. Please
address the minor issues I commented above and then I can take the patch.

Kalle