Re: [PATCH v2 12/18] wl1271: AP mode - add STA add/remove ops

[Luciano Coelho <luciano.coelho@nokia.com>]

On Wed, 2010-12-22 at 16:27 +0200, ext Arik Nemtsov wrote:
> Allocate and free host link IDs (HLIDs) for each link. A per-STA
> data structure keeps the HLID of each STA.
> 
> Signed-off-by: Arik Nemtsov <arik@wizery.com>
> ---

[...]

> +static int wl1271_op_sta_add(struct ieee80211_hw *hw,
> +			     struct ieee80211_vif *vif,
> +			     struct ieee80211_sta *sta)
> +{
> +	struct wl1271 *wl = hw->priv;
> +	int ret = 0;
> +	u8 hlid;
> +
> +	mutex_lock(&wl->mutex);
> +
> +	if (unlikely(wl->state == WL1271_STATE_OFF))
> +		goto out;
> +
> +	if (wl->bss_type != BSS_TYPE_AP_BSS)
> +		goto out;

Should we ever bump into these two conditions? Shouldn't we warn or
return errors?


> +static int wl1271_op_sta_remove(struct ieee80211_hw *hw,
> +				struct ieee80211_vif *vif,
> +				struct ieee80211_sta *sta)
> +{
> +	struct wl1271 *wl = hw->priv;
> +	struct wl1271_station *wl_sta;
> +	int ret = 0;
> +
> +	mutex_lock(&wl->mutex);
> +
> +	if (unlikely(wl->state == WL1271_STATE_OFF))
> +		goto out;
> +
> +	if (wl->bss_type != BSS_TYPE_AP_BSS)
> +		goto out;

Same here...


> +	wl1271_debug(DEBUG_MAC80211, "mac80211 remove sta %d", (int)sta->aid);
> +
> +	ret = wl1271_ps_elp_wakeup(wl, false);
> +	if (ret < 0)
> +		goto out;
> +
> +	wl_sta = (struct wl1271_station *)sta->drv_priv;
> +	ret = wl1271_cmd_remove_sta(wl, wl_sta->hlid);
> +	if (ret < 0)
> +		goto out_sleep;

Would it make sense to check if the sta is really in use (ie. it is
marked in the ap_sta_map) before trying to remove it?


-- 
Cheers,
Luca.