bcm: Fix codingstyle problems

Signed-off-by Devendra Naga <[email protected]>

---
 drivers/staging/bcm/Debug.h |    6 +++---

 1 file changed, 3 insertions(+), 3 deletions(-)

Hi Devendra,

Quoted 5 lines · Devendra Naga

Signed-off-by Devendra Naga <[email protected]>

---
 drivers/staging/bcm/Debug.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

I understand this is a simple change, but please add a descriptive change log to your patch for consistency.

Quoted 8 lines · Kevin McKinney

Hi Devendra,

Quoted 5 lines · Devendra Naga

Signed-off-by Devendra Naga <[email protected]>

---
 drivers/staging/bcm/Debug.h |    6 +++---

 1 file changed, 3 insertions(+), 3 deletions(-)

Quoted 2 lines

I understand this is a simple change, but please add a descriptive change log to your patch for consistency.

Hello as well.

Devendra, sometimes the simpler change isn't the best change. Consider converting some of these macros into functions.

Hello Joe,

Quoted 12 lines · Joe Perches

Quoted 10 lines · Kevin McKinney

Hi Devendra,

Quoted 5 lines · Devendra Naga

Signed-off-by Devendra Naga <[email protected]>

---
 drivers/staging/bcm/Debug.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

I understand this is a simple change, but please add a descriptive change log to your patch for consistency.

Quoted 5 lines

Hello as well.

Devendra, sometimes the simpler change isn't the best change. Consider converting some of these macros into functions.

So How about converting BCM_DEBUG_PRINT, BCM_DEBUG_PRINT_BUFFER, BCM_SHOW_DEBUG_BITMAP into a single function? and using a type to print the corresponding values?

I think it'd be better with 3 functions.

Maybe something like the below:

But read the TODO file.  I'm not sure if
there's any value in working on bcm at all.
---

 create mode 100644 drivers/staging/bcm/Debug.c

drivers/staging/bcm/Debug.c new file +0 -0

new file mode 100644
index 0000000..3ca720c

drivers/staging/bcm/Debug.c new file +52 -0

+
+void bcm_debug_print(PMINI_ADAPTER Adapter, int Type, int SubType,
+                    int dbg_level, const char *fmt, ...)
+{
+       struct va_format vaf;
+       va_list args;
+
+       va_start(args, fmt);
+
+       vaf.fmt = fmt;
+       vaf.va = &args;
+
+       if (DBG_TYPE_PRINTK == Type)
+               pr_info("%s: %pV", __func__, &vaf);
+       else if (Adapter &&
+                (dbg_level & DBG_LVL_BITMASK) <= Adapter->stDebugState.debug_level &&
+                (Type & Adapter->stDebugState.type) &&
+                (SubType & Adapter->stDebugState.subtype[Type])) {
+               if (dbg_level & DBG_NO_FUNC_PRINT)
+                       printk(KERN_DEBUG "%pV" , &vaf);
+               else
+                       printk(KERN_DEBUG "%s: %pV", __func__, &vaf);
+       }
+
+       va_end(args);
+}
+
+void bcm_debug_print_buffer(PMINI_ADAPTER Adapter, int Type, int SubType,
+                           int dbg_level, const void *buffer, size_t bufferlen)
+{
+       if (DBG_TYPE_PRINTK == Type ||
+           (Adapter &&
+            (dbg_level & DBG_LVL_BITMASK) <= Adapter->stDebugState.debug_level  &&
+            (Type & Adapter->stDebugState.type) &&
+            (SubType & Adapter->stDebugState.subtype[Type]))) {
+               printk(KERN_DEBUG "%s:\n", __func__);
+               print_hex_dump(KERN_DEBUG, " ", DUMP_PREFIX_OFFSET,
+                              16, 1, buffer, bufferlen, false);
+       }
+}
+
+void bcm_show_debug_bitmap(PMINI_ADAPTER Adapter)
+{
+       int i;
+       for (i = 0; i < (NUMTYPES * 2) + 1; i++) {
+               if ((i == 1) || (i == 2) || (i == 4) || (i == 8)) {
+                       BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0,
+                                       "subtype[%d] = 0x%08x\n",
+                                       i, Adapter->stDebugState.subtype[i]);
+               }
+       }
+}

drivers/staging/bcm/Debug.h +20 -42

index 420382d..0fbb206 100644
@@ -203,57 +203,35 @@ typedef struct _S_BCM_DEBUG_STATE {
         * corresponding to valid Type values. Hence we use the 'Type' field
         * as the index value, ignoring the array entries 0,3,5,6,7 !
         */
-       UINT subtype[(NUMTYPES*2)+1];
+       UINT subtype[(NUMTYPES * 2) + 1];
        UINT debug_level;
  } S_BCM_DEBUG_STATE;
  /* Instantiated in the Adapter structure */
  /* We'll reuse the debug level parameter to include a bit (the MSB) to indicate whether or not
  * we want the function's name printed.  */
-#define DBG_NO_FUNC_PRINT      1 << 31
+#define DBG_NO_FUNC_PRINT      (1 << 31)
  #define DBG_LVL_BITMASK                0xFF
 
  //--- Only for direct printk's; "hidden" to API.
  #define DBG_TYPE_PRINTK                3
 
-#define BCM_DEBUG_PRINT(Adapter, Type, SubType, dbg_level, string, args...) \
-       do {                                                            \
-               if (DBG_TYPE_PRINTK == Type)                            \
-                       pr_info("%s:" string, __func__, ##args);        \
-               else if (Adapter &&                                     \
-                        (dbg_level & DBG_LVL_BITMASK) <= Adapter->stDebugState.debug_level && \
-                        (Type & Adapter->stDebugState.type) &&         \
-                        (SubType & Adapter->stDebugState.subtype[Type])) { \
-                       if (dbg_level & DBG_NO_FUNC_PRINT)              \
-                               printk(KERN_DEBUG string, ##args);      \
-                       else                                            \
-                               printk(KERN_DEBUG "%s:" string, __func__, ##args);      \
-               }                                                       \
-       } while (0)
-
-#define BCM_DEBUG_PRINT_BUFFER(Adapter, Type, SubType, dbg_level,  buffer, bufferlen) do { \
-       if (DBG_TYPE_PRINTK == Type ||                                  \
-           (Adapter &&                                                 \
-            (dbg_level & DBG_LVL_BITMASK) <= Adapter->stDebugState.debug_level  && \
-            (Type & Adapter->stDebugState.type) &&                     \
-            (SubType & Adapter->stDebugState.subtype[Type]))) {        \
-               printk(KERN_DEBUG "%s:\n", __func__);                   \
-               print_hex_dump(KERN_DEBUG, " ", DUMP_PREFIX_OFFSET,     \
-                              16, 1, buffer, bufferlen, false);        \
-       }                                                               \
-} while(0)
-
-
-#define BCM_SHOW_DEBUG_BITMAP(Adapter) do { \
-       int i;                                                                  \
-       for (i=0; i<(NUMTYPES*2)+1; i++) {              \
-               if ((i == 1) || (i == 2) || (i == 4) || (i == 8)) {             \
-               /* CAUTION! Forcefully turn on ALL debug paths and subpaths!    \
-               Adapter->stDebugState.subtype[i] = 0xffffffff;  */ \
-               BCM_DEBUG_PRINT (Adapter, DBG_TYPE_PRINTK, 0, 0, "subtype[%d] = 0x%08x\n",      \
-               i, Adapter->stDebugState.subtype[i]);   \
-               }       \
-       }               \
-} while (0)
+struct _MINI_ADAPTER;
 
-#endif
+__printf(5, 6)
+void bcm_debug_print(struct _MINI_ADAPTER *Adapter, int Type, int SubType,
+                    int dbg_level, const char *fmt, ...);
+void bcm_debug_print_buffer(struct _MINI_ADAPTER *Adapter, int Type, int SubType,
+                           int dbg_level, const void *buffer, size_t bufferlen);
+void bcm_show_debug_bitmap(struct _MINI_ADAPTER *Adapter);
+
+#define BCM_DEBUG_PRINT(Adapter, Type, SubType, dbg_level, fmt, ...)   \
+       bcm_debug_print(Adapter, Type, SubType, dbg_level, fmt, ##__VA_ARGS__)
 
+#define BCM_DEBUG_PRINT_BUFFER(Adapter, Type, SubType, dbg_level,      \
+                              buffer, bufferlen)                       \
+       bcm_debug_print_buffer(Adapter, Type, SubType, dbg_level,       \
+                              buffer, bufferlen)
+#define BCM_SHOW_DEBUG_BITMAP(Adapter)                                 \
+       bcm_show_debug_bitmap(Adapter)
+
+#endif

drivers/staging/bcm/Makefile +1 -0

index 652b7f8..a858ac1 100644
@@ -7,6 +7,7 @@ obj-$(CONFIG_BCM_WIMAX) +=      bcm_wimax.o
  bcm_wimax-y :=  InterfaceDld.o InterfaceIdleMode.o InterfaceInit.o InterfaceRx.o \
                InterfaceIsr.o InterfaceMisc.o InterfaceTx.o \
                CmHost.o IPv6Protocol.o Qos.o Transmit.o\
+               Debug.o\
                Bcmnet.o DDRInit.o HandleControlPacket.o\
                LeakyBucket.o Misc.o sort.o Bcmchar.o hostmibs.o PHSModule.o\

               led_control.o nvm.o vendorspecificextn.o --
1.7.6.rc3

On Mon, 2012-05-28 at 09:53 +0530, devendra.aaru wrote:
But read the TODO file. I'm not sure if there's any value in working on bcm at all.

Quoted 3 lines · Joe Perches

On Mon, 2012-05-28 at 09:53 +0530, devendra.aaru wrote:
But read the TODO file. I'm not sure if there's any value in working on bcm at all.

The bcm driver is pretty crappy and can't maintain a connection for very long but we don't have a replacement, so it's worth fixing.

Quoted 3 lines · Joe Perches

On Mon, 2012-05-28 at 09:53 +0530, devendra.aaru wrote:
But read the TODO file.  I'm not sure if there's any value in working on bcm at all.

In America Sprint was the only company doing WiMax and I think they're moving away from it now.  But WiMax and this chip are used in Africa.

Hi Devendra,

Quoted 5 lines · Devendra Naga

Signed-off-by Devendra Naga <[email protected]>

---
 drivers/staging/bcm/Debug.h |    6 +++---

 1 file changed, 3 insertions(+), 3 deletions(-)

I understand this is a simple change, but please add a descriptive change log to your patch for consistency.

Thanks,
Kevin