Subject: Re: CVS commit: syssrc
To: Matthew Jacob <mjacob@netbsd.org>
From: John Hawkinson <jhawk@MIT.EDU>
List: source-changes
Date: 07/05/2000 23:27:35
| ----------------------------
| revision 1.53
| date: 2000/06/03 22:44:43;  author: fair;  state: Exp;  lines: +3 -3
| Change the debug level from 1 to 3 for "skipping target" diagnostic
| which spews unreasonably for a Qlogic SCSI-2 narrow controller, which
| does not have ID's above 7.
| ----------------------------
| revision 1.54
| date: 2000/07/05 22:20:51;  author: mjacob;  state: Exp;  lines: +476 -521
| Back out previous commit- the author is incorrect. There is no 'narrow'
| Qlogic controller driven by this chipset. If they don't want the verbosity,
| don't compile a DIAGNOSTIC kernel.

This does not follow.

"DIAGNOSTIC" is for "cheap kernel consistency checks". It is not for
debugging messages. DIAGNOSTIC is part of the GENERIC kernel for most
architectures.

This whole section of code, isp_netbsd.h:

   107  #if     defined(SCSIDEBUG)
   108  #define DFLT_DBLEVEL            3
   109  #define CFGPRINTF               printf
   110  #elif   defined(DEBUG)
   111  #define DFLT_DBLEVEL            2
   112  #define CFGPRINTF               printf
   113  #elif   defined(DIAGNOSTIC)
   114  #define DFLT_DBLEVEL            1
   115  #define CFGPRINTF               printf
   116  #else
   117  #define DFLT_DBLEVEL            0
   118  #define CFGPRINTF               if (0) printf
   119  #endif

is bogus. DIAGNOSTIC should not cause increased debugging printfs.

To my mind, fair's code was better than the current state or the state
prior to it. Neither was correct, though.

Further, most drivers do this sort of thing with a driver-specific macro,
such as ISPDEBUG_VALUE or ISPDEBUG or somesuch. This behavior
of changing verbosity based on other unrelated items
(e.g. DEBUG and DIAGNOSTIC) is non-intuitive and can produce
nasty changes in behavior when trying to debug other subsytems.

Can we please axe it, or at least make the DIAGNOSTIC level the same
as that without it?

--jhawk