Re: [PATCH] staging: gdm72xx: conditionally compile debug code

2014-07-16 Thread Greg KH 
On Wed, Jul 16, 2014 at 09:40:18PM +0100, Michalis Pappas wrote:
 On 07/09/2014 07:51 PM, Greg KH wrote:
  diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
  b/drivers/staging/gdm72xx/gdm_sdio.c
  index 9d2de6f..914fd75 100644
  --- a/drivers/staging/gdm72xx/gdm_sdio.c
  +++ b/drivers/staging/gdm72xx/gdm_sdio.c
  @@ -280,9 +280,11 @@ static void send_sdu(struct sdio_func *func, struct 
  tx_cxt *tx)
   
 spin_unlock_irqrestore(tx-lock, flags);
   
  +  #if defined(GDM72xx_DEBUG)
 print_hex_dump_debug(sdio_send: , DUMP_PREFIX_NONE, 16, 1,
  tx-sdu_buf + TYPE_A_HEADER_SIZE,
  aggr_len - TYPE_A_HEADER_SIZE, false);
  +  #endif
  
  This should be moved to use dev_dbg(), along with the other calls to
  this function in this file.
  
 
 But dev_dbg() gets eventually to be printk(), which cannot print the
 buffer, so using print_hex_dump_debug() seems to be correct for this
 case, no?

No, you don't want to print the message unless debugging is enabled, and
dev_dbg() uses the proper in-kernel dynamic debug infrastructure.  There
should never be a separate config option for debugging a driver, that
ensures that no user will ever be able to help you out with it.

So delete the ifdef stuff, and the config option, and just use the
proper in-kernel infrastructure for this.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: gdm72xx: conditionally compile debug code

2014-07-16 Thread Michalis Pappas 
On 07/16/2014 09:50 PM, Greg KH wrote:
 On Wed, Jul 16, 2014 at 09:40:18PM +0100, Michalis Pappas wrote:
 On 07/09/2014 07:51 PM, Greg KH wrote:
 diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
 b/drivers/staging/gdm72xx/gdm_sdio.c
 index 9d2de6f..914fd75 100644
 --- a/drivers/staging/gdm72xx/gdm_sdio.c
 +++ b/drivers/staging/gdm72xx/gdm_sdio.c
 @@ -280,9 +280,11 @@ static void send_sdu(struct sdio_func *func, struct 
 tx_cxt *tx)
  
spin_unlock_irqrestore(tx-lock, flags);
  
 +  #if defined(GDM72xx_DEBUG)
print_hex_dump_debug(sdio_send: , DUMP_PREFIX_NONE, 16, 1,
 tx-sdu_buf + TYPE_A_HEADER_SIZE,
 aggr_len - TYPE_A_HEADER_SIZE, false);
 +  #endif

 This should be moved to use dev_dbg(), along with the other calls to
 this function in this file.


 But dev_dbg() gets eventually to be printk(), which cannot print the
 buffer, so using print_hex_dump_debug() seems to be correct for this
 case, no?
 
 No, you don't want to print the message unless debugging is enabled, and
 dev_dbg() uses the proper in-kernel dynamic debug infrastructure.  There
 should never be a separate config option for debugging a driver, that
 ensures that no user will ever be able to help you out with it.
 
 So delete the ifdef stuff, and the config option, and just use the
 proper in-kernel infrastructure for this.
 
 thanks,
 
 greg k-h
 

Ok, I agree on the ifdef stuff. My question was regarding your
suggestion above to replace print_hex_debug() with dev_dbg()


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: gdm72xx: conditionally compile debug code

2014-07-16 Thread Greg KH 
On Wed, Jul 16, 2014 at 11:03:06PM +0100, Michalis Pappas wrote:
 On 07/16/2014 09:50 PM, Greg KH wrote:
  On Wed, Jul 16, 2014 at 09:40:18PM +0100, Michalis Pappas wrote:
  On 07/09/2014 07:51 PM, Greg KH wrote:
  diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
  b/drivers/staging/gdm72xx/gdm_sdio.c
  index 9d2de6f..914fd75 100644
  --- a/drivers/staging/gdm72xx/gdm_sdio.c
  +++ b/drivers/staging/gdm72xx/gdm_sdio.c
  @@ -280,9 +280,11 @@ static void send_sdu(struct sdio_func *func, struct 
  tx_cxt *tx)
   
   spin_unlock_irqrestore(tx-lock, flags);
   
  +#if defined(GDM72xx_DEBUG)
   print_hex_dump_debug(sdio_send: , DUMP_PREFIX_NONE, 16, 1,
tx-sdu_buf + TYPE_A_HEADER_SIZE,
aggr_len - TYPE_A_HEADER_SIZE, false);
  +#endif
 
  This should be moved to use dev_dbg(), along with the other calls to
  this function in this file.
 
 
  But dev_dbg() gets eventually to be printk(), which cannot print the
  buffer, so using print_hex_dump_debug() seems to be correct for this
  case, no?
  
  No, you don't want to print the message unless debugging is enabled, and
  dev_dbg() uses the proper in-kernel dynamic debug infrastructure.  There
  should never be a separate config option for debugging a driver, that
  ensures that no user will ever be able to help you out with it.
  
  So delete the ifdef stuff, and the config option, and just use the
  proper in-kernel infrastructure for this.
  
  thanks,
  
  greg k-h
  
 
 Ok, I agree on the ifdef stuff. My question was regarding your
 suggestion above to replace print_hex_debug() with dev_dbg()

You want the device name/id/label to show up as well, that is why you
should use the dev_dbg() version, print_hex_dump() does not take a
struct device *, so the user has no idea what device this data was
coming from.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: gdm72xx: conditionally compile debug code

2014-07-16 Thread Greg KH 
On Wed, Jul 16, 2014 at 03:46:09PM -0700, Joe Perches wrote:
 On Wed, 2014-07-16 at 15:10 -0700, Greg KH wrote:
  On Wed, Jul 16, 2014 at 11:03:06PM +0100, Michalis Pappas wrote:
 []
   Ok, I agree on the ifdef stuff. My question was regarding your
   suggestion above to replace print_hex_debug() with dev_dbg()
  
  You want the device name/id/label to show up as well, that is why you
  should use the dev_dbg() version, print_hex_dump() does not take a
  struct device *, so the user has no idea what device this data was
  coming from.
 
 But Michalis could alway add something like:
   dev_hex_dump()
 and
   dev_dbg_hex_dump()
 

With the built-in hex dump primitive in printk(), why would you want
to do that?  You shouldn't be putting more than 64 bytes in a single
printk message in a hex dump, if you want to do more, use debugfs.

thanks,

greg k-h

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: gdm72xx: conditionally compile debug code

2014-07-16 Thread Joe Perches 
On Wed, 2014-07-16 at 15:10 -0700, Greg KH wrote:
 On Wed, Jul 16, 2014 at 11:03:06PM +0100, Michalis Pappas wrote:
[]
  Ok, I agree on the ifdef stuff. My question was regarding your
  suggestion above to replace print_hex_debug() with dev_dbg()
 
 You want the device name/id/label to show up as well, that is why you
 should use the dev_dbg() version, print_hex_dump() does not take a
 struct device *, so the user has no idea what device this data was
 coming from.

But Michalis could alway add something like:
dev_hex_dump()
and
dev_dbg_hex_dump()


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: gdm72xx: conditionally compile debug code

2014-07-16 Thread Joe Perches 
On Wed, 2014-07-16 at 15:57 -0700, Greg KH wrote:
 On Wed, Jul 16, 2014 at 03:46:09PM -0700, Joe Perches wrote:
  On Wed, 2014-07-16 at 15:10 -0700, Greg KH wrote:
   On Wed, Jul 16, 2014 at 11:03:06PM +0100, Michalis Pappas wrote:
  []
Ok, I agree on the ifdef stuff. My question was regarding your
suggestion above to replace print_hex_debug() with dev_dbg()
   
   You want the device name/id/label to show up as well, that is why you
   should use the dev_dbg() version, print_hex_dump() does not take a
   struct device *, so the user has no idea what device this data was
   coming from.
  
  But Michalis could alway add something like:
  dev_hex_dump()
  and
  dev_dbg_hex_dump()
  
 
 With the built-in hex dump primitive in printk(), why would you want
 to do that?  You shouldn't be putting more than 64 bytes in a single
 printk message in a hex dump, if you want to do more, use debugfs.

%p*h doesn't also emit ascii or dots for one.
 
I agree %p*h should be preferred but people do
all sorts of strange things in debugging printks.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: gdm72xx: conditionally compile debug code

2014-07-09 Thread Greg KH 
On Tue, Jul 01, 2014 at 02:00:15PM +0100, Michalis Pappas wrote:
 Signed-off-by: Michalis Pappas mpap...@fastmail.fm
 ---
  drivers/staging/gdm72xx/gdm_qos.c   | 2 ++
  drivers/staging/gdm72xx/gdm_sdio.c  | 7 +++
  drivers/staging/gdm72xx/gdm_usb.c   | 7 +++
  drivers/staging/gdm72xx/gdm_wimax.c | 6 ++
  drivers/staging/gdm72xx/gdm_wimax.h | 2 ++
  5 files changed, 24 insertions(+)
 
 diff --git a/drivers/staging/gdm72xx/gdm_qos.c 
 b/drivers/staging/gdm72xx/gdm_qos.c
 index b08c8e1..7900981 100644
 --- a/drivers/staging/gdm72xx/gdm_qos.c
 +++ b/drivers/staging/gdm72xx/gdm_qos.c
 @@ -88,7 +88,9 @@ static void free_qos_entry_list(struct list_head *free_list)
   total_free++;
   }
  
 + #if defined(GDM72xx_DEBUG)
   pr_debug(%s: total_free_cnt=%d\n, __func__, total_free);
 + #endif

Ick, no, never put #ifdef in .c code if you can help it.  For stuff like
this, just rely on the dynamic debug core and use the pr_debug and
dev_dbg() calls, like the driver is doing, so all should be fine.

 diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
 b/drivers/staging/gdm72xx/gdm_sdio.c
 index 9d2de6f..914fd75 100644
 --- a/drivers/staging/gdm72xx/gdm_sdio.c
 +++ b/drivers/staging/gdm72xx/gdm_sdio.c
 @@ -280,9 +280,11 @@ static void send_sdu(struct sdio_func *func, struct 
 tx_cxt *tx)
  
   spin_unlock_irqrestore(tx-lock, flags);
  
 + #if defined(GDM72xx_DEBUG)
   print_hex_dump_debug(sdio_send: , DUMP_PREFIX_NONE, 16, 1,
tx-sdu_buf + TYPE_A_HEADER_SIZE,
aggr_len - TYPE_A_HEADER_SIZE, false);
 + #endif

This should be moved to use dev_dbg(), along with the other calls to
this function in this file.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: gdm72xx: conditionally compile debug code

2014-07-09 Thread Michalis Pappas 
On 07/09/2014 07:51 PM, Greg KH wrote:
 On Tue, Jul 01, 2014 at 02:00:15PM +0100, Michalis Pappas wrote:
 Signed-off-by: Michalis Pappas mpap...@fastmail.fm
 ---
  drivers/staging/gdm72xx/gdm_qos.c   | 2 ++
  drivers/staging/gdm72xx/gdm_sdio.c  | 7 +++
  drivers/staging/gdm72xx/gdm_usb.c   | 7 +++
  drivers/staging/gdm72xx/gdm_wimax.c | 6 ++
  drivers/staging/gdm72xx/gdm_wimax.h | 2 ++
  5 files changed, 24 insertions(+)

 diff --git a/drivers/staging/gdm72xx/gdm_qos.c 
 b/drivers/staging/gdm72xx/gdm_qos.c
 index b08c8e1..7900981 100644
 --- a/drivers/staging/gdm72xx/gdm_qos.c
 +++ b/drivers/staging/gdm72xx/gdm_qos.c
 @@ -88,7 +88,9 @@ static void free_qos_entry_list(struct list_head 
 *free_list)
  total_free++;
  }
  
 +#if defined(GDM72xx_DEBUG)
  pr_debug(%s: total_free_cnt=%d\n, __func__, total_free);
 +#endif
 
 Ick, no, never put #ifdef in .c code if you can help it.  For stuff like
 this, just rely on the dynamic debug core and use the pr_debug and
 dev_dbg() calls, like the driver is doing, so all should be fine.


But how about those cases where debug code consists of more than a
simple call to pr_debug() / dev_dbg()?

For instance consider dump_eth_packet(), defined in gdm_wimax.c. This
function is called every time a packet is received or transmitted, and
calls other helper functions too (get_protocol_name(),
get_ip_protocol_name(), get_port_name()).

Doesn't all this debug logic provide an overhead to the tx / rx functions?

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: gdm72xx: conditionally compile debug code

2014-07-09 Thread Greg KH 
On Wed, Jul 09, 2014 at 08:52:07PM +0100, Michalis Pappas wrote:
 On 07/09/2014 07:51 PM, Greg KH wrote:
  On Tue, Jul 01, 2014 at 02:00:15PM +0100, Michalis Pappas wrote:
  Signed-off-by: Michalis Pappas mpap...@fastmail.fm
  ---
   drivers/staging/gdm72xx/gdm_qos.c   | 2 ++
   drivers/staging/gdm72xx/gdm_sdio.c  | 7 +++
   drivers/staging/gdm72xx/gdm_usb.c   | 7 +++
   drivers/staging/gdm72xx/gdm_wimax.c | 6 ++
   drivers/staging/gdm72xx/gdm_wimax.h | 2 ++
   5 files changed, 24 insertions(+)
 
  diff --git a/drivers/staging/gdm72xx/gdm_qos.c 
  b/drivers/staging/gdm72xx/gdm_qos.c
  index b08c8e1..7900981 100644
  --- a/drivers/staging/gdm72xx/gdm_qos.c
  +++ b/drivers/staging/gdm72xx/gdm_qos.c
  @@ -88,7 +88,9 @@ static void free_qos_entry_list(struct list_head 
  *free_list)
 total_free++;
 }
   
  +  #if defined(GDM72xx_DEBUG)
 pr_debug(%s: total_free_cnt=%d\n, __func__, total_free);
  +  #endif
  
  Ick, no, never put #ifdef in .c code if you can help it.  For stuff like
  this, just rely on the dynamic debug core and use the pr_debug and
  dev_dbg() calls, like the driver is doing, so all should be fine.
 
 
 But how about those cases where debug code consists of more than a
 simple call to pr_debug() / dev_dbg()?

Then that code is wrong :)

 For instance consider dump_eth_packet(), defined in gdm_wimax.c. This
 function is called every time a packet is received or transmitted, and
 calls other helper functions too (get_protocol_name(),
 get_ip_protocol_name(), get_port_name()).
 
 Doesn't all this debug logic provide an overhead to the tx / rx functions?

Yes, so much so it doesn't make sense to even have that type of
function, right?

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: gdm72xx: conditionally compile debug code

2014-07-03 Thread Michalis Pappas 
On 07/01/2014 07:08 PM, Ben Chan wrote:
 
 
 
 On Tue, Jul 1, 2014 at 9:40 AM, Michalis Pappas mpap...@fastmail.fm
 mailto:mpap...@fastmail.fm wrote:
 
 On 07/01/2014 04:30 PM, Ben Chan wrote:
 
 
 
  On Tue, Jul 1, 2014 at 6:00 AM, Michalis Pappas
 mpap...@fastmail.fm mailto:mpap...@fastmail.fm
  mailto:mpap...@fastmail.fm mailto:mpap...@fastmail.fm wrote:
 
  Signed-off-by: Michalis Pappas mpap...@fastmail.fm
 mailto:mpap...@fastmail.fm
  mailto:mpap...@fastmail.fm mailto:mpap...@fastmail.fm
  ---
   drivers/staging/gdm72xx/gdm_qos.c   | 2 ++
   drivers/staging/gdm72xx/gdm_sdio.c  | 7 +++
   drivers/staging/gdm72xx/gdm_usb.c   | 7 +++
   drivers/staging/gdm72xx/gdm_wimax.c | 6 ++
   drivers/staging/gdm72xx/gdm_wimax.h | 2 ++
   5 files changed, 24 insertions(+)
 
  diff --git a/drivers/staging/gdm72xx/gdm_qos.c
  b/drivers/staging/gdm72xx/gdm_qos.c
  index b08c8e1..7900981 100644
  --- a/drivers/staging/gdm72xx/gdm_qos.c
  +++ b/drivers/staging/gdm72xx/gdm_qos.c
  @@ -88,7 +88,9 @@ static void free_qos_entry_list(struct list_head
  *free_list)
  total_free++;
  }
 
  +   #if defined(GDM72xx_DEBUG)
  pr_debug(%s: total_free_cnt=%d\n, __func__, total_free);
  +   #endif
   }
 
   void gdm_qos_init(void *nic_ptr)
  diff --git a/drivers/staging/gdm72xx/gdm_sdio.c
  b/drivers/staging/gdm72xx/gdm_sdio.c
  index 9d2de6f..914fd75 100644
  --- a/drivers/staging/gdm72xx/gdm_sdio.c
  +++ b/drivers/staging/gdm72xx/gdm_sdio.c
  @@ -280,9 +280,11 @@ static void send_sdu(struct sdio_func *func,
  struct tx_cxt *tx)
 
  spin_unlock_irqrestore(tx-lock, flags);
 
  +   #if defined(GDM72xx_DEBUG)
  print_hex_dump_debug(sdio_send: , DUMP_PREFIX_NONE,
 16, 1,
   tx-sdu_buf + TYPE_A_HEADER_SIZE,
   aggr_len - TYPE_A_HEADER_SIZE,
 false);
  +   #endif
 
  for (pos = TYPE_A_HEADER_SIZE; pos  aggr_len; pos +=
  TX_CHUNK_SIZE) {
  len = aggr_len - pos;
  @@ -317,9 +319,12 @@ static void send_hci(struct sdio_func *func,
  struct tx_cxt *tx,
   {
  unsigned long flags;
 
  +   #if defined(GDM72xx_DEBUG)
  print_hex_dump_debug(sdio_send: , DUMP_PREFIX_NONE,
 16, 1,
   t-buf + TYPE_A_HEADER_SIZE,
   t-len - TYPE_A_HEADER_SIZE, false);
  +   #endif
  +
  send_sdio_pkt(func, t-buf, t-len);
 
  spin_lock_irqsave(tx-lock, flags);
  @@ -551,8 +556,10 @@ static void gdm_sdio_irq(struct sdio_func
 *func)
  }
 
   end_io:
  +   #if defined(GDM72xx_DEBUG)
  print_hex_dump_debug(sdio_receive: ,
 DUMP_PREFIX_NONE, 16, 1,
   rx-rx_buf, len, false);
  +   #endif
  len = control_sdu_tx_flow(sdev, rx-rx_buf, len);
 
  spin_lock_irqsave(rx-lock, flags);
  diff --git a/drivers/staging/gdm72xx/gdm_usb.c
  b/drivers/staging/gdm72xx/gdm_usb.c
  index 971976c..bfd347a 100644
  --- a/drivers/staging/gdm72xx/gdm_usb.c
  +++ b/drivers/staging/gdm72xx/gdm_usb.c
  @@ -348,8 +348,11 @@ static int gdm_usb_send(void *priv_dev, void
  *data, int len,
  usb_fill_bulk_urb(t-urb, usbdev, usb_sndbulkpipe(usbdev,
  1), t-buf,
len + padding,
 gdm_usb_send_complete, t);
 
  +   #if defined(GDM72xx_DEBUG)
  print_hex_dump_debug(usb_send: , DUMP_PREFIX_NONE,
 16, 1,
  t-buf,
   len + padding, false);
  +   #endif
  +
   #ifdef CONFIG_WIMAX_GDM72XX_USB_PM
  if (usbdev-state  USB_STATE_SUSPENDED) {
  list_add_tail(t-p_list, tx-pending_list);
  @@ -427,8 +430,12 @@ static void gdm_usb_rcv_complete(struct
 urb *urb)
 
  if (!urb-status) {
  cmd_evt = (r-buf[0]  8) | (r-buf[1]);
  +
  +   #if defined(GDM72xx_DEBUG)
  print_hex_dump_debug(usb_receive: ,
  DUMP_PREFIX_NONE, 16, 1,
   r-buf,
 urb-actual_length, false);
  +   #endif
  +
  if (cmd_evt == WIMAX_SDU_TX_FLOW) {
  if (r-buf[4] == 0)

[PATCH] staging: gdm72xx: conditionally compile debug code

2014-07-01 Thread Michalis Pappas 
Signed-off-by: Michalis Pappas mpap...@fastmail.fm
---
 drivers/staging/gdm72xx/gdm_qos.c   | 2 ++
 drivers/staging/gdm72xx/gdm_sdio.c  | 7 +++
 drivers/staging/gdm72xx/gdm_usb.c   | 7 +++
 drivers/staging/gdm72xx/gdm_wimax.c | 6 ++
 drivers/staging/gdm72xx/gdm_wimax.h | 2 ++
 5 files changed, 24 insertions(+)

diff --git a/drivers/staging/gdm72xx/gdm_qos.c 
b/drivers/staging/gdm72xx/gdm_qos.c
index b08c8e1..7900981 100644
--- a/drivers/staging/gdm72xx/gdm_qos.c
+++ b/drivers/staging/gdm72xx/gdm_qos.c
@@ -88,7 +88,9 @@ static void free_qos_entry_list(struct list_head *free_list)
total_free++;
}
 
+   #if defined(GDM72xx_DEBUG)
pr_debug(%s: total_free_cnt=%d\n, __func__, total_free);
+   #endif
 }
 
 void gdm_qos_init(void *nic_ptr)
diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
b/drivers/staging/gdm72xx/gdm_sdio.c
index 9d2de6f..914fd75 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.c
+++ b/drivers/staging/gdm72xx/gdm_sdio.c
@@ -280,9 +280,11 @@ static void send_sdu(struct sdio_func *func, struct tx_cxt 
*tx)
 
spin_unlock_irqrestore(tx-lock, flags);
 
+   #if defined(GDM72xx_DEBUG)
print_hex_dump_debug(sdio_send: , DUMP_PREFIX_NONE, 16, 1,
 tx-sdu_buf + TYPE_A_HEADER_SIZE,
 aggr_len - TYPE_A_HEADER_SIZE, false);
+   #endif
 
for (pos = TYPE_A_HEADER_SIZE; pos  aggr_len; pos += TX_CHUNK_SIZE) {
len = aggr_len - pos;
@@ -317,9 +319,12 @@ static void send_hci(struct sdio_func *func, struct tx_cxt 
*tx,
 {
unsigned long flags;
 
+   #if defined(GDM72xx_DEBUG)
print_hex_dump_debug(sdio_send: , DUMP_PREFIX_NONE, 16, 1,
 t-buf + TYPE_A_HEADER_SIZE,
 t-len - TYPE_A_HEADER_SIZE, false);
+   #endif
+
send_sdio_pkt(func, t-buf, t-len);
 
spin_lock_irqsave(tx-lock, flags);
@@ -551,8 +556,10 @@ static void gdm_sdio_irq(struct sdio_func *func)
}
 
 end_io:
+   #if defined(GDM72xx_DEBUG)
print_hex_dump_debug(sdio_receive: , DUMP_PREFIX_NONE, 16, 1,
 rx-rx_buf, len, false);
+   #endif
len = control_sdu_tx_flow(sdev, rx-rx_buf, len);
 
spin_lock_irqsave(rx-lock, flags);
diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 971976c..bfd347a 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -348,8 +348,11 @@ static int gdm_usb_send(void *priv_dev, void *data, int 
len,
usb_fill_bulk_urb(t-urb, usbdev, usb_sndbulkpipe(usbdev, 1), t-buf,
  len + padding, gdm_usb_send_complete, t);
 
+   #if defined(GDM72xx_DEBUG)
print_hex_dump_debug(usb_send: , DUMP_PREFIX_NONE, 16, 1, t-buf,
 len + padding, false);
+   #endif
+
 #ifdef CONFIG_WIMAX_GDM72XX_USB_PM
if (usbdev-state  USB_STATE_SUSPENDED) {
list_add_tail(t-p_list, tx-pending_list);
@@ -427,8 +430,12 @@ static void gdm_usb_rcv_complete(struct urb *urb)
 
if (!urb-status) {
cmd_evt = (r-buf[0]  8) | (r-buf[1]);
+
+   #if defined(GDM72xx_DEBUG)
print_hex_dump_debug(usb_receive: , DUMP_PREFIX_NONE, 16, 1,
 r-buf, urb-actual_length, false);
+   #endif
+
if (cmd_evt == WIMAX_SDU_TX_FLOW) {
if (r-buf[4] == 0) {
dev_dbg(dev-dev, WIMAX == STOP SDU TX\n);
diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
b/drivers/staging/gdm72xx/gdm_wimax.c
index c2e6bfe..63a760b 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.c
+++ b/drivers/staging/gdm72xx/gdm_wimax.c
@@ -61,6 +61,7 @@ static u8 gdm_wimax_macaddr[6] = {0x00, 0x0a, 0x3b, 0xf0, 
0x01, 0x30};
 static void gdm_wimax_ind_fsm_update(struct net_device *dev, struct fsm_s 
*fsm);
 static void gdm_wimax_ind_if_updown(struct net_device *dev, int if_up);
 
+#if defined(GDM72xx_DEBUG)
 static const char *get_protocol_name(u16 protocol)
 {
static char buf[32];
@@ -162,6 +163,7 @@ static void dump_eth_packet(struct net_device *dev, const 
char *title,
 
print_hex_dump_debug(, DUMP_PREFIX_NONE, 16, 1, data, len, false);
 }
+#endif
 
 static inline int gdm_wimax_header(struct sk_buff **pskb)
 {
@@ -369,7 +371,9 @@ static int gdm_wimax_tx(struct sk_buff *skb, struct 
net_device *dev)
 {
int ret = 0;
 
+   #if defined(GDM72xx_DEBUG)
dump_eth_packet(dev, TX, skb-data, skb-len);
+   #endif
 
ret = gdm_wimax_header(skb);
if (ret  0) {
@@ -698,7 +702,9 @@ static void gdm_wimax_netif_rx(struct net_device *dev, char 
*buf, int len)
struct sk_buff *skb;
int ret;
 
+   #if defined(GDM72xx_DEBUG)
dump_eth_packet(dev, RX, buf, len);
+   #endif
 
skb = dev_alloc_skb(len + 2);
if (!skb) {
diff --git

Re: [PATCH] staging: gdm72xx: conditionally compile debug code

2014-07-01 Thread Michalis Pappas 
On 07/01/2014 04:30 PM, Ben Chan wrote:
 
 
 
 On Tue, Jul 1, 2014 at 6:00 AM, Michalis Pappas mpap...@fastmail.fm
 mailto:mpap...@fastmail.fm wrote:
 
 Signed-off-by: Michalis Pappas mpap...@fastmail.fm
 mailto:mpap...@fastmail.fm
 ---
  drivers/staging/gdm72xx/gdm_qos.c   | 2 ++
  drivers/staging/gdm72xx/gdm_sdio.c  | 7 +++
  drivers/staging/gdm72xx/gdm_usb.c   | 7 +++
  drivers/staging/gdm72xx/gdm_wimax.c | 6 ++
  drivers/staging/gdm72xx/gdm_wimax.h | 2 ++
  5 files changed, 24 insertions(+)
 
 diff --git a/drivers/staging/gdm72xx/gdm_qos.c
 b/drivers/staging/gdm72xx/gdm_qos.c
 index b08c8e1..7900981 100644
 --- a/drivers/staging/gdm72xx/gdm_qos.c
 +++ b/drivers/staging/gdm72xx/gdm_qos.c
 @@ -88,7 +88,9 @@ static void free_qos_entry_list(struct list_head
 *free_list)
 total_free++;
 }
 
 +   #if defined(GDM72xx_DEBUG)
 pr_debug(%s: total_free_cnt=%d\n, __func__, total_free);
 +   #endif
  }
 
  void gdm_qos_init(void *nic_ptr)
 diff --git a/drivers/staging/gdm72xx/gdm_sdio.c
 b/drivers/staging/gdm72xx/gdm_sdio.c
 index 9d2de6f..914fd75 100644
 --- a/drivers/staging/gdm72xx/gdm_sdio.c
 +++ b/drivers/staging/gdm72xx/gdm_sdio.c
 @@ -280,9 +280,11 @@ static void send_sdu(struct sdio_func *func,
 struct tx_cxt *tx)
 
 spin_unlock_irqrestore(tx-lock, flags);
 
 +   #if defined(GDM72xx_DEBUG)
 print_hex_dump_debug(sdio_send: , DUMP_PREFIX_NONE, 16, 1,
  tx-sdu_buf + TYPE_A_HEADER_SIZE,
  aggr_len - TYPE_A_HEADER_SIZE, false);
 +   #endif
 
 for (pos = TYPE_A_HEADER_SIZE; pos  aggr_len; pos +=
 TX_CHUNK_SIZE) {
 len = aggr_len - pos;
 @@ -317,9 +319,12 @@ static void send_hci(struct sdio_func *func,
 struct tx_cxt *tx,
  {
 unsigned long flags;
 
 +   #if defined(GDM72xx_DEBUG)
 print_hex_dump_debug(sdio_send: , DUMP_PREFIX_NONE, 16, 1,
  t-buf + TYPE_A_HEADER_SIZE,
  t-len - TYPE_A_HEADER_SIZE, false);
 +   #endif
 +
 send_sdio_pkt(func, t-buf, t-len);
 
 spin_lock_irqsave(tx-lock, flags);
 @@ -551,8 +556,10 @@ static void gdm_sdio_irq(struct sdio_func *func)
 }
 
  end_io:
 +   #if defined(GDM72xx_DEBUG)
 print_hex_dump_debug(sdio_receive: , DUMP_PREFIX_NONE, 16, 1,
  rx-rx_buf, len, false);
 +   #endif
 len = control_sdu_tx_flow(sdev, rx-rx_buf, len);
 
 spin_lock_irqsave(rx-lock, flags);
 diff --git a/drivers/staging/gdm72xx/gdm_usb.c
 b/drivers/staging/gdm72xx/gdm_usb.c
 index 971976c..bfd347a 100644
 --- a/drivers/staging/gdm72xx/gdm_usb.c
 +++ b/drivers/staging/gdm72xx/gdm_usb.c
 @@ -348,8 +348,11 @@ static int gdm_usb_send(void *priv_dev, void
 *data, int len,
 usb_fill_bulk_urb(t-urb, usbdev, usb_sndbulkpipe(usbdev,
 1), t-buf,
   len + padding, gdm_usb_send_complete, t);
 
 +   #if defined(GDM72xx_DEBUG)
 print_hex_dump_debug(usb_send: , DUMP_PREFIX_NONE, 16, 1,
 t-buf,
  len + padding, false);
 +   #endif
 +
  #ifdef CONFIG_WIMAX_GDM72XX_USB_PM
 if (usbdev-state  USB_STATE_SUSPENDED) {
 list_add_tail(t-p_list, tx-pending_list);
 @@ -427,8 +430,12 @@ static void gdm_usb_rcv_complete(struct urb *urb)
 
 if (!urb-status) {
 cmd_evt = (r-buf[0]  8) | (r-buf[1]);
 +
 +   #if defined(GDM72xx_DEBUG)
 print_hex_dump_debug(usb_receive: ,
 DUMP_PREFIX_NONE, 16, 1,
  r-buf, urb-actual_length, false);
 +   #endif
 +
 if (cmd_evt == WIMAX_SDU_TX_FLOW) {
 if (r-buf[4] == 0) {
 dev_dbg(dev-dev, WIMAX == STOP
 SDU TX\n);
 diff --git a/drivers/staging/gdm72xx/gdm_wimax.c
 b/drivers/staging/gdm72xx/gdm_wimax.c
 index c2e6bfe..63a760b 100644
 --- a/drivers/staging/gdm72xx/gdm_wimax.c
 +++ b/drivers/staging/gdm72xx/gdm_wimax.c
 @@ -61,6 +61,7 @@ static u8 gdm_wimax_macaddr[6] = {0x00, 0x0a,
 0x3b, 0xf0, 0x01, 0x30};
  static void gdm_wimax_ind_fsm_update(struct net_device *dev, struct
 fsm_s *fsm);
  static void gdm_wimax_ind_if_updown(struct net_device *dev, int if_up);
 
 +#if defined(GDM72xx_DEBUG)
  static const char *get_protocol_name(u16 protocol)
  {
 static char buf[32];
 @@ -162,6 +163,7 @@ static void dump_eth_packet(struct net_device
 *dev, const char *title,