sound: Simplify how snd_mixer is fetched and how the cdev is created

The primary snd_mixer was reached by accessing the mixer cdev's si_drv1.
This is tedious and ugly, so store the mixer in snddev_info->mixer and
access it directly.

Additionally, create the cdev in a new mixer_make_dev() function (in
similar fashion to dsp_make_dev()) in pcm_register(), when everything is
initialized, instead of risking potential races because mixer_init()
(called before pcm_register()) used to create the cdev.

Also add some NULL checks in pcm_register(), to avoid creating a mixer
cdev when the driver (e.g., fdt/audio_soc.c) does not create a mixer in
the first place, and similarly in pcm_unregister().

Sponsored by:	The FreeBSD Foundation
MFC after:	1 month
Differential Revision:	https://reviews.freebsd.org/D59070

sys/dev/sound/pci/es137x.c

@@ -1516,8 +1516,7 @@ sysctl_es137x_single_pcm_mixer(SYSCTL_HANDLER_ARGS)
 
 	dev = oidp->oid_arg1;
 	d = device_get_softc(dev);
-	if (!PCM_REGISTERED(d) || d->mixer_dev == NULL ||
-	    d->mixer_dev->si_drv1 == NULL)
+	if (!PCM_REGISTERED(d) || d->mixer_dev == NULL || d->mixer == NULL)
 		return (EINVAL);
 	es = d->devinfo;
 	if (es == NULL)
@@ -1535,7 +1534,7 @@ sysctl_es137x_single_pcm_mixer(SYSCTL_HANDLER_ARGS)
 	if (val == set)
 		return (0);
 	PCM_ACQUIRE_QUICK(d);
-	m = (d->mixer_dev != NULL) ? d->mixer_dev->si_drv1 : NULL;
+	m = d->mixer;
 	if (m == NULL) {
 		PCM_RELEASE_QUICK(d);
 		return (ENODEV);

sys/dev/sound/pcm/channel.c

@@ -2157,8 +2157,7 @@ chn_syncstate(struct pcm_channel *c)
 	struct snd_mixer *m;
 
 	d = (c != NULL) ? c->parentsnddev : NULL;
-	m = (d != NULL && d->mixer_dev != NULL) ? d->mixer_dev->si_drv1 :
-	    NULL;
+	m = (d != NULL) ? d->mixer : NULL;
 
 	if (d == NULL || m == NULL)
 		return;

sys/dev/sound/pcm/dsp.c

@@ -925,7 +925,6 @@ dsp_ioctl(struct cdev *i_dev, unsigned long cmd, caddr_t arg, int mode,
 		{
 	    		snd_capabilities *p = (snd_capabilities *)arg;
 			struct pcmchan_caps *pcaps = NULL, *rcaps = NULL;
-			struct cdev *pdev;
 #ifdef COMPAT_FREEBSD32
 			snd_capabilities32 *p32 = (snd_capabilities32 *)arg;
 			snd_capabilities capabilities;
@@ -965,9 +964,8 @@ dsp_ioctl(struct cdev *i_dev, unsigned long cmd, caddr_t arg, int mode,
 				    (pcm_getflags(d->dev) & SD_F_SIMPLEX) ? 0 :
 				    AFMT_FULLDUPLEX;
 			}
-			pdev = d->mixer_dev;
 	    		p->mixers = 1; /* default: one mixer */
-	    		p->inputs = pdev->si_drv1? mix_getdevs(pdev->si_drv1) : 0;
+			p->inputs = d->mixer ? mix_getdevs(d->mixer) : 0;
 	    		p->left = p->right = 100;
 			if (wrch)
 				CHN_UNLOCK(wrch);

sys/dev/sound/pcm/feeder_volume.c

@@ -270,7 +270,7 @@ feed_volume_feed(struct pcm_feeder *f, struct pcm_channel *c, uint8_t *b,
 
 	/* Check if any controls are muted. */
 	d = (c != NULL) ? c->parentsnddev : NULL;
-	m = (d != NULL && d->mixer_dev != NULL) ? d->mixer_dev->si_drv1 : NULL;
+	m = (d != NULL) ? d->mixer : NULL;
 
 	if (m != NULL)
 		master_muted = (mix_getmutedevs(m) & (1 << SND_VOL_C_MASTER));

sys/dev/sound/pcm/mixer.c

@@ -75,14 +75,14 @@ static struct cdevsw mixer_cdevsw = {
 
 static eventhandler_tag mixer_ehtag = NULL;
 
-static struct cdev *
+static struct snd_mixer *
 mixer_get_devt(device_t dev)
 {
 	struct snddev_info *snddev;
 
 	snddev = device_get_softc(dev);
 
-	return snddev->mixer_dev;
+	return (snddev->mixer);
 }
 
 static int
@@ -616,7 +616,6 @@ mixer_init(device_t dev, kobj_class_t cls, void *devinfo)
 	struct snddev_info *snddev;
 	struct snd_mixer *m;
 	uint16_t v;
-	struct cdev *pdev;
 	const char *name;
 	int i, unit, val;
 
@@ -646,10 +645,7 @@ mixer_init(device_t dev, kobj_class_t cls, void *devinfo)
 
 	mixer_setrecsrc(m, 0); /* Set default input. */
 
-	pdev = make_dev(&mixer_cdevsw, 0, UID_ROOT, GID_AUDIO, 0660, "mixer%d",
-	    unit);
-	pdev->si_drv1 = m;
-	snddev->mixer_dev = pdev;
+	snddev->mixer = m;
 
 	if (bootverbose) {
 		for (i = 0; i < SOUND_MIXER_NRDEVICES; i++) {
@@ -685,20 +681,23 @@ mixer_uninit(device_t dev)
 	int i;
 	struct snddev_info *d;
 	struct snd_mixer *m;
-	struct cdev *pdev;
 
 	d = device_get_softc(dev);
-	pdev = mixer_get_devt(dev);
-	if (d == NULL || pdev == NULL || pdev->si_drv1 == NULL)
+	if (d == NULL)
 		return EBADF;
+	m = d->mixer;
 
-	m = pdev->si_drv1;
 	KASSERT(m != NULL, ("NULL snd_mixer"));
 	KASSERT(m->type == MIXER_TYPE_PRIMARY,
 	    ("%s(): illegal mixer type=%d", __func__, m->type));
 
-	pdev->si_drv1 = NULL;
-	destroy_dev(pdev);
+	/*
+	 * snd_uaudio(4) in particular can call mixer_uninit() directly if
+	 * attach failed prior to pcm_register(), in which case the cdev will
+	 * not have been created. Do not call destroy_dev() unconditionally.
+	*/
+	if (d->mixer_dev != NULL)
+		destroy_dev(d->mixer_dev);
 
 	mtx_lock(&m->lock);
 
@@ -717,6 +716,7 @@ mixer_uninit(device_t dev)
 	kobj_delete((kobj_t)m, M_DEVBUF);
 
 	d->mixer_dev = NULL;
+	d->mixer = NULL;
 
 	return 0;
 }
@@ -725,11 +725,9 @@ int
 mixer_reinit(device_t dev)
 {
 	struct snd_mixer *m;
-	struct cdev *pdev;
 	int i;
 
-	pdev = mixer_get_devt(dev);
-	m = pdev->si_drv1;
+	m = mixer_get_devt(dev);
 	mtx_lock(&m->lock);
 
 	i = MIXER_REINIT(m);
@@ -751,6 +749,32 @@ mixer_reinit(device_t dev)
 	return 0;
 }
 
+int
+mixer_make_dev(device_t dev)
+{
+	struct make_dev_args devargs;
+	struct snddev_info *sc;
+	int err, unit;
+
+	sc = device_get_softc(dev);
+	unit = device_get_unit(dev);
+
+	make_dev_args_init(&devargs);
+	devargs.mda_devsw = &mixer_cdevsw;
+	devargs.mda_uid = UID_ROOT;
+	devargs.mda_gid = GID_AUDIO;
+	devargs.mda_mode = 0660;
+	devargs.mda_si_drv1 = sc->mixer;
+	err = make_dev_s(&devargs, &sc->mixer_dev, "mixer%d", unit);
+	if (err != 0) {
+		device_printf(dev, "failed to create mixer%d: error %d\n",
+		    unit, err);
+		return (err);
+	}
+
+	return (0);
+}
+
 static int
 sysctl_hw_snd_hwvol_mixer(SYSCTL_HANDLER_ARGS)
 {
@@ -781,10 +805,8 @@ int
 mixer_hwvol_init(device_t dev)
 {
 	struct snd_mixer *m;
-	struct cdev *pdev;
 
-	pdev = mixer_get_devt(dev);
-	m = pdev->si_drv1;
+	m = mixer_get_devt(dev);
 
 	m->hwvol_mixer = SOUND_MIXER_VOLUME;
 	m->hwvol_step = 5;
@@ -808,10 +830,8 @@ void
 mixer_hwvol_mute(device_t dev)
 {
 	struct snd_mixer *m;
-	struct cdev *pdev;
 
-	pdev = mixer_get_devt(dev);
-	m = pdev->si_drv1;
+	m = mixer_get_devt(dev);
 	mtx_lock(&m->lock);
 	mixer_hwvol_mute_locked(m);
 	mtx_unlock(&m->lock);
@@ -846,10 +866,8 @@ void
 mixer_hwvol_step(device_t dev, int left_step, int right_step)
 {
 	struct snd_mixer *m;
-	struct cdev *pdev;
 
-	pdev = mixer_get_devt(dev);
-	m = pdev->si_drv1;
+	m = mixer_get_devt(dev);
 	mtx_lock(&m->lock);
 	mixer_hwvol_step_locked(m, left_step, right_step);
 	mtx_unlock(&m->lock);
@@ -927,10 +945,9 @@ mixer_open(struct cdev *i_dev, int flags, int mode, struct thread *td)
 	struct snddev_info *d;
 	struct snd_mixer *m;
 
-	if (i_dev == NULL || i_dev->si_drv1 == NULL)
-		return (EBADF);
-
 	m = i_dev->si_drv1;
+	if (m == NULL)
+		return (EBADF);
 	d = device_get_softc(m->dev);
 	if (!PCM_REGISTERED(d))
 		return (EBADF);
@@ -944,10 +961,9 @@ mixer_close(struct cdev *i_dev, int flags, int mode, struct thread *td)
 	struct snddev_info *d;
 	struct snd_mixer *m;
 
-	if (i_dev == NULL || i_dev->si_drv1 == NULL)
-		return (EBADF);
-
 	m = i_dev->si_drv1;
+	if (m == NULL)
+		return (EBADF);
 	d = device_get_softc(m->dev);
 	if (!PCM_REGISTERED(d))
 		return (EBADF);
@@ -960,12 +976,13 @@ mixer_ioctl(struct cdev *i_dev, unsigned long cmd, caddr_t arg, int mode,
     struct thread *td)
 {
 	struct snddev_info *d;
+	struct snd_mixer *m;
 	int ret;
 
-	if (i_dev == NULL || i_dev->si_drv1 == NULL)
+	m = i_dev->si_drv1;
+	if (m == NULL)
 		return (EBADF);
-
-	d = device_get_softc(((struct snd_mixer *)i_dev->si_drv1)->dev);
+	d = device_get_softc(m->dev);
 	if (!PCM_REGISTERED(d))
 		return (EBADF);
 
@@ -1235,14 +1252,14 @@ mixer_oss_mixerinfo(struct cdev *i_dev, oss_mixerinfo *mi)
 			continue;
 		}
 
-		if (d->mixer_dev->si_drv1 == NULL) {
+		if (d->mixer == NULL) {
 			mixer_oss_mixerinfo_unavail(mi, i);
 			PCM_UNLOCK(d);
 			bus_topo_unlock();
 			return (0);
 		}
 
-		m = d->mixer_dev->si_drv1;
+		m = d->mixer;
 		mtx_lock(&m->lock);
 
 		/*

sys/dev/sound/pcm/mixer.h

@@ -62,6 +62,7 @@ int mixer_delete(struct snd_mixer *m);
 int mixer_init(device_t dev, kobj_class_t cls, void *devinfo);
 int mixer_uninit(device_t dev);
 int mixer_reinit(device_t dev);
+int mixer_make_dev(device_t dev);
 int mixer_ioctl_cmd(struct cdev *i_dev, unsigned long cmd, caddr_t arg,
     int mode, struct thread *td);
 int mixer_oss_mixerinfo(struct cdev *i_dev, oss_mixerinfo *mi);

sys/dev/sound/pcm/sound.c

@@ -332,7 +332,7 @@ sysctl_dev_pcm_mode(SYSCTL_HANDLER_ARGS)
 		mode |= PCM_MODE_PLAY;
 	if (d->reccount > 0)
 		mode |= PCM_MODE_REC;
-	if (d->mixer_dev != NULL)
+	if (d->mixer != NULL)
 		mode |= PCM_MODE_MIXER;
 	PCM_UNLOCK(d);
 
@@ -433,6 +433,13 @@ pcm_register(device_t dev, char *str)
 	err = dsp_make_dev(dev);
 	if (err)
 		return (err);
+	if (d->mixer != NULL) {
+		err = mixer_make_dev(dev);
+		if (err) {
+			dsp_destroy_dev(dev);
+			return (err);
+		}
+	}
 
 	bus_topo_lock();
 	if (snd_unit_auto < 0)
@@ -483,7 +490,8 @@ pcm_unregister(device_t dev)
 	}
 
 	sndstat_unregister(dev);
-	mixer_uninit(dev);
+	if (d->mixer != NULL)
+		mixer_uninit(dev);
 	dsp_destroy_dev(dev);
 
 	cv_destroy(&d->cv);

sys/dev/sound/pcm/sound.h

@@ -191,6 +191,7 @@ struct snddev_info {
 	struct mtx lock;
 	struct cdev *mixer_dev;
 	struct cdev *dsp_dev;
+	struct snd_mixer *mixer;
 	uint32_t pvchanrate, pvchanformat, pvchanmode;
 	uint32_t rvchanrate, rvchanformat, rvchanmode;
 	int32_t eqpreamp;