Re: [PATCH 3/32] rt2x00: use pci_*_consistent for DMA mapping

[Ivo van Doorn <ivdoorn@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5053 bytes --]

On Friday 28 April 2006 15:15, Christoph Hellwig wrote:
> On Fri, Apr 28, 2006 at 02:57:31PM +0200, Ivo van Doorn wrote:
> > On Friday 28 April 2006 00:11, Christoph Hellwig wrote:
> > > On Fri, Apr 28, 2006 at 12:02:52AM +0200, Ivo van Doorn wrote:
> > > > From: Ivo van Doorn <IvDoorn@gmail.com>
> > > > 
> > > > Instead of dma_*_coherent
> > > > use pci_*consistent functions.
> > > 
> > > 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?

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;
}

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]