[U-Boot-Users] [RFD] Consistent debugging output structure

[Vladimir Gurevich]

Hello Robert,

Robert Schwebel wrote:
> What we don't agree on is if we need several debug levels. What do the
> others think? For more complex drivers like the ethernet ones this is
> really useful. 

Based on how I normally do this sort of debugging in my drivers
these are the questions I ask myself:

1) Do I want dynamic debugging levels (i.e. the ones I can choose
    at runtime). If the answer is "yes" then debug level should be
    stored in a variable, otherwise it should be a macro.

2) How big the performance hit will be and how important it is?
    If I don't mind performance hit, then I have dynamic debugging
    with default level "0". Otherwise I use a macro and let the
    compiler optimize it out.

About debug levels. Normally I use bits to specify what do I want
to debug instead of relying on "greater than" comparisons. This way
I have much greater flexibility and can enable only the debug outputs
I really need right now.

Here what it normally translates to in C:
===========================================================
/*
  *  Debug levels for my driver (say, vag01)
  */
enum {
     DEBUG_VAG01_INIT    = 0x00000001, /* Initialization sequence   */
     DEBUG_VAG01_INTR    = 0x00000002, /* Interrupt handling        */
     DEBUG_VAG01_API     = 0x00000004, /* Standard API calls (open, */
                                       /* close, read, write, etc.  */
     DEBUG_VAG01_RX      = 0x00000008, /* Receive data path         */
     DEBUG_VAG01_TX      = 0x00000010, /* Transmit data path        */
     DEBUG_VAG01_RX_DATA = 0x00000014, /* Dump received data        */
     DEBUG_VAG01_TX_DATA = 0x00000018, /* Dumt transmit data        */
     DEBUG_VAG01_RELOCK  = 0x00000020, /* Debug relock thread       */
};
/*
  * Compile-time configuration parameters for the driver vag01
  */
#define DEBUG_VAG01           /* No debugging compiled in if it */
                               /* is not defined                 */

#define DEBUG_VAG01_DEFAULT 0 /* The initial debug level. Can be */
                               /* (DEBUG_VAG01_INTR | DEBUG_VAG01_API) 
*/
                               /* or something like that */

/*
  * Here is the debug print
  */
#if defined (DEBUG_VAG01)
#define debug(level, fmt, args...)    \
     do {                              \
        if (debug_vag01 & (level)) {   \
            printf(fmt , ##args);      \
        }                              \
     } while (0)
#else
#define debug(level, fmt, args...)
#endif

/*
  * Here is the debug level variable
  */
static __u32 debug_vag01 = DEBUG_VAG01_DEFAULT;

===============================================================

Some other comments:
   1) I use an anonymous enum to define debug levels. This way it
      becomes easier to change them from gdb by typing:

      (gdb) set debug_vag01|=(DEBUG_VAG01_TX | DEBUG_VAG01_RX)

   2) When I am doing that for Linux I also provide /proc interface
      to change this debug level dynamically (plus a "debug" module
      parameter, or course)

   3) In case of multiple devices of the same type (not quite important
      for u-boot) you can move the debug_level variable into the driver
      instance structure

   4) Again, probably not important for u-boot, but important for a
      Linux-based system: it becomes very easy to add a registration
      API so that each debuggable component could register its levels
      with a centralized debug command. Then you can easily control
      debug levels by typing
          debug <component> <level>
          undebug <component> <level>
          show debug
          show debug <component>
      pretty much the same like Cisco IOS does.

   5) Checking the debug level upsets the cache and this might become
      important in the performance critical parts. Then I use a couple
      more compile-time flags to remove just this debug from the
      production-level code.

Just my 2 cents.

Best regards,
Vladimir Gurevich