[PATCH v5] cfg80211: Add nl80211 antenna configuration

[PATCH v5] cfg80211: Add nl80211 antenna configuration

[Bruno Randolf]

Allow setting of TX and RX antenna configuration via nl80211.

The antenna configuration is defined as a bitmap of allowed antennas to use.
This API can be used to mask out antennas which are not attached or should not
be used for other reasons like regulatory concerns or special setups.

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 at the first bit. If an antenna bit is
set, this means the driver is allowed to use this antenna for RX or TX
respectively; if the bit is not set the hardware is not allowed to use this
antenna.

Using bitmaps has the benefit of allowing for a flexible configuration
interface which can support many different configurations and which can be used
for 802.11n as well as non-802.11n devices. Instead of relying on some hardware
specific assumptions, drivers can use this information to know which antennas
are actually attached to the system and derive their capabilities based on
that.

802.11n devices should enable or disable chains, based on which antennas are
present (If all antennas belonging to a particular chain are disabled, the
entire chain should be disabled). HT capabilities (like STBC, TX Beamforming,
Antenna selection) should be calculated based on the available chains after
applying the antenna masks. Should a 802.11n device have diversity antennas
attached to one of their chains, diversity can be enabled or disabled based on
the antenna information.

Non-802.11n drivers can use the antenna masks to enable or disable antenna
diversity.

While covering chainmasks for 802.11n and the standard "legacy" 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 even with pre 802.11n hardware where more than 2 antennas are
available. It's good to keep the API simple, yet flexible.

Signed-off-by: Bruno Randolf <br1@einfach.org>
---

v5: Removed the special casing of "0" for diversity. I hope i made the
descriptions more clear and incorportated all comments concerning 802.11n. If
we can agree on this i will re-send the whole series.

---
 include/linux/nl80211.h |   25 +++++++++++++++++++++++++
 include/net/cfg80211.h  |    3 +++
 net/wireless/nl80211.c  |   30 +++++++++++++++++++++++++++++-
 3 files changed, 57 insertions(+), 1 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 2c87016..8a88921 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -731,6 +731,28 @@ 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.
+ *	This can be used to mask out antennas which are not attached or should
+ *	not be used for transmitting. If an antenna is not selected in this
+ *	bitmap the hardware is not allowed to transmit on this antenna.
+ *
+ *	Each bit represents one antenna, starting with antenna 1 at the first
+ *	bit. Depending on which antennas are selected in the bitmap, 802.11n
+ *	drivers can derive which chainmasks to use (if all antennas belonging to
+ *	a particular chain are disabled this chain should be disabled) and if
+ *	a chain has diversity antennas wether diversity should be used or not.
+ *	HT capabilities (STBC, TX Beamforming, Antenna selection) can be
+ *	derived from the available chains after applying the antenna mask.
+ *	Non-802.11n drivers can derive wether to use diversity or not.
+ *	Drivers may reject configurations or RX/TX mask combinations they cannot
+ *	support by returning -EINVAL.
+ *
+ * @NL80211_ATTR_WIPHY_ANTENNA_RX: Bitmap of allowed antennas for receiving.
+ *	This can be used to mask out antennas which are not attached or should
+ *	not be used for receiving. If an antenna is not selected in this bitmap
+ *	the hardware should not be configured to receive on this antenna.
+ *	For a more detailed descripton see @NL80211_ATTR_WIPHY_ANTENNA_TX.
+ *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
  */
@@ -891,6 +913,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]) {

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Wed, Jul 28, 2010 at 8:58 PM, Bruno Randolf <br1@einfach.org> wrote:
> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> index 2c87016..8a88921 100644
> --- a/include/linux/nl80211.h
> +++ b/include/linux/nl80211.h
> @@ -731,6 +731,28 @@ 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.
> + *     This can be used to mask out antennas which are not attached or should
> + *     not be used for transmitting. If an antenna is not selected in this
> + *     bitmap the hardware is not allowed to transmit on this antenna.
> + *
> + *     Each bit represents one antenna, starting with antenna 1 at the first
> + *     bit. Depending on which antennas are selected in the bitmap, 802.11n
> + *     drivers can derive which chainmasks to use (if all antennas belonging to
> + *     a particular chain are disabled this chain should be disabled) and if
> + *     a chain has diversity antennas wether diversity should be used or not.
> + *     HT capabilities (STBC, TX Beamforming, Antenna selection) can be
> + *     derived from the available chains after applying the antenna mask.

I don't want to do any work myself on drivers for this, can we have
cfg80211/mac80211 do this for us?

  Luis

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Johannes Berg]

On Wed, 2010-07-28 at 23:12 -0700, Luis R. Rodriguez wrote:
> On Wed, Jul 28, 2010 at 8:58 PM, Bruno Randolf <br1@einfach.org> wrote:
> > diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> > index 2c87016..8a88921 100644
> > --- a/include/linux/nl80211.h
> > +++ b/include/linux/nl80211.h
> > @@ -731,6 +731,28 @@ 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.
> > + *     This can be used to mask out antennas which are not attached or should
> > + *     not be used for transmitting. If an antenna is not selected in this
> > + *     bitmap the hardware is not allowed to transmit on this antenna.
> > + *
> > + *     Each bit represents one antenna, starting with antenna 1 at the first
> > + *     bit. Depending on which antennas are selected in the bitmap, 802.11n
> > + *     drivers can derive which chainmasks to use (if all antennas belonging to
> > + *     a particular chain are disabled this chain should be disabled) and if
> > + *     a chain has diversity antennas wether diversity should be used or not.
> > + *     HT capabilities (STBC, TX Beamforming, Antenna selection) can be
> > + *     derived from the available chains after applying the antenna mask.
> 
> I don't want to do any work myself on drivers for this, can we have
> cfg80211/mac80211 do this for us?

Also, this is a serious issue when you want to support this call -- it
may not be done while you're associated since you cannot change those
things properly then.

johannes

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Bruno Randolf]

On Thu July 29 2010 15:12:29 you wrote:
> On Wed, Jul 28, 2010 at 8:58 PM, Bruno Randolf <br1@einfach.org> wrote:
> > diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> > index 2c87016..8a88921 100644
> > --- a/include/linux/nl80211.h
> > +++ b/include/linux/nl80211.h
> > @@ -731,6 +731,28 @@ 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. + *     This can be used to mask out antennas which are
> > not attached or should + *     not be used for transmitting. If an
> > antenna is not selected in this + *     bitmap the hardware is not
> > allowed to transmit on this antenna. + *
> > + *     Each bit represents one antenna, starting with antenna 1 at the
> > first + *     bit. Depending on which antennas are selected in the
> > bitmap, 802.11n + *     drivers can derive which chainmasks to use (if
> > all antennas belonging to + *     a particular chain are disabled this
> > chain should be disabled) and if + *     a chain has diversity antennas
> > wether diversity should be used or not. + *     HT capabilities (STBC,
> > TX Beamforming, Antenna selection) can be + *     derived from the
> > available chains after applying the antenna mask.
> 
> I don't want to do any work myself on drivers for this, can we have
> cfg80211/mac80211 do this for us?

is this not a separate issue from defining the API? you could have it do this 
for you even now, with or without the antenna API, no?

i think this should be dealt with seperately. for now let's just define an API 
for an antenna mask.

bruno

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Thu, Jul 29, 2010 at 2:12 AM, Bruno Randolf <br1@einfach.org> wrote:
> On Thu July 29 2010 15:12:29 you wrote:
>> On Wed, Jul 28, 2010 at 8:58 PM, Bruno Randolf <br1@einfach.org> wrote:
>> > diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
>> > index 2c87016..8a88921 100644
>> > --- a/include/linux/nl80211.h
>> > +++ b/include/linux/nl80211.h
>> > @@ -731,6 +731,28 @@ 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. + *     This can be used to mask out antennas which are
>> > not attached or should + *     not be used for transmitting. If an
>> > antenna is not selected in this + *     bitmap the hardware is not
>> > allowed to transmit on this antenna. + *
>> > + *     Each bit represents one antenna, starting with antenna 1 at the
>> > first + *     bit. Depending on which antennas are selected in the
>> > bitmap, 802.11n + *     drivers can derive which chainmasks to use (if
>> > all antennas belonging to + *     a particular chain are disabled this
>> > chain should be disabled) and if + *     a chain has diversity antennas
>> > wether diversity should be used or not. + *     HT capabilities (STBC,
>> > TX Beamforming, Antenna selection) can be + *     derived from the
>> > available chains after applying the antenna mask.
>>
>> I don't want to do any work myself on drivers for this, can we have
>> cfg80211/mac80211 do this for us?
>
> is this not a separate issue from defining the API? you could have it do this
> for you even now, with or without the antenna API, no?

It depends on the consumer and at what level we think the consumer
should be doing whatever checks or changes it should. For starters I
would expect at least a check for association within cfg80211 which
disallows hw config changes. Then, we would need to address changes on
the hw config, and caching the original values and determine whether
or not we want to differentiate them on userspace to make it clear to
the user that the hw config is just tweaked right now but the real
capability is different than the current setting.

> i think this should be dealt with seperately. for now let's just define an API
> for an antenna mask.

This needs to be dealt with in cfg80211 if we want to allow cfg80211
drivers to use this and can deal with sanity checks in cfg80211. If
mac80211 and other cfg80211 drivers will be the consumers then the
change would be in mac80211 but a separate patch would be required.

  Luis

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Bruno Randolf]

On Fri July 30 2010 00:04:49 Luis R. Rodriguez wrote:
> >> I don't want to do any work myself on drivers for this, can we have
> >> cfg80211/mac80211 do this for us?
> > 
> > is this not a separate issue from defining the API? you could have it do
> > this for you even now, with or without the antenna API, no?
> 
> It depends on the consumer and at what level we think the consumer
> should be doing whatever checks or changes it should. For starters I
> would expect at least a check for association within cfg80211 which
> disallows hw config changes. 

for ath5k at least, it's o.k. to change antenna configuration during 
association.

if 802.11n devices cannot change antenna setup during association, the driver 
could simply reject the configuration. but i can see that it can make sense to 
handle some of this in cfg80211 or mac80211. it should be possible to add that 
on top of my patches.

i think you are expecting to much. for now i just want to define an API which 
1) can be used right now and 2) is flexible enough to support 802.11n devices. 
as i don't work with 802.11n devices right now, please don't expect me to 
solve all 802.11n related antenna problems. as long as the API is flexible 
enough i think it can be added later as 802.11n devices are going to accept 
antenna configuration.

bruno

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Sun, Aug 1, 2010 at 9:13 PM, Bruno Randolf <br1@einfach.org> wrote:
> On Fri July 30 2010 00:04:49 Luis R. Rodriguez wrote:
>> >> I don't want to do any work myself on drivers for this, can we have
>> >> cfg80211/mac80211 do this for us?
>> >
>> > is this not a separate issue from defining the API? you could have it do
>> > this for you even now, with or without the antenna API, no?
>>
>> It depends on the consumer and at what level we think the consumer
>> should be doing whatever checks or changes it should. For starters I
>> would expect at least a check for association within cfg80211 which
>> disallows hw config changes.
>
> for ath5k at least, it's o.k. to change antenna configuration during
> association.

Then do it through debugfs.

> if 802.11n devices cannot change antenna setup during association, the driver
> could simply reject the configuration. but i can see that it can make sense to
> handle some of this in cfg80211 or mac80211. it should be possible to add that
> on top of my patches.

I would expect the respective changes within your patches, not after.

> i think you are expecting to much.

I refuse to accept patches which can potentially create bogus bug
reports or loose crap implementations in drivers. Wext had a lot of
this and I am glad we haven't yet allowed for this crap with nl80211.
To allow your patch to go through as-is without addressing all the
points made would be enabling driver developers to get real sloppy.

> for now i just want to define an API which
> 1) can be used right now and 2) is flexible enough to support 802.11n devices.

Use debugfs or consider a simple API for legacy for nl80211.

> as i don't work with 802.11n devices right now, please don't expect me to
> solve all 802.11n related antenna problems.

I'm not, the point I was trying to make is that solving this for
legacy and that for 802.11n this needs more thought and work.

> as long as the API is flexible
> enough i think it can be added later as 802.11n devices are going to accept
> antenna configuration.

If you want really flexible stuff without addressing serious
considerations before introducing a new kernel API then use debugfs.

  Luis

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Felix Fietkau]

On 2010-08-02 7:37 AM, Luis R. Rodriguez wrote:
> I'm not, the point I was trying to make is that solving this for
> legacy and that for 802.11n this needs more thought and work.
I need this for 802.11n as well, and I believe the API is good enough
for that.

>> as long as the API is flexible
>> enough i think it can be added later as 802.11n devices are going to accept
>> antenna configuration.
> 
> If you want really flexible stuff without addressing serious
> considerations before introducing a new kernel API then use debugfs.
I'd like to see this in nl80211. Is there any specific concern left that
I haven't addressed already? If so, please point me at it...

- Felix

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Mon, Aug 2, 2010 at 1:59 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2010-08-02 7:37 AM, Luis R. Rodriguez wrote:
>> I'm not, the point I was trying to make is that solving this for
>> legacy and that for 802.11n this needs more thought and work.
> I need this for 802.11n as well, and I believe the API is good enough
> for that.

Sure.

>>> as long as the API is flexible
>>> enough i think it can be added later as 802.11n devices are going to accept
>>> antenna configuration.
>>
>> If you want really flexible stuff without addressing serious
>> considerations before introducing a new kernel API then use debugfs.
> I'd like to see this in nl80211. Is there any specific concern left that
> I haven't addressed already? If so, please point me at it...

The code that deals with not accepting changes unless we're not
associated, and that also caches the old real hw config vs the
modified one.

  Luis

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Felix Fietkau]

On 2010-08-02 11:10 AM, Luis R. Rodriguez wrote:
> On Mon, Aug 2, 2010 at 1:59 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>> On 2010-08-02 7:37 AM, Luis R. Rodriguez wrote:
>>> I'm not, the point I was trying to make is that solving this for
>>> legacy and that for 802.11n this needs more thought and work.
>> I need this for 802.11n as well, and I believe the API is good enough
>> for that.
> 
> Sure.
> 
>>>> as long as the API is flexible
>>>> enough i think it can be added later as 802.11n devices are going to accept
>>>> antenna configuration.
>>>
>>> If you want really flexible stuff without addressing serious
>>> considerations before introducing a new kernel API then use debugfs.
>> I'd like to see this in nl80211. Is there any specific concern left that
>> I haven't addressed already? If so, please point me at it...
> 
> The code that deals with not accepting changes unless we're not
> associated, and that also caches the old real hw config vs the
> modified one.
This is not really an API issue, this is more of an implementation
aspect. I think it's up to the driver to reject changes that it cannot
handle at the moment.
For legacy, changing settings is fine in any state (even with assoc).
For 802.11n, changes that affect the chainmask should be rejected while
the interface is up. That way we don't run into weird cached config vs
hw config issues, and also keep a sane state for the HT capabilities.

- Felix

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Mon, Aug 2, 2010 at 2:17 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2010-08-02 11:10 AM, Luis R. Rodriguez wrote:
>> On Mon, Aug 2, 2010 at 1:59 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>> On 2010-08-02 7:37 AM, Luis R. Rodriguez wrote:
>>>> I'm not, the point I was trying to make is that solving this for
>>>> legacy and that for 802.11n this needs more thought and work.
>>> I need this for 802.11n as well, and I believe the API is good enough
>>> for that.
>>
>> Sure.
>>
>>>>> as long as the API is flexible
>>>>> enough i think it can be added later as 802.11n devices are going to accept
>>>>> antenna configuration.
>>>>
>>>> If you want really flexible stuff without addressing serious
>>>> considerations before introducing a new kernel API then use debugfs.
>>> I'd like to see this in nl80211. Is there any specific concern left that
>>> I haven't addressed already? If so, please point me at it...
>>
>> The code that deals with not accepting changes unless we're not
>> associated, and that also caches the old real hw config vs the
>> modified one.
> This is not really an API issue, this is more of an implementation
> aspect. I think it's up to the driver to reject changes that it cannot
> handle at the moment.

Huh,why not deal with this on cfg80211 and/or mac80211?

> For legacy, changing settings is fine in any state (even with assoc).

Sure.

> For 802.11n, changes that affect the chainmask should be rejected while
> the interface is up. That way we don't run into weird cached config vs
> hw config issues, and also keep a sane state for the HT capabilities.

Sure, I just did not see any code for this in these patches. My point
about the hw config vs fake/mod'd is if we'd expose the mod'd config
changes to userspace or if we'd keep them internal to cfg80211. How
would this be dealt with?

  Luis

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Felix Fietkau]

On 2010-08-02 11:23 AM, Luis R. Rodriguez wrote:
> On Mon, Aug 2, 2010 at 2:17 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>> On 2010-08-02 11:10 AM, Luis R. Rodriguez wrote:
>>> On Mon, Aug 2, 2010 at 1:59 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>>> On 2010-08-02 7:37 AM, Luis R. Rodriguez wrote:
>>>>> I'm not, the point I was trying to make is that solving this for
>>>>> legacy and that for 802.11n this needs more thought and work.
>>>> I need this for 802.11n as well, and I believe the API is good enough
>>>> for that.
>>>
>>> Sure.
>>>
>>>>>> as long as the API is flexible
>>>>>> enough i think it can be added later as 802.11n devices are going to accept
>>>>>> antenna configuration.
>>>>>
>>>>> If you want really flexible stuff without addressing serious
>>>>> considerations before introducing a new kernel API then use debugfs.
>>>> I'd like to see this in nl80211. Is there any specific concern left that
>>>> I haven't addressed already? If so, please point me at it...
>>>
>>> The code that deals with not accepting changes unless we're not
>>> associated, and that also caches the old real hw config vs the
>>> modified one.
>> This is not really an API issue, this is more of an implementation
>> aspect. I think it's up to the driver to reject changes that it cannot
>> handle at the moment.
> 
> Huh,why not deal with this on cfg80211 and/or mac80211?
Right now, the drivers calculate this. We could make a helper function
in cfg80211 at some point, but we're talking about really small chunks
of code.

>> For 802.11n, changes that affect the chainmask should be rejected while
>> the interface is up. That way we don't run into weird cached config vs
>> hw config issues, and also keep a sane state for the HT capabilities.
> 
> Sure, I just did not see any code for this in these patches. My point
> about the hw config vs fake/mod'd is if we'd expose the mod'd config
> changes to userspace or if we'd keep them internal to cfg80211. How
> would this be dealt with?
Right now, cfg80211 doesn't know enough to handle this stuff on its own,
so let's handle it in the driver completely on the first iteration. The
patches do not need any changes for this right now.

- Felix

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Mon, Aug 2, 2010 at 2:32 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2010-08-02 11:23 AM, Luis R. Rodriguez wrote:
>> On Mon, Aug 2, 2010 at 2:17 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>> On 2010-08-02 11:10 AM, Luis R. Rodriguez wrote:
>>>> On Mon, Aug 2, 2010 at 1:59 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>>>> On 2010-08-02 7:37 AM, Luis R. Rodriguez wrote:
>>>>>> I'm not, the point I was trying to make is that solving this for
>>>>>> legacy and that for 802.11n this needs more thought and work.
>>>>> I need this for 802.11n as well, and I believe the API is good enough
>>>>> for that.
>>>>
>>>> Sure.
>>>>
>>>>>>> as long as the API is flexible
>>>>>>> enough i think it can be added later as 802.11n devices are going to accept
>>>>>>> antenna configuration.
>>>>>>
>>>>>> If you want really flexible stuff without addressing serious
>>>>>> considerations before introducing a new kernel API then use debugfs.
>>>>> I'd like to see this in nl80211. Is there any specific concern left that
>>>>> I haven't addressed already? If so, please point me at it...
>>>>
>>>> The code that deals with not accepting changes unless we're not
>>>> associated, and that also caches the old real hw config vs the
>>>> modified one.
>>> This is not really an API issue, this is more of an implementation
>>> aspect. I think it's up to the driver to reject changes that it cannot
>>> handle at the moment.
>>
>> Huh,why not deal with this on cfg80211 and/or mac80211?
> Right now, the drivers calculate this. We could make a helper function
> in cfg80211 at some point, but we're talking about really small chunks
> of code.
>
>>> For 802.11n, changes that affect the chainmask should be rejected while
>>> the interface is up. That way we don't run into weird cached config vs
>>> hw config issues, and also keep a sane state for the HT capabilities.
>>
>> Sure, I just did not see any code for this in these patches. My point
>> about the hw config vs fake/mod'd is if we'd expose the mod'd config
>> changes to userspace or if we'd keep them internal to cfg80211. How
>> would this be dealt with?
> Right now, cfg80211 doesn't know enough to handle this stuff on its own,
> so let's handle it in the driver completely on the first iteration. The
> patches do not need any changes for this right now.

I'd prefer that code to be written rather then let this be defined as
API now and let drivers deal with this differently. But that's me, I'm
not the maintainer, I just will not deal with bug reports dealing with
this and I'll assign them to you guys if this gets through. Still
think its crap and should just go through debugfs until all the code
mentioned does exist.

  Luis

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Felix Fietkau]

On 2010-08-02 11:52 AM, Luis R. Rodriguez wrote:
> On Mon, Aug 2, 2010 at 2:32 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>> On 2010-08-02 11:23 AM, Luis R. Rodriguez wrote:
>>> Sure, I just did not see any code for this in these patches. My point
>>> about the hw config vs fake/mod'd is if we'd expose the mod'd config
>>> changes to userspace or if we'd keep them internal to cfg80211. How
>>> would this be dealt with?
>> Right now, cfg80211 doesn't know enough to handle this stuff on its own,
>> so let's handle it in the driver completely on the first iteration. The
>> patches do not need any changes for this right now.
> 
> I'd prefer that code to be written rather then let this be defined as
> API now and let drivers deal with this differently. But that's me, I'm
> not the maintainer, I just will not deal with bug reports dealing with
> this and I'll assign them to you guys if this gets through. Still
> think its crap and should just go through debugfs until all the code
> mentioned does exist.
Sorry, but WTF? There's two parts to this: API visible to user space,
and the internal API for handling changes.
So you're suggesting to reject the user space API, because of missing
parts in the internal API (which we can change any time) that will only
be used for drivers that this series doesn't even contain any code for??
Am I confused here, or does this seem rather strange?

- Felix

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Mon, Aug 2, 2010 at 3:10 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2010-08-02 11:52 AM, Luis R. Rodriguez wrote:
>> On Mon, Aug 2, 2010 at 2:32 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>> On 2010-08-02 11:23 AM, Luis R. Rodriguez wrote:
>>>> Sure, I just did not see any code for this in these patches. My point
>>>> about the hw config vs fake/mod'd is if we'd expose the mod'd config
>>>> changes to userspace or if we'd keep them internal to cfg80211. How
>>>> would this be dealt with?
>>> Right now, cfg80211 doesn't know enough to handle this stuff on its own,
>>> so let's handle it in the driver completely on the first iteration. The
>>> patches do not need any changes for this right now.
>>
>> I'd prefer that code to be written rather then let this be defined as
>> API now and let drivers deal with this differently. But that's me, I'm
>> not the maintainer, I just will not deal with bug reports dealing with
>> this and I'll assign them to you guys if this gets through. Still
>> think its crap and should just go through debugfs until all the code
>> mentioned does exist.
> Sorry, but WTF? There's two parts to this: API visible to user space,
> and the internal API for handling changes.
> So you're suggesting to reject the user space API, because of missing
> parts in the internal API (which we can change any time) that will only
> be used for drivers that this series doesn't even contain any code for??
> Am I confused here, or does this seem rather strange?

If the current patches are accepted it means anyone *can* submit
patches for an 802.11n driver and expect it to be accepted. Hence why
I was asking for this to be defined as a legacy API only, if that is
the only purpose right now.

  Luis

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Felix Fietkau]

On 2010-08-02 12:18 PM, Luis R. Rodriguez wrote:
> On Mon, Aug 2, 2010 at 3:10 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>> On 2010-08-02 11:52 AM, Luis R. Rodriguez wrote:
>>> On Mon, Aug 2, 2010 at 2:32 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>>> On 2010-08-02 11:23 AM, Luis R. Rodriguez wrote:
>>>>> Sure, I just did not see any code for this in these patches. My point
>>>>> about the hw config vs fake/mod'd is if we'd expose the mod'd config
>>>>> changes to userspace or if we'd keep them internal to cfg80211. How
>>>>> would this be dealt with?
>>>> Right now, cfg80211 doesn't know enough to handle this stuff on its own,
>>>> so let's handle it in the driver completely on the first iteration. The
>>>> patches do not need any changes for this right now.
>>>
>>> I'd prefer that code to be written rather then let this be defined as
>>> API now and let drivers deal with this differently. But that's me, I'm
>>> not the maintainer, I just will not deal with bug reports dealing with
>>> this and I'll assign them to you guys if this gets through. Still
>>> think its crap and should just go through debugfs until all the code
>>> mentioned does exist.
>> Sorry, but WTF? There's two parts to this: API visible to user space,
>> and the internal API for handling changes.
>> So you're suggesting to reject the user space API, because of missing
>> parts in the internal API (which we can change any time) that will only
>> be used for drivers that this series doesn't even contain any code for??
>> Am I confused here, or does this seem rather strange?
> 
> If the current patches are accepted it means anyone *can* submit
> patches for an 802.11n driver and expect it to be accepted. Hence why
> I was asking for this to be defined as a legacy API only, if that is
> the only purpose right now.
This discussion is getting quite redundant. I would like to ask you
again, what the specific concern purely from a user space API point of
view is.

You asked how run time changes should be handled, I made a proposal for
that (simply not accepting them at run time on 802.11n hardware if they
affect the chainmask), and then you started complaining about what
should be in cfg80211 and what should be in the drivers. What does that
have to do with the public API?

The limitations wrt. chainmask changes at run time are known, and they
aren't really hardware specific, so you won't see some random driver
suddenly implementing some hypothetical crazy scheme that nobody
expects. This has nothing to do with code being in cfg80211 vs code
being in the driver.

It's really simple: in AP mode, hostapd needs to have the HT stuff set
when it starts up, in STA mode, it needs to be set at least at assoc
time. Since this is affected by the chainmask, we can't just change
stuff at a random point in time, but this is expected.

So the lowest common denominator that we can use for all 802.11n
hardware in all modes is to just refuse changes unless the interface is
down. Then we use the antenna mask to calculate the chainmask (an
inherently driver specific aspect, at least at the moment), recalculate
the HT capabilities based on the chainmask (driver has to do this at
init time already, needs very little refactoring), and then just apply
the new settings to the hardware (again, driver specific, will probably
be deferred to the interface bringup sequence).

So aside from the meaning of the actual value, which we need to make
easier to understand in the user space code, the only thing that users
need to be aware of is this:

Legacy hardware  => run time changes possible (actually optional)
802.11n hardware => need to bring down the interface before changing
                    the antenna settings

What else is left that we need to figure out? What would we need the
temporary debugfs crap for?

- Felix

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Mon, Aug 2, 2010 at 3:47 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> 802.11n hardware => need to bring down the interface before changing
>                    the antenna settings

The current patch does not deal with the 802.11n case you mentioned.

  Luis

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Johannes Berg]

On Mon, 2010-08-02 at 12:47 +0200, Felix Fietkau wrote:

> Legacy hardware  => run time changes possible (actually optional)
> 802.11n hardware => need to bring down the interface before changing
>                     the antenna settings

I think I'd prefer if all hw just required the device to be stopped.
Otherwise things either become awkward when you have, say. software
diversity control, or they become unpredictable quite, no?

johannes

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Felix Fietkau]

On 2010-08-02 12:51 PM, Luis R. Rodriguez wrote:
> On Mon, Aug 2, 2010 at 3:47 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>> 802.11n hardware => need to bring down the interface before changing
>>                    the antenna settings
> 
> The current patch does not deal with the 802.11n case you mentioned.
And why should it? The series does not add antenna control support to
any specific 802.11n driver.

- Felix

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Felix Fietkau]

On 2010-08-02 1:01 PM, Johannes Berg wrote:
> On Mon, 2010-08-02 at 12:47 +0200, Felix Fietkau wrote:
> 
>> Legacy hardware  => run time changes possible (actually optional)
>> 802.11n hardware => need to bring down the interface before changing
>>                     the antenna settings
> 
> I think I'd prefer if all hw just required the device to be stopped.
> Otherwise things either become awkward when you have, say. software
> diversity control, or they become unpredictable quite, no?
If the driver does something like that, then the antenna control
callback should reset that state as well. But then again, I don't mind
if legacy hw requires the interface to be down as well.

- Felix

Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration

[Luis R. Rodriguez]

On Mon, Aug 2, 2010 at 4:18 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2010-08-02 12:51 PM, Luis R. Rodriguez wrote:
>> On Mon, Aug 2, 2010 at 3:47 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>> 802.11n hardware => need to bring down the interface before changing
>>>                    the antenna settings
>>
>> The current patch does not deal with the 802.11n case you mentioned.
> And why should it? The series does not add antenna control support to
> any specific 802.11n driver.

Because it documents support for 802.11n. You should not expect
someone writing their ops for an 802.11n driver to fill in the 802.11n
gaps for you unless you mark your API as being only for legacy.

  Luis