From: Kalle Valo <kvalo@qca.qualcomm.com> To: Michal Kazior <michal.kazior@tieto.com> Cc: <ath10k@lists.infradead.org>, <linux-wireless@vger.kernel.org> Subject: Re: [PATCH] ath10k: fix initial radar detection logic Date: Wed, 2 Apr 2014 08:27:48 +0300 [thread overview] Message-ID: <87eh1gs3l7.fsf@kamboji.qca.qualcomm.com> (raw) In-Reply-To: <1396337123-12622-1-git-send-email-michal.kazior@tieto.com> (Michal Kazior's message of "Tue, 1 Apr 2014 09:25:23 +0200") Michal Kazior <michal.kazior@tieto.com> 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. > 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@tieto.com> [...] > --- 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; > --- 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? > @@ -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. -- Kalle Valo
WARNING: multiple messages have this Message-ID (diff)
From: Kalle Valo <kvalo@qca.qualcomm.com> To: Michal Kazior <michal.kazior@tieto.com> Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org Subject: Re: [PATCH] ath10k: fix initial radar detection logic Date: Wed, 2 Apr 2014 08:27:48 +0300 [thread overview] Message-ID: <87eh1gs3l7.fsf@kamboji.qca.qualcomm.com> (raw) In-Reply-To: <1396337123-12622-1-git-send-email-michal.kazior@tieto.com> (Michal Kazior's message of "Tue, 1 Apr 2014 09:25:23 +0200") Michal Kazior <michal.kazior@tieto.com> 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. > 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@tieto.com> [...] > --- 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; > --- 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? > @@ -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. -- Kalle Valo _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
next prev parent reply other threads:[~2014-04-02 5:27 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <1396015934-7723-1-git-send-email-michal.kazior@tieto.com> 2014-04-01 7:25 ` [PATCH] ath10k: fix initial radar detection logic Michal Kazior 2014-04-01 7:25 ` Michal Kazior 2014-04-02 5:27 ` Kalle Valo [this message] 2014-04-02 5:27 ` Kalle Valo 2014-04-02 5:48 ` Michal Kazior 2014-04-02 5:48 ` Michal Kazior 2014-04-04 11:28 ` [PATCH v2 0/2] ath10k: fix radar/cac Michal Kazior 2014-04-04 11:28 ` Michal Kazior 2014-04-04 11:28 ` [PATCH v2 1/2] ath10k: reorder functions Michal Kazior 2014-04-04 11:28 ` Michal Kazior 2014-04-07 13:52 ` Kalle Valo 2014-04-07 13:52 ` Kalle Valo 2014-04-08 6:07 ` Michal Kazior 2014-04-08 6:07 ` Michal Kazior 2014-04-04 11:28 ` [PATCH v2 2/2] ath10k: refactor radar detection code Michal Kazior 2014-04-04 11:28 ` Michal Kazior 2014-04-07 14:04 ` Kalle Valo 2014-04-07 14:04 ` Kalle Valo 2014-04-08 6:32 ` Michal Kazior 2014-04-08 6:32 ` Michal Kazior 2014-04-08 7:04 ` Kalle Valo 2014-04-08 7:04 ` Kalle Valo 2014-04-11 5:25 ` [PATCH v2 0/2] ath10k: fix radar/cac Kalle Valo 2014-04-11 5:25 ` Kalle Valo
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=87eh1gs3l7.fsf@kamboji.qca.qualcomm.com \ --to=kvalo@qca.qualcomm.com \ --cc=ath10k@lists.infradead.org \ --cc=linux-wireless@vger.kernel.org \ --cc=michal.kazior@tieto.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.