>Number:         4374
>Category:       kern
>Synopsis:       audio attachment method bogus, doesn't print when not attaching
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Oct 28 16:50:13 1997
>Last-Modified:
>Originator:     Chris G. Demetriou
>Organization:
Kernel Hackers 'r' Us
>Release:        NetBSD-current as of October 27, 1997
>Environment:
System: NetBSD brick.demetriou.com 1.3_ALPHA NetBSD 1.3_ALPHA (BRICK) #8: Mon Oct 27 22:40:17 PST 1997 cgd@brick.demetriou.com:/usr/src/sys/arch/i386/compile/BRICK i386


>Description:
	The code to attach MI audio and midi device (audio_attach_mi())
	is bogus, for a couple of reasons:

	(1) The right way to configure direct config devices is to provide
	    their match routines with useful information to match, then
	    config_found() each direct config device you have.  Looping
	    around config_found() is, in most cases (including this one)
	    wrong.  In this case, config_found() is called for a device
	    which isn't there, which is logically wrong.

	(2) When a direct config device isn't configured, the system
	    normally (and "should") print a message saying that it
	    wasn't configured so that people will know to put the right
	    entry in their config file.  No such message can be printed
	    with the existing code, because it intentionally, wrongly
	    calls config_found() for devices which aren't present.

	Unfortunately, you can't fix (2) (which is what got me started
	looking into this) without fixing (1) as well.

>How-To-Repeat:
	Configure an audio hardware device (e.g. a 'sb'), but don't
	configure an 'audio' device to attach to it.  Boot the kernel.
	Note that an "audio at sb[unit] not configured" message isn't
	printed.

	Change the audio code so that it provides a print routine to
	config_found(), so that a message _is_ printed an 'audio'
	device isn't configured.  Configure a kernel to attach
	an 'audio' device to the audio hardware device.  Note that
	an extra message is printed, claiming that an 'audio'
	device wasn't configured.
	
>Fix:
	Fix the 'audio' device match and attachment code, so that it
	does configuration the right way.  Below is a diff that
	will, in my opinion, do that.  It also has the useful side
	effect that it would allow hardware devices which support
	the MIDI interface (but not the audio interface) to configure
	correctly, rather than in the ... odd (wrong 8-) way that the
	existing code would try.

*** -	Tue Oct 28 00:15:14 1997
--- sys/dev/audio.c	Mon Oct 27 22:39:45 1997
***************
*** 208,216 ****
  
  	DPRINTF(("audioprobe: done=%d sa=%p hw=%p\n", 
  		 sa->audiodone, sa, sa->ahw));
! 	if (sa->audiodone)
! 		return 0;
! 	return sa->ahw != 0;
  }
  
  void
--- 208,214 ----
  
  	DPRINTF(("audioprobe: done=%d sa=%p hw=%p\n", 
  		 sa->audiodone, sa, sa->ahw));
! 	return (sa->type == AUDIODEV_TYPE_AUDIO) ? 1 : 0;
  }
  
  void
***************
*** 220,226 ****
  {
  	struct audio_softc *sc = (void *)self;
  	struct audio_attach_args *sa = aux;
! 	struct audio_hw_if *hwp = sa->ahw;
  	void *hdlp = sa->hdl;
  	int error;
  	mixer_devinfo_t mi;
--- 218,224 ----
  {
  	struct audio_softc *sc = (void *)self;
  	struct audio_attach_args *sa = aux;
! 	struct audio_hw_if *hwp = sa->hwif;
  	void *hdlp = sa->hdl;
  	int error;
  	mixer_devinfo_t mi;
***************
*** 228,235 ****
  
  	printf("\n");
  
- 	sa->audiodone++;
- 
  #ifdef DIAGNOSTIC
  	if (hwp == 0 ||
  	    hwp->open == 0 ||
--- 226,231 ----
***************
*** 396,407 ****
  {
  	struct audio_attach_args arg;
  
! 	arg.ahw = ahwp;
! 	arg.mhw = mhwp;
! 	arg.hdl = hdlp;
! 	arg.audiodone = arg.mididone = 0;
! 	while(config_found(dev, &arg, 0))
! 		;
  }
  
  #ifdef AUDIO_DEBUG
--- 392,433 ----
  {
  	struct audio_attach_args arg;
  
! 	if (ahwp != NULL) {
! 		arg.type = AUDIODEV_TYPE_AUDIO;
! 		arg.hwif = ahwp;
! 		arg.hdl = hdlp;
! 		(void)config_found(dev, &arg, audioprint);
! 	}
! 	if (mhwp != NULL) {
! 		arg.type = AUDIODEV_TYPE_MIDI;
! 		arg.hwif = mhwp;
! 		arg.hdl = hdlp;
! 		(void)config_found(dev, &arg, audioprint);
! 	}
! }
! 
! int
! audioprint(aux, pnp)
! 	void *aux;
! 	const char *pnp;
! {
! 	struct audio_attach_args *arg = aux;
! 	const char *type;
! 
! 	if (pnp != NULL) {
! 		switch (arg->type) {
! 		case AUDIODEV_TYPE_AUDIO:
! 			type = "audio";
! 			break;
! 		case AUDIODEV_TYPE_MIDI:
! 			type = "midi";
! 			break;
! 		default:
! 			panic("audioprint: unknown type %d", arg->type);
! 		}
! 		printf("%s at %s", type, pnp);
! 	}
! 	return (UNCONF);
  }
  
  #ifdef AUDIO_DEBUG
*** -	Tue Oct 28 00:15:14 1997
--- sys/dev/audio_if.h	Mon Oct 27 22:00:51 1997
***************
*** 133,143 ****
  };
  
  struct audio_attach_args {
! 	struct audio_hw_if *ahw;
! 	struct midi_hw_if *mhw;
! 	void *hdl;
! 	char audiodone, mididone;
  };
  
  /* Attach the MI driver(s) to the MD driver. */
  extern void	audio_attach_mi __P((struct audio_hw_if *, struct midi_hw_if *, void *, struct device *));
--- 133,144 ----
  };
  
  struct audio_attach_args {
! 	int	type;
! 	void	*hwif;		/* either audio_hw_if * or midi_hw_if * */
! 	void	*hdl;
  };
+ #define	AUDIODEV_TYPE_AUDIO	0
+ #define	AUDIODEV_TYPE_MIDI	1
  
  /* Attach the MI driver(s) to the MD driver. */
  extern void	audio_attach_mi __P((struct audio_hw_if *, struct midi_hw_if *, void *, struct device *));
>Audit-Trail:
>Unformatted: