Subject: Re: CVS commit: syssrc
To: John Hawkinson <jhawk@MIT.EDU>
From: Matthew Jacob <mjacob@feral.com>
List: source-changes
Date: 07/05/2000 20:31:00
Yeah, allright, Jason kicked my ass on this also- made some changes that
should restore things. My main beef is that Fair didn't contact the
maintainer/author (me) about this- it makes it hard to maintain when NetBSD
changes are different from other platforms (this driver exists in 5, soon to
be 6, different Unix and non-Unix systems). I don't think it's too much to ask
to at least coordinate changes with me.
What I really wanted was a 'boot verbose'- which isn't available in NetBSD.
The whole message stuff in this driver is about to get nuked in a few
weeks anyway, at which point in time, the cheezy defines will go away.
On Wed, 5 Jul 2000, John Hawkinson wrote:
> | ----------------------------
> | 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
>