On Fri, 2011-12-09 at 20:38 +0100, Johannes Tenschert wrote:
> Signed-off-by: Johannes Tenschert 
> <[email protected]>

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
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

Reply via email to