[PATCH v4 0/3] Antenna configuration

[PATCH v4 0/3] Antenna configuration

[Bruno Randolf]

Sorry for the long delay in following this up.

This is version 4 of my tries to some antenna configuration interface accepted - now using get/set thru the wiphy as Johannes has suggested.

Please see the first patch for a longer description.

bruno
---

Bruno Randolf (3):
      cfg80211: Add nl80211 antenna configuration
      mac80211: Add antenna configuration
      ath5k: Add support for antenna configuration


 drivers/net/wireless/ath/ath5k/base.c |   34 ++++++++++++++++++++++
 include/linux/nl80211.h               |   17 +++++++++++
 include/net/cfg80211.h                |    3 ++
 include/net/mac80211.h                |    2 +
 net/mac80211/cfg.c                    |   16 +++++++++++
 net/mac80211/driver-ops.h             |   23 +++++++++++++++
 net/mac80211/driver-trace.h           |   50 +++++++++++++++++++++++++++++++++
 net/wireless/nl80211.c                |   30 +++++++++++++++++++-
 8 files changed, 174 insertions(+), 1 deletions(-)

-- 
Signature

[PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Bruno Randolf]

Allow setting TX and RX antenna configuration via nl80211.

The antenna configuration is defined as a bitmap of allowed antennas to use for
either receiving or transmitting. Separate bitmaps are used for RX and TX to
allow configuring different antennas for receiving and transmitting. The bitmap
is 8 bit long, each bit representing one antenna, starting with antenna 1.

If multiple receive (RX) antennas are selected, the driver may use diversity or
multiple 802.11n RX chains, according to the chipset capabilities and
configuration.

If multiple transmit (TX) antennas are selected, the driver has to send on all
selected antennas, making this mode of operation only possible on 802.11n
chipsets with multiple TX chains. If the transmit antenna bitmap is set to the
special value zero (0) the driver should select the TX antenna based on the
antenna which was used for receiving (TX antenna follows RX diversity - this is
the usual mode of 'diversity') for pre 802.11n devices.

Using bitmaps has the benefit of allowing for a flexible configuration
interface which can support many different configurations. Drivers should
reject configurations they cannot support. While covering the standard modes
"fixed antenna 1", "fixed antenna 2" and "diversity" this API also allows more
rare - but useful - configurations as follows:

1) Send on antenna 1, receive on antenna 2 (or vice versa). This can be used to
have a low gain antenna for TX in order to keep within the regulatory
constraints and a high gain antenna for RX in order to receive weaker signals
("speak softly, but listen harder"). This can be useful for building long-shot
outdoor links. Another usage of this setup is having a low-noise pre-amplifier
on antenna 1 and a power amplifier on the other antenna. This way transmit
noise is mostly kept out of the low noise receive channel.
(This would be bitmaps: tx 1 rx 2).

2) Another similar setup is: Use RX diversity on both antennas, but always send
on antenna 1. Again that would allow us to benefit from a higher gain RX
antenna, while staying within the legal limits.
(This would be: tx 0 rx 3).

3) And finally there can be special experimental setups in research and
development where more than 2 antennas are available. It's good to keep the API
flexible.

Signed-off-by: Bruno Randolf <br1@einfach.org>
---
 include/linux/nl80211.h |   17 +++++++++++++++++
 include/net/cfg80211.h  |    3 +++
 net/wireless/nl80211.c  |   30 +++++++++++++++++++++++++++++-
 3 files changed, 49 insertions(+), 1 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 2c87016..b9de53c 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -731,6 +731,20 @@ enum nl80211_commands {
  *      This is used in association with @NL80211_ATTR_WIPHY_TX_POWER_SETTING
  *      for non-automatic settings.
  *
+ * @NL80211_ATTR_WIPHY_ANTENNA_TX: Bitmap of allowed antennas for transmitting.
+ *	Each bit represents one antenna, starting with antenna 1 at the first
+ *	bit. If the bitmap is zero (0), the TX antenna follows RX diversity.
+ *	If multiple antennas are selected all selected antennas have to be used
+ *	for transmitting (801.11n multiple TX chains).
+ *	Drivers may reject configurations they cannot support.
+ *
+ * @NL80211_ATTR_WIPHY_ANTENNA_RX: Bitmap of allowed antennas for receiving.
+ *	Each bit represents one antenna, starting with antenna 1 at the first
+ *	bit. If multiple antennas are selected in the bitmap, 802.11n devices
+ *	should use multiple RX chains on these antennas, while non-802.11n
+ *	drivers should use antenna diversity between these antennas.
+ *	Drivers may reject configurations they cannot support.
+ *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
  */
@@ -891,6 +905,9 @@ enum nl80211_attrs {
 	NL80211_ATTR_WIPHY_TX_POWER_SETTING,
 	NL80211_ATTR_WIPHY_TX_POWER_LEVEL,
 
+	NL80211_ATTR_WIPHY_ANTENNA_TX,
+	NL80211_ATTR_WIPHY_ANTENNA_RX,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index f68ae54..7dc0e8d 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1184,6 +1184,9 @@ struct cfg80211_ops {
 	int	(*set_cqm_rssi_config)(struct wiphy *wiphy,
 				       struct net_device *dev,
 				       s32 rssi_thold, u32 rssi_hyst);
+
+	int	(*set_antenna)(struct wiphy *wiphy, u8 tx_ant, u8 rx_ant);
+	int	(*get_antenna)(struct wiphy *wiphy, u8 *tx_ant, u8 *rx_ant);
 };
 
 /*
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index cea595e..62fc5fe 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -156,6 +156,9 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
 
 	[NL80211_ATTR_WIPHY_TX_POWER_SETTING] = { .type = NLA_U32 },
 	[NL80211_ATTR_WIPHY_TX_POWER_LEVEL] = { .type = NLA_U32 },
+
+	[NL80211_ATTR_WIPHY_ANTENNA_TX] = { .type = NLA_U8 },
+	[NL80211_ATTR_WIPHY_ANTENNA_RX] = { .type = NLA_U8 },
 };
 
 /* policy for the attributes */
@@ -458,7 +461,6 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
 		    dev->wiphy.rts_threshold);
 	NLA_PUT_U8(msg, NL80211_ATTR_WIPHY_COVERAGE_CLASS,
 		    dev->wiphy.coverage_class);
-
 	NLA_PUT_U8(msg, NL80211_ATTR_MAX_NUM_SCAN_SSIDS,
 		   dev->wiphy.max_scan_ssids);
 	NLA_PUT_U16(msg, NL80211_ATTR_MAX_SCAN_IE_LEN,
@@ -471,6 +473,16 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
 	NLA_PUT_U8(msg, NL80211_ATTR_MAX_NUM_PMKIDS,
 		   dev->wiphy.max_num_pmkids);
 
+	if (dev->ops->get_antenna) {
+		u8 tx_ant = 0, rx_ant = 0;
+		int res;
+		res = dev->ops->get_antenna(&dev->wiphy, &tx_ant, &rx_ant);
+		if (!res) {
+			NLA_PUT_U8(msg, NL80211_ATTR_WIPHY_ANTENNA_TX, tx_ant);
+			NLA_PUT_U8(msg, NL80211_ATTR_WIPHY_ANTENNA_RX, rx_ant);
+		}
+	}
+
 	nl_modes = nla_nest_start(msg, NL80211_ATTR_SUPPORTED_IFTYPES);
 	if (!nl_modes)
 		goto nla_put_failure;
@@ -900,6 +912,22 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 			goto bad_res;
 	}
 
+	if (info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX] &&
+	    info->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]) {
+		u8 tx_ant, rx_ant;
+		if (!rdev->ops->set_antenna) {
+			result = -EOPNOTSUPP;
+			goto bad_res;
+		}
+
+		tx_ant = nla_get_u8(info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX]);
+		rx_ant = nla_get_u8(info->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]);
+
+		result = rdev->ops->set_antenna(&rdev->wiphy, tx_ant, rx_ant);
+		if (result)
+			goto bad_res;
+	}
+
 	changed = 0;
 
 	if (info->attrs[NL80211_ATTR_WIPHY_RETRY_SHORT]) {

[PATCH v4 2/3] mac80211: Add antenna configuration

[Bruno Randolf]

Allow antenna configuration by calling driver's function for it.

Signed-off-by: Bruno Randolf <br1@einfach.org>
---
 include/net/mac80211.h      |    2 ++
 net/mac80211/cfg.c          |   16 ++++++++++++++
 net/mac80211/driver-ops.h   |   23 ++++++++++++++++++++
 net/mac80211/driver-trace.h |   50 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 91 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 20d372e..f8e04d1 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1744,6 +1744,8 @@ struct ieee80211_ops {
 	void (*flush)(struct ieee80211_hw *hw, bool drop);
 	void (*channel_switch)(struct ieee80211_hw *hw,
 			       struct ieee80211_channel_switch *ch_switch);
+	int (*set_antenna)(struct ieee80211_hw *hw, u8 tx_ant, u8 rx_ant);
+	int (*get_antenna)(struct ieee80211_hw *hw, u8 *tx_ant, u8 *rx_ant);
 };
 
 /**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index f09a52d..7b99e6a 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1598,6 +1598,20 @@ static int ieee80211_action(struct wiphy *wiphy, struct net_device *dev,
 	return 0;
 }
 
+static int ieee80211_set_antenna(struct wiphy *wiphy, u8 tx_ant, u8 rx_ant)
+{
+	struct ieee80211_local *local = wiphy_priv(wiphy);
+
+	return drv_set_antenna(local, tx_ant, rx_ant);
+}
+
+static int ieee80211_get_antenna(struct wiphy *wiphy, u8 *tx_ant, u8 *rx_ant)
+{
+	struct ieee80211_local *local = wiphy_priv(wiphy);
+
+	return drv_get_antenna(local, tx_ant, rx_ant);
+}
+
 struct cfg80211_ops mac80211_config_ops = {
 	.add_virtual_intf = ieee80211_add_iface,
 	.del_virtual_intf = ieee80211_del_iface,
@@ -1649,4 +1663,6 @@ struct cfg80211_ops mac80211_config_ops = {
 	.cancel_remain_on_channel = ieee80211_cancel_remain_on_channel,
 	.action = ieee80211_action,
 	.set_cqm_rssi_config = ieee80211_set_cqm_rssi_config,
+	.set_antenna = ieee80211_set_antenna,
+	.get_antenna = ieee80211_get_antenna,
 };
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 14123dc..0aa6399 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -414,4 +414,27 @@ static inline void drv_channel_switch(struct ieee80211_local *local,
 	trace_drv_return_void(local);
 }
 
+
+static inline int drv_set_antenna(struct ieee80211_local *local,
+				  u8 tx_ant, u8 rx_ant)
+{
+	int ret = -EOPNOTSUPP;
+	might_sleep();
+	if (local->ops->set_antenna)
+		ret = local->ops->set_antenna(&local->hw, tx_ant, rx_ant);
+	trace_drv_set_antenna(local, tx_ant, rx_ant, ret);
+	return ret;
+}
+
+static inline int drv_get_antenna(struct ieee80211_local *local,
+				  u8 *tx_ant, u8 *rx_ant)
+{
+	int ret = -EOPNOTSUPP;
+	might_sleep();
+	if (local->ops->get_antenna)
+		ret = local->ops->get_antenna(&local->hw, tx_ant, rx_ant);
+	trace_drv_get_antenna(local, *tx_ant, *rx_ant, ret);
+	return ret;
+}
+
 #endif /* __MAC80211_DRIVER_OPS */
diff --git a/net/mac80211/driver-trace.h b/net/mac80211/driver-trace.h
index 5d5d2a9..3aac91b 100644
--- a/net/mac80211/driver-trace.h
+++ b/net/mac80211/driver-trace.h
@@ -832,6 +832,56 @@ TRACE_EVENT(drv_channel_switch,
 	)
 );
 
+TRACE_EVENT(drv_set_antenna,
+	TP_PROTO(struct ieee80211_local *local, u8 tx_ant, u8 rx_ant, int ret),
+
+	TP_ARGS(local, tx_ant, rx_ant, ret),
+
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+		__field(u8, tx_ant)
+		__field(u8, rx_ant)
+		__field(int, ret)
+	),
+
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+		__entry->tx_ant = tx_ant;
+		__entry->rx_ant = rx_ant;
+		__entry->ret = ret;
+	),
+
+	TP_printk(
+		LOCAL_PR_FMT " tx_ant:%d rx_ant:%d ret:%d",
+		LOCAL_PR_ARG, __entry->tx_ant, __entry->rx_ant, __entry->ret
+	)
+);
+
+TRACE_EVENT(drv_get_antenna,
+	TP_PROTO(struct ieee80211_local *local, u8 tx_ant, u8 rx_ant, int ret),
+
+	TP_ARGS(local, tx_ant, rx_ant, ret),
+
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+		__field(u8, tx_ant)
+		__field(u8, rx_ant)
+		__field(int, ret)
+	),
+
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+		__entry->tx_ant = tx_ant;
+		__entry->rx_ant = rx_ant;
+		__entry->ret = ret;
+	),
+
+	TP_printk(
+		LOCAL_PR_FMT " tx_ant:%d rx_ant:%d ret:%d",
+		LOCAL_PR_ARG, __entry->tx_ant, __entry->rx_ant, __entry->ret
+	)
+);
+
 /*
  * Tracing for API calls that drivers call.
  */

[PATCH v4 3/3] ath5k: Add support for antenna configuration

[Bruno Randolf]

Support setting the antenna configuration via cfg/mac80211. At the moment only
allow the simple pre-defined configurations we already have (fixed antenna A/B
or diversity), but more advanced settings are possible to implement.

Signed-off-by: Bruno Randolf <br1@einfach.org>
---
 drivers/net/wireless/ath/ath5k/base.c |   34 +++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 0d5de25..9150d1a 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -256,6 +256,8 @@ static void ath5k_sw_scan_start(struct ieee80211_hw *hw);
 static void ath5k_sw_scan_complete(struct ieee80211_hw *hw);
 static void ath5k_set_coverage_class(struct ieee80211_hw *hw,
 		u8 coverage_class);
+static int ath5k_set_antenna(struct ieee80211_hw *hw, u8 tx_ant, u8 rx_ant);
+static int ath5k_get_antenna(struct ieee80211_hw *hw, u8 *tx_ant, u8 *rx_ant);
 
 static const struct ieee80211_ops ath5k_hw_ops = {
 	.tx 		= ath5k_tx,
@@ -277,6 +279,8 @@ static const struct ieee80211_ops ath5k_hw_ops = {
 	.sw_scan_start	= ath5k_sw_scan_start,
 	.sw_scan_complete = ath5k_sw_scan_complete,
 	.set_coverage_class = ath5k_set_coverage_class,
+	.set_antenna	= ath5k_set_antenna,
+	.get_antenna	= ath5k_get_antenna,
 };
 
 /*
@@ -3526,3 +3530,33 @@ static void ath5k_set_coverage_class(struct ieee80211_hw *hw, u8 coverage_class)
 	ath5k_hw_set_coverage_class(sc->ah, coverage_class);
 	mutex_unlock(&sc->lock);
 }
+
+static int ath5k_set_antenna(struct ieee80211_hw *hw, u8 tx_ant, u8 rx_ant)
+{
+	struct ath5k_softc *sc = hw->priv;
+
+	if (tx_ant == 1 && rx_ant == 1)
+		ath5k_hw_set_antenna_mode(sc->ah, AR5K_ANTMODE_FIXED_A);
+	else if (tx_ant == 2 && rx_ant == 2)
+		ath5k_hw_set_antenna_mode(sc->ah, AR5K_ANTMODE_FIXED_B);
+	else if (tx_ant == 0 && (rx_ant & 3) == 3)
+		ath5k_hw_set_antenna_mode(sc->ah, AR5K_ANTMODE_DEFAULT);
+	else
+		return -EINVAL;
+	return 0;
+}
+
+static int ath5k_get_antenna(struct ieee80211_hw *hw, u8 *tx_ant, u8 *rx_ant)
+{
+	struct ath5k_softc *sc = hw->priv;
+
+	switch (sc->ah->ah_ant_mode) {
+	case AR5K_ANTMODE_FIXED_A:
+		*tx_ant = 1; *rx_ant = 1; break;
+	case AR5K_ANTMODE_FIXED_B:
+		*tx_ant = 2; *rx_ant = 2; break;
+	case AR5K_ANTMODE_DEFAULT:
+		*tx_ant = 0; *rx_ant = 3; break;
+	}
+	return 0;
+}

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Johannes Berg]

On Tue, 2010-07-27 at 18:48 +0900, Bruno Randolf wrote:

> The antenna configuration is defined as a bitmap of allowed antennas to use for
> either receiving or transmitting. Separate bitmaps are used for RX and TX to
> allow configuring different antennas for receiving and transmitting. The bitmap
> is 8 bit long, each bit representing one antenna, starting with antenna 1.
> 
> If multiple receive (RX) antennas are selected, the driver may use diversity or
> multiple 802.11n RX chains, according to the chipset capabilities and
> configuration.

I'm still not convinced that this is the best approach. Can you discuss
this a bit?

johannes

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Tue, Jul 27, 2010 at 2:48 AM, Bruno Randolf <br1@einfach.org> wrote:
> Allow setting TX and RX antenna configuration via nl80211.
>
> The antenna configuration is defined as a bitmap of allowed antennas to use for
> either receiving or transmitting. Separate bitmaps are used for RX and TX to
> allow configuring different antennas for receiving and transmitting. The bitmap
> is 8 bit long, each bit representing one antenna, starting with antenna 1.
>
> If multiple receive (RX) antennas are selected, the driver may use diversity or
> multiple 802.11n RX chains, according to the chipset capabilities and
> configuration.
>
> If multiple transmit (TX) antennas are selected, the driver has to send on all
> selected antennas, making this mode of operation only possible on 802.11n
> chipsets with multiple TX chains. If the transmit antenna bitmap is set to the
> special value zero (0) the driver should select the TX antenna based on the
> antenna which was used for receiving (TX antenna follows RX diversity - this is
> the usual mode of 'diversity') for pre 802.11n devices.
>
> Using bitmaps has the benefit of allowing for a flexible configuration
> interface which can support many different configurations. Drivers should
> reject configurations they cannot support. While covering the standard modes
> "fixed antenna 1", "fixed antenna 2" and "diversity" this API also allows more
> rare - but useful - configurations as follows:
>
> 1) Send on antenna 1, receive on antenna 2 (or vice versa). This can be used to
> have a low gain antenna for TX in order to keep within the regulatory
> constraints and a high gain antenna for RX in order to receive weaker signals
> ("speak softly, but listen harder"). This can be useful for building long-shot
> outdoor links. Another usage of this setup is having a low-noise pre-amplifier
> on antenna 1 and a power amplifier on the other antenna. This way transmit
> noise is mostly kept out of the low noise receive channel.
> (This would be bitmaps: tx 1 rx 2).
>
> 2) Another similar setup is: Use RX diversity on both antennas, but always send
> on antenna 1. Again that would allow us to benefit from a higher gain RX
> antenna, while staying within the legal limits.
> (This would be: tx 0 rx 3).
>
> 3) And finally there can be special experimental setups in research and
> development where more than 2 antennas are available. It's good to keep the API
> flexible.
>
> Signed-off-by: Bruno Randolf <br1@einfach.org>
> ---
>  include/linux/nl80211.h |   17 +++++++++++++++++
>  include/net/cfg80211.h  |    3 +++
>  net/wireless/nl80211.c  |   30 +++++++++++++++++++++++++++++-
>  3 files changed, 49 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> index 2c87016..b9de53c 100644
> --- a/include/linux/nl80211.h
> +++ b/include/linux/nl80211.h
> @@ -731,6 +731,20 @@ enum nl80211_commands {
>  *      This is used in association with @NL80211_ATTR_WIPHY_TX_POWER_SETTING
>  *      for non-automatic settings.
>  *
> + * @NL80211_ATTR_WIPHY_ANTENNA_TX: Bitmap of allowed antennas for transmitting.
> + *     Each bit represents one antenna, starting with antenna 1 at the first
> + *     bit. If the bitmap is zero (0), the TX antenna follows RX diversity.

What about for 802.11n? What if you want to disable TX?

> + *     If multiple antennas are selected all selected antennas have to be used
> + *     for transmitting (801.11n multiple TX chains).

I rather call this TX / RX chainmask then.

> + *     Drivers may reject configurations they cannot support.
> + *
> + * @NL80211_ATTR_WIPHY_ANTENNA_RX: Bitmap of allowed antennas for receiving.
> + *     Each bit represents one antenna, starting with antenna 1 at the first
> + *     bit. If multiple antennas are selected in the bitmap, 802.11n devices
> + *     should use multiple RX chains on these antennas, while non-802.11n
> + *     drivers should use antenna diversity between these antennas.

What about TX beamforming, and STBC?

Unless 802.11n is completely dealt with I really prefer this patch to
only address legacy. Otherwise I see sloppyness and inconsistencies on
supporting this feature throughout different drivers. I'd like to
avoid that at all costs on nl80211. What you are trying to address is
legacy antenna setup, not 802.11n RX/TX chainmask dynamic settings so
I'd really try to avoid it unless you really want to address all
aspects of chain configuration for 802.11n and even then what I'm
leading on to say is I think you'll see if you try to address both it
just gets messy.

  Luis

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Felix Fietkau]

On 2010-07-27 6:19 PM, Luis R. Rodriguez wrote:
>> + * @NL80211_ATTR_WIPHY_ANTENNA_TX: Bitmap of allowed antennas for transmitting.
>> + *     Each bit represents one antenna, starting with antenna 1 at the first
>> + *     bit. If the bitmap is zero (0), the TX antenna follows RX diversity.
> 
> What about for 802.11n? What if you want to disable TX?
Disabling tx shouldn't be handled by the antenna setting, IMHO.

>> + *     If multiple antennas are selected all selected antennas have to be used
>> + *     for transmitting (801.11n multiple TX chains).
> 
> I rather call this TX / RX chainmask then.
Well, for legacy hardware, these aren't really chains, as there is only
one rx and one tx path, just with switching onto multiple antennas.

>> + *     Drivers may reject configurations they cannot support.
>> + *
>> + * @NL80211_ATTR_WIPHY_ANTENNA_RX: Bitmap of allowed antennas for receiving.
>> + *     Each bit represents one antenna, starting with antenna 1 at the first
>> + *     bit. If multiple antennas are selected in the bitmap, 802.11n devices
>> + *     should use multiple RX chains on these antennas, while non-802.11n
>> + *     drivers should use antenna diversity between these antennas.
> 
> What about TX beamforming, and STBC?
Disabling one antenna/chain on a two-chain device would naturally
disable TxBF and STBC as well, since it limits the number of available
chains. The driver should simply act as if the disabled chains didn't exist.

> Unless 802.11n is completely dealt with I really prefer this patch to
> only address legacy. Otherwise I see sloppyness and inconsistencies on
> supporting this feature throughout different drivers. I'd like to
> avoid that at all costs on nl80211. What you are trying to address is
> legacy antenna setup, not 802.11n RX/TX chainmask dynamic settings so
> I'd really try to avoid it unless you really want to address all
> aspects of chain configuration for 802.11n and even then what I'm
> leading on to say is I think you'll see if you try to address both it
> just gets messy.
I think 802.11n is already completely dealt with if you treat this as
the chainmask on 11n devices.

- Felix

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Tue, Jul 27, 2010 at 9:39 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2010-07-27 6:19 PM, Luis R. Rodriguez wrote:
>>> + * @NL80211_ATTR_WIPHY_ANTENNA_TX: Bitmap of allowed antennas for transmitting.
>>> + *     Each bit represents one antenna, starting with antenna 1 at the first
>>> + *     bit. If the bitmap is zero (0), the TX antenna follows RX diversity.
>>
>> What about for 802.11n? What if you want to disable TX?
> Disabling tx shouldn't be handled by the antenna setting, IMHO.
>
>>> + *     If multiple antennas are selected all selected antennas have to be used
>>> + *     for transmitting (801.11n multiple TX chains).
>>
>> I rather call this TX / RX chainmask then.
> Well, for legacy hardware, these aren't really chains, as there is only
> one rx and one tx path, just with switching onto multiple antennas.
>
>>> + *     Drivers may reject configurations they cannot support.
>>> + *
>>> + * @NL80211_ATTR_WIPHY_ANTENNA_RX: Bitmap of allowed antennas for receiving.
>>> + *     Each bit represents one antenna, starting with antenna 1 at the first
>>> + *     bit. If multiple antennas are selected in the bitmap, 802.11n devices
>>> + *     should use multiple RX chains on these antennas, while non-802.11n
>>> + *     drivers should use antenna diversity between these antennas.
>>
>> What about TX beamforming, and STBC?
> Disabling one antenna/chain on a two-chain device would naturally
> disable TxBF and STBC as well, since it limits the number of available
> chains. The driver should simply act as if the disabled chains didn't exist.

That would work.

>> Unless 802.11n is completely dealt with I really prefer this patch to
>> only address legacy. Otherwise I see sloppyness and inconsistencies on
>> supporting this feature throughout different drivers. I'd like to
>> avoid that at all costs on nl80211. What you are trying to address is
>> legacy antenna setup, not 802.11n RX/TX chainmask dynamic settings so
>> I'd really try to avoid it unless you really want to address all
>> aspects of chain configuration for 802.11n and even then what I'm
>> leading on to say is I think you'll see if you try to address both it
>> just gets messy.
> I think 802.11n is already completely dealt with if you treat this as
> the chainmask on 11n devices.

Its fine by me if the above cases are also embedded into the
documentation, just don't want these questions lingering. I can't
think of other 802.11n special cases.

  Luis

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Bruno Randolf]

On Tue July 27 2010 19:05:42 you wrote:
> I'm still not convinced that this is the best approach. Can you discuss
> this a bit?

well... i am mainly concerned about what you call "legacy" hardware at the 
moment, but i am trying to make the API useful for 802.11n devices as well. 

first, thinking only about legacy devices with 2 antennas, the three most 
common settings are "fixed antenna 1", "fixed antenna 2" and "antenna 
diversity". luis suggested to use a simple value and constants for this, but i 
believe that this covers only 90% of the cases, the others being antenna 
setups i have described (in the commit message and per email before), which 
use a different antenna for TX than for RX, or diversity for RX and a single 
fixed antenna for TX. also, while not hugely popular or common, there *is* 
"legacy" hardware out there which has more than two antennas (e.g.: "pre-11n 
RangeMax" and "large phased array switch" which were based on ath5k chipsets) 
and at least the atheros eeprom supports more antennas. i'm not sure about 
other hardware, but i think it's good to have the flexibility to support more 
than two antennas in the API, also since this matches with what we need for 
802.11n. that's why a bitmask makes sense.

honestly, i don't know too much about 802.11n, so i need the help of people 
who work with 802.11n to see if this is useful for them and possibly extend 
it, or update the documentation to make the API more clear. but i believe that 
if you think 'chainmask' instead of 'antenna' it should be o.k...
in the end we are just talking about antennas :) for most of the users the 
antenna settings will just be default, but sometimes, especially in outdoor 
installations less antennas are present, or special setups can be desired and 
this API can be used to inform the HW about available antennas, wether it's 
802.11n or not. 

bruno

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Bruno Randolf]

On Wed July 28 2010 01:47:34 Luis R. Rodriguez wrote:
> On Tue, Jul 27, 2010 at 9:39 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> > On 2010-07-27 6:19 PM, Luis R. Rodriguez wrote:
> >>> + * @NL80211_ATTR_WIPHY_ANTENNA_TX: Bitmap of allowed antennas for
> >>> transmitting. + *     Each bit represents one antenna, starting with
> >>> antenna 1 at the first + *     bit. If the bitmap is zero (0), the TX
> >>> antenna follows RX diversity.
> >> 
> >> What about for 802.11n? What if you want to disable TX?
> > 
> > Disabling tx shouldn't be handled by the antenna setting, IMHO.
> > 
> >>> + *     If multiple antennas are selected all selected antennas have to
> >>> be used + *     for transmitting (801.11n multiple TX chains).
> >> 
> >> I rather call this TX / RX chainmask then.
> > 
> > Well, for legacy hardware, these aren't really chains, as there is only
> > one rx and one tx path, just with switching onto multiple antennas.
> > 
> >>> + *     Drivers may reject configurations they cannot support.
> >>> + *
> >>> + * @NL80211_ATTR_WIPHY_ANTENNA_RX: Bitmap of allowed antennas for
> >>> receiving. + *     Each bit represents one antenna, starting with
> >>> antenna 1 at the first + *     bit. If multiple antennas are selected
> >>> in the bitmap, 802.11n devices + *     should use multiple RX chains
> >>> on these antennas, while non-802.11n + *     drivers should use
> >>> antenna diversity between these antennas.
> >> 
> >> What about TX beamforming, and STBC?
> > 
> > Disabling one antenna/chain on a two-chain device would naturally
> > disable TxBF and STBC as well, since it limits the number of available
> > chains. The driver should simply act as if the disabled chains didn't
> > exist.
> 
> That would work.
> 
> >> Unless 802.11n is completely dealt with I really prefer this patch to
> >> only address legacy. Otherwise I see sloppyness and inconsistencies on
> >> supporting this feature throughout different drivers. I'd like to
> >> avoid that at all costs on nl80211. What you are trying to address is
> >> legacy antenna setup, not 802.11n RX/TX chainmask dynamic settings so
> >> I'd really try to avoid it unless you really want to address all
> >> aspects of chain configuration for 802.11n and even then what I'm
> >> leading on to say is I think you'll see if you try to address both it
> >> just gets messy.
> > 
> > I think 802.11n is already completely dealt with if you treat this as
> > the chainmask on 11n devices.
> 
> Its fine by me if the above cases are also embedded into the
> documentation, just don't want these questions lingering. I can't
> think of other 802.11n special cases.

thanks felix :)

luis, could you tell me what exactly you would want to include in the 
documentation?

bruno

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Tue, Jul 27, 2010 at 7:06 PM, Bruno Randolf <br1@einfach.org> wrote:
> On Wed July 28 2010 01:47:34 Luis R. Rodriguez wrote:
>> On Tue, Jul 27, 2010 at 9:39 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>> > On 2010-07-27 6:19 PM, Luis R. Rodriguez wrote:
>> >>> + * @NL80211_ATTR_WIPHY_ANTENNA_TX: Bitmap of allowed antennas for
>> >>> transmitting. + *     Each bit represents one antenna, starting with
>> >>> antenna 1 at the first + *     bit. If the bitmap is zero (0), the TX
>> >>> antenna follows RX diversity.
>> >>
>> >> What about for 802.11n? What if you want to disable TX?
>> >
>> > Disabling tx shouldn't be handled by the antenna setting, IMHO.
>> >
>> >>> + *     If multiple antennas are selected all selected antennas have to
>> >>> be used + *     for transmitting (801.11n multiple TX chains).
>> >>
>> >> I rather call this TX / RX chainmask then.
>> >
>> > Well, for legacy hardware, these aren't really chains, as there is only
>> > one rx and one tx path, just with switching onto multiple antennas.
>> >
>> >>> + *     Drivers may reject configurations they cannot support.
>> >>> + *
>> >>> + * @NL80211_ATTR_WIPHY_ANTENNA_RX: Bitmap of allowed antennas for
>> >>> receiving. + *     Each bit represents one antenna, starting with
>> >>> antenna 1 at the first + *     bit. If multiple antennas are selected
>> >>> in the bitmap, 802.11n devices + *     should use multiple RX chains
>> >>> on these antennas, while non-802.11n + *     drivers should use
>> >>> antenna diversity between these antennas.
>> >>
>> >> What about TX beamforming, and STBC?
>> >
>> > Disabling one antenna/chain on a two-chain device would naturally
>> > disable TxBF and STBC as well, since it limits the number of available
>> > chains. The driver should simply act as if the disabled chains didn't
>> > exist.
>>
>> That would work.
>>
>> >> Unless 802.11n is completely dealt with I really prefer this patch to
>> >> only address legacy. Otherwise I see sloppyness and inconsistencies on
>> >> supporting this feature throughout different drivers. I'd like to
>> >> avoid that at all costs on nl80211. What you are trying to address is
>> >> legacy antenna setup, not 802.11n RX/TX chainmask dynamic settings so
>> >> I'd really try to avoid it unless you really want to address all
>> >> aspects of chain configuration for 802.11n and even then what I'm
>> >> leading on to say is I think you'll see if you try to address both it
>> >> just gets messy.
>> >
>> > I think 802.11n is already completely dealt with if you treat this as
>> > the chainmask on 11n devices.
>>
>> Its fine by me if the above cases are also embedded into the
>> documentation, just don't want these questions lingering. I can't
>> think of other 802.11n special cases.
>
> thanks felix :)
>
> luis, could you tell me what exactly you would want to include in the
> documentation?

Sure, but after some though I'm sticking to my preference for an API
for this, legacy and 802.11n chainmask / antenna selection should be
dealt with separately.

Reason is I just reviewed the IEEE 802.11 sections 19.19 for TX
beamforming but also happened to stumble upon section 19.20 Antenna
selection. This Antenna Selection section indicates you can support
more antennas than chains and you can use and that the antenna you use
for your chains can vary over time. You select which antenna to use
based on training/sounding tests on each antenna. Antenna selection as
per 19.19 supports up to 8 antennas and up to 4 RF chains. Section
19.20.2 deals with how a STA can initiate antenna selection training
with another STA. Now granted we don't have code for this and I don't
think this is all done explicitly in hardware so one can argue we can
deal with this when/if we do add support for this but this just
highlights how different "antenna selection" or knobs to tune antennas
is even from a standard perspective than legacy. For STBC and TX
beamforming requirements I would like to see mac80211 actually have
code which disables STBC and later TX beamforming (once we have code
for it) if we enable only one chain, I don't see why drivers should
deal with this.

So if you want to define an API I do believe its best to treat these
separately, for legacy I think your API can be fine tuned to consider
FIXED_A, FIXED_B, or DIVERSITY. If you really want some knobs to even
allow for 11n I would recommend debugfs for now which would let you do
anything you want.

  Luis

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Felix Fietkau]

On 2010-07-28 7:24 PM, Luis R. Rodriguez wrote:
> On Tue, Jul 27, 2010 at 7:06 PM, Bruno Randolf <br1@einfach.org> wrote:
>> On Wed July 28 2010 01:47:34 Luis R. Rodriguez wrote:
>>> On Tue, Jul 27, 2010 at 9:39 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>> > On 2010-07-27 6:19 PM, Luis R. Rodriguez wrote:
>>> >>> + * @NL80211_ATTR_WIPHY_ANTENNA_TX: Bitmap of allowed antennas for
>>> >>> transmitting. + *     Each bit represents one antenna, starting with
>>> >>> antenna 1 at the first + *     bit. If the bitmap is zero (0), the TX
>>> >>> antenna follows RX diversity.
>>> >>
>>> >> What about for 802.11n? What if you want to disable TX?
>>> >
>>> > Disabling tx shouldn't be handled by the antenna setting, IMHO.
>>> >
>>> >>> + *     If multiple antennas are selected all selected antennas have to
>>> >>> be used + *     for transmitting (801.11n multiple TX chains).
>>> >>
>>> >> I rather call this TX / RX chainmask then.
>>> >
>>> > Well, for legacy hardware, these aren't really chains, as there is only
>>> > one rx and one tx path, just with switching onto multiple antennas.
>>> >
>>> >>> + *     Drivers may reject configurations they cannot support.
>>> >>> + *
>>> >>> + * @NL80211_ATTR_WIPHY_ANTENNA_RX: Bitmap of allowed antennas for
>>> >>> receiving. + *     Each bit represents one antenna, starting with
>>> >>> antenna 1 at the first + *     bit. If multiple antennas are selected
>>> >>> in the bitmap, 802.11n devices + *     should use multiple RX chains
>>> >>> on these antennas, while non-802.11n + *     drivers should use
>>> >>> antenna diversity between these antennas.
>>> >>
>>> >> What about TX beamforming, and STBC?
>>> >
>>> > Disabling one antenna/chain on a two-chain device would naturally
>>> > disable TxBF and STBC as well, since it limits the number of available
>>> > chains. The driver should simply act as if the disabled chains didn't
>>> > exist.
>>>
>>> That would work.
>>>
>>> >> Unless 802.11n is completely dealt with I really prefer this patch to
>>> >> only address legacy. Otherwise I see sloppyness and inconsistencies on
>>> >> supporting this feature throughout different drivers. I'd like to
>>> >> avoid that at all costs on nl80211. What you are trying to address is
>>> >> legacy antenna setup, not 802.11n RX/TX chainmask dynamic settings so
>>> >> I'd really try to avoid it unless you really want to address all
>>> >> aspects of chain configuration for 802.11n and even then what I'm
>>> >> leading on to say is I think you'll see if you try to address both it
>>> >> just gets messy.
>>> >
>>> > I think 802.11n is already completely dealt with if you treat this as
>>> > the chainmask on 11n devices.
>>>
>>> Its fine by me if the above cases are also embedded into the
>>> documentation, just don't want these questions lingering. I can't
>>> think of other 802.11n special cases.
>>
>> thanks felix :)
>>
>> luis, could you tell me what exactly you would want to include in the
>> documentation?
> 
> Sure, but after some though I'm sticking to my preference for an API
> for this, legacy and 802.11n chainmask / antenna selection should be
> dealt with separately.
As I pointed out in an earlier discussion, they *cannot* really be
treated separately if you're running in AP mode, dealing with both
legacy and 11n clients.

> Reason is I just reviewed the IEEE 802.11 sections 19.19 for TX
> beamforming but also happened to stumble upon section 19.20 Antenna
> selection. This Antenna Selection section indicates you can support
> more antennas than chains and you can use and that the antenna you use
> for your chains can vary over time. You select which antenna to use
> based on training/sounding tests on each antenna. Antenna selection as
> per 19.19 supports up to 8 antennas and up to 4 RF chains. Section
> 19.20.2 deals with how a STA can initiate antenna selection training
> with another STA. Now granted we don't have code for this and I don't
> think this is all done explicitly in hardware so one can argue we can
> deal with this when/if we do add support for this but this just
> highlights how different "antenna selection" or knobs to tune antennas
> is even from a standard perspective than legacy. For STBC and TX
> beamforming requirements I would like to see mac80211 actually have
> code which disables STBC and later TX beamforming (once we have code
> for it) if we enable only one chain, I don't see why drivers should
> deal with this.
This is easy to handle, it can work like this:
We treat the setting not as a raw chainmask, but actually as an antenna
mask. The chainmask gets calculated from that. If we disable all
antennas belonging to a particular chain, we disable the entire chain.
For AR9285 we will already deviate from treating this as a chainmask,
because it has one chain with rx diversity. Please accept this API, I'm
sure the API can be used to handle everything related to 802.11n
antenna/chain selection properly. :)

- Felix

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Wed, Jul 28, 2010 at 10:50 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2010-07-28 7:24 PM, Luis R. Rodriguez wrote:
>> On Tue, Jul 27, 2010 at 7:06 PM, Bruno Randolf <br1@einfach.org> wrote:
>>> On Wed July 28 2010 01:47:34 Luis R. Rodriguez wrote:
>>>> On Tue, Jul 27, 2010 at 9:39 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>>> > On 2010-07-27 6:19 PM, Luis R. Rodriguez wrote:
>>>> >>> + * @NL80211_ATTR_WIPHY_ANTENNA_TX: Bitmap of allowed antennas for
>>>> >>> transmitting. + *     Each bit represents one antenna, starting with
>>>> >>> antenna 1 at the first + *     bit. If the bitmap is zero (0), the TX
>>>> >>> antenna follows RX diversity.
>>>> >>
>>>> >> What about for 802.11n? What if you want to disable TX?
>>>> >
>>>> > Disabling tx shouldn't be handled by the antenna setting, IMHO.
>>>> >
>>>> >>> + *     If multiple antennas are selected all selected antennas have to
>>>> >>> be used + *     for transmitting (801.11n multiple TX chains).
>>>> >>
>>>> >> I rather call this TX / RX chainmask then.
>>>> >
>>>> > Well, for legacy hardware, these aren't really chains, as there is only
>>>> > one rx and one tx path, just with switching onto multiple antennas.
>>>> >
>>>> >>> + *     Drivers may reject configurations they cannot support.
>>>> >>> + *
>>>> >>> + * @NL80211_ATTR_WIPHY_ANTENNA_RX: Bitmap of allowed antennas for
>>>> >>> receiving. + *     Each bit represents one antenna, starting with
>>>> >>> antenna 1 at the first + *     bit. If multiple antennas are selected
>>>> >>> in the bitmap, 802.11n devices + *     should use multiple RX chains
>>>> >>> on these antennas, while non-802.11n + *     drivers should use
>>>> >>> antenna diversity between these antennas.
>>>> >>
>>>> >> What about TX beamforming, and STBC?
>>>> >
>>>> > Disabling one antenna/chain on a two-chain device would naturally
>>>> > disable TxBF and STBC as well, since it limits the number of available
>>>> > chains. The driver should simply act as if the disabled chains didn't
>>>> > exist.
>>>>
>>>> That would work.
>>>>
>>>> >> Unless 802.11n is completely dealt with I really prefer this patch to
>>>> >> only address legacy. Otherwise I see sloppyness and inconsistencies on
>>>> >> supporting this feature throughout different drivers. I'd like to
>>>> >> avoid that at all costs on nl80211. What you are trying to address is
>>>> >> legacy antenna setup, not 802.11n RX/TX chainmask dynamic settings so
>>>> >> I'd really try to avoid it unless you really want to address all
>>>> >> aspects of chain configuration for 802.11n and even then what I'm
>>>> >> leading on to say is I think you'll see if you try to address both it
>>>> >> just gets messy.
>>>> >
>>>> > I think 802.11n is already completely dealt with if you treat this as
>>>> > the chainmask on 11n devices.
>>>>
>>>> Its fine by me if the above cases are also embedded into the
>>>> documentation, just don't want these questions lingering. I can't
>>>> think of other 802.11n special cases.
>>>
>>> thanks felix :)
>>>
>>> luis, could you tell me what exactly you would want to include in the
>>> documentation?
>>
>> Sure, but after some though I'm sticking to my preference for an API
>> for this, legacy and 802.11n chainmask / antenna selection should be
>> dealt with separately.
>
> As I pointed out in an earlier discussion, they *cannot* really be
> treated separately if you're running in AP mode, dealing with both
> legacy and 11n clients.

In AP mode you'd use the 11n mode stuff not the legacy stuff, so not
sure how this matters.

>> Reason is I just reviewed the IEEE 802.11 sections 19.19 for TX
>> beamforming but also happened to stumble upon section 19.20 Antenna
>> selection. This Antenna Selection section indicates you can support
>> more antennas than chains and you can use and that the antenna you use
>> for your chains can vary over time. You select which antenna to use
>> based on training/sounding tests on each antenna. Antenna selection as
>> per 19.19 supports up to 8 antennas and up to 4 RF chains. Section
>> 19.20.2 deals with how a STA can initiate antenna selection training
>> with another STA. Now granted we don't have code for this and I don't
>> think this is all done explicitly in hardware so one can argue we can
>> deal with this when/if we do add support for this but this just
>> highlights how different "antenna selection" or knobs to tune antennas
>> is even from a standard perspective than legacy. For STBC and TX
>> beamforming requirements I would like to see mac80211 actually have
>> code which disables STBC and later TX beamforming (once we have code
>> for it) if we enable only one chain, I don't see why drivers should
>> deal with this.
>
> This is easy to handle, it can work like this:
> We treat the setting not as a raw chainmask, but actually as an antenna
> mask. The chainmask gets calculated from that. If we disable all
> antennas belonging to a particular chain, we disable the entire chain.

Fair enough. Would cfg80211 deal with this logic then? That would be
my preference. But then, do we even expose enough hardware
capabilities for cfg80211 to make this decision already?

> For AR9285 we will already deviate from treating this as a chainmask,
> because it has one chain with rx diversity.

Good point but the single stream 802.11n case just needs to be
documented as well, it'll be like that for other single stream 802.11n
devices, if there are others, not sure if Atheros has the only ones in
the market or what.

> Please accept this API

If you really need some knobs without detailed review and discussion
shoot for debugfs, otherwise I will take my time reviewing carefully
all the details because once its API then.. well its set in stone and
we have to deal with it. I realize I'm being a real big fucking pain
in the ass with this but.. I believe these discussions have also been
helpful.

> I'm sure the API can be used to handle everything related to 802.11n
> antenna/chain selection properly. :)

Sure, but who deals with the 11n stuff? Right now this patches just
pass two variables, TX and RX antenna mask, and has no special case
handling for the different 802.11n cases, nor does it document who
should handle that.

  * STBC (11n 9.6.0.c, 9.7g, etc)
  * TX Beamforming (11n 19.19)
  * Antenna selection (11n 19.20)

IMHO we should be thinking about this if we want to define an API that
*all* 802.11 drivers can use if the API will be used for 802.11n as
well.

  Luis

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Felix Fietkau]

On 2010-07-28 11:15 PM, Luis R. Rodriguez wrote:
> On Wed, Jul 28, 2010 at 10:50 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>> This is easy to handle, it can work like this:
>> We treat the setting not as a raw chainmask, but actually as an antenna
>> mask. The chainmask gets calculated from that. If we disable all
>> antennas belonging to a particular chain, we disable the entire chain.
> 
> Fair enough. Would cfg80211 deal with this logic then? That would be
> my preference. But then, do we even expose enough hardware
> capabilities for cfg80211 to make this decision already?
There isn't much for cfg80211 to deal with, really.

>> For AR9285 we will already deviate from treating this as a chainmask,
>> because it has one chain with rx diversity.
> 
> Good point but the single stream 802.11n case just needs to be
> documented as well, it'll be like that for other single stream 802.11n
> devices, if there are others, not sure if Atheros has the only ones in
> the market or what.
> 
>> Please accept this API
> 
> If you really need some knobs without detailed review and discussion
> shoot for debugfs, otherwise I will take my time reviewing carefully
> all the details because once its API then.. well its set in stone and
> we have to deal with it. I realize I'm being a real big fucking pain
> in the ass with this but.. I believe these discussions have also been
> helpful.
> 
>> I'm sure the API can be used to handle everything related to 802.11n
>> antenna/chain selection properly. :)
> 
> Sure, but who deals with the 11n stuff? Right now this patches just
> pass two variables, TX and RX antenna mask, and has no special case
> handling for the different 802.11n cases, nor does it document who
> should handle that.
> 
>   * STBC (11n 9.6.0.c, 9.7g, etc)
>   * TX Beamforming (11n 19.19)
>   * Antenna selection (11n 19.20)
> 
> IMHO we should be thinking about this if we want to define an API that
> *all* 802.11 drivers can use if the API will be used for 802.11n as
> well.
We don't need any special case handling for this at all. Drivers already
calculate their HT capabilities based on the number of available chains.
Once the antenna selection stuff is actually used, they will have some
internal information about which chains have how many antennas.

The reason why we can ignore *all* of this stuff for the API is simple:
We only need to refactor the code to calculate these settings based on
effective chainmask / antenna mask instead of pure hardware capability.

The effective chainmask / antenna mask is basically the same as the
hardware settings, except that it gets masked with the values that are
configured through this API. That leaves us with something that's easy
to configure, easy to implement for drivers, and doesn't need special
case stuff for various 802.11n features.

- Felix

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Wed, Jul 28, 2010 at 02:26:35PM -0700, Felix Fietkau wrote:
> On 2010-07-28 11:15 PM, Luis R. Rodriguez wrote:
> > On Wed, Jul 28, 2010 at 10:50 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> >> For AR9285 we will already deviate from treating this as a chainmask,
> >> because it has one chain with rx diversity.
> > 
> > Good point but the single stream 802.11n case just needs to be
> > documented as well, it'll be like that for other single stream 802.11n
> > devices, if there are others, not sure if Atheros has the only ones in
> > the market or what.
> > 
> >> Please accept this API
> > 
> > If you really need some knobs without detailed review and discussion
> > shoot for debugfs, otherwise I will take my time reviewing carefully
> > all the details because once its API then.. well its set in stone and
> > we have to deal with it. I realize I'm being a real big fucking pain
> > in the ass with this but.. I believe these discussions have also been
> > helpful.
> > 
> >> I'm sure the API can be used to handle everything related to 802.11n
> >> antenna/chain selection properly. :)
> > 
> > Sure, but who deals with the 11n stuff? Right now this patches just
> > pass two variables, TX and RX antenna mask, and has no special case
> > handling for the different 802.11n cases, nor does it document who
> > should handle that.
> > 
> >   * STBC (11n 9.6.0.c, 9.7g, etc)
> >   * TX Beamforming (11n 19.19)
> >   * Antenna selection (11n 19.20)
> > 
> > IMHO we should be thinking about this if we want to define an API that
> > *all* 802.11 drivers can use if the API will be used for 802.11n as
> > well.
>
> We don't need any special case handling for this at all. Drivers already
> calculate their HT capabilities based on the number of available chains.
> Once the antenna selection stuff is actually used, they will have some
> internal information about which chains have how many antennas.
> 
> The reason why we can ignore *all* of this stuff for the API is simple:
> We only need to refactor the code to calculate these settings based on
> effective chainmask / antenna mask instead of pure hardware capability.
> 
> The effective chainmask / antenna mask is basically the same as the
> hardware settings, except that it gets masked with the values that are
> configured through this API. That leaves us with something that's easy
> to configure, easy to implement for drivers, and doesn't need special
> case stuff for various 802.11n features.

Consider the case of an already associated STA in 3x3 mode, and someone
uses this API to limit it to a 1 stream 1x1 chaimask setting using
only one antenna. How would the AP find out about this RX setting
from the STA? Are we going to deny mucking with this prior to association?
What about if you're the AP?

If you are using STBC and the user tunes the device to 1 stream 1x1 chainmask
settings, who deals with the required adaptations?

  Luis

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Felix Fietkau]

On 2010-07-28 11:39 PM, Luis R. Rodriguez wrote:
> On Wed, Jul 28, 2010 at 02:26:35PM -0700, Felix Fietkau wrote:
>> We don't need any special case handling for this at all. Drivers already
>> calculate their HT capabilities based on the number of available chains.
>> Once the antenna selection stuff is actually used, they will have some
>> internal information about which chains have how many antennas.
>> 
>> The reason why we can ignore *all* of this stuff for the API is simple:
>> We only need to refactor the code to calculate these settings based on
>> effective chainmask / antenna mask instead of pure hardware capability.
>> 
>> The effective chainmask / antenna mask is basically the same as the
>> hardware settings, except that it gets masked with the values that are
>> configured through this API. That leaves us with something that's easy
>> to configure, easy to implement for drivers, and doesn't need special
>> case stuff for various 802.11n features.
> 
> Consider the case of an already associated STA in 3x3 mode, and someone
> uses this API to limit it to a 1 stream 1x1 chaimask setting using
> only one antenna. How would the AP find out about this RX setting
> from the STA? Are we going to deny mucking with this prior to association?
> What about if you're the AP?
> 
> If you are using STBC and the user tunes the device to 1 stream 1x1 chainmask
> settings, who deals with the required adaptations?
I think we should simply not accept runtime modifications of this stuff.
The user should bring down the interface, change the value, then bring
it back up again. That allows the driver to recalculate all the HT stuff
based on the updated chainmask/antenna mask without special cases.

- Felix

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Wed, Jul 28, 2010 at 3:05 PM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2010-07-28 11:39 PM, Luis R. Rodriguez wrote:
>> On Wed, Jul 28, 2010 at 02:26:35PM -0700, Felix Fietkau wrote:
>>> We don't need any special case handling for this at all. Drivers already
>>> calculate their HT capabilities based on the number of available chains.
>>> Once the antenna selection stuff is actually used, they will have some
>>> internal information about which chains have how many antennas.
>>>
>>> The reason why we can ignore *all* of this stuff for the API is simple:
>>> We only need to refactor the code to calculate these settings based on
>>> effective chainmask / antenna mask instead of pure hardware capability.
>>>
>>> The effective chainmask / antenna mask is basically the same as the
>>> hardware settings, except that it gets masked with the values that are
>>> configured through this API. That leaves us with something that's easy
>>> to configure, easy to implement for drivers, and doesn't need special
>>> case stuff for various 802.11n features.
>>
>> Consider the case of an already associated STA in 3x3 mode, and someone
>> uses this API to limit it to a 1 stream 1x1 chaimask setting using
>> only one antenna. How would the AP find out about this RX setting
>> from the STA? Are we going to deny mucking with this prior to association?
>> What about if you're the AP?
>>
>> If you are using STBC and the user tunes the device to 1 stream 1x1 chainmask
>> settings, who deals with the required adaptations?
> I think we should simply not accept runtime modifications of this stuff.
> The user should bring down the interface, change the value, then bring
> it back up again. That allows the driver to recalculate all the HT stuff
> based on the updated chainmask/antenna mask without special cases.

Sound good, so based on the passed info, will cfg80211 lift off STBC
capability settings if 1 stream 1x1 settings are used?

  Luis

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Wed, Jul 28, 2010 at 03:05:45PM -0700, Felix Fietkau wrote:
> On 2010-07-28 11:39 PM, Luis R. Rodriguez wrote:
> > On Wed, Jul 28, 2010 at 02:26:35PM -0700, Felix Fietkau wrote:
> >> We don't need any special case handling for this at all. Drivers already
> >> calculate their HT capabilities based on the number of available chains.
> >> Once the antenna selection stuff is actually used, they will have some
> >> internal information about which chains have how many antennas.
> >> 
> >> The reason why we can ignore *all* of this stuff for the API is simple:
> >> We only need to refactor the code to calculate these settings based on
> >> effective chainmask / antenna mask instead of pure hardware capability.
> >> 
> >> The effective chainmask / antenna mask is basically the same as the
> >> hardware settings, except that it gets masked with the values that are
> >> configured through this API. That leaves us with something that's easy
> >> to configure, easy to implement for drivers, and doesn't need special
> >> case stuff for various 802.11n features.
> > 
> > Consider the case of an already associated STA in 3x3 mode, and someone
> > uses this API to limit it to a 1 stream 1x1 chaimask setting using
> > only one antenna. How would the AP find out about this RX setting
> > from the STA? Are we going to deny mucking with this prior to association?
> > What about if you're the AP?
> > 
> > If you are using STBC and the user tunes the device to 1 stream 1x1 chainmask
> > settings, who deals with the required adaptations?
> I think we should simply not accept runtime modifications of this stuff.
> The user should bring down the interface, change the value, then bring
> it back up again. That allows the driver to recalculate all the HT stuff
> based on the updated chainmask/antenna mask without special cases.

Works with me.

Would we keep two values for some of these settings, an "actual hw"
capablity and then also a "configured" values? Would this be exposed
and visible through iw?

  Luis

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Bruno Randolf]

On Thu July 29 2010 02:24:46 Luis R. Rodriguez wrote:
> So if you want to define an API I do believe its best to treat these
> separately, for legacy I think your API can be fine tuned to consider
> FIXED_A, FIXED_B, or DIVERSITY. 

hi luis, 

i already tried to explain several times why FIXED_A, FIXED_B, or DIVERSITY is 
not enough even for "legacy". please re-read the mails and the description of 
the first patch - i really don't want to re-iterate it *again*. thanks :)

as felix pointed out, it's best to think of this as the antenna mask, which 
can be used to mask out antennas which are not attached or should not be used. 
it does not matter if the device is "legacy" or 802.11n or "802.11xyz" - we 
can always use this API to inform mac80211 / the driver about the available 
antennas if they are in fact different from the default HW settings. based on 
this information 802.11n drivers can derive which chainmasks to use, and in 
some cases wether diversity should be used; "legacy" drivers can derive wether 
to use diversity or not.

actually thinking about it like this, i would like to remove the special 
casing of "0" for tx diversity (which means go back to my original proposal): 
if an antenna can be used for TX it should be set to one in the mask, and the 
driver can enable diversity or mulitple chainmasks as suitable.

> Would we keep two values for some of these settings, an "actual hw"
> capablity and then also a "configured" values? Would this be exposed
> and visible through iw?

the actual HW capability could be exported via the wiphy info as well, but i 
think this could be dealt with in a seperate patch, as well as other 
improvements. for now let's just define an API for an antenna mask, which can 
be used to mask out antennas seperately for RX and TX.

i'll try to make that more clear in the descriptions and re-send my proposal.

bruno

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Wed, Jul 28, 2010 at 7:11 PM, Bruno Randolf <br1@einfach.org> wrote:
> i already tried to explain several times why FIXED_A, FIXED_B, or DIVERSITY is
> not enough even for "legacy". please re-read the mails and the description of
> the first patch - i really don't want to re-iterate it *again*. thanks :)

What legacy device and respective device driver do we support upstream
on the kernel that has this sort of extra antenna setup? Do you
foresee this happening? New silicon simply won't be using legacy
(non-802.11) so I do not expect new chipsets to have this setup and I
do not foresee neither a driver supporting these devices nor interest
from anyone actually implementing this upstream.

Even consider Turbo stuff on Atheros legacy chipsets. For eons I've
been recommending this crap just be removed from ath5k but yet there
has been resistance. To this day no one has worked on it nor do I
expect anyone will and actually get Turbo support upstream.

So yes, perhaps some odd ball legacy devices exist with a setup other
than ANT_A ANT_B and DIVERSITY but I do not think we will support them
upstream, ever.

  Luis

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Bruno Randolf]

On Fri July 30 2010 00:09:17 you wrote:
> On Wed, Jul 28, 2010 at 7:11 PM, Bruno Randolf <br1@einfach.org> wrote:
> > i already tried to explain several times why FIXED_A, FIXED_B, or
> > DIVERSITY is not enough even for "legacy". please re-read the mails and
> > the description of the first patch - i really don't want to re-iterate
> > it *again*. thanks :)
> 
> What legacy device and respective device driver do we support upstream
> on the kernel that has this sort of extra antenna setup? Do you
> foresee this happening?

right now, probably none. but it's easy to this to ath5k. madwifi supported it 
and it's just a matter of setting tx and rx antennas (it's already there as 
AR5K_ANTMODE_DEBUG). i might add it, just to make my case... ;)

anyhow, i can see your point of view now. but while you might not care about 
anything before 802.11n - there are millions of what you call 'legacy' chips 
out there (and you guys are actually still selling them!), so i think it's 
worth it to properly support them, especially since i still believe that my 
proposal is usable for both 802.11n and before.

bruno

Re: [PATCH v4 1/3] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Sun, Aug 1, 2010 at 8:59 PM, Bruno Randolf <br1@einfach.org> wrote:
> On Fri July 30 2010 00:09:17 you wrote:
>> On Wed, Jul 28, 2010 at 7:11 PM, Bruno Randolf <br1@einfach.org> wrote:
>> > i already tried to explain several times why FIXED_A, FIXED_B, or
>> > DIVERSITY is not enough even for "legacy". please re-read the mails and
>> > the description of the first patch - i really don't want to re-iterate
>> > it *again*. thanks :)
>>
>> What legacy device and respective device driver do we support upstream
>> on the kernel that has this sort of extra antenna setup? Do you
>> foresee this happening?
>
> right now, probably none. but it's easy to this to ath5k. madwifi supported it
> and it's just a matter of setting tx and rx antennas (it's already there as
> AR5K_ANTMODE_DEBUG). i might add it, just to make my case... ;)
>
> anyhow, i can see your point of view now. but while you might not care about
> anything before 802.11n - there are millions of what you call 'legacy' chips
> out there (and you guys are actually still selling them!), so i think it's
> worth it to properly support them, especially since i still believe that my
> proposal is usable for both 802.11n and before.

I am not saying to drop legacy support I am saying to just support
legacy via the two standard antenna model because there is nothing
visible being supported which I think merits expanding a simple API
for legacy.

 Luis