Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [ath5k-devel] [PATCH] [ath5k][leds] Ability to disable leds support. If leds support enabled do not force mac802.11 leds layer selection.
From: Pavel Roskin @ 2010-05-12 15:48 UTC (permalink / raw)
  To: Dmytro Milinevskyy
  Cc: ath5k-devel, Kalle Valo, linux-wireless, GeunSik Lim, Jiri Slaby,
	Greg Kroah-Hartman, John W. Linville, Keng-Yu Lin, netdev,
	Jiri Kosina, Johannes Berg, Shahar Or, linux-kernel,
	Luca Verdesca
In-Reply-To: <4bea1f81.09b6660a.746e.1c12@mx.google.com>

On Wed, 2010-04-07 at 21:58 +0300, Dmytro Milinevskyy wrote:

> Here is the patch to disable ath5k leds support on build stage.
> However if the leds support was enabled do not force selection of 802.11 leds layer.

The idea is good, but the implementation could be improved.

There are too many preprocessor conditionals in your patch.

> +#ifdef CONFIG_ATH5K_LEDS
>  /*
>   * These match net80211 definitions (not used in
>   * mac80211).
> @@ -939,11 +940,7 @@ enum ath5k_power_mode {
>  #define AR5K_LED_AUTH	2 /*IEEE80211_S_AUTH*/
>  #define AR5K_LED_ASSOC	3 /*IEEE80211_S_ASSOC*/
>  #define AR5K_LED_RUN	4 /*IEEE80211_S_RUN*/

It should be OK to leave the constants defined even if they are not
used.

> +#ifdef CONFIG_ATH5K_LEDS
>  /* LED functions */
>  extern int ath5k_init_leds(struct ath5k_softc *sc);
>  extern void ath5k_led_enable(struct ath5k_softc *sc);
>  extern void ath5k_led_off(struct ath5k_softc *sc);
>  extern void ath5k_unregister_leds(struct ath5k_softc *sc);
> +#endif

You could add inline functions for the case when CONFIG_ATH5K_LEDS is
not defined.  That would avoid may conditionals in the code.

>  /* GPIO Functions */
> +#ifdef CONFIG_ATH5K_LEDS
>  extern void ath5k_hw_set_ledstate(struct ath5k_hw *ah, unsigned int state);
> +#endif

The same comment applies.

Also, there is nothing wrong with having an external declaration that is
not used in some particular configuration.

> +#ifdef CONFIG_ATH5K_LEDS
>  	/* turn on HW LEDs */
>  	ath5k_hw_set_ledstate(ah, AR5K_LED_INIT);
> +#endif

This is avoidable by having an inline ath5k_hw_set_ledstate() that does
nothing.

> +#ifdef CONFIG_ATH5K_LEDS
>  	struct ieee80211_hw *hw = pci_get_drvdata(to_pci_dev(dev));
>  	struct ath5k_softc *sc = hw->priv;
>  
>  	ath5k_led_off(sc);
> +#endif

Even this is avoidable if ath5k_led_off() does nothing.  gcc should be
smart enough to optimize out unneeded function calls.

> +#ifdef CONFIG_ATH5K_LEDS
>  /*
>   * State for LED triggers
>   */
>  struct ath5k_led
>  {
> +#ifdef CONFIG_LEDS_CLASS

I'm not sure this complexity is needed.  Are you going to support LEDs
if CONFIG_LEDS_CLASS is disabled?

> +#ifdef CONFIG_ATH5K_LEDS
>  	unsigned int		led_pin,	/* GPIO pin for driving LED */
>  				led_on;		/* pin setting for LED on */
> +#endif
>  
>  	struct tasklet_struct	restq;		/* reset tasklet */
>  
> @@ -164,7 +172,9 @@ struct ath5k_softc {
>  	spinlock_t		rxbuflock;
>  	u32			*rxlink;	/* link ptr in last RX desc */
>  	struct tasklet_struct	rxtq;		/* rx intr tasklet */
> +#ifdef CONFIG_ATH5K_LEDS
>  	struct ath5k_led	rx_led;		/* rx led */
> +#endif

You may want to group those fields together to make the code more
readable.

> --- a/drivers/net/wireless/ath/ath5k/led.c
> +++ b/drivers/net/wireless/ath/ath5k/led.c

I wonder if you could omit led.c completely in the Makefile.  If there
are some parts of led.c that are needed without CONFIG_ATH5K_LEDS, maybe
they belong elsewhere?

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* Re: [PATCH 2/2] compat-wireless: make patches apply again
From: Luis R. Rodriguez @ 2010-05-12 16:28 UTC (permalink / raw)
  To: Bruno Randolf; +Cc: Hauke Mehrtens, linux-wireless, mcgrof
In-Reply-To: <201005121720.50514.br1@einfach.org>

On Wed, May 12, 2010 at 1:20 AM, Bruno Randolf <br1@einfach.org> wrote:
> On Tuesday 11 May 2010 06:24:10 Hauke Mehrtens wrote:
>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
>> ---
>>  patches/07-change-default-rate-alg.patch |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/patches/07-change-default-rate-alg.patch
>> b/patches/07-change-default-rate-alg.patch index af65207..f0ccbce 100644
>> --- a/patches/07-change-default-rate-alg.patch
>> +++ b/patches/07-change-default-rate-alg.patch
>> @@ -21,7 +21,7 @@ at compilation time.
>>   module_param(ieee80211_default_rc_algo, charp, 0644);
>>   MODULE_PARM_DESC(ieee80211_default_rc_algo,
>>                "Default rate control algorithm for mac80211 to use");
>> -@@ -118,8 +118,8 @@ ieee80211_rate_control_ops_get(const cha
>> +@@ -119,8 +119,8 @@ ieee80211_rate_control_ops_get(const cha
>>               ops =
> ieee80211_try_rate_control_ops_get(ieee80211_default_rc_algo);
>>
>>       /* try built-in one if specific alg requested but not found */
>> @@ -29,6 +29,6 @@ at compilation time.
>>  -            ops =
> ieee80211_try_rate_control_ops_get(CONFIG_MAC80211_RC_DEFAULT);
>>  +    if (!ops && strlen(CONFIG_COMPAT_MAC80211_RC_DEFAULT))
>>  +            ops =
>> ieee80211_try_rate_control_ops_get(CONFIG_COMPAT_MAC80211_RC_DEFAULT); +
>>       kparam_unblock_sysfs_write(ieee80211_default_rc_algo);
>>
>>       return ops;
>> - }
>
> this is somehow messed up and doesn't apply. here is a fixed version of the
> patch (not a patch for a patch)...

Odd, it applied, here with git am and all. Hm, oh well.

  Luis

^ permalink raw reply

* Re: Packet injection with ath9k
From: Roberto Riggio @ 2010-05-12 16:42 UTC (permalink / raw)
  To: Gábor Stefanik; +Cc: linux-wireless
In-Reply-To: <r2y69e28c911005091324sbff07538i28babb432e6751db@mail.gmail.com>

On 05/09/2010 10:24 PM, Gábor Stefanik wrote:
>>     u_int16_t       it_len;
>>     u_int32_t       it_present;
>>      
> AFAIK these 2 fields need to be little-endian even on big-endian
> machines. If your system is big-endian, this can cause problems.
>    

Thanks, that was indeed the issue. I totally missed this info
in the radiotap website. Everything is working fine now.

R.

^ permalink raw reply

* Re: Packet injection with ath9k
From: Johannes Berg @ 2010-05-12 16:46 UTC (permalink / raw)
  To: Roberto Riggio; +Cc: Gábor Stefanik, linux-wireless
In-Reply-To: <4BEADA79.5050000@create-net.org>

On Wed, 2010-05-12 at 18:42 +0200, Roberto Riggio wrote:
> On 05/09/2010 10:24 PM, Gábor Stefanik wrote:
> >>     u_int16_t       it_len;
> >>     u_int32_t       it_present;
> >>      
> > AFAIK these 2 fields need to be little-endian even on big-endian
> > machines. If your system is big-endian, this can cause problems.
> >    
> 
> Thanks, that was indeed the issue. I totally missed this info
> in the radiotap website. Everything is working fine now.

I'd make it more prominent, but ... this is what it says now:

Important Radiotap Characteristics

      * Fields are strictly ordered; The developer can specify any
        combination of fields, but the data must appear following the
        radiotap header in the order they are specified in the
        it_present bitmask (or more accurately, in the order the bit
        numbers for the it_present bitmask are defined).
      * Data is specified in little endian byte-order, all data fields
        including the it_version, it_len and it_present fields in the
        radiotap header are to be specified in little endian byte-order.
        This wiki has adopted the Linux convention of using __le64,
        __le32 and __le16 for 64-, 32- and 16-bit little endian
        quantities.

johannes


^ permalink raw reply

* Re: [PATCH 2/9] rt2x00: Fix beacon descriptor writing for rt61pci.
From: Ivo Van Doorn @ 2010-05-12 17:43 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <AANLkTilDv-tY_UIbc_lh8EaNdV7MRNAbDA4Oe5nFiDNh@mail.gmail.com>

On Wed, May 12, 2010 at 11:46 AM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> On Wed, May 12, 2010 at 8:59 AM, Ivo Van Doorn <ivdoorn@gmail.com> wrote:
>> On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
>> <gwingerde@gmail.com> wrote:
>>> The buffer address descriptor word is not part of the TXINFO structure
>>> needed for beacons. The current writing of that word for beacons is
>>> therefore an out-of-bounds write.
>>> Fix this by only writing the buffer address descriptor word for TX
>>> queues.
>>>
>>> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
>>> ---
>>>  drivers/net/wireless/rt2x00/rt61pci.c |   10 +++++-----
>>>  1 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
>>> index 2436363..99c2981 100644
>>> --- a/drivers/net/wireless/rt2x00/rt61pci.c
>>> +++ b/drivers/net/wireless/rt2x00/rt61pci.c
>>> @@ -1801,12 +1801,12 @@ static void rt61pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
>>>        rt2x00_set_field32(&word, TXD_W5_WAITING_DMA_DONE_INT, 1);
>>>        rt2x00_desc_write(txd, 5, word);
>>>
>>> -       rt2x00_desc_read(txd, 6, &word);
>>> -       rt2x00_set_field32(&word, TXD_W6_BUFFER_PHYSICAL_ADDRESS,
>>> -                          skbdesc->skb_dma);
>>> -       rt2x00_desc_write(txd, 6, word);
>>> +       if (txdesc->queue != QID_BEACON) {
>>> +               rt2x00_desc_read(txd, 6, &word);
>>> +               rt2x00_set_field32(&word, TXD_W6_BUFFER_PHYSICAL_ADDRESS,
>>> +                                  skbdesc->skb_dma);
>>> +               rt2x00_desc_write(txd, 6, word);
>>>
>>> -       if (skbdesc->desc_len > TXINFO_SIZE) {
>>>                rt2x00_desc_read(txd, 11, &word);
>>>                rt2x00_set_field32(&word, TXD_W11_BUFFER_LENGTH0,
>>>                                   txdesc->length);
>>
>> Shouldn't the check for TXINFO_SIZE be used rather than explicitly
>> checking for the QID?
>>
>
> I agree that this is a change that didn't have to be made in this patch.
> However, after patch 4 of the series we cannot depend on the
> skbdesc->desc_len being set anymore, and we would have to depend on
> checking the QID anyway.
> Note that in reality these two checks are completely equivalent with
> respect to the result.

Hmm, is that a good idea? I mean we are using the skbdesc inside the
function, but we can't be sure that one of the basic values contains the right
value?

Ivo

^ permalink raw reply

* Re: [PATCH 6/9] rt2x00: Push beacon TX descriptor writing to drivers.
From: Ivo Van Doorn @ 2010-05-12 17:45 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <AANLkTikc49OYX7l9H1fpU0RCDU9cHAe4XAIr-nq1nWmq@mail.gmail.com>

On Wed, May 12, 2010 at 11:53 AM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> On Wed, May 12, 2010 at 9:06 AM, Ivo Van Doorn <ivdoorn@gmail.com> wrote:
>> On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
>> <gwingerde@gmail.com> wrote:
>>> Not all the devices require a TX descriptor to be written (i.e. rt2800
>>> device don't require them). Push down the creation of the TX descriptor
>>> to the device drivers so that they can decide for themselves whether
>>> a TX descriptor is to be created.
>>>
>>> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
>>> ---
>>>  drivers/net/wireless/rt2x00/rt2400pci.c   |   16 ++++++++++------
>>>  drivers/net/wireless/rt2x00/rt2500pci.c   |   16 ++++++++++------
>>>  drivers/net/wireless/rt2x00/rt2500usb.c   |   11 +++++++++++
>>>  drivers/net/wireless/rt2x00/rt2800pci.c   |   17 +++++++++++++++++
>>>  drivers/net/wireless/rt2x00/rt2800usb.c   |   17 +++++++++++++++++
>>>  drivers/net/wireless/rt2x00/rt2x00debug.c |    1 +
>>>  drivers/net/wireless/rt2x00/rt2x00queue.c |   10 +---------
>>>  drivers/net/wireless/rt2x00/rt61pci.c     |   11 +++++++++++
>>>  drivers/net/wireless/rt2x00/rt73usb.c     |   11 +++++++++++
>>>  9 files changed, 89 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
>>> index def3fa4..741c531 100644
>>> --- a/drivers/net/wireless/rt2x00/rt2400pci.c
>>> +++ b/drivers/net/wireless/rt2x00/rt2400pci.c
>>> @@ -33,6 +33,7 @@
>>>  #include <linux/eeprom_93cx6.h>
>>>
>>>  #include "rt2x00.h"
>>> +#include "rt2x00lib.h"
>>>  #include "rt2x00pci.h"
>>>  #include "rt2400pci.h"
>>
>> rt2x00lib.h must not be used in the drivers. It is for the rt2x00lib
>> internal files only.
>
> OK. This include is/was necessary for the rt2x00debug_dump_frame call.
> So this may be handled with your next issue.
>
>>
>>> @@ -1074,9 +1075,6 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
>>>                                   struct txentry_desc *txdesc)
>>>  {
>>>        struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
>>> -       struct queue_entry_priv_pci *entry_priv = entry->priv_data;
>>> -       struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
>>> -       u32 word;
>>>        u32 reg;
>>>
>>>        /*
>>> @@ -1089,9 +1087,15 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
>>>
>>>        rt2x00queue_map_txskb(rt2x00dev, entry->skb);
>>>
>>> -       rt2x00_desc_read(entry_priv->desc, 1, &word);
>>> -       rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
>>> -       rt2x00_desc_write(entry_priv->desc, 1, word);
>>> +       /*
>>> +        * Write the TX descriptor for the beacon.
>>> +        */
>>> +       rt2400pci_write_tx_desc(rt2x00dev, entry->skb, txdesc);
>>> +
>>> +       /*
>>> +        * Dump beacon to userspace through debugfs.
>>> +        */
>>> +       rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
>>
>> The goal for rt2x00debug was that the logic must be inside rt2x00lib
>> as much as possible.
>> This can/should be moved into rt2x00lib where write_beacon() is being called.
>>
>
> Yes, I wasn't too happy about this part about the patch, but couldn't
> find a better solution. The problem is that with the patch the frame
> cannot be dumped before the call to write_beacon, as the descriptor
> hasn't been set up yet. Also, it cannot be dumped after the call to
> write_beacon as most of the write_beacon functions actually free the
> skb with the beacon.
> So, I ran out of ideas as to how to keep rt2x00debug only inside
> rt2x00lib. I'm open for suggestions, though.

Well ok, if there aren't obvious alternatives this change is fine.
However, please move the declaration of rt2x00debug_dump_frame()
into rt2x00.h? That way we don't have to include the rt2x00lib.h header.

Ivo

^ permalink raw reply

* Re: [PATCH 8/9] rt2x00: Reverse calling order of bus write_tx_desc and driver write_tx_desc.
From: Ivo Van Doorn @ 2010-05-12 17:47 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <AANLkTinzPPIbH6bivhjTF1jB6R2mZ5e9rnhX23Yr8bJ7@mail.gmail.com>

On Wed, May 12, 2010 at 11:55 AM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> On Wed, May 12, 2010 at 9:02 AM, Ivo Van Doorn <ivdoorn@gmail.com> wrote:
>> On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
>> <gwingerde@gmail.com> wrote:
>>> For rt2800 reverse the calling order of rt2x00pci_write_data and
>>> rt2800pci_write_data. Currently rt2800pci_write_data calls rt2x00pci_write_data
>>> as there can be only 1 driver callback function specified by the driver.
>>> Reverse this calling order by introducing a new driver callback function,
>>> called add_tx_datadesc, which is called from the bus-specific write_tx_data
>>> functions.
>>> Preparation for futher cleanups in the skb data handling of rt2x00.
>>>
>>> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
>>> ---
>>>  drivers/net/wireless/rt2x00/rt2800pci.c |   15 ++++-----------
>>>  drivers/net/wireless/rt2x00/rt2x00.h    |    2 ++
>>>  drivers/net/wireless/rt2x00/rt2x00pci.c |    6 ++++++
>>>  drivers/net/wireless/rt2x00/rt2x00usb.c |    6 ++++++
>>>  4 files changed, 18 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
>>> index fcca30c..bbd6481 100644
>>> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
>>> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
>>> @@ -614,18 +614,10 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
>>>  /*
>>>  * TX descriptor initialization
>>>  */
>>> -static int rt2800pci_write_tx_data(struct queue_entry* entry,
>>> -                                  struct txentry_desc *txdesc)
>>> +static void rt2800pci_add_tx_datadesc(struct queue_entry* entry,
>>> +                                     struct txentry_desc *txdesc)
>>>  {
>>> -       int ret;
>>> -
>>> -       ret = rt2x00pci_write_tx_data(entry, txdesc);
>>> -       if (ret)
>>> -               return ret;
>>> -
>>>        rt2800_write_txwi(entry->skb, txdesc);
>>> -
>>> -       return 0;
>>>  }
>>>
>>>
>>> @@ -1080,7 +1072,8 @@ static const struct rt2x00lib_ops rt2800pci_rt2x00_ops = {
>>>        .reset_tuner            = rt2800_reset_tuner,
>>>        .link_tuner             = rt2800_link_tuner,
>>>        .write_tx_desc          = rt2800pci_write_tx_desc,
>>> -       .write_tx_data          = rt2800pci_write_tx_data,
>>> +       .write_tx_data          = rt2x00pci_write_tx_data,
>>> +       .add_tx_datadesc        = rt2800pci_add_tx_datadesc,
>>>        .write_beacon           = rt2800pci_write_beacon,
>>>        .kick_tx_queue          = rt2800pci_kick_tx_queue,
>>>        .kill_tx_queue          = rt2800pci_kill_tx_queue,
>>
>> Doesn't the name write_tx_datadesc make more sense?
>>
>
> The names are equivalent to me. But if that name makes more sense to
> you then I have no problem with changing the name of the callback
> function.

Yes please rename it. That way it fits in the
write_tx_desc
write_tx_data
write_tx_datadesc

Thanks,

Ivo

^ permalink raw reply

* Re: [PATCH 2/9] rt2x00: Fix beacon descriptor writing for rt61pci.
From: Gertjan van Wingerde @ 2010-05-12 18:42 UTC (permalink / raw)
  To: Ivo Van Doorn; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <AANLkTimubF1UWmzLwhJ7vhquBHmRywJQTvJciU3D3SNx@mail.gmail.com>

On 05/12/10 19:43, Ivo Van Doorn wrote:
> On Wed, May 12, 2010 at 11:46 AM, Gertjan van Wingerde
> <gwingerde@gmail.com> wrote:
>> On Wed, May 12, 2010 at 8:59 AM, Ivo Van Doorn <ivdoorn@gmail.com> wrote:
>>> On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
>>> <gwingerde@gmail.com> wrote:
>>>> The buffer address descriptor word is not part of the TXINFO structure
>>>> needed for beacons. The current writing of that word for beacons is
>>>> therefore an out-of-bounds write.
>>>> Fix this by only writing the buffer address descriptor word for TX
>>>> queues.
>>>>
>>>> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
>>>> ---
>>>>  drivers/net/wireless/rt2x00/rt61pci.c |   10 +++++-----
>>>>  1 files changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
>>>> index 2436363..99c2981 100644
>>>> --- a/drivers/net/wireless/rt2x00/rt61pci.c
>>>> +++ b/drivers/net/wireless/rt2x00/rt61pci.c
>>>> @@ -1801,12 +1801,12 @@ static void rt61pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
>>>>        rt2x00_set_field32(&word, TXD_W5_WAITING_DMA_DONE_INT, 1);
>>>>        rt2x00_desc_write(txd, 5, word);
>>>>
>>>> -       rt2x00_desc_read(txd, 6, &word);
>>>> -       rt2x00_set_field32(&word, TXD_W6_BUFFER_PHYSICAL_ADDRESS,
>>>> -                          skbdesc->skb_dma);
>>>> -       rt2x00_desc_write(txd, 6, word);
>>>> +       if (txdesc->queue != QID_BEACON) {
>>>> +               rt2x00_desc_read(txd, 6, &word);
>>>> +               rt2x00_set_field32(&word, TXD_W6_BUFFER_PHYSICAL_ADDRESS,
>>>> +                                  skbdesc->skb_dma);
>>>> +               rt2x00_desc_write(txd, 6, word);
>>>>
>>>> -       if (skbdesc->desc_len > TXINFO_SIZE) {
>>>>                rt2x00_desc_read(txd, 11, &word);
>>>>                rt2x00_set_field32(&word, TXD_W11_BUFFER_LENGTH0,
>>>>                                   txdesc->length);
>>>
>>> Shouldn't the check for TXINFO_SIZE be used rather than explicitly
>>> checking for the QID?
>>>
>>
>> I agree that this is a change that didn't have to be made in this patch.
>> However, after patch 4 of the series we cannot depend on the
>> skbdesc->desc_len being set anymore, and we would have to depend on
>> checking the QID anyway.
>> Note that in reality these two checks are completely equivalent with
>> respect to the result.
> 
> Hmm, is that a good idea? I mean we are using the skbdesc inside the
> function, but we can't be sure that one of the basic values contains the right
> value?
> 

To be honest, the chipset drivers don't need the skbdesc->desc and skbdesc->desc_len
at all. Based on the queue ID they already have everything they need to know to write
a descriptor. As far as I can tell the only code that really needs those two fields is
the rt2x00debug code, to dump a frame to userspace via debugfs.

As the dumping through debugfs is done after writing the TX descriptor, with this patch
I changed the desc and desc_len fields of the skbdesc to be output of the write_tx_desc
function (for the generic parts of the rt2x00 code) rather than being input.

Note that originally my plan was to get rid of the desc and desc_len field altogether,
but for now I refrained from doing that.

---
Gertjan.


^ permalink raw reply

* Re: [PATCH 2/9] rt2x00: Fix beacon descriptor writing for rt61pci.
From: Ivo Van Doorn @ 2010-05-12 18:46 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <4BEAF6B1.3000200@gmail.com>

On Wed, May 12, 2010 at 8:42 PM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> On 05/12/10 19:43, Ivo Van Doorn wrote:
>> On Wed, May 12, 2010 at 11:46 AM, Gertjan van Wingerde
>> <gwingerde@gmail.com> wrote:
>>> On Wed, May 12, 2010 at 8:59 AM, Ivo Van Doorn <ivdoorn@gmail.com> wrote:
>>>> On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
>>>> <gwingerde@gmail.com> wrote:
>>>>> The buffer address descriptor word is not part of the TXINFO structure
>>>>> needed for beacons. The current writing of that word for beacons is
>>>>> therefore an out-of-bounds write.
>>>>> Fix this by only writing the buffer address descriptor word for TX
>>>>> queues.
>>>>>
>>>>> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
>>>>> ---
>>>>>  drivers/net/wireless/rt2x00/rt61pci.c |   10 +++++-----
>>>>>  1 files changed, 5 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
>>>>> index 2436363..99c2981 100644
>>>>> --- a/drivers/net/wireless/rt2x00/rt61pci.c
>>>>> +++ b/drivers/net/wireless/rt2x00/rt61pci.c
>>>>> @@ -1801,12 +1801,12 @@ static void rt61pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
>>>>>        rt2x00_set_field32(&word, TXD_W5_WAITING_DMA_DONE_INT, 1);
>>>>>        rt2x00_desc_write(txd, 5, word);
>>>>>
>>>>> -       rt2x00_desc_read(txd, 6, &word);
>>>>> -       rt2x00_set_field32(&word, TXD_W6_BUFFER_PHYSICAL_ADDRESS,
>>>>> -                          skbdesc->skb_dma);
>>>>> -       rt2x00_desc_write(txd, 6, word);
>>>>> +       if (txdesc->queue != QID_BEACON) {
>>>>> +               rt2x00_desc_read(txd, 6, &word);
>>>>> +               rt2x00_set_field32(&word, TXD_W6_BUFFER_PHYSICAL_ADDRESS,
>>>>> +                                  skbdesc->skb_dma);
>>>>> +               rt2x00_desc_write(txd, 6, word);
>>>>>
>>>>> -       if (skbdesc->desc_len > TXINFO_SIZE) {
>>>>>                rt2x00_desc_read(txd, 11, &word);
>>>>>                rt2x00_set_field32(&word, TXD_W11_BUFFER_LENGTH0,
>>>>>                                   txdesc->length);
>>>>
>>>> Shouldn't the check for TXINFO_SIZE be used rather than explicitly
>>>> checking for the QID?
>>>>
>>>
>>> I agree that this is a change that didn't have to be made in this patch.
>>> However, after patch 4 of the series we cannot depend on the
>>> skbdesc->desc_len being set anymore, and we would have to depend on
>>> checking the QID anyway.
>>> Note that in reality these two checks are completely equivalent with
>>> respect to the result.
>>
>> Hmm, is that a good idea? I mean we are using the skbdesc inside the
>> function, but we can't be sure that one of the basic values contains the right
>> value?
>>
>
> To be honest, the chipset drivers don't need the skbdesc->desc and skbdesc->desc_len
> at all. Based on the queue ID they already have everything they need to know to write
> a descriptor. As far as I can tell the only code that really needs those two fields is
> the rt2x00debug code, to dump a frame to userspace via debugfs.
>
> As the dumping through debugfs is done after writing the TX descriptor, with this patch
> I changed the desc and desc_len fields of the skbdesc to be output of the write_tx_desc
> function (for the generic parts of the rt2x00 code) rather than being input.
>
> Note that originally my plan was to get rid of the desc and desc_len field altogether,
> but for now I refrained from doing that.

Ah ok. Thanks for the explanation.

Ivo

^ permalink raw reply

* Re: [PATCH 2/9] rt2x00: Fix beacon descriptor writing for rt61pci.
From: Ivo Van Doorn @ 2010-05-12 18:47 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <1273614705-25118-3-git-send-email-gwingerde@gmail.com>

On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> The buffer address descriptor word is not part of the TXINFO structure
> needed for beacons. The current writing of that word for beacons is
> therefore an out-of-bounds write.
> Fix this by only writing the buffer address descriptor word for TX
> queues.
>
> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>

Acked-by: Ivo van Doorn <IvDoorn@gmail.com>

^ permalink raw reply

* Re: [PATCH 8/9] rt2x00: Reverse calling order of bus write_tx_desc and driver write_tx_desc.
From: Gertjan van Wingerde @ 2010-05-12 19:02 UTC (permalink / raw)
  To: Ivo Van Doorn; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <AANLkTilKi1zKyRWUNuBoQnHaTy2UdX05Pd2E2gOvygrJ@mail.gmail.com>

On 05/12/10 19:47, Ivo Van Doorn wrote:
> On Wed, May 12, 2010 at 11:55 AM, Gertjan van Wingerde
> <gwingerde@gmail.com> wrote:
>> On Wed, May 12, 2010 at 9:02 AM, Ivo Van Doorn <ivdoorn@gmail.com> wrote:
>>> On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
>>> <gwingerde@gmail.com> wrote:
>>>> For rt2800 reverse the calling order of rt2x00pci_write_data and
>>>> rt2800pci_write_data. Currently rt2800pci_write_data calls rt2x00pci_write_data
>>>> as there can be only 1 driver callback function specified by the driver.
>>>> Reverse this calling order by introducing a new driver callback function,
>>>> called add_tx_datadesc, which is called from the bus-specific write_tx_data
>>>> functions.
>>>> Preparation for futher cleanups in the skb data handling of rt2x00.
>>>>
>>>> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
>>>> ---
>>>>  drivers/net/wireless/rt2x00/rt2800pci.c |   15 ++++-----------
>>>>  drivers/net/wireless/rt2x00/rt2x00.h    |    2 ++
>>>>  drivers/net/wireless/rt2x00/rt2x00pci.c |    6 ++++++
>>>>  drivers/net/wireless/rt2x00/rt2x00usb.c |    6 ++++++
>>>>  4 files changed, 18 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
>>>> index fcca30c..bbd6481 100644
>>>> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
>>>> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
>>>> @@ -614,18 +614,10 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
>>>>  /*
>>>>  * TX descriptor initialization
>>>>  */
>>>> -static int rt2800pci_write_tx_data(struct queue_entry* entry,
>>>> -                                  struct txentry_desc *txdesc)
>>>> +static void rt2800pci_add_tx_datadesc(struct queue_entry* entry,
>>>> +                                     struct txentry_desc *txdesc)
>>>>  {
>>>> -       int ret;
>>>> -
>>>> -       ret = rt2x00pci_write_tx_data(entry, txdesc);
>>>> -       if (ret)
>>>> -               return ret;
>>>> -
>>>>        rt2800_write_txwi(entry->skb, txdesc);
>>>> -
>>>> -       return 0;
>>>>  }
>>>>
>>>>
>>>> @@ -1080,7 +1072,8 @@ static const struct rt2x00lib_ops rt2800pci_rt2x00_ops = {
>>>>        .reset_tuner            = rt2800_reset_tuner,
>>>>        .link_tuner             = rt2800_link_tuner,
>>>>        .write_tx_desc          = rt2800pci_write_tx_desc,
>>>> -       .write_tx_data          = rt2800pci_write_tx_data,
>>>> +       .write_tx_data          = rt2x00pci_write_tx_data,
>>>> +       .add_tx_datadesc        = rt2800pci_add_tx_datadesc,
>>>>        .write_beacon           = rt2800pci_write_beacon,
>>>>        .kick_tx_queue          = rt2800pci_kick_tx_queue,
>>>>        .kill_tx_queue          = rt2800pci_kill_tx_queue,
>>>
>>> Doesn't the name write_tx_datadesc make more sense?
>>>
>>
>> The names are equivalent to me. But if that name makes more sense to
>> you then I have no problem with changing the name of the callback
>> function.
> 
> Yes please rename it. That way it fits in the
> write_tx_desc
> write_tx_data
> write_tx_datadesc
> 

OK. Will do.

---
Gertjan

^ permalink raw reply

* Re: [PATCH 6/9] rt2x00: Push beacon TX descriptor writing to drivers.
From: Gertjan van Wingerde @ 2010-05-12 19:03 UTC (permalink / raw)
  To: Ivo Van Doorn; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <AANLkTil8eCI40L90Siwpqh6qAx27IYJjVL9dCNl7o5Jm@mail.gmail.com>

On 05/12/10 19:45, Ivo Van Doorn wrote:
> On Wed, May 12, 2010 at 11:53 AM, Gertjan van Wingerde
> <gwingerde@gmail.com> wrote:
>> On Wed, May 12, 2010 at 9:06 AM, Ivo Van Doorn <ivdoorn@gmail.com> wrote:
>>> On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
>>> <gwingerde@gmail.com> wrote:
>>>> Not all the devices require a TX descriptor to be written (i.e. rt2800
>>>> device don't require them). Push down the creation of the TX descriptor
>>>> to the device drivers so that they can decide for themselves whether
>>>> a TX descriptor is to be created.
>>>>
>>>> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
>>>> ---
>>>>  drivers/net/wireless/rt2x00/rt2400pci.c   |   16 ++++++++++------
>>>>  drivers/net/wireless/rt2x00/rt2500pci.c   |   16 ++++++++++------
>>>>  drivers/net/wireless/rt2x00/rt2500usb.c   |   11 +++++++++++
>>>>  drivers/net/wireless/rt2x00/rt2800pci.c   |   17 +++++++++++++++++
>>>>  drivers/net/wireless/rt2x00/rt2800usb.c   |   17 +++++++++++++++++
>>>>  drivers/net/wireless/rt2x00/rt2x00debug.c |    1 +
>>>>  drivers/net/wireless/rt2x00/rt2x00queue.c |   10 +---------
>>>>  drivers/net/wireless/rt2x00/rt61pci.c     |   11 +++++++++++
>>>>  drivers/net/wireless/rt2x00/rt73usb.c     |   11 +++++++++++
>>>>  9 files changed, 89 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
>>>> index def3fa4..741c531 100644
>>>> --- a/drivers/net/wireless/rt2x00/rt2400pci.c
>>>> +++ b/drivers/net/wireless/rt2x00/rt2400pci.c
>>>> @@ -33,6 +33,7 @@
>>>>  #include <linux/eeprom_93cx6.h>
>>>>
>>>>  #include "rt2x00.h"
>>>> +#include "rt2x00lib.h"
>>>>  #include "rt2x00pci.h"
>>>>  #include "rt2400pci.h"
>>>
>>> rt2x00lib.h must not be used in the drivers. It is for the rt2x00lib
>>> internal files only.
>>
>> OK. This include is/was necessary for the rt2x00debug_dump_frame call.
>> So this may be handled with your next issue.
>>
>>>
>>>> @@ -1074,9 +1075,6 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
>>>>                                   struct txentry_desc *txdesc)
>>>>  {
>>>>        struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
>>>> -       struct queue_entry_priv_pci *entry_priv = entry->priv_data;
>>>> -       struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
>>>> -       u32 word;
>>>>        u32 reg;
>>>>
>>>>        /*
>>>> @@ -1089,9 +1087,15 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
>>>>
>>>>        rt2x00queue_map_txskb(rt2x00dev, entry->skb);
>>>>
>>>> -       rt2x00_desc_read(entry_priv->desc, 1, &word);
>>>> -       rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
>>>> -       rt2x00_desc_write(entry_priv->desc, 1, word);
>>>> +       /*
>>>> +        * Write the TX descriptor for the beacon.
>>>> +        */
>>>> +       rt2400pci_write_tx_desc(rt2x00dev, entry->skb, txdesc);
>>>> +
>>>> +       /*
>>>> +        * Dump beacon to userspace through debugfs.
>>>> +        */
>>>> +       rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
>>>
>>> The goal for rt2x00debug was that the logic must be inside rt2x00lib
>>> as much as possible.
>>> This can/should be moved into rt2x00lib where write_beacon() is being called.
>>>
>>
>> Yes, I wasn't too happy about this part about the patch, but couldn't
>> find a better solution. The problem is that with the patch the frame
>> cannot be dumped before the call to write_beacon, as the descriptor
>> hasn't been set up yet. Also, it cannot be dumped after the call to
>> write_beacon as most of the write_beacon functions actually free the
>> skb with the beacon.
>> So, I ran out of ideas as to how to keep rt2x00debug only inside
>> rt2x00lib. I'm open for suggestions, though.
> 
> Well ok, if there aren't obvious alternatives this change is fine.
> However, please move the declaration of rt2x00debug_dump_frame()
> into rt2x00.h? That way we don't have to include the rt2x00lib.h header.
> 

The only alternative I can think of is to have the chipset driver clone the skb to have
a private copy of the skb, so that the original one can be freed in the generic code.
In that case the rt2x00debug_dump_frame call can be done from rt2x00queue code, after
the write_beacon function has been called.

What do you think of that solution?

---
Gertjan.

^ permalink raw reply

* Re: [PATCH 6/9] rt2x00: Push beacon TX descriptor writing to drivers.
From: Ivo Van Doorn @ 2010-05-12 19:10 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <4BEAFB90.3090301@gmail.com>

On Wed, May 12, 2010 at 9:03 PM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> On 05/12/10 19:45, Ivo Van Doorn wrote:
>> On Wed, May 12, 2010 at 11:53 AM, Gertjan van Wingerde
>> <gwingerde@gmail.com> wrote:
>>> On Wed, May 12, 2010 at 9:06 AM, Ivo Van Doorn <ivdoorn@gmail.com> wrote:
>>>> On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
>>>> <gwingerde@gmail.com> wrote:
>>>>> Not all the devices require a TX descriptor to be written (i.e. rt2800
>>>>> device don't require them). Push down the creation of the TX descriptor
>>>>> to the device drivers so that they can decide for themselves whether
>>>>> a TX descriptor is to be created.
>>>>>
>>>>> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
>>>>> ---
>>>>>  drivers/net/wireless/rt2x00/rt2400pci.c   |   16 ++++++++++------
>>>>>  drivers/net/wireless/rt2x00/rt2500pci.c   |   16 ++++++++++------
>>>>>  drivers/net/wireless/rt2x00/rt2500usb.c   |   11 +++++++++++
>>>>>  drivers/net/wireless/rt2x00/rt2800pci.c   |   17 +++++++++++++++++
>>>>>  drivers/net/wireless/rt2x00/rt2800usb.c   |   17 +++++++++++++++++
>>>>>  drivers/net/wireless/rt2x00/rt2x00debug.c |    1 +
>>>>>  drivers/net/wireless/rt2x00/rt2x00queue.c |   10 +---------
>>>>>  drivers/net/wireless/rt2x00/rt61pci.c     |   11 +++++++++++
>>>>>  drivers/net/wireless/rt2x00/rt73usb.c     |   11 +++++++++++
>>>>>  9 files changed, 89 insertions(+), 21 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
>>>>> index def3fa4..741c531 100644
>>>>> --- a/drivers/net/wireless/rt2x00/rt2400pci.c
>>>>> +++ b/drivers/net/wireless/rt2x00/rt2400pci.c
>>>>> @@ -33,6 +33,7 @@
>>>>>  #include <linux/eeprom_93cx6.h>
>>>>>
>>>>>  #include "rt2x00.h"
>>>>> +#include "rt2x00lib.h"
>>>>>  #include "rt2x00pci.h"
>>>>>  #include "rt2400pci.h"
>>>>
>>>> rt2x00lib.h must not be used in the drivers. It is for the rt2x00lib
>>>> internal files only.
>>>
>>> OK. This include is/was necessary for the rt2x00debug_dump_frame call.
>>> So this may be handled with your next issue.
>>>
>>>>
>>>>> @@ -1074,9 +1075,6 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
>>>>>                                   struct txentry_desc *txdesc)
>>>>>  {
>>>>>        struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
>>>>> -       struct queue_entry_priv_pci *entry_priv = entry->priv_data;
>>>>> -       struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
>>>>> -       u32 word;
>>>>>        u32 reg;
>>>>>
>>>>>        /*
>>>>> @@ -1089,9 +1087,15 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
>>>>>
>>>>>        rt2x00queue_map_txskb(rt2x00dev, entry->skb);
>>>>>
>>>>> -       rt2x00_desc_read(entry_priv->desc, 1, &word);
>>>>> -       rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
>>>>> -       rt2x00_desc_write(entry_priv->desc, 1, word);
>>>>> +       /*
>>>>> +        * Write the TX descriptor for the beacon.
>>>>> +        */
>>>>> +       rt2400pci_write_tx_desc(rt2x00dev, entry->skb, txdesc);
>>>>> +
>>>>> +       /*
>>>>> +        * Dump beacon to userspace through debugfs.
>>>>> +        */
>>>>> +       rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
>>>>
>>>> The goal for rt2x00debug was that the logic must be inside rt2x00lib
>>>> as much as possible.
>>>> This can/should be moved into rt2x00lib where write_beacon() is being called.
>>>>
>>>
>>> Yes, I wasn't too happy about this part about the patch, but couldn't
>>> find a better solution. The problem is that with the patch the frame
>>> cannot be dumped before the call to write_beacon, as the descriptor
>>> hasn't been set up yet. Also, it cannot be dumped after the call to
>>> write_beacon as most of the write_beacon functions actually free the
>>> skb with the beacon.
>>> So, I ran out of ideas as to how to keep rt2x00debug only inside
>>> rt2x00lib. I'm open for suggestions, though.
>>
>> Well ok, if there aren't obvious alternatives this change is fine.
>> However, please move the declaration of rt2x00debug_dump_frame()
>> into rt2x00.h? That way we don't have to include the rt2x00lib.h header.
>>
>
> The only alternative I can think of is to have the chipset driver clone the skb to have
> a private copy of the skb, so that the original one can be freed in the generic code.
> In that case the rt2x00debug_dump_frame call can be done from rt2x00queue code, after
> the write_beacon function has been called.
>
> What do you think of that solution?

I think the extra cloning isn't really worth the effort. Since it is
only needed for debugfs,
so the code in rt2x00queue must check for debugfs support, and debugfs code must
be updated not to clone the frame again.

Overall I think it is in this case better to just do your change with calling
the debugfs function from the driver, but moving the declaration of the function
into rt2x00.h to prevent the inclusion of rt2x00lib.h into the drivers.

Ivo

^ permalink raw reply

* Re: [PATCH 6/9] rt2x00: Push beacon TX descriptor writing to drivers.
From: Gertjan van Wingerde @ 2010-05-12 19:22 UTC (permalink / raw)
  To: Ivo Van Doorn; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <AANLkTinx3zLY3pPSfk93oaHBp_IlWuFGRJJoFiMIirws@mail.gmail.com>

On 05/12/10 21:10, Ivo Van Doorn wrote:
> On Wed, May 12, 2010 at 9:03 PM, Gertjan van Wingerde
> <gwingerde@gmail.com> wrote:
>> On 05/12/10 19:45, Ivo Van Doorn wrote:
>>> On Wed, May 12, 2010 at 11:53 AM, Gertjan van Wingerde
>>> <gwingerde@gmail.com> wrote:
>>>> On Wed, May 12, 2010 at 9:06 AM, Ivo Van Doorn <ivdoorn@gmail.com> wrote:
>>>>> On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
>>>>> <gwingerde@gmail.com> wrote:
>>>>>> Not all the devices require a TX descriptor to be written (i.e. rt2800
>>>>>> device don't require them). Push down the creation of the TX descriptor
>>>>>> to the device drivers so that they can decide for themselves whether
>>>>>> a TX descriptor is to be created.
>>>>>>
>>>>>> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
>>>>>> ---
>>>>>>  drivers/net/wireless/rt2x00/rt2400pci.c   |   16 ++++++++++------
>>>>>>  drivers/net/wireless/rt2x00/rt2500pci.c   |   16 ++++++++++------
>>>>>>  drivers/net/wireless/rt2x00/rt2500usb.c   |   11 +++++++++++
>>>>>>  drivers/net/wireless/rt2x00/rt2800pci.c   |   17 +++++++++++++++++
>>>>>>  drivers/net/wireless/rt2x00/rt2800usb.c   |   17 +++++++++++++++++
>>>>>>  drivers/net/wireless/rt2x00/rt2x00debug.c |    1 +
>>>>>>  drivers/net/wireless/rt2x00/rt2x00queue.c |   10 +---------
>>>>>>  drivers/net/wireless/rt2x00/rt61pci.c     |   11 +++++++++++
>>>>>>  drivers/net/wireless/rt2x00/rt73usb.c     |   11 +++++++++++
>>>>>>  9 files changed, 89 insertions(+), 21 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
>>>>>> index def3fa4..741c531 100644
>>>>>> --- a/drivers/net/wireless/rt2x00/rt2400pci.c
>>>>>> +++ b/drivers/net/wireless/rt2x00/rt2400pci.c
>>>>>> @@ -33,6 +33,7 @@
>>>>>>  #include <linux/eeprom_93cx6.h>
>>>>>>
>>>>>>  #include "rt2x00.h"
>>>>>> +#include "rt2x00lib.h"
>>>>>>  #include "rt2x00pci.h"
>>>>>>  #include "rt2400pci.h"
>>>>>
>>>>> rt2x00lib.h must not be used in the drivers. It is for the rt2x00lib
>>>>> internal files only.
>>>>
>>>> OK. This include is/was necessary for the rt2x00debug_dump_frame call.
>>>> So this may be handled with your next issue.
>>>>
>>>>>
>>>>>> @@ -1074,9 +1075,6 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
>>>>>>                                   struct txentry_desc *txdesc)
>>>>>>  {
>>>>>>        struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
>>>>>> -       struct queue_entry_priv_pci *entry_priv = entry->priv_data;
>>>>>> -       struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
>>>>>> -       u32 word;
>>>>>>        u32 reg;
>>>>>>
>>>>>>        /*
>>>>>> @@ -1089,9 +1087,15 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
>>>>>>
>>>>>>        rt2x00queue_map_txskb(rt2x00dev, entry->skb);
>>>>>>
>>>>>> -       rt2x00_desc_read(entry_priv->desc, 1, &word);
>>>>>> -       rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
>>>>>> -       rt2x00_desc_write(entry_priv->desc, 1, word);
>>>>>> +       /*
>>>>>> +        * Write the TX descriptor for the beacon.
>>>>>> +        */
>>>>>> +       rt2400pci_write_tx_desc(rt2x00dev, entry->skb, txdesc);
>>>>>> +
>>>>>> +       /*
>>>>>> +        * Dump beacon to userspace through debugfs.
>>>>>> +        */
>>>>>> +       rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
>>>>>
>>>>> The goal for rt2x00debug was that the logic must be inside rt2x00lib
>>>>> as much as possible.
>>>>> This can/should be moved into rt2x00lib where write_beacon() is being called.
>>>>>
>>>>
>>>> Yes, I wasn't too happy about this part about the patch, but couldn't
>>>> find a better solution. The problem is that with the patch the frame
>>>> cannot be dumped before the call to write_beacon, as the descriptor
>>>> hasn't been set up yet. Also, it cannot be dumped after the call to
>>>> write_beacon as most of the write_beacon functions actually free the
>>>> skb with the beacon.
>>>> So, I ran out of ideas as to how to keep rt2x00debug only inside
>>>> rt2x00lib. I'm open for suggestions, though.
>>>
>>> Well ok, if there aren't obvious alternatives this change is fine.
>>> However, please move the declaration of rt2x00debug_dump_frame()
>>> into rt2x00.h? That way we don't have to include the rt2x00lib.h header.
>>>
>>
>> The only alternative I can think of is to have the chipset driver clone the skb to have
>> a private copy of the skb, so that the original one can be freed in the generic code.
>> In that case the rt2x00debug_dump_frame call can be done from rt2x00queue code, after
>> the write_beacon function has been called.
>>
>> What do you think of that solution?
> 
> I think the extra cloning isn't really worth the effort. Since it is
> only needed for debugfs,
> so the code in rt2x00queue must check for debugfs support, and debugfs code must
> be updated not to clone the frame again.
> 
> Overall I think it is in this case better to just do your change with calling
> the debugfs function from the driver, but moving the declaration of the function
> into rt2x00.h to prevent the inclusion of rt2x00lib.h into the drivers.
> 

OK. I'll move the declaration to rt2x00.h.

---
Gertjan.

^ permalink raw reply

* Re: [PATCH v2] mac80211: Add HT IE to IBSS beacons and probe responses.
From: Benoit Papillault @ 2010-05-12 21:02 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1273574892.3669.54.camel@jlt3.sipsolutions.net>

Le 11/05/2010 12:48, Johannes Berg a écrit :
> On Sat, 2010-05-08 at 01:01 +0200, Benoit Papillault wrote:
>
>>> It would be helpful if you were to rebase over my patch that adds the
>>> channel type tracking.
>>
>> Your patches are now in wireless-testing, so I am doing it at the moment.
>
> Thanks.
>
>>>> +	if (channel_type != NL80211_CHAN_NO_HT&&
>>>> +	    sband->ht_cap.ht_supported) {
>>>
>>> You shouldn't be able to get here with an HT channel but !ht_supported,
>>> no? Or is that to support the case where you have HT only on one band?
>>
>> Good point. I think there are several cases here :
>>
>> - a non-HT STA is joining an HT IBSS (we need to check 802.11n-2009 to
>> see how it's supposed to be handled). In this case channel_type could be
>> ht40+ and ht_supported = false
>
> Hmm, true.
>
>> - an HT STA is joining a non-HT IBSS. It's clear in this case that no HT
>> IE should be sent, which is catched by (channel_type !=
>> NL80211_CHAN_NO_HT) condition.
>
> Right.
>
>> Could we examine those cases in a follow up patch?
>
> Well what do we actually need to do then?

I thought again about 2 HT STA joining a non-HT IBSS. If we want those 2 
STA to be able to send/receive HT frames then they must know each other 
capabilities. So HT Capabilities should be sent if ht_supported = true, 
even if channel_type is CHAN_NO_HT

>
>>>> +}
>>>> +
>>>> +int ieee80211_add_ht_info(u8 **ppos,
>>>> +			  struct ieee80211_supported_band *sband,
>>>> +			  struct ieee80211_channel *channel,
>>>> +			  enum nl80211_channel_type channel_type)
>>>
>>> what's wrong with ieee80211_add_ht_ie()
>>
>> Seems it's close to what I'm doing, but not entirely the same. I will
>> read it. If applicable, should I move ieee80211_add_ht_ie to util.c (and
>> declaration to ieee80211_i.h) then ?
>
> Yes.

Done. I still keep both ieee80211_add_ht_ie and ieee80211_add_ht_cap. 
What is done in ieee80211_add_ht_ie is only required for an HT AP, not 
an HT IBSS STA. I guess we should pass "sdata" to those functions before 
merging them.

>
> Also I just noticed that there's a TODO item in rx.c when we receive an
> HT frame from a peer we don't know about yet. Not sure what to do there,
> but you'll need to look at it.

I did some patch in this area in my tree. Basically, the peer STA is 
created only on beacon/probe response since only those frames contains 
peer capabilities. Any frames received before is simply ignored.

>
> johannes
>
>

Regards,
Benoit

^ permalink raw reply

* [PATCH] cfg80211: Parse channel_type in NL80211_CMD_JOIN_IBSS
From: Benoit Papillault @ 2010-05-12 21:07 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Benoit Papillault

The channel_type field is filled by iw and parse by cfg80211 before
being sent to mac80211 for instance (we also check if the specified
channel is capable of ht40+, ht40- or ht20). This information is needed
to properly configure an HT IBSS.

It requires a patch to iw to fill this field.

Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
---
 include/net/cfg80211.h |    1 +
 net/wireless/nl80211.c |   20 ++++++++++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index b44a2e5..5ead439 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -806,6 +806,7 @@ struct cfg80211_ibss_params {
 	u8 *ssid;
 	u8 *bssid;
 	struct ieee80211_channel *channel;
+	enum nl80211_channel_type channel_type;
 	u8 *ie;
 	u8 ssid_len, ie_len;
 	u16 beacon_interval;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index aaa1aad..039c13f 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3879,6 +3879,7 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
 	struct cfg80211_ibss_params ibss;
 	struct wiphy *wiphy;
 	struct cfg80211_cached_keys *connkeys = NULL;
+	enum nl80211_channel_type channel_type = NL80211_CHAN_NO_HT;
 	int err;
 
 	memset(&ibss, 0, sizeof(ibss));
@@ -3900,6 +3901,17 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
 			return -EINVAL;
 	}
 
+	if (info->attrs[NL80211_ATTR_WIPHY_CHANNEL_TYPE]) {
+		channel_type = nla_get_u32(
+			info->attrs[NL80211_ATTR_WIPHY_CHANNEL_TYPE]);
+		if (channel_type != NL80211_CHAN_NO_HT &&
+		    channel_type != NL80211_CHAN_HT20 &&
+		    channel_type != NL80211_CHAN_HT40PLUS &&
+		    channel_type != NL80211_CHAN_HT40MINUS)
+			return -EINVAL;
+	}
+	ibss.channel_type = channel_type;
+
 	rtnl_lock();
 
 	err = get_rdev_dev_by_info_ifindex(info, &rdev, &dev);
@@ -3933,11 +3945,11 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
 		ibss.ie_len = nla_len(info->attrs[NL80211_ATTR_IE]);
 	}
 
-	ibss.channel = ieee80211_get_channel(wiphy,
-		nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ]));
+	ibss.channel = rdev_freq_to_chan(rdev,
+		nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ]),
+		channel_type);
 	if (!ibss.channel ||
-	    ibss.channel->flags & IEEE80211_CHAN_NO_IBSS ||
-	    ibss.channel->flags & IEEE80211_CHAN_DISABLED) {
+	    ibss.channel->flags & IEEE80211_CHAN_NO_IBSS) {
 		err = -EINVAL;
 		goto out;
 	}
-- 
1.5.6.5


^ permalink raw reply related

* [PATCH] mac80211: Use struct ieee80211_ht_cap to build HT Capabilities IE
From: Benoit Papillault @ 2010-05-12 21:07 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Benoit Papillault
In-Reply-To: <1273698460-18061-2-git-send-email-benoit.papillault@free.fr>

Instead of writing the HT Capabilities IE byte after byte, we use the
existing ieee80211_ht_cap structure.

Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
---
 net/mac80211/util.c |   55 +++++++++++++++++++++++++--------------------------
 1 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 4a76fa5..047acb8 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1402,7 +1402,7 @@ u8 *ieee80211_add_ht_cap(u8 *pos,
 {
 	if (sband->ht_cap.ht_supported) {
 		u16 cap = sband->ht_cap.cap;
-		__le16 tmp;
+		struct ieee80211_ht_cap ht_cap;
 
 		if (ieee80211_disable_40mhz_24ghz &&
 		    sband->band == IEEE80211_BAND_2GHZ) {
@@ -1410,18 +1410,20 @@ u8 *ieee80211_add_ht_cap(u8 *pos,
 			cap &= ~IEEE80211_HT_CAP_SGI_40;
 		}
 
-		*pos++ = WLAN_EID_HT_CAPABILITY;
-		*pos++ = sizeof(struct ieee80211_ht_cap);
-		memset(pos, 0, sizeof(struct ieee80211_ht_cap));
-		tmp = cpu_to_le16(cap);
-		memcpy(pos, &tmp, sizeof(u16));
-		pos += sizeof(u16);
-		*pos++ = sband->ht_cap.ampdu_factor |
+		ht_cap.cap_info = cpu_to_le16(cap);
+		ht_cap.ampdu_params_info = sband->ht_cap.ampdu_factor |
 			(sband->ht_cap.ampdu_density <<
 			 IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT);
-		memcpy(pos, &sband->ht_cap.mcs, sizeof(sband->ht_cap.mcs));
-		pos += sizeof(sband->ht_cap.mcs);
-		pos += 2 + 4 + 1; /* ext info, BF cap, antsel */
+		ht_cap.mcs = sband->ht_cap.mcs;
+		ht_cap.extended_ht_cap_info = cpu_to_le16(0);
+		ht_cap.tx_BF_cap_info = cpu_to_le32(0);
+		ht_cap.antenna_selection_info = 0;
+
+		/* HT Capabilities element */
+		*pos++ = WLAN_EID_HT_CAPABILITY;
+		*pos++ = sizeof(struct ieee80211_ht_cap);
+		memcpy(pos, &ht_cap, sizeof(struct ieee80211_ht_cap));
+		pos += sizeof(struct ieee80211_ht_cap);
 	}
 
 	return pos;
@@ -1436,7 +1438,7 @@ void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
 	u8 *pos;
 	u32 flags = channel->flags;
 	u16 cap = sband->ht_cap.cap;
-	__le16 tmp;
+	struct ieee80211_ht_cap ht_cap;
 
 	if (!sband->ht_cap.ht_supported)
 		return;
@@ -1492,33 +1494,30 @@ void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
 		break;
 	}
 
-	/* reserve and fill IE */
-
-	pos = skb_put(skb, sizeof(struct ieee80211_ht_cap) + 2);
-	*pos++ = WLAN_EID_HT_CAPABILITY;
-	*pos++ = sizeof(struct ieee80211_ht_cap);
-	memset(pos, 0, sizeof(struct ieee80211_ht_cap));
-
 	/* capability flags */
-	tmp = cpu_to_le16(cap);
-	memcpy(pos, &tmp, sizeof(u16));
-	pos += sizeof(u16);
+	ht_cap.cap_info = cpu_to_le16(cap);
 
 	/* AMPDU parameters */
-	*pos++ = sband->ht_cap.ampdu_factor |
+	ht_cap.ampdu_params_info = sband->ht_cap.ampdu_factor |
 		 (sband->ht_cap.ampdu_density <<
 			IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT);
 
 	/* MCS set */
-	memcpy(pos, &sband->ht_cap.mcs, sizeof(sband->ht_cap.mcs));
-	pos += sizeof(sband->ht_cap.mcs);
+	ht_cap.mcs = sband->ht_cap.mcs;
 
 	/* extended capabilities */
-	pos += sizeof(__le16);
+	ht_cap.extended_ht_cap_info = cpu_to_le16(0);
 
 	/* BF capabilities */
-	pos += sizeof(__le32);
+	ht_cap.tx_BF_cap_info = cpu_to_le32(0);
 
 	/* antenna selection */
-	pos += sizeof(u8);
+	ht_cap.antenna_selection_info = 0;
+
+	/* reserve and fill IE */
+	pos = skb_put(skb, 2 + sizeof(struct ieee80211_ht_cap));
+
+	*pos++ = WLAN_EID_HT_CAPABILITY;
+	*pos++ = sizeof(struct ieee80211_ht_cap);
+	memcpy(pos, &ht_cap, sizeof(struct ieee80211_ht_cap));
 }
-- 
1.5.6.5


^ permalink raw reply related

* [PATCH] mac80211: Move part of the MLME code to helper functions
From: Benoit Papillault @ 2010-05-12 21:07 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Benoit Papillault
In-Reply-To: <1273698460-18061-1-git-send-email-benoit.papillault@free.fr>

We moved code parsing the HT Operation IE to get the associated channel
type and the HT Capabilities IE generation code to util.c. This is a
preliminary step that will be used by HT IBSS code later.

Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
---
 net/mac80211/ieee80211_i.h |   12 +++
 net/mac80211/mlme.c        |   20 +-----
 net/mac80211/util.c        |  179 ++++++++++++++++++++++++++++++++++++++------
 net/mac80211/work.c        |   96 -----------------------
 4 files changed, 170 insertions(+), 137 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 69e7f41..742e12b 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1215,6 +1215,18 @@ size_t ieee80211_ie_split(const u8 *ies, size_t ielen,
 			  const u8 *ids, int n_ids, size_t offset);
 size_t ieee80211_ie_split_vendor(const u8 *ies, size_t ielen, size_t offset);
 
+enum nl80211_channel_type ieee80211_channel_type_from_ht_info(
+	struct ieee80211_local *local,
+	struct ieee80211_ht_info *hti, u16 ap_ht_cap_flags);
+
+void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
+			 struct ieee80211_supported_band *sband,
+			 struct ieee80211_channel *channel,
+			 enum ieee80211_smps_mode smps);
+
+u8 *ieee80211_add_ht_cap(u8 *pos,
+			 struct ieee80211_supported_band *sband);
+
 /* internal work items */
 void ieee80211_work_init(struct ieee80211_local *local);
 void ieee80211_add_work(struct ieee80211_work *wk);
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 3093e46..55d96a6 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -155,24 +155,8 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 		enable_ht = false;
 
 	if (enable_ht) {
-		channel_type = NL80211_CHAN_HT20;
-
-		if (!(ap_ht_cap_flags & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
-		    (sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) &&
-		    (hti->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)) {
-			switch(hti->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
-			case IEEE80211_HT_PARAM_CHA_SEC_ABOVE:
-				if (!(local->hw.conf.channel->flags &
-				    IEEE80211_CHAN_NO_HT40PLUS))
-					channel_type = NL80211_CHAN_HT40PLUS;
-				break;
-			case IEEE80211_HT_PARAM_CHA_SEC_BELOW:
-				if (!(local->hw.conf.channel->flags &
-				    IEEE80211_CHAN_NO_HT40MINUS))
-					channel_type = NL80211_CHAN_HT40MINUS;
-				break;
-			}
-		}
+		channel_type = ieee80211_channel_type_from_ht_info(local,
+					 hti, ap_ht_cap_flags);
 	}
 
 	if (local->tmp_channel)
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 5b79d55..4a76fa5 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -965,29 +965,7 @@ int ieee80211_build_preq_ies(struct ieee80211_local *local, u8 *buffer,
 		offset = noffset;
 	}
 
-	if (sband->ht_cap.ht_supported) {
-		u16 cap = sband->ht_cap.cap;
-		__le16 tmp;
-
-		if (ieee80211_disable_40mhz_24ghz &&
-		    sband->band == IEEE80211_BAND_2GHZ) {
-			cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
-			cap &= ~IEEE80211_HT_CAP_SGI_40;
-		}
-
-		*pos++ = WLAN_EID_HT_CAPABILITY;
-		*pos++ = sizeof(struct ieee80211_ht_cap);
-		memset(pos, 0, sizeof(struct ieee80211_ht_cap));
-		tmp = cpu_to_le16(cap);
-		memcpy(pos, &tmp, sizeof(u16));
-		pos += sizeof(u16);
-		*pos++ = sband->ht_cap.ampdu_factor |
-			 (sband->ht_cap.ampdu_density <<
-				IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT);
-		memcpy(pos, &sband->ht_cap.mcs, sizeof(sband->ht_cap.mcs));
-		pos += sizeof(sband->ht_cap.mcs);
-		pos += 2 + 4 + 1; /* ext info, BF cap, antsel */
-	}
+	pos = ieee80211_add_ht_cap(pos, sband);
 
 	/*
 	 * If adding more here, adjust code in main.c
@@ -1389,3 +1367,158 @@ size_t ieee80211_ie_split_vendor(const u8 *ies, size_t ielen, size_t offset)
 
 	return pos;
 }
+
+enum nl80211_channel_type ieee80211_channel_type_from_ht_info(
+	struct ieee80211_local *local,
+	struct ieee80211_ht_info *hti, u16 ap_ht_cap_flags)
+{
+	struct ieee80211_supported_band *sband;
+	enum nl80211_channel_type channel_type = NL80211_CHAN_HT20;
+
+	sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
+
+	if (!(ap_ht_cap_flags & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
+	    (sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) &&
+	    (hti->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)) {
+		switch (hti->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
+		case IEEE80211_HT_PARAM_CHA_SEC_ABOVE:
+			if (!(local->hw.conf.channel->flags &
+			      IEEE80211_CHAN_NO_HT40PLUS))
+				channel_type = NL80211_CHAN_HT40PLUS;
+			break;
+		case IEEE80211_HT_PARAM_CHA_SEC_BELOW:
+			if (!(local->hw.conf.channel->flags &
+			      IEEE80211_CHAN_NO_HT40MINUS))
+				channel_type = NL80211_CHAN_HT40MINUS;
+			break;
+		}
+	}
+
+	return channel_type;
+}
+
+u8 *ieee80211_add_ht_cap(u8 *pos,
+			 struct ieee80211_supported_band *sband)
+{
+	if (sband->ht_cap.ht_supported) {
+		u16 cap = sband->ht_cap.cap;
+		__le16 tmp;
+
+		if (ieee80211_disable_40mhz_24ghz &&
+		    sband->band == IEEE80211_BAND_2GHZ) {
+			cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
+			cap &= ~IEEE80211_HT_CAP_SGI_40;
+		}
+
+		*pos++ = WLAN_EID_HT_CAPABILITY;
+		*pos++ = sizeof(struct ieee80211_ht_cap);
+		memset(pos, 0, sizeof(struct ieee80211_ht_cap));
+		tmp = cpu_to_le16(cap);
+		memcpy(pos, &tmp, sizeof(u16));
+		pos += sizeof(u16);
+		*pos++ = sband->ht_cap.ampdu_factor |
+			(sband->ht_cap.ampdu_density <<
+			 IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT);
+		memcpy(pos, &sband->ht_cap.mcs, sizeof(sband->ht_cap.mcs));
+		pos += sizeof(sband->ht_cap.mcs);
+		pos += 2 + 4 + 1; /* ext info, BF cap, antsel */
+	}
+
+	return pos;
+}
+
+void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
+			 struct ieee80211_supported_band *sband,
+			 struct ieee80211_channel *channel,
+			 enum ieee80211_smps_mode smps)
+{
+	struct ieee80211_ht_info *ht_info;
+	u8 *pos;
+	u32 flags = channel->flags;
+	u16 cap = sband->ht_cap.cap;
+	__le16 tmp;
+
+	if (!sband->ht_cap.ht_supported)
+		return;
+
+	if (!ht_info_ie)
+		return;
+
+	if (ht_info_ie[1] < sizeof(struct ieee80211_ht_info))
+		return;
+
+	ht_info = (struct ieee80211_ht_info *)(ht_info_ie + 2);
+
+	/* determine capability flags */
+
+	if (ieee80211_disable_40mhz_24ghz &&
+	    sband->band == IEEE80211_BAND_2GHZ) {
+		cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
+		cap &= ~IEEE80211_HT_CAP_SGI_40;
+	}
+
+	switch (ht_info->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
+	case IEEE80211_HT_PARAM_CHA_SEC_ABOVE:
+		if (flags & IEEE80211_CHAN_NO_HT40PLUS) {
+			cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
+			cap &= ~IEEE80211_HT_CAP_SGI_40;
+		}
+		break;
+	case IEEE80211_HT_PARAM_CHA_SEC_BELOW:
+		if (flags & IEEE80211_CHAN_NO_HT40MINUS) {
+			cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
+			cap &= ~IEEE80211_HT_CAP_SGI_40;
+		}
+		break;
+	}
+
+	/* set SM PS mode properly */
+	cap &= ~IEEE80211_HT_CAP_SM_PS;
+	switch (smps) {
+	case IEEE80211_SMPS_AUTOMATIC:
+	case IEEE80211_SMPS_NUM_MODES:
+		WARN_ON(1);
+	case IEEE80211_SMPS_OFF:
+		cap |= WLAN_HT_CAP_SM_PS_DISABLED <<
+			IEEE80211_HT_CAP_SM_PS_SHIFT;
+		break;
+	case IEEE80211_SMPS_STATIC:
+		cap |= WLAN_HT_CAP_SM_PS_STATIC <<
+			IEEE80211_HT_CAP_SM_PS_SHIFT;
+		break;
+	case IEEE80211_SMPS_DYNAMIC:
+		cap |= WLAN_HT_CAP_SM_PS_DYNAMIC <<
+			IEEE80211_HT_CAP_SM_PS_SHIFT;
+		break;
+	}
+
+	/* reserve and fill IE */
+
+	pos = skb_put(skb, sizeof(struct ieee80211_ht_cap) + 2);
+	*pos++ = WLAN_EID_HT_CAPABILITY;
+	*pos++ = sizeof(struct ieee80211_ht_cap);
+	memset(pos, 0, sizeof(struct ieee80211_ht_cap));
+
+	/* capability flags */
+	tmp = cpu_to_le16(cap);
+	memcpy(pos, &tmp, sizeof(u16));
+	pos += sizeof(u16);
+
+	/* AMPDU parameters */
+	*pos++ = sband->ht_cap.ampdu_factor |
+		 (sband->ht_cap.ampdu_density <<
+			IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT);
+
+	/* MCS set */
+	memcpy(pos, &sband->ht_cap.mcs, sizeof(sband->ht_cap.mcs));
+	pos += sizeof(sband->ht_cap.mcs);
+
+	/* extended capabilities */
+	pos += sizeof(__le16);
+
+	/* BF capabilities */
+	pos += sizeof(__le32);
+
+	/* antenna selection */
+	pos += sizeof(u8);
+}
diff --git a/net/mac80211/work.c b/net/mac80211/work.c
index 3dd0760..df6940d 100644
--- a/net/mac80211/work.c
+++ b/net/mac80211/work.c
@@ -101,102 +101,6 @@ static int ieee80211_compatible_rates(const u8 *supp_rates, int supp_rates_len,
 
 /* frame sending functions */
 
-static void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
-				struct ieee80211_supported_band *sband,
-				struct ieee80211_channel *channel,
-				enum ieee80211_smps_mode smps)
-{
-	struct ieee80211_ht_info *ht_info;
-	u8 *pos;
-	u32 flags = channel->flags;
-	u16 cap = sband->ht_cap.cap;
-	__le16 tmp;
-
-	if (!sband->ht_cap.ht_supported)
-		return;
-
-	if (!ht_info_ie)
-		return;
-
-	if (ht_info_ie[1] < sizeof(struct ieee80211_ht_info))
-		return;
-
-	ht_info = (struct ieee80211_ht_info *)(ht_info_ie + 2);
-
-	/* determine capability flags */
-
-	if (ieee80211_disable_40mhz_24ghz &&
-	    sband->band == IEEE80211_BAND_2GHZ) {
-		cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
-		cap &= ~IEEE80211_HT_CAP_SGI_40;
-	}
-
-	switch (ht_info->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
-	case IEEE80211_HT_PARAM_CHA_SEC_ABOVE:
-		if (flags & IEEE80211_CHAN_NO_HT40PLUS) {
-			cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
-			cap &= ~IEEE80211_HT_CAP_SGI_40;
-		}
-		break;
-	case IEEE80211_HT_PARAM_CHA_SEC_BELOW:
-		if (flags & IEEE80211_CHAN_NO_HT40MINUS) {
-			cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
-			cap &= ~IEEE80211_HT_CAP_SGI_40;
-		}
-		break;
-	}
-
-	/* set SM PS mode properly */
-	cap &= ~IEEE80211_HT_CAP_SM_PS;
-	switch (smps) {
-	case IEEE80211_SMPS_AUTOMATIC:
-	case IEEE80211_SMPS_NUM_MODES:
-		WARN_ON(1);
-	case IEEE80211_SMPS_OFF:
-		cap |= WLAN_HT_CAP_SM_PS_DISABLED <<
-			IEEE80211_HT_CAP_SM_PS_SHIFT;
-		break;
-	case IEEE80211_SMPS_STATIC:
-		cap |= WLAN_HT_CAP_SM_PS_STATIC <<
-			IEEE80211_HT_CAP_SM_PS_SHIFT;
-		break;
-	case IEEE80211_SMPS_DYNAMIC:
-		cap |= WLAN_HT_CAP_SM_PS_DYNAMIC <<
-			IEEE80211_HT_CAP_SM_PS_SHIFT;
-		break;
-	}
-
-	/* reserve and fill IE */
-
-	pos = skb_put(skb, sizeof(struct ieee80211_ht_cap) + 2);
-	*pos++ = WLAN_EID_HT_CAPABILITY;
-	*pos++ = sizeof(struct ieee80211_ht_cap);
-	memset(pos, 0, sizeof(struct ieee80211_ht_cap));
-
-	/* capability flags */
-	tmp = cpu_to_le16(cap);
-	memcpy(pos, &tmp, sizeof(u16));
-	pos += sizeof(u16);
-
-	/* AMPDU parameters */
-	*pos++ = sband->ht_cap.ampdu_factor |
-		 (sband->ht_cap.ampdu_density <<
-			IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT);
-
-	/* MCS set */
-	memcpy(pos, &sband->ht_cap.mcs, sizeof(sband->ht_cap.mcs));
-	pos += sizeof(sband->ht_cap.mcs);
-
-	/* extended capabilities */
-	pos += sizeof(__le16);
-
-	/* BF capabilities */
-	pos += sizeof(__le32);
-
-	/* antenna selection */
-	pos += sizeof(u8);
-}
-
 static void ieee80211_send_assoc(struct ieee80211_sub_if_data *sdata,
 				 struct ieee80211_work *wk)
 {
-- 
1.5.6.5


^ permalink raw reply related

* [PATCH] mac80211: Add HT IE to IBSS beacons and probe responses
From: Benoit Papillault @ 2010-05-12 21:07 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Benoit Papillault
In-Reply-To: <1273698460-18061-3-git-send-email-benoit.papillault@free.fr>

When an HT IBSS is configured, we add HT Capabilities and HT Operation
IE to beacons and probe responses. This is done according to
channel_type transmitted by iw/cfg80211.

Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
---
 net/mac80211/ibss.c        |   67 ++++++++++++++++++++++++++++++++++++++-----
 net/mac80211/ieee80211_i.h |   14 +++++++++
 net/mac80211/util.c        |   40 ++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 8 deletions(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index b2cc1fd..288f512 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -64,6 +64,7 @@ static void ieee80211_rx_mgmt_auth_ibss(struct ieee80211_sub_if_data *sdata,
 static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 				      const u8 *bssid, const int beacon_int,
 				      struct ieee80211_channel *chan,
+				      enum nl80211_channel_type channel_type,
 				      const u32 basic_rates,
 				      const u16 capability, u64 tsf)
 {
@@ -103,7 +104,7 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 	sdata->drop_unencrypted = capability & WLAN_CAPABILITY_PRIVACY ? 1 : 0;
 
 	local->oper_channel = chan;
-	WARN_ON(!ieee80211_set_channel_type(local, sdata, NL80211_CHAN_NO_HT));
+	WARN_ON(!ieee80211_set_channel_type(local, sdata, channel_type));
 	ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
 
 	sband = local->hw.wiphy->bands[chan->band];
@@ -118,7 +119,10 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 		*pos++ = basic | (u8) (rate / 5);
 	}
 
-	/* Build IBSS probe response */
+	/*
+	 * Build IBSS probe response template (this template is also used
+	 * when sending beacon, see ieee80211_beacon_get_tim())
+	 */
 	mgmt = (void *) skb_put(skb, 24 + sizeof(mgmt->u.beacon));
 	memset(mgmt, 0, 24 + sizeof(mgmt->u.beacon));
 	mgmt->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
@@ -165,6 +169,17 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 		memcpy(pos, &supp_rates[8], rates);
 	}
 
+	if (sband->ht_cap.ht_supported) {
+		pos = skb_put(skb, 2 + sizeof(struct ieee80211_ht_cap));
+		ieee80211_add_ht_cap(pos, sband);
+	}
+
+	if (channel_type != NL80211_CHAN_NO_HT &&
+	    sband->ht_cap.ht_supported) {
+		pos = skb_put(skb, 2 + sizeof(struct ieee80211_ht_info));
+		ieee80211_add_ht_info(pos, sband, chan, channel_type);
+	}
+
 	if (ifibss->ie_len)
 		memcpy(skb_put(skb, ifibss->ie_len),
 		       ifibss->ie, ifibss->ie_len);
@@ -202,6 +217,8 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 	u32 basic_rates;
 	int i, j;
 	u16 beacon_int = cbss->beacon_interval;
+	const u8 *ht_cap_ie, *ht_info_ie;
+	enum nl80211_channel_type channel_type = NL80211_CHAN_NO_HT;
 
 	if (beacon_int < 10)
 		beacon_int = 10;
@@ -223,9 +240,27 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 		}
 	}
 
+	/* parse HT Capabilities & Information IE, if present */
+	ht_cap_ie = ieee80211_bss_get_ie(cbss, WLAN_EID_HT_CAPABILITY);
+	ht_info_ie = ieee80211_bss_get_ie(cbss, WLAN_EID_HT_INFORMATION);
+	if (ht_cap_ie && ht_info_ie) {
+		struct ieee80211_ht_cap *ht_cap;
+		struct ieee80211_ht_info *ht_info;
+		struct ieee80211_sta_ht_cap sta_ht_cap;
+
+		ht_cap = (struct ieee80211_ht_cap *)(ht_cap_ie + 2);
+		ht_info = (struct ieee80211_ht_info *)(ht_info_ie + 2);
+
+		ieee80211_ht_cap_ie_to_sta_ht_cap(sband, ht_cap, &sta_ht_cap);
+
+		channel_type = ieee80211_channel_type_from_ht_info(
+					sdata->local, ht_info, sta_ht_cap.cap);
+	}
+
 	__ieee80211_sta_join_ibss(sdata, cbss->bssid,
 				  beacon_int,
 				  cbss->channel,
+				  channel_type,
 				  basic_rates,
 				  cbss->capability,
 				  cbss->tsf);
@@ -529,7 +564,8 @@ static void ieee80211_sta_create_ibss(struct ieee80211_sub_if_data *sdata)
 		sdata->drop_unencrypted = 0;
 
 	__ieee80211_sta_join_ibss(sdata, bssid, sdata->vif.bss_conf.beacon_int,
-				  ifibss->channel, 3, /* first two are basic */
+				  ifibss->channel, ifibss->channel_type,
+				  3, /* first two are basic */
 				  capability, 0);
 }
 
@@ -906,13 +942,14 @@ int ieee80211_ibss_join(struct ieee80211_sub_if_data *sdata,
 	sdata->vif.bss_conf.beacon_int = params->beacon_interval;
 
 	sdata->u.ibss.channel = params->channel;
+	sdata->u.ibss.channel_type = params->channel_type;
 	sdata->u.ibss.fixed_channel = params->channel_fixed;
 
 	/* fix ourselves to that channel now already */
 	if (params->channel_fixed) {
 		sdata->local->oper_channel = params->channel;
 		WARN_ON(!ieee80211_set_channel_type(sdata->local, sdata,
-						    NL80211_CHAN_NO_HT));
+						    params->channel_type));
 	}
 
 	if (params->ie) {
@@ -922,11 +959,25 @@ int ieee80211_ibss_join(struct ieee80211_sub_if_data *sdata,
 			sdata->u.ibss.ie_len = params->ie_len;
 	}
 
+	/*
+	 * Allocate IBSS probe response template (see
+	 * __ieee80211_sta_join_ibss for the needed size). According to IEEE
+	 * 802.11-2007 10.4.4.2, there is only 20 possibles values. We
+	 * support up IEEE80211_MAX_SUPP_RATES (currently 32) : so 8 for
+	 * Supported Rates and IEEE80211_MAX_SUPP_RATES-8 for Extended
+	 * Supported Rates
+	 */
+
 	skb = dev_alloc_skb(sdata->local->hw.extra_tx_headroom +
-			    36 /* bitrates */ +
-			    34 /* SSID */ +
-			    3  /* DS params */ +
-			    4  /* IBSS params */ +
+			    sizeof(struct ieee80211_hdr_3addr) +
+			    12 /* struct ieee80211_mgmt.u.beacon */ +
+			    2 + IEEE80211_MAX_SSID_LEN /* max SSID */ +
+			    2 + 8 /* max Supported Rates */ +
+			    3 /* max DS params */ +
+			    4 /* IBSS params */ +
+			    2 + IEEE80211_MAX_SUPP_RATES-8 /* max Ext Rates */ +
+			    2 + sizeof(struct ieee80211_ht_cap) +
+			    2 + sizeof(struct ieee80211_ht_info) +
 			    params->ie_len);
 	if (!skb)
 		return -ENOMEM;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 742e12b..78c4716 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -404,6 +404,7 @@ struct ieee80211_if_ibss {
 	u8 ssid_len, ie_len;
 	u8 *ie;
 	struct ieee80211_channel *channel;
+	enum nl80211_channel_type channel_type;
 
 	unsigned long ibss_join_req;
 	/* probe response/beacon for IBSS */
@@ -1193,6 +1194,19 @@ void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
 			 u16 transaction, u16 auth_alg,
 			 u8 *extra, size_t extra_len, const u8 *bssid,
 			 const u8 *key, u8 key_len, u8 key_idx);
+
+u8 *ieee80211_add_ht_cap(u8 *pos,
+			 struct ieee80211_supported_band *sband);
+
+u8 *ieee80211_add_ht_info(u8 *pos,
+			  struct ieee80211_supported_band *sband,
+			  struct ieee80211_channel *channel,
+			  enum nl80211_channel_type channel_type);
+
+enum nl80211_channel_type ieee80211_channel_type_from_ht_info(
+	struct ieee80211_local *local,
+	struct ieee80211_ht_info *hti, u16 ap_ht_cap_flags);
+
 int ieee80211_build_preq_ies(struct ieee80211_local *local, u8 *buffer,
 			     const u8 *ie, size_t ie_len,
 			     enum ieee80211_band band);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 047acb8..78c43b8 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -898,6 +898,46 @@ void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
 	ieee80211_tx_skb(sdata, skb);
 }
 
+u8 *ieee80211_add_ht_info(u8 *pos,
+			  struct ieee80211_supported_band *sband,
+			  struct ieee80211_channel *channel,
+			  enum nl80211_channel_type channel_type)
+{
+	struct ieee80211_ht_info ht_info;
+
+	ht_info.control_chan =
+		ieee80211_frequency_to_channel(channel->center_freq);
+	ht_info.ht_param = 0;
+	switch (channel_type) {
+	case NL80211_CHAN_NO_HT: /* to make compiler happy */
+	case NL80211_CHAN_HT20:
+		ht_info.ht_param |= IEEE80211_HT_PARAM_CHA_SEC_NONE;
+		break;
+	case NL80211_CHAN_HT40MINUS:
+		ht_info.ht_param |= IEEE80211_HT_PARAM_CHA_SEC_BELOW;
+		break;
+	case NL80211_CHAN_HT40PLUS:
+		ht_info.ht_param |= IEEE80211_HT_PARAM_CHA_SEC_ABOVE;
+		break;
+	}
+	if (sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40)
+		ht_info.ht_param |= IEEE80211_HT_PARAM_CHAN_WIDTH_ANY;
+	ht_info.operation_mode = cpu_to_le16(0);
+	ht_info.stbc_param = cpu_to_le16(0);
+	/* It seems that Basic MCS set and Supported MCS set are identical
+	 * for the first 10 bytes */
+	memset(&ht_info.basic_set, 0, 16);
+	memcpy(&ht_info.basic_set, &sband->ht_cap.mcs, 10);
+
+	/* HT Information element */
+	*pos++ = WLAN_EID_HT_INFORMATION;
+	*pos++ = sizeof(struct ieee80211_ht_info);
+	memcpy(pos, &ht_info, sizeof(struct ieee80211_ht_info));
+	pos += sizeof(struct ieee80211_ht_info);
+
+	return pos;
+}
+
 int ieee80211_build_preq_ies(struct ieee80211_local *local, u8 *buffer,
 			     const u8 *ie, size_t ie_len,
 			     enum ieee80211_band band)
-- 
1.5.6.5


^ permalink raw reply related

* Re: [PATCH v2] mac80211: Add HT IE to IBSS beacons and probe responses.
From: Bruno Randolf @ 2010-05-13  0:47 UTC (permalink / raw)
  To: Benoit Papillault; +Cc: Johannes Berg, linux-wireless
In-Reply-To: <4BEB175F.7000001@free.fr>

On Thursday 13 May 2010 06:02:23 Benoit Papillault wrote:
> > Also I just noticed that there's a TODO item in rx.c when we receive an
> > HT frame from a peer we don't know about yet. Not sure what to do there,
> > but you'll need to look at it.
> 
> I did some patch in this area in my tree. Basically, the peer STA is
> created only on beacon/probe response since only those frames contains
> peer capabilities. Any frames received before is simply ignored.

i think the same applies to non-HT. when we receive a frame from a STA we 
haven't seen a beacon from yet, we just mark the rate at which we received the 
frame and can use that for a reply. later, when we receive a beacon, the rate-
set is updated. i guess the same can be done for HT and i'd argue that it 
should be done like this in order to be able to communicate even though we 
have not received a beacon from that particular STA yet - there are some 
reasons why the beacon might not have reached us: 

 * the STA might have deferred beacon sending for a few intervals as part of 
the normal beacon backoff

 * the beacon might have been lost due to interference

 * or the other STA might be a buggy implementation which doesn't send IBSS 
beacons for a while, like some madwifi versions.

in any case the ability to communicate is more important than a complete rate-
set...

bruno

^ permalink raw reply

* Re: [PATCH] cfg80211: Parse channel_type in NL80211_CMD_JOIN_IBSS
From: Bruno Randolf @ 2010-05-13  0:56 UTC (permalink / raw)
  To: Benoit Papillault; +Cc: johannes, linux-wireless
In-Reply-To: <1273698460-18061-1-git-send-email-benoit.papillault@free.fr>

On Thursday 13 May 2010 06:07:37 Benoit Papillault wrote:
> The channel_type field is filled by iw and parse by cfg80211 before
> being sent to mac80211 for instance (we also check if the specified
> channel is capable of ht40+, ht40- or ht20). This information is needed
> to properly configure an HT IBSS.
> 
> It requires a patch to iw to fill this field.
> 
> Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
> ---
>  include/net/cfg80211.h |    1 +
>  net/wireless/nl80211.c |   20 ++++++++++++++++----
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index b44a2e5..5ead439 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -806,6 +806,7 @@ struct cfg80211_ibss_params {
>  	u8 *ssid;
>  	u8 *bssid;
>  	struct ieee80211_channel *channel;
> +	enum nl80211_channel_type channel_type;
>  	u8 *ie;
>  	u8 ssid_len, ie_len;
>  	u16 beacon_interval;
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index aaa1aad..039c13f 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -3879,6 +3879,7 @@ static int nl80211_join_ibss(struct sk_buff *skb,
> struct genl_info *info) struct cfg80211_ibss_params ibss;
>  	struct wiphy *wiphy;
>  	struct cfg80211_cached_keys *connkeys = NULL;
> +	enum nl80211_channel_type channel_type = NL80211_CHAN_NO_HT;
>  	int err;
> 
>  	memset(&ibss, 0, sizeof(ibss));
> @@ -3900,6 +3901,17 @@ static int nl80211_join_ibss(struct sk_buff *skb,
> struct genl_info *info) return -EINVAL;
>  	}
> 
> +	if (info->attrs[NL80211_ATTR_WIPHY_CHANNEL_TYPE]) {
> +		channel_type = nla_get_u32(
> +			info->attrs[NL80211_ATTR_WIPHY_CHANNEL_TYPE]);
> +		if (channel_type != NL80211_CHAN_NO_HT &&
> +		    channel_type != NL80211_CHAN_HT20 &&
> +		    channel_type != NL80211_CHAN_HT40PLUS &&
> +		    channel_type != NL80211_CHAN_HT40MINUS)
> +			return -EINVAL;
> +	}
> +	ibss.channel_type = channel_type;
> +
>  	rtnl_lock();
> 
>  	err = get_rdev_dev_by_info_ifindex(info, &rdev, &dev);
> @@ -3933,11 +3945,11 @@ static int nl80211_join_ibss(struct sk_buff *skb,
> struct genl_info *info) ibss.ie_len =
> nla_len(info->attrs[NL80211_ATTR_IE]);
>  	}
> 
> -	ibss.channel = ieee80211_get_channel(wiphy,
> -		nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ]));
> +	ibss.channel = rdev_freq_to_chan(rdev,
> +		nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ]),
> +		channel_type);
>  	if (!ibss.channel ||
> -	    ibss.channel->flags & IEEE80211_CHAN_NO_IBSS ||
> -	    ibss.channel->flags & IEEE80211_CHAN_DISABLED) {
> +	    ibss.channel->flags & IEEE80211_CHAN_NO_IBSS) {
>  		err = -EINVAL;
>  		goto out;
>  	}

do you think this could also be used to configure "pure G" channels? remember 
the discussion we had about the basic rate set in IBSS? if we could use this 
to configure the channel as "pure G" we could use a basic rate set with higher 
rates (ERP mandatory rates: 1, 2, 5.5, 11, 6, 12, 24). i still need this 
feature in order to configure higher-thruput G-only IBSS...

bruno

^ permalink raw reply

* Re: [PATCH 2/2] compat-wireless: make patches apply again
From: Bruno Randolf @ 2010-05-13  1:00 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: Hauke Mehrtens, linux-wireless, mcgrof
In-Reply-To: <AANLkTinkg1lG6Zc-9CfxABp0DVpTsym1YFKlcFCHTMTE@mail.gmail.com>

On Thursday 13 May 2010 01:28:18 Luis R. Rodriguez wrote:
> On Wed, May 12, 2010 at 1:20 AM, Bruno Randolf <br1@einfach.org> wrote:
> > On Tuesday 11 May 2010 06:24:10 Hauke Mehrtens wrote:
> >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> >> ---
> >>  patches/07-change-default-rate-alg.patch |    4 ++--
> >>  1 files changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/patches/07-change-default-rate-alg.patch
> >> b/patches/07-change-default-rate-alg.patch index af65207..f0ccbce 100644
> >> --- a/patches/07-change-default-rate-alg.patch
> >> +++ b/patches/07-change-default-rate-alg.patch
> >> @@ -21,7 +21,7 @@ at compilation time.
> >>   module_param(ieee80211_default_rc_algo, charp, 0644);
> >>   MODULE_PARM_DESC(ieee80211_default_rc_algo,
> >>                "Default rate control algorithm for mac80211 to use");
> >> -@@ -118,8 +118,8 @@ ieee80211_rate_control_ops_get(const cha
> >> +@@ -119,8 +119,8 @@ ieee80211_rate_control_ops_get(const cha
> >>               ops =
> > 
> > ieee80211_try_rate_control_ops_get(ieee80211_default_rc_algo);
> > 
> >>       /* try built-in one if specific alg requested but not found */
> >> @@ -29,6 +29,6 @@ at compilation time.
> >>  -            ops =
> > 
> > ieee80211_try_rate_control_ops_get(CONFIG_MAC80211_RC_DEFAULT);
> > 
> >>  +    if (!ops && strlen(CONFIG_COMPAT_MAC80211_RC_DEFAULT))
> >>  +            ops =
> >> ieee80211_try_rate_control_ops_get(CONFIG_COMPAT_MAC80211_RC_DEFAULT); +
> >>       kparam_unblock_sysfs_write(ieee80211_default_rc_algo);
> >> 
> >>       return ops;
> >> - }
> > 
> > this is somehow messed up and doesn't apply. here is a fixed version of
> > the patch (not a patch for a patch)...
> 
> Odd, it applied, here with git am and all. Hm, oh well.

sorry, i wasn't clear about it. the patch from the mail obviously applied and 
i got it by git pull. but the resulting patch "07-change-default-rate-
alg.patch" does not apply any more, so i sent a fixed up version of that file.

bruno

^ permalink raw reply

* Re: [PATCH 2/2] compat-wireless: make patches apply again
From: Luis R. Rodriguez @ 2010-05-13  1:06 UTC (permalink / raw)
  To: Bruno Randolf
  Cc: Luis Rodriguez, Hauke Mehrtens, linux-wireless@vger.kernel.org,
	mcgrof@infradead.org
In-Reply-To: <201005131000.57369.br1@einfach.org>

On Wed, May 12, 2010 at 06:00:57PM -0700, Bruno Randolf wrote:
> On Thursday 13 May 2010 01:28:18 Luis R. Rodriguez wrote:
> > On Wed, May 12, 2010 at 1:20 AM, Bruno Randolf <br1@einfach.org> wrote:
> > > On Tuesday 11 May 2010 06:24:10 Hauke Mehrtens wrote:
> > >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> > >> ---
> > >>  patches/07-change-default-rate-alg.patch |    4 ++--
> > >>  1 files changed, 2 insertions(+), 2 deletions(-)
> > >> 
> > >> diff --git a/patches/07-change-default-rate-alg.patch
> > >> b/patches/07-change-default-rate-alg.patch index af65207..f0ccbce 100644
> > >> --- a/patches/07-change-default-rate-alg.patch
> > >> +++ b/patches/07-change-default-rate-alg.patch
> > >> @@ -21,7 +21,7 @@ at compilation time.
> > >>   module_param(ieee80211_default_rc_algo, charp, 0644);
> > >>   MODULE_PARM_DESC(ieee80211_default_rc_algo,
> > >>                "Default rate control algorithm for mac80211 to use");
> > >> -@@ -118,8 +118,8 @@ ieee80211_rate_control_ops_get(const cha
> > >> +@@ -119,8 +119,8 @@ ieee80211_rate_control_ops_get(const cha
> > >>               ops =
> > > 
> > > ieee80211_try_rate_control_ops_get(ieee80211_default_rc_algo);
> > > 
> > >>       /* try built-in one if specific alg requested but not found */
> > >> @@ -29,6 +29,6 @@ at compilation time.
> > >>  -            ops =
> > > 
> > > ieee80211_try_rate_control_ops_get(CONFIG_MAC80211_RC_DEFAULT);
> > > 
> > >>  +    if (!ops && strlen(CONFIG_COMPAT_MAC80211_RC_DEFAULT))
> > >>  +            ops =
> > >> ieee80211_try_rate_control_ops_get(CONFIG_COMPAT_MAC80211_RC_DEFAULT); +
> > >>       kparam_unblock_sysfs_write(ieee80211_default_rc_algo);
> > >> 
> > >>       return ops;
> > >> - }
> > > 
> > > this is somehow messed up and doesn't apply. here is a fixed version of
> > > the patch (not a patch for a patch)...
> > 
> > Odd, it applied, here with git am and all. Hm, oh well.
> 
> sorry, i wasn't clear about it. the patch from the mail obviously applied and 
> i got it by git pull. but the resulting patch "07-change-default-rate-
> alg.patch" does not apply any more, so i sent a fixed up version of that file.

Sorry I'm still a little lost still, can you post any pending patch on a
new e-mail ?

  Luis

^ permalink raw reply

* [PATCH 0/2] ath9k: Add AR9003 2.2 support
From: Luis R. Rodriguez @ 2010-05-13  1:15 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Luis R. Rodriguez

These two patches address AR9300 2.2 support, the second
one just renames the 2.0 initvals file.

Luis R. Rodriguez (2):
  ath9k_hw: add support for the AR9003 2.2
  ath9k_hw: rename the ar9003_initvals.h to ar9003_2p0_initvals.h

 .../{ar9003_initvals.h => ar9003_2p0_initvals.h}   |    6 +-
 .../{ar9003_initvals.h => ar9003_2p2_initvals.h}   |  185 ++++++++++----------
 drivers/net/wireless/ath/ath9k/ar9003_hw.c         |  163 +++++++++++++++---
 3 files changed, 233 insertions(+), 121 deletions(-)
 copy drivers/net/wireless/ath/ath9k/{ar9003_initvals.h => ar9003_2p0_initvals.h} (99%)
 rename drivers/net/wireless/ath/ath9k/{ar9003_initvals.h => ar9003_2p2_initvals.h} (91%)


^ permalink raw reply

* [PATCH 2/2] ath9k_hw: rename the ar9003_initvals.h to ar9003_2p0_initvals.h
From: Luis R. Rodriguez @ 2010-05-13  1:15 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Luis R. Rodriguez
In-Reply-To: <1273713306-13789-1-git-send-email-lrodriguez@atheros.com>

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 .../{ar9003_initvals.h => ar9003_2p0_initvals.h}   |    6 +++---
 drivers/net/wireless/ath/ath9k/ar9003_hw.c         |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
 rename drivers/net/wireless/ath/ath9k/{ar9003_initvals.h => ar9003_2p0_initvals.h} (99%)

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_initvals.h b/drivers/net/wireless/ath/ath9k/ar9003_2p0_initvals.h
similarity index 99%
rename from drivers/net/wireless/ath/ath9k/ar9003_initvals.h
rename to drivers/net/wireless/ath/ath9k/ar9003_2p0_initvals.h
index db019dd..f82a00d 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_initvals.h
+++ b/drivers/net/wireless/ath/ath9k/ar9003_2p0_initvals.h
@@ -14,8 +14,8 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-#ifndef INITVALS_9003_H
-#define INITVALS_9003_H
+#ifndef INITVALS_9003_2P0_H
+#define INITVALS_9003_2P0_H
 
 /* AR9003 2.0 */
 
@@ -1781,4 +1781,4 @@ static const u32 ar9300PciePhy_clkreq_disable_L1_2p0[][2] = {
 	{0x00004044, 0x00000000},
 };
 
-#endif /* INITVALS_9003_H */
+#endif /* INITVALS_9003_2P0_H */
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_hw.c b/drivers/net/wireless/ath/ath9k/ar9003_hw.c
index 3c4a446..863f61e 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_hw.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_hw.c
@@ -16,7 +16,7 @@
 
 #include "hw.h"
 #include "ar9003_mac.h"
-#include "ar9003_initvals.h"
+#include "ar9003_2p0_initvals.h"
 #include "ar9003_2p2_initvals.h"
 
 /* General hardware code for the AR9003 hadware family */
-- 
1.6.3.3


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox