[PATCH 1/2] rt2x00: convert rt2x00_ops.extra_tx_headroom to be a fuction

[PATCH 1/2] rt2x00: convert rt2x00_ops.extra_tx_headroom to be a fuction

[Gabor Juhos]

The rt2x00_ops structure has a static field to indicate
the extra TX headroom size required for a given device.
The drawback of this is that we have to use a separate
rt2x00_ops structure for each chipset which requires
different size.

Convert the static field into a callback function.
This allows the drivers to dynamically determine the
extra TX headroom size based on the actual chipset.
Also implement the the callback in the drivers which
needs an extra TX headroom, and remove the field
initialization from the others.

Additionally, introduce a new extra_tx_headroom field
in struct rt2x00_dev, initialize its value in the probe
routine and use the cached value in the rest of the code.

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
The patch depends on the 'rt2x00: get rid of static data queue descriptors'
series.
---
 drivers/net/wireless/rt2x00/rt2400pci.c   |    1 -
 drivers/net/wireless/rt2x00/rt2500pci.c   |    1 -
 drivers/net/wireless/rt2x00/rt2500usb.c   |    8 +++++++-
 drivers/net/wireless/rt2x00/rt2800pci.c   |    8 +++++++-
 drivers/net/wireless/rt2x00/rt2800usb.c   |   13 +++++++++++--
 drivers/net/wireless/rt2x00/rt2x00.h      |    5 ++++-
 drivers/net/wireless/rt2x00/rt2x00dev.c   |   10 ++++++++--
 drivers/net/wireless/rt2x00/rt2x00queue.c |    6 +++---
 drivers/net/wireless/rt2x00/rt61pci.c     |    1 -
 drivers/net/wireless/rt2x00/rt73usb.c     |    8 +++++++-
 10 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index e1ec9a4..3d53a09 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -1813,7 +1813,6 @@ static const struct rt2x00_ops rt2400pci_ops = {
 	.eeprom_size		= EEPROM_SIZE,
 	.rf_size		= RF_SIZE,
 	.tx_queues		= NUM_TX_QUEUES,
-	.extra_tx_headroom	= 0,
 	.queue_init		= rt2400pci_queue_init,
 	.lib			= &rt2400pci_rt2x00_ops,
 	.hw			= &rt2400pci_mac80211_ops,
diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index a1670e8..0ac5c58 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -2102,7 +2102,6 @@ static const struct rt2x00_ops rt2500pci_ops = {
 	.eeprom_size		= EEPROM_SIZE,
 	.rf_size		= RF_SIZE,
 	.tx_queues		= NUM_TX_QUEUES,
-	.extra_tx_headroom	= 0,
 	.queue_init		= rt2500pci_queue_init,
 	.lib			= &rt2500pci_rt2x00_ops,
 	.hw			= &rt2500pci_mac80211_ops,
diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index e5e5479..f8fa2a8 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -1907,13 +1907,19 @@ static void rt2500usb_queue_init(struct data_queue *queue)
 	}
 }
 
+static unsigned int
+rt2500usb_extra_tx_headroom(struct rt2x00_dev *rt2x00dev)
+{
+	return TXD_DESC_SIZE;
+}
+
 static const struct rt2x00_ops rt2500usb_ops = {
 	.name			= KBUILD_MODNAME,
 	.max_ap_intf		= 1,
 	.eeprom_size		= EEPROM_SIZE,
 	.rf_size		= RF_SIZE,
 	.tx_queues		= NUM_TX_QUEUES,
-	.extra_tx_headroom	= TXD_DESC_SIZE,
+	.extra_tx_headroom	= rt2500usb_extra_tx_headroom,
 	.queue_init		= rt2500usb_queue_init,
 	.lib			= &rt2500usb_rt2x00_ops,
 	.hw			= &rt2500usb_mac80211_ops,
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 260c8b4..4e69792 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -1224,6 +1224,12 @@ static void rt2800pci_queue_init(struct data_queue *queue)
 	}
 }
 
+static unsigned int
+rt2800pci_extra_tx_headroom(struct rt2x00_dev *rt2x00dev)
+{
+	return TXWI_DESC_SIZE;
+}
+
 static const struct rt2x00_ops rt2800pci_ops = {
 	.name			= KBUILD_MODNAME,
 	.drv_data_size		= sizeof(struct rt2800_drv_data),
@@ -1231,7 +1237,7 @@ static const struct rt2x00_ops rt2800pci_ops = {
 	.eeprom_size		= EEPROM_SIZE,
 	.rf_size		= RF_SIZE,
 	.tx_queues		= NUM_TX_QUEUES,
-	.extra_tx_headroom	= TXWI_DESC_SIZE,
+	.extra_tx_headroom	= rt2800pci_extra_tx_headroom,
 	.queue_init		= rt2800pci_queue_init,
 	.lib			= &rt2800pci_rt2x00_ops,
 	.drv			= &rt2800pci_rt2800_ops,
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index b81d509..57729a8 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -898,6 +898,15 @@ static void rt2800usb_queue_init(struct data_queue *queue)
 	}
 }
 
+static unsigned int
+rt2800usb_extra_tx_headroom(struct rt2x00_dev *rt2x00dev)
+{
+	if (rt2x00_rt(rt2x00dev, RT5592))
+		return TXINFO_DESC_SIZE + TXWI_DESC_SIZE_5592;
+
+	return TXINFO_DESC_SIZE + TXWI_DESC_SIZE;
+}
+
 static const struct rt2x00_ops rt2800usb_ops = {
 	.name			= KBUILD_MODNAME,
 	.drv_data_size		= sizeof(struct rt2800_drv_data),
@@ -905,7 +914,7 @@ static const struct rt2x00_ops rt2800usb_ops = {
 	.eeprom_size		= EEPROM_SIZE,
 	.rf_size		= RF_SIZE,
 	.tx_queues		= NUM_TX_QUEUES,
-	.extra_tx_headroom	= TXINFO_DESC_SIZE + TXWI_DESC_SIZE,
+	.extra_tx_headroom	= rt2800usb_extra_tx_headroom,
 	.queue_init		= rt2800usb_queue_init,
 	.lib			= &rt2800usb_rt2x00_ops,
 	.drv			= &rt2800usb_rt2800_ops,
@@ -922,7 +931,7 @@ static const struct rt2x00_ops rt2800usb_ops_5592 = {
 	.eeprom_size		= EEPROM_SIZE,
 	.rf_size		= RF_SIZE,
 	.tx_queues		= NUM_TX_QUEUES,
-	.extra_tx_headroom	= TXINFO_DESC_SIZE + TXWI_DESC_SIZE_5592,
+	.extra_tx_headroom	= rt2800usb_extra_tx_headroom,
 	.queue_init		= rt2800usb_queue_init,
 	.lib			= &rt2800usb_rt2x00_ops,
 	.drv			= &rt2800usb_rt2800_ops,
diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
index 2a17f7e..41ca342 100644
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -648,7 +648,7 @@ struct rt2x00_ops {
 	const unsigned int eeprom_size;
 	const unsigned int rf_size;
 	const unsigned int tx_queues;
-	const unsigned int extra_tx_headroom;
+	unsigned int (*extra_tx_headroom)(struct rt2x00_dev *rt2x00dev);
 	void (*queue_init)(struct data_queue *queue);
 	const struct rt2x00lib_ops *lib;
 	const void *drv;
@@ -1007,6 +1007,9 @@ struct rt2x00_dev {
 	 */
 	struct list_head bar_list;
 	spinlock_t bar_list_lock;
+
+	/* Extra TX headroom required for alignment purposes. */
+	unsigned int extra_tx_headroom;
 };
 
 struct rt2x00_bar_list_entry {
diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index dff5012..964bf1a 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -334,7 +334,7 @@ void rt2x00lib_txdone(struct queue_entry *entry,
 	/*
 	 * Remove the extra tx headroom from the skb.
 	 */
-	skb_pull(entry->skb, rt2x00dev->ops->extra_tx_headroom);
+	skb_pull(entry->skb, rt2x00dev->extra_tx_headroom);
 
 	/*
 	 * Signal that the TX descriptor is no longer in the skb.
@@ -1049,7 +1049,7 @@ static int rt2x00lib_probe_hw(struct rt2x00_dev *rt2x00dev)
 	 */
 	rt2x00dev->hw->extra_tx_headroom =
 		max_t(unsigned int, IEEE80211_TX_STATUS_HEADROOM,
-		      rt2x00dev->ops->extra_tx_headroom);
+		      rt2x00dev->extra_tx_headroom);
 
 	/*
 	 * Take TX headroom required for alignment into account.
@@ -1280,6 +1280,12 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
 		}
 	}
 
+	if (rt2x00dev->ops->extra_tx_headroom) {
+		/* Cache TX headroom value */
+		rt2x00dev->extra_tx_headroom =
+			rt2x00dev->ops->extra_tx_headroom(rt2x00dev);
+	}
+
 	spin_lock_init(&rt2x00dev->irqmask_lock);
 	mutex_init(&rt2x00dev->csr_mutex);
 	INIT_LIST_HEAD(&rt2x00dev->bar_list);
diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index c4f1e2b..6c0a91f 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -542,8 +542,8 @@ static int rt2x00queue_write_tx_data(struct queue_entry *entry,
 	/*
 	 * Add the requested extra tx headroom in front of the skb.
 	 */
-	skb_push(entry->skb, rt2x00dev->ops->extra_tx_headroom);
-	memset(entry->skb->data, 0, rt2x00dev->ops->extra_tx_headroom);
+	skb_push(entry->skb, rt2x00dev->extra_tx_headroom);
+	memset(entry->skb->data, 0, rt2x00dev->extra_tx_headroom);
 
 	/*
 	 * Call the driver's write_tx_data function, if it exists.
@@ -596,7 +596,7 @@ static void rt2x00queue_bar_check(struct queue_entry *entry)
 {
 	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
 	struct ieee80211_bar *bar = (void *) (entry->skb->data +
-				    rt2x00dev->ops->extra_tx_headroom);
+				    rt2x00dev->extra_tx_headroom);
 	struct rt2x00_bar_list_entry *bar_entry;
 
 	if (likely(!ieee80211_is_back_req(bar->frame_control)))
diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index 17507d1..53754bc6 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -3066,7 +3066,6 @@ static const struct rt2x00_ops rt61pci_ops = {
 	.eeprom_size		= EEPROM_SIZE,
 	.rf_size		= RF_SIZE,
 	.tx_queues		= NUM_TX_QUEUES,
-	.extra_tx_headroom	= 0,
 	.queue_init		= rt61pci_queue_init,
 	.lib			= &rt61pci_rt2x00_ops,
 	.hw			= &rt61pci_mac80211_ops,
diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index b2e346a..5545fa7 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -2394,13 +2394,19 @@ static void rt73usb_queue_init(struct data_queue *queue)
 	}
 }
 
+static unsigned int
+rt73usb_extra_tx_headroom(struct rt2x00_dev *rt2x00dev)
+{
+	return TXD_DESC_SIZE;
+}
+
 static const struct rt2x00_ops rt73usb_ops = {
 	.name			= KBUILD_MODNAME,
 	.max_ap_intf		= 4,
 	.eeprom_size		= EEPROM_SIZE,
 	.rf_size		= RF_SIZE,
 	.tx_queues		= NUM_TX_QUEUES,
-	.extra_tx_headroom	= TXD_DESC_SIZE,
+	.extra_tx_headroom	= rt73usb_extra_tx_headroom,
 	.queue_init		= rt73usb_queue_init,
 	.lib			= &rt73usb_rt2x00_ops,
 	.hw			= &rt73usb_mac80211_ops,
-- 
1.7.10

[PATCH 2/2] rt2x00: rt2800usb: nuke rt2800usb_ops_5592

[Gabor Juhos]

It is exactly the same like the generic rt2800usb_ops.
Remove the duplicate and use the generic ops for all
devices.

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/net/wireless/rt2x00/rt2800usb.c |   30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 57729a8..c065d43 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -924,23 +924,6 @@ static const struct rt2x00_ops rt2800usb_ops = {
 #endif /* CONFIG_RT2X00_LIB_DEBUGFS */
 };
 
-static const struct rt2x00_ops rt2800usb_ops_5592 = {
-	.name			= KBUILD_MODNAME,
-	.drv_data_size		= sizeof(struct rt2800_drv_data),
-	.max_ap_intf		= 8,
-	.eeprom_size		= EEPROM_SIZE,
-	.rf_size		= RF_SIZE,
-	.tx_queues		= NUM_TX_QUEUES,
-	.extra_tx_headroom	= rt2800usb_extra_tx_headroom,
-	.queue_init		= rt2800usb_queue_init,
-	.lib			= &rt2800usb_rt2x00_ops,
-	.drv			= &rt2800usb_rt2800_ops,
-	.hw			= &rt2800usb_mac80211_ops,
-#ifdef CONFIG_RT2X00_LIB_DEBUGFS
-	.debugfs		= &rt2800_rt2x00debug,
-#endif /* CONFIG_RT2X00_LIB_DEBUGFS */
-};
-
 /*
  * rt2800usb module information.
  */
@@ -1253,15 +1236,15 @@ static struct usb_device_id rt2800usb_device_table[] = {
 #endif
 #ifdef CONFIG_RT2800USB_RT55XX
 	/* Arcadyan */
-	{ USB_DEVICE(0x043e, 0x7a32), .driver_info = 5592 },
+	{ USB_DEVICE(0x043e, 0x7a32) },
 	/* AVM GmbH */
-	{ USB_DEVICE(0x057c, 0x8501), .driver_info = 5592 },
+	{ USB_DEVICE(0x057c, 0x8501) },
 	/* D-Link DWA-160-B2 */
-	{ USB_DEVICE(0x2001, 0x3c1a), .driver_info = 5592 },
+	{ USB_DEVICE(0x2001, 0x3c1a) },
 	/* Proware */
-	{ USB_DEVICE(0x043e, 0x7a13), .driver_info = 5592 },
+	{ USB_DEVICE(0x043e, 0x7a13) },
 	/* Ralink */
-	{ USB_DEVICE(0x148f, 0x5572), .driver_info = 5592 },
+	{ USB_DEVICE(0x148f, 0x5572) },
 #endif
 #ifdef CONFIG_RT2800USB_UNKNOWN
 	/*
@@ -1366,9 +1349,6 @@ MODULE_LICENSE("GPL");
 static int rt2800usb_probe(struct usb_interface *usb_intf,
 			   const struct usb_device_id *id)
 {
-	if (id->driver_info == 5592)
-		return rt2x00usb_probe(usb_intf, &rt2800usb_ops_5592);
-
 	return rt2x00usb_probe(usb_intf, &rt2800usb_ops);
 }
 
-- 
1.7.10

Re: [PATCH 1/2] rt2x00: convert rt2x00_ops.extra_tx_headroom to be a fuction

[Stanislaw Gruszka]

On Wed, Jun 05, 2013 at 12:53:59PM +0200, Gabor Juhos wrote:
> The rt2x00_ops structure has a static field to indicate
> the extra TX headroom size required for a given device.
> The drawback of this is that we have to use a separate
> rt2x00_ops structure for each chipset which requires
> different size.
> 
> Convert the static field into a callback function.
> This allows the drivers to dynamically determine the
> extra TX headroom size based on the actual chipset.
> Also implement the the callback in the drivers which
> needs an extra TX headroom, and remove the field
> initialization from the others.
> 
> Additionally, introduce a new extra_tx_headroom field
> in struct rt2x00_dev, initialize its value in the probe
> routine and use the cached value in the rest of the code.

Could we rather get rid of that extra_tx_headroom variable and use queue
parameters: winfo_size and desc_size ?

Stanislaw

Re: [PATCH 1/2] rt2x00: convert rt2x00_ops.extra_tx_headroom to be a fuction

[Gabor Juhos]

2013.06.05. 14:51 keltezéssel, Stanislaw Gruszka írta:
> On Wed, Jun 05, 2013 at 12:53:59PM +0200, Gabor Juhos wrote:
>> The rt2x00_ops structure has a static field to indicate
>> the extra TX headroom size required for a given device.
>> The drawback of this is that we have to use a separate
>> rt2x00_ops structure for each chipset which requires
>> different size.
>>
>> Convert the static field into a callback function.
>> This allows the drivers to dynamically determine the
>> extra TX headroom size based on the actual chipset.
>> Also implement the the callback in the drivers which
>> needs an extra TX headroom, and remove the field
>> initialization from the others.
>>
>> Additionally, introduce a new extra_tx_headroom field
>> in struct rt2x00_dev, initialize its value in the probe
>> routine and use the cached value in the rest of the code.
> 
> Could we rather get rid of that extra_tx_headroom variable and use queue
> parameters: winfo_size and desc_size ?

Yes, it seems that we can compute the value of extra_tx_headrom from those
fields. The driver uses the following values now:

driver/device    desc_size         winfo_size           extra_tx_headroom

rt2400pci        TXD_DESC_SIZE     0                    0
rt2500pci        TXD_DESC_SIZE     0                    0
rt2800pci        TXD_DESC_SIZE     TXWI_DESC_SIZE       TXWI_DESC_SIZE
rt61pci          TXD_DESC_SIZE     0                    0

rt2500usb        TXD_DESC_SIZE     0                    TXD_DESC_SIZE
rt2800usb        TXINFO_DESC_SIZE  TXWI_DESC_SIZE       TXINFO_DESC_SIZE +
TXWI_DESC_SIZE
rt2800usb/RT5592 TXINFO_DESC_SIZE  TXWI_DESC_SIZE_5592  TXINFO_DESC_SIZE +
TXWI_DESC_SIZE_5592
rt73usb          TXD_DESC_SIZE     0                    TXD_DESC_SIZE

On USB devices we can get it from desc_size + winfo_size and on PCI/PCIe/SoC
devices it equals with winfo_size. So the callbacks and the extra_tx_headroom
field in rt2x00_ops can be removed.

However I would keep the new extra_tx_headroom field in rt2x00_dev. It would
ensure that we don't have to compute its value every time it is used.

-Gabor

Re: [PATCH 1/2] rt2x00: convert rt2x00_ops.extra_tx_headroom to be a fuction

[Stanislaw Gruszka]

On Wed, Jun 05, 2013 at 09:58:10PM +0200, Gabor Juhos wrote:
> > Could we rather get rid of that extra_tx_headroom variable and use queue
> > parameters: winfo_size and desc_size ?
> 
> Yes, it seems that we can compute the value of extra_tx_headrom from those
> fields. The driver uses the following values now:
> 
> driver/device    desc_size         winfo_size           extra_tx_headroom
> 
> rt2400pci        TXD_DESC_SIZE     0                    0
> rt2500pci        TXD_DESC_SIZE     0                    0
> rt2800pci        TXD_DESC_SIZE     TXWI_DESC_SIZE       TXWI_DESC_SIZE
> rt61pci          TXD_DESC_SIZE     0                    0
> 
> rt2500usb        TXD_DESC_SIZE     0                    TXD_DESC_SIZE
> rt2800usb        TXINFO_DESC_SIZE  TXWI_DESC_SIZE       TXINFO_DESC_SIZE +
> TXWI_DESC_SIZE
> rt2800usb/RT5592 TXINFO_DESC_SIZE  TXWI_DESC_SIZE_5592  TXINFO_DESC_SIZE +
> TXWI_DESC_SIZE_5592
> rt73usb          TXD_DESC_SIZE     0                    TXD_DESC_SIZE
> 
> On USB devices we can get it from desc_size + winfo_size and on PCI/PCIe/SoC
> devices it equals with winfo_size. So the callbacks and the extra_tx_headroom
> field in rt2x00_ops can be removed.
> 
> However I would keep the new extra_tx_headroom field in rt2x00_dev. It would
> ensure that we don't have to compute its value every time it is used.

Ok, that make sense.

Thanks
Stanislaw