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