2008-10-07 16:58:01

by Johannes Berg

[permalink] [raw]
Subject: mac80211 and IEEE80211_CONF_SHORT_SLOT_TIME

Hi,

I just realised that for whatever reason mac80211 isn't setting the
IEEE80211_CONF_SHORT_SLOT_TIME flag. I'm not sure when or why that was
removed, but clearly it isn't used.

Can you please fix the drivers using it (b43, b43legacy, iwlwifi, p54,
rt2x00, rtl8180, rtl8187) to use the bss_conf's use_short_slot instead?
I'll fix mac80211 to set that appropriately.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-10-07 18:03:09

by Johannes Berg

[permalink] [raw]
Subject: Re: mac80211 and IEEE80211_CONF_SHORT_SLOT_TIME

On Tue, 2008-10-07 at 19:57 +0200, Ivo van Doorn wrote:
> Hi,
>
> > I just realised that for whatever reason mac80211 isn't setting the
> > IEEE80211_CONF_SHORT_SLOT_TIME flag. I'm not sure when or why that was
> > removed, but clearly it isn't used.
> >
> > Can you please fix the drivers using it (b43, b43legacy, iwlwifi, p54,
> > rt2x00, rtl8180, rtl8187) to use the bss_conf's use_short_slot instead?
> > I'll fix mac80211 to set that appropriately.
>
> Didn't you have such a patch for rt2x00 already?

I had something a long time ago, but it never worked out right. I still
have it in my patches dir as *-BROKEN-mac80211-bss-slot-time.patch.

> If not I'll see what I can do in a few days.

Thanks,
Johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part
Subject: Re: mac80211 and IEEE80211_CONF_SHORT_SLOT_TIME

On Tuesday 07 October 2008 22:14:12 Herton Ronaldo Krzesinski wrote:
> On Tuesday 07 October 2008 06:05:24 Johannes Berg wrote:
> > Hi,
> >
> > I just realised that for whatever reason mac80211 isn't setting the
> > IEEE80211_CONF_SHORT_SLOT_TIME flag. I'm not sure when or why that was
> > removed, but clearly it isn't used.
> >
> > Can you please fix the drivers using it (b43, b43legacy, iwlwifi, p54,
> > rt2x00, rtl8180, rtl8187) to use the bss_conf's use_short_slot instead?
> > I'll fix mac80211 to set that appropriately.
>
> About 8187 and 8187b, I think the changes below may handle it, I have yet
> to test them, just posting to have some feedback in case there is something
> missing or wrong. I'll try to review the other things too (like
> use_short_preamble warned in the other thread):
>
> diff --git a/drivers/net/wireless/rtl8187_dev.c
> b/drivers/net/wireless/rtl8187_dev.c
> index e990261..7f1cac8 100644
> --- a/drivers/net/wireless/rtl8187_dev.c
> +++ b/drivers/net/wireless/rtl8187_dev.c
> @@ -870,6 +870,32 @@ static void rtl8187_remove_interface(struct
> ieee80211_hw *dev,
> mutex_unlock(&priv->conf_mutex);
> }
>
> +static void rtl8187_conf_slot(struct rtl8187_priv *priv, bool
> use_short_slot) +{
> + if (priv->is_rtl8187b) {
> + if (use_short_slot) {
> + rtl818x_iowrite8(priv, &priv->map->SLOT, 0x9);
> + rtl818x_iowrite8(priv, &priv->map->DIFS, 0x1c);
> + } else {
> + rtl818x_iowrite8(priv, &priv->map->SLOT, 0x14);
> + rtl818x_iowrite8(priv, &priv->map->DIFS, 0x32);
> + }
> + rtl818x_iowrite8(priv, &priv->map->EIFS, 0x5b);
> + } else {
> + if (use_short_slot) {
> + rtl818x_iowrite8(priv, &priv->map->SLOT, 0x9);
> + rtl818x_iowrite8(priv, &priv->map->DIFS, 0x14);
> + rtl818x_iowrite8(priv, &priv->map->EIFS, 91 - 0x14);
> + rtl818x_iowrite8(priv, &priv->map->CW_VAL, 0x73);
> + } else {
> + rtl818x_iowrite8(priv, &priv->map->SLOT, 0x14);
> + rtl818x_iowrite8(priv, &priv->map->DIFS, 0x24);
> + rtl818x_iowrite8(priv, &priv->map->EIFS, 91 - 0x24);
> + rtl818x_iowrite8(priv, &priv->map->CW_VAL, 0xa5);
> + }
> + }
> +}
> +
> static int rtl8187_config(struct ieee80211_hw *dev, struct ieee80211_conf
> *conf)
> {
> struct rtl8187_priv *priv = dev->priv;
> @@ -888,21 +914,8 @@ static int rtl8187_config(struct ieee80211_hw *dev,
> struct ieee80211_conf *conf)
> msleep(10);
> rtl818x_iowrite32(priv, &priv->map->TX_CONF, reg);
>
> - if (!priv->is_rtl8187b) {
> - rtl818x_iowrite8(priv, &priv->map->SIFS, 0x22);
> -
> - if (conf->flags & IEEE80211_CONF_SHORT_SLOT_TIME) {
> - rtl818x_iowrite8(priv, &priv->map->SLOT, 0x9);
> - rtl818x_iowrite8(priv, &priv->map->DIFS, 0x14);
> - rtl818x_iowrite8(priv, &priv->map->EIFS, 91 - 0x14);
> - rtl818x_iowrite8(priv, &priv->map->CW_VAL, 0x73);
> - } else {
> - rtl818x_iowrite8(priv, &priv->map->SLOT, 0x14);
> - rtl818x_iowrite8(priv, &priv->map->DIFS, 0x24);
> - rtl818x_iowrite8(priv, &priv->map->EIFS, 91 - 0x24);
> - rtl818x_iowrite8(priv, &priv->map->CW_VAL, 0xa5);
> - }
> - }
> + rtl818x_iowrite8(priv, &priv->map->SIFS, 0x22);
> + rtl8187_conf_slot(priv, conf->flags & IEEE80211_CONF_SHORT_SLOT_TIME);

grrr, just ignore these two additions that shouldn't be there (I'll move SIFS
to initialization code or conf_slot functions), and the second line is wrong
of course.

>
> rtl818x_iowrite16(priv, &priv->map->ATIM_WND, 2);
> rtl818x_iowrite16(priv, &priv->map->ATIMTR_INTERVAL, 100);
> @@ -938,6 +951,15 @@ static int rtl8187_config_interface(struct
> ieee80211_hw *dev,
> return 0;
> }
>
> +static void rtl8187_bss_info_changed(struct ieee80211_hw *dev,
> + struct ieee80211_vif *vif,
> + struct ieee80211_bss_conf *info,
> + u32 changed)
> +{
> + if (changed & BSS_CHANGED_ERP_SLOT)
> + rtl8187_conf_slot(dev->priv, info->use_short_slot);
> +}
> +
> static void rtl8187_configure_filter(struct ieee80211_hw *dev,
> unsigned int changed_flags,
> unsigned int *total_flags,
> @@ -978,6 +1000,7 @@ static const struct ieee80211_ops rtl8187_ops = {
> .remove_interface = rtl8187_remove_interface,
> .config = rtl8187_config,
> .config_interface = rtl8187_config_interface,
> + .bss_info_changed = rtl8187_bss_info_changed,
> .configure_filter = rtl8187_configure_filter,
> };
>
>
> The values used for SLOT etc. are kept for 8187, for 8187b I got the values
> from realtek GPL driver (nothing is specified in the datasheet).
>
> > johannes

--
[]'s
Herton

2008-10-08 08:45:03

by Johannes Berg

[permalink] [raw]
Subject: Re: mac80211 and IEEE80211_CONF_SHORT_SLOT_TIME

On Tue, 2008-10-07 at 22:14 -0300, Herton Ronaldo Krzesinski wrote:

> About 8187 and 8187b, I think the changes below may handle it, I have yet to
> test them, just posting to have some feedback in case there is something
> missing or wrong. I'll try to review the other things too (like
> use_short_preamble warned in the other thread):

Looks ok to me, except for the stuff you yourself pointed out.


> The values used for SLOT etc. are kept for 8187, for 8187b I got the values
> from realtek GPL driver (nothing is specified in the datasheet).

They seem to be very close to just the spec values in usecs, so maybe
double-check 802.11-2007, I don't know the values off the top of my
head.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part
Subject: Re: mac80211 and IEEE80211_CONF_SHORT_SLOT_TIME

On Tuesday 07 October 2008 06:05:24 Johannes Berg wrote:
> Hi,
>
> I just realised that for whatever reason mac80211 isn't setting the
> IEEE80211_CONF_SHORT_SLOT_TIME flag. I'm not sure when or why that was
> removed, but clearly it isn't used.
>
> Can you please fix the drivers using it (b43, b43legacy, iwlwifi, p54,
> rt2x00, rtl8180, rtl8187) to use the bss_conf's use_short_slot instead?
> I'll fix mac80211 to set that appropriately.

About 8187 and 8187b, I think the changes below may handle it, I have yet to
test them, just posting to have some feedback in case there is something
missing or wrong. I'll try to review the other things too (like
use_short_preamble warned in the other thread):

diff --git a/drivers/net/wireless/rtl8187_dev.c
b/drivers/net/wireless/rtl8187_dev.c
index e990261..7f1cac8 100644
--- a/drivers/net/wireless/rtl8187_dev.c
+++ b/drivers/net/wireless/rtl8187_dev.c
@@ -870,6 +870,32 @@ static void rtl8187_remove_interface(struct ieee80211_hw
*dev,
mutex_unlock(&priv->conf_mutex);
}

+static void rtl8187_conf_slot(struct rtl8187_priv *priv, bool use_short_slot)
+{
+ if (priv->is_rtl8187b) {
+ if (use_short_slot) {
+ rtl818x_iowrite8(priv, &priv->map->SLOT, 0x9);
+ rtl818x_iowrite8(priv, &priv->map->DIFS, 0x1c);
+ } else {
+ rtl818x_iowrite8(priv, &priv->map->SLOT, 0x14);
+ rtl818x_iowrite8(priv, &priv->map->DIFS, 0x32);
+ }
+ rtl818x_iowrite8(priv, &priv->map->EIFS, 0x5b);
+ } else {
+ if (use_short_slot) {
+ rtl818x_iowrite8(priv, &priv->map->SLOT, 0x9);
+ rtl818x_iowrite8(priv, &priv->map->DIFS, 0x14);
+ rtl818x_iowrite8(priv, &priv->map->EIFS, 91 - 0x14);
+ rtl818x_iowrite8(priv, &priv->map->CW_VAL, 0x73);
+ } else {
+ rtl818x_iowrite8(priv, &priv->map->SLOT, 0x14);
+ rtl818x_iowrite8(priv, &priv->map->DIFS, 0x24);
+ rtl818x_iowrite8(priv, &priv->map->EIFS, 91 - 0x24);
+ rtl818x_iowrite8(priv, &priv->map->CW_VAL, 0xa5);
+ }
+ }
+}
+
static int rtl8187_config(struct ieee80211_hw *dev, struct ieee80211_conf
*conf)
{
struct rtl8187_priv *priv = dev->priv;
@@ -888,21 +914,8 @@ static int rtl8187_config(struct ieee80211_hw *dev,
struct ieee80211_conf *conf)
msleep(10);
rtl818x_iowrite32(priv, &priv->map->TX_CONF, reg);

- if (!priv->is_rtl8187b) {
- rtl818x_iowrite8(priv, &priv->map->SIFS, 0x22);
-
- if (conf->flags & IEEE80211_CONF_SHORT_SLOT_TIME) {
- rtl818x_iowrite8(priv, &priv->map->SLOT, 0x9);
- rtl818x_iowrite8(priv, &priv->map->DIFS, 0x14);
- rtl818x_iowrite8(priv, &priv->map->EIFS, 91 - 0x14);
- rtl818x_iowrite8(priv, &priv->map->CW_VAL, 0x73);
- } else {
- rtl818x_iowrite8(priv, &priv->map->SLOT, 0x14);
- rtl818x_iowrite8(priv, &priv->map->DIFS, 0x24);
- rtl818x_iowrite8(priv, &priv->map->EIFS, 91 - 0x24);
- rtl818x_iowrite8(priv, &priv->map->CW_VAL, 0xa5);
- }
- }
+ rtl818x_iowrite8(priv, &priv->map->SIFS, 0x22);
+ rtl8187_conf_slot(priv, conf->flags & IEEE80211_CONF_SHORT_SLOT_TIME);

rtl818x_iowrite16(priv, &priv->map->ATIM_WND, 2);
rtl818x_iowrite16(priv, &priv->map->ATIMTR_INTERVAL, 100);
@@ -938,6 +951,15 @@ static int rtl8187_config_interface(struct ieee80211_hw
*dev,
return 0;
}

+static void rtl8187_bss_info_changed(struct ieee80211_hw *dev,
+ struct ieee80211_vif *vif,
+ struct ieee80211_bss_conf *info,
+ u32 changed)
+{
+ if (changed & BSS_CHANGED_ERP_SLOT)
+ rtl8187_conf_slot(dev->priv, info->use_short_slot);
+}
+
static void rtl8187_configure_filter(struct ieee80211_hw *dev,
unsigned int changed_flags,
unsigned int *total_flags,
@@ -978,6 +1000,7 @@ static const struct ieee80211_ops rtl8187_ops = {
.remove_interface = rtl8187_remove_interface,
.config = rtl8187_config,
.config_interface = rtl8187_config_interface,
+ .bss_info_changed = rtl8187_bss_info_changed,
.configure_filter = rtl8187_configure_filter,
};


The values used for SLOT etc. are kept for 8187, for 8187b I got the values
from realtek GPL driver (nothing is specified in the datasheet).

>
> johannes

--
[]'s
Herton

2008-10-07 17:58:00

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: mac80211 and IEEE80211_CONF_SHORT_SLOT_TIME

Hi,

> I just realised that for whatever reason mac80211 isn't setting the
> IEEE80211_CONF_SHORT_SLOT_TIME flag. I'm not sure when or why that was
> removed, but clearly it isn't used.
>
> Can you please fix the drivers using it (b43, b43legacy, iwlwifi, p54,
> rt2x00, rtl8180, rtl8187) to use the bss_conf's use_short_slot instead?
> I'll fix mac80211 to set that appropriately.

Didn't you have such a patch for rt2x00 already?
If not I'll see what I can do in a few days.

Ivo