On 05/13/10 11:45, Ivo Van Doorn wrote:
> On Thu, May 13, 2010 at 11:36 AM, Gertjan van Wingerde
> <gwingerde@xxxxxxxxx> wrote:
>> This allows rt2x00debug_dump_frame to be used from everywhere.
>>
>> This is preparation for beacon writing clean ups.
>>
>> Signed-off-by: Gertjan van Wingerde <gwingerde@xxxxxxxxx>
>> ---
>> drivers/net/wireless/rt2x00/rt2x00.h | 35 +++++++++++++++++++++++++++++
>> drivers/net/wireless/rt2x00/rt2x00debug.c | 1 +
>> drivers/net/wireless/rt2x00/rt2x00dump.h | 20 ----------------
>> drivers/net/wireless/rt2x00/rt2x00lib.h | 10 --------
>> 4 files changed, 36 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
>> index 6c1ff4c..1329f6c 100644
>> --- a/drivers/net/wireless/rt2x00/rt2x00.h
>> +++ b/drivers/net/wireless/rt2x00/rt2x00.h
>> @@ -1015,6 +1015,41 @@ struct queue_entry *rt2x00queue_get_entry(struct data_queue *queue,
>> enum queue_index index);
>>
>> /*
>> + * Debugfs handlers.
>> + */
>> +/**
>> + * enum rt2x00_dump_type - Frame type
>> + *
>> + * These values are used for the indicate the type of frame that is being
>> + * dumped:
>> + * @DUMP_FRAME_RXDONE: This frame has been received by the hardware.
>> + * @DUMP_FRAME_TX: This frame is queued for transmission to the hardware.
>> + * @DUMP_FRAME_TXDONE: This frame indicates the device has handled
>> + * the tx event which has either succeeded or failed. A frame
>> + * with this type should also have been reported with as a
>> + * %DUMP_FRAME_TX frame.
>> + * @DUMP_FRAME_BEACON: This beacon frame is queued for transmission to the
>> + * hardware.
>> + */
>> +enum rt2x00_dump_type {
>> + DUMP_FRAME_RXDONE = 1,
>> + DUMP_FRAME_TX = 2,
>> + DUMP_FRAME_TXDONE = 3,
>> + DUMP_FRAME_BEACON = 4,
>> +};
>
> Can't this stay in rt2x00dump.h? The rt2x00dump.h is part of the
> public interface towards
> userspace which can use these defined to determine the frame type.
> We can safely include this header in rt2x00.h since it contains the
> public interface anyway.
Hmpf, it looks like there is a lot of unspecified assumptions about the placement
of information in files and for some functionality. We must get better in documenting
this.
Although it is a bit strange to have a public interface to userspace defined this deep
in the kernel source tree, I'll move it back and put a big fat comment is there that this
actually is the public interface towards userspace and that all this information is to
be kept together.
>
>> +#ifdef CONFIG_RT2X00_LIB_DEBUGFS
>> +void rt2x00debug_dump_frame(struct rt2x00_dev *rt2x00dev,
>> + enum rt2x00_dump_type type, struct sk_buff *skb);
>> +#else
>> +static inline void rt2x00debug_dump_frame(struct rt2x00_dev *rt2x00dev,
>> + enum rt2x00_dump_type type,
>> + struct sk_buff *skb)
>> +{
>> +}
>> +#endif /* CONFIG_RT2X00_LIB_DEBUGFS */
>
> Could you add some documentation to this function?
>
Off course I can; although that is a bit more than just moving declarations ;-)
---
Gertjan.
--
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
