[rt2x00-users] [RFC] rt2x00: Use tasklets for rt2x00usb

Helmut Schaa helmut.schaa at googlemail.com
Thu Jul 22 16:40:13 AEST 2010


Hi Ivo,

Am Donnerstag 22 Juli 2010 schrieb Ivo van Doorn:
> This updates the rt2x00usb driver to use tasklets for handling interrupts.
> This simplifies the code in rt2x00lib since it no longer needs to check if the
> device is USB or PCI to decide which mac80211 function should be used.

Ehhm, tasklets will be executed in a softirq, right? That means we cannot use the
non-irqsafe rx and txstatus methods from within the tx/tx tasklet.

Helmut

> This patch is work in progress, but I would to hear if there are any comments
> on the approach taken.
> 
> Signed-off-by: Ivo van Doorn <IvDoorn at gmail.com>
> ---
> diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
> index c21af38..204a5a5 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00.h
> +++ b/drivers/net/wireless/rt2x00/rt2x00.h
> @@ -862,6 +862,12 @@ struct rt2x00_dev {
>  	 */
>  	struct work_struct intf_work;
>  
> +	/**
> + 	 * Tasklet for bottom half TX/RX done handling (USB devices)
> + 	 */
> +	struct tasklet_struct tx_tasklet;
> +	struct tasklet_struct rx_tasklet;
> +
>  	/*
>  	 * Data queue arrays for RX, TX and Beacon.
>  	 * The Beacon array also contains the Atim queue
> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> index 585e816..a5a0b48 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> @@ -383,15 +383,7 @@ void rt2x00lib_txdone(struct queue_entry *entry,
>  	 * send the status report back.
>  	 */
>  	if (!(skbdesc_flags & SKBDESC_NOT_MAC80211))
> -		/*
> -		 * Only PCI and SOC devices process the tx status in process
> -		 * context. Hence use ieee80211_tx_status for PCI and SOC
> -		 * devices and stick to ieee80211_tx_status_irqsafe for USB.
> -		 */
> -		if (rt2x00_is_usb(rt2x00dev))
> -			ieee80211_tx_status_irqsafe(rt2x00dev->hw, entry->skb);
> -		else
> -			ieee80211_tx_status(rt2x00dev->hw, entry->skb);
> +		ieee80211_tx_status(rt2x00dev->hw, entry->skb);
>  	else
>  		dev_kfree_skb_any(entry->skb);
>  
> @@ -403,7 +395,6 @@ void rt2x00lib_txdone(struct queue_entry *entry,
>  
>  	rt2x00dev->ops->lib->clear_entry(entry);
>  
> -	clear_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags);
>  	rt2x00queue_index_inc(entry->queue, Q_INDEX_DONE);
>  
>  	/*
> @@ -463,6 +454,10 @@ void rt2x00lib_rxdone(struct rt2x00_dev *rt2x00dev,
>  	struct ieee80211_rx_status *rx_status = &rt2x00dev->rx_status;
>  	unsigned int header_length;
>  	int rate_idx;
> +
> +	if (test_bit(ENTRY_DATA_IO_FAILED, &entry->flags))
> +		goto submit_entry;
> +
>  	/*
>  	 * Allocate a new sk_buffer. If no new buffer available, drop the
>  	 * received frame and reuse the existing buffer.
> @@ -540,25 +535,15 @@ void rt2x00lib_rxdone(struct rt2x00_dev *rt2x00dev,
>  	 */
>  	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_RXDONE, entry->skb);
>  	memcpy(IEEE80211_SKB_RXCB(entry->skb), rx_status, sizeof(*rx_status));
> -
> -	/*
> -	 * Currently only PCI and SOC devices handle rx interrupts in process
> -	 * context. Hence, use ieee80211_rx_irqsafe for USB and ieee80211_rx_ni
> -	 * for PCI and SOC devices.
> -	 */
> -	if (rt2x00_is_usb(rt2x00dev))
> -		ieee80211_rx_irqsafe(rt2x00dev->hw, entry->skb);
> -	else
> -		ieee80211_rx_ni(rt2x00dev->hw, entry->skb);
> +	ieee80211_rx_ni(rt2x00dev->hw, entry->skb);
>  
>  	/*
>  	 * Replace the skb with the freshly allocated one.
>  	 */
>  	entry->skb = skb;
> -	entry->flags = 0;
>  
> +submit_entry:
>  	rt2x00dev->ops->lib->clear_entry(entry);
> -
>  	rt2x00queue_index_inc(entry->queue, Q_INDEX);
>  }
>  EXPORT_SYMBOL_GPL(rt2x00lib_rxdone);
> @@ -1019,6 +1004,14 @@ void rt2x00lib_remove_dev(struct rt2x00_dev *rt2x00dev)
>  	cancel_work_sync(&rt2x00dev->intf_work);
>  
>  	/*
> +	 * Stop all tasklets.
> +	 */
> +	tasklet_disable(&rt2x00dev->tx_tasklet);
> +	tasklet_kill(&rt2x00dev->tx_tasklet);
> +	tasklet_disable(&rt2x00dev->rx_tasklet);
> +	tasklet_kill(&rt2x00dev->rx_tasklet);
> +
> +	/*
>  	 * Uninitialize device.
>  	 */
>  	rt2x00lib_uninitialize(rt2x00dev);
> diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.h b/drivers/net/wireless/rt2x00/rt2x00queue.h
> index 191e777..5e57138 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00queue.h
> +++ b/drivers/net/wireless/rt2x00/rt2x00queue.h
> @@ -363,12 +363,16 @@ struct txentry_desc {
>   *	the device has signaled it is done with it.
>   * @ENTRY_DATA_PENDING: This entry contains a valid frame and is waiting
>   *	for the signal to start sending.
> + * @ENTRY_DATA_IO_FAILED: Hardware indicated that an IO error occured
> + *	while transfering the data to the hardware. No TX status report will
> + *	be expected from the hardware.
>   */
>  enum queue_entry_flags {
>  	ENTRY_BCN_ASSIGNED,
>  	ENTRY_OWNER_DEVICE_DATA,
>  	ENTRY_OWNER_DEVICE_CRYPTO,
>  	ENTRY_DATA_PENDING,
> +	ENTRY_DATA_IO_FAILED
>  };
>  
>  /**
> diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
> index ff3a366..3354fb0 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
> @@ -167,33 +167,59 @@ EXPORT_SYMBOL_GPL(rt2x00usb_regbusy_read);
>  /*
>   * TX data handlers.
>   */
> +static void rt2x00usb_tasklet_txdone(unsigned long data)
> +{
> +	struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
> +	struct data_queue *queue;
> +	struct queue_entry *entry;
> +	struct txdone_entry_desc txdesc;
> +
> +	tx_queue_for_each(rt2x00dev, queue) {
> +		while (!rt2x00queue_empty(queue)) {
> +			entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
> +
> +			if (!test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags))
> +				break;
> +
> +			/*
> +			 * If the transfer to hardware succeeded, it does not mean the
> +			 * frame was send out correctly. It only means the frame
> +			 * was succesfully pushed to the hardware, we have no
> +			 * way to determine the transmission status right now.
> +			 * (Only indirectly by looking at the failed TX counters
> +			 * in the register).
> +			 */
> +			txdesc.flags = 0;
> +			if (test_bit(ENTRY_DATA_IO_FAILED, &entry->flags))
> +				__set_bit(TXDONE_FAILURE, &txdesc.flags);
> +			else
> +				__set_bit(TXDONE_UNKNOWN, &txdesc.flags);
> +			txdesc.retry = 0;
> +			rt2x00lib_txdone(entry, &txdesc);
> +		}
> +	}
> +}
> +
>  static void rt2x00usb_interrupt_txdone(struct urb *urb)
>  {
>  	struct queue_entry *entry = (struct queue_entry *)urb->context;
>  	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
> -	struct txdone_entry_desc txdesc;
>  
>  	if (!test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags) ||
>  	    !test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags))
>  		return;
>  
>  	/*
> -	 * Obtain the status about this packet.
> -	 * Note that when the status is 0 it does not mean the
> -	 * frame was send out correctly. It only means the frame
> -	 * was succesfully pushed to the hardware, we have no
> -	 * way to determine the transmission status right now.
> -	 * (Only indirectly by looking at the failed TX counters
> -	 * in the register).
> +	 * Check if the frame was correctly uploaded
>  	 */
> -	txdesc.flags = 0;
> -	if (!urb->status)
> -		__set_bit(TXDONE_UNKNOWN, &txdesc.flags);
> -	else
> -		__set_bit(TXDONE_FAILURE, &txdesc.flags);
> -	txdesc.retry = 0;
> -
> -	rt2x00lib_txdone(entry, &txdesc);
> +	if (urb->status)
> +		set_bit(ENTRY_DATA_IO_FAILED, &entry->flags);
> +
> +	/*
> +	 * Schedule the tasklet for reading the TX status
> +	 * from the device.
> +	 */
> +	tasklet_schedule(&rt2x00dev->tx_tasklet);
>  }
>  
>  static inline void rt2x00usb_kick_tx_entry(struct queue_entry *entry)
> @@ -345,15 +371,40 @@ EXPORT_SYMBOL_GPL(rt2x00usb_watchdog);
>  /*
>   * RX data handlers.
>   */
> +static void rt2x00usb_tasklet_rxdone(unsigned long data)
> +{
> +	struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
> +	struct queue_entry *entry;
> +	struct skb_frame_desc *skbdesc;
> +	u8 rxd[32];
> +
> +	while (!rt2x00queue_empty(rt2x00dev->rx)) {
> +		entry = rt2x00queue_get_entry(rt2x00dev->rx, Q_INDEX);
> +
> +		if (test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags))
> +			break;
> +
> +		/*
> +		 * Fill in desc fields of the skb descriptor
> +		 */
> +		skbdesc = get_skb_frame_desc(entry->skb);
> +		skbdesc->desc = rxd;
> +		skbdesc->desc_len = entry->queue->desc_size;
> +
> +		/*
> +		 * Send the frame to rt2x00lib for further processing.
> +		 */
> +		rt2x00lib_rxdone(rt2x00dev, entry);
> +	}
> +}
> +
>  static void rt2x00usb_interrupt_rxdone(struct urb *urb)
>  {
>  	struct queue_entry *entry = (struct queue_entry *)urb->context;
>  	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
> -	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
> -	u8 rxd[32];
>  
>  	if (!test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags) ||
> -	    !test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags))
> +	    !__test_and_clear_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags))
>  		return;
>  
>  	/*
> @@ -361,22 +412,14 @@ static void rt2x00usb_interrupt_rxdone(struct urb *urb)
>  	 * to be actually valid, or if the urb is signaling
>  	 * a problem.
>  	 */
> -	if (urb->actual_length < entry->queue->desc_size || urb->status) {
> -		set_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags);
> -		usb_submit_urb(urb, GFP_ATOMIC);
> -		return;
> -	}
> -
> -	/*
> -	 * Fill in desc fields of the skb descriptor
> -	 */
> -	skbdesc->desc = rxd;
> -	skbdesc->desc_len = entry->queue->desc_size;
> +	if (urb->actual_length < entry->queue->desc_size || urb->status)
> +		__set_bit(ENTRY_DATA_IO_FAILED, &entry->flags);
>  
>  	/*
> -	 * Send the frame to rt2x00lib for further processing.
> +	 * Schedule the tasklet for reading the RX status
> +	 * from the device.
>  	 */
> -	rt2x00lib_rxdone(rt2x00dev, entry);
> +	tasklet_schedule(&rt2x00dev->rx_tasklet);
>  }
>  
>  /*
> @@ -405,6 +448,8 @@ void rt2x00usb_clear_entry(struct queue_entry *entry)
>  	struct queue_entry_priv_usb *entry_priv = entry->priv_data;
>  	int pipe;
>  
> +	entry->flags = 0;
> +
>  	if (entry->queue->qid == QID_RX) {
>  		pipe = usb_rcvbulkpipe(usb_dev, entry->queue->usb_endpoint);
>  		usb_fill_bulk_urb(entry_priv->urb, usb_dev, pipe,
> @@ -413,8 +458,6 @@ void rt2x00usb_clear_entry(struct queue_entry *entry)
>  
>  		set_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags);
>  		usb_submit_urb(entry_priv->urb, GFP_ATOMIC);
> -	} else {
> -		entry->flags = 0;
>  	}
>  }
>  EXPORT_SYMBOL_GPL(rt2x00usb_clear_entry);
> @@ -659,6 +702,12 @@ int rt2x00usb_probe(struct usb_interface *usb_intf,
>  
>  	rt2x00_set_chip_intf(rt2x00dev, RT2X00_CHIP_INTF_USB);
>  
> +        tasklet_init(&rt2x00dev->tx_tasklet, rt2x00usb_tasklet_txdone,
> +		     (unsigned long)rt2x00dev);
> +
> +        tasklet_init(&rt2x00dev->rx_tasklet, rt2x00usb_tasklet_rxdone,
> +		     (unsigned long)rt2x00dev);
> +
>  	retval = rt2x00usb_alloc_reg(rt2x00dev);
>  	if (retval)
>  		goto exit_free_device;
> 
> _______________________________________________
> users mailing list
> users at rt2x00.serialmonkey.com
> http://rt2x00.serialmonkey.com/mailman/listinfo/users_rt2x00.serialmonkey.com
> 





More information about the users mailing list