On 2 April 2014 07:27, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
> Michal Kazior <michal.kazior@xxxxxxxxx> writes:
>
>> This fixes a problem of initial radar detection
>> (CAC) being stuck and blocking Rx in some cases
>> until all interfaces were brought down.
>
> It would be good to describe more about the cases where this problem
> happened.
Ok.
>> For userspace this meant first run of hostapd
>> would perform CAC but due to filtered Rx no
>> clients would associate. Subsequent runs of
>> hostapd would not perform CAC (as it was already
>> done) and would associate clients.
>>
>> This also makes sure radar detection is performed
>> when bandwidth is widened. Before if 20MHz CAC was
>> performed then 40MHz CAC wouldn't start monitor
>> vdev effectively disabling initial radar
>> detection.
>>
>> A driver should just start/stop radar detection
>> based on hw->conf.radar_enabled. However, since
>> ath10k needs to start a monitor vdev for the
>> initial radar detection special care needs to be
>> applied.
>>
>> While at it cleanup the code a bit.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@xxxxxxxxx>
>
> [...]
>
>> --- a/drivers/net/wireless/ath/ath10k/core.h
>> +++ b/drivers/net/wireless/ath/ath10k/core.h
>> @@ -434,6 +434,8 @@ struct ath10k {
>> unsigned int filter_flags;
>> unsigned long dev_flags;
>> u32 dfs_block_radar_events;
>> + bool radar_enabled; /* protected by conf_mutex */
>> + int num_started_vdevs; /* protected by conf_mutex */
>
> I would prefer style like this:
>
> u32 dfs_block_radar_events;
>
> /* these are protected by conf_mutex */
> bool radar_enabled;
> int num_started_vdevs;
Ok.
>
>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>> @@ -489,6 +489,8 @@ static inline int ath10k_vdev_setup_sync(struct ath10k *ar)
>> return 0;
>> }
>>
>> +static void ath10k_recalc_radar_detection(struct ath10k *ar);
>
> Forward declarations should be avoided if possible. Can you add a new
> patch which just moves ath10k_recalc_radar_detection() and in this patch
> you do the modifications in the function?
The we need to move ath10k_cac_start/stop and possibly a few other
functions as well. I'll check that out.
>> @@ -571,6 +576,11 @@ static int ath10k_vdev_stop(struct ath10k_vif *arvif)
>> return ret;
>> }
>>
>> + if (WARN_ON(ar->num_started_vdevs == 0)) {
>> + ar->num_started_vdevs--;
>> + ath10k_recalc_radar_detection(ar);
>
> Now num_started_vdevs will be -1, what does that mean? It would be good
> to document that in struct ath10k.
Ah, it's missing "!" in front of the WARN_ON..
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
