[rt2x00-users] [PATCH 1/5] rt2x00: Move direct access to queue->entries to rt2x00queue.c
Helmut Schaa
helmut.schaa at googlemail.com
Tue Aug 17 06:55:41 UTC 2010
Ivo, did you plan to merge this series? Just wanted to rebase my watchdog
patches on top of these here but they are not in rt2x00 git yet, right?
However, I didn't have time to review them ;)
Helmut
Am Saturday 14 August 2010 schrieb Ivo van Doorn:
> All access to queue->entries through the Q_INDEX/Q_INDEX_DONE
> variables must be done using spinlock protection. It is best
> to manage this completely from rt2x00queue.c.
>
> For safely looping through all entries in the queue, the function
> rt2x00queue_for_each_entry is added which will walk from Q_INDEX_DONE
> to Q_INDEX in a safe manner.
>
> This also fixes rt2x00usb which walked the entries list from
> 0 to length to kill each entry (killing entries must be done
> from Q_INDEX_DONE to Q_INDEX to enforce TX status reporting to
> occur in the correct order.
>
> Signed-off-by: IvDoorn at gmail.com>
> ---
> drivers/net/wireless/rt2x00/rt2800pci.c | 4 +-
> drivers/net/wireless/rt2x00/rt2x00queue.c | 36 +++++++++++
> drivers/net/wireless/rt2x00/rt2x00queue.h | 12 ++++
> drivers/net/wireless/rt2x00/rt2x00usb.c | 98 +++++++++--------------------
> 4 files changed, 81 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> index af1c691..a5e5870 100644
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> @@ -629,7 +629,7 @@ static void rt2800pci_write_tx_desc(struct queue_entry *entry,
> static void rt2800pci_kick_tx_queue(struct data_queue *queue)
> {
> struct rt2x00_dev *rt2x00dev = queue->rt2x00dev;
> - unsigned int idx = queue->index[Q_INDEX];
> + struct queue_entry *entry = rt2x00queue_get_entry(queue, Q_INDEX);
> unsigned int qidx = 0;
>
> if (queue->qid == QID_MGMT)
> @@ -637,7 +637,7 @@ static void rt2800pci_kick_tx_queue(struct data_queue *queue)
> else
> qidx = queue->qid;
>
> - rt2800_register_write(rt2x00dev, TX_CTX_IDX(qidx), idx);
> + rt2800_register_write(rt2x00dev, TX_CTX_IDX(qidx), entry->entry_idx);
> }
>
> static void rt2800pci_kill_tx_queue(struct data_queue *queue)
> diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
> index 189eaf7..2486963 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00queue.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
> @@ -625,6 +625,42 @@ int rt2x00queue_update_beacon(struct rt2x00_dev *rt2x00dev,
> return 0;
> }
>
> +void rt2x00queue_for_each_entry(struct data_queue *queue,
> + void (*fn)(struct queue_entry *entry))
> +{
> + unsigned long irqflags;
> + unsigned int index;
> + unsigned int index_done;
> + unsigned int i;
> +
> + /*
> + * Only protect the range we are going to loop over,
> + * if during our loop a extra entry is set to pending
> + * it should not be kicked during this run, since it
> + * is part of another TX operation.
> + */
> + spin_lock_irqsave(&queue->lock, irqflags);
> + index = queue->index[Q_INDEX];
> + index_done = queue->index[Q_INDEX_DONE];
> + spin_unlock_irqrestore(&queue->lock, irqflags);
> +
> + /*
> + * Start from the TX done pointer, this guarentees that we will
> + * send out all frames in the correct order.
> + */
> + if (index_done < index) {
> + for (i = index_done; i < index; i++)
> + fn(&queue->entries[i]);
> + } else {
> + for (i = index_done; i < queue->limit; i++)
> + fn(&queue->entries[i]);
> +
> + for (i = 0; i < index; i++)
> + fn(&queue->entries[i]);
> + }
> +}
> +EXPORT_SYMBOL_GPL(rt2x00queue_for_each_entry);
> +
> struct data_queue *rt2x00queue_get_queue(struct rt2x00_dev *rt2x00dev,
> const enum data_queue_qid queue)
> {
> diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.h b/drivers/net/wireless/rt2x00/rt2x00queue.h
> index 2d3bf84..4de6596 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00queue.h
> +++ b/drivers/net/wireless/rt2x00/rt2x00queue.h
> @@ -571,6 +571,18 @@ struct data_queue_desc {
> queue_loop(__entry, (__dev)->tx, queue_end(__dev))
>
> /**
> + * rt2x00queue_for_each_entry - Loop through all entries in the queue
> + * @queue: Pointer to @data_queue
> + * @fn: The function to call for each &struct queue_entry
> + *
> + * This will walk through all entries in the queue, in chronological
> + * order. This means it will start at the current Q_INDEX_DONE pointer
> + * and will walk through the queue until it reaches the Q_INDEX pointer.
> + */
> +void rt2x00queue_for_each_entry(struct data_queue *queue,
> + void (*fn)(struct queue_entry *entry));
> +
> +/**
> * rt2x00queue_empty - Check if the queue is empty.
> * @queue: Queue to check if empty.
> */
> diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
> index 52ed3f4..17e42ea 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
> @@ -232,88 +232,52 @@ static inline void rt2x00usb_kick_tx_entry(struct queue_entry *entry)
> struct queue_entry_priv_usb *entry_priv = entry->priv_data;
> u32 length;
>
> - if (test_and_clear_bit(ENTRY_DATA_PENDING, &entry->flags)) {
> - /*
> - * USB devices cannot blindly pass the skb->len as the
> - * length of the data to usb_fill_bulk_urb. Pass the skb
> - * to the driver to determine what the length should be.
> - */
> - length = rt2x00dev->ops->lib->get_tx_data_len(entry);
> + if (!test_and_clear_bit(ENTRY_DATA_PENDING, &entry->flags))
> + return;
> +
> + /*
> + * USB devices cannot blindly pass the skb->len as the
> + * length of the data to usb_fill_bulk_urb. Pass the skb
> + * to the driver to determine what the length should be.
> + */
> + length = rt2x00dev->ops->lib->get_tx_data_len(entry);
>
> - usb_fill_bulk_urb(entry_priv->urb, usb_dev,
> - usb_sndbulkpipe(usb_dev, entry->queue->usb_endpoint),
> - entry->skb->data, length,
> - rt2x00usb_interrupt_txdone, entry);
> + usb_fill_bulk_urb(entry_priv->urb, usb_dev,
> + usb_sndbulkpipe(usb_dev, entry->queue->usb_endpoint),
> + entry->skb->data, length,
> + rt2x00usb_interrupt_txdone, entry);
>
> - usb_submit_urb(entry_priv->urb, GFP_ATOMIC);
> - }
> + usb_submit_urb(entry_priv->urb, GFP_ATOMIC);
> }
>
> void rt2x00usb_kick_tx_queue(struct data_queue *queue)
> {
> - unsigned long irqflags;
> - unsigned int index;
> - unsigned int index_done;
> - unsigned int i;
> -
> - /*
> - * Only protect the range we are going to loop over,
> - * if during our loop a extra entry is set to pending
> - * it should not be kicked during this run, since it
> - * is part of another TX operation.
> - */
> - spin_lock_irqsave(&queue->lock, irqflags);
> - index = queue->index[Q_INDEX];
> - index_done = queue->index[Q_INDEX_DONE];
> - spin_unlock_irqrestore(&queue->lock, irqflags);
> -
> - /*
> - * Start from the TX done pointer, this guarentees that we will
> - * send out all frames in the correct order.
> - */
> - if (index_done < index) {
> - for (i = index_done; i < index; i++)
> - rt2x00usb_kick_tx_entry(&queue->entries[i]);
> - } else {
> - for (i = index_done; i < queue->limit; i++)
> - rt2x00usb_kick_tx_entry(&queue->entries[i]);
> -
> - for (i = 0; i < index; i++)
> - rt2x00usb_kick_tx_entry(&queue->entries[i]);
> - }
> + rt2x00queue_for_each_entry(queue, rt2x00usb_kick_tx_entry);
> }
> EXPORT_SYMBOL_GPL(rt2x00usb_kick_tx_queue);
>
> -void rt2x00usb_kill_tx_queue(struct data_queue *queue)
> +static inline void rt2x00usb_kill_tx_entry(struct queue_entry *entry)
> {
> - struct queue_entry_priv_usb *entry_priv;
> - struct queue_entry_priv_usb_bcn *bcn_priv;
> - unsigned int i;
> - bool kill_guard;
> + struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
> + struct queue_entry_priv_usb *entry_priv = entry->priv_data;
> + struct queue_entry_priv_usb_bcn *bcn_priv = entry->priv_data;
>
> - /*
> - * When killing the beacon queue, we must also kill
> - * the beacon guard byte.
> - */
> - kill_guard =
> - (queue->qid == QID_BEACON) &&
> - (test_bit(DRIVER_REQUIRE_BEACON_GUARD, &queue->rt2x00dev->flags));
> + if (!test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags))
> + return;
> +
> + usb_kill_urb(entry_priv->urb);
>
> /*
> - * Cancel all entries.
> + * Kill guardian urb (if required by driver).
> */
> - for (i = 0; i < queue->limit; i++) {
> - entry_priv = queue->entries[i].priv_data;
> - usb_kill_urb(entry_priv->urb);
> + if ((entry->queue->qid == QID_BEACON) &&
> + (test_bit(DRIVER_REQUIRE_BEACON_GUARD, &rt2x00dev->flags)))
> + usb_kill_urb(bcn_priv->guardian_urb);
> +}
>
> - /*
> - * Kill guardian urb (if required by driver).
> - */
> - if (kill_guard) {
> - bcn_priv = queue->entries[i].priv_data;
> - usb_kill_urb(bcn_priv->guardian_urb);
> - }
> - }
> +void rt2x00usb_kill_tx_queue(struct data_queue *queue)
> +{
> + rt2x00queue_for_each_entry(queue, rt2x00usb_kill_tx_entry);
> }
> EXPORT_SYMBOL_GPL(rt2x00usb_kill_tx_queue);
>
>
More information about the users
mailing list