On Wed, May 12, 2010 at 11:53 AM, Gertjan van Wingerde
<gwingerde@xxxxxxxxx> wrote:
> On Wed, May 12, 2010 at 9:06 AM, Ivo Van Doorn <ivdoorn@xxxxxxxxx> wrote:
>> On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
>> <gwingerde@xxxxxxxxx> wrote:
>>> Not all the devices require a TX descriptor to be written (i.e. rt2800
>>> device don't require them). Push down the creation of the TX descriptor
>>> to the device drivers so that they can decide for themselves whether
>>> a TX descriptor is to be created.
>>>
>>> Signed-off-by: Gertjan van Wingerde <gwingerde@xxxxxxxxx>
>>> ---
>>> drivers/net/wireless/rt2x00/rt2400pci.c | 16 ++++++++++------
>>> drivers/net/wireless/rt2x00/rt2500pci.c | 16 ++++++++++------
>>> drivers/net/wireless/rt2x00/rt2500usb.c | 11 +++++++++++
>>> drivers/net/wireless/rt2x00/rt2800pci.c | 17 +++++++++++++++++
>>> drivers/net/wireless/rt2x00/rt2800usb.c | 17 +++++++++++++++++
>>> drivers/net/wireless/rt2x00/rt2x00debug.c | 1 +
>>> drivers/net/wireless/rt2x00/rt2x00queue.c | 10 +---------
>>> drivers/net/wireless/rt2x00/rt61pci.c | 11 +++++++++++
>>> drivers/net/wireless/rt2x00/rt73usb.c | 11 +++++++++++
>>> 9 files changed, 89 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
>>> index def3fa4..741c531 100644
>>> --- a/drivers/net/wireless/rt2x00/rt2400pci.c
>>> +++ b/drivers/net/wireless/rt2x00/rt2400pci.c
>>> @@ -33,6 +33,7 @@
>>> #include <linux/eeprom_93cx6.h>
>>>
>>> #include "rt2x00.h"
>>> +#include "rt2x00lib.h"
>>> #include "rt2x00pci.h"
>>> #include "rt2400pci.h"
>>
>> rt2x00lib.h must not be used in the drivers. It is for the rt2x00lib
>> internal files only.
>
> OK. This include is/was necessary for the rt2x00debug_dump_frame call.
> So this may be handled with your next issue.
>
>>
>>> @@ -1074,9 +1075,6 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
>>> struct txentry_desc *txdesc)
>>> {
>>> struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
>>> - struct queue_entry_priv_pci *entry_priv = entry->priv_data;
>>> - struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
>>> - u32 word;
>>> u32 reg;
>>>
>>> /*
>>> @@ -1089,9 +1087,15 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
>>>
>>> rt2x00queue_map_txskb(rt2x00dev, entry->skb);
>>>
>>> - rt2x00_desc_read(entry_priv->desc, 1, &word);
>>> - rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
>>> - rt2x00_desc_write(entry_priv->desc, 1, word);
>>> + /*
>>> + * Write the TX descriptor for the beacon.
>>> + */
>>> + rt2400pci_write_tx_desc(rt2x00dev, entry->skb, txdesc);
>>> +
>>> + /*
>>> + * Dump beacon to userspace through debugfs.
>>> + */
>>> + rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
>>
>> The goal for rt2x00debug was that the logic must be inside rt2x00lib
>> as much as possible.
>> This can/should be moved into rt2x00lib where write_beacon() is being called.
>>
>
> Yes, I wasn't too happy about this part about the patch, but couldn't
> find a better solution. The problem is that with the patch the frame
> cannot be dumped before the call to write_beacon, as the descriptor
> hasn't been set up yet. Also, it cannot be dumped after the call to
> write_beacon as most of the write_beacon functions actually free the
> skb with the beacon.
> So, I ran out of ideas as to how to keep rt2x00debug only inside
> rt2x00lib. I'm open for suggestions, though.
Well ok, if there aren't obvious alternatives this change is fine.
However, please move the declaration of rt2x00debug_dump_frame()
into rt2x00.h? That way we don't have to include the rt2x00lib.h header.
Ivo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
