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

Mark Asselstine asselsm at gmail.com
Thu Jul 22 13:05:29 AEST 2010


On Wed, Jul 21, 2010 at 6:32 PM, Ivo van Doorn <ivdoorn at gmail.com> wrote:
> 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.
>
> 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>
> ---

Ivo,  I know it has been far too long since I have done any work on
this project and I am far from an expert so feel free hit the delete
key... that being said. Tasklets are used less and less in the kernel
as there are now other mechanisms which offer the same flexibility.
Seeing this trend makes me feel rt2x00 is moving in the opposite
direction. Have you looked at threaded interrupts (which are now in
mainline) as an alternative? If I can get my development environment
setup quickly I will try to do better then these few words and
actually attempt an implementation if you think it is worth it.

Regards,
Mark

> 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