[PATCH] wl1271: set wl->vif only if add_interface succeeded.

[PATCH] wl1271: set wl->vif only if add_interface succeeded.

[Eliad Peller]

set wl->vif to the newly created interface only after the firmware booted
successfully. on the way - make the function flow more clear.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/wl12xx/wl1271_main.c |   32 +++++++++++++++++-----------
 1 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
index c54887c..2599157 100644
--- a/drivers/net/wireless/wl12xx/wl1271_main.c
+++ b/drivers/net/wireless/wl12xx/wl1271_main.c
@@ -950,18 +950,18 @@ static int wl1271_op_add_interface(struct ieee80211_hw *hw,
 	struct wiphy *wiphy = hw->wiphy;
 	int retries = WL1271_BOOT_RETRIES;
 	int ret = 0;
+	bool booted = false;
 
 	wl1271_debug(DEBUG_MAC80211, "mac80211 add interface type %d mac %pM",
 		     vif->type, vif->addr);
 
 	mutex_lock(&wl->mutex);
 	if (wl->vif) {
+		wl1271_error("multiple vifs are not supported yet");
 		ret = -EBUSY;
 		goto out;
 	}
 
-	wl->vif = vif;
-
 	switch (vif->type) {
 	case NL80211_IFTYPE_STATION:
 		wl->bss_type = BSS_TYPE_STA_BSS;
@@ -999,15 +999,8 @@ static int wl1271_op_add_interface(struct ieee80211_hw *hw,
 		if (ret < 0)
 			goto irq_disable;
 
-		wl->state = WL1271_STATE_ON;
-		wl1271_info("firmware booted (%s)", wl->chip.fw_ver);
-
-		/* update hw/fw version info in wiphy struct */
-		wiphy->hw_version = wl->chip.id;
-		strncpy(wiphy->fw_version, wl->chip.fw_ver,
-			sizeof(wiphy->fw_version));
-
-		goto out;
+		booted = true;
+		break;
 
 irq_disable:
 		wl1271_disable_interrupts(wl);
@@ -1025,8 +1018,21 @@ power_off:
 		wl1271_power_off(wl);
 	}
 
-	wl1271_error("firmware boot failed despite %d retries",
-		     WL1271_BOOT_RETRIES);
+	if (!booted) {
+		wl1271_error("firmware boot failed despite %d retries",
+			     WL1271_BOOT_RETRIES);
+		goto out;
+	}
+
+	wl->vif = vif;
+	wl->state = WL1271_STATE_ON;
+	wl1271_info("firmware booted (%s)", wl->chip.fw_ver);
+
+	/* update hw/fw version info in wiphy struct */
+	wiphy->hw_version = wl->chip.id;
+	strncpy(wiphy->fw_version, wl->chip.fw_ver,
+		sizeof(wiphy->fw_version));
+
 out:
 	mutex_unlock(&wl->mutex);
 
-- 
1.7.0.4

Re: [PATCH] wl1271: set wl->vif only if add_interface succeeded.

[Luciano Coelho]

On Tue, 2010-10-26 at 17:26 +0200, ext Eliad Peller wrote:
> set wl->vif to the newly created interface only after the firmware booted
> successfully. on the way - make the function flow more clear.
> 
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---

Looks good.  Just one comment below.


>  drivers/net/wireless/wl12xx/wl1271_main.c |   32 +++++++++++++++++-----------
>  1 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
> index c54887c..2599157 100644
> --- a/drivers/net/wireless/wl12xx/wl1271_main.c
> +++ b/drivers/net/wireless/wl12xx/wl1271_main.c
> @@ -950,18 +950,18 @@ static int wl1271_op_add_interface(struct ieee80211_hw *hw,
>  	struct wiphy *wiphy = hw->wiphy;
>  	int retries = WL1271_BOOT_RETRIES;
>  	int ret = 0;
> +	bool booted = false;
>  
>  	wl1271_debug(DEBUG_MAC80211, "mac80211 add interface type %d mac %pM",
>  		     vif->type, vif->addr);
>  
>  	mutex_lock(&wl->mutex);
>  	if (wl->vif) {
> +		wl1271_error("multiple vifs are not supported yet");

I don't think we should use wl1271_error here.  This will cause a lot of
information to be printed out and look bad.  It is not an error if
someone tries to add a new vif when we already have one.  Use
wl1271_debug instead.


-- 
Cheers,
Luca.