[rt2x00-users] [RFC 1/2] rt2x00: Make rt2x00_queue_entry_for_each more flexible

[Ivo Van Doorn]

Hi,

>> > Allow passing a void pointer to rt2x00_queue_entry_for_each which in
>> > turn in provided to the callback function.
>> >
>> > Furthermore, allow the callback function to stop processing by returning
>> > true. And also notify the caller of rt2x00_queue_entry_for_each if the
>> > loop was canceled by the callback.
>> >
>> > No functional changes, just preparation for an upcoming patch.
>>
>> Nice changes, I was thinking about returning boolean as well,
>> but didn't had any user for it at that time. :)
>>
>> > diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
>> > index fbe735f..2b65ef2 100644
>> > --- a/drivers/net/wireless/rt2x00/rt2x00usb.c
>> > +++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
>> > @@ -230,7 +230,7 @@ static void rt2x00usb_interrupt_txdone(struct urb *urb)
>> >        queue_work(rt2x00dev->workqueue, &rt2x00dev->txdone_work);
>> >  }
>> >
>> > -static void rt2x00usb_kick_tx_entry(struct queue_entry *entry)
>> > +static bool rt2x00usb_kick_tx_entry(struct queue_entry *entry, void* data)
>> >  {
>> >        struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
>> >        struct usb_device *usb_dev = to_usb_device_intf(rt2x00dev->dev);
>> > @@ -240,7 +240,7 @@ static void rt2x00usb_kick_tx_entry(struct queue_entry *entry)
>> >
>> >        if (!test_and_clear_bit(ENTRY_DATA_PENDING, &entry->flags) ||
>> >            test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags))
>> > -               return;
>> > +               return false;
>>
>> Since the queue is completely FIFO, as soon as we hit the first entry
>> which does not have the ENTRY_DATA_PENDING set, means that all
>> subsequent entries also will not have that flag.
>>
>> For ENTRY_DATA_STATUS_PENDING it is a bit different, but it at least implies
>> that all subsequent entries do not have to be kicked (as the presence of the
>> ENTRY_DATA_STATUS_PENDING flag implies that all subsequent entries have
>> already been kicked).
>>
>> So we could return true here, and break looping.
>>
>> > -static void rt2x00usb_kick_rx_entry(struct queue_entry *entry)
>> > +static bool rt2x00usb_kick_rx_entry(struct queue_entry *entry, void* data)
>> >  {
>> >        struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
>> >        struct usb_device *usb_dev = to_usb_device_intf(rt2x00dev->dev);
>> > @@ -332,7 +333,7 @@ static void rt2x00usb_kick_rx_entry(struct queue_entry *entry)
>> >
>> >        if (test_and_set_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) ||
>> >            test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags))
>> > -               return;
>> > +               return false;
>>
>> Same here. It would be save to return true here, and break looping.
>>
>> > -static void rt2x00usb_flush_entry(struct queue_entry *entry)
>> > +static bool rt2x00usb_flush_entry(struct queue_entry *entry, void* data)
>> >  {
>> >        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;
>> >
>> >        if (!test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags))
>> > -               return;
>> > +               return false;
>>
>> Not sure about the proper return status here. I think that URBs are being
>> completed in the order they were send to the device, which implies we could
>> return true here as well.
>
> Yeah, I haven't reviewed the USB changes much and just took the safe approach
> of returning false everywhere :)
>
> Feel free to pick the patch up and do the cleanups you suggested by yourself
> or wait for me to respin (and rethink) the other patch as well (which might
> take quite some time due to some unresolved issues Gertjan pointed out).

I've applied my changes, and fixed one compile warning from your original patch.

Thanks,

Ivo