rt2x00: use pci_*_consistent for DMA mapping

From: Ivo van Doorn <[email protected]>

Add linux/dma-mapping.h header to allow compilation on some architectures. Instead of dma_*_coherent use pci_*consistent functions.

diff -uprN wireless-dev-rt2x00/drivers/net/wireless/d80211/rt2x00/rt2400pci.c wireless-dev-rt2x00-patch/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
@@ -29,6 +29,7 @@
 #include <linux/version.h>
 #include <linux/init.h>
 #include <linux/pci.h>
+#include <linux/dma-mapping.h>
 #include <linux/delay.h>
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>

@@ -928,8 +929,8 @@ rt2400pci_alloc_ring(
 	/*
 	 * Allocate DMA memory for descriptor and buffer.
 	 */
-	ring->data_addr = dma_alloc_coherent(&rt2x00pci->pci_dev->dev,
-		rt2x00_get_ring_size(ring), &ring->data_dma, GFP_KERNEL);
+	ring->data_addr = pci_alloc_consistent(rt2x00pci->pci_dev,
+		rt2x00_get_ring_size(ring), &ring->data_dma);
 	if (!ring->data_addr) {
 		kfree(ring->entry);
 		return -ENOMEM;

@@ -959,7 +960,7 @@ static void
 rt2400pci_free_ring(struct rt2x00_pci *rt2x00pci, struct data_ring *ring)
 {
 	if (ring->data_addr)
-		dma_free_coherent(&rt2x00pci->pci_dev->dev,
+		pci_free_consistent(rt2x00pci->pci_dev,
 			rt2x00_get_ring_size(ring),
 			ring->data_addr, ring->data_dma);
 	ring->data_addr = NULL;

diff -uprN wireless-dev-rt2x00/drivers/net/wireless/d80211/rt2x00/rt2500pci.c wireless-dev-rt2x00-patch/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
@@ -29,6 +29,7 @@
 #include <linux/version.h>
 #include <linux/init.h>
 #include <linux/pci.h>
+#include <linux/dma-mapping.h>
 #include <linux/delay.h>
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>

@@ -964,8 +965,8 @@ rt2500pci_alloc_ring(
 	/*
 	 * Allocate DMA memory for descriptor and buffer.
 	 */
-	ring->data_addr = dma_alloc_coherent(&rt2x00pci->pci_dev->dev,
-		rt2x00_get_ring_size(ring), &ring->data_dma, GFP_KERNEL);
+	ring->data_addr = pci_alloc_consistent(rt2x00pci->pci_dev,
+		rt2x00_get_ring_size(ring), &ring->data_dma);
 	if (!ring->data_addr) {
 		kfree(ring->entry);
 		return -ENOMEM;

@@ -995,7 +996,7 @@ static void
 rt2500pci_free_ring(struct rt2x00_pci *rt2x00pci, struct data_ring *ring)
 {
 	if (ring->data_addr)
-		dma_free_coherent(&rt2x00pci->pci_dev->dev,
+		pci_free_consistent(rt2x00pci->pci_dev,
 			rt2x00_get_ring_size(ring),
 			ring->data_addr, ring->data_dma);
 	ring->data_addr = NULL;

No point in doing that, quite reverse as you use the gfp_mask argument which is a (small) pessimation here.

Would it be recommended that I replace the usb_alloc_buffers in the rt2570 driver with dma_*coherent?
This would get the dma rings allocation and freeing a bit more generic between PCI and USB and could reduce some duplicate code between them. But I am not sure if there is any major difference between the two.

I don't have the full code for your driver anywhere near me so I can't comment on this. Maybe you could send the code of 'usb_alloc_buffers' for comments?

---end quoted text---

Below is the function of the rt2500usb_alloc_ring, it works very similar to the pci equivalent, except that urbs are allocated and usb_buffer_alloc is used. The data_ring structure where the information is stored in is also the same as used in the pci driver.

Basicly what I want to do is move the urbs to somewhere else,
the data_ring structure has been very generic, and the only real difference between the pci and usb version of the data_entry has been the urb structure pointer. If the usb_buffer_* calls could be replaced by dma_*coherent a lot
of duplicate code in the pci and usb drivers could be moved into
a seperate header or source file.

struct data_ring{
	/*
	 * net_device where this ring belongs to.
	 */
	struct net_device			*net_dev;

	/*
	 * Work structure for bottom half interrupt handling.
	 */
	struct work_struct			irq_work;

	/*
	 * Base address for the device specific data entries.
	 */
	void					*entry;

	/*
	 * TX queue statistic info.
	 */
	struct ieee80211_tx_queue_stats_data	stats;

	/*
	 * TX Queue parameters.
	 */
	struct ieee80211_tx_queue_params	tx_params;

	/*
	 * Base address for data ring.
	 */
	dma_addr_t				data_dma;
	void					*data_addr;

	/*
	 * Index variables.
	 */
	u8					index;
	u8					index_done;

	/*
	 * Size of device specific data entry structure.
	 */
	u16					entry_size;

	/*
	 * Size of packet and descriptor in bytes.
	 */
	u16					data_size;
	u16					desc_size;
} __attribute__ ((packed));

static int
rt2500usb_alloc_ring(
struct rt2x00_usb *rt2x00usb,
struct data_ring *ring,
void (*handler)(void *),
const u16 max_entries,
const u16 data_size,
const u16 desc_size)

{
	struct data_entry	*entry;
	u8			counter;
	int			status;

	/*
	 *Set device structure.
	 */
	ring->net_dev = usb_get_intfdata(rt2x00usb->usb_intf);

	/*
	 * Initialize work structure for deferred work.
	 */
	INIT_WORK(&ring->irq_work, handler, ring);

	/*
	 * Initialize ring parameters.
	 */
	ring->tx_params.aifs = 2;
	ring->tx_params.cw_min = 5;	/* cw_min: 2^5 = 32. */
	ring->tx_params.cw_max = 10;	/* cw_max: 2^10 = 1024. */

	rt2x00_ring_index_clear(ring);

	ring->stats.limit = max_entries;
	ring->entry_size = sizeof(struct data_entry);
	ring->data_size = data_size;
	ring->desc_size = desc_size;

	/*
	 * Allocate all ring entries.
	 */
	ring->entry = kmalloc(ring->stats.limit * ring->entry_size,
		GFP_KERNEL);
	if (!ring->entry)
		return -ENOMEM;

	/*
	 * Allocate DMA memory for descriptor and buffer.
	 */
	ring->data_addr = usb_buffer_alloc(
		interface_to_usbdev(rt2x00usb->usb_intf),
		rt2x00_get_ring_size(ring), GFP_KERNEL, &ring->data_dma);
	if (!ring->data_addr) {
		kfree(ring->entry);
		return -ENOMEM;
	}

	/*
	 * Initialize all ring entries to contain valid
	 * addresses.
	 */
	status = 0;
	entry = ring->entry;
	for (counter = 0; counter < ring->stats.limit; counter++) {
		entry[counter].ring = ring;
		if (!status)
			entry[counter].urb = usb_alloc_urb(0, GFP_KERNEL);
		else
			entry[counter].urb = NULL;
		if (!entry[counter].urb)
			status = -ENOMEM;
		entry[counter].skb = NULL;
		entry[counter].data_addr = ring->data_addr
			+ (counter * ring->desc_size)
			+ (counter * ring->data_size);
		entry[counter].data_dma = ring->data_dma
			+ (counter * ring->desc_size)
			+ (counter * ring->data_size);
	}

	return status;
}

static void
rt2500usb_free_ring(struct rt2x00_usb *rt2x00usb, struct data_ring *ring)

{
	struct	data_entry	*entry;
	u8			counter;

	if (!ring->entry)
		goto exit;

	entry = ring->entry;
	for (counter = 0; counter < ring->stats.limit; counter++) {
		usb_kill_urb(entry[counter].urb);
		usb_free_urb(entry[counter].urb);
	}

	kfree(ring->entry);
	ring->entry = NULL;

exit:

	if (ring->data_addr)
		usb_buffer_free(
			interface_to_usbdev(rt2x00usb->usb_intf),
			rt2x00_get_ring_size(ring), ring->data_addr,
			ring->data_dma);
	ring->data_addr = NULL;
}

Why is this packed? I don't believe you write such a data structure (which contains dscape specific structs) to some device registers.

Greetings Michael.

I can't remember actually, I believe this came from a patch we had received last year with the legacy drivers from a user.
Since then most structures have received the __attribute__ ((packed)) tag. I'll create a patch to remove the tags where they are not needed.