On Fri, 2011-12-09 at 20:38 +0100, Johannes Tenschert wrote:
> Signed-off-by: Johannes Tenschert <Johannes.Tenschert@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
The BCM_DBG_PRINT macro is really ugly and verbose.
You could do a few things to make it clearer.
1. Rename the macro to BCM_DEBUG or bcm_debug or bcm_dbg
2. Add #define DBG_LVL_0 0 for equivalence to the 0 use
3. Add #define DBG_TYPE_CMHOST CMHOST
4. Add DBG_TYPE_ and DBG_LVL_ to the BCM_DBG_PRINT macro
and delete those prefixes from the uses.
5. Add missing newline terminations to all the formats.
So the macro becomes:
#define bcm_debug(Adapter, Type, SubType, Level, fmt, ...) \
do { \
if (DBG_TYPE_PRINTK == DBG_TYPE_##Type) \
pr_info("%s:" fmt, __func__, ##__VA_ARGS__); \
else if ((Adapter) && \
((DBG_LVL_##Level & DBG_LVL_BITMASK) <= \
(Adapter)->stDebugState.debug_level) && \
(DBG_TYPE_##Type & (Adapter)->stDebugState.type) && \
(SubType & \
(Adapter)->stDebugState.subtype[DBG_TYPE_##Type])) { \
if (DBG_LVL_##Level & DBG_NO_FUNC_PRINT) \
printk(KERN_DEBUG fmt, ##__VA_ARGS__); \
else \
printk(KERN_DEBUG "%s:" fmt, \
__func__, ##__VA_ARGS__); \
} \
} while (0)
(though I think those printk(KERN_DEBUG should just be pr_debug)
> diff --git a/drivers/staging/bcm/HandleControlPacket.c b/drivers/staging/bcm/HandleControlPacket.c
[]
> @@ -26,26 +26,33 @@ static VOID handle_rx_control_packet(PMINI_ADAPTER Adapter, struct sk_buff *skb)
>
> switch (usStatus) {
> case CM_RESPONSES: /* 0xA0 */
> - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, CP_CTRL_PKT, DBG_LVL_ALL, "MAC Version Seems to be Non Multi-Classifier, rejected by Driver");
> + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, CP_CTRL_PKT,
> + DBG_LVL_ALL,
> + "MAC Version Seems to be Non Multi-Classifier, rejected by Driver");
Than then this use becomes shorter
bcm_debug(Adapter, OTHERS, CP_CTRL_PKT, ALL,
"MAC Version Seems to be Non Multi-Classifier, rejected by Driver\n");
[]
> case LINK_CONTROL_RESP: /* 0xA2 */
> case STATUS_RSP: /* 0xA1 */
> - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, CP_CTRL_PKT, DBG_LVL_ALL, "LINK_CONTROL_RESP");
> + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, CP_CTRL_PKT,
> + DBG_LVL_ALL, "LINK_CONTROL_RESP");
bcm_debug(Adapter, OTHERS, CP_CTRL_PKT, ALL,
"LINK_CONTROL_RESP\n");
etc.
Also, when you're splitting long lines, it's better
though not strictly necessary to align the indentation
to the appropriate preceding parenthesis of the previous
line as above.
cheers, Joe
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
