[RFC 0/6] ath9k: DFS pattern detection

[RFC 0/6] ath9k: DFS pattern detection

[Zefir Kurtisi]

(now with patches ;))

This patch series proposes DFS pattern detection for ath9k.

With the source code for the proprietary wireless driver provided by
Atheros, the basic functionality to inspect radar pulses detected by
the HW and to post-filter false pulses has been re-implemented
for ath9k.



Zefir Kurtisi (6):
  ath9k: add DFS statistics to debugfs
  ath9k: add DFS debug flag
  ath9k: initial radar pulse detection for DFS
  ath9k: add DFS build parameter
  ath9k: enable DFS pulse detection
  ath9k: handle pulse data reported by DFS HW

 drivers/net/wireless/ath/ath.h          |   34 +++---
 drivers/net/wireless/ath/ath9k/Kconfig  |    7 +
 drivers/net/wireless/ath/ath9k/Makefile |    2 +
 drivers/net/wireless/ath/ath9k/debug.c  |   51 ++++++++
 drivers/net/wireless/ath/ath9k/debug.h  |   29 +++++
 drivers/net/wireless/ath/ath9k/dfs.c    |  192 +++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/dfs.h    |   24 ++++
 drivers/net/wireless/ath/ath9k/main.c   |   12 ++
 drivers/net/wireless/ath/ath9k/recv.c   |   22 +++-
 9 files changed, 352 insertions(+), 21 deletions(-)
 create mode 100644 drivers/net/wireless/ath/ath9k/dfs.c
 create mode 100644 drivers/net/wireless/ath/ath9k/dfs.h

-- 
1.7.4.1

[ath9k-devel] [RFC 0/6] ath9k: DFS pattern detection

[Zefir Kurtisi]

(now with patches ;))

This patch series proposes DFS pattern detection for ath9k.

With the source code for the proprietary wireless driver provided by
Atheros, the basic functionality to inspect radar pulses detected by
the HW and to post-filter false pulses has been re-implemented
for ath9k.



Zefir Kurtisi (6):
  ath9k: add DFS statistics to debugfs
  ath9k: add DFS debug flag
  ath9k: initial radar pulse detection for DFS
  ath9k: add DFS build parameter
  ath9k: enable DFS pulse detection
  ath9k: handle pulse data reported by DFS HW

 drivers/net/wireless/ath/ath.h          |   34 +++---
 drivers/net/wireless/ath/ath9k/Kconfig  |    7 +
 drivers/net/wireless/ath/ath9k/Makefile |    2 +
 drivers/net/wireless/ath/ath9k/debug.c  |   51 ++++++++
 drivers/net/wireless/ath/ath9k/debug.h  |   29 +++++
 drivers/net/wireless/ath/ath9k/dfs.c    |  192 +++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/dfs.h    |   24 ++++
 drivers/net/wireless/ath/ath9k/main.c   |   12 ++
 drivers/net/wireless/ath/ath9k/recv.c   |   22 +++-
 9 files changed, 352 insertions(+), 21 deletions(-)
 create mode 100644 drivers/net/wireless/ath/ath9k/dfs.c
 create mode 100644 drivers/net/wireless/ath/ath9k/dfs.h

-- 
1.7.4.1

[RFC 1/6] ath9k: add DFS statistics to debugfs

[Zefir Kurtisi]

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
---
 drivers/net/wireless/ath/ath9k/debug.c |   51 ++++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/debug.h |   29 ++++++++++++++++++
 2 files changed, 80 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index cdece82..6ad2266 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -89,6 +89,53 @@ static const struct file_operations fops_debug = {
 
 #endif
 
+
+#ifdef CONFIG_ATH9K_DFS
+
+#define DFS_STAT(s, p) \
+	len += snprintf(buf + len, size - len, "%28s : %10u\n", s, \
+			sc->debug.stats.dfs_stats.p);
+
+static ssize_t read_file_dfs(struct file *file, char __user *user_buf,
+			     size_t count, loff_t *ppos)
+{
+	struct ath_softc *sc = file->private_data;
+	char *buf;
+	unsigned int len = 0, size = 8000;
+	ssize_t retval = 0;
+
+	buf = kzalloc(size, GFP_KERNEL);
+	if (buf == NULL)
+		return -ENOMEM;
+
+	DFS_STAT("DFS pulses detected     ", pulses_detected);
+	DFS_STAT("Datalen discards        ", datalen_discards);
+	DFS_STAT("RSSI discards           ", rssi_discards);
+	DFS_STAT("BW info discards        ", bwinfo_discards);
+	DFS_STAT("Primary channel pulses  ", pri_phy_errors);
+	DFS_STAT("Secondary channel pulses", ext_phy_errors);
+	DFS_STAT("Dual channel pulses     ", dc_phy_errors);
+
+	if (len > size)
+		len = size;
+
+	retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+	kfree(buf);
+
+	return retval;
+}
+
+
+static const struct file_operations fops_dfs_stats = {
+	.read = read_file_dfs,
+	.open = ath9k_debugfs_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+#endif /* CONFIG_ATH9K_DFS */
+
+
+
 #define DMA_BUF_LEN 1024
 
 static ssize_t read_file_tx_chainmask(struct file *file, char __user *user_buf,
@@ -1385,6 +1432,10 @@ int ath9k_init_debug(struct ath_hw *ah)
 	debugfs_create_u32("chanbw", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
 			   &sc->chan_bw);
 
+#ifdef CONFIG_ATH9K_DFS
+	debugfs_create_file("dfs_stats", S_IRUSR, sc->debug.debugfs_phy, sc,
+			    &fops_dfs_stats);
+#endif
 	sc->debug.regidx = 0;
 	return 0;
 }
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index 4a04510..3a3c3b7 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -25,8 +25,10 @@ struct ath_buf;
 
 #ifdef CONFIG_ATH9K_DEBUGFS
 #define TX_STAT_INC(q, c) sc->debug.stats.txstats[q].c++
+#define DFS_STAT_INC(sc, c) (sc->debug.stats.dfs_stats.c++)
 #else
 #define TX_STAT_INC(q, c) do { } while (0)
+#define DFS_STAT_INC(sc, c) do { } while (0)
 #endif
 
 #ifdef CONFIG_ATH9K_DEBUGFS
@@ -171,10 +173,37 @@ struct ath_rx_stats {
 	u8 rs_antenna;
 };
 
+#ifdef CONFIG_ATH9K_DFS
+/**
+ * struct ath_dfs_stats - DFS Statistics
+ *
+ * @pulses_detected: No. of pulses detected so far
+ * @datalen_discards:  No. of pulses discarded due to invalid datalen
+ * @rssi_discards: No. of pulses discarded due to invalid RSSI
+ * @bwinfo_discards: No. of pulses discarded due to invalid BW info
+ * @pri_phy_errors: No. of pulses reported for primary channel
+ * @ext_phy_errors: No. of pulses reported for extension channel
+ * @dc_phy_errors: No. of pulses reported for primary + extension channel
+ */
+struct ath_dfs_stats {
+	u32 pulses_detected;
+	u32 datalen_discards;
+	u32 rssi_discards;
+	u32 bwinfo_discards;
+	u32 pri_phy_errors;
+	u32 ext_phy_errors;
+	u32 dc_phy_errors;
+};
+#endif
+
+
 struct ath_stats {
 	struct ath_interrupt_stats istats;
 	struct ath_tx_stats txstats[ATH9K_NUM_TX_QUEUES];
 	struct ath_rx_stats rxstats;
+#ifdef CONFIG_ATH9K_DFS
+	struct ath_dfs_stats dfs_stats;
+#endif
 };
 
 struct ath9k_debug {
-- 
1.7.4.1

[ath9k-devel] [RFC 1/6] ath9k: add DFS statistics to debugfs

[Zefir Kurtisi]

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
---
 drivers/net/wireless/ath/ath9k/debug.c |   51 ++++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/debug.h |   29 ++++++++++++++++++
 2 files changed, 80 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index cdece82..6ad2266 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -89,6 +89,53 @@ static const struct file_operations fops_debug = {
 
 #endif
 
+
+#ifdef CONFIG_ATH9K_DFS
+
+#define DFS_STAT(s, p) \
+	len += snprintf(buf + len, size - len, "%28s : %10u\n", s, \
+			sc->debug.stats.dfs_stats.p);
+
+static ssize_t read_file_dfs(struct file *file, char __user *user_buf,
+			     size_t count, loff_t *ppos)
+{
+	struct ath_softc *sc = file->private_data;
+	char *buf;
+	unsigned int len = 0, size = 8000;
+	ssize_t retval = 0;
+
+	buf = kzalloc(size, GFP_KERNEL);
+	if (buf == NULL)
+		return -ENOMEM;
+
+	DFS_STAT("DFS pulses detected     ", pulses_detected);
+	DFS_STAT("Datalen discards        ", datalen_discards);
+	DFS_STAT("RSSI discards           ", rssi_discards);
+	DFS_STAT("BW info discards        ", bwinfo_discards);
+	DFS_STAT("Primary channel pulses  ", pri_phy_errors);
+	DFS_STAT("Secondary channel pulses", ext_phy_errors);
+	DFS_STAT("Dual channel pulses     ", dc_phy_errors);
+
+	if (len > size)
+		len = size;
+
+	retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+	kfree(buf);
+
+	return retval;
+}
+
+
+static const struct file_operations fops_dfs_stats = {
+	.read = read_file_dfs,
+	.open = ath9k_debugfs_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+#endif /* CONFIG_ATH9K_DFS */
+
+
+
 #define DMA_BUF_LEN 1024
 
 static ssize_t read_file_tx_chainmask(struct file *file, char __user *user_buf,
@@ -1385,6 +1432,10 @@ int ath9k_init_debug(struct ath_hw *ah)
 	debugfs_create_u32("chanbw", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
 			   &sc->chan_bw);
 
+#ifdef CONFIG_ATH9K_DFS
+	debugfs_create_file("dfs_stats", S_IRUSR, sc->debug.debugfs_phy, sc,
+			    &fops_dfs_stats);
+#endif
 	sc->debug.regidx = 0;
 	return 0;
 }
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index 4a04510..3a3c3b7 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -25,8 +25,10 @@ struct ath_buf;
 
 #ifdef CONFIG_ATH9K_DEBUGFS
 #define TX_STAT_INC(q, c) sc->debug.stats.txstats[q].c++
+#define DFS_STAT_INC(sc, c) (sc->debug.stats.dfs_stats.c++)
 #else
 #define TX_STAT_INC(q, c) do { } while (0)
+#define DFS_STAT_INC(sc, c) do { } while (0)
 #endif
 
 #ifdef CONFIG_ATH9K_DEBUGFS
@@ -171,10 +173,37 @@ struct ath_rx_stats {
 	u8 rs_antenna;
 };
 
+#ifdef CONFIG_ATH9K_DFS
+/**
+ * struct ath_dfs_stats - DFS Statistics
+ *
+ * @pulses_detected: No. of pulses detected so far
+ * @datalen_discards:  No. of pulses discarded due to invalid datalen
+ * @rssi_discards: No. of pulses discarded due to invalid RSSI
+ * @bwinfo_discards: No. of pulses discarded due to invalid BW info
+ * @pri_phy_errors: No. of pulses reported for primary channel
+ * @ext_phy_errors: No. of pulses reported for extension channel
+ * @dc_phy_errors: No. of pulses reported for primary + extension channel
+ */
+struct ath_dfs_stats {
+	u32 pulses_detected;
+	u32 datalen_discards;
+	u32 rssi_discards;
+	u32 bwinfo_discards;
+	u32 pri_phy_errors;
+	u32 ext_phy_errors;
+	u32 dc_phy_errors;
+};
+#endif
+
+
 struct ath_stats {
 	struct ath_interrupt_stats istats;
 	struct ath_tx_stats txstats[ATH9K_NUM_TX_QUEUES];
 	struct ath_rx_stats rxstats;
+#ifdef CONFIG_ATH9K_DFS
+	struct ath_dfs_stats dfs_stats;
+#endif
 };
 
 struct ath9k_debug {
-- 
1.7.4.1

Re: [RFC 1/6] ath9k: add DFS statistics to debugfs

[Luis R. Rodriguez]

On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>
> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> ---
>  drivers/net/wireless/ath/ath9k/debug.c |   51 ++++++++++++++++++++++++++++++++
>  drivers/net/wireless/ath/ath9k/debug.h |   29 ++++++++++++++++++
>  2 files changed, 80 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
> index cdece82..6ad2266 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.c
> +++ b/drivers/net/wireless/ath/ath9k/debug.c
> @@ -89,6 +89,53 @@ static const struct file_operations fops_debug = {
>
>  #endif
>
> +
> +#ifdef CONFIG_ATH9K_DFS

This kconfig entry doesn't exist yet, so no point in referring to it
yet, you can add a patch that adds this but describe it as work in
progress or something like that and later correct the text as you move
on.

> +
> +#define DFS_STAT(s, p) \
> +       len += snprintf(buf + len, size - len, "%28s : %10u\n", s, \
> +                       sc->debug.stats.dfs_stats.p);
> +

Either rename DFS_STAT to ATH9K_DFS_STAT or undef DFS_STAT after its usage.

> +static ssize_t read_file_dfs(struct file *file, char __user *user_buf,
> +                            size_t count, loff_t *ppos)
> +{
> +       struct ath_softc *sc = file->private_data;
> +       char *buf;
> +       unsigned int len = 0, size = 8000;
> +       ssize_t retval = 0;
> +
> +       buf = kzalloc(size, GFP_KERNEL);
> +       if (buf == NULL)
> +               return -ENOMEM;
> +
> +       DFS_STAT("DFS pulses detected     ", pulses_detected);
> +       DFS_STAT("Datalen discards        ", datalen_discards);
> +       DFS_STAT("RSSI discards           ", rssi_discards);
> +       DFS_STAT("BW info discards        ", bwinfo_discards);
> +       DFS_STAT("Primary channel pulses  ", pri_phy_errors);
> +       DFS_STAT("Secondary channel pulses", ext_phy_errors);
> +       DFS_STAT("Dual channel pulses     ", dc_phy_errors);
> +
> +       if (len > size)
> +               len = size;
> +
> +       retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
> +       kfree(buf);
> +
> +       return retval;
> +}
> +
> +
> +static const struct file_operations fops_dfs_stats = {
> +       .read = read_file_dfs,
> +       .open = ath9k_debugfs_open,
> +       .owner = THIS_MODULE,
> +       .llseek = default_llseek,
> +};
> +#endif /* CONFIG_ATH9K_DFS */

I'd prefer to keep this apart into a debugfs_dfs.c but that's just me,
I would like to ensure to keep *all* DFS stat as easy to review as
possible and am not a fan of the ifdef sprinkle.

> +
> +
>  #define DMA_BUF_LEN 1024
>
>  static ssize_t read_file_tx_chainmask(struct file *file, char __user *user_buf,
> @@ -1385,6 +1432,10 @@ int ath9k_init_debug(struct ath_hw *ah)
>        debugfs_create_u32("chanbw", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
>                           &sc->chan_bw);
>
> +#ifdef CONFIG_ATH9K_DFS
> +       debugfs_create_file("dfs_stats", S_IRUSR, sc->debug.debugfs_phy, sc,
> +                           &fops_dfs_stats);
> +#endif

If you stuff the code into a file here you'd just need a caller to
ath9k_debugfs_dfs_create() or something like that.

>        sc->debug.regidx = 0;
>        return 0;
>  }
> diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
> index 4a04510..3a3c3b7 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.h
> +++ b/drivers/net/wireless/ath/ath9k/debug.h
> @@ -25,8 +25,10 @@ struct ath_buf;
>
>  #ifdef CONFIG_ATH9K_DEBUGFS
>  #define TX_STAT_INC(q, c) sc->debug.stats.txstats[q].c++
> +#define DFS_STAT_INC(sc, c) (sc->debug.stats.dfs_stats.c++)
>  #else
>  #define TX_STAT_INC(q, c) do { } while (0)
> +#define DFS_STAT_INC(sc, c) do { } while (0)

Who's using this? If no one, then why add it? That is add it only when
its in code.

>  #endif
>
>  #ifdef CONFIG_ATH9K_DEBUGFS
> @@ -171,10 +173,37 @@ struct ath_rx_stats {
>        u8 rs_antenna;
>  };
>
> +#ifdef CONFIG_ATH9K_DFS
> +/**
> + * struct ath_dfs_stats - DFS Statistics
> + *
> + * @pulses_detected: No. of pulses detected so far
> + * @datalen_discards:  No. of pulses discarded due to invalid datalen
> + * @rssi_discards: No. of pulses discarded due to invalid RSSI
> + * @bwinfo_discards: No. of pulses discarded due to invalid BW info
> + * @pri_phy_errors: No. of pulses reported for primary channel
> + * @ext_phy_errors: No. of pulses reported for extension channel
> + * @dc_phy_errors: No. of pulses reported for primary + extension channel
> + */
> +struct ath_dfs_stats {
> +       u32 pulses_detected;
> +       u32 datalen_discards;
> +       u32 rssi_discards;
> +       u32 bwinfo_discards;
> +       u32 pri_phy_errors;
> +       u32 ext_phy_errors;
> +       u32 dc_phy_errors;
> +};
> +#endif
> +
> +
>  struct ath_stats {
>        struct ath_interrupt_stats istats;
>        struct ath_tx_stats txstats[ATH9K_NUM_TX_QUEUES];
>        struct ath_rx_stats rxstats;
> +#ifdef CONFIG_ATH9K_DFS
> +       struct ath_dfs_stats dfs_stats;
> +#endif

If code used this set of stats this would give a compile error if
CONFIG_ATH9K_DEBUGFS is enabled but CONFIG_ATH9K_DFS was disabled as
the ifdef over the increment stat stuff is over CONFIG_ATH9K_DEBUGFS
and not CONFIG_ATH9K_DFS, but again, no one uses this code yet.

  Luis

[ath9k-devel] [RFC 1/6] ath9k: add DFS statistics to debugfs

[Luis R. Rodriguez]

On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>
> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> ---
> ?drivers/net/wireless/ath/ath9k/debug.c | ? 51 ++++++++++++++++++++++++++++++++
> ?drivers/net/wireless/ath/ath9k/debug.h | ? 29 ++++++++++++++++++
> ?2 files changed, 80 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
> index cdece82..6ad2266 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.c
> +++ b/drivers/net/wireless/ath/ath9k/debug.c
> @@ -89,6 +89,53 @@ static const struct file_operations fops_debug = {
>
> ?#endif
>
> +
> +#ifdef CONFIG_ATH9K_DFS

This kconfig entry doesn't exist yet, so no point in referring to it
yet, you can add a patch that adds this but describe it as work in
progress or something like that and later correct the text as you move
on.

> +
> +#define DFS_STAT(s, p) \
> + ? ? ? len += snprintf(buf + len, size - len, "%28s : %10u\n", s, \
> + ? ? ? ? ? ? ? ? ? ? ? sc->debug.stats.dfs_stats.p);
> +

Either rename DFS_STAT to ATH9K_DFS_STAT or undef DFS_STAT after its usage.

> +static ssize_t read_file_dfs(struct file *file, char __user *user_buf,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?size_t count, loff_t *ppos)
> +{
> + ? ? ? struct ath_softc *sc = file->private_data;
> + ? ? ? char *buf;
> + ? ? ? unsigned int len = 0, size = 8000;
> + ? ? ? ssize_t retval = 0;
> +
> + ? ? ? buf = kzalloc(size, GFP_KERNEL);
> + ? ? ? if (buf == NULL)
> + ? ? ? ? ? ? ? return -ENOMEM;
> +
> + ? ? ? DFS_STAT("DFS pulses detected ? ? ", pulses_detected);
> + ? ? ? DFS_STAT("Datalen discards ? ? ? ?", datalen_discards);
> + ? ? ? DFS_STAT("RSSI discards ? ? ? ? ? ", rssi_discards);
> + ? ? ? DFS_STAT("BW info discards ? ? ? ?", bwinfo_discards);
> + ? ? ? DFS_STAT("Primary channel pulses ?", pri_phy_errors);
> + ? ? ? DFS_STAT("Secondary channel pulses", ext_phy_errors);
> + ? ? ? DFS_STAT("Dual channel pulses ? ? ", dc_phy_errors);
> +
> + ? ? ? if (len > size)
> + ? ? ? ? ? ? ? len = size;
> +
> + ? ? ? retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
> + ? ? ? kfree(buf);
> +
> + ? ? ? return retval;
> +}
> +
> +
> +static const struct file_operations fops_dfs_stats = {
> + ? ? ? .read = read_file_dfs,
> + ? ? ? .open = ath9k_debugfs_open,
> + ? ? ? .owner = THIS_MODULE,
> + ? ? ? .llseek = default_llseek,
> +};
> +#endif /* CONFIG_ATH9K_DFS */

I'd prefer to keep this apart into a debugfs_dfs.c but that's just me,
I would like to ensure to keep *all* DFS stat as easy to review as
possible and am not a fan of the ifdef sprinkle.

> +
> +
> ?#define DMA_BUF_LEN 1024
>
> ?static ssize_t read_file_tx_chainmask(struct file *file, char __user *user_buf,
> @@ -1385,6 +1432,10 @@ int ath9k_init_debug(struct ath_hw *ah)
> ? ? ? ?debugfs_create_u32("chanbw", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
> ? ? ? ? ? ? ? ? ? ? ? ? ? &sc->chan_bw);
>
> +#ifdef CONFIG_ATH9K_DFS
> + ? ? ? debugfs_create_file("dfs_stats", S_IRUSR, sc->debug.debugfs_phy, sc,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? &fops_dfs_stats);
> +#endif

If you stuff the code into a file here you'd just need a caller to
ath9k_debugfs_dfs_create() or something like that.

> ? ? ? ?sc->debug.regidx = 0;
> ? ? ? ?return 0;
> ?}
> diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
> index 4a04510..3a3c3b7 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.h
> +++ b/drivers/net/wireless/ath/ath9k/debug.h
> @@ -25,8 +25,10 @@ struct ath_buf;
>
> ?#ifdef CONFIG_ATH9K_DEBUGFS
> ?#define TX_STAT_INC(q, c) sc->debug.stats.txstats[q].c++
> +#define DFS_STAT_INC(sc, c) (sc->debug.stats.dfs_stats.c++)
> ?#else
> ?#define TX_STAT_INC(q, c) do { } while (0)
> +#define DFS_STAT_INC(sc, c) do { } while (0)

Who's using this? If no one, then why add it? That is add it only when
its in code.

> ?#endif
>
> ?#ifdef CONFIG_ATH9K_DEBUGFS
> @@ -171,10 +173,37 @@ struct ath_rx_stats {
> ? ? ? ?u8 rs_antenna;
> ?};
>
> +#ifdef CONFIG_ATH9K_DFS
> +/**
> + * struct ath_dfs_stats - DFS Statistics
> + *
> + * @pulses_detected: No. of pulses detected so far
> + * @datalen_discards: ?No. of pulses discarded due to invalid datalen
> + * @rssi_discards: No. of pulses discarded due to invalid RSSI
> + * @bwinfo_discards: No. of pulses discarded due to invalid BW info
> + * @pri_phy_errors: No. of pulses reported for primary channel
> + * @ext_phy_errors: No. of pulses reported for extension channel
> + * @dc_phy_errors: No. of pulses reported for primary + extension channel
> + */
> +struct ath_dfs_stats {
> + ? ? ? u32 pulses_detected;
> + ? ? ? u32 datalen_discards;
> + ? ? ? u32 rssi_discards;
> + ? ? ? u32 bwinfo_discards;
> + ? ? ? u32 pri_phy_errors;
> + ? ? ? u32 ext_phy_errors;
> + ? ? ? u32 dc_phy_errors;
> +};
> +#endif
> +
> +
> ?struct ath_stats {
> ? ? ? ?struct ath_interrupt_stats istats;
> ? ? ? ?struct ath_tx_stats txstats[ATH9K_NUM_TX_QUEUES];
> ? ? ? ?struct ath_rx_stats rxstats;
> +#ifdef CONFIG_ATH9K_DFS
> + ? ? ? struct ath_dfs_stats dfs_stats;
> +#endif

If code used this set of stats this would give a compile error if
CONFIG_ATH9K_DEBUGFS is enabled but CONFIG_ATH9K_DFS was disabled as
the ifdef over the increment stat stuff is over CONFIG_ATH9K_DEBUGFS
and not CONFIG_ATH9K_DFS, but again, no one uses this code yet.

  Luis

Re: [RFC 1/6] ath9k: add DFS statistics to debugfs

[Zefir Kurtisi]

On 10/03/2011 08:14 PM, Luis R. Rodriguez wrote:
> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>
>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>> ---
>>  drivers/net/wireless/ath/ath9k/debug.c |   51 ++++++++++++++++++++++++++++++++
>>  drivers/net/wireless/ath/ath9k/debug.h |   29 ++++++++++++++++++
>>  2 files changed, 80 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
>> index cdece82..6ad2266 100644
>> --- a/drivers/net/wireless/ath/ath9k/debug.c
>> +++ b/drivers/net/wireless/ath/ath9k/debug.c
>> @@ -89,6 +89,53 @@ static const struct file_operations fops_debug = {
>>
>>  #endif
>>
>> +
>> +#ifdef CONFIG_ATH9K_DFS
> 
> This kconfig entry doesn't exist yet, so no point in referring to it
> yet, you can add a patch that adds this but describe it as work in
> progress or something like that and later correct the text as you move
> on.
> 
True, but the Kconfig entry is introduced in 4/6 of this series. I am not aware that the ordering of patches within a series is relevant, as long as it compiles after each additionally applied patch. But sure I can reorder, if that solves your concern.
>> +
>> +#define DFS_STAT(s, p) \
>> +       len += snprintf(buf + len, size - len, "%28s : %10u\n", s, \
>> +                       sc->debug.stats.dfs_stats.p);
>> +
> 
> Either rename DFS_STAT to ATH9K_DFS_STAT or undef DFS_STAT after its usage.
> 
Ok.
>> +static ssize_t read_file_dfs(struct file *file, char __user *user_buf,
>> +                            size_t count, loff_t *ppos)
>> +{
>> +       struct ath_softc *sc = file->private_data;
>> +       char *buf;
>> +       unsigned int len = 0, size = 8000;
>> +       ssize_t retval = 0;
>> +
>> +       buf = kzalloc(size, GFP_KERNEL);
>> +       if (buf == NULL)
>> +               return -ENOMEM;
>> +
>> +       DFS_STAT("DFS pulses detected     ", pulses_detected);
>> +       DFS_STAT("Datalen discards        ", datalen_discards);
>> +       DFS_STAT("RSSI discards           ", rssi_discards);
>> +       DFS_STAT("BW info discards        ", bwinfo_discards);
>> +       DFS_STAT("Primary channel pulses  ", pri_phy_errors);
>> +       DFS_STAT("Secondary channel pulses", ext_phy_errors);
>> +       DFS_STAT("Dual channel pulses     ", dc_phy_errors);
>> +
>> +       if (len > size)
>> +               len = size;
>> +
>> +       retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
>> +       kfree(buf);
>> +
>> +       return retval;
>> +}
>> +
>> +
>> +static const struct file_operations fops_dfs_stats = {
>> +       .read = read_file_dfs,
>> +       .open = ath9k_debugfs_open,
>> +       .owner = THIS_MODULE,
>> +       .llseek = default_llseek,
>> +};
>> +#endif /* CONFIG_ATH9K_DFS */
> 
> I'd prefer to keep this apart into a debugfs_dfs.c but that's just me,
> I would like to ensure to keep *all* DFS stat as easy to review as
> possible and am not a fan of the ifdef sprinkle.
> 
Ok.
>> +
>> +
>>  #define DMA_BUF_LEN 1024
>>
>>  static ssize_t read_file_tx_chainmask(struct file *file, char __user *user_buf,
>> @@ -1385,6 +1432,10 @@ int ath9k_init_debug(struct ath_hw *ah)
>>        debugfs_create_u32("chanbw", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
>>                           &sc->chan_bw);
>>
>> +#ifdef CONFIG_ATH9K_DFS
>> +       debugfs_create_file("dfs_stats", S_IRUSR, sc->debug.debugfs_phy, sc,
>> +                           &fops_dfs_stats);
>> +#endif
> 
> If you stuff the code into a file here you'd just need a caller to
> ath9k_debugfs_dfs_create() or something like that.
> 
Ok.
>>        sc->debug.regidx = 0;
>>        return 0;
>>  }
>> diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
>> index 4a04510..3a3c3b7 100644
>> --- a/drivers/net/wireless/ath/ath9k/debug.h
>> +++ b/drivers/net/wireless/ath/ath9k/debug.h
>> @@ -25,8 +25,10 @@ struct ath_buf;
>>
>>  #ifdef CONFIG_ATH9K_DEBUGFS
>>  #define TX_STAT_INC(q, c) sc->debug.stats.txstats[q].c++
>> +#define DFS_STAT_INC(sc, c) (sc->debug.stats.dfs_stats.c++)
>>  #else
>>  #define TX_STAT_INC(q, c) do { } while (0)
>> +#define DFS_STAT_INC(sc, c) do { } while (0)
> 
> Who's using this? If no one, then why add it? That is add it only when
> its in code.
> 
It is used in 3/6, see reordering issue above.
>>  #endif
>>
>>  #ifdef CONFIG_ATH9K_DEBUGFS
>> @@ -171,10 +173,37 @@ struct ath_rx_stats {
>>        u8 rs_antenna;
>>  };
>>
>> +#ifdef CONFIG_ATH9K_DFS
>> +/**
>> + * struct ath_dfs_stats - DFS Statistics
>> + *
>> + * @pulses_detected: No. of pulses detected so far
>> + * @datalen_discards:  No. of pulses discarded due to invalid datalen
>> + * @rssi_discards: No. of pulses discarded due to invalid RSSI
>> + * @bwinfo_discards: No. of pulses discarded due to invalid BW info
>> + * @pri_phy_errors: No. of pulses reported for primary channel
>> + * @ext_phy_errors: No. of pulses reported for extension channel
>> + * @dc_phy_errors: No. of pulses reported for primary + extension channel
>> + */
>> +struct ath_dfs_stats {
>> +       u32 pulses_detected;
>> +       u32 datalen_discards;
>> +       u32 rssi_discards;
>> +       u32 bwinfo_discards;
>> +       u32 pri_phy_errors;
>> +       u32 ext_phy_errors;
>> +       u32 dc_phy_errors;
>> +};
>> +#endif
>> +
>> +
>>  struct ath_stats {
>>        struct ath_interrupt_stats istats;
>>        struct ath_tx_stats txstats[ATH9K_NUM_TX_QUEUES];
>>        struct ath_rx_stats rxstats;
>> +#ifdef CONFIG_ATH9K_DFS
>> +       struct ath_dfs_stats dfs_stats;
>> +#endif
> 
> If code used this set of stats this would give a compile error if
> CONFIG_ATH9K_DEBUGFS is enabled but CONFIG_ATH9K_DFS was disabled as
> the ifdef over the increment stat stuff is over CONFIG_ATH9K_DEBUGFS
> and not CONFIG_ATH9K_DFS, but again, no one uses this code yet.
> 
True, will be fixed.

>   Luis

Thanks,
Zefir

[ath9k-devel] [RFC 1/6] ath9k: add DFS statistics to debugfs

[Zefir Kurtisi]

On 10/03/2011 08:14 PM, Luis R. Rodriguez wrote:
> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>
>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>> ---
>>  drivers/net/wireless/ath/ath9k/debug.c |   51 ++++++++++++++++++++++++++++++++
>>  drivers/net/wireless/ath/ath9k/debug.h |   29 ++++++++++++++++++
>>  2 files changed, 80 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
>> index cdece82..6ad2266 100644
>> --- a/drivers/net/wireless/ath/ath9k/debug.c
>> +++ b/drivers/net/wireless/ath/ath9k/debug.c
>> @@ -89,6 +89,53 @@ static const struct file_operations fops_debug = {
>>
>>  #endif
>>
>> +
>> +#ifdef CONFIG_ATH9K_DFS
> 
> This kconfig entry doesn't exist yet, so no point in referring to it
> yet, you can add a patch that adds this but describe it as work in
> progress or something like that and later correct the text as you move
> on.
> 
True, but the Kconfig entry is introduced in 4/6 of this series. I am not aware that the ordering of patches within a series is relevant, as long as it compiles after each additionally applied patch. But sure I can reorder, if that solves your concern.
>> +
>> +#define DFS_STAT(s, p) \
>> +       len += snprintf(buf + len, size - len, "%28s : %10u\n", s, \
>> +                       sc->debug.stats.dfs_stats.p);
>> +
> 
> Either rename DFS_STAT to ATH9K_DFS_STAT or undef DFS_STAT after its usage.
> 
Ok.
>> +static ssize_t read_file_dfs(struct file *file, char __user *user_buf,
>> +                            size_t count, loff_t *ppos)
>> +{
>> +       struct ath_softc *sc = file->private_data;
>> +       char *buf;
>> +       unsigned int len = 0, size = 8000;
>> +       ssize_t retval = 0;
>> +
>> +       buf = kzalloc(size, GFP_KERNEL);
>> +       if (buf == NULL)
>> +               return -ENOMEM;
>> +
>> +       DFS_STAT("DFS pulses detected     ", pulses_detected);
>> +       DFS_STAT("Datalen discards        ", datalen_discards);
>> +       DFS_STAT("RSSI discards           ", rssi_discards);
>> +       DFS_STAT("BW info discards        ", bwinfo_discards);
>> +       DFS_STAT("Primary channel pulses  ", pri_phy_errors);
>> +       DFS_STAT("Secondary channel pulses", ext_phy_errors);
>> +       DFS_STAT("Dual channel pulses     ", dc_phy_errors);
>> +
>> +       if (len > size)
>> +               len = size;
>> +
>> +       retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
>> +       kfree(buf);
>> +
>> +       return retval;
>> +}
>> +
>> +
>> +static const struct file_operations fops_dfs_stats = {
>> +       .read = read_file_dfs,
>> +       .open = ath9k_debugfs_open,
>> +       .owner = THIS_MODULE,
>> +       .llseek = default_llseek,
>> +};
>> +#endif /* CONFIG_ATH9K_DFS */
> 
> I'd prefer to keep this apart into a debugfs_dfs.c but that's just me,
> I would like to ensure to keep *all* DFS stat as easy to review as
> possible and am not a fan of the ifdef sprinkle.
> 
Ok.
>> +
>> +
>>  #define DMA_BUF_LEN 1024
>>
>>  static ssize_t read_file_tx_chainmask(struct file *file, char __user *user_buf,
>> @@ -1385,6 +1432,10 @@ int ath9k_init_debug(struct ath_hw *ah)
>>        debugfs_create_u32("chanbw", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
>>                           &sc->chan_bw);
>>
>> +#ifdef CONFIG_ATH9K_DFS
>> +       debugfs_create_file("dfs_stats", S_IRUSR, sc->debug.debugfs_phy, sc,
>> +                           &fops_dfs_stats);
>> +#endif
> 
> If you stuff the code into a file here you'd just need a caller to
> ath9k_debugfs_dfs_create() or something like that.
> 
Ok.
>>        sc->debug.regidx = 0;
>>        return 0;
>>  }
>> diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
>> index 4a04510..3a3c3b7 100644
>> --- a/drivers/net/wireless/ath/ath9k/debug.h
>> +++ b/drivers/net/wireless/ath/ath9k/debug.h
>> @@ -25,8 +25,10 @@ struct ath_buf;
>>
>>  #ifdef CONFIG_ATH9K_DEBUGFS
>>  #define TX_STAT_INC(q, c) sc->debug.stats.txstats[q].c++
>> +#define DFS_STAT_INC(sc, c) (sc->debug.stats.dfs_stats.c++)
>>  #else
>>  #define TX_STAT_INC(q, c) do { } while (0)
>> +#define DFS_STAT_INC(sc, c) do { } while (0)
> 
> Who's using this? If no one, then why add it? That is add it only when
> its in code.
> 
It is used in 3/6, see reordering issue above.
>>  #endif
>>
>>  #ifdef CONFIG_ATH9K_DEBUGFS
>> @@ -171,10 +173,37 @@ struct ath_rx_stats {
>>        u8 rs_antenna;
>>  };
>>
>> +#ifdef CONFIG_ATH9K_DFS
>> +/**
>> + * struct ath_dfs_stats - DFS Statistics
>> + *
>> + * @pulses_detected: No. of pulses detected so far
>> + * @datalen_discards:  No. of pulses discarded due to invalid datalen
>> + * @rssi_discards: No. of pulses discarded due to invalid RSSI
>> + * @bwinfo_discards: No. of pulses discarded due to invalid BW info
>> + * @pri_phy_errors: No. of pulses reported for primary channel
>> + * @ext_phy_errors: No. of pulses reported for extension channel
>> + * @dc_phy_errors: No. of pulses reported for primary + extension channel
>> + */
>> +struct ath_dfs_stats {
>> +       u32 pulses_detected;
>> +       u32 datalen_discards;
>> +       u32 rssi_discards;
>> +       u32 bwinfo_discards;
>> +       u32 pri_phy_errors;
>> +       u32 ext_phy_errors;
>> +       u32 dc_phy_errors;
>> +};
>> +#endif
>> +
>> +
>>  struct ath_stats {
>>        struct ath_interrupt_stats istats;
>>        struct ath_tx_stats txstats[ATH9K_NUM_TX_QUEUES];
>>        struct ath_rx_stats rxstats;
>> +#ifdef CONFIG_ATH9K_DFS
>> +       struct ath_dfs_stats dfs_stats;
>> +#endif
> 
> If code used this set of stats this would give a compile error if
> CONFIG_ATH9K_DEBUGFS is enabled but CONFIG_ATH9K_DFS was disabled as
> the ifdef over the increment stat stuff is over CONFIG_ATH9K_DEBUGFS
> and not CONFIG_ATH9K_DFS, but again, no one uses this code yet.
> 
True, will be fixed.

>   Luis

Thanks,
Zefir

[RFC 2/6] ath9k: add DFS debug flag

[Zefir Kurtisi]

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
---
 drivers/net/wireless/ath/ath.h |   34 ++++++++++++++++++----------------
 1 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index 6d56532..34d4da1 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -211,6 +211,7 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
  * @ATH_DBG_HWTIMER: hardware timer handling
  * @ATH_DBG_BTCOEX: bluetooth coexistance
  * @ATH_DBG_BSTUCK: stuck beacons
+ * @ATH_DBG_DFS: radar datection
  * @ATH_DBG_ANY: enable all debugging
  *
  * The debug level is used to control the amount and type of debugging output
@@ -220,22 +221,23 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
  * entry.
  */
 enum ATH_DEBUG {
-	ATH_DBG_RESET		= 0x00000001,
-	ATH_DBG_QUEUE		= 0x00000002,
-	ATH_DBG_EEPROM		= 0x00000004,
-	ATH_DBG_CALIBRATE	= 0x00000008,
-	ATH_DBG_INTERRUPT	= 0x00000010,
-	ATH_DBG_REGULATORY	= 0x00000020,
-	ATH_DBG_ANI		= 0x00000040,
-	ATH_DBG_XMIT		= 0x00000080,
-	ATH_DBG_BEACON		= 0x00000100,
-	ATH_DBG_CONFIG		= 0x00000200,
-	ATH_DBG_FATAL		= 0x00000400,
-	ATH_DBG_PS		= 0x00000800,
-	ATH_DBG_HWTIMER		= 0x00001000,
-	ATH_DBG_BTCOEX		= 0x00002000,
-	ATH_DBG_WMI		= 0x00004000,
-	ATH_DBG_BSTUCK		= 0x00008000,
+	ATH_DBG_RESET		= BIT(0),
+	ATH_DBG_QUEUE		= BIT(1),
+	ATH_DBG_EEPROM		= BIT(2),
+	ATH_DBG_CALIBRATE	= BIT(3),
+	ATH_DBG_INTERRUPT	= BIT(4),
+	ATH_DBG_REGULATORY	= BIT(5),
+	ATH_DBG_ANI		= BIT(6),
+	ATH_DBG_XMIT		= BIT(7),
+	ATH_DBG_BEACON		= BIT(8),
+	ATH_DBG_CONFIG		= BIT(9),
+	ATH_DBG_FATAL		= BIT(10),
+	ATH_DBG_PS		= BIT(11),
+	ATH_DBG_HWTIMER		= BIT(12),
+	ATH_DBG_BTCOEX		= BIT(13),
+	ATH_DBG_WMI		= BIT(14),
+	ATH_DBG_BSTUCK		= BIT(15),
+	ATH_DBG_DFS		= BIT(16),
 	ATH_DBG_ANY		= 0xffffffff
 };
 
-- 
1.7.4.1

[ath9k-devel] [RFC 2/6] ath9k: add DFS debug flag

[Zefir Kurtisi]

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
---
 drivers/net/wireless/ath/ath.h |   34 ++++++++++++++++++----------------
 1 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index 6d56532..34d4da1 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -211,6 +211,7 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
  * @ATH_DBG_HWTIMER: hardware timer handling
  * @ATH_DBG_BTCOEX: bluetooth coexistance
  * @ATH_DBG_BSTUCK: stuck beacons
+ * @ATH_DBG_DFS: radar datection
  * @ATH_DBG_ANY: enable all debugging
  *
  * The debug level is used to control the amount and type of debugging output
@@ -220,22 +221,23 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
  * entry.
  */
 enum ATH_DEBUG {
-	ATH_DBG_RESET		= 0x00000001,
-	ATH_DBG_QUEUE		= 0x00000002,
-	ATH_DBG_EEPROM		= 0x00000004,
-	ATH_DBG_CALIBRATE	= 0x00000008,
-	ATH_DBG_INTERRUPT	= 0x00000010,
-	ATH_DBG_REGULATORY	= 0x00000020,
-	ATH_DBG_ANI		= 0x00000040,
-	ATH_DBG_XMIT		= 0x00000080,
-	ATH_DBG_BEACON		= 0x00000100,
-	ATH_DBG_CONFIG		= 0x00000200,
-	ATH_DBG_FATAL		= 0x00000400,
-	ATH_DBG_PS		= 0x00000800,
-	ATH_DBG_HWTIMER		= 0x00001000,
-	ATH_DBG_BTCOEX		= 0x00002000,
-	ATH_DBG_WMI		= 0x00004000,
-	ATH_DBG_BSTUCK		= 0x00008000,
+	ATH_DBG_RESET		= BIT(0),
+	ATH_DBG_QUEUE		= BIT(1),
+	ATH_DBG_EEPROM		= BIT(2),
+	ATH_DBG_CALIBRATE	= BIT(3),
+	ATH_DBG_INTERRUPT	= BIT(4),
+	ATH_DBG_REGULATORY	= BIT(5),
+	ATH_DBG_ANI		= BIT(6),
+	ATH_DBG_XMIT		= BIT(7),
+	ATH_DBG_BEACON		= BIT(8),
+	ATH_DBG_CONFIG		= BIT(9),
+	ATH_DBG_FATAL		= BIT(10),
+	ATH_DBG_PS		= BIT(11),
+	ATH_DBG_HWTIMER		= BIT(12),
+	ATH_DBG_BTCOEX		= BIT(13),
+	ATH_DBG_WMI		= BIT(14),
+	ATH_DBG_BSTUCK		= BIT(15),
+	ATH_DBG_DFS		= BIT(16),
 	ATH_DBG_ANY		= 0xffffffff
 };
 
-- 
1.7.4.1

Re: [RFC 2/6] ath9k: add DFS debug flag

[Luis R. Rodriguez]

T24gTW9uLCBPY3QgMywgMjAxMSBhdCAzOjI5IEFNLCBaZWZpciBLdXJ0aXNpIDx6ZWZpci5rdXJ0
aXNpQG5lcmF0ZWMuY29tPiB3cm90ZToKPgo+IFNpZ25lZC1vZmYtYnk6IFplZmlyIEt1cnRpc2kg
PHplZmlyLmt1cnRpc2lAbmVyYXRlYy5jb20+Cj4gLS0tCj4gwqBkcml2ZXJzL25ldC93aXJlbGVz
cy9hdGgvYXRoLmggfCDCoCAzNCArKysrKysrKysrKysrKysrKystLS0tLS0tLS0tLS0tLS0tCj4g
wqAxIGZpbGVzIGNoYW5nZWQsIDE4IGluc2VydGlvbnMoKyksIDE2IGRlbGV0aW9ucygtKQo+Cj4g
ZGlmZiAtLWdpdCBhL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGguaCBiL2RyaXZlcnMvbmV0
L3dpcmVsZXNzL2F0aC9hdGguaAo+IGluZGV4IDZkNTY1MzIuLjM0ZDRkYTEgMTAwNjQ0Cj4gLS0t
IGEvZHJpdmVycy9uZXQvd2lyZWxlc3MvYXRoL2F0aC5oCj4gKysrIGIvZHJpdmVycy9uZXQvd2ly
ZWxlc3MvYXRoL2F0aC5oCj4gQEAgLTIxMSw2ICsyMTEsNyBAQCBhdGhfcHJpbnRrKGNvbnN0IGNo
YXIgKmxldmVsLCBzdHJ1Y3QgYXRoX2NvbW1vbiAqY29tbW9uLCBjb25zdCBjaGFyICpmbXQsIC4u
Lik7Cj4gwqAqIEBBVEhfREJHX0hXVElNRVI6IGhhcmR3YXJlIHRpbWVyIGhhbmRsaW5nCj4gwqAq
IEBBVEhfREJHX0JUQ09FWDogYmx1ZXRvb3RoIGNvZXhpc3RhbmNlCj4gwqAqIEBBVEhfREJHX0JT
VFVDSzogc3R1Y2sgYmVhY29ucwo+ICsgKiBAQVRIX0RCR19ERlM6IHJhZGFyIGRhdGVjdGlvbgo+
IMKgKiBAQVRIX0RCR19BTlk6IGVuYWJsZSBhbGwgZGVidWdnaW5nCj4gwqAqCj4gwqAqIFRoZSBk
ZWJ1ZyBsZXZlbCBpcyB1c2VkIHRvIGNvbnRyb2wgdGhlIGFtb3VudCBhbmQgdHlwZSBvZiBkZWJ1
Z2dpbmcgb3V0cHV0Cj4gQEAgLTIyMCwyMiArMjIxLDIzIEBAIGF0aF9wcmludGsoY29uc3QgY2hh
ciAqbGV2ZWwsIHN0cnVjdCBhdGhfY29tbW9uICpjb21tb24sIGNvbnN0IGNoYXIgKmZtdCwgLi4u
KTsKPiDCoCogZW50cnkuCj4gwqAqLwo+IMKgZW51bSBBVEhfREVCVUcgewo+IC0gwqAgwqAgwqAg
QVRIX0RCR19SRVNFVCDCoCDCoCDCoCDCoCDCoCA9IDB4MDAwMDAwMDEsCj4gLSDCoCDCoCDCoCBB
VEhfREJHX1FVRVVFIMKgIMKgIMKgIMKgIMKgID0gMHgwMDAwMDAwMiwKPiAtIMKgIMKgIMKgIEFU
SF9EQkdfRUVQUk9NIMKgIMKgIMKgIMKgIMKgPSAweDAwMDAwMDA0LAo+IC0gwqAgwqAgwqAgQVRI
X0RCR19DQUxJQlJBVEUgwqAgwqAgwqAgPSAweDAwMDAwMDA4LAo+IC0gwqAgwqAgwqAgQVRIX0RC
R19JTlRFUlJVUFQgwqAgwqAgwqAgPSAweDAwMDAwMDEwLAo+IC0gwqAgwqAgwqAgQVRIX0RCR19S
RUdVTEFUT1JZIMKgIMKgIMKgPSAweDAwMDAwMDIwLAo+IC0gwqAgwqAgwqAgQVRIX0RCR19BTkkg
wqAgwqAgwqAgwqAgwqAgwqAgPSAweDAwMDAwMDQwLAo+IC0gwqAgwqAgwqAgQVRIX0RCR19YTUlU
IMKgIMKgIMKgIMKgIMKgIMKgPSAweDAwMDAwMDgwLAo+IC0gwqAgwqAgwqAgQVRIX0RCR19CRUFD
T04gwqAgwqAgwqAgwqAgwqA9IDB4MDAwMDAxMDAsCj4gLSDCoCDCoCDCoCBBVEhfREJHX0NPTkZJ
RyDCoCDCoCDCoCDCoCDCoD0gMHgwMDAwMDIwMCwKPiAtIMKgIMKgIMKgIEFUSF9EQkdfRkFUQUwg
wqAgwqAgwqAgwqAgwqAgPSAweDAwMDAwNDAwLAo+IC0gwqAgwqAgwqAgQVRIX0RCR19QUyDCoCDC
oCDCoCDCoCDCoCDCoCDCoD0gMHgwMDAwMDgwMCwKPiAtIMKgIMKgIMKgIEFUSF9EQkdfSFdUSU1F
UiDCoCDCoCDCoCDCoCA9IDB4MDAwMDEwMDAsCj4gLSDCoCDCoCDCoCBBVEhfREJHX0JUQ09FWCDC
oCDCoCDCoCDCoCDCoD0gMHgwMDAwMjAwMCwKPiAtIMKgIMKgIMKgIEFUSF9EQkdfV01JIMKgIMKg
IMKgIMKgIMKgIMKgID0gMHgwMDAwNDAwMCwKPiAtIMKgIMKgIMKgIEFUSF9EQkdfQlNUVUNLIMKg
IMKgIMKgIMKgIMKgPSAweDAwMDA4MDAwLAo+ICsgwqAgwqAgwqAgQVRIX0RCR19SRVNFVCDCoCDC
oCDCoCDCoCDCoCA9IEJJVCgwKSwKPiArIMKgIMKgIMKgIEFUSF9EQkdfUVVFVUUgwqAgwqAgwqAg
wqAgwqAgPSBCSVQoMSksCj4gKyDCoCDCoCDCoCBBVEhfREJHX0VFUFJPTSDCoCDCoCDCoCDCoCDC
oD0gQklUKDIpLAo+ICsgwqAgwqAgwqAgQVRIX0RCR19DQUxJQlJBVEUgwqAgwqAgwqAgPSBCSVQo
MyksCj4gKyDCoCDCoCDCoCBBVEhfREJHX0lOVEVSUlVQVCDCoCDCoCDCoCA9IEJJVCg0KSwKPiAr
IMKgIMKgIMKgIEFUSF9EQkdfUkVHVUxBVE9SWSDCoCDCoCDCoD0gQklUKDUpLAo+ICsgwqAgwqAg
wqAgQVRIX0RCR19BTkkgwqAgwqAgwqAgwqAgwqAgwqAgPSBCSVQoNiksCj4gKyDCoCDCoCDCoCBB
VEhfREJHX1hNSVQgwqAgwqAgwqAgwqAgwqAgwqA9IEJJVCg3KSwKPiArIMKgIMKgIMKgIEFUSF9E
QkdfQkVBQ09OIMKgIMKgIMKgIMKgIMKgPSBCSVQoOCksCj4gKyDCoCDCoCDCoCBBVEhfREJHX0NP
TkZJRyDCoCDCoCDCoCDCoCDCoD0gQklUKDkpLAo+ICsgwqAgwqAgwqAgQVRIX0RCR19GQVRBTCDC
oCDCoCDCoCDCoCDCoCA9IEJJVCgxMCksCj4gKyDCoCDCoCDCoCBBVEhfREJHX1BTIMKgIMKgIMKg
IMKgIMKgIMKgIMKgPSBCSVQoMTEpLAo+ICsgwqAgwqAgwqAgQVRIX0RCR19IV1RJTUVSIMKgIMKg
IMKgIMKgID0gQklUKDEyKSwKPiArIMKgIMKgIMKgIEFUSF9EQkdfQlRDT0VYIMKgIMKgIMKgIMKg
IMKgPSBCSVQoMTMpLAo+ICsgwqAgwqAgwqAgQVRIX0RCR19XTUkgwqAgwqAgwqAgwqAgwqAgwqAg
PSBCSVQoMTQpLAo+ICsgwqAgwqAgwqAgQVRIX0RCR19CU1RVQ0sgwqAgwqAgwqAgwqAgwqA9IEJJ
VCgxNSksCj4gKyDCoCDCoCDCoCBBVEhfREJHX0RGUyDCoCDCoCDCoCDCoCDCoCDCoCA9IEJJVCgx
NiksCj4gwqAgwqAgwqAgwqBBVEhfREJHX0FOWSDCoCDCoCDCoCDCoCDCoCDCoCA9IDB4ZmZmZmZm
ZmYKClNwbGl0IHRoaXMgaW50byB0d28gcGF0Y2hlcywgb25lIHRvIGNvbnZlcnQgc3R1ZmYgdG8g
QklUKGZvbykgYW5kIHRoZQpvdGhlciBvbmUgdG8gYWRkIEFUSF9EQkdfREZTCgogIEx1aXMK

[ath9k-devel] [RFC 2/6] ath9k: add DFS debug flag

[Luis R. Rodriguez]

On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>
> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> ---
> ?drivers/net/wireless/ath/ath.h | ? 34 ++++++++++++++++++----------------
> ?1 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
> index 6d56532..34d4da1 100644
> --- a/drivers/net/wireless/ath/ath.h
> +++ b/drivers/net/wireless/ath/ath.h
> @@ -211,6 +211,7 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
> ?* @ATH_DBG_HWTIMER: hardware timer handling
> ?* @ATH_DBG_BTCOEX: bluetooth coexistance
> ?* @ATH_DBG_BSTUCK: stuck beacons
> + * @ATH_DBG_DFS: radar datection
> ?* @ATH_DBG_ANY: enable all debugging
> ?*
> ?* The debug level is used to control the amount and type of debugging output
> @@ -220,22 +221,23 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
> ?* entry.
> ?*/
> ?enum ATH_DEBUG {
> - ? ? ? ATH_DBG_RESET ? ? ? ? ? = 0x00000001,
> - ? ? ? ATH_DBG_QUEUE ? ? ? ? ? = 0x00000002,
> - ? ? ? ATH_DBG_EEPROM ? ? ? ? ?= 0x00000004,
> - ? ? ? ATH_DBG_CALIBRATE ? ? ? = 0x00000008,
> - ? ? ? ATH_DBG_INTERRUPT ? ? ? = 0x00000010,
> - ? ? ? ATH_DBG_REGULATORY ? ? ?= 0x00000020,
> - ? ? ? ATH_DBG_ANI ? ? ? ? ? ? = 0x00000040,
> - ? ? ? ATH_DBG_XMIT ? ? ? ? ? ?= 0x00000080,
> - ? ? ? ATH_DBG_BEACON ? ? ? ? ?= 0x00000100,
> - ? ? ? ATH_DBG_CONFIG ? ? ? ? ?= 0x00000200,
> - ? ? ? ATH_DBG_FATAL ? ? ? ? ? = 0x00000400,
> - ? ? ? ATH_DBG_PS ? ? ? ? ? ? ?= 0x00000800,
> - ? ? ? ATH_DBG_HWTIMER ? ? ? ? = 0x00001000,
> - ? ? ? ATH_DBG_BTCOEX ? ? ? ? ?= 0x00002000,
> - ? ? ? ATH_DBG_WMI ? ? ? ? ? ? = 0x00004000,
> - ? ? ? ATH_DBG_BSTUCK ? ? ? ? ?= 0x00008000,
> + ? ? ? ATH_DBG_RESET ? ? ? ? ? = BIT(0),
> + ? ? ? ATH_DBG_QUEUE ? ? ? ? ? = BIT(1),
> + ? ? ? ATH_DBG_EEPROM ? ? ? ? ?= BIT(2),
> + ? ? ? ATH_DBG_CALIBRATE ? ? ? = BIT(3),
> + ? ? ? ATH_DBG_INTERRUPT ? ? ? = BIT(4),
> + ? ? ? ATH_DBG_REGULATORY ? ? ?= BIT(5),
> + ? ? ? ATH_DBG_ANI ? ? ? ? ? ? = BIT(6),
> + ? ? ? ATH_DBG_XMIT ? ? ? ? ? ?= BIT(7),
> + ? ? ? ATH_DBG_BEACON ? ? ? ? ?= BIT(8),
> + ? ? ? ATH_DBG_CONFIG ? ? ? ? ?= BIT(9),
> + ? ? ? ATH_DBG_FATAL ? ? ? ? ? = BIT(10),
> + ? ? ? ATH_DBG_PS ? ? ? ? ? ? ?= BIT(11),
> + ? ? ? ATH_DBG_HWTIMER ? ? ? ? = BIT(12),
> + ? ? ? ATH_DBG_BTCOEX ? ? ? ? ?= BIT(13),
> + ? ? ? ATH_DBG_WMI ? ? ? ? ? ? = BIT(14),
> + ? ? ? ATH_DBG_BSTUCK ? ? ? ? ?= BIT(15),
> + ? ? ? ATH_DBG_DFS ? ? ? ? ? ? = BIT(16),
> ? ? ? ?ATH_DBG_ANY ? ? ? ? ? ? = 0xffffffff

Split this into two patches, one to convert stuff to BIT(foo) and the
other one to add ATH_DBG_DFS

  Luis

Re: [RFC 2/6] ath9k: add DFS debug flag

[Zefir Kurtisi]

On 10/03/2011 08:15 PM, Luis R. Rodriguez wrote:
> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>
>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>> ---
>>  drivers/net/wireless/ath/ath.h |   34 ++++++++++++++++++----------------
>>  1 files changed, 18 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
>> index 6d56532..34d4da1 100644
>> --- a/drivers/net/wireless/ath/ath.h
>> +++ b/drivers/net/wireless/ath/ath.h
>> @@ -211,6 +211,7 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
>>  * @ATH_DBG_HWTIMER: hardware timer handling
>>  * @ATH_DBG_BTCOEX: bluetooth coexistance
>>  * @ATH_DBG_BSTUCK: stuck beacons
>> + * @ATH_DBG_DFS: radar datection
>>  * @ATH_DBG_ANY: enable all debugging
>>  *
>>  * The debug level is used to control the amount and type of debugging output
>> @@ -220,22 +221,23 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
>>  * entry.
>>  */
>>  enum ATH_DEBUG {
>> -       ATH_DBG_RESET           = 0x00000001,
>> -       ATH_DBG_QUEUE           = 0x00000002,
>> -       ATH_DBG_EEPROM          = 0x00000004,
>> -       ATH_DBG_CALIBRATE       = 0x00000008,
>> -       ATH_DBG_INTERRUPT       = 0x00000010,
>> -       ATH_DBG_REGULATORY      = 0x00000020,
>> -       ATH_DBG_ANI             = 0x00000040,
>> -       ATH_DBG_XMIT            = 0x00000080,
>> -       ATH_DBG_BEACON          = 0x00000100,
>> -       ATH_DBG_CONFIG          = 0x00000200,
>> -       ATH_DBG_FATAL           = 0x00000400,
>> -       ATH_DBG_PS              = 0x00000800,
>> -       ATH_DBG_HWTIMER         = 0x00001000,
>> -       ATH_DBG_BTCOEX          = 0x00002000,
>> -       ATH_DBG_WMI             = 0x00004000,
>> -       ATH_DBG_BSTUCK          = 0x00008000,
>> +       ATH_DBG_RESET           = BIT(0),
>> +       ATH_DBG_QUEUE           = BIT(1),
>> +       ATH_DBG_EEPROM          = BIT(2),
>> +       ATH_DBG_CALIBRATE       = BIT(3),
>> +       ATH_DBG_INTERRUPT       = BIT(4),
>> +       ATH_DBG_REGULATORY      = BIT(5),
>> +       ATH_DBG_ANI             = BIT(6),
>> +       ATH_DBG_XMIT            = BIT(7),
>> +       ATH_DBG_BEACON          = BIT(8),
>> +       ATH_DBG_CONFIG          = BIT(9),
>> +       ATH_DBG_FATAL           = BIT(10),
>> +       ATH_DBG_PS              = BIT(11),
>> +       ATH_DBG_HWTIMER         = BIT(12),
>> +       ATH_DBG_BTCOEX          = BIT(13),
>> +       ATH_DBG_WMI             = BIT(14),
>> +       ATH_DBG_BSTUCK          = BIT(15),
>> +       ATH_DBG_DFS             = BIT(16),
>>        ATH_DBG_ANY             = 0xffffffff
> 
> Split this into two patches, one to convert stuff to BIT(foo) and the
> other one to add ATH_DBG_DFS
In fact, the BIT values will be reverted, since it is easier to have the hex values when setting debug mask than the bit position (for me at least).
> 
>   Luis

Thanks,
Zefir

[ath9k-devel] [RFC 2/6] ath9k: add DFS debug flag

[Zefir Kurtisi]

On 10/03/2011 08:15 PM, Luis R. Rodriguez wrote:
> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>
>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>> ---
>>  drivers/net/wireless/ath/ath.h |   34 ++++++++++++++++++----------------
>>  1 files changed, 18 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
>> index 6d56532..34d4da1 100644
>> --- a/drivers/net/wireless/ath/ath.h
>> +++ b/drivers/net/wireless/ath/ath.h
>> @@ -211,6 +211,7 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
>>  * @ATH_DBG_HWTIMER: hardware timer handling
>>  * @ATH_DBG_BTCOEX: bluetooth coexistance
>>  * @ATH_DBG_BSTUCK: stuck beacons
>> + * @ATH_DBG_DFS: radar datection
>>  * @ATH_DBG_ANY: enable all debugging
>>  *
>>  * The debug level is used to control the amount and type of debugging output
>> @@ -220,22 +221,23 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
>>  * entry.
>>  */
>>  enum ATH_DEBUG {
>> -       ATH_DBG_RESET           = 0x00000001,
>> -       ATH_DBG_QUEUE           = 0x00000002,
>> -       ATH_DBG_EEPROM          = 0x00000004,
>> -       ATH_DBG_CALIBRATE       = 0x00000008,
>> -       ATH_DBG_INTERRUPT       = 0x00000010,
>> -       ATH_DBG_REGULATORY      = 0x00000020,
>> -       ATH_DBG_ANI             = 0x00000040,
>> -       ATH_DBG_XMIT            = 0x00000080,
>> -       ATH_DBG_BEACON          = 0x00000100,
>> -       ATH_DBG_CONFIG          = 0x00000200,
>> -       ATH_DBG_FATAL           = 0x00000400,
>> -       ATH_DBG_PS              = 0x00000800,
>> -       ATH_DBG_HWTIMER         = 0x00001000,
>> -       ATH_DBG_BTCOEX          = 0x00002000,
>> -       ATH_DBG_WMI             = 0x00004000,
>> -       ATH_DBG_BSTUCK          = 0x00008000,
>> +       ATH_DBG_RESET           = BIT(0),
>> +       ATH_DBG_QUEUE           = BIT(1),
>> +       ATH_DBG_EEPROM          = BIT(2),
>> +       ATH_DBG_CALIBRATE       = BIT(3),
>> +       ATH_DBG_INTERRUPT       = BIT(4),
>> +       ATH_DBG_REGULATORY      = BIT(5),
>> +       ATH_DBG_ANI             = BIT(6),
>> +       ATH_DBG_XMIT            = BIT(7),
>> +       ATH_DBG_BEACON          = BIT(8),
>> +       ATH_DBG_CONFIG          = BIT(9),
>> +       ATH_DBG_FATAL           = BIT(10),
>> +       ATH_DBG_PS              = BIT(11),
>> +       ATH_DBG_HWTIMER         = BIT(12),
>> +       ATH_DBG_BTCOEX          = BIT(13),
>> +       ATH_DBG_WMI             = BIT(14),
>> +       ATH_DBG_BSTUCK          = BIT(15),
>> +       ATH_DBG_DFS             = BIT(16),
>>        ATH_DBG_ANY             = 0xffffffff
> 
> Split this into two patches, one to convert stuff to BIT(foo) and the
> other one to add ATH_DBG_DFS
In fact, the BIT values will be reverted, since it is easier to have the hex values when setting debug mask than the bit position (for me at least).
> 
>   Luis

Thanks,
Zefir

Re: [RFC 2/6] ath9k: add DFS debug flag

[Mohammed Shafi]

On Tue, Oct 4, 2011 at 2:01 PM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> On 10/03/2011 08:15 PM, Luis R. Rodriguez wrote:
>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>>
>>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>>> ---
>>>  drivers/net/wireless/ath/ath.h |   34 ++++++++++++++++++----------------
>>>  1 files changed, 18 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
>>> index 6d56532..34d4da1 100644
>>> --- a/drivers/net/wireless/ath/ath.h
>>> +++ b/drivers/net/wireless/ath/ath.h
>>> @@ -211,6 +211,7 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
>>>  * @ATH_DBG_HWTIMER: hardware timer handling
>>>  * @ATH_DBG_BTCOEX: bluetooth coexistance
>>>  * @ATH_DBG_BSTUCK: stuck beacons
>>> + * @ATH_DBG_DFS: radar datection
>>>  * @ATH_DBG_ANY: enable all debugging
>>>  *
>>>  * The debug level is used to control the amount and type of debugging output
>>> @@ -220,22 +221,23 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
>>>  * entry.
>>>  */
>>>  enum ATH_DEBUG {
>>> -       ATH_DBG_RESET           = 0x00000001,
>>> -       ATH_DBG_QUEUE           = 0x00000002,
>>> -       ATH_DBG_EEPROM          = 0x00000004,
>>> -       ATH_DBG_CALIBRATE       = 0x00000008,
>>> -       ATH_DBG_INTERRUPT       = 0x00000010,
>>> -       ATH_DBG_REGULATORY      = 0x00000020,
>>> -       ATH_DBG_ANI             = 0x00000040,
>>> -       ATH_DBG_XMIT            = 0x00000080,
>>> -       ATH_DBG_BEACON          = 0x00000100,
>>> -       ATH_DBG_CONFIG          = 0x00000200,
>>> -       ATH_DBG_FATAL           = 0x00000400,
>>> -       ATH_DBG_PS              = 0x00000800,
>>> -       ATH_DBG_HWTIMER         = 0x00001000,
>>> -       ATH_DBG_BTCOEX          = 0x00002000,
>>> -       ATH_DBG_WMI             = 0x00004000,
>>> -       ATH_DBG_BSTUCK          = 0x00008000,
>>> +       ATH_DBG_RESET           = BIT(0),
>>> +       ATH_DBG_QUEUE           = BIT(1),
>>> +       ATH_DBG_EEPROM          = BIT(2),
>>> +       ATH_DBG_CALIBRATE       = BIT(3),
>>> +       ATH_DBG_INTERRUPT       = BIT(4),
>>> +       ATH_DBG_REGULATORY      = BIT(5),
>>> +       ATH_DBG_ANI             = BIT(6),
>>> +       ATH_DBG_XMIT            = BIT(7),
>>> +       ATH_DBG_BEACON          = BIT(8),
>>> +       ATH_DBG_CONFIG          = BIT(9),
>>> +       ATH_DBG_FATAL           = BIT(10),
>>> +       ATH_DBG_PS              = BIT(11),
>>> +       ATH_DBG_HWTIMER         = BIT(12),
>>> +       ATH_DBG_BTCOEX          = BIT(13),
>>> +       ATH_DBG_WMI             = BIT(14),
>>> +       ATH_DBG_BSTUCK          = BIT(15),
>>> +       ATH_DBG_DFS             = BIT(16),
>>>        ATH_DBG_ANY             = 0xffffffff
>>
>> Split this into two patches, one to convert stuff to BIT(foo) and the
>> other one to add ATH_DBG_DFS
> In fact, the BIT values will be reverted, since it is easier to have the hex values when setting debug mask than the bit position (for me at least).

yes,  i agree having hex values is much better. especially when we
have a combination of 2 or 3 debug masks for debugging  issues.
thanks!

>>
>>   Luis
>
> Thanks,
> Zefir
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
shafi

[ath9k-devel] [RFC 2/6] ath9k: add DFS debug flag

[Mohammed Shafi]

On Tue, Oct 4, 2011 at 2:01 PM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> On 10/03/2011 08:15 PM, Luis R. Rodriguez wrote:
>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>>
>>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>>> ---
>>> ?drivers/net/wireless/ath/ath.h | ? 34 ++++++++++++++++++----------------
>>> ?1 files changed, 18 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
>>> index 6d56532..34d4da1 100644
>>> --- a/drivers/net/wireless/ath/ath.h
>>> +++ b/drivers/net/wireless/ath/ath.h
>>> @@ -211,6 +211,7 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
>>> ?* @ATH_DBG_HWTIMER: hardware timer handling
>>> ?* @ATH_DBG_BTCOEX: bluetooth coexistance
>>> ?* @ATH_DBG_BSTUCK: stuck beacons
>>> + * @ATH_DBG_DFS: radar datection
>>> ?* @ATH_DBG_ANY: enable all debugging
>>> ?*
>>> ?* The debug level is used to control the amount and type of debugging output
>>> @@ -220,22 +221,23 @@ ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
>>> ?* entry.
>>> ?*/
>>> ?enum ATH_DEBUG {
>>> - ? ? ? ATH_DBG_RESET ? ? ? ? ? = 0x00000001,
>>> - ? ? ? ATH_DBG_QUEUE ? ? ? ? ? = 0x00000002,
>>> - ? ? ? ATH_DBG_EEPROM ? ? ? ? ?= 0x00000004,
>>> - ? ? ? ATH_DBG_CALIBRATE ? ? ? = 0x00000008,
>>> - ? ? ? ATH_DBG_INTERRUPT ? ? ? = 0x00000010,
>>> - ? ? ? ATH_DBG_REGULATORY ? ? ?= 0x00000020,
>>> - ? ? ? ATH_DBG_ANI ? ? ? ? ? ? = 0x00000040,
>>> - ? ? ? ATH_DBG_XMIT ? ? ? ? ? ?= 0x00000080,
>>> - ? ? ? ATH_DBG_BEACON ? ? ? ? ?= 0x00000100,
>>> - ? ? ? ATH_DBG_CONFIG ? ? ? ? ?= 0x00000200,
>>> - ? ? ? ATH_DBG_FATAL ? ? ? ? ? = 0x00000400,
>>> - ? ? ? ATH_DBG_PS ? ? ? ? ? ? ?= 0x00000800,
>>> - ? ? ? ATH_DBG_HWTIMER ? ? ? ? = 0x00001000,
>>> - ? ? ? ATH_DBG_BTCOEX ? ? ? ? ?= 0x00002000,
>>> - ? ? ? ATH_DBG_WMI ? ? ? ? ? ? = 0x00004000,
>>> - ? ? ? ATH_DBG_BSTUCK ? ? ? ? ?= 0x00008000,
>>> + ? ? ? ATH_DBG_RESET ? ? ? ? ? = BIT(0),
>>> + ? ? ? ATH_DBG_QUEUE ? ? ? ? ? = BIT(1),
>>> + ? ? ? ATH_DBG_EEPROM ? ? ? ? ?= BIT(2),
>>> + ? ? ? ATH_DBG_CALIBRATE ? ? ? = BIT(3),
>>> + ? ? ? ATH_DBG_INTERRUPT ? ? ? = BIT(4),
>>> + ? ? ? ATH_DBG_REGULATORY ? ? ?= BIT(5),
>>> + ? ? ? ATH_DBG_ANI ? ? ? ? ? ? = BIT(6),
>>> + ? ? ? ATH_DBG_XMIT ? ? ? ? ? ?= BIT(7),
>>> + ? ? ? ATH_DBG_BEACON ? ? ? ? ?= BIT(8),
>>> + ? ? ? ATH_DBG_CONFIG ? ? ? ? ?= BIT(9),
>>> + ? ? ? ATH_DBG_FATAL ? ? ? ? ? = BIT(10),
>>> + ? ? ? ATH_DBG_PS ? ? ? ? ? ? ?= BIT(11),
>>> + ? ? ? ATH_DBG_HWTIMER ? ? ? ? = BIT(12),
>>> + ? ? ? ATH_DBG_BTCOEX ? ? ? ? ?= BIT(13),
>>> + ? ? ? ATH_DBG_WMI ? ? ? ? ? ? = BIT(14),
>>> + ? ? ? ATH_DBG_BSTUCK ? ? ? ? ?= BIT(15),
>>> + ? ? ? ATH_DBG_DFS ? ? ? ? ? ? = BIT(16),
>>> ? ? ? ?ATH_DBG_ANY ? ? ? ? ? ? = 0xffffffff
>>
>> Split this into two patches, one to convert stuff to BIT(foo) and the
>> other one to add ATH_DBG_DFS
> In fact, the BIT values will be reverted, since it is easier to have the hex values when setting debug mask than the bit position (for me at least).

yes,  i agree having hex values is much better. especially when we
have a combination of 2 or 3 debug masks for debugging  issues.
thanks!

>>
>> ? Luis
>
> Thanks,
> Zefir
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>



-- 
shafi

[RFC 3/6] ath9k: initial radar pulse detection for DFS

[Zefir Kurtisi]

This initial DFS module provides basic functionality to deal with radar
pulses reported by the DFS HW pattern detector.

It is based on Atheros proprietary driver sources with the core
functionality ported to ath9k to inspect pulse data and perform
plausibility checks to filter false pulses.

The numerous TODOs left include
 * checks for chirping pulses
 * differentiation between different chipsets
 * support for configuring the DFS HW
 * etc.

The output of this module are radar events ready to be feed to a
pattern detection module.

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
---
 drivers/net/wireless/ath/ath9k/dfs.c |  192 ++++++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/dfs.h |   24 ++++
 2 files changed, 216 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/wireless/ath/ath9k/dfs.c
 create mode 100644 drivers/net/wireless/ath/ath9k/dfs.h

diff --git a/drivers/net/wireless/ath/ath9k/dfs.c b/drivers/net/wireless/ath/ath9k/dfs.c
new file mode 100644
index 0000000..1fc9596
--- /dev/null
+++ b/drivers/net/wireless/ath/ath9k/dfs.c
@@ -0,0 +1,192 @@
+/*
+ * Copyright (c) 2008-2011 Atheros Communications Inc.
+ * Copyright (c) 2011 Neratec Solutions AG
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include "hw.h"
+#include "hw-ops.h"
+#include "ath9k.h"
+#include "dfs.h"
+
+
+/* pulse duration reported is scaled with 1000/800 us */
+#define AR93X_NSECS_PER_DUR 800
+static u32 dur_to_usecs(u32 dur)
+{
+	return (dur * AR93X_NSECS_PER_DUR + 500) / 1000;
+}
+
+/* internal struct to pass radar data */
+struct ath_radar_data {
+	u8 pulse_bw_info;
+	u8 rssi;
+	u8 ext_rssi;
+	u8 pulse_length_ext;
+	u8 pulse_length_pri;
+};
+
+/* TODO: move into or synchronize this with generic header
+ *       as soon as IF is defined */
+struct dfs_radar_pulse {
+	u16 freq;
+	u64 ts;
+	u32 width;
+	u8 rssi;
+};
+
+#define PRI_CH_RADAR_FOUND 0x01
+#define EXT_CH_RADAR_FOUND 0x02
+static bool postprocess_radar_event(struct ath_softc *sc,
+		struct ath_radar_data *are, struct dfs_radar_pulse *drp)
+{
+	u8 rssi;
+	u16 dur;
+
+	ath_dbg(ath9k_hw_common(sc->sc_ah), ATH_DBG_DFS,
+		"pulse_bw_info=0x%x, pri,ext len/rssi=(%u/%u, %u/%u)\n",
+		are->pulse_bw_info,
+		are->pulse_length_pri, are->rssi,
+		are->pulse_length_ext, are->ext_rssi);
+
+	/* Only the last 2 bits of the BW info are relevant, they indicate
+	 which channel the radar was detected in.*/
+	are->pulse_bw_info &= 0x03;
+
+	switch (are->pulse_bw_info) {
+	case 0:
+		/* Bogus bandwidth info received in descriptor,
+		 so ignore this PHY error */
+		DFS_STAT_INC(sc, bwinfo_discards);
+		return false;
+	case PRI_CH_RADAR_FOUND:
+		/* radar in ctrl channel */
+		dur = are->pulse_length_pri;
+		DFS_STAT_INC(sc, pri_phy_errors);
+		/* cannot use ctrl channel RSSI
+		 * if extension channel is stronger */
+		rssi = (are->ext_rssi >= (are->rssi + 3)) ? 0 : are->rssi;
+		break;
+	case EXT_CH_RADAR_FOUND:
+		/* radar in extension channel */
+		dur = are->pulse_length_ext;
+		DFS_STAT_INC(sc, ext_phy_errors);
+		/* cannot use extension channel RSSI
+		 * if control channel is stronger */
+		rssi = (are->rssi >= (are->ext_rssi + 12)) ? 0 : are->ext_rssi;
+		break;
+	case (PRI_CH_RADAR_FOUND | EXT_CH_RADAR_FOUND):
+		/*
+		 * Conducted testing, when pulse is on DC, both pri and ext
+		 * durations are reported to be same
+		 *
+		 * Radiated testing, when pulse is on DC, different pri and
+		 * ext durations are reported, so take the larger of the two
+		 * */
+		if (are->pulse_length_ext >= are->pulse_length_pri)
+			dur = are->pulse_length_ext;
+		else
+			dur = are->pulse_length_pri;
+		DFS_STAT_INC(sc, dc_phy_errors);
+
+		/* when both are present use stronger one */
+		rssi = (are->rssi < are->ext_rssi) ? are->ext_rssi : are->rssi;
+		break;
+	}
+
+	if (rssi == 0) {
+		DFS_STAT_INC(sc, rssi_discards);
+		return false;
+	}
+
+	/*
+	 * TODO: check chirping pulses
+	 */
+
+	/* convert duration to usecs */
+	drp->width = dur_to_usecs(dur);
+	drp->rssi = rssi;
+
+	DFS_STAT_INC(sc, pulses_detected);
+	return true;
+}
+
+
+/*
+ * DFS: check PHY-error for radar pulse and feed the detector
+ */
+void ath9k_dfs_process_phyerr(struct ath_softc *sc, void *data,
+		struct ath_rx_status *rs, u64 mactime)
+{
+	struct ath_radar_data ard;
+	u16 datalen;
+	char *vdata_end;
+	struct dfs_radar_pulse drp;
+	struct ath_hw *ah = sc->sc_ah;
+	struct ath_common *common = ath9k_hw_common(ah);
+
+	if ((!(rs->rs_phyerr != ATH9K_PHYERR_RADAR)) &&
+		(!(rs->rs_phyerr != ATH9K_PHYERR_FALSE_RADAR_EXT))) {
+		ath_dbg(common, ATH_DBG_DFS,
+			"Error: rs_phyer=0x%x not a radar error\n",
+			rs->rs_phyerr);
+		return;
+	}
+
+	datalen = rs->rs_datalen;
+	if (datalen == 0) {
+		DFS_STAT_INC(sc, datalen_discards);
+		return;
+	}
+
+	ard.rssi = rs->rs_rssi_ctl0;
+	ard.ext_rssi = rs->rs_rssi_ext0;
+
+	/* hardware stores this as 8 bit signed value.
+	 * we will cap it at 0 if it is a negative number
+	 */
+	if (ard.rssi & 0x80)
+		ard.rssi = 0;
+	if (ard.ext_rssi & 0x80)
+		ard.ext_rssi = 0;
+
+	vdata_end = (char *)data + datalen;
+	ard.pulse_bw_info = vdata_end[-1];
+	ard.pulse_length_ext = vdata_end[-2];
+	ard.pulse_length_pri = vdata_end[-3];
+
+	ath_dbg(common, ATH_DBG_DFS,
+		"bw_info=%d, length_pri=%d, length_ext=%d, "
+		"rssi_pri=%d, rssi_ext=%d\n",
+		ard.pulse_bw_info, ard.pulse_length_pri, ard.pulse_length_ext,
+		ard.rssi, ard.ext_rssi);
+
+	drp.freq = ah->curchan->channel;
+	drp.ts = mactime;
+	if (postprocess_radar_event(sc, &ard, &drp)) {
+		static u64 last_ts;
+		ath_dbg(common, ATH_DBG_DFS,
+			"ath9k_dfs_process_phyerr: channel=%d, ts=%llu, "
+			"width=%d, rssi=%d, delta_ts=%llu\n",
+			drp.freq, drp.ts, drp.width, drp.rssi, drp.ts-last_ts);
+		last_ts = drp.ts;
+		/*
+		 * TODO: forward pulse to pattern detector
+		 *
+		 * ieee80211_add_radar_pulse(drp.freq, drp.ts,
+		 *                           drp.width, drp.rssi);
+		 */
+	}
+}
+EXPORT_SYMBOL(ath9k_dfs_process_phyerr);
diff --git a/drivers/net/wireless/ath/ath9k/dfs.h b/drivers/net/wireless/ath/ath9k/dfs.h
new file mode 100644
index 0000000..4d95cad
--- /dev/null
+++ b/drivers/net/wireless/ath/ath9k/dfs.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2008-2011 Atheros Communications Inc.
+ * Copyright (c) 2011 Neratec Solutions AG
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef ATH9K_DFS_H
+#define ATH9K_DFS_H
+
+void ath9k_dfs_process_phyerr(struct ath_softc *sc, void *data,
+		struct ath_rx_status *rs, u64 mactime);
+
+#endif /* ATH9K_DFS_H */
-- 
1.7.4.1

[ath9k-devel] [RFC 3/6] ath9k: initial radar pulse detection for DFS

[Zefir Kurtisi]

This initial DFS module provides basic functionality to deal with radar
pulses reported by the DFS HW pattern detector.

It is based on Atheros proprietary driver sources with the core
functionality ported to ath9k to inspect pulse data and perform
plausibility checks to filter false pulses.

The numerous TODOs left include
 * checks for chirping pulses
 * differentiation between different chipsets
 * support for configuring the DFS HW
 * etc.

The output of this module are radar events ready to be feed to a
pattern detection module.

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
---
 drivers/net/wireless/ath/ath9k/dfs.c |  192 ++++++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/dfs.h |   24 ++++
 2 files changed, 216 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/wireless/ath/ath9k/dfs.c
 create mode 100644 drivers/net/wireless/ath/ath9k/dfs.h

diff --git a/drivers/net/wireless/ath/ath9k/dfs.c b/drivers/net/wireless/ath/ath9k/dfs.c
new file mode 100644
index 0000000..1fc9596
--- /dev/null
+++ b/drivers/net/wireless/ath/ath9k/dfs.c
@@ -0,0 +1,192 @@
+/*
+ * Copyright (c) 2008-2011 Atheros Communications Inc.
+ * Copyright (c) 2011 Neratec Solutions AG
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include "hw.h"
+#include "hw-ops.h"
+#include "ath9k.h"
+#include "dfs.h"
+
+
+/* pulse duration reported is scaled with 1000/800 us */
+#define AR93X_NSECS_PER_DUR 800
+static u32 dur_to_usecs(u32 dur)
+{
+	return (dur * AR93X_NSECS_PER_DUR + 500) / 1000;
+}
+
+/* internal struct to pass radar data */
+struct ath_radar_data {
+	u8 pulse_bw_info;
+	u8 rssi;
+	u8 ext_rssi;
+	u8 pulse_length_ext;
+	u8 pulse_length_pri;
+};
+
+/* TODO: move into or synchronize this with generic header
+ *       as soon as IF is defined */
+struct dfs_radar_pulse {
+	u16 freq;
+	u64 ts;
+	u32 width;
+	u8 rssi;
+};
+
+#define PRI_CH_RADAR_FOUND 0x01
+#define EXT_CH_RADAR_FOUND 0x02
+static bool postprocess_radar_event(struct ath_softc *sc,
+		struct ath_radar_data *are, struct dfs_radar_pulse *drp)
+{
+	u8 rssi;
+	u16 dur;
+
+	ath_dbg(ath9k_hw_common(sc->sc_ah), ATH_DBG_DFS,
+		"pulse_bw_info=0x%x, pri,ext len/rssi=(%u/%u, %u/%u)\n",
+		are->pulse_bw_info,
+		are->pulse_length_pri, are->rssi,
+		are->pulse_length_ext, are->ext_rssi);
+
+	/* Only the last 2 bits of the BW info are relevant, they indicate
+	 which channel the radar was detected in.*/
+	are->pulse_bw_info &= 0x03;
+
+	switch (are->pulse_bw_info) {
+	case 0:
+		/* Bogus bandwidth info received in descriptor,
+		 so ignore this PHY error */
+		DFS_STAT_INC(sc, bwinfo_discards);
+		return false;
+	case PRI_CH_RADAR_FOUND:
+		/* radar in ctrl channel */
+		dur = are->pulse_length_pri;
+		DFS_STAT_INC(sc, pri_phy_errors);
+		/* cannot use ctrl channel RSSI
+		 * if extension channel is stronger */
+		rssi = (are->ext_rssi >= (are->rssi + 3)) ? 0 : are->rssi;
+		break;
+	case EXT_CH_RADAR_FOUND:
+		/* radar in extension channel */
+		dur = are->pulse_length_ext;
+		DFS_STAT_INC(sc, ext_phy_errors);
+		/* cannot use extension channel RSSI
+		 * if control channel is stronger */
+		rssi = (are->rssi >= (are->ext_rssi + 12)) ? 0 : are->ext_rssi;
+		break;
+	case (PRI_CH_RADAR_FOUND | EXT_CH_RADAR_FOUND):
+		/*
+		 * Conducted testing, when pulse is on DC, both pri and ext
+		 * durations are reported to be same
+		 *
+		 * Radiated testing, when pulse is on DC, different pri and
+		 * ext durations are reported, so take the larger of the two
+		 * */
+		if (are->pulse_length_ext >= are->pulse_length_pri)
+			dur = are->pulse_length_ext;
+		else
+			dur = are->pulse_length_pri;
+		DFS_STAT_INC(sc, dc_phy_errors);
+
+		/* when both are present use stronger one */
+		rssi = (are->rssi < are->ext_rssi) ? are->ext_rssi : are->rssi;
+		break;
+	}
+
+	if (rssi == 0) {
+		DFS_STAT_INC(sc, rssi_discards);
+		return false;
+	}
+
+	/*
+	 * TODO: check chirping pulses
+	 */
+
+	/* convert duration to usecs */
+	drp->width = dur_to_usecs(dur);
+	drp->rssi = rssi;
+
+	DFS_STAT_INC(sc, pulses_detected);
+	return true;
+}
+
+
+/*
+ * DFS: check PHY-error for radar pulse and feed the detector
+ */
+void ath9k_dfs_process_phyerr(struct ath_softc *sc, void *data,
+		struct ath_rx_status *rs, u64 mactime)
+{
+	struct ath_radar_data ard;
+	u16 datalen;
+	char *vdata_end;
+	struct dfs_radar_pulse drp;
+	struct ath_hw *ah = sc->sc_ah;
+	struct ath_common *common = ath9k_hw_common(ah);
+
+	if ((!(rs->rs_phyerr != ATH9K_PHYERR_RADAR)) &&
+		(!(rs->rs_phyerr != ATH9K_PHYERR_FALSE_RADAR_EXT))) {
+		ath_dbg(common, ATH_DBG_DFS,
+			"Error: rs_phyer=0x%x not a radar error\n",
+			rs->rs_phyerr);
+		return;
+	}
+
+	datalen = rs->rs_datalen;
+	if (datalen == 0) {
+		DFS_STAT_INC(sc, datalen_discards);
+		return;
+	}
+
+	ard.rssi = rs->rs_rssi_ctl0;
+	ard.ext_rssi = rs->rs_rssi_ext0;
+
+	/* hardware stores this as 8 bit signed value.
+	 * we will cap it at 0 if it is a negative number
+	 */
+	if (ard.rssi & 0x80)
+		ard.rssi = 0;
+	if (ard.ext_rssi & 0x80)
+		ard.ext_rssi = 0;
+
+	vdata_end = (char *)data + datalen;
+	ard.pulse_bw_info = vdata_end[-1];
+	ard.pulse_length_ext = vdata_end[-2];
+	ard.pulse_length_pri = vdata_end[-3];
+
+	ath_dbg(common, ATH_DBG_DFS,
+		"bw_info=%d, length_pri=%d, length_ext=%d, "
+		"rssi_pri=%d, rssi_ext=%d\n",
+		ard.pulse_bw_info, ard.pulse_length_pri, ard.pulse_length_ext,
+		ard.rssi, ard.ext_rssi);
+
+	drp.freq = ah->curchan->channel;
+	drp.ts = mactime;
+	if (postprocess_radar_event(sc, &ard, &drp)) {
+		static u64 last_ts;
+		ath_dbg(common, ATH_DBG_DFS,
+			"ath9k_dfs_process_phyerr: channel=%d, ts=%llu, "
+			"width=%d, rssi=%d, delta_ts=%llu\n",
+			drp.freq, drp.ts, drp.width, drp.rssi, drp.ts-last_ts);
+		last_ts = drp.ts;
+		/*
+		 * TODO: forward pulse to pattern detector
+		 *
+		 * ieee80211_add_radar_pulse(drp.freq, drp.ts,
+		 *                           drp.width, drp.rssi);
+		 */
+	}
+}
+EXPORT_SYMBOL(ath9k_dfs_process_phyerr);
diff --git a/drivers/net/wireless/ath/ath9k/dfs.h b/drivers/net/wireless/ath/ath9k/dfs.h
new file mode 100644
index 0000000..4d95cad
--- /dev/null
+++ b/drivers/net/wireless/ath/ath9k/dfs.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2008-2011 Atheros Communications Inc.
+ * Copyright (c) 2011 Neratec Solutions AG
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef ATH9K_DFS_H
+#define ATH9K_DFS_H
+
+void ath9k_dfs_process_phyerr(struct ath_softc *sc, void *data,
+		struct ath_rx_status *rs, u64 mactime);
+
+#endif /* ATH9K_DFS_H */
-- 
1.7.4.1

Re: [RFC 3/6] ath9k: initial radar pulse detection for DFS

[Adrian Chadd]

Just out of curiousity, is there any "fast clock" on or off with the AR93xx ?

(That's one of the gotchas I came across when porting DFS code to
FreeBSD for the AR9160/AR9280.)

Thanks,


Adrian

[ath9k-devel] [RFC 3/6] ath9k: initial radar pulse detection for DFS

[Adrian Chadd]

Just out of curiousity, is there any "fast clock" on or off with the AR93xx ?

(That's one of the gotchas I came across when porting DFS code to
FreeBSD for the AR9160/AR9280.)

Thanks,


Adrian

Re: [RFC 3/6] ath9k: initial radar pulse detection for DFS

[Zefir Kurtisi]

On 10/03/2011 01:57 PM, Adrian Chadd wrote:
> Just out of curiousity, is there any "fast clock" on or off with the AR93xx ?
> 
> (That's one of the gotchas I came across when porting DFS code to
> FreeBSD for the AR9160/AR9280.)
> 
> Thanks,
> 
> 
> Adrian

There might be one, but I left it out (for now) for the sake of simplicity.

I anyhow doubt it has a practical relevance, since the pulse width is reported with usec granularity and therefore a scaling factor of 1/811 vs. 1/800 has no impact on the re-scaled value for widths up to 50 usecs.


Zefir

[ath9k-devel] [RFC 3/6] ath9k: initial radar pulse detection for DFS

[Zefir Kurtisi]

On 10/03/2011 01:57 PM, Adrian Chadd wrote:
> Just out of curiousity, is there any "fast clock" on or off with the AR93xx ?
> 
> (That's one of the gotchas I came across when porting DFS code to
> FreeBSD for the AR9160/AR9280.)
> 
> Thanks,
> 
> 
> Adrian

There might be one, but I left it out (for now) for the sake of simplicity.

I anyhow doubt it has a practical relevance, since the pulse width is reported with usec granularity and therefore a scaling factor of 1/811 vs. 1/800 has no impact on the re-scaled value for widths up to 50 usecs.


Zefir

Re: [RFC 3/6] ath9k: initial radar pulse detection for DFS

[Adrian Chadd]

On 3 October 2011 20:23, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:

> There might be one, but I left it out (for now) for the sake of simplicity.
>
> I anyhow doubt it has a practical relevance, since the pulse width is reported with usec granularity and therefore a scaling factor of 1/811 vs. 1/800 has no impact on the re-scaled value for widths up to 50 usecs.

I just remember this was a sticking point when porting the fusion code
to FreeBSD.
Without that particular fix, some of the radar patterns didn't
actually match reliably.

It's possible it doesn't matter as much for your classification code.
I'm just bringing it up to be complete. :)



Adrian

[ath9k-devel] [RFC 3/6] ath9k: initial radar pulse detection for DFS

[Adrian Chadd]

On 3 October 2011 20:23, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:

> There might be one, but I left it out (for now) for the sake of simplicity.
>
> I anyhow doubt it has a practical relevance, since the pulse width is reported with usec granularity and therefore a scaling factor of 1/811 vs. 1/800 has no impact on the re-scaled value for widths up to 50 usecs.

I just remember this was a sticking point when porting the fusion code
to FreeBSD.
Without that particular fix, some of the radar patterns didn't
actually match reliably.

It's possible it doesn't matter as much for your classification code.
I'm just bringing it up to be complete. :)



Adrian

Re: [RFC 3/6] ath9k: initial radar pulse detection for DFS

[Zefir Kurtisi]

On 10/03/2011 02:43 PM, Adrian Chadd wrote:
> On 3 October 2011 20:23, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> 
>> There might be one, but I left it out (for now) for the sake of simplicity.
>>
>> I anyhow doubt it has a practical relevance, since the pulse width is reported with usec granularity and therefore a scaling factor of 1/811 vs. 1/800 has no impact on the re-scaled value for widths up to 50 usecs.
> 
> I just remember this was a sticking point when porting the fusion code
> to FreeBSD.
> Without that particular fix, some of the radar patterns didn't
> actually match reliably.
> 
> It's possible it doesn't matter as much for your classification code.
> I'm just bringing it up to be complete. :)
> 
> 
> 
> Adrian

Thanks!

Most probably it'll end being added to the lengthy TODO list.


Zefir

[ath9k-devel] [RFC 3/6] ath9k: initial radar pulse detection for DFS

[Zefir Kurtisi]

On 10/03/2011 02:43 PM, Adrian Chadd wrote:
> On 3 October 2011 20:23, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> 
>> There might be one, but I left it out (for now) for the sake of simplicity.
>>
>> I anyhow doubt it has a practical relevance, since the pulse width is reported with usec granularity and therefore a scaling factor of 1/811 vs. 1/800 has no impact on the re-scaled value for widths up to 50 usecs.
> 
> I just remember this was a sticking point when porting the fusion code
> to FreeBSD.
> Without that particular fix, some of the radar patterns didn't
> actually match reliably.
> 
> It's possible it doesn't matter as much for your classification code.
> I'm just bringing it up to be complete. :)
> 
> 
> 
> Adrian

Thanks!

Most probably it'll end being added to the lengthy TODO list.


Zefir

Re: [RFC 3/6] ath9k: initial radar pulse detection for DFS

[Adrian Chadd]

On 3 October 2011 22:21, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:


> Most probably it'll end being added to the lengthy TODO list.

:) No worries. I've just added the freebsd version of this to
FreeBSD-HEAD. I'll add in the duration calculation function soon.

Now all we need is an ISC/GPL dual licenced packet classification
module. *wink* :)


Adrian

[ath9k-devel] [RFC 3/6] ath9k: initial radar pulse detection for DFS

[Adrian Chadd]

On 3 October 2011 22:21, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:


> Most probably it'll end being added to the lengthy TODO list.

:) No worries. I've just added the freebsd version of this to
FreeBSD-HEAD. I'll add in the duration calculation function soon.

Now all we need is an ISC/GPL dual licenced packet classification
module. *wink* :)


Adrian

[RFC 4/6] ath9k: add DFS build parameter

[Zefir Kurtisi]

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
---
 drivers/net/wireless/ath/ath9k/Kconfig  |    7 +++++++
 drivers/net/wireless/ath/ath9k/Makefile |    2 ++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
index d9c08c6..adddcca 100644
--- a/drivers/net/wireless/ath/ath9k/Kconfig
+++ b/drivers/net/wireless/ath/ath9k/Kconfig
@@ -58,6 +58,13 @@ config ATH9K_RATE_CONTROL
 	  Say Y, if you want to use the ath9k specific rate control
 	  module instead of minstrel_ht.
 
+config ATH9K_DFS
+	bool "Atheros ath9k DFS support"
+	depends on ATH9K
+	default y
+	---help---
+	  Say Y, if you want to include DFS support in ath9k.
+
 config ATH9K_HTC
        tristate "Atheros HTC based wireless cards support"
        depends on USB && MAC80211
diff --git a/drivers/net/wireless/ath/ath9k/Makefile b/drivers/net/wireless/ath/ath9k/Makefile
index 05a6fad..8b133c2 100644
--- a/drivers/net/wireless/ath/ath9k/Makefile
+++ b/drivers/net/wireless/ath/ath9k/Makefile
@@ -34,6 +34,8 @@ ath9k_hw-y:=	\
 		ar9003_eeprom.o \
 		ar9003_paprd.o
 
+ath9k_hw-$(CONFIG_ATH9K_DFS) += dfs.o
+
 obj-$(CONFIG_ATH9K_HW) += ath9k_hw.o
 
 obj-$(CONFIG_ATH9K_COMMON) += ath9k_common.o
-- 
1.7.4.1

[ath9k-devel] [RFC 4/6] ath9k: add DFS build parameter

[Zefir Kurtisi]

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
---
 drivers/net/wireless/ath/ath9k/Kconfig  |    7 +++++++
 drivers/net/wireless/ath/ath9k/Makefile |    2 ++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
index d9c08c6..adddcca 100644
--- a/drivers/net/wireless/ath/ath9k/Kconfig
+++ b/drivers/net/wireless/ath/ath9k/Kconfig
@@ -58,6 +58,13 @@ config ATH9K_RATE_CONTROL
 	  Say Y, if you want to use the ath9k specific rate control
 	  module instead of minstrel_ht.
 
+config ATH9K_DFS
+	bool "Atheros ath9k DFS support"
+	depends on ATH9K
+	default y
+	---help---
+	  Say Y, if you want to include DFS support in ath9k.
+
 config ATH9K_HTC
        tristate "Atheros HTC based wireless cards support"
        depends on USB && MAC80211
diff --git a/drivers/net/wireless/ath/ath9k/Makefile b/drivers/net/wireless/ath/ath9k/Makefile
index 05a6fad..8b133c2 100644
--- a/drivers/net/wireless/ath/ath9k/Makefile
+++ b/drivers/net/wireless/ath/ath9k/Makefile
@@ -34,6 +34,8 @@ ath9k_hw-y:=	\
 		ar9003_eeprom.o \
 		ar9003_paprd.o
 
+ath9k_hw-$(CONFIG_ATH9K_DFS) += dfs.o
+
 obj-$(CONFIG_ATH9K_HW) += ath9k_hw.o
 
 obj-$(CONFIG_ATH9K_COMMON) += ath9k_common.o
-- 
1.7.4.1

Re: [RFC 4/6] ath9k: add DFS build parameter

[Luis R. Rodriguez]

On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>
> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> ---
>  drivers/net/wireless/ath/ath9k/Kconfig  |    7 +++++++
>  drivers/net/wireless/ath/ath9k/Makefile |    2 ++
>  2 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
> index d9c08c6..adddcca 100644
> --- a/drivers/net/wireless/ath/ath9k/Kconfig
> +++ b/drivers/net/wireless/ath/ath9k/Kconfig
> @@ -58,6 +58,13 @@ config ATH9K_RATE_CONTROL
>          Say Y, if you want to use the ath9k specific rate control
>          module instead of minstrel_ht.
>
> +config ATH9K_DFS
> +       bool "Atheros ath9k DFS support"
> +       depends on ATH9K
> +       default y

At this point selecting y does nothing. Leave this patch out until
selecting "y" means something.

Default should be n, and in particular Atheros itself can only likely
commit to supporting DFS for AR9003 when it finds resources to do so
as well as properly test it, so DFS support kconfig should state this.
If someone wants to step up to completely support all bugs for older
families that is their prerogative but we cannot commit to it, due to
the regulatory considerations though unless this happens this cannot
and should not be enabled for older families in code.

  Luis

[ath9k-devel] [RFC 4/6] ath9k: add DFS build parameter

[Luis R. Rodriguez]

On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>
> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> ---
> ?drivers/net/wireless/ath/ath9k/Kconfig ?| ? ?7 +++++++
> ?drivers/net/wireless/ath/ath9k/Makefile | ? ?2 ++
> ?2 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
> index d9c08c6..adddcca 100644
> --- a/drivers/net/wireless/ath/ath9k/Kconfig
> +++ b/drivers/net/wireless/ath/ath9k/Kconfig
> @@ -58,6 +58,13 @@ config ATH9K_RATE_CONTROL
> ? ? ? ? ?Say Y, if you want to use the ath9k specific rate control
> ? ? ? ? ?module instead of minstrel_ht.
>
> +config ATH9K_DFS
> + ? ? ? bool "Atheros ath9k DFS support"
> + ? ? ? depends on ATH9K
> + ? ? ? default y

At this point selecting y does nothing. Leave this patch out until
selecting "y" means something.

Default should be n, and in particular Atheros itself can only likely
commit to supporting DFS for AR9003 when it finds resources to do so
as well as properly test it, so DFS support kconfig should state this.
If someone wants to step up to completely support all bugs for older
families that is their prerogative but we cannot commit to it, due to
the regulatory considerations though unless this happens this cannot
and should not be enabled for older families in code.

  Luis

Re: [RFC 4/6] ath9k: add DFS build parameter

[Zefir Kurtisi]

On 10/03/2011 08:26 PM, Luis R. Rodriguez wrote:
> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>
>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>> ---
>>  drivers/net/wireless/ath/ath9k/Kconfig  |    7 +++++++
>>  drivers/net/wireless/ath/ath9k/Makefile |    2 ++
>>  2 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
>> index d9c08c6..adddcca 100644
>> --- a/drivers/net/wireless/ath/ath9k/Kconfig
>> +++ b/drivers/net/wireless/ath/ath9k/Kconfig
>> @@ -58,6 +58,13 @@ config ATH9K_RATE_CONTROL
>>          Say Y, if you want to use the ath9k specific rate control
>>          module instead of minstrel_ht.
>>
>> +config ATH9K_DFS
>> +       bool "Atheros ath9k DFS support"
>> +       depends on ATH9K
>> +       default y
> 
> At this point selecting y does nothing. Leave this patch out until
> selecting "y" means something.
> 
What do you mean by 'nothing'? It allows you to select DFS as ath9k feature in your kernel configuration, or? Though, I agree that enabling it by default is not a good idea.

> Default should be n, and in particular Atheros itself can only likely
> commit to supporting DFS for AR9003 when it finds resources to do so
> as well as properly test it, so DFS support kconfig should state this.
> If someone wants to step up to completely support all bugs for older
> families that is their prerogative but we cannot commit to it, due to
> the regulatory considerations though unless this happens this cannot
> and should not be enabled for older families in code.
> 
In fact, AR9003 is the platform we are interested in. Although it seems that older chipsets do also detect pulses with this patches (AR9280 does, IIRC), I agree to limit DFS support to AR9003 (and later). This should be easily possible by setting priv_ops->set_radar_params for AR9003 only. I'll remove it for AR5008 in my v2 RFC.

>   Luis

Thanks,
Zefir

[ath9k-devel] [RFC 4/6] ath9k: add DFS build parameter

[Zefir Kurtisi]

On 10/03/2011 08:26 PM, Luis R. Rodriguez wrote:
> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>
>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>> ---
>>  drivers/net/wireless/ath/ath9k/Kconfig  |    7 +++++++
>>  drivers/net/wireless/ath/ath9k/Makefile |    2 ++
>>  2 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
>> index d9c08c6..adddcca 100644
>> --- a/drivers/net/wireless/ath/ath9k/Kconfig
>> +++ b/drivers/net/wireless/ath/ath9k/Kconfig
>> @@ -58,6 +58,13 @@ config ATH9K_RATE_CONTROL
>>          Say Y, if you want to use the ath9k specific rate control
>>          module instead of minstrel_ht.
>>
>> +config ATH9K_DFS
>> +       bool "Atheros ath9k DFS support"
>> +       depends on ATH9K
>> +       default y
> 
> At this point selecting y does nothing. Leave this patch out until
> selecting "y" means something.
> 
What do you mean by 'nothing'? It allows you to select DFS as ath9k feature in your kernel configuration, or? Though, I agree that enabling it by default is not a good idea.

> Default should be n, and in particular Atheros itself can only likely
> commit to supporting DFS for AR9003 when it finds resources to do so
> as well as properly test it, so DFS support kconfig should state this.
> If someone wants to step up to completely support all bugs for older
> families that is their prerogative but we cannot commit to it, due to
> the regulatory considerations though unless this happens this cannot
> and should not be enabled for older families in code.
> 
In fact, AR9003 is the platform we are interested in. Although it seems that older chipsets do also detect pulses with this patches (AR9280 does, IIRC), I agree to limit DFS support to AR9003 (and later). This should be easily possible by setting priv_ops->set_radar_params for AR9003 only. I'll remove it for AR5008 in my v2 RFC.

>   Luis

Thanks,
Zefir

Re: [RFC 4/6] ath9k: add DFS build parameter

[Felix Fietkau]

On 2011-10-04 11:55 AM, Zefir Kurtisi wrote:
> On 10/03/2011 08:26 PM, Luis R. Rodriguez wrote:
>>  On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi<zefir.kurtisi@neratec.com>  wrote:
>>>
>>>  Signed-off-by: Zefir Kurtisi<zefir.kurtisi@neratec.com>
>>>  ---
>>>   drivers/net/wireless/ath/ath9k/Kconfig  |    7 +++++++
>>>   drivers/net/wireless/ath/ath9k/Makefile |    2 ++
>>>   2 files changed, 9 insertions(+), 0 deletions(-)
>>>
>>>  diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
>>>  index d9c08c6..adddcca 100644
>>>  --- a/drivers/net/wireless/ath/ath9k/Kconfig
>>>  +++ b/drivers/net/wireless/ath/ath9k/Kconfig
>>>  @@ -58,6 +58,13 @@ config ATH9K_RATE_CONTROL
>>>           Say Y, if you want to use the ath9k specific rate control
>>>           module instead of minstrel_ht.
>>>
>>>  +config ATH9K_DFS
>>>  +       bool "Atheros ath9k DFS support"
>>>  +       depends on ATH9K
>>>  +       default y
>>
>>  At this point selecting y does nothing. Leave this patch out until
>>  selecting "y" means something.
>>
> What do you mean by 'nothing'? It allows you to select DFS as ath9k feature in your kernel configuration, or? Though, I agree that enabling it by default is not a good idea.
>
>>  Default should be n, and in particular Atheros itself can only likely
>>  commit to supporting DFS for AR9003 when it finds resources to do so
>>  as well as properly test it, so DFS support kconfig should state this.
>>  If someone wants to step up to completely support all bugs for older
>>  families that is their prerogative but we cannot commit to it, due to
>>  the regulatory considerations though unless this happens this cannot
>>  and should not be enabled for older families in code.
>>
> In fact, AR9003 is the platform we are interested in. Although it
> seems that older chipsets do also detect pulses with this patches
> (AR9280 does, IIRC), I agree to limit DFS support to AR9003 (and later).
> This should be easily possible by setting priv_ops->set_radar_params for
> AR9003 only. I'll remove it for AR5008 in my v2 RFC.
Please don't remove support code for older hardware. I'm fine with 
adding a SREV check that prevents it from being enabled by default on 
older hardware, but eventually I will need at least AR9280 DFS support 
for a few devices in OpenWrt.

- Felix

[ath9k-devel] [RFC 4/6] ath9k: add DFS build parameter

[Felix Fietkau]

On 2011-10-04 11:55 AM, Zefir Kurtisi wrote:
> On 10/03/2011 08:26 PM, Luis R. Rodriguez wrote:
>>  On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi<zefir.kurtisi@neratec.com>  wrote:
>>>
>>>  Signed-off-by: Zefir Kurtisi<zefir.kurtisi@neratec.com>
>>>  ---
>>>   drivers/net/wireless/ath/ath9k/Kconfig  |    7 +++++++
>>>   drivers/net/wireless/ath/ath9k/Makefile |    2 ++
>>>   2 files changed, 9 insertions(+), 0 deletions(-)
>>>
>>>  diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
>>>  index d9c08c6..adddcca 100644
>>>  --- a/drivers/net/wireless/ath/ath9k/Kconfig
>>>  +++ b/drivers/net/wireless/ath/ath9k/Kconfig
>>>  @@ -58,6 +58,13 @@ config ATH9K_RATE_CONTROL
>>>           Say Y, if you want to use the ath9k specific rate control
>>>           module instead of minstrel_ht.
>>>
>>>  +config ATH9K_DFS
>>>  +       bool "Atheros ath9k DFS support"
>>>  +       depends on ATH9K
>>>  +       default y
>>
>>  At this point selecting y does nothing. Leave this patch out until
>>  selecting "y" means something.
>>
> What do you mean by 'nothing'? It allows you to select DFS as ath9k feature in your kernel configuration, or? Though, I agree that enabling it by default is not a good idea.
>
>>  Default should be n, and in particular Atheros itself can only likely
>>  commit to supporting DFS for AR9003 when it finds resources to do so
>>  as well as properly test it, so DFS support kconfig should state this.
>>  If someone wants to step up to completely support all bugs for older
>>  families that is their prerogative but we cannot commit to it, due to
>>  the regulatory considerations though unless this happens this cannot
>>  and should not be enabled for older families in code.
>>
> In fact, AR9003 is the platform we are interested in. Although it
> seems that older chipsets do also detect pulses with this patches
> (AR9280 does, IIRC), I agree to limit DFS support to AR9003 (and later).
> This should be easily possible by setting priv_ops->set_radar_params for
> AR9003 only. I'll remove it for AR5008 in my v2 RFC.
Please don't remove support code for older hardware. I'm fine with 
adding a SREV check that prevents it from being enabled by default on 
older hardware, but eventually I will need at least AR9280 DFS support 
for a few devices in OpenWrt.

- Felix

Re: [RFC 4/6] ath9k: add DFS build parameter

[Adrian Chadd]

.. ditto for FreeBSD.

I'm hoping we can both leverage the same shared radar classification
modules with little changes needed in code. I'd like this to work for
at least AR9280.


Adrian

[ath9k-devel] [RFC 4/6] ath9k: add DFS build parameter

[Adrian Chadd]

.. ditto for FreeBSD.

I'm hoping we can both leverage the same shared radar classification
modules with little changes needed in code. I'd like this to work for
at least AR9280.


Adrian

Re: [RFC 4/6] ath9k: add DFS build parameter

[Luis R. Rodriguez]

On Tue, Oct 4, 2011 at 3:37 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2011-10-04 11:55 AM, Zefir Kurtisi wrote:
>>
>> On 10/03/2011 08:26 PM, Luis R. Rodriguez wrote:
>>>
>>>  On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi<zefir.kurtisi@neratec.com>
>>>  wrote:
>>>>
>>>>  Signed-off-by: Zefir Kurtisi<zefir.kurtisi@neratec.com>
>>>>  ---
>>>>  drivers/net/wireless/ath/ath9k/Kconfig  |    7 +++++++
>>>>  drivers/net/wireless/ath/ath9k/Makefile |    2 ++
>>>>  2 files changed, 9 insertions(+), 0 deletions(-)
>>>>
>>>>  diff --git a/drivers/net/wireless/ath/ath9k/Kconfig
>>>> b/drivers/net/wireless/ath/ath9k/Kconfig
>>>>  index d9c08c6..adddcca 100644
>>>>  --- a/drivers/net/wireless/ath/ath9k/Kconfig
>>>>  +++ b/drivers/net/wireless/ath/ath9k/Kconfig
>>>>  @@ -58,6 +58,13 @@ config ATH9K_RATE_CONTROL
>>>>          Say Y, if you want to use the ath9k specific rate control
>>>>          module instead of minstrel_ht.
>>>>
>>>>  +config ATH9K_DFS
>>>>  +       bool "Atheros ath9k DFS support"
>>>>  +       depends on ATH9K
>>>>  +       default y
>>>
>>>  At this point selecting y does nothing. Leave this patch out until
>>>  selecting "y" means something.
>>>
>> What do you mean by 'nothing'? It allows you to select DFS as ath9k
>> feature in your kernel configuration, or? Though, I agree that enabling it
>> by default is not a good idea.
>>
>>>  Default should be n, and in particular Atheros itself can only likely
>>>  commit to supporting DFS for AR9003 when it finds resources to do so
>>>  as well as properly test it, so DFS support kconfig should state this.
>>>  If someone wants to step up to completely support all bugs for older
>>>  families that is their prerogative but we cannot commit to it, due to
>>>  the regulatory considerations though unless this happens this cannot
>>>  and should not be enabled for older families in code.
>>>
>> In fact, AR9003 is the platform we are interested in. Although it
>> seems that older chipsets do also detect pulses with this patches
>> (AR9280 does, IIRC), I agree to limit DFS support to AR9003 (and later).
>> This should be easily possible by setting priv_ops->set_radar_params for
>> AR9003 only. I'll remove it for AR5008 in my v2 RFC.
>
> Please don't remove support code for older hardware. I'm fine with adding a
> SREV check that prevents it from being enabled by default on older hardware,
> but eventually I will need at least AR9280 DFS support for a few devices in
> OpenWrt.

That's fine, leaving it disabled is what I meant, and only once
something has been properly tested do we enable it. I personally only
want to spend my own energy on AR9003 and newer.

  Luis

[ath9k-devel] [RFC 4/6] ath9k: add DFS build parameter

[Luis R. Rodriguez]

On Tue, Oct 4, 2011 at 3:37 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2011-10-04 11:55 AM, Zefir Kurtisi wrote:
>>
>> On 10/03/2011 08:26 PM, Luis R. Rodriguez wrote:
>>>
>>> ?On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi<zefir.kurtisi@neratec.com>
>>> ?wrote:
>>>>
>>>> ?Signed-off-by: Zefir Kurtisi<zefir.kurtisi@neratec.com>
>>>> ?---
>>>> ?drivers/net/wireless/ath/ath9k/Kconfig ?| ? ?7 +++++++
>>>> ?drivers/net/wireless/ath/ath9k/Makefile | ? ?2 ++
>>>> ?2 files changed, 9 insertions(+), 0 deletions(-)
>>>>
>>>> ?diff --git a/drivers/net/wireless/ath/ath9k/Kconfig
>>>> b/drivers/net/wireless/ath/ath9k/Kconfig
>>>> ?index d9c08c6..adddcca 100644
>>>> ?--- a/drivers/net/wireless/ath/ath9k/Kconfig
>>>> ?+++ b/drivers/net/wireless/ath/ath9k/Kconfig
>>>> ?@@ -58,6 +58,13 @@ config ATH9K_RATE_CONTROL
>>>> ? ? ? ? ?Say Y, if you want to use the ath9k specific rate control
>>>> ? ? ? ? ?module instead of minstrel_ht.
>>>>
>>>> ?+config ATH9K_DFS
>>>> ?+ ? ? ? bool "Atheros ath9k DFS support"
>>>> ?+ ? ? ? depends on ATH9K
>>>> ?+ ? ? ? default y
>>>
>>> ?At this point selecting y does nothing. Leave this patch out until
>>> ?selecting "y" means something.
>>>
>> What do you mean by 'nothing'? It allows you to select DFS as ath9k
>> feature in your kernel configuration, or? Though, I agree that enabling it
>> by default is not a good idea.
>>
>>> ?Default should be n, and in particular Atheros itself can only likely
>>> ?commit to supporting DFS for AR9003 when it finds resources to do so
>>> ?as well as properly test it, so DFS support kconfig should state this.
>>> ?If someone wants to step up to completely support all bugs for older
>>> ?families that is their prerogative but we cannot commit to it, due to
>>> ?the regulatory considerations though unless this happens this cannot
>>> ?and should not be enabled for older families in code.
>>>
>> In fact, AR9003 is the platform we are interested in. Although it
>> seems that older chipsets do also detect pulses with this patches
>> (AR9280 does, IIRC), I agree to limit DFS support to AR9003 (and later).
>> This should be easily possible by setting priv_ops->set_radar_params for
>> AR9003 only. I'll remove it for AR5008 in my v2 RFC.
>
> Please don't remove support code for older hardware. I'm fine with adding a
> SREV check that prevents it from being enabled by default on older hardware,
> but eventually I will need at least AR9280 DFS support for a few devices in
> OpenWrt.

That's fine, leaving it disabled is what I meant, and only once
something has been properly tested do we enable it. I personally only
want to spend my own energy on AR9003 and newer.

  Luis

[RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
---
 drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index e8aeb98..5defebe 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
 			"Unable to reset channel, reset status %d\n", r);
 		goto out;
 	}
+#ifdef CONFIG_ATH9K_DFS
+	/**
+	 * enable radar pulse detection
+	 *
+	 * TODO: do this only for DFS channels
+	 */
+	ah->private_ops.set_radar_params(ah, &ah->radar_conf);
+	ath9k_hw_setrxfilter(ah,
+			ath9k_hw_getrxfilter(ah) | ATH9K_RX_FILTER_PHYRADAR);
+	ath_dbg(common, ATH_DBG_DFS,
+		"DFS enabled for channel %d\n", hchan->chan->center_freq);
+#endif
 
 	if (!ath_complete_reset(sc, true))
 		r = -EIO;
-- 
1.7.4.1

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
---
 drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index e8aeb98..5defebe 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
 			"Unable to reset channel, reset status %d\n", r);
 		goto out;
 	}
+#ifdef CONFIG_ATH9K_DFS
+	/**
+	 * enable radar pulse detection
+	 *
+	 * TODO: do this only for DFS channels
+	 */
+	ah->private_ops.set_radar_params(ah, &ah->radar_conf);
+	ath9k_hw_setrxfilter(ah,
+			ath9k_hw_getrxfilter(ah) | ATH9K_RX_FILTER_PHYRADAR);
+	ath_dbg(common, ATH_DBG_DFS,
+		"DFS enabled for channel %d\n", hchan->chan->center_freq);
+#endif
 
 	if (!ath_complete_reset(sc, true))
 		r = -EIO;
-- 
1.7.4.1

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>
> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> ---
>  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index e8aeb98..5defebe 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>                        "Unable to reset channel, reset status %d\n", r);
>                goto out;
>        }
> +#ifdef CONFIG_ATH9K_DFS
> +       /**
> +        * enable radar pulse detection
> +        *
> +        * TODO: do this only for DFS channels
> +        */
> +       ah->private_ops.set_radar_params(ah, &ah->radar_conf);
> +       ath9k_hw_setrxfilter(ah,
> +                       ath9k_hw_getrxfilter(ah) | ATH9K_RX_FILTER_PHYRADAR);
> +       ath_dbg(common, ATH_DBG_DFS,
> +               "DFS enabled for channel %d\n", hchan->chan->center_freq);
> +#endif

Please spare the #ifdef and just call something within dfs.c, then
dfs.h would wrap it to nothing if DFS is disabled.

  Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>
> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> ---
> ?drivers/net/wireless/ath/ath9k/main.c | ? 12 ++++++++++++
> ?1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index e8aeb98..5defebe 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
> ? ? ? ? ? ? ? ? ? ? ? ?"Unable to reset channel, reset status %d\n", r);
> ? ? ? ? ? ? ? ?goto out;
> ? ? ? ?}
> +#ifdef CONFIG_ATH9K_DFS
> + ? ? ? /**
> + ? ? ? ?* enable radar pulse detection
> + ? ? ? ?*
> + ? ? ? ?* TODO: do this only for DFS channels
> + ? ? ? ?*/
> + ? ? ? ah->private_ops.set_radar_params(ah, &ah->radar_conf);
> + ? ? ? ath9k_hw_setrxfilter(ah,
> + ? ? ? ? ? ? ? ? ? ? ? ath9k_hw_getrxfilter(ah) | ATH9K_RX_FILTER_PHYRADAR);
> + ? ? ? ath_dbg(common, ATH_DBG_DFS,
> + ? ? ? ? ? ? ? "DFS enabled for channel %d\n", hchan->chan->center_freq);
> +#endif

Please spare the #ifdef and just call something within dfs.c, then
dfs.h would wrap it to nothing if DFS is disabled.

  Luis

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> >
> > Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> > ---
> >  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> > index e8aeb98..5defebe 100644
> > --- a/drivers/net/wireless/ath/ath9k/main.c
> > +++ b/drivers/net/wireless/ath/ath9k/main.c
> > @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
> >                        "Unable to reset channel, reset status %d\n", r);
> >                goto out;
> >        }
> > +#ifdef CONFIG_ATH9K_DFS
> > +       /**
> > +        * enable radar pulse detection
> > +        *
> > +        * TODO: do this only for DFS channels
> > +        */
> > +       ah->private_ops.set_radar_params(ah, &ah->radar_conf);
> > +       ath9k_hw_setrxfilter(ah,
> > +                       ath9k_hw_getrxfilter(ah) | ATH9K_RX_FILTER_PHYRADAR);
> > +       ath_dbg(common, ATH_DBG_DFS,
> > +               "DFS enabled for channel %d\n", hchan->chan->center_freq);
> > +#endif
> 
> Please spare the #ifdef and just call something within dfs.c, then
> dfs.h would wrap it to nothing if DFS is disabled.
Why would anyone want to disable DFS driver support?
I would say: drop the ifdefs altogether since DFS
is and will be "required".

Regards,
	Chr

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> >
> > Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> > ---
> >  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> > index e8aeb98..5defebe 100644
> > --- a/drivers/net/wireless/ath/ath9k/main.c
> > +++ b/drivers/net/wireless/ath/ath9k/main.c
> > @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
> >                        "Unable to reset channel, reset status %d\n", r);
> >                goto out;
> >        }
> > +#ifdef CONFIG_ATH9K_DFS
> > +       /**
> > +        * enable radar pulse detection
> > +        *
> > +        * TODO: do this only for DFS channels
> > +        */
> > +       ah->private_ops.set_radar_params(ah, &ah->radar_conf);
> > +       ath9k_hw_setrxfilter(ah,
> > +                       ath9k_hw_getrxfilter(ah) | ATH9K_RX_FILTER_PHYRADAR);
> > +       ath_dbg(common, ATH_DBG_DFS,
> > +               "DFS enabled for channel %d\n", hchan->chan->center_freq);
> > +#endif
> 
> Please spare the #ifdef and just call something within dfs.c, then
> dfs.h would wrap it to nothing if DFS is disabled.
Why would anyone want to disable DFS driver support?
I would say: drop the ifdefs altogether since DFS
is and will be "required".

Regards,
	Chr

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>> >
>> > Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>> > ---
>> >  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
>> >  1 files changed, 12 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> > index e8aeb98..5defebe 100644
>> > --- a/drivers/net/wireless/ath/ath9k/main.c
>> > +++ b/drivers/net/wireless/ath/ath9k/main.c
>> > @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>> >                        "Unable to reset channel, reset status %d\n", r);
>> >                goto out;
>> >        }
>> > +#ifdef CONFIG_ATH9K_DFS
>> > +       /**
>> > +        * enable radar pulse detection
>> > +        *
>> > +        * TODO: do this only for DFS channels
>> > +        */
>> > +       ah->private_ops.set_radar_params(ah, &ah->radar_conf);
>> > +       ath9k_hw_setrxfilter(ah,
>> > +                       ath9k_hw_getrxfilter(ah) | ATH9K_RX_FILTER_PHYRADAR);
>> > +       ath_dbg(common, ATH_DBG_DFS,
>> > +               "DFS enabled for channel %d\n", hchan->chan->center_freq);
>> > +#endif
>>
>> Please spare the #ifdef and just call something within dfs.c, then
>> dfs.h would wrap it to nothing if DFS is disabled.
> Why would anyone want to disable DFS driver support?
> I would say: drop the ifdefs altogether since DFS
> is and will be "required".

Because DFS requires to be properly tested before being enabled. You
may also want to simply disable DFS if you do not want to deal with
the regulatory test implications of having it enabled.

  Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>> >
>> > Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>> > ---
>> > ?drivers/net/wireless/ath/ath9k/main.c | ? 12 ++++++++++++
>> > ?1 files changed, 12 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> > index e8aeb98..5defebe 100644
>> > --- a/drivers/net/wireless/ath/ath9k/main.c
>> > +++ b/drivers/net/wireless/ath/ath9k/main.c
>> > @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>> > ? ? ? ? ? ? ? ? ? ? ? ?"Unable to reset channel, reset status %d\n", r);
>> > ? ? ? ? ? ? ? ?goto out;
>> > ? ? ? ?}
>> > +#ifdef CONFIG_ATH9K_DFS
>> > + ? ? ? /**
>> > + ? ? ? ?* enable radar pulse detection
>> > + ? ? ? ?*
>> > + ? ? ? ?* TODO: do this only for DFS channels
>> > + ? ? ? ?*/
>> > + ? ? ? ah->private_ops.set_radar_params(ah, &ah->radar_conf);
>> > + ? ? ? ath9k_hw_setrxfilter(ah,
>> > + ? ? ? ? ? ? ? ? ? ? ? ath9k_hw_getrxfilter(ah) | ATH9K_RX_FILTER_PHYRADAR);
>> > + ? ? ? ath_dbg(common, ATH_DBG_DFS,
>> > + ? ? ? ? ? ? ? "DFS enabled for channel %d\n", hchan->chan->center_freq);
>> > +#endif
>>
>> Please spare the #ifdef and just call something within dfs.c, then
>> dfs.h would wrap it to nothing if DFS is disabled.
> Why would anyone want to disable DFS driver support?
> I would say: drop the ifdefs altogether since DFS
> is and will be "required".

Because DFS requires to be properly tested before being enabled. You
may also want to simply disable DFS if you do not want to deal with
the regulatory test implications of having it enabled.

  Luis

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
> <chunkeey@googlemail.com> wrote:
> > On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
> >> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> >> >
> >> > Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> >> > ---
> >> >  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
> >> >  1 files changed, 12 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> >> > index e8aeb98..5defebe 100644
> >> > --- a/drivers/net/wireless/ath/ath9k/main.c
> >> > +++ b/drivers/net/wireless/ath/ath9k/main.c
> >> > @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
> >> >                        "Unable to reset channel, reset status %d\n", r);
> >> >                goto out;
> >> >        }
> >> > +#ifdef CONFIG_ATH9K_DFS
> >>
> >> Please spare the #ifdef and just call something within dfs.c, then
> >> dfs.h would wrap it to nothing if DFS is disabled.
> > Why would anyone want to disable DFS driver support?
> > I would say: drop the ifdefs altogether since DFS
> > is and will be "required".
> 
> Because DFS requires to be properly tested before being enabled.
Testing if a driver detects a pulse is "trivial" compared to the
stuff mac80211/cfg80211 and hostapd will have to do to make a
channel-change as smooth as possible. I think if there's a DFS
"OFF" switch, it should be in hostapd and I hope more people
agree on this one.  

> You may also want to simply disable DFS if you do not want to
> deal with the regulatory test implications of having it enabled.
AFAIK you can't "simply" disable the DFS requirement: hostapd
(hw_features.c), [cfg80211] (checks if tx on secondary channel
is possible) and mac80211 (tx.c) all have checks. Indeed, the
easiest way is to modify crda's database. So there's no need
for an extra compile-time option.

Regards,
	Chr

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
> <chunkeey@googlemail.com> wrote:
> > On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
> >> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> >> >
> >> > Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> >> > ---
> >> >  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
> >> >  1 files changed, 12 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> >> > index e8aeb98..5defebe 100644
> >> > --- a/drivers/net/wireless/ath/ath9k/main.c
> >> > +++ b/drivers/net/wireless/ath/ath9k/main.c
> >> > @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
> >> >                        "Unable to reset channel, reset status %d\n", r);
> >> >                goto out;
> >> >        }
> >> > +#ifdef CONFIG_ATH9K_DFS
> >>
> >> Please spare the #ifdef and just call something within dfs.c, then
> >> dfs.h would wrap it to nothing if DFS is disabled.
> > Why would anyone want to disable DFS driver support?
> > I would say: drop the ifdefs altogether since DFS
> > is and will be "required".
> 
> Because DFS requires to be properly tested before being enabled.
Testing if a driver detects a pulse is "trivial" compared to the
stuff mac80211/cfg80211 and hostapd will have to do to make a
channel-change as smooth as possible. I think if there's a DFS
"OFF" switch, it should be in hostapd and I hope more people
agree on this one.  

> You may also want to simply disable DFS if you do not want to
> deal with the regulatory test implications of having it enabled.
AFAIK you can't "simply" disable the DFS requirement: hostapd
(hw_features.c), [cfg80211] (checks if tx on secondary channel
is possible) and mac80211 (tx.c) all have checks. Indeed, the
easiest way is to modify crda's database. So there's no need
for an extra compile-time option.

Regards,
	Chr

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

On 10/04/2011 03:38 PM, Christian Lamparter wrote:
> On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
>> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
>> <chunkeey@googlemail.com> wrote:
>>> On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
>>>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>>>>
>>>>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>>>>> ---
>>>>>  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
>>>>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>>>> index e8aeb98..5defebe 100644
>>>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>>>> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>>>>>                        "Unable to reset channel, reset status %d\n", r);
>>>>>                goto out;
>>>>>        }
>>>>> +#ifdef CONFIG_ATH9K_DFS
>>>>
>>>> Please spare the #ifdef and just call something within dfs.c, then
>>>> dfs.h would wrap it to nothing if DFS is disabled.
>>> Why would anyone want to disable DFS driver support?
>>> I would say: drop the ifdefs altogether since DFS
>>> is and will be "required".
>>
>> Because DFS requires to be properly tested before being enabled.
> Testing if a driver detects a pulse is "trivial" compared to the
> stuff mac80211/cfg80211 and hostapd will have to do to make a
> channel-change as smooth as possible. I think if there's a DFS
> "OFF" switch, it should be in hostapd and I hope more people
> agree on this one.  
> 
Yes on both. Work on the management part of the DFS module has just been started by TI guys. When this is in, hostapd will be able to query the driver's DFS detection capabilities and leave DFS channels disabled for those devices with no (or insufficient) support (like it is generally done today for DFS channels).

The proper way for a driver's OFF switch would then be to just announce missing DFS capabilities.

>> You may also want to simply disable DFS if you do not want to
>> deal with the regulatory test implications of having it enabled.
> AFAIK you can't "simply" disable the DFS requirement: hostapd
> (hw_features.c), [cfg80211] (checks if tx on secondary channel
> is possible) and mac80211 (tx.c) all have checks. Indeed, the
> easiest way is to modify crda's database. So there's no need
> for an extra compile-time option.
> 
There might be a demand for conditional compiling in addition to DFS capabilities announcements to reduce memory footprint, since (especially) pattern matching algorithms will increase it significantly.

> Regards,
> 	Chr

Zefir

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

On 10/04/2011 03:38 PM, Christian Lamparter wrote:
> On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
>> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
>> <chunkeey@googlemail.com> wrote:
>>> On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
>>>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>>>>
>>>>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>>>>> ---
>>>>>  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
>>>>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>>>> index e8aeb98..5defebe 100644
>>>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>>>> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>>>>>                        "Unable to reset channel, reset status %d\n", r);
>>>>>                goto out;
>>>>>        }
>>>>> +#ifdef CONFIG_ATH9K_DFS
>>>>
>>>> Please spare the #ifdef and just call something within dfs.c, then
>>>> dfs.h would wrap it to nothing if DFS is disabled.
>>> Why would anyone want to disable DFS driver support?
>>> I would say: drop the ifdefs altogether since DFS
>>> is and will be "required".
>>
>> Because DFS requires to be properly tested before being enabled.
> Testing if a driver detects a pulse is "trivial" compared to the
> stuff mac80211/cfg80211 and hostapd will have to do to make a
> channel-change as smooth as possible. I think if there's a DFS
> "OFF" switch, it should be in hostapd and I hope more people
> agree on this one.  
> 
Yes on both. Work on the management part of the DFS module has just been started by TI guys. When this is in, hostapd will be able to query the driver's DFS detection capabilities and leave DFS channels disabled for those devices with no (or insufficient) support (like it is generally done today for DFS channels).

The proper way for a driver's OFF switch would then be to just announce missing DFS capabilities.

>> You may also want to simply disable DFS if you do not want to
>> deal with the regulatory test implications of having it enabled.
> AFAIK you can't "simply" disable the DFS requirement: hostapd
> (hw_features.c), [cfg80211] (checks if tx on secondary channel
> is possible) and mac80211 (tx.c) all have checks. Indeed, the
> easiest way is to modify crda's database. So there's no need
> for an extra compile-time option.
> 
There might be a demand for conditional compiling in addition to DFS capabilities announcements to reduce memory footprint, since (especially) pattern matching algorithms will increase it significantly.

> Regards,
> 	Chr

Zefir

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Adrian Chadd]

.. erm, how complicated is the pattern matching code when it's compiled?

The port of the fusion radar detection module didn't end up being all that big.



Adrian

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Adrian Chadd]

.. erm, how complicated is the pattern matching code when it's compiled?

The port of the fusion radar detection module didn't end up being all that big.



Adrian

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Tue, Oct 4, 2011 at 7:34 AM, Adrian Chadd <adrian@freebsd.org> wrote:
> .. erm, how complicated is the pattern matching code when it's compiled?
>
> The port of the fusion radar detection module didn't end up being all that big.

Adrian, please keep terms like "fusion codebase" out of public lists.

  Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Tue, Oct 4, 2011 at 7:34 AM, Adrian Chadd <adrian@freebsd.org> wrote:
> .. erm, how complicated is the pattern matching code when it's compiled?
>
> The port of the fusion radar detection module didn't end up being all that big.

Adrian, please keep terms like "fusion codebase" out of public lists.

  Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Peter Stuge]

Luis R. Rodriguez wrote:
> > The port of the fusion radar detection module didn't end up being
> > all that big.
> 
> Adrian, please keep terms like "fusion codebase" out of public
> lists.

I think that's silly. But you already knew that.


//Peter

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Wed, Oct 5, 2011 at 3:53 PM, Peter Stuge <peter@stuge.se> wrote:
> Luis R. Rodriguez wrote:
>> > The port of the fusion radar detection module didn't end up being
>> > all that big.
>>
>> Adrian, please keep terms like "fusion codebase" out of public
>> lists.
>
> I think that's silly. But you already knew that.

Silly, but it confuses people who have not signed proper agreements to
contribute upstream with access to alternative codebases. We have
proper agreements in place with key active developers to do this.
Making broad statements like the above sometimes gives hints to some
developers under other agreements they can simply contribute upstream
with the code they received under other agreements.

  Luis

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Tuesday, October 04, 2011 04:17:35 PM Zefir Kurtisi wrote:
> On 10/04/2011 03:38 PM, Christian Lamparter wrote:
> > On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
> >> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
> >> <chunkeey@googlemail.com> wrote:
> >>> On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
> >>>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> >>>>>
> >>>>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> >>>>> ---
> >>>>>  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
> >>>>>  1 files changed, 12 insertions(+), 0 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> >>>>> index e8aeb98..5defebe 100644
> >>>>> --- a/drivers/net/wireless/ath/ath9k/main.c
> >>>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
> >>>>> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
> >>>>>                        "Unable to reset channel, reset status %d\n", r);
> >>>>>                goto out;
> >>>>>        }
> >>>>> +#ifdef CONFIG_ATH9K_DFS
> >>>>
> >>>> Please spare the #ifdef and just call something within dfs.c, then
> >>>> dfs.h would wrap it to nothing if DFS is disabled.
> >>> Why would anyone want to disable DFS driver support?
> >>> I would say: drop the ifdefs altogether since DFS
> >>> is and will be "required".
> >>
> >> Because DFS requires to be properly tested before being enabled.
> > Testing if a driver detects a pulse is "trivial" compared to the
> > stuff mac80211/cfg80211 and hostapd will have to do to make a
> > channel-change as smooth as possible. I think if there's a DFS
> > "OFF" switch, it should be in hostapd and I hope more people
> > agree on this one.  
> > 
> Yes on both. Work on the management part of the DFS module has just
> been started by TI guys. When this is in, hostapd will be able to
> query the driver's DFS detection capabilities and leave DFS channels
> disabled for those devices with no (or insufficient) support
> (like it is generally done today for DFS channels).
> 
> The proper way for a driver's OFF switch would then be to just
> announce missing DFS capabilities.
Actually, I think we already have a flag for such a
purpose:
 * @IEEE80211_HW_SPECTRUM_MGMT:
 *      Hardware supports spectrum management defined in 802.11h
AFAIK 802.11h[now integrated in 802.11-2007] included DFS, right? 

> >> You may also want to simply disable DFS if you do not want to
> >> deal with the regulatory test implications of having it enabled.
> > AFAIK you can't "simply" disable the DFS requirement: hostapd
> > (hw_features.c), [cfg80211] (checks if tx on secondary channel
> > is possible) and mac80211 (tx.c) all have checks. Indeed, the
> > easiest way is to modify crda's database. So there's no need
> > for an extra compile-time option.
> > 
> There might be a demand for conditional compiling in addition to
> DFS capabilities announcements to reduce memory footprint, since
> (especially) pattern matching algorithms will increase it significantly.
I don't think memory footprint is such a big problem. After all ath9k
has around 170 kb [please correct me if I'm wrong here] of initvals
for all supported solutions since AR5008.

Regards,
	Chr

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Tuesday, October 04, 2011 04:17:35 PM Zefir Kurtisi wrote:
> On 10/04/2011 03:38 PM, Christian Lamparter wrote:
> > On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
> >> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
> >> <chunkeey@googlemail.com> wrote:
> >>> On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
> >>>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> >>>>>
> >>>>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> >>>>> ---
> >>>>>  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
> >>>>>  1 files changed, 12 insertions(+), 0 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> >>>>> index e8aeb98..5defebe 100644
> >>>>> --- a/drivers/net/wireless/ath/ath9k/main.c
> >>>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
> >>>>> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
> >>>>>                        "Unable to reset channel, reset status %d\n", r);
> >>>>>                goto out;
> >>>>>        }
> >>>>> +#ifdef CONFIG_ATH9K_DFS
> >>>>
> >>>> Please spare the #ifdef and just call something within dfs.c, then
> >>>> dfs.h would wrap it to nothing if DFS is disabled.
> >>> Why would anyone want to disable DFS driver support?
> >>> I would say: drop the ifdefs altogether since DFS
> >>> is and will be "required".
> >>
> >> Because DFS requires to be properly tested before being enabled.
> > Testing if a driver detects a pulse is "trivial" compared to the
> > stuff mac80211/cfg80211 and hostapd will have to do to make a
> > channel-change as smooth as possible. I think if there's a DFS
> > "OFF" switch, it should be in hostapd and I hope more people
> > agree on this one.  
> > 
> Yes on both. Work on the management part of the DFS module has just
> been started by TI guys. When this is in, hostapd will be able to
> query the driver's DFS detection capabilities and leave DFS channels
> disabled for those devices with no (or insufficient) support
> (like it is generally done today for DFS channels).
> 
> The proper way for a driver's OFF switch would then be to just
> announce missing DFS capabilities.
Actually, I think we already have a flag for such a
purpose:
 * @IEEE80211_HW_SPECTRUM_MGMT:
 *      Hardware supports spectrum management defined in 802.11h
AFAIK 802.11h[now integrated in 802.11-2007] included DFS, right? 

> >> You may also want to simply disable DFS if you do not want to
> >> deal with the regulatory test implications of having it enabled.
> > AFAIK you can't "simply" disable the DFS requirement: hostapd
> > (hw_features.c), [cfg80211] (checks if tx on secondary channel
> > is possible) and mac80211 (tx.c) all have checks. Indeed, the
> > easiest way is to modify crda's database. So there's no need
> > for an extra compile-time option.
> > 
> There might be a demand for conditional compiling in addition to
> DFS capabilities announcements to reduce memory footprint, since
> (especially) pattern matching algorithms will increase it significantly.
I don't think memory footprint is such a big problem. After all ath9k
has around 170 kb [please correct me if I'm wrong here] of initvals
for all supported solutions since AR5008.

Regards,
	Chr

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Adrian Chadd]

Also whilst I'm at it, "SPECTRUM_MANAGEMENT" is a very broad flag to set.

For example: you may not want to do DFS on the AR5416 NICs because (as
documented in the open hal and earlier ath9k bits) there isn't support
for radar pulses on the ext channel. So even if you had a successful
DFS algorithm for this NIC, you'd have to somehow tell the DFS
machinery that HT40+DFS channels aren't supported but HT20+DFS
channels are.

But then, the AR5416 supports per-packet TPC, so you could use it in
STA mode perfectly fine and it'd support that part of spectrum
management. Since you get per-frame RSSI of RX'ed frames, you can
support the spectrum power histogram IE.

And since it supports quiet time stuff, you can use it in STA and
hostap mode for supporting the quiet time IE.

(Yes, I'm looking at how to make all of this work in FreeBSD net80211,
as some patches have been supplied to start fleshing out these
functions. :)

I'm not saying this needs to be solved now, but I think it's worth
thinking about how to encapsulate exactly what it is that NICs
support, rather than simply saying "yup, 11h is here, all good mate."


Adrian

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Adrian Chadd]

Also whilst I'm at it, "SPECTRUM_MANAGEMENT" is a very broad flag to set.

For example: you may not want to do DFS on the AR5416 NICs because (as
documented in the open hal and earlier ath9k bits) there isn't support
for radar pulses on the ext channel. So even if you had a successful
DFS algorithm for this NIC, you'd have to somehow tell the DFS
machinery that HT40+DFS channels aren't supported but HT20+DFS
channels are.

But then, the AR5416 supports per-packet TPC, so you could use it in
STA mode perfectly fine and it'd support that part of spectrum
management. Since you get per-frame RSSI of RX'ed frames, you can
support the spectrum power histogram IE.

And since it supports quiet time stuff, you can use it in STA and
hostap mode for supporting the quiet time IE.

(Yes, I'm looking at how to make all of this work in FreeBSD net80211,
as some patches have been supplied to start fleshing out these
functions. :)

I'm not saying this needs to be solved now, but I think it's worth
thinking about how to encapsulate exactly what it is that NICs
support, rather than simply saying "yup, 11h is here, all good mate."


Adrian

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Tuesday, October 04, 2011 04:50:42 PM Adrian Chadd wrote:
> Also whilst I'm at it, "SPECTRUM_MANAGEMENT" is a very broad flag to set.

Hm, 802.11-2007 5.4.4 "Spectrum management services" explicitly
mentions that both services TPC and DFS are required in some
regulatory domains for operation in the 5 GHz band.

> For example: you may not want to do DFS on the AR5416 NICs because
> (as documented in the open hal and earlier ath9k bits) there isn't
> support for radar pulses on the ext channel. So even if you had a
> successful DFS algorithm for this NIC, you'd have to somehow tell
> the DFS machinery that HT40+DFS channels aren't supported but
> HT20+DFS channels are.
well, that's a pity. We do have seperated HT feature sets for each
band but they are not seperated by operation mode...

[ Just for fun: Do you know how the AR5418 handled radars in
the proprietary turbo mode in the 5 GHz band? And how does
Atheros own driver/stack/etc. deals with this limitation?
After all Unex sold/sells the DNBA-81 on their product page
as: <http://www.unex.com.tw/product/dnba-81>

"DNBA-81 is a 802.11n 2x3 2-stream a/b/g dual band wifi
Cardbus designed specifically for laptops and *access points*/
home gateways/consumer electronics/multimedia entertainment
devices/peripherals with standard Cardbus slot." ]

Regards,
	Chr

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Tuesday, October 04, 2011 04:50:42 PM Adrian Chadd wrote:
> Also whilst I'm at it, "SPECTRUM_MANAGEMENT" is a very broad flag to set.

Hm, 802.11-2007 5.4.4 "Spectrum management services" explicitly
mentions that both services TPC and DFS are required in some
regulatory domains for operation in the 5 GHz band.

> For example: you may not want to do DFS on the AR5416 NICs because
> (as documented in the open hal and earlier ath9k bits) there isn't
> support for radar pulses on the ext channel. So even if you had a
> successful DFS algorithm for this NIC, you'd have to somehow tell
> the DFS machinery that HT40+DFS channels aren't supported but
> HT20+DFS channels are.
well, that's a pity. We do have seperated HT feature sets for each
band but they are not seperated by operation mode...

[ Just for fun: Do you know how the AR5418 handled radars in
the proprietary turbo mode in the 5 GHz band? And how does
Atheros own driver/stack/etc. deals with this limitation?
After all Unex sold/sells the DNBA-81 on their product page
as: <http://www.unex.com.tw/product/dnba-81>

"DNBA-81 is a 802.11n 2x3 2-stream a/b/g dual band wifi
Cardbus designed specifically for laptops and *access points*/
home gateways/consumer electronics/multimedia entertainment
devices/peripherals with standard Cardbus slot." ]

Regards,
	Chr

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Adrian Chadd]

On 4 October 2011 23:26, Christian Lamparter <chunkeey@googlemail.com> wrote:
> On Tuesday, October 04, 2011 04:50:42 PM Adrian Chadd wrote:
>> Also whilst I'm at it, "SPECTRUM_MANAGEMENT" is a very broad flag to set.
>
> Hm, 802.11-2007 5.4.4 "Spectrum management services" explicitly
> mentions that both services TPC and DFS are required in some
> regulatory domains for operation in the 5 GHz band.

Right, but just for hostap mode, correct? I'm also thinking about
non-hostap mode support.

> [ Just for fun: Do you know how the AR5418 handled radars in
> the proprietary turbo mode in the 5 GHz band? And how does
> Atheros own driver/stack/etc. deals with this limitation?
> After all Unex sold/sells the DNBA-81 on their product page
> as: <http://www.unex.com.tw/product/dnba-81>
>
> "DNBA-81 is a 802.11n 2x3 2-stream a/b/g dual band wifi
> Cardbus designed specifically for laptops and *access points*/
> home gateways/consumer electronics/multimedia entertainment
> devices/peripherals with standard Cardbus slot." ]

I haven't sat down and looked at exactly what the AR5416/AR5418
support is like save what I've seen when digging into the meaning
behind the extended EEPROM bits. In any case, the easiest solution is
to just disable HT40 for those (which, incidentally, is exactly what
FreeBSD's ath and net80211 regulatory code did until recently.)

And as I said, that isn't needed for station mode operation, so you
can just use HT/40 channels. :)

Besides, you realise that vendors say the darnest things at times? :)
For example, one well-known vendor lists their AR9160 NICs as DFS
ready (and thus people who buy them think they're DFS ready), but if
you buy some of their MIPS SOC hardware, they only support
OpenWRT+ath9k on it. I'm sure more than a few people fell into that
trap. :)



Adrian

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Adrian Chadd]

On 4 October 2011 23:26, Christian Lamparter <chunkeey@googlemail.com> wrote:
> On Tuesday, October 04, 2011 04:50:42 PM Adrian Chadd wrote:
>> Also whilst I'm at it, "SPECTRUM_MANAGEMENT" is a very broad flag to set.
>
> Hm, 802.11-2007 5.4.4 "Spectrum management services" explicitly
> mentions that both services TPC and DFS are required in some
> regulatory domains for operation in the 5 GHz band.

Right, but just for hostap mode, correct? I'm also thinking about
non-hostap mode support.

> [ Just for fun: Do you know how the AR5418 handled radars in
> the proprietary turbo mode in the 5 GHz band? And how does
> Atheros own driver/stack/etc. deals with this limitation?
> After all Unex sold/sells the DNBA-81 on their product page
> as: <http://www.unex.com.tw/product/dnba-81>
>
> "DNBA-81 is a 802.11n 2x3 2-stream a/b/g dual band wifi
> Cardbus designed specifically for laptops and *access points*/
> home gateways/consumer electronics/multimedia entertainment
> devices/peripherals with standard Cardbus slot." ]

I haven't sat down and looked at exactly what the AR5416/AR5418
support is like save what I've seen when digging into the meaning
behind the extended EEPROM bits. In any case, the easiest solution is
to just disable HT40 for those (which, incidentally, is exactly what
FreeBSD's ath and net80211 regulatory code did until recently.)

And as I said, that isn't needed for station mode operation, so you
can just use HT/40 channels. :)

Besides, you realise that vendors say the darnest things at times? :)
For example, one well-known vendor lists their AR9160 NICs as DFS
ready (and thus people who buy them think they're DFS ready), but if
you buy some of their MIPS SOC hardware, they only support
OpenWRT+ath9k on it. I'm sure more than a few people fell into that
trap. :)



Adrian

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Tuesday, October 04, 2011 05:57:32 PM Adrian Chadd wrote:
> On 4 October 2011 23:26, Christian Lamparter <chunkeey@googlemail.com> wrote:
> > On Tuesday, October 04, 2011 04:50:42 PM Adrian Chadd wrote:
> >> Also whilst I'm at it, "SPECTRUM_MANAGEMENT" is a very broad flag to set.
> >
> > Hm, 802.11-2007 5.4.4 "Spectrum management services" explicitly
> > mentions that both services TPC and DFS are required in some
> > regulatory domains for operation in the 5 GHz band.
> 
> Right, but just for hostap mode, correct? I'm also thinking about
> non-hostap mode support.
I don't think it's just hostap mode. More like hostap, IBSS and MESH.
In fact, with P2P/Wifi-Direct, the station mode might becoming a
thing of the past, right?

> > [ Just for fun: Do you know how the AR5418 handled radars in
> > the proprietary turbo mode in the 5 GHz band? And how does
> > Atheros own driver/stack/etc. deals with this limitation?
> > After all Unex sold/sells the DNBA-81 on their product page
> > as: <http://www.unex.com.tw/product/dnba-81>
> >
> > "DNBA-81 is a 802.11n 2x3 2-stream a/b/g dual band wifi
> > Cardbus designed specifically for laptops and *access points*/
> > home gateways/consumer electronics/multimedia entertainment
> > devices/peripherals with standard Cardbus slot." ]
> 
> Besides, you realise that vendors say the darnest things at times? :)
> For example, one well-known vendor lists their AR9160 NICs as DFS
> ready (and thus people who buy them think they're DFS ready), but if
> you buy some of their MIPS SOC hardware, they only support
> OpenWRT+ath9k on it. I'm sure more than a few people fell into that
> trap. :)
Well, ok. I found a better example:
http://www.wikidevi.com/wiki/Apple_AirPort_Extreme_Base_Station_A1143_%28MA073LL/A%29

You see, they put an AR5418(or 6?) with an AR5133 into the first draft-n
Apple Airport Base Station. If this much money was involved, do you
think that an American Company would "lie" to another American Company?

Regards,
	Chr

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Tuesday, October 04, 2011 05:57:32 PM Adrian Chadd wrote:
> On 4 October 2011 23:26, Christian Lamparter <chunkeey@googlemail.com> wrote:
> > On Tuesday, October 04, 2011 04:50:42 PM Adrian Chadd wrote:
> >> Also whilst I'm at it, "SPECTRUM_MANAGEMENT" is a very broad flag to set.
> >
> > Hm, 802.11-2007 5.4.4 "Spectrum management services" explicitly
> > mentions that both services TPC and DFS are required in some
> > regulatory domains for operation in the 5 GHz band.
> 
> Right, but just for hostap mode, correct? I'm also thinking about
> non-hostap mode support.
I don't think it's just hostap mode. More like hostap, IBSS and MESH.
In fact, with P2P/Wifi-Direct, the station mode might becoming a
thing of the past, right?

> > [ Just for fun: Do you know how the AR5418 handled radars in
> > the proprietary turbo mode in the 5 GHz band? And how does
> > Atheros own driver/stack/etc. deals with this limitation?
> > After all Unex sold/sells the DNBA-81 on their product page
> > as: <http://www.unex.com.tw/product/dnba-81>
> >
> > "DNBA-81 is a 802.11n 2x3 2-stream a/b/g dual band wifi
> > Cardbus designed specifically for laptops and *access points*/
> > home gateways/consumer electronics/multimedia entertainment
> > devices/peripherals with standard Cardbus slot." ]
> 
> Besides, you realise that vendors say the darnest things at times? :)
> For example, one well-known vendor lists their AR9160 NICs as DFS
> ready (and thus people who buy them think they're DFS ready), but if
> you buy some of their MIPS SOC hardware, they only support
> OpenWRT+ath9k on it. I'm sure more than a few people fell into that
> trap. :)
Well, ok. I found a better example:
http://www.wikidevi.com/wiki/Apple_AirPort_Extreme_Base_Station_A1143_%28MA073LL/A%29

You see, they put an AR5418(or 6?) with an AR5133 into the first draft-n
Apple Airport Base Station. If this much money was involved, do you
think that an American Company would "lie" to another American Company?

Regards,
	Chr

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Adrian Chadd]

Nope, what I'm saying is that someone's likely just copy/paste'd the
"we support x and y!" from some marketing material and then suggested
people use linux/openwrt/ath9k without really looking at it.

There were plenty of people advertising the AR9160 as a "3x3 MIMO"
when it's a 2x2 stream device with 3 antennas. I know this confused me
when I started tinkering with wifi stuff.

Anyway, we're getting off-track here.


Adrian

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Adrian Chadd]

Nope, what I'm saying is that someone's likely just copy/paste'd the
"we support x and y!" from some marketing material and then suggested
people use linux/openwrt/ath9k without really looking at it.

There were plenty of people advertising the AR9160 as a "3x3 MIMO"
when it's a 2x2 stream device with 3 antennas. I know this confused me
when I started tinkering with wifi stuff.

Anyway, we're getting off-track here.


Adrian

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Tuesday, October 04, 2011 07:03:10 PM Adrian Chadd wrote:
> Nope, what I'm saying is that someone's likely just copy/paste'd the
> "we support x and y!" from some marketing material and then suggested
> people use linux/openwrt/ath9k without really looking at it.
I'm still curious how they got the Apple access point through the FCC
testing back then. Well, I guess we'll never know for sure.
 
> There were plenty of people advertising the AR9160 as a "3x3 MIMO"
> when it's a 2x2 stream device with 3 antennas. I know this confused me
> when I started tinkering with wifi stuff.
AFAIK the 3x3 really just stands for the amount of tx X rx chains [and
not antennas]. So if the hardware has 3 tx and rx chains it can be sold
as a "3x3" device even if does not support 3 streams.

> Anyway, we're getting off-track here.
we can stop anytime.

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Tuesday, October 04, 2011 07:03:10 PM Adrian Chadd wrote:
> Nope, what I'm saying is that someone's likely just copy/paste'd the
> "we support x and y!" from some marketing material and then suggested
> people use linux/openwrt/ath9k without really looking at it.
I'm still curious how they got the Apple access point through the FCC
testing back then. Well, I guess we'll never know for sure.
 
> There were plenty of people advertising the AR9160 as a "3x3 MIMO"
> when it's a 2x2 stream device with 3 antennas. I know this confused me
> when I started tinkering with wifi stuff.
AFAIK the 3x3 really just stands for the amount of tx X rx chains [and
not antennas]. So if the hardware has 3 tx and rx chains it can be sold
as a "3x3" device even if does not support 3 streams.

> Anyway, we're getting off-track here.
we can stop anytime.

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Tue, Oct 4, 2011 at 7:50 AM, Adrian Chadd <adrian@freebsd.org> wrote:
> Also whilst I'm at it, "SPECTRUM_MANAGEMENT" is a very broad flag to set.
>
> For example: you may not want to do DFS on the AR5416 NICs because (as
> documented in the open hal and earlier ath9k bits) there isn't support
> for radar pulses on the ext channel. So even if you had a successful
> DFS algorithm for this NIC, you'd have to somehow tell the DFS
> machinery that HT40+DFS channels aren't supported but HT20+DFS
> channels are.

Good point. I simply rather start out with the best possible DFS
support on Linux and go with the best hardware we have instead of
dealing with old hardware. Think about the support issues that can
come up with supporting the above. I rather simply not deal with it as
I also do not care about Turbo crap. Let legacy crap die.

> But then, the AR5416 supports per-packet TPC, so you could use it in
> STA mode perfectly fine and it'd support that part of spectrum
> management. Since you get per-frame RSSI of RX'ed frames, you can
> support the spectrum power histogram IE.

TPC is not implemented even in a lot of proprietary code bases,
although TPC is part of 802.11h its requirements are me by statically
reducing the maximum EIRP by 3 dB in frequency bands requiring TPC in
consideration for interference with satellites. In my latest
evaluation of TPC the only thing we want to do is simply enable the
option to explicitly state the max delta on power in consideration for
TPC in such a way that *if* TPC is implemented we can reduce the
reduction. But given that this is hardware specific and vendor
specific and not many people implement it right now I frankly don't
care too much about it. DFS is a bigger aspect.

  Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Tue, Oct 4, 2011 at 7:50 AM, Adrian Chadd <adrian@freebsd.org> wrote:
> Also whilst I'm at it, "SPECTRUM_MANAGEMENT" is a very broad flag to set.
>
> For example: you may not want to do DFS on the AR5416 NICs because (as
> documented in the open hal and earlier ath9k bits) there isn't support
> for radar pulses on the ext channel. So even if you had a successful
> DFS algorithm for this NIC, you'd have to somehow tell the DFS
> machinery that HT40+DFS channels aren't supported but HT20+DFS
> channels are.

Good point. I simply rather start out with the best possible DFS
support on Linux and go with the best hardware we have instead of
dealing with old hardware. Think about the support issues that can
come up with supporting the above. I rather simply not deal with it as
I also do not care about Turbo crap. Let legacy crap die.

> But then, the AR5416 supports per-packet TPC, so you could use it in
> STA mode perfectly fine and it'd support that part of spectrum
> management. Since you get per-frame RSSI of RX'ed frames, you can
> support the spectrum power histogram IE.

TPC is not implemented even in a lot of proprietary code bases,
although TPC is part of 802.11h its requirements are me by statically
reducing the maximum EIRP by 3 dB in frequency bands requiring TPC in
consideration for interference with satellites. In my latest
evaluation of TPC the only thing we want to do is simply enable the
option to explicitly state the max delta on power in consideration for
TPC in such a way that *if* TPC is implemented we can reduce the
reduction. But given that this is hardware specific and vendor
specific and not many people implement it right now I frankly don't
care too much about it. DFS is a bigger aspect.

  Luis

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

On 10/04/2011 04:42 PM, Christian Lamparter wrote:
> On Tuesday, October 04, 2011 04:17:35 PM Zefir Kurtisi wrote:
>> On 10/04/2011 03:38 PM, Christian Lamparter wrote:
>>> On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
>>>> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
>>>> <chunkeey@googlemail.com> wrote:
>>>>> On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
>>>>>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>>>>>>
>>>>>>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>>>>>>> ---
>>>>>>>  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
>>>>>>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>>>>>> index e8aeb98..5defebe 100644
>>>>>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>>>>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>>>>>> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>>>>>>>                        "Unable to reset channel, reset status %d\n", r);
>>>>>>>                goto out;
>>>>>>>        }
>>>>>>> +#ifdef CONFIG_ATH9K_DFS
>>>>>>
>>>>>> Please spare the #ifdef and just call something within dfs.c, then
>>>>>> dfs.h would wrap it to nothing if DFS is disabled.
>>>>> Why would anyone want to disable DFS driver support?
>>>>> I would say: drop the ifdefs altogether since DFS
>>>>> is and will be "required".
>>>>
>>>> Because DFS requires to be properly tested before being enabled.
>>> Testing if a driver detects a pulse is "trivial" compared to the
>>> stuff mac80211/cfg80211 and hostapd will have to do to make a
>>> channel-change as smooth as possible. I think if there's a DFS
>>> "OFF" switch, it should be in hostapd and I hope more people
>>> agree on this one.  
>>>
>> Yes on both. Work on the management part of the DFS module has just
>> been started by TI guys. When this is in, hostapd will be able to
>> query the driver's DFS detection capabilities and leave DFS channels
>> disabled for those devices with no (or insufficient) support
>> (like it is generally done today for DFS channels).
>>
>> The proper way for a driver's OFF switch would then be to just
>> announce missing DFS capabilities.
> Actually, I think we already have a flag for such a
> purpose:
>  * @IEEE80211_HW_SPECTRUM_MGMT:
>  *      Hardware supports spectrum management defined in 802.11h
> AFAIK 802.11h[now integrated in 802.11-2007] included DFS, right? 
> 
Yes, 802.11h includes DFS and TPC.

But this flag is already used (i.e. is set by ath9k) without having DFS support so far. Either it is having some different interpretation, or it is just set wrong. This was irrelevant so far, since there is no DFS support in mac80211 yet and might require to clarify the specs for this flag.

>>>> You may also want to simply disable DFS if you do not want to
>>>> deal with the regulatory test implications of having it enabled.
>>> AFAIK you can't "simply" disable the DFS requirement: hostapd
>>> (hw_features.c), [cfg80211] (checks if tx on secondary channel
>>> is possible) and mac80211 (tx.c) all have checks. Indeed, the
>>> easiest way is to modify crda's database. So there's no need
>>> for an extra compile-time option.
>>>
>> There might be a demand for conditional compiling in addition to
>> DFS capabilities announcements to reduce memory footprint, since
>> (especially) pattern matching algorithms will increase it significantly.
> I don't think memory footprint is such a big problem. After all ath9k
> has around 170 kb [please correct me if I'm wrong here] of initvals
> for all supported solutions since AR5008.
> 
It might be even more. But this does not imply that it is not worth to save some bytes, IMHO.

> Regards,
> 	Chr

Zefir

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

On 10/04/2011 04:42 PM, Christian Lamparter wrote:
> On Tuesday, October 04, 2011 04:17:35 PM Zefir Kurtisi wrote:
>> On 10/04/2011 03:38 PM, Christian Lamparter wrote:
>>> On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
>>>> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
>>>> <chunkeey@googlemail.com> wrote:
>>>>> On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
>>>>>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>>>>>>
>>>>>>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>>>>>>> ---
>>>>>>>  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
>>>>>>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>>>>>> index e8aeb98..5defebe 100644
>>>>>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>>>>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>>>>>> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>>>>>>>                        "Unable to reset channel, reset status %d\n", r);
>>>>>>>                goto out;
>>>>>>>        }
>>>>>>> +#ifdef CONFIG_ATH9K_DFS
>>>>>>
>>>>>> Please spare the #ifdef and just call something within dfs.c, then
>>>>>> dfs.h would wrap it to nothing if DFS is disabled.
>>>>> Why would anyone want to disable DFS driver support?
>>>>> I would say: drop the ifdefs altogether since DFS
>>>>> is and will be "required".
>>>>
>>>> Because DFS requires to be properly tested before being enabled.
>>> Testing if a driver detects a pulse is "trivial" compared to the
>>> stuff mac80211/cfg80211 and hostapd will have to do to make a
>>> channel-change as smooth as possible. I think if there's a DFS
>>> "OFF" switch, it should be in hostapd and I hope more people
>>> agree on this one.  
>>>
>> Yes on both. Work on the management part of the DFS module has just
>> been started by TI guys. When this is in, hostapd will be able to
>> query the driver's DFS detection capabilities and leave DFS channels
>> disabled for those devices with no (or insufficient) support
>> (like it is generally done today for DFS channels).
>>
>> The proper way for a driver's OFF switch would then be to just
>> announce missing DFS capabilities.
> Actually, I think we already have a flag for such a
> purpose:
>  * @IEEE80211_HW_SPECTRUM_MGMT:
>  *      Hardware supports spectrum management defined in 802.11h
> AFAIK 802.11h[now integrated in 802.11-2007] included DFS, right? 
> 
Yes, 802.11h includes DFS and TPC.

But this flag is already used (i.e. is set by ath9k) without having DFS support so far. Either it is having some different interpretation, or it is just set wrong. This was irrelevant so far, since there is no DFS support in mac80211 yet and might require to clarify the specs for this flag.

>>>> You may also want to simply disable DFS if you do not want to
>>>> deal with the regulatory test implications of having it enabled.
>>> AFAIK you can't "simply" disable the DFS requirement: hostapd
>>> (hw_features.c), [cfg80211] (checks if tx on secondary channel
>>> is possible) and mac80211 (tx.c) all have checks. Indeed, the
>>> easiest way is to modify crda's database. So there's no need
>>> for an extra compile-time option.
>>>
>> There might be a demand for conditional compiling in addition to
>> DFS capabilities announcements to reduce memory footprint, since
>> (especially) pattern matching algorithms will increase it significantly.
> I don't think memory footprint is such a big problem. After all ath9k
> has around 170 kb [please correct me if I'm wrong here] of initvals
> for all supported solutions since AR5008.
> 
It might be even more. But this does not imply that it is not worth to save some bytes, IMHO.

> Regards,
> 	Chr

Zefir

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Tue, Oct 4, 2011 at 7:17 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> On 10/04/2011 03:38 PM, Christian Lamparter wrote:
>> On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
>>> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
>>> <chunkeey@googlemail.com> wrote:
>>>> On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
>>>>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>>>>>
>>>>>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>>>>>> ---
>>>>>>  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
>>>>>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>>>>> index e8aeb98..5defebe 100644
>>>>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>>>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>>>>> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>>>>>>                        "Unable to reset channel, reset status %d\n", r);
>>>>>>                goto out;
>>>>>>        }
>>>>>> +#ifdef CONFIG_ATH9K_DFS
>>>>>
>>>>> Please spare the #ifdef and just call something within dfs.c, then
>>>>> dfs.h would wrap it to nothing if DFS is disabled.
>>>> Why would anyone want to disable DFS driver support?
>>>> I would say: drop the ifdefs altogether since DFS
>>>> is and will be "required".
>>>
>>> Because DFS requires to be properly tested before being enabled.
>> Testing if a driver detects a pulse is "trivial" compared to the
>> stuff mac80211/cfg80211 and hostapd will have to do to make a
>> channel-change as smooth as possible. I think if there's a DFS
>> "OFF" switch, it should be in hostapd and I hope more people
>> agree on this one.
>>
> Yes on both. Work on the management part of the DFS module has just been started by TI guys. When this is in, hostapd will be able to query the driver's DFS detection capabilities and leave DFS channels disabled for those devices with no (or insufficient) support (like it is generally done today for DFS channels).
>
> The proper way for a driver's OFF switch would then be to just announce missing DFS capabilities.

And this is what I meant by leaving a kernel build option available
for each driver to enable/disable DFS. If a vendor does not want to
deal with the question of enabling DFS they can disable it on the
driver front.

>>> You may also want to simply disable DFS if you do not want to
>>> deal with the regulatory test implications of having it enabled.
>> AFAIK you can't "simply" disable the DFS requirement: hostapd
>> (hw_features.c), [cfg80211] (checks if tx on secondary channel
>> is possible) and mac80211 (tx.c) all have checks. Indeed, the
>> easiest way is to modify crda's database. So there's no need
>> for an extra compile-time option.
>>
> There might be a demand for conditional compiling in addition to DFS capabilities announcements to reduce memory footprint, since (especially) pattern matching algorithms will increase it significantly.

I doubt space considerations will be that much given that we don't
even have build options to disable hardware families, at least nbd had
determine already that separating families at build time wouldn't save
us much. Given that -- I suspect using build size as an argument for
introducing a flag here doesn't carry much weight. The argument I'm
using is to simply enable integrators to decide whether or not to have
to deal with testing such features.

 Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Tue, Oct 4, 2011 at 7:17 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> On 10/04/2011 03:38 PM, Christian Lamparter wrote:
>> On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
>>> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
>>> <chunkeey@googlemail.com> wrote:
>>>> On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
>>>>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>>>>>
>>>>>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>>>>>> ---
>>>>>> ?drivers/net/wireless/ath/ath9k/main.c | ? 12 ++++++++++++
>>>>>> ?1 files changed, 12 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>>>>> index e8aeb98..5defebe 100644
>>>>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>>>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>>>>> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>>>>>> ? ? ? ? ? ? ? ? ? ? ? ?"Unable to reset channel, reset status %d\n", r);
>>>>>> ? ? ? ? ? ? ? ?goto out;
>>>>>> ? ? ? ?}
>>>>>> +#ifdef CONFIG_ATH9K_DFS
>>>>>
>>>>> Please spare the #ifdef and just call something within dfs.c, then
>>>>> dfs.h would wrap it to nothing if DFS is disabled.
>>>> Why would anyone want to disable DFS driver support?
>>>> I would say: drop the ifdefs altogether since DFS
>>>> is and will be "required".
>>>
>>> Because DFS requires to be properly tested before being enabled.
>> Testing if a driver detects a pulse is "trivial" compared to the
>> stuff mac80211/cfg80211 and hostapd will have to do to make a
>> channel-change as smooth as possible. I think if there's a DFS
>> "OFF" switch, it should be in hostapd and I hope more people
>> agree on this one.
>>
> Yes on both. Work on the management part of the DFS module has just been started by TI guys. When this is in, hostapd will be able to query the driver's DFS detection capabilities and leave DFS channels disabled for those devices with no (or insufficient) support (like it is generally done today for DFS channels).
>
> The proper way for a driver's OFF switch would then be to just announce missing DFS capabilities.

And this is what I meant by leaving a kernel build option available
for each driver to enable/disable DFS. If a vendor does not want to
deal with the question of enabling DFS they can disable it on the
driver front.

>>> You may also want to simply disable DFS if you do not want to
>>> deal with the regulatory test implications of having it enabled.
>> AFAIK you can't "simply" disable the DFS requirement: hostapd
>> (hw_features.c), [cfg80211] (checks if tx on secondary channel
>> is possible) and mac80211 (tx.c) all have checks. Indeed, the
>> easiest way is to modify crda's database. So there's no need
>> for an extra compile-time option.
>>
> There might be a demand for conditional compiling in addition to DFS capabilities announcements to reduce memory footprint, since (especially) pattern matching algorithms will increase it significantly.

I doubt space considerations will be that much given that we don't
even have build options to disable hardware families, at least nbd had
determine already that separating families at build time wouldn't save
us much. Given that -- I suspect using build size as an argument for
introducing a flag here doesn't carry much weight. The argument I'm
using is to simply enable integrators to decide whether or not to have
to deal with testing such features.

 Luis

Re: [ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Tue, Oct 4, 2011 at 6:38 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
>> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
>> <chunkeey@googlemail.com> wrote:
>> > On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
>> >> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>> >> >
>> >> > Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>> >> > ---
>> >> >  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
>> >> >  1 files changed, 12 insertions(+), 0 deletions(-)
>> >> >
>> >> > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> >> > index e8aeb98..5defebe 100644
>> >> > --- a/drivers/net/wireless/ath/ath9k/main.c
>> >> > +++ b/drivers/net/wireless/ath/ath9k/main.c
>> >> > @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>> >> >                        "Unable to reset channel, reset status %d\n", r);
>> >> >                goto out;
>> >> >        }
>> >> > +#ifdef CONFIG_ATH9K_DFS
>> >>
>> >> Please spare the #ifdef and just call something within dfs.c, then
>> >> dfs.h would wrap it to nothing if DFS is disabled.
>> > Why would anyone want to disable DFS driver support?
>> > I would say: drop the ifdefs altogether since DFS
>> > is and will be "required".
>>
>> Because DFS requires to be properly tested before being enabled.
> Testing if a driver detects a pulse is "trivial" compared to the
> stuff mac80211/cfg80211 and hostapd will have to do to make a
> channel-change as smooth as possible. I think if there's a DFS
> "OFF" switch, it should be in hostapd and I hope more people
> agree on this one.

You do have a good point, but I disagree that you do not need to test
/ regress test hardware / driver code for DFS. This is what I'm
talking about. But yes, userspace also submits itself to the same
criteria.

>> You may also want to simply disable DFS if you do not want to
>> deal with the regulatory test implications of having it enabled.
> AFAIK you can't "simply" disable the DFS requirement: hostapd
> (hw_features.c), [cfg80211] (checks if tx on secondary channel
> is possible) and mac80211 (tx.c) all have checks. Indeed, the
> easiest way is to modify crda's database. So there's no need
> for an extra compile-time option.

No, DFS is set for certain channels on wireless-regdb/CRDA, I just
posted DFS master region support for wireless-regdb and CRDA. Apart
from this we then need driver support. To get DFS you need all of
these + hostapd part. Each one has its own set of components and does
deserve its own set of tests and review.

  Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Tue, Oct 4, 2011 at 6:38 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
>> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
>> <chunkeey@googlemail.com> wrote:
>> > On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
>> >> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>> >> >
>> >> > Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>> >> > ---
>> >> > ?drivers/net/wireless/ath/ath9k/main.c | ? 12 ++++++++++++
>> >> > ?1 files changed, 12 insertions(+), 0 deletions(-)
>> >> >
>> >> > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> >> > index e8aeb98..5defebe 100644
>> >> > --- a/drivers/net/wireless/ath/ath9k/main.c
>> >> > +++ b/drivers/net/wireless/ath/ath9k/main.c
>> >> > @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>> >> > ? ? ? ? ? ? ? ? ? ? ? ?"Unable to reset channel, reset status %d\n", r);
>> >> > ? ? ? ? ? ? ? ?goto out;
>> >> > ? ? ? ?}
>> >> > +#ifdef CONFIG_ATH9K_DFS
>> >>
>> >> Please spare the #ifdef and just call something within dfs.c, then
>> >> dfs.h would wrap it to nothing if DFS is disabled.
>> > Why would anyone want to disable DFS driver support?
>> > I would say: drop the ifdefs altogether since DFS
>> > is and will be "required".
>>
>> Because DFS requires to be properly tested before being enabled.
> Testing if a driver detects a pulse is "trivial" compared to the
> stuff mac80211/cfg80211 and hostapd will have to do to make a
> channel-change as smooth as possible. I think if there's a DFS
> "OFF" switch, it should be in hostapd and I hope more people
> agree on this one.

You do have a good point, but I disagree that you do not need to test
/ regress test hardware / driver code for DFS. This is what I'm
talking about. But yes, userspace also submits itself to the same
criteria.

>> You may also want to simply disable DFS if you do not want to
>> deal with the regulatory test implications of having it enabled.
> AFAIK you can't "simply" disable the DFS requirement: hostapd
> (hw_features.c), [cfg80211] (checks if tx on secondary channel
> is possible) and mac80211 (tx.c) all have checks. Indeed, the
> easiest way is to modify crda's database. So there's no need
> for an extra compile-time option.

No, DFS is set for certain channels on wireless-regdb/CRDA, I just
posted DFS master region support for wireless-regdb and CRDA. Apart
from this we then need driver support. To get DFS you need all of
these + hostapd part. Each one has its own set of components and does
deserve its own set of tests and review.

  Luis

Re: [ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Thursday, October 06, 2011 12:27:03 AM Luis R. Rodriguez wrote:
> On Tue, Oct 4, 2011 at 6:38 AM, Christian Lamparter
> <chunkeey@googlemail.com> wrote:
> > On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
> >> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
> >> <chunkeey@googlemail.com> wrote:
> >> > On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
> >> >> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> >> >> >
> >> >> > Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> >> >> > ---
> >> >> >  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
> >> >> >  1 files changed, 12 insertions(+), 0 deletions(-)
> >> >> >
> >> >> > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> >> >> > index e8aeb98..5defebe 100644
> >> >> > --- a/drivers/net/wireless/ath/ath9k/main.c
> >> >> > +++ b/drivers/net/wireless/ath/ath9k/main.c
> >> >> > [...]
> >> >> > +#ifdef CONFIG_ATH9K_DFS
> >> >>
> >> >> Please spare the #ifdef and just call something within dfs.c, then
> >> >> dfs.h would wrap it to nothing if DFS is disabled.
> >> > Why would anyone want to disable DFS driver support?
> >> > I would say: drop the ifdefs altogether since DFS
> >> > is and will be "required".
> >>
> >> Because DFS requires to be properly tested before being enabled.
> > Testing if a driver detects a pulse is "trivial" compared to the
> > stuff mac80211/cfg80211 and hostapd will have to do to make a
> > channel-change as smooth as possible. I think if there's a DFS
> > "OFF" switch, it should be in hostapd and I hope more people
> > agree on this one.
> 
> You do have a good point, but I disagree that you do not need to test
> / regress test hardware / driver code for DFS.

Actually, you are sort of contradicting yourself here.

Take a look at your "wireless: add DFS master support" patch
series. I don't see any IFDEFs to select between the old
and the new "way" even though you know full well that there's
some black magic going on.

http://permalink.gmane.org/gmane.linux.kernel.wireless.general/78455
Quote:
"Here's a puzzle though... If we change this series to use the other
pad byte that was available, the first pad byte, instead of the last
one, we loose backward compatibility support and I cannot figure out
why."

[Note: This is just an recent enough example. I do think I could come
up with a better one, e.g.: why didn't athXk have a compile-time
switch to disable ANI when it was introduced because it can(and has?)
caused some regressions as well?]

so, why do you want a useless compile-time for *this option* *now*?
Is this something about politics/laws I don't know about [I'm just
curious, because I don't really buy "testing" here, since Zefir
obviously has a working prototype and Atheros has a working and
certificated codebase as well which he can access and base his
work on. So I don't think its that unstable and needs added
ugliness.]
> This is what I'm talking about. But yes, userspace also submits
> itself to the same criteria.
> >> You may also want to simply disable DFS if you do not want to
> >> deal with the regulatory test implications of having it enabled.
> > AFAIK you can't "simply" disable the DFS requirement: hostapd
> > (hw_features.c), [cfg80211] (checks if tx on secondary channel
> > is possible) and mac80211 (tx.c) all have checks. Indeed, the
> > easiest way is to modify crda's database. So there's no need
> > for an extra compile-time option.
> 
> No, DFS is set for certain channels on wireless-regdb/CRDA, I just
> posted DFS master region support for wireless-regdb and CRDA. Apart
> from this we then need driver support. To get DFS you need all of
> these + hostapd part. Each one has its own set of components and does
> deserve its own set of tests and review.
This "deserve its own set of tests and review". Does it translate in:
"ath9k [every driver], mac80211, cfg80211 and hostapd need extra
DFS IFDEFS?". In fact, ifdefs make it harder to do reviews, because
sometimes you just forget the IFDEF/ELSIF/ELSE context of the code.
And regression testing can be done by "git bisect". In fact, isn't
this what git bisect is for?

Regards,
	Chr

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Christian Lamparter]

On Thursday, October 06, 2011 12:27:03 AM Luis R. Rodriguez wrote:
> On Tue, Oct 4, 2011 at 6:38 AM, Christian Lamparter
> <chunkeey@googlemail.com> wrote:
> > On Monday, October 03, 2011 09:31:12 PM Luis R. Rodriguez wrote:
> >> On Mon, Oct 3, 2011 at 12:24 PM, Christian Lamparter
> >> <chunkeey@googlemail.com> wrote:
> >> > On Monday, October 03, 2011 08:27:39 PM Luis R. Rodriguez wrote:
> >> >> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> >> >> >
> >> >> > Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> >> >> > ---
> >> >> >  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
> >> >> >  1 files changed, 12 insertions(+), 0 deletions(-)
> >> >> >
> >> >> > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> >> >> > index e8aeb98..5defebe 100644
> >> >> > --- a/drivers/net/wireless/ath/ath9k/main.c
> >> >> > +++ b/drivers/net/wireless/ath/ath9k/main.c
> >> >> > [...]
> >> >> > +#ifdef CONFIG_ATH9K_DFS
> >> >>
> >> >> Please spare the #ifdef and just call something within dfs.c, then
> >> >> dfs.h would wrap it to nothing if DFS is disabled.
> >> > Why would anyone want to disable DFS driver support?
> >> > I would say: drop the ifdefs altogether since DFS
> >> > is and will be "required".
> >>
> >> Because DFS requires to be properly tested before being enabled.
> > Testing if a driver detects a pulse is "trivial" compared to the
> > stuff mac80211/cfg80211 and hostapd will have to do to make a
> > channel-change as smooth as possible. I think if there's a DFS
> > "OFF" switch, it should be in hostapd and I hope more people
> > agree on this one.
> 
> You do have a good point, but I disagree that you do not need to test
> / regress test hardware / driver code for DFS.

Actually, you are sort of contradicting yourself here.

Take a look at your "wireless: add DFS master support" patch
series. I don't see any IFDEFs to select between the old
and the new "way" even though you know full well that there's
some black magic going on.

http://permalink.gmane.org/gmane.linux.kernel.wireless.general/78455
Quote:
"Here's a puzzle though... If we change this series to use the other
pad byte that was available, the first pad byte, instead of the last
one, we loose backward compatibility support and I cannot figure out
why."

[Note: This is just an recent enough example. I do think I could come
up with a better one, e.g.: why didn't athXk have a compile-time
switch to disable ANI when it was introduced because it can(and has?)
caused some regressions as well?]

so, why do you want a useless compile-time for *this option* *now*?
Is this something about politics/laws I don't know about [I'm just
curious, because I don't really buy "testing" here, since Zefir
obviously has a working prototype and Atheros has a working and
certificated codebase as well which he can access and base his
work on. So I don't think its that unstable and needs added
ugliness.]
> This is what I'm talking about. But yes, userspace also submits
> itself to the same criteria.
> >> You may also want to simply disable DFS if you do not want to
> >> deal with the regulatory test implications of having it enabled.
> > AFAIK you can't "simply" disable the DFS requirement: hostapd
> > (hw_features.c), [cfg80211] (checks if tx on secondary channel
> > is possible) and mac80211 (tx.c) all have checks. Indeed, the
> > easiest way is to modify crda's database. So there's no need
> > for an extra compile-time option.
> 
> No, DFS is set for certain channels on wireless-regdb/CRDA, I just
> posted DFS master region support for wireless-regdb and CRDA. Apart
> from this we then need driver support. To get DFS you need all of
> these + hostapd part. Each one has its own set of components and does
> deserve its own set of tests and review.
This "deserve its own set of tests and review". Does it translate in:
"ath9k [every driver], mac80211, cfg80211 and hostapd need extra
DFS IFDEFS?". In fact, ifdefs make it harder to do reviews, because
sometimes you just forget the IFDEF/ELSIF/ELSE context of the code.
And regression testing can be done by "git bisect". In fact, isn't
this what git bisect is for?

Regards,
	Chr

Re: [ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Thu, Oct 6, 2011 at 9:49 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Thursday, October 06, 2011 12:27:03 AM Luis R. Rodriguez wrote:
>> You do have a good point, but I disagree that you do not need to test
>> / regress test hardware / driver code for DFS.
>
> Actually, you are sort of contradicting yourself here.
>
> Take a look at your "wireless: add DFS master support" patch
> series. I don't see any IFDEFs to select between the old
> and the new "way" even though you know full well that there's
> some black magic going on.

Well its true, but the regdb and CRDA stuff is can have the DFS master
region support, its just the mapping, not a technical implementation.
IMHO DFS support should be a kconfig on both the 802.11 stack and
driver part, and the driver part depend on the 802.11 stack option.

> http://permalink.gmane.org/gmane.linux.kernel.wireless.general/78455
> Quote:
> "Here's a puzzle though... If we change this series to use the other
> pad byte that was available, the first pad byte, instead of the last
> one, we loose backward compatibility support and I cannot figure out
> why."

That's an implementation weirdness with python pack on generating the
binary output, not a DFS issue per se.

> [Note: This is just an recent enough example. I do think I could come
> up with a better one, e.g.: why didn't athXk have a compile-time
> switch to disable ANI when it was introduced because it can(and has?)
> caused some regressions as well?]

No ANI is *required*, without it the cards are useless. ANI is also
properly tested and validated by our systems team. Have you tried
disabling ANI? When we introduced a revamp of the new ANI though we
did enable users to use the new ANI for older generation cards, the
module parameter is still there.

DFS is a different beast. Testing DFS cannot be compared to testing
ANI, DFS has a slew of different tests you need to run against, and
then *if* you do want to sell cards in certain geographies with
support for DFS channels you need to get proper regulatory
certification for an intended radiator that supports DFS properly. If
you get this certification technically you cannot even expose a knob
to users to disable DFS when operating on a DFS channel.

> so, why do you want a useless compile-time for *this option* *now*?

What I want to do is enable an option which lets distributors disable
DFS if they don't want to even deal with the question of whether or
not a card had DFS support enabled through driver support.

> Is this something about politics/laws I don't know about [I'm just
> curious, because I don't really buy "testing" here, since Zefir
> obviously has a working prototype

No, we haven't passed certified testing with this code yet.

> and Atheros has a working and certificated codebase as well

Exactly, and that is the code we are enabling Neratec with to be able
to upstream into the kernel for, but the code being referenced uses a
different 802.11 stack, different base driver, etc.

> which he can access and base his work on. So I don't think its that unstable and needs added
> ugliness.]

Its the same with 802.11s, its new code and people may want to disable
this crap to not deal with it in code path or even consider the
support for it.

>> This is what I'm talking about. But yes, userspace also submits
>> itself to the same criteria.
>> >> You may also want to simply disable DFS if you do not want to
>> >> deal with the regulatory test implications of having it enabled.
>> > AFAIK you can't "simply" disable the DFS requirement: hostapd
>> > (hw_features.c), [cfg80211] (checks if tx on secondary channel
>> > is possible) and mac80211 (tx.c) all have checks. Indeed, the
>> > easiest way is to modify crda's database. So there's no need
>> > for an extra compile-time option.
>>
>> No, DFS is set for certain channels on wireless-regdb/CRDA, I just
>> posted DFS master region support for wireless-regdb and CRDA. Apart
>> from this we then need driver support. To get DFS you need all of
>> these + hostapd part. Each one has its own set of components and does
>> deserve its own set of tests and review.
>
> This "deserve its own set of tests and review". Does it translate in:
> "ath9k [every driver], mac80211, cfg80211 and hostapd need extra
> DFS IFDEFS?".

I still have yet to see patches for cfg80211 / mac80211 for DFS. What
I'm saying is we have an kconfig option on the 802.11 stack to  allow
us to disable DFS support, and the driver respective component depend
on it.

> In fact, ifdefs make it harder to do reviews, because
> sometimes you just forget the IFDEF/ELSIF/ELSE context of the code.

I hate ifdefs, and if you read my e-mail carefully what I was
suggesting was to do this properly by building all the code if the
kconfig option is enabled therefore eliminating all ifdef junk from
the code and only leaving it for header files.

> And regression testing can be done by "git bisect". In fact, isn't
> this what git bisect is for?

There is a difference between regression testing and finding the
culprit of an issue. git bisect is used to find the culprit of an
issue. However if you want to ensure code does not regress you need to
ensure to run a suite of tests on code after a delta is applied. Only
if you find an issue do you then use git bisect.

I want proper test infrastructure set up before I even consider
enabling any DFS code upstream for ath9k.

  Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Thu, Oct 6, 2011 at 9:49 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Thursday, October 06, 2011 12:27:03 AM Luis R. Rodriguez wrote:
>> You do have a good point, but I disagree that you do not need to test
>> / regress test hardware / driver code for DFS.
>
> Actually, you are sort of contradicting yourself here.
>
> Take a look at your "wireless: add DFS master support" patch
> series. I don't see any IFDEFs to select between the old
> and the new "way" even though you know full well that there's
> some black magic going on.

Well its true, but the regdb and CRDA stuff is can have the DFS master
region support, its just the mapping, not a technical implementation.
IMHO DFS support should be a kconfig on both the 802.11 stack and
driver part, and the driver part depend on the 802.11 stack option.

> http://permalink.gmane.org/gmane.linux.kernel.wireless.general/78455
> Quote:
> "Here's a puzzle though... If we change this series to use the other
> pad byte that was available, the first pad byte, instead of the last
> one, we loose backward compatibility support and I cannot figure out
> why."

That's an implementation weirdness with python pack on generating the
binary output, not a DFS issue per se.

> [Note: This is just an recent enough example. I do think I could come
> up with a better one, e.g.: why didn't athXk have a compile-time
> switch to disable ANI when it was introduced because it can(and has?)
> caused some regressions as well?]

No ANI is *required*, without it the cards are useless. ANI is also
properly tested and validated by our systems team. Have you tried
disabling ANI? When we introduced a revamp of the new ANI though we
did enable users to use the new ANI for older generation cards, the
module parameter is still there.

DFS is a different beast. Testing DFS cannot be compared to testing
ANI, DFS has a slew of different tests you need to run against, and
then *if* you do want to sell cards in certain geographies with
support for DFS channels you need to get proper regulatory
certification for an intended radiator that supports DFS properly. If
you get this certification technically you cannot even expose a knob
to users to disable DFS when operating on a DFS channel.

> so, why do you want a useless compile-time for *this option* *now*?

What I want to do is enable an option which lets distributors disable
DFS if they don't want to even deal with the question of whether or
not a card had DFS support enabled through driver support.

> Is this something about politics/laws I don't know about [I'm just
> curious, because I don't really buy "testing" here, since Zefir
> obviously has a working prototype

No, we haven't passed certified testing with this code yet.

> and Atheros has a working and certificated codebase as well

Exactly, and that is the code we are enabling Neratec with to be able
to upstream into the kernel for, but the code being referenced uses a
different 802.11 stack, different base driver, etc.

> which he can access and base his work on. So I don't think its that unstable and needs added
> ugliness.]

Its the same with 802.11s, its new code and people may want to disable
this crap to not deal with it in code path or even consider the
support for it.

>> This is what I'm talking about. But yes, userspace also submits
>> itself to the same criteria.
>> >> You may also want to simply disable DFS if you do not want to
>> >> deal with the regulatory test implications of having it enabled.
>> > AFAIK you can't "simply" disable the DFS requirement: hostapd
>> > (hw_features.c), [cfg80211] (checks if tx on secondary channel
>> > is possible) and mac80211 (tx.c) all have checks. Indeed, the
>> > easiest way is to modify crda's database. So there's no need
>> > for an extra compile-time option.
>>
>> No, DFS is set for certain channels on wireless-regdb/CRDA, I just
>> posted DFS master region support for wireless-regdb and CRDA. Apart
>> from this we then need driver support. To get DFS you need all of
>> these + hostapd part. Each one has its own set of components and does
>> deserve its own set of tests and review.
>
> This "deserve its own set of tests and review". Does it translate in:
> "ath9k [every driver], mac80211, cfg80211 and hostapd need extra
> DFS IFDEFS?".

I still have yet to see patches for cfg80211 / mac80211 for DFS. What
I'm saying is we have an kconfig option on the 802.11 stack to  allow
us to disable DFS support, and the driver respective component depend
on it.

> In fact, ifdefs make it harder to do reviews, because
> sometimes you just forget the IFDEF/ELSIF/ELSE context of the code.

I hate ifdefs, and if you read my e-mail carefully what I was
suggesting was to do this properly by building all the code if the
kconfig option is enabled therefore eliminating all ifdef junk from
the code and only leaving it for header files.

> And regression testing can be done by "git bisect". In fact, isn't
> this what git bisect is for?

There is a difference between regression testing and finding the
culprit of an issue. git bisect is used to find the culprit of an
issue. However if you want to ensure code does not regress you need to
ensure to run a suite of tests on code after a delta is applied. Only
if you find an issue do you then use git bisect.

I want proper test infrastructure set up before I even consider
enabling any DFS code upstream for ath9k.

  Luis

Re: [ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Thu, Oct 6, 2011 at 11:36 AM, Luis R. Rodriguez
<rodrigue@qca.qualcomm.com> wrote:
> On Thu, Oct 6, 2011 at 9:49 AM, Christian Lamparter
> <chunkeey@googlemail.com> wrote:

>> Is this something about politics/laws I don't know about [I'm just
>> curious, because I don't really buy "testing" here, since Zefir
>> obviously has a working prototype
>
> No, we haven't passed certified testing with this code yet.

And to be clear, no there is no political issues here or concerns. The
law however is strict about DFS, extremely strict and I want to make
sure upstream code respect it well and we get the best DFS test
infrastructure out there to test this to ensure we get to the point to
always have DFS properly working upstream.

  Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Thu, Oct 6, 2011 at 11:36 AM, Luis R. Rodriguez
<rodrigue@qca.qualcomm.com> wrote:
> On Thu, Oct 6, 2011 at 9:49 AM, Christian Lamparter
> <chunkeey@googlemail.com> wrote:

>> Is this something about politics/laws I don't know about [I'm just
>> curious, because I don't really buy "testing" here, since Zefir
>> obviously has a working prototype
>
> No, we haven't passed certified testing with this code yet.

And to be clear, no there is no political issues here or concerns. The
law however is strict about DFS, extremely strict and I want to make
sure upstream code respect it well and we get the best DFS test
infrastructure out there to test this to ensure we get to the point to
always have DFS properly working upstream.

  Luis

Re: [ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

On 06.10.2011 20:36, Luis R. Rodriguez wrote:
> On Thu, Oct 6, 2011 at 9:49 AM, Christian Lamparter
> <chunkeey@googlemail.com>  wrote:
>> On Thursday, October 06, 2011 12:27:03 AM Luis R. Rodriguez wrote:
>>> You do have a good point, but I disagree that you do not need to test
>>> / regress test hardware / driver code for DFS.
>>
>> Actually, you are sort of contradicting yourself here.
>>
>> Take a look at your "wireless: add DFS master support" patch
>> series. I don't see any IFDEFs to select between the old
>> and the new "way" even though you know full well that there's
>> some black magic going on.
>
> Well its true, but the regdb and CRDA stuff is can have the DFS master
> region support, its just the mapping, not a technical implementation.
> IMHO DFS support should be a kconfig on both the 802.11 stack and
> driver part, and the driver part depend on the 802.11 stack option.
>
The concept for the management part that will be located in mac80211 and 
hostapd is still WIP at TI, so we can just speculate about its 
implementation for now.

If one consider object size irrelevant (working in the embedded world, I 
tend to not fully agree), it is enough to disable the DFS capability in 
the driver, since DFS requires a full support chain in all three layers 
to enable operation in DFS bands.

>> http://permalink.gmane.org/gmane.linux.kernel.wireless.general/78455
>> Quote:
>> "Here's a puzzle though... If we change this series to use the other
>> pad byte that was available, the first pad byte, instead of the last
>> one, we loose backward compatibility support and I cannot figure out
>> why."
>
> That's an implementation weirdness with python pack on generating the
> binary output, not a DFS issue per se.
>
>> [Note: This is just an recent enough example. I do think I could come
>> up with a better one, e.g.: why didn't athXk have a compile-time
>> switch to disable ANI when it was introduced because it can(and has?)
>> caused some regressions as well?]
>
> No ANI is *required*, without it the cards are useless. ANI is also
> properly tested and validated by our systems team. Have you tried
> disabling ANI? When we introduced a revamp of the new ANI though we
> did enable users to use the new ANI for older generation cards, the
> module parameter is still there.
>
> DFS is a different beast. Testing DFS cannot be compared to testing
> ANI, DFS has a slew of different tests you need to run against, and
> then *if* you do want to sell cards in certain geographies with
> support for DFS channels you need to get proper regulatory
> certification for an intended radiator that supports DFS properly. If
> you get this certification technically you cannot even expose a knob
> to users to disable DFS when operating on a DFS channel.
>
>> so, why do you want a useless compile-time for *this option* *now*?
>
> What I want to do is enable an option which lets distributors disable
> DFS if they don't want to even deal with the question of whether or
> not a card had DFS support enabled through driver support.
>
>> Is this something about politics/laws I don't know about [I'm just
>> curious, because I don't really buy "testing" here, since Zefir
>> obviously has a working prototype
>
> No, we haven't passed certified testing with this code yet.
>
>> and Atheros has a working and certificated codebase as well
>
> Exactly, and that is the code we are enabling Neratec with to be able
> to upstream into the kernel for, but the code being referenced uses a
> different 802.11 stack, different base driver, etc.
>
I see, I should have officially announce the wiki page where we want to 
collect our joint DFS development information [1] to include all those 
valuable thoughts.

For clarification: the existing referenced implementation is not 
portable to ath9k/mac80211. It follows a monolithic approach where the 
DFS detector takes over full processing control after pattern match and 
handles all required management functionality from within. Refactoring 
those processing path to distribute it between driver, mac80211 and 
hostapd is more complicated than a full redesign. FYI, Adrian is 
following this approach for FreeBSD.

For the detector prototype: yes, I worked on some pattern matching 
algorithms and have some working prototype. With the HW reliably 
providing pulse events it's no rocket science to design a radar detector 
that can pass certification (I tested it for ETSI), while it is to 
design one that allows you to use DFS channels (i.e. low false detections).


[1] http://linuxwireless.org/en/developers/DFS/Development

>> which he can access and base his work on. So I don't think its that unstable and needs added
>> ugliness.]
>
> Its the same with 802.11s, its new code and people may want to disable
> this crap to not deal with it in code path or even consider the
> support for it.
>
>>> This is what I'm talking about. But yes, userspace also submits
>>> itself to the same criteria.
>>>>> You may also want to simply disable DFS if you do not want to
>>>>> deal with the regulatory test implications of having it enabled.
>>>> AFAIK you can't "simply" disable the DFS requirement: hostapd
>>>> (hw_features.c), [cfg80211] (checks if tx on secondary channel
>>>> is possible) and mac80211 (tx.c) all have checks. Indeed, the
>>>> easiest way is to modify crda's database. So there's no need
>>>> for an extra compile-time option.
>>>
>>> No, DFS is set for certain channels on wireless-regdb/CRDA, I just
>>> posted DFS master region support for wireless-regdb and CRDA. Apart
>>> from this we then need driver support. To get DFS you need all of
>>> these + hostapd part. Each one has its own set of components and does
>>> deserve its own set of tests and review.
>>
>> This "deserve its own set of tests and review". Does it translate in:
>> "ath9k [every driver], mac80211, cfg80211 and hostapd need extra
>> DFS IFDEFS?".
>
> I still have yet to see patches for cfg80211 / mac80211 for DFS. What
> I'm saying is we have an kconfig option on the 802.11 stack to  allow
> us to disable DFS support, and the driver respective component depend
> on it.
>
As said above, disabling a driver's capability through a Kconfig option 
should be enough (one ifdef per driver).

>> In fact, ifdefs make it harder to do reviews, because
>> sometimes you just forget the IFDEF/ELSIF/ELSE context of the code.
>
> I hate ifdefs, and if you read my e-mail carefully what I was
> suggesting was to do this properly by building all the code if the
> kconfig option is enabled therefore eliminating all ifdef junk from
> the code and only leaving it for header files.
>
>> And regression testing can be done by "git bisect". In fact, isn't
>> this what git bisect is for?
>
> There is a difference between regression testing and finding the
> culprit of an issue. git bisect is used to find the culprit of an
> issue. However if you want to ensure code does not regress you need to
> ensure to run a suite of tests on code after a delta is applied. Only
> if you find an issue do you then use git bisect.
>
> I want proper test infrastructure set up before I even consider
> enabling any DFS code upstream for ath9k.
>
Since regulatory compliance and open source by principle form a 
gray-zone combination [2], testing for sure is essential to keep it more 
white than black ;)

[2] http://linuxwireless.org/en/developers/Regulatory/statement#Principles

>    Luis

Zefir

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

On 06.10.2011 20:36, Luis R. Rodriguez wrote:
> On Thu, Oct 6, 2011 at 9:49 AM, Christian Lamparter
> <chunkeey@googlemail.com>  wrote:
>> On Thursday, October 06, 2011 12:27:03 AM Luis R. Rodriguez wrote:
>>> You do have a good point, but I disagree that you do not need to test
>>> / regress test hardware / driver code for DFS.
>>
>> Actually, you are sort of contradicting yourself here.
>>
>> Take a look at your "wireless: add DFS master support" patch
>> series. I don't see any IFDEFs to select between the old
>> and the new "way" even though you know full well that there's
>> some black magic going on.
>
> Well its true, but the regdb and CRDA stuff is can have the DFS master
> region support, its just the mapping, not a technical implementation.
> IMHO DFS support should be a kconfig on both the 802.11 stack and
> driver part, and the driver part depend on the 802.11 stack option.
>
The concept for the management part that will be located in mac80211 and 
hostapd is still WIP at TI, so we can just speculate about its 
implementation for now.

If one consider object size irrelevant (working in the embedded world, I 
tend to not fully agree), it is enough to disable the DFS capability in 
the driver, since DFS requires a full support chain in all three layers 
to enable operation in DFS bands.

>> http://permalink.gmane.org/gmane.linux.kernel.wireless.general/78455
>> Quote:
>> "Here's a puzzle though... If we change this series to use the other
>> pad byte that was available, the first pad byte, instead of the last
>> one, we loose backward compatibility support and I cannot figure out
>> why."
>
> That's an implementation weirdness with python pack on generating the
> binary output, not a DFS issue per se.
>
>> [Note: This is just an recent enough example. I do think I could come
>> up with a better one, e.g.: why didn't athXk have a compile-time
>> switch to disable ANI when it was introduced because it can(and has?)
>> caused some regressions as well?]
>
> No ANI is *required*, without it the cards are useless. ANI is also
> properly tested and validated by our systems team. Have you tried
> disabling ANI? When we introduced a revamp of the new ANI though we
> did enable users to use the new ANI for older generation cards, the
> module parameter is still there.
>
> DFS is a different beast. Testing DFS cannot be compared to testing
> ANI, DFS has a slew of different tests you need to run against, and
> then *if* you do want to sell cards in certain geographies with
> support for DFS channels you need to get proper regulatory
> certification for an intended radiator that supports DFS properly. If
> you get this certification technically you cannot even expose a knob
> to users to disable DFS when operating on a DFS channel.
>
>> so, why do you want a useless compile-time for *this option* *now*?
>
> What I want to do is enable an option which lets distributors disable
> DFS if they don't want to even deal with the question of whether or
> not a card had DFS support enabled through driver support.
>
>> Is this something about politics/laws I don't know about [I'm just
>> curious, because I don't really buy "testing" here, since Zefir
>> obviously has a working prototype
>
> No, we haven't passed certified testing with this code yet.
>
>> and Atheros has a working and certificated codebase as well
>
> Exactly, and that is the code we are enabling Neratec with to be able
> to upstream into the kernel for, but the code being referenced uses a
> different 802.11 stack, different base driver, etc.
>
I see, I should have officially announce the wiki page where we want to 
collect our joint DFS development information [1] to include all those 
valuable thoughts.

For clarification: the existing referenced implementation is not 
portable to ath9k/mac80211. It follows a monolithic approach where the 
DFS detector takes over full processing control after pattern match and 
handles all required management functionality from within. Refactoring 
those processing path to distribute it between driver, mac80211 and 
hostapd is more complicated than a full redesign. FYI, Adrian is 
following this approach for FreeBSD.

For the detector prototype: yes, I worked on some pattern matching 
algorithms and have some working prototype. With the HW reliably 
providing pulse events it's no rocket science to design a radar detector 
that can pass certification (I tested it for ETSI), while it is to 
design one that allows you to use DFS channels (i.e. low false detections).


[1] http://linuxwireless.org/en/developers/DFS/Development

>> which he can access and base his work on. So I don't think its that unstable and needs added
>> ugliness.]
>
> Its the same with 802.11s, its new code and people may want to disable
> this crap to not deal with it in code path or even consider the
> support for it.
>
>>> This is what I'm talking about. But yes, userspace also submits
>>> itself to the same criteria.
>>>>> You may also want to simply disable DFS if you do not want to
>>>>> deal with the regulatory test implications of having it enabled.
>>>> AFAIK you can't "simply" disable the DFS requirement: hostapd
>>>> (hw_features.c), [cfg80211] (checks if tx on secondary channel
>>>> is possible) and mac80211 (tx.c) all have checks. Indeed, the
>>>> easiest way is to modify crda's database. So there's no need
>>>> for an extra compile-time option.
>>>
>>> No, DFS is set for certain channels on wireless-regdb/CRDA, I just
>>> posted DFS master region support for wireless-regdb and CRDA. Apart
>>> from this we then need driver support. To get DFS you need all of
>>> these + hostapd part. Each one has its own set of components and does
>>> deserve its own set of tests and review.
>>
>> This "deserve its own set of tests and review". Does it translate in:
>> "ath9k [every driver], mac80211, cfg80211 and hostapd need extra
>> DFS IFDEFS?".
>
> I still have yet to see patches for cfg80211 / mac80211 for DFS. What
> I'm saying is we have an kconfig option on the 802.11 stack to  allow
> us to disable DFS support, and the driver respective component depend
> on it.
>
As said above, disabling a driver's capability through a Kconfig option 
should be enough (one ifdef per driver).

>> In fact, ifdefs make it harder to do reviews, because
>> sometimes you just forget the IFDEF/ELSIF/ELSE context of the code.
>
> I hate ifdefs, and if you read my e-mail carefully what I was
> suggesting was to do this properly by building all the code if the
> kconfig option is enabled therefore eliminating all ifdef junk from
> the code and only leaving it for header files.
>
>> And regression testing can be done by "git bisect". In fact, isn't
>> this what git bisect is for?
>
> There is a difference between regression testing and finding the
> culprit of an issue. git bisect is used to find the culprit of an
> issue. However if you want to ensure code does not regress you need to
> ensure to run a suite of tests on code after a delta is applied. Only
> if you find an issue do you then use git bisect.
>
> I want proper test infrastructure set up before I even consider
> enabling any DFS code upstream for ath9k.
>
Since regulatory compliance and open source by principle form a 
gray-zone combination [2], testing for sure is essential to keep it more 
white than black ;)

[2] http://linuxwireless.org/en/developers/Regulatory/statement#Principles

>    Luis

Zefir

Re: [ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Thu, Oct 6, 2011 at 1:32 PM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> As said above, disabling a driver's capability through a Kconfig option
> should be enough (one ifdef per driver).

OK cool.

> Since regulatory compliance and open source by principle form a
> gray-zone combination [2], testing for sure is essential to keep it more
> white than black ;)
>
> [2] http://linuxwireless.org/en/developers/Regulatory/statement#Principles

I actually do not think its grey now at all, we in fact IMHO have the
best regulatory framework out there, while still ensuring freedoms.

  Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Thu, Oct 6, 2011 at 1:32 PM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> As said above, disabling a driver's capability through a Kconfig option
> should be enough (one ifdef per driver).

OK cool.

> Since regulatory compliance and open source by principle form a
> gray-zone combination [2], testing for sure is essential to keep it more
> white than black ;)
>
> [2] http://linuxwireless.org/en/developers/Regulatory/statement#Principles

I actually do not think its grey now at all, we in fact IMHO have the
best regulatory framework out there, while still ensuring freedoms.

  Luis

Re: [ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

On 06.10.2011 22:41, Luis R. Rodriguez wrote:
> On Thu, Oct 6, 2011 at 1:32 PM, Zefir Kurtisi<zefir.kurtisi@neratec.com>  wrote:
>> As said above, disabling a driver's capability through a Kconfig option
>> should be enough (one ifdef per driver).
>
> OK cool.
>
>> Since regulatory compliance and open source by principle form a
>> gray-zone combination [2], testing for sure is essential to keep it more
>> white than black ;)
>>
>> [2] http://linuxwireless.org/en/developers/Regulatory/statement#Principles
>
> I actually do not think its grey now at all, we in fact IMHO have the
> best regulatory framework out there, while still ensuring freedoms.
>
>    Luis

Sorry, of course it is, I was not specific enough.

I was just wondering if principle 3 generally would prevent us from 
adding a Kconfig option to enable DFS for ath9k as long as it is not 
certified (which is the only way to ensure to have a 'known compliant 
usage') plus depending on mac80211 and hostapd.


Zefir

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

On 06.10.2011 22:41, Luis R. Rodriguez wrote:
> On Thu, Oct 6, 2011 at 1:32 PM, Zefir Kurtisi<zefir.kurtisi@neratec.com>  wrote:
>> As said above, disabling a driver's capability through a Kconfig option
>> should be enough (one ifdef per driver).
>
> OK cool.
>
>> Since regulatory compliance and open source by principle form a
>> gray-zone combination [2], testing for sure is essential to keep it more
>> white than black ;)
>>
>> [2] http://linuxwireless.org/en/developers/Regulatory/statement#Principles
>
> I actually do not think its grey now at all, we in fact IMHO have the
> best regulatory framework out there, while still ensuring freedoms.
>
>    Luis

Sorry, of course it is, I was not specific enough.

I was just wondering if principle 3 generally would prevent us from 
adding a Kconfig option to enable DFS for ath9k as long as it is not 
certified (which is the only way to ensure to have a 'known compliant 
usage') plus depending on mac80211 and hostapd.


Zefir

Re: [ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Thu, Oct 6, 2011 at 2:08 PM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> On 06.10.2011 22:41, Luis R. Rodriguez wrote:
>> On Thu, Oct 6, 2011 at 1:32 PM, Zefir Kurtisi<zefir.kurtisi@neratec.com>  wrote:
>>> As said above, disabling a driver's capability through a Kconfig option
>>> should be enough (one ifdef per driver).
>>
>> OK cool.
>>
>>> Since regulatory compliance and open source by principle form a
>>> gray-zone combination [2], testing for sure is essential to keep it more
>>> white than black ;)
>>>
>>> [2] http://linuxwireless.org/en/developers/Regulatory/statement#Principles
>>
>> I actually do not think its grey now at all, we in fact IMHO have the
>> best regulatory framework out there, while still ensuring freedoms.
>>
>>    Luis
>
> Sorry, of course it is, I was not specific enough.
>
> I was just wondering if principle 3 generally would prevent us from
> adding a Kconfig option to enable DFS for ath9k as long as it is not
> certified (which is the only way to ensure to have a 'known compliant
> usage') plus depending on mac80211 and hostapd.

Ah yeah great point. I punted internally to see if there was a EEPROM
bit we can read that may tell us if a card is DFS certified. If that
is not available I think what we can do is leave the option but set
the default to disabled and simply state that only platforms that have
passed DFS certification should have it enabled.

 Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Thu, Oct 6, 2011 at 2:08 PM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> On 06.10.2011 22:41, Luis R. Rodriguez wrote:
>> On Thu, Oct 6, 2011 at 1:32 PM, Zefir Kurtisi<zefir.kurtisi@neratec.com> ?wrote:
>>> As said above, disabling a driver's capability through a Kconfig option
>>> should be enough (one ifdef per driver).
>>
>> OK cool.
>>
>>> Since regulatory compliance and open source by principle form a
>>> gray-zone combination [2], testing for sure is essential to keep it more
>>> white than black ;)
>>>
>>> [2] http://linuxwireless.org/en/developers/Regulatory/statement#Principles
>>
>> I actually do not think its grey now at all, we in fact IMHO have the
>> best regulatory framework out there, while still ensuring freedoms.
>>
>> ? ?Luis
>
> Sorry, of course it is, I was not specific enough.
>
> I was just wondering if principle 3 generally would prevent us from
> adding a Kconfig option to enable DFS for ath9k as long as it is not
> certified (which is the only way to ensure to have a 'known compliant
> usage') plus depending on mac80211 and hostapd.

Ah yeah great point. I punted internally to see if there was a EEPROM
bit we can read that may tell us if a card is DFS certified. If that
is not available I think what we can do is leave the option but set
the default to disabled and simply state that only platforms that have
passed DFS certification should have it enabled.

 Luis

Re: [ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Adrian Chadd]

Just FYI, that's what I'm likely to do for FreeBSD 10 (as I just don't
have time to try and make all the required regulatory changes before
the upcoming 9.0 release.)

Ie:

* DFS station mode (net80211) is going to be compiled and enabled by default;
* DFS master mode (net80211) is going to be compiled and enabled by default;
* The drivers which support radar detection in firmware (currently
only if_mwl) will set the DFS net80211 flag (ie, for master mode radar
detection);
* ath won't ship with the DFS enable flag (for master mode);
* I'll modify the regulatory database code to include per-band DFS
information (DFS domain, CAC/NOL timeout, interference timeout, etc)
and some device information (eg whether it supports HT20/HT40/etc DFS
detection);
* I'll then flip on the DFS channel enforcement in the net80211 code
so disables master mode on channels requiring DFS.

The radar detection code and channel interference code will live in
the ath driver but the DFS machinery (CAC, NOL, CSA, etc) is in
net80211. That way other NICs (eg if_mwl Marvell NICs) with DFS radar
detection support can leverage this.

I'll then likely ship two DFS modules - dfs_null (no DFS support, just
a placeholder for the API) and whatever code is ported from the
reference driver. Maybe I'll also include the code from Neratec if
it's dual-licenced. But I won't include radar patterns by default -
I'll include those on a documentation page which explains the how and
why of regulatory domain stuff.

Once FreeBSD ships DFS radar detection code, I'll make sure it isn't
compiled by default and even if enabled, it won't advertise DFS
channel support unless a valid radar pattern and radar parameter
configuration is loaded in. That way users won't inadvertently enable
it without being compliant.

Finally, I'm hoping to get all of this documented as much as possible
so the community can pick this stuff up and run with it. I was hoping
someone would throw me a 5ghz SDR (software defined radio) so I could
prototype up some open source radar pulse generation code, just to
lower the entry barrier to all of this.

HTH,


Adrian

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Adrian Chadd]

Just FYI, that's what I'm likely to do for FreeBSD 10 (as I just don't
have time to try and make all the required regulatory changes before
the upcoming 9.0 release.)

Ie:

* DFS station mode (net80211) is going to be compiled and enabled by default;
* DFS master mode (net80211) is going to be compiled and enabled by default;
* The drivers which support radar detection in firmware (currently
only if_mwl) will set the DFS net80211 flag (ie, for master mode radar
detection);
* ath won't ship with the DFS enable flag (for master mode);
* I'll modify the regulatory database code to include per-band DFS
information (DFS domain, CAC/NOL timeout, interference timeout, etc)
and some device information (eg whether it supports HT20/HT40/etc DFS
detection);
* I'll then flip on the DFS channel enforcement in the net80211 code
so disables master mode on channels requiring DFS.

The radar detection code and channel interference code will live in
the ath driver but the DFS machinery (CAC, NOL, CSA, etc) is in
net80211. That way other NICs (eg if_mwl Marvell NICs) with DFS radar
detection support can leverage this.

I'll then likely ship two DFS modules - dfs_null (no DFS support, just
a placeholder for the API) and whatever code is ported from the
reference driver. Maybe I'll also include the code from Neratec if
it's dual-licenced. But I won't include radar patterns by default -
I'll include those on a documentation page which explains the how and
why of regulatory domain stuff.

Once FreeBSD ships DFS radar detection code, I'll make sure it isn't
compiled by default and even if enabled, it won't advertise DFS
channel support unless a valid radar pattern and radar parameter
configuration is loaded in. That way users won't inadvertently enable
it without being compliant.

Finally, I'm hoping to get all of this documented as much as possible
so the community can pick this stuff up and run with it. I was hoping
someone would throw me a 5ghz SDR (software defined radio) so I could
prototype up some open source radar pulse generation code, just to
lower the entry barrier to all of this.

HTH,


Adrian

Re: [ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Thu, Oct 6, 2011 at 8:06 PM, Adrian Chadd <adrian@freebsd.org> wrote:
> Just FYI, that's what I'm likely to do for FreeBSD 10 (as I just don't
> have time to try and make all the required regulatory changes before
> the upcoming 9.0 release.)
>
> Ie:
>
> * DFS station mode (net80211) is going to be compiled and enabled by default;
> * DFS master mode (net80211) is going to be compiled and enabled by default;
> * The drivers which support radar detection in firmware (currently
> only if_mwl) will set the DFS net80211 flag (ie, for master mode radar
> detection);
> * ath won't ship with the DFS enable flag (for master mode);
> * I'll modify the regulatory database code to include per-band DFS
> information (DFS domain, CAC/NOL timeout, interference timeout, etc)
> and some device information (eg whether it supports HT20/HT40/etc DFS
> detection);
> * I'll then flip on the DFS channel enforcement in the net80211 code
> so disables master mode on channels requiring DFS.
>
> The radar detection code and channel interference code will live in
> the ath driver but the DFS machinery (CAC, NOL, CSA, etc) is in
> net80211. That way other NICs (eg if_mwl Marvell NICs) with DFS radar
> detection support can leverage this.
>
> I'll then likely ship two DFS modules - dfs_null (no DFS support, just
> a placeholder for the API) and whatever code is ported from the
> reference driver. Maybe I'll also include the code from Neratec if
> it's dual-licenced. But I won't include radar patterns by default -
> I'll include those on a documentation page which explains the how and
> why of regulatory domain stuff.
>
> Once FreeBSD ships DFS radar detection code, I'll make sure it isn't
> compiled by default and even if enabled, it won't advertise DFS
> channel support unless a valid radar pattern and radar parameter
> configuration is loaded in. That way users won't inadvertently enable
> it without being compliant.

Neat, best of luck! FWIW, I've been moving along on the regulatory
simulator, feel free do send me patches in ways you want that to move
if that seems reasonable to you as place to share code between OSes,
that was at least my own goal, to help share as much test framework
and code to let us then cherry pick for our own OSes in whatever way
we want.

> Finally, I'm hoping to get all of this documented as much as possible
> so the community can pick this stuff up and run with it. I was hoping
> someone would throw me a 5ghz SDR (software defined radio) so I could
> prototype up some open source radar pulse generation code, just to
> lower the entry barrier to all of this.

You can use the Winlab Orbit GNU Radio SDRs, I think you can make a
reservation for them.

  Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Thu, Oct 6, 2011 at 8:06 PM, Adrian Chadd <adrian@freebsd.org> wrote:
> Just FYI, that's what I'm likely to do for FreeBSD 10 (as I just don't
> have time to try and make all the required regulatory changes before
> the upcoming 9.0 release.)
>
> Ie:
>
> * DFS station mode (net80211) is going to be compiled and enabled by default;
> * DFS master mode (net80211) is going to be compiled and enabled by default;
> * The drivers which support radar detection in firmware (currently
> only if_mwl) will set the DFS net80211 flag (ie, for master mode radar
> detection);
> * ath won't ship with the DFS enable flag (for master mode);
> * I'll modify the regulatory database code to include per-band DFS
> information (DFS domain, CAC/NOL timeout, interference timeout, etc)
> and some device information (eg whether it supports HT20/HT40/etc DFS
> detection);
> * I'll then flip on the DFS channel enforcement in the net80211 code
> so disables master mode on channels requiring DFS.
>
> The radar detection code and channel interference code will live in
> the ath driver but the DFS machinery (CAC, NOL, CSA, etc) is in
> net80211. That way other NICs (eg if_mwl Marvell NICs) with DFS radar
> detection support can leverage this.
>
> I'll then likely ship two DFS modules - dfs_null (no DFS support, just
> a placeholder for the API) and whatever code is ported from the
> reference driver. Maybe I'll also include the code from Neratec if
> it's dual-licenced. But I won't include radar patterns by default -
> I'll include those on a documentation page which explains the how and
> why of regulatory domain stuff.
>
> Once FreeBSD ships DFS radar detection code, I'll make sure it isn't
> compiled by default and even if enabled, it won't advertise DFS
> channel support unless a valid radar pattern and radar parameter
> configuration is loaded in. That way users won't inadvertently enable
> it without being compliant.

Neat, best of luck! FWIW, I've been moving along on the regulatory
simulator, feel free do send me patches in ways you want that to move
if that seems reasonable to you as place to share code between OSes,
that was at least my own goal, to help share as much test framework
and code to let us then cherry pick for our own OSes in whatever way
we want.

> Finally, I'm hoping to get all of this documented as much as possible
> so the community can pick this stuff up and run with it. I was hoping
> someone would throw me a 5ghz SDR (software defined radio) so I could
> prototype up some open source radar pulse generation code, just to
> lower the entry barrier to all of this.

You can use the Winlab Orbit GNU Radio SDRs, I think you can make a
reservation for them.

  Luis

Re: [ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

On 10/07/2011 05:06 AM, Adrian Chadd wrote:
> Just FYI, that's what I'm likely to do for FreeBSD 10 (as I just don't
> have time to try and make all the required regulatory changes before
> the upcoming 9.0 release.)
> 
> Ie:
> 
> * DFS station mode (net80211) is going to be compiled and enabled by default;
> * DFS master mode (net80211) is going to be compiled and enabled by default;
> * The drivers which support radar detection in firmware (currently
> only if_mwl) will set the DFS net80211 flag (ie, for master mode radar
> detection);
> * ath won't ship with the DFS enable flag (for master mode);
> * I'll modify the regulatory database code to include per-band DFS
> information (DFS domain, CAC/NOL timeout, interference timeout, etc)
> and some device information (eg whether it supports HT20/HT40/etc DFS
> detection);
> * I'll then flip on the DFS channel enforcement in the net80211 code
> so disables master mode on channels requiring DFS.
> 
> The radar detection code and channel interference code will live in
> the ath driver but the DFS machinery (CAC, NOL, CSA, etc) is in
> net80211. That way other NICs (eg if_mwl Marvell NICs) with DFS radar
> detection support can leverage this.
> 
Quite some work ahead. Good luck on that!

So you are confident to get existing code re-factored and split into multiple layers?

> I'll then likely ship two DFS modules - dfs_null (no DFS support, just
> a placeholder for the API) and whatever code is ported from the
> reference driver. Maybe I'll also include the code from Neratec if
> it's dual-licenced. But I won't include radar patterns by default -
> I'll include those on a documentation page which explains the how and
> why of regulatory domain stuff.
> 
Initially we planned to have common pattern detectors in mac80211 to handle pulses reported by all drivers, but so far only TI is working on DFS -- with pattern matching done in firmware. So, no need yet for common detectors, we might end up going the other way around, i.e. taking your port and integrate in into ath9k -- if licensing allowed us to do.

> Once FreeBSD ships DFS radar detection code, I'll make sure it isn't
> compiled by default and even if enabled, it won't advertise DFS
> channel support unless a valid radar pattern and radar parameter
> configuration is loaded in. That way users won't inadvertently enable
> it without being compliant.
> 
> Finally, I'm hoping to get all of this documented as much as possible
> so the community can pick this stuff up and run with it. I was hoping
> someone would throw me a 5ghz SDR (software defined radio) so I could
> prototype up some open source radar pulse generation code, just to
> lower the entry barrier to all of this.
> 
Yeah, an AR9003 card used as cheap radar generator is on top of my wish list for DFS testing without our always otherwise occupied vector signal generator. But fur obvious reasons we wont get the required documentation for such unofficial hacks.

> HTH,
> 
> 
> Adrian

Thanks
Zefir

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

On 10/07/2011 05:06 AM, Adrian Chadd wrote:
> Just FYI, that's what I'm likely to do for FreeBSD 10 (as I just don't
> have time to try and make all the required regulatory changes before
> the upcoming 9.0 release.)
> 
> Ie:
> 
> * DFS station mode (net80211) is going to be compiled and enabled by default;
> * DFS master mode (net80211) is going to be compiled and enabled by default;
> * The drivers which support radar detection in firmware (currently
> only if_mwl) will set the DFS net80211 flag (ie, for master mode radar
> detection);
> * ath won't ship with the DFS enable flag (for master mode);
> * I'll modify the regulatory database code to include per-band DFS
> information (DFS domain, CAC/NOL timeout, interference timeout, etc)
> and some device information (eg whether it supports HT20/HT40/etc DFS
> detection);
> * I'll then flip on the DFS channel enforcement in the net80211 code
> so disables master mode on channels requiring DFS.
> 
> The radar detection code and channel interference code will live in
> the ath driver but the DFS machinery (CAC, NOL, CSA, etc) is in
> net80211. That way other NICs (eg if_mwl Marvell NICs) with DFS radar
> detection support can leverage this.
> 
Quite some work ahead. Good luck on that!

So you are confident to get existing code re-factored and split into multiple layers?

> I'll then likely ship two DFS modules - dfs_null (no DFS support, just
> a placeholder for the API) and whatever code is ported from the
> reference driver. Maybe I'll also include the code from Neratec if
> it's dual-licenced. But I won't include radar patterns by default -
> I'll include those on a documentation page which explains the how and
> why of regulatory domain stuff.
> 
Initially we planned to have common pattern detectors in mac80211 to handle pulses reported by all drivers, but so far only TI is working on DFS -- with pattern matching done in firmware. So, no need yet for common detectors, we might end up going the other way around, i.e. taking your port and integrate in into ath9k -- if licensing allowed us to do.

> Once FreeBSD ships DFS radar detection code, I'll make sure it isn't
> compiled by default and even if enabled, it won't advertise DFS
> channel support unless a valid radar pattern and radar parameter
> configuration is loaded in. That way users won't inadvertently enable
> it without being compliant.
> 
> Finally, I'm hoping to get all of this documented as much as possible
> so the community can pick this stuff up and run with it. I was hoping
> someone would throw me a 5ghz SDR (software defined radio) so I could
> prototype up some open source radar pulse generation code, just to
> lower the entry barrier to all of this.
> 
Yeah, an AR9003 card used as cheap radar generator is on top of my wish list for DFS testing without our always otherwise occupied vector signal generator. But fur obvious reasons we wont get the required documentation for such unofficial hacks.

> HTH,
> 
> 
> Adrian

Thanks
Zefir

Re: [ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Adrian Chadd]

On 7 October 2011 16:48, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:

> Quite some work ahead. Good luck on that!

Thanks!

> So you are confident to get existing code re-factored and split into multiple layers?

Thankfully, it's already done and in net80211. :)




Adrian

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Adrian Chadd]

On 7 October 2011 16:48, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:

> Quite some work ahead. Good luck on that!

Thanks!

> So you are confident to get existing code re-factored and split into multiple layers?

Thankfully, it's already done and in net80211. :)




Adrian

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

On 10/03/2011 08:27 PM, Luis R. Rodriguez wrote:
> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>
>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>> ---
>>  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index e8aeb98..5defebe 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>>                        "Unable to reset channel, reset status %d\n", r);
>>                goto out;
>>        }
>> +#ifdef CONFIG_ATH9K_DFS
>> +       /**
>> +        * enable radar pulse detection
>> +        *
>> +        * TODO: do this only for DFS channels
>> +        */
>> +       ah->private_ops.set_radar_params(ah, &ah->radar_conf);
>> +       ath9k_hw_setrxfilter(ah,
>> +                       ath9k_hw_getrxfilter(ah) | ATH9K_RX_FILTER_PHYRADAR);
>> +       ath_dbg(common, ATH_DBG_DFS,
>> +               "DFS enabled for channel %d\n", hchan->chan->center_freq);
>> +#endif
> 
> Please spare the #ifdef and just call something within dfs.c, then
> dfs.h would wrap it to nothing if DFS is disabled.
> 
This possibly won't work, since setting up DFS registers is part of HW layer and not done in the dfs module. If you want to have DFS conditionally compilable, you can not spare this #ifdefs.

But I see that this would work in patch 6/6 when calling ath9k_dfs_process_phyerr() on ATH9K_PHYERR_RADAR. Will be considered.

>   Luis

Thanks,
Zefir

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Zefir Kurtisi]

On 10/03/2011 08:27 PM, Luis R. Rodriguez wrote:
> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>
>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>> ---
>>  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index e8aeb98..5defebe 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>>                        "Unable to reset channel, reset status %d\n", r);
>>                goto out;
>>        }
>> +#ifdef CONFIG_ATH9K_DFS
>> +       /**
>> +        * enable radar pulse detection
>> +        *
>> +        * TODO: do this only for DFS channels
>> +        */
>> +       ah->private_ops.set_radar_params(ah, &ah->radar_conf);
>> +       ath9k_hw_setrxfilter(ah,
>> +                       ath9k_hw_getrxfilter(ah) | ATH9K_RX_FILTER_PHYRADAR);
>> +       ath_dbg(common, ATH_DBG_DFS,
>> +               "DFS enabled for channel %d\n", hchan->chan->center_freq);
>> +#endif
> 
> Please spare the #ifdef and just call something within dfs.c, then
> dfs.h would wrap it to nothing if DFS is disabled.
> 
This possibly won't work, since setting up DFS registers is part of HW layer and not done in the dfs module. If you want to have DFS conditionally compilable, you can not spare this #ifdefs.

But I see that this would work in patch 6/6 when calling ath9k_dfs_process_phyerr() on ATH9K_PHYERR_RADAR. Will be considered.

>   Luis

Thanks,
Zefir

Re: [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Tue, Oct 4, 2011 at 3:11 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> On 10/03/2011 08:27 PM, Luis R. Rodriguez wrote:
>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>>
>>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>>> ---
>>>  drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++++
>>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>> index e8aeb98..5defebe 100644
>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>>>                        "Unable to reset channel, reset status %d\n", r);
>>>                goto out;
>>>        }
>>> +#ifdef CONFIG_ATH9K_DFS
>>> +       /**
>>> +        * enable radar pulse detection
>>> +        *
>>> +        * TODO: do this only for DFS channels
>>> +        */
>>> +       ah->private_ops.set_radar_params(ah, &ah->radar_conf);
>>> +       ath9k_hw_setrxfilter(ah,
>>> +                       ath9k_hw_getrxfilter(ah) | ATH9K_RX_FILTER_PHYRADAR);
>>> +       ath_dbg(common, ATH_DBG_DFS,
>>> +               "DFS enabled for channel %d\n", hchan->chan->center_freq);
>>> +#endif
>>
>> Please spare the #ifdef and just call something within dfs.c, then
>> dfs.h would wrap it to nothing if DFS is disabled.
>>
> This possibly won't work, since setting up DFS registers is part of HW layer and not
> done in the dfs module. If you want to have DFS conditionally compilable, you can not spare this #ifdefs.

Its not about sparing the ifdefs completely but instead to place them
strategically to remove #ifdef sprinkling all over C code. You can
leave ifdefs on header files, and for C files leaves this as a
conditional build time option.

  Luis

[ath9k-devel] [RFC 5/6] ath9k: enable DFS pulse detection

[Luis R. Rodriguez]

On Tue, Oct 4, 2011 at 3:11 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> On 10/03/2011 08:27 PM, Luis R. Rodriguez wrote:
>> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>>>
>>> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
>>> ---
>>> ?drivers/net/wireless/ath/ath9k/main.c | ? 12 ++++++++++++
>>> ?1 files changed, 12 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>> index e8aeb98..5defebe 100644
>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>> @@ -344,6 +344,18 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
>>> ? ? ? ? ? ? ? ? ? ? ? ?"Unable to reset channel, reset status %d\n", r);
>>> ? ? ? ? ? ? ? ?goto out;
>>> ? ? ? ?}
>>> +#ifdef CONFIG_ATH9K_DFS
>>> + ? ? ? /**
>>> + ? ? ? ?* enable radar pulse detection
>>> + ? ? ? ?*
>>> + ? ? ? ?* TODO: do this only for DFS channels
>>> + ? ? ? ?*/
>>> + ? ? ? ah->private_ops.set_radar_params(ah, &ah->radar_conf);
>>> + ? ? ? ath9k_hw_setrxfilter(ah,
>>> + ? ? ? ? ? ? ? ? ? ? ? ath9k_hw_getrxfilter(ah) | ATH9K_RX_FILTER_PHYRADAR);
>>> + ? ? ? ath_dbg(common, ATH_DBG_DFS,
>>> + ? ? ? ? ? ? ? "DFS enabled for channel %d\n", hchan->chan->center_freq);
>>> +#endif
>>
>> Please spare the #ifdef and just call something within dfs.c, then
>> dfs.h would wrap it to nothing if DFS is disabled.
>>
> This possibly won't work, since setting up DFS registers is part of HW layer and not
> done in the dfs module. If you want to have DFS conditionally compilable, you can not spare this #ifdefs.

Its not about sparing the ifdefs completely but instead to place them
strategically to remove #ifdef sprinkling all over C code. You can
leave ifdefs on header files, and for C files leaves this as a
conditional build time option.

  Luis

[RFC 6/6] ath9k: handle pulse data reported by DFS HW

[Zefir Kurtisi]

Note: calculation of mactime had to be shifted before
ath9k_rx_skb_preprocess() since it is used to time-stamp the
radar pulse.

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
---
 drivers/net/wireless/ath/ath9k/recv.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 02c9f97..f5bb114 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -17,6 +17,10 @@
 #include <linux/dma-mapping.h>
 #include "ath9k.h"
 #include "ar9003_mac.h"
+#ifdef CONFIG_ATH9K_DFS
+#include "dfs.h"
+#endif
+
 
 #define SKB_CB_ATHBUF(__skb)	(*((struct ath_buf **)__skb->cb))
 
@@ -1850,11 +1854,6 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 		if (flush)
 			goto requeue_drop_frag;
 
-		retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs,
-						 rxs, &decrypt_error);
-		if (retval)
-			goto requeue_drop_frag;
-
 		rxs->mactime = (tsf & ~0xffffffffULL) | rs.rs_tstamp;
 		if (rs.rs_tstamp > tsf_lower &&
 		    unlikely(rs.rs_tstamp - tsf_lower > 0x10000000))
@@ -1864,6 +1863,19 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 		    unlikely(tsf_lower - rs.rs_tstamp > 0x10000000))
 			rxs->mactime += 0x100000000ULL;
 
+#ifdef CONFIG_ATH9K_DFS
+		if ((hdr != NULL) && ((rs.rs_status & ATH9K_RXERR_PHY) != 0) &&
+				(rs.rs_phyerr == ATH9K_PHYERR_RADAR)) {
+			/* DFS: feed radar pulse */
+			ath9k_dfs_process_phyerr(sc, hdr, &rs, rxs->mactime);
+		}
+#endif
+
+		retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs,
+						 rxs, &decrypt_error);
+		if (retval)
+			goto requeue_drop_frag;
+
 		/* Ensure we always have an skb to requeue once we are done
 		 * processing the current buffer's skb */
 		requeue_skb = ath_rxbuf_alloc(common, common->rx_bufsize, GFP_ATOMIC);
-- 
1.7.4.1

[ath9k-devel] [RFC 6/6] ath9k: handle pulse data reported by DFS HW

[Zefir Kurtisi]

Note: calculation of mactime had to be shifted before
ath9k_rx_skb_preprocess() since it is used to time-stamp the
radar pulse.

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
---
 drivers/net/wireless/ath/ath9k/recv.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 02c9f97..f5bb114 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -17,6 +17,10 @@
 #include <linux/dma-mapping.h>
 #include "ath9k.h"
 #include "ar9003_mac.h"
+#ifdef CONFIG_ATH9K_DFS
+#include "dfs.h"
+#endif
+
 
 #define SKB_CB_ATHBUF(__skb)	(*((struct ath_buf **)__skb->cb))
 
@@ -1850,11 +1854,6 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 		if (flush)
 			goto requeue_drop_frag;
 
-		retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs,
-						 rxs, &decrypt_error);
-		if (retval)
-			goto requeue_drop_frag;
-
 		rxs->mactime = (tsf & ~0xffffffffULL) | rs.rs_tstamp;
 		if (rs.rs_tstamp > tsf_lower &&
 		    unlikely(rs.rs_tstamp - tsf_lower > 0x10000000))
@@ -1864,6 +1863,19 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 		    unlikely(tsf_lower - rs.rs_tstamp > 0x10000000))
 			rxs->mactime += 0x100000000ULL;
 
+#ifdef CONFIG_ATH9K_DFS
+		if ((hdr != NULL) && ((rs.rs_status & ATH9K_RXERR_PHY) != 0) &&
+				(rs.rs_phyerr == ATH9K_PHYERR_RADAR)) {
+			/* DFS: feed radar pulse */
+			ath9k_dfs_process_phyerr(sc, hdr, &rs, rxs->mactime);
+		}
+#endif
+
+		retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs,
+						 rxs, &decrypt_error);
+		if (retval)
+			goto requeue_drop_frag;
+
 		/* Ensure we always have an skb to requeue once we are done
 		 * processing the current buffer's skb */
 		requeue_skb = ath_rxbuf_alloc(common, common->rx_bufsize, GFP_ATOMIC);
-- 
1.7.4.1

Re: [RFC 6/6] ath9k: handle pulse data reported by DFS HW

[Luis R. Rodriguez]

On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> Note: calculation of mactime had to be shifted before
> ath9k_rx_skb_preprocess() since it is used to time-stamp the
> radar pulse.

Do this in a separate patch and provide a valid commit log here of
what the addition actually does. As I see it you can unify the last
set of patches together. The debugfs stuff can be kept apart though
but with the considerations I mentioned.

  Luis

[ath9k-devel] [RFC 6/6] ath9k: handle pulse data reported by DFS HW

[Luis R. Rodriguez]

On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> Note: calculation of mactime had to be shifted before
> ath9k_rx_skb_preprocess() since it is used to time-stamp the
> radar pulse.

Do this in a separate patch and provide a valid commit log here of
what the addition actually does. As I see it you can unify the last
set of patches together. The debugfs stuff can be kept apart though
but with the considerations I mentioned.

  Luis