Re: [PATCH] ath9k: keep ANI in monitor mode

["Toke Høiland-Jørgensen" <toke@kernel.org>]

Oleksiy Protas <elfy.ua@gmail.com> writes:

> Earlier changes that made is_monitoring a separate flag as opposed to setting the opmode caused AMI to never start when monitoring
> ---
>  drivers/net/wireless/ath/ath9k/link.c | 2 +-
>  drivers/net/wireless/ath/ath9k/main.c | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c
> index 9d84003db800..cd2ead7b2883 100644
> --- a/drivers/net/wireless/ath/ath9k/link.c
> +++ b/drivers/net/wireless/ath/ath9k/link.c
> @@ -468,7 +468,7 @@ void ath_check_ani(struct ath_softc *sc)
>  			if (!test_bit(ATH_OP_PRIM_STA_VIF, &common->op_flags))
>  				goto stop_ani;
>  		}
> -	} else if (ah->opmode == NL80211_IFTYPE_STATION) {
> +	} else if (ah->opmode == NL80211_IFTYPE_STATION && !ah->is_monitoring) {
>  		if (!test_bit(ATH_OP_PRIM_STA_VIF, &common->op_flags))
>  			goto stop_ani;
>  	}

AFAICT, if you're trying to restore the old logic, ANI should always be
enabled if monitoring is. But here you're only changing the station
branch of the logic. Shouldn't it be something like:

--- i/drivers/net/wireless/ath/ath9k/link.c
+++ w/drivers/net/wireless/ath/ath9k/link.c
@@ -481,6 +481,9 @@ void ath_check_ani(struct ath_softc *sc)
 	struct ath_common *common = ath9k_hw_common(sc->sc_ah);
 	struct ath_beacon_config *cur_conf = &sc->cur_chan->beacon;
 
+	if (ah->is_monitoring)
+		goto start_ani;
+
 	/*
 	 * Check for the various conditions in which ANI has to
 	 * be stopped.
@@ -502,6 +505,7 @@ void ath_check_ani(struct ath_softc *sc)
 			goto stop_ani;
 	}
 
+start_ani:
 	if (!test_bit(ATH_OP_ANI_RUN, &common->op_flags)) {
 		set_bit(ATH_OP_ANI_RUN, &common->op_flags);
 		ath_start_ani(sc);


> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index aa271b82875e..2685aeecffad 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -1530,6 +1530,7 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
>  		if (conf->flags & IEEE80211_CONF_MONITOR) {
>  			ath_dbg(common, CONFIG, "Monitor mode is enabled\n");
>  			sc->sc_ah->is_monitoring = true;
> +			ath_check_ani(sc);
>  		} else {
>  			ath_dbg(common, CONFIG, "Monitor mode is disabled\n");
>  			sc->sc_ah->is_monitoring = false;

And shouldn't this call to ath_check_ani() be done regardless of whether
monitoring mode is being enabled or disabled? I.e., go after the else of
that if branch?

-Toke