bcm: Fix codingstyle problems
PATCH 1/1 bcm: Fix codingstyle problems
--- drivers/staging/bcm/Debug.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/staging/bcm/Debug.h +3 -3
@@ -240,16 +240,16 @@ typedef struct _S_BCM_DEBUG_STATE {print_hex_dump (KERN_DEBUG ," " ,DUMP_PREFIX_OFFSET , \16 ,1 , buffer, bufferlen, false); \ } \ -} while(0) +} while (0)#define BCM_SHOW_DEBUG_BITMAP (Adapter) do { \ int i; \ - for (i=0; i<(NUMTYPES*2)+1; 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", \ +BCM_DEBUG_PRINT (Adapter,DBG_TYPE_PRINTK ,0 ,0 ,"subtype[%d] = 0x%08x\n" , \ i, Adapter->stDebugState.subtype[i]); \ } \ } \
Signature · 1 line
1.7.9.5
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
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.
Hello as well.
Devendra, sometimes the simpler change isn't the best change. Consider converting some of these macros into functions.
Hello Joe,
Quoted 9 lines · Joe Perches
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 8 lines
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.
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?
Quoted 1 line
Thanks,
Devendra.
Quoted 15 lines · devendra.aaru
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 8 lines
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_BITMAPinto 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 +53 -0
@@ -0,0 +1,53 @@ +#include "headers.h" + +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
77 lines not on this page · raw message
drivers/staging/bcm/Makefile +1 -0
@@ -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
Signature · 1 line
1.7.6.rc3
Hello Joe,
Thanks for a very quick reply with a code suggestion.
Just a small change at your code,
Quoted 17 lines · Joe Perches
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.cdrivers/staging/bcm/Debug.c new file +0
-0
drivers/staging/bcm/Debug.c line 1
Quoted 1 line
drivers/staging/bcm/Debug.c new file +1
-0@@ -0,0 +1,53 @@ +#include "headers.h"
/**
* for the BCM_DEBUG_PRINT macro
*/
+#include "debug.h"
Quoted 145 lines
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@@ -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@@ -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\
Quoted 6 lines
led_control.o nvm.o vendorspecificextn.o --
1.7.6.rc3
The TODO says these are developer debug prints, and may be set for removal. I think its better to have macros, anyway they are going to be removed in future.
Thanks,
Devendra.
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.
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.
regards,
dan carpenter
Quoted 7 lines · Dan Carpenter
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.
Well, the patch I suggested drops the size by 25%/100K so it's probably worth something.
$ size drivers/staging/bcm/*.ko* text data bss dec hex filename 260530 7248 53632 321410 4e782 drivers/staging/bcm/bcm_wimax.ko.new 338001 7192 84888 430081 69001 drivers/staging/bcm/bcm_wimax.ko.old
Quoted 8 lines · Dan Carpenter
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.
In this case, is it worth trying to get the hardware for this driver so I can test?
Thanks,
Kevin
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.
Thanks for reviewing the patch, i will add a description and send out V2 of this patch.
Quoted 3 lines
Thanks,
Kevin
Thanks,
Dev.