Re: CVS commit: syssrc

[Matthew Jacob <mjacob%feral.com@localhost>]

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
>