2012-05-27 20:54:42

by devendra.aaru

[permalink] [raw]
Subject: [PATCH] [staging] bcm: Fix codingstyle problems

Signed-off-by: Devendra Naga <[email protected]>
---
drivers/staging/bcm/Debug.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/bcm/Debug.h b/drivers/staging/bcm/Debug.h
index 420382d..9b5ab4b 100644
--- a/drivers/staging/bcm/Debug.h
+++ b/drivers/staging/bcm/Debug.h
@@ -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]); \
} \
} \
--
1.7.9.5


2012-05-28 02:30:57

by Kevin McKinney

[permalink] [raw]
Subject: Re: [PATCH] [staging] bcm: Fix codingstyle problems

Hi Devendra,

On Sun, May 27, 2012 at 4:54 PM, Devendra Naga <[email protected]> wrote:
> 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

2012-05-28 02:38:56

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] [staging] bcm: Fix codingstyle problems

On Sun, 2012-05-27 at 22:30 -0400, Kevin McKinney wrote:
> Hi Devendra,
>
> On Sun, May 27, 2012 at 4:54 PM, Devendra Naga <[email protected]> wrote:
> > 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.

2012-05-28 04:17:33

by devendra.aaru

[permalink] [raw]
Subject: Re: [PATCH] [staging] bcm: Fix codingstyle problems

On Mon, May 28, 2012 at 8:00 AM, Kevin McKinney <[email protected]> wrote:
> Hi Devendra,
>
> On Sun, May 27, 2012 at 4:54 PM, Devendra Naga <[email protected]> wrote:
>> 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.
>
> Thanks,
> Kevin

Thanks,
Dev.

2012-05-28 04:23:26

by devendra.aaru

[permalink] [raw]
Subject: Re: [PATCH] [staging] bcm: Fix codingstyle problems

Hello Joe,

On Mon, May 28, 2012 at 8:08 AM, Joe Perches <[email protected]> wrote:
> On Sun, 2012-05-27 at 22:30 -0400, Kevin McKinney wrote:
>> Hi Devendra,
>>
>> On Sun, May 27, 2012 at 4:54 PM, Devendra Naga <[email protected]> wrote:
>> > 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.
>
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?
>
Thanks,
Devendra.

2012-05-28 04:37:43

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] [staging] bcm: Fix codingstyle problems

On Mon, 2012-05-28 at 09:53 +0530, devendra.aaru wrote:
> Hello Joe,
>
> On Mon, May 28, 2012 at 8:08 AM, Joe Perches <[email protected]> wrote:
> > On Sun, 2012-05-27 at 22:30 -0400, Kevin McKinney wrote:
> >> Hi Devendra,
> >>
> >> On Sun, May 27, 2012 at 4:54 PM, Devendra Naga <[email protected]> wrote:
> >> > 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.
> >
> 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

diff --git a/drivers/staging/bcm/Debug.c b/drivers/staging/bcm/Debug.c
new file mode 100644
index 0000000..3ca720c
--- /dev/null
+++ b/drivers/staging/bcm/Debug.c
@@ -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]);
+ }
+ }
+}
diff --git a/drivers/staging/bcm/Debug.h b/drivers/staging/bcm/Debug.h
index 420382d..0fbb206 100644
--- a/drivers/staging/bcm/Debug.h
+++ b/drivers/staging/bcm/Debug.h
@@ -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
diff --git a/drivers/staging/bcm/Makefile b/drivers/staging/bcm/Makefile
index 652b7f8..a858ac1 100644
--- a/drivers/staging/bcm/Makefile
+++ b/drivers/staging/bcm/Makefile
@@ -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


2012-05-28 07:22:39

by devendra.aaru

[permalink] [raw]
Subject: Re: [PATCH] [staging] bcm: Fix codingstyle problems

Hello Joe,

Thanks for a very quick reply with a code suggestion.

Just a small change at your code,

On Mon, May 28, 2012 at 10:07 AM, Joe Perches <[email protected]> wrote:
>
> 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
>
> diff --git a/drivers/staging/bcm/Debug.c b/drivers/staging/bcm/Debug.c
> new file mode 100644
> index 0000000..3ca720c
> --- /dev/null
> +++ b/drivers/staging/bcm/Debug.c
> @@ -0,0 +1,53 @@
> +#include "headers.h"
/**
* for the BCM_DEBUG_PRINT macro
*/
+#include "debug.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]);
> + ? ? ? ? ? ? ? }
> + ? ? ? }
> +}
> diff --git a/drivers/staging/bcm/Debug.h b/drivers/staging/bcm/Debug.h
> index 420382d..0fbb206 100644
> --- a/drivers/staging/bcm/Debug.h
> +++ b/drivers/staging/bcm/Debug.h
> @@ -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
> diff --git a/drivers/staging/bcm/Makefile b/drivers/staging/bcm/Makefile
> index 652b7f8..a858ac1 100644
> --- a/drivers/staging/bcm/Makefile
> +++ b/drivers/staging/bcm/Makefile
> @@ -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
>
>
>

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.

2012-05-28 09:35:12

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] [staging] bcm: Fix codingstyle problems

On Sun, May 27, 2012 at 09:37:39PM -0700, Joe Perches wrote:
> 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

2012-05-28 15:02:35

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] [staging] bcm: Fix codingstyle problems

On Mon, 2012-05-28 at 12:34 +0300, Dan Carpenter wrote:
> On Sun, May 27, 2012 at 09:37:39PM -0700, Joe Perches wrote:
> > 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

2012-05-28 20:58:49

by Kevin McKinney

[permalink] [raw]
Subject: Re: [PATCH] [staging] bcm: Fix codingstyle problems

On Mon, May 28, 2012 at 5:34 AM, Dan Carpenter <[email protected]> wrote:
> On Sun, May 27, 2012 at 09:37:39PM -0700, Joe Perches wrote:
>> 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