Re: [PATCH 05/40] wl12xx: remove rx filtering stuff

[Luciano Coelho <coelho@ti.com>]

On Tue, 2011-08-09 at 12:13 +0300, Eliad Peller wrote: 
> The new fw doesn't support rx_filtering configuration (as a
> stand-alone command. the rx filtering is done automatically
> according to the active role).
> 
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
>  drivers/net/wireless/wl12xx/acx.c     |   28 --------
>  drivers/net/wireless/wl12xx/acx.h     |  118 ---------------------------------
>  drivers/net/wireless/wl12xx/boot.c    |    3 -
>  drivers/net/wireless/wl12xx/cmd.c     |    4 -
>  drivers/net/wireless/wl12xx/debugfs.c |    3 -
>  drivers/net/wireless/wl12xx/init.c    |   12 +---
>  drivers/net/wireless/wl12xx/io.h      |    1 -
>  drivers/net/wireless/wl12xx/main.c    |   62 ++----------------
>  drivers/net/wireless/wl12xx/reg.h     |   75 ---------------------
>  drivers/net/wireless/wl12xx/rx.c      |   11 ---
>  drivers/net/wireless/wl12xx/rx.h      |    1 -
>  drivers/net/wireless/wl12xx/scan.c    |    3 -
>  drivers/net/wireless/wl12xx/tx.c      |    4 +-
>  drivers/net/wireless/wl12xx/wl12xx.h  |   22 ------
>  14 files changed, 8 insertions(+), 339 deletions(-)

Yay! This looks good, lots of code could be removed! This filtering
things were a bit messy before. :)


> diff --git a/drivers/net/wireless/wl12xx/init.c b/drivers/net/wireless/wl12xx/init.c
> index c3e9a2e..7aa8fc7 100644
> --- a/drivers/net/wireless/wl12xx/init.c
> +++ b/drivers/net/wireless/wl12xx/init.c
> @@ -224,24 +224,20 @@ static int wl1271_ap_init_templates_config(struct wl1271 *wl)
>  	if (ret < 0)
>  		return ret;
>  
>  	return 0;
>  }
>  
> -static int wl1271_init_rx_config(struct wl1271 *wl, u32 config, u32 filter)
> +static int wl1271_init_rx_config(struct wl1271 *wl)

Could you change the wl1271_init_rx_config() to wl12xx_init_rx_config()?
We more or less agreed that we would start converting wl1271 to wl12xx
slowly, each time we need to change the function declaration.  It gets a
bit inconsistent, but at least we don't screw up git-blame by doing this
in one go with a huge patch.


> @@ -2425,24 +2385,17 @@ static void wl1271_op_configure_filter(struct ieee80211_hw *hw,
>  							   fp->mc_list,
>  							   fp->mc_list_length);
>  		if (ret < 0)
>  			goto out_sleep;
>  	}
>  
> -	/* determine, whether supported filter values have changed */
> -	if (changed == 0)
> -		goto out_sleep;
> -
> -	/* configure filters */
> -	wl->filters = *total;
> -	wl1271_configure_filters(wl, 0);
> -
> -	/* apply configured filters */
> -	ret = wl1271_acx_rx_config(wl, wl->rx_config, wl->rx_filter);
> -	if (ret < 0)
> -		goto out_sleep;
> +	/*
> +	 * the fw doesn't provide an api to configure the filters. instead,
> +	 * the filters configuration is based on the active roles / ROC
> +	 * state.
> +	 */

Is this really safe? I guess we should trust the firmware to set the
filters correctly, but isn't there *any* other reason why mac80211 would
want to change the filters? Just want to be sure. ;)

BTW, this patch could be before 04.  If we really include those #if 0's,
we should try to keep them for as few commits as possible.

-- 
Cheers,
Luca.