2013-05-01 15:17:33

by Gabor Juhos

[permalink] [raw]
Subject: [PATCH 0/7] rt2x00: avoid a few superfluous pointer dereferences

Gabor Juhos (7):
rt2x00: rt2x00dev: use rt2x00dev->tx->limit
rt2x00: rt61pci: use rt2x00dev->tx->limit
rt2x00: rt2800pci: use rt2x00dev->tx->limit
rt2x00: rt2800usb: use rt2x00dev->rx->limit
rt2x00: rt2800lib: use rt2x00dev->bcn->winfo_size
rt2x00: rt2x00dev: defer operational mode detection
rt2x00: rt2x00dev: use rt2x00dev->bcn->limit

drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
drivers/net/wireless/rt2x00/rt2800pci.c | 2 +-
drivers/net/wireless/rt2x00/rt2800usb.c | 2 +-
drivers/net/wireless/rt2x00/rt2x00dev.c | 36 +++++++++++++++----------------
drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
5 files changed, 22 insertions(+), 22 deletions(-)

--
1.7.10



2013-05-01 15:17:36

by Gabor Juhos

[permalink] [raw]
Subject: [PATCH 7/7] rt2x00: rt2x00dev: use rt2x00dev->bcn->limit

The beacon data queue is initialized already,
so fetch the number of the queue entries from
that instead of using the entry_num field of
the data queue descriptor.

The two values are the same, and the use of the
rt2x00dev->bcn->limit value allows us to get rid
of a superfluous pointer dereference.

Signed-off-by: Gabor Juhos <[email protected]>
---
drivers/net/wireless/rt2x00/rt2x00dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index 6a20172..dff5012 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -1336,7 +1336,7 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
* beacon entries.
*/
rt2x00dev->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION);
- if (rt2x00dev->ops->bcn->entry_num > 0)
+ if (rt2x00dev->bcn->limit > 0)
rt2x00dev->hw->wiphy->interface_modes |=
BIT(NL80211_IFTYPE_ADHOC) |
BIT(NL80211_IFTYPE_AP) |
--
1.7.10


2013-05-01 15:17:36

by Gabor Juhos

[permalink] [raw]
Subject: [PATCH 5/7] rt2x00: rt2800lib: use rt2x00dev->bcn->winfo_size

The beacon data queue is initialized already when
the rt2800_clear_beacon_register() function is called.

Fetch the size of the TXWI descriptor from that
instead of using the winfo_size field of the data
queue descriptor.

The two values are the same, and the use of the
rt2x00dev->bcn->winfo_size value allows us to
get rid of a superfluous pointer dereference.

Signed-off-by: Gabor Juhos <[email protected]>
---
drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index b52d70c..bddfd6d 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -840,7 +840,7 @@ static inline void rt2800_clear_beacon_register(struct rt2x00_dev *rt2x00dev,
unsigned int beacon_base)
{
int i;
- const int txwi_desc_size = rt2x00dev->ops->bcn->winfo_size;
+ const int txwi_desc_size = rt2x00dev->bcn->winfo_size;

/*
* For the Beacon base registers we only need to clear
--
1.7.10


2013-05-01 20:06:37

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 3/7] rt2x00: rt2800pci: use rt2x00dev->tx->limit

On Wed, May 1, 2013 at 5:17 PM, Gabor Juhos <[email protected]> wrote:
> The TX data queue is initialized already when
> the rt2800pci_txstatus_interrupt() function is
> called.
>
> Fetch the number of the queue entries from that
> instead of using the entry_num field of the data
> queue descriptor.
>
> The two values are the same, and the use of the
> rt2x00dev->tx->limit value allows us to get rid
> of a superfluous pointer dereference.
>
> Signed-off-by: Gabor Juhos <[email protected]>

Acked-by: Gertjan van Wingerde <[email protected]>

> ---
> drivers/net/wireless/rt2x00/rt2800pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> index 6f4a861..330f1d2 100644
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> @@ -1014,7 +1014,7 @@ static void rt2800pci_txstatus_interrupt(struct rt2x00_dev *rt2x00dev)
> * Since we have only one producer and one consumer we don't
> * need to lock the kfifo.
> */
> - for (i = 0; i < rt2x00dev->ops->tx->entry_num; i++) {
> + for (i = 0; i < rt2x00dev->tx->limit; i++) {
> rt2x00mmio_register_read(rt2x00dev, TX_STA_FIFO, &status);
>
> if (!rt2x00_get_field32(status, TX_STA_FIFO_VALID))
> --
> 1.7.10
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
---
Gertjan

2013-05-01 20:08:37

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 7/7] rt2x00: rt2x00dev: use rt2x00dev->bcn->limit

On Wed, May 1, 2013 at 5:17 PM, Gabor Juhos <[email protected]> wrote:
> The beacon data queue is initialized already,
> so fetch the number of the queue entries from
> that instead of using the entry_num field of
> the data queue descriptor.
>
> The two values are the same, and the use of the
> rt2x00dev->bcn->limit value allows us to get rid
> of a superfluous pointer dereference.
>
> Signed-off-by: Gabor Juhos <[email protected]>

Acked-by: Gertjan van Wingerde <[email protected]>

> ---
> drivers/net/wireless/rt2x00/rt2x00dev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> index 6a20172..dff5012 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> @@ -1336,7 +1336,7 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
> * beacon entries.
> */
> rt2x00dev->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION);
> - if (rt2x00dev->ops->bcn->entry_num > 0)
> + if (rt2x00dev->bcn->limit > 0)
> rt2x00dev->hw->wiphy->interface_modes |=
> BIT(NL80211_IFTYPE_ADHOC) |
> BIT(NL80211_IFTYPE_AP) |
> --
> 1.7.10
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
---
Gertjan

2013-05-02 07:35:50

by Gabor Juhos

[permalink] [raw]
Subject: Re: [PATCH 7/7] rt2x00: rt2x00dev: use rt2x00dev->bcn->limit

2013.05.01. 22:08 keltez?ssel, Gertjan van Wingerde ?rta:
> On Wed, May 1, 2013 at 5:17 PM, Gabor Juhos <[email protected]> wrote:
>> The beacon data queue is initialized already,
>> so fetch the number of the queue entries from
>> that instead of using the entry_num field of
>> the data queue descriptor.
>>
>> The two values are the same, and the use of the
>> rt2x00dev->bcn->limit value allows us to get rid
>> of a superfluous pointer dereference.
>>
>> Signed-off-by: Gabor Juhos <[email protected]>
>
> Acked-by: Gertjan van Wingerde <[email protected]>

John, please ignore the last patch. This depends on other patches which were not
included in the series. Without those it causes the following warning:

[ 11.990000] WARNING: at net/wireless/core.c:476 wiphy_register+0x570/0x680()
[ 12.000000] Modules linked in: rt73usb(+) rt61pci rt2800lib rt2500usb
rt2500pci rt2400pci rt2x00usb rt2x00pci rt2x00mmio r
t2x00lib crc_itu_t leds_gpio gpio_keys_polled input_polldev input_core
[ 12.040000] Call Trace:
[ 12.050000] [<80305288>] dump_stack+0x8/0x34
[ 12.050000] [<80025498>] warn_slowpath_common+0x78/0xa8
[ 12.060000] [<800254e0>] warn_slowpath_null+0x18/0x24
[ 12.070000] [<802a156c>] wiphy_register+0x570/0x680
[ 12.080000] [<802c71bc>] ieee80211_register_hw+0x450/0x8a4
[ 12.090000] [<c017b03c>] rt2x00lib_probe_dev+0x7a0/0x90c [rt2x00lib]
[ 12.110000] [<c019e320>] rt2x00usb_probe+0x15c/0x4b8 [rt2x00usb]
[ 12.120000] [<801d1f74>] usb_probe_interface+0xf8/0x234
[ 12.130000] [<8019f810>] driver_probe_device+0xa0/0x2bc
[ 12.140000] [<8019fae8>] __driver_attach+0xbc/0xc4
[ 12.150000] [<8019dcf4>] bus_for_each_dev+0x5c/0xa8
[ 12.160000] [<8019edec>] bus_add_driver+0x110/0x278
[ 12.170000] [<8019fe44>] driver_register+0x90/0x1a0
[ 12.180000] [<801d1480>] usb_register_driver+0x84/0x170
[ 12.190000] [<80004570>] do_one_initcall+0x160/0x1f0
[ 12.200000] [<800643e8>] load_module+0x17f0/0x1f54
[ 12.210000] [<80064c00>] sys_init_module+0xb4/0xec
[ 12.220000] [<8000de70>] stack_done+0x20/0x40
[ 12.230000]
[ 12.230000] ---[ end trace b1452c251acfb9c2 ]---

I will resend this as a part of another patch-set. Sorry for the inconvenience.

Thanks,
Gabor

2013-05-01 20:07:36

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 5/7] rt2x00: rt2800lib: use rt2x00dev->bcn->winfo_size

On Wed, May 1, 2013 at 5:17 PM, Gabor Juhos <[email protected]> wrote:
> The beacon data queue is initialized already when
> the rt2800_clear_beacon_register() function is called.
>
> Fetch the size of the TXWI descriptor from that
> instead of using the winfo_size field of the data
> queue descriptor.
>
> The two values are the same, and the use of the
> rt2x00dev->bcn->winfo_size value allows us to
> get rid of a superfluous pointer dereference.
>
> Signed-off-by: Gabor Juhos <[email protected]>

Acked-by: Gertjan van Wingerde <[email protected]>

> ---
> drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index b52d70c..bddfd6d 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -840,7 +840,7 @@ static inline void rt2800_clear_beacon_register(struct rt2x00_dev *rt2x00dev,
> unsigned int beacon_base)
> {
> int i;
> - const int txwi_desc_size = rt2x00dev->ops->bcn->winfo_size;
> + const int txwi_desc_size = rt2x00dev->bcn->winfo_size;
>
> /*
> * For the Beacon base registers we only need to clear
> --
> 1.7.10
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
---
Gertjan

2013-05-01 15:17:34

by Gabor Juhos

[permalink] [raw]
Subject: [PATCH 3/7] rt2x00: rt2800pci: use rt2x00dev->tx->limit

The TX data queue is initialized already when
the rt2800pci_txstatus_interrupt() function is
called.

Fetch the number of the queue entries from that
instead of using the entry_num field of the data
queue descriptor.

The two values are the same, and the use of the
rt2x00dev->tx->limit value allows us to get rid
of a superfluous pointer dereference.

Signed-off-by: Gabor Juhos <[email protected]>
---
drivers/net/wireless/rt2x00/rt2800pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 6f4a861..330f1d2 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -1014,7 +1014,7 @@ static void rt2800pci_txstatus_interrupt(struct rt2x00_dev *rt2x00dev)
* Since we have only one producer and one consumer we don't
* need to lock the kfifo.
*/
- for (i = 0; i < rt2x00dev->ops->tx->entry_num; i++) {
+ for (i = 0; i < rt2x00dev->tx->limit; i++) {
rt2x00mmio_register_read(rt2x00dev, TX_STA_FIFO, &status);

if (!rt2x00_get_field32(status, TX_STA_FIFO_VALID))
--
1.7.10


2013-05-01 15:17:34

by Gabor Juhos

[permalink] [raw]
Subject: [PATCH 2/7] rt2x00: rt61pci: use rt2x00dev->tx->limit

The TX data queue is initialized already when
the rt61pci_txdone() function is called.

Fetch the number of the queue entries from that
instead of using the entry_num field of the data
queue descriptor.

The two values are the same, and the use of the
rt2x00dev->tx->limit value allows us to get rid
of a superfluous pointer dereference.

Signed-off-by: Gabor Juhos <[email protected]>
---
drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index 0dc8180..7e1759b 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -2175,7 +2175,7 @@ static void rt61pci_txdone(struct rt2x00_dev *rt2x00dev)
* that the TX_STA_FIFO stack has a size of 16. We stick to our
* tx ring size for now.
*/
- for (i = 0; i < rt2x00dev->ops->tx->entry_num; i++) {
+ for (i = 0; i < rt2x00dev->tx->limit; i++) {
rt2x00mmio_register_read(rt2x00dev, STA_CSR4, &reg);
if (!rt2x00_get_field32(reg, STA_CSR4_VALID))
break;
--
1.7.10


2013-05-01 20:10:14

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 0/7] rt2x00: avoid a few superfluous pointer dereferences

Hi Gabor,


On Wed, May 1, 2013 at 5:17 PM, Gabor Juhos <[email protected]> wrote:
> Gabor Juhos (7):
> rt2x00: rt2x00dev: use rt2x00dev->tx->limit
> rt2x00: rt61pci: use rt2x00dev->tx->limit
> rt2x00: rt2800pci: use rt2x00dev->tx->limit
> rt2x00: rt2800usb: use rt2x00dev->rx->limit
> rt2x00: rt2800lib: use rt2x00dev->bcn->winfo_size
> rt2x00: rt2x00dev: defer operational mode detection
> rt2x00: rt2x00dev: use rt2x00dev->bcn->limit
>
> drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
> drivers/net/wireless/rt2x00/rt2800pci.c | 2 +-
> drivers/net/wireless/rt2x00/rt2800usb.c | 2 +-
> drivers/net/wireless/rt2x00/rt2x00dev.c | 36 +++++++++++++++----------------
> drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
> 5 files changed, 22 insertions(+), 22 deletions(-)
>

Thanks for these. Although they are micro optimizations, I guess it is
cleaner to use
the actual queue data rather than the queue description data.

I've acked all patches, but I'm afraid they may have come just too
late to be included in
the 3.10 timeframe, but I leave it up to John to decide.

---
Gertjan

2013-05-01 20:06:07

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 2/7] rt2x00: rt61pci: use rt2x00dev->tx->limit

On Wed, May 1, 2013 at 5:17 PM, Gabor Juhos <[email protected]> wrote:
> The TX data queue is initialized already when
> the rt61pci_txdone() function is called.
>
> Fetch the number of the queue entries from that
> instead of using the entry_num field of the data
> queue descriptor.
>
> The two values are the same, and the use of the
> rt2x00dev->tx->limit value allows us to get rid
> of a superfluous pointer dereference.
>
> Signed-off-by: Gabor Juhos <[email protected]>

Acked-by: Gertjan van Wingerde <[email protected]>

> ---
> drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
> index 0dc8180..7e1759b 100644
> --- a/drivers/net/wireless/rt2x00/rt61pci.c
> +++ b/drivers/net/wireless/rt2x00/rt61pci.c
> @@ -2175,7 +2175,7 @@ static void rt61pci_txdone(struct rt2x00_dev *rt2x00dev)
> * that the TX_STA_FIFO stack has a size of 16. We stick to our
> * tx ring size for now.
> */
> - for (i = 0; i < rt2x00dev->ops->tx->entry_num; i++) {
> + for (i = 0; i < rt2x00dev->tx->limit; i++) {
> rt2x00mmio_register_read(rt2x00dev, STA_CSR4, &reg);
> if (!rt2x00_get_field32(reg, STA_CSR4_VALID))
> break;
> --
> 1.7.10
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
---
Gertjan

2013-05-01 15:17:34

by Gabor Juhos

[permalink] [raw]
Subject: [PATCH 4/7] rt2x00: rt2800usb: use rt2x00dev->rx->limit

The RX data queue is initialized already when
the rt2800_usb_enable_radio() function is called.

Fetch the number of the queue entries from that
instead of using the entry_num field of the data
queue descriptor.

The two values are the same, and the use of the
rt2x00dev->rx->limit value allows us to get rid
of a superfluous pointer dereference.

Signed-off-by: Gabor Juhos <[email protected]>
---
drivers/net/wireless/rt2x00/rt2800usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index ac854d7..c71a48d 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -327,7 +327,7 @@ static int rt2800usb_enable_radio(struct rt2x00_dev *rt2x00dev)
* this limit so reduce the number to prevent errors.
*/
rt2x00_set_field32(&reg, USB_DMA_CFG_RX_BULK_AGG_LIMIT,
- ((rt2x00dev->ops->rx->entry_num * DATA_FRAME_SIZE)
+ ((rt2x00dev->rx->limit * DATA_FRAME_SIZE)
/ 1024) - 3);
rt2x00_set_field32(&reg, USB_DMA_CFG_RX_BULK_EN, 1);
rt2x00_set_field32(&reg, USB_DMA_CFG_TX_BULK_EN, 1);
--
1.7.10


2013-05-01 20:41:33

by Gabor Juhos

[permalink] [raw]
Subject: Re: [PATCH 0/7] rt2x00: avoid a few superfluous pointer dereferences

Hi Gertjan,

> On Wed, May 1, 2013 at 5:17 PM, Gabor Juhos <[email protected]> wrote:
>> Gabor Juhos (7):
>> rt2x00: rt2x00dev: use rt2x00dev->tx->limit
>> rt2x00: rt61pci: use rt2x00dev->tx->limit
>> rt2x00: rt2800pci: use rt2x00dev->tx->limit
>> rt2x00: rt2800usb: use rt2x00dev->rx->limit
>> rt2x00: rt2800lib: use rt2x00dev->bcn->winfo_size
>> rt2x00: rt2x00dev: defer operational mode detection
>> rt2x00: rt2x00dev: use rt2x00dev->bcn->limit
>>
>> drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
>> drivers/net/wireless/rt2x00/rt2800pci.c | 2 +-
>> drivers/net/wireless/rt2x00/rt2800usb.c | 2 +-
>> drivers/net/wireless/rt2x00/rt2x00dev.c | 36 +++++++++++++++----------------
>> drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
>> 5 files changed, 22 insertions(+), 22 deletions(-)
>>
>
> Thanks for these. Although they are micro optimizations, I guess it is
> cleaner to use the actual queue data rather than the queue description data.

To be honest, I made these patches in preparation of queue descriptor data
removal. However these changes are not strictly related to that, so I posted the
patches as a separate series.

> I've acked all patches,

Thanks!

> but I'm afraid they may have come just too late to be included in
> the 3.10 timeframe, but I leave it up to John to decide.

No problem, the patches can wait for 3.11.

-Gabor

2013-05-01 20:08:12

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 6/7] rt2x00: rt2x00dev: defer operational mode detection

On Wed, May 1, 2013 at 5:17 PM, Gabor Juhos <[email protected]> wrote:
> Only do it after the queues are allocated. This
> will allow to use the 'rt2x00dev->bcn->limit'
> instead of 'rt2x00dev->ops->bcn->entry_num'.
>
> Signed-off-by: Gabor Juhos <[email protected]>

Acked-by: Gertjan van Wingerde <[email protected]>

> ---
> drivers/net/wireless/rt2x00/rt2x00dev.c | 34 +++++++++++++++----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> index b287467..6a20172 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> @@ -1301,23 +1301,6 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
> (rt2x00dev->ops->max_ap_intf - 1);
>
> /*
> - * Determine which operating modes are supported, all modes
> - * which require beaconing, depend on the availability of
> - * beacon entries.
> - */
> - rt2x00dev->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION);
> - if (rt2x00dev->ops->bcn->entry_num > 0)
> - rt2x00dev->hw->wiphy->interface_modes |=
> - BIT(NL80211_IFTYPE_ADHOC) |
> - BIT(NL80211_IFTYPE_AP) |
> -#ifdef CONFIG_MAC80211_MESH
> - BIT(NL80211_IFTYPE_MESH_POINT) |
> -#endif
> - BIT(NL80211_IFTYPE_WDS);
> -
> - rt2x00dev->hw->wiphy->flags |= WIPHY_FLAG_IBSS_RSN;
> -
> - /*
> * Initialize work.
> */
> rt2x00dev->workqueue =
> @@ -1348,6 +1331,23 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
> goto exit;
>
> /*
> + * Determine which operating modes are supported, all modes
> + * which require beaconing, depend on the availability of
> + * beacon entries.
> + */
> + rt2x00dev->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION);
> + if (rt2x00dev->ops->bcn->entry_num > 0)
> + rt2x00dev->hw->wiphy->interface_modes |=
> + BIT(NL80211_IFTYPE_ADHOC) |
> + BIT(NL80211_IFTYPE_AP) |
> +#ifdef CONFIG_MAC80211_MESH
> + BIT(NL80211_IFTYPE_MESH_POINT) |
> +#endif
> + BIT(NL80211_IFTYPE_WDS);
> +
> + rt2x00dev->hw->wiphy->flags |= WIPHY_FLAG_IBSS_RSN;
> +
> + /*
> * Initialize ieee80211 structure.
> */
> retval = rt2x00lib_probe_hw(rt2x00dev);
> --
> 1.7.10
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
---
Gertjan

2013-05-01 15:17:36

by Gabor Juhos

[permalink] [raw]
Subject: [PATCH 6/7] rt2x00: rt2x00dev: defer operational mode detection

Only do it after the queues are allocated. This
will allow to use the 'rt2x00dev->bcn->limit'
instead of 'rt2x00dev->ops->bcn->entry_num'.

Signed-off-by: Gabor Juhos <[email protected]>
---
drivers/net/wireless/rt2x00/rt2x00dev.c | 34 +++++++++++++++----------------
1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index b287467..6a20172 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -1301,23 +1301,6 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
(rt2x00dev->ops->max_ap_intf - 1);

/*
- * Determine which operating modes are supported, all modes
- * which require beaconing, depend on the availability of
- * beacon entries.
- */
- rt2x00dev->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION);
- if (rt2x00dev->ops->bcn->entry_num > 0)
- rt2x00dev->hw->wiphy->interface_modes |=
- BIT(NL80211_IFTYPE_ADHOC) |
- BIT(NL80211_IFTYPE_AP) |
-#ifdef CONFIG_MAC80211_MESH
- BIT(NL80211_IFTYPE_MESH_POINT) |
-#endif
- BIT(NL80211_IFTYPE_WDS);
-
- rt2x00dev->hw->wiphy->flags |= WIPHY_FLAG_IBSS_RSN;
-
- /*
* Initialize work.
*/
rt2x00dev->workqueue =
@@ -1348,6 +1331,23 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
goto exit;

/*
+ * Determine which operating modes are supported, all modes
+ * which require beaconing, depend on the availability of
+ * beacon entries.
+ */
+ rt2x00dev->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION);
+ if (rt2x00dev->ops->bcn->entry_num > 0)
+ rt2x00dev->hw->wiphy->interface_modes |=
+ BIT(NL80211_IFTYPE_ADHOC) |
+ BIT(NL80211_IFTYPE_AP) |
+#ifdef CONFIG_MAC80211_MESH
+ BIT(NL80211_IFTYPE_MESH_POINT) |
+#endif
+ BIT(NL80211_IFTYPE_WDS);
+
+ rt2x00dev->hw->wiphy->flags |= WIPHY_FLAG_IBSS_RSN;
+
+ /*
* Initialize ieee80211 structure.
*/
retval = rt2x00lib_probe_hw(rt2x00dev);
--
1.7.10


2013-05-22 19:00:13

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 7/7] rt2x00: rt2x00dev: use rt2x00dev->bcn->limit

On Thu, May 02, 2013 at 09:36:11AM +0200, Gabor Juhos wrote:
> 2013.05.01. 22:08 keltez?ssel, Gertjan van Wingerde ?rta:
> > On Wed, May 1, 2013 at 5:17 PM, Gabor Juhos <[email protected]> wrote:
> >> The beacon data queue is initialized already,
> >> so fetch the number of the queue entries from
> >> that instead of using the entry_num field of
> >> the data queue descriptor.
> >>
> >> The two values are the same, and the use of the
> >> rt2x00dev->bcn->limit value allows us to get rid
> >> of a superfluous pointer dereference.
> >>
> >> Signed-off-by: Gabor Juhos <[email protected]>
> >
> > Acked-by: Gertjan van Wingerde <[email protected]>
>
> John, please ignore the last patch. This depends on other patches which were not
> included in the series. Without those it causes the following warning:

OK, dropped...

--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2013-05-01 20:05:37

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 1/7] rt2x00: rt2x00dev: use rt2x00dev->tx->limit

On Wed, May 1, 2013 at 5:17 PM, Gabor Juhos <[email protected]> wrote:
> The TX data queue is initialized already when
> the rt2x00lib_probe_hw() function is called.
>
> Fetch the number of the queue entries from that
> instead of using the entry_num field of the data
> queue descriptor.
>
> The two values are the same, and the use of the
> rt2x00dev->tx->limit value allows us to get rid
> of a superfluous pointer dereference.
>
> Signed-off-by: Gabor Juhos <[email protected]>

Acked-by: Gertjan van Wingerde <[email protected]>

> ---
> drivers/net/wireless/rt2x00/rt2x00dev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> index 90dc143..b287467 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> @@ -1077,7 +1077,7 @@ static int rt2x00lib_probe_hw(struct rt2x00_dev *rt2x00dev)
> */
> int kfifo_size =
> roundup_pow_of_two(rt2x00dev->ops->tx_queues *
> - rt2x00dev->ops->tx->entry_num *
> + rt2x00dev->tx->limit *
> sizeof(u32));
>
> status = kfifo_alloc(&rt2x00dev->txstatus_fifo, kfifo_size,
> --
> 1.7.10
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
---
Gertjan

2013-05-01 20:07:01

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 4/7] rt2x00: rt2800usb: use rt2x00dev->rx->limit

On Wed, May 1, 2013 at 5:17 PM, Gabor Juhos <[email protected]> wrote:
> The RX data queue is initialized already when
> the rt2800_usb_enable_radio() function is called.
>
> Fetch the number of the queue entries from that
> instead of using the entry_num field of the data
> queue descriptor.
>
> The two values are the same, and the use of the
> rt2x00dev->rx->limit value allows us to get rid
> of a superfluous pointer dereference.
>
> Signed-off-by: Gabor Juhos <[email protected]>

Acked-by: Gertjan van Wingerde <[email protected]>

> ---
> drivers/net/wireless/rt2x00/rt2800usb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
> index ac854d7..c71a48d 100644
> --- a/drivers/net/wireless/rt2x00/rt2800usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2800usb.c
> @@ -327,7 +327,7 @@ static int rt2800usb_enable_radio(struct rt2x00_dev *rt2x00dev)
> * this limit so reduce the number to prevent errors.
> */
> rt2x00_set_field32(&reg, USB_DMA_CFG_RX_BULK_AGG_LIMIT,
> - ((rt2x00dev->ops->rx->entry_num * DATA_FRAME_SIZE)
> + ((rt2x00dev->rx->limit * DATA_FRAME_SIZE)
> / 1024) - 3);
> rt2x00_set_field32(&reg, USB_DMA_CFG_RX_BULK_EN, 1);
> rt2x00_set_field32(&reg, USB_DMA_CFG_TX_BULK_EN, 1);
> --
> 1.7.10
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
---
Gertjan

2013-05-01 15:17:33

by Gabor Juhos

[permalink] [raw]
Subject: [PATCH 1/7] rt2x00: rt2x00dev: use rt2x00dev->tx->limit

The TX data queue is initialized already when
the rt2x00lib_probe_hw() function is called.

Fetch the number of the queue entries from that
instead of using the entry_num field of the data
queue descriptor.

The two values are the same, and the use of the
rt2x00dev->tx->limit value allows us to get rid
of a superfluous pointer dereference.

Signed-off-by: Gabor Juhos <[email protected]>
---
drivers/net/wireless/rt2x00/rt2x00dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index 90dc143..b287467 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -1077,7 +1077,7 @@ static int rt2x00lib_probe_hw(struct rt2x00_dev *rt2x00dev)
*/
int kfifo_size =
roundup_pow_of_two(rt2x00dev->ops->tx_queues *
- rt2x00dev->ops->tx->entry_num *
+ rt2x00dev->tx->limit *
sizeof(u32));

status = kfifo_alloc(&rt2x00dev->txstatus_fifo, kfifo_size,
--
1.7.10


2013-06-03 17:10:02

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH 1/7] rt2x00: rt2x00dev: use rt2x00dev->tx->limit

Hi Gabor!

On Wed, 1 May 2013 17:17:29 +0200, Gabor Juhos wrote:
> The TX data queue is initialized already when
> the rt2x00lib_probe_hw() function is called.
>
> Fetch the number of the queue entries from that
> instead of using the entry_num field of the data
> queue descriptor.
>
> The two values are the same, and the use of the
> rt2x00dev->tx->limit value allows us to get rid
> of a superfluous pointer dereference.
>
> Signed-off-by: Gabor Juhos <[email protected]>
> ---
> drivers/net/wireless/rt2x00/rt2x00dev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> index 90dc143..b287467 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> @@ -1077,7 +1077,7 @@ static int rt2x00lib_probe_hw(struct rt2x00_dev *rt2x00dev)
> */
> int kfifo_size =
> roundup_pow_of_two(rt2x00dev->ops->tx_queues *
> - rt2x00dev->ops->tx->entry_num *
> + rt2x00dev->tx->limit *
> sizeof(u32));
>
> status = kfifo_alloc(&rt2x00dev->txstatus_fifo, kfifo_size,

Unfortunately this does not work without your "get rid of
static data queue descriptors" series as well.

queue->limit is set when rt2x00lib_start is called, not on
probe. kfifo_alloc will fail with EINVAL here. As a result
support for all rt2x00 devices is broken on wireless-testing.

Maybe you can just post the mentioned series for inclusion?
I have been running with it since you posted it as RFC and
I did not notice any problems so far...

-- kuba

2013-06-03 20:32:13

by Gabor Juhos

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH 1/7] rt2x00: rt2x00dev: use rt2x00dev->tx->limit

Hi Kuba!

> On Wed, 1 May 2013 17:17:29 +0200, Gabor Juhos wrote:
>> The TX data queue is initialized already when
>> the rt2x00lib_probe_hw() function is called.
>>
>> Fetch the number of the queue entries from that
>> instead of using the entry_num field of the data
>> queue descriptor.
>>
>> The two values are the same, and the use of the
>> rt2x00dev->tx->limit value allows us to get rid
>> of a superfluous pointer dereference.
>>
>> Signed-off-by: Gabor Juhos <[email protected]>
>> ---
>> drivers/net/wireless/rt2x00/rt2x00dev.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
>> index 90dc143..b287467 100644
>> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
>> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
>> @@ -1077,7 +1077,7 @@ static int rt2x00lib_probe_hw(struct rt2x00_dev *rt2x00dev)
>> */
>> int kfifo_size =
>> roundup_pow_of_two(rt2x00dev->ops->tx_queues *
>> - rt2x00dev->ops->tx->entry_num *
>> + rt2x00dev->tx->limit *
>> sizeof(u32));
>>
>> status = kfifo_alloc(&rt2x00dev->txstatus_fifo, kfifo_size,
>
> Unfortunately this does not work without your "get rid of
> static data queue descriptors" series as well.
>
> queue->limit is set when rt2x00lib_start is called, not on
> probe. kfifo_alloc will fail with EINVAL here.

You are right. I should have been more careful while I have tested this patch-set.

> As a result support for all rt2x00 devices is broken on wireless-testing.

Only rt2800 devices are affected. The offending code runs only if
REQUIRE_TXSTATUS_FIFO is set in rt2x00dev->cap_flags which is not set for older
chips.

> Maybe you can just post the mentioned series for inclusion?

I will post a fix for the actual problem first, and will rebase the series on
top of that.

> I have been running with it since you posted it as RFC and
> I did not notice any problems so far...

Great!

-Gabor