sound: Merge pcm_chn_create() and chn_init()

Follow-up of b3ea087c05d8c75978a302cbb3fa92ce1afa3e49 ("sound: Merge
pcm_chn_destroy() and chn_kill()")

While here, add device_printf()'s to all failure points. Also fix an
existing bug where we'd unlock an already unlocked channel, in case we
went to "out" (now "out2") before locking the channel.

Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Reviewed by:	dev_submerge.ch
Differential Revision:	https://reviews.freebsd.org/D44993

5 files changed, 151 insertions, 136 deletions

diff --git a/sys/dev/sound/pcm/channel.c b/sys/dev/sound/pcm/channel.c
index 859476f212ae..996cb0cb04f7 100644
--- a/sys/dev/sound/pcm/channel.c
+++ b/sys/dev/sound/pcm/channel.c
@@ -1157,14 +1157,112 @@ chn_reset(struct pcm_channel *c, uint32_t fmt, uint32_t spd)
 	return r;
 }
 
-int
-chn_init(struct pcm_channel *c, void *devinfo, int dir, int direction)
+struct pcm_channel *
+chn_init(struct snddev_info *d, struct pcm_channel *parent, kobj_class_t cls,
+    int dir, int num, void *devinfo)
 {
+	struct pcm_channel *c;
 	struct feeder_class *fc;
 	struct snd_dbuf *b, *bs;
-	int i, ret;
+	char *dirs, *devname, buf[CHN_NAMELEN];
+	int i, ret, direction, rpnum, *pnum, max, type, unit;
 
+	PCM_BUSYASSERT(d);
+	PCM_LOCKASSERT(d);
+	KASSERT(num >= -1, ("invalid num=%d", num));
+
+	switch (dir) {
+	case PCMDIR_PLAY:
+		dirs = "play";
+		direction = PCMDIR_PLAY;
+		pnum = &d->playcount;
+		type = SND_DEV_DSPHW_PLAY;
+		max = SND_MAXHWCHAN;
+		break;
+	case PCMDIR_PLAY_VIRTUAL:
+		dirs = "virtual_play";
+		direction = PCMDIR_PLAY;
+		pnum = &d->pvchancount;
+		type = SND_DEV_DSPHW_VPLAY;
+		max = SND_MAXVCHANS;
+		break;
+	case PCMDIR_REC:
+		dirs = "record";
+		direction = PCMDIR_REC;
+		pnum = &d->reccount;
+		type = SND_DEV_DSPHW_REC;
+		max = SND_MAXHWCHAN;
+		break;
+	case PCMDIR_REC_VIRTUAL:
+		dirs = "virtual_record";
+		direction = PCMDIR_REC;
+		pnum = &d->rvchancount;
+		type = SND_DEV_DSPHW_VREC;
+		max = SND_MAXVCHANS;
+		break;
+	default:
+		device_printf(d->dev,
+		    "%s(): invalid channel direction: %d\n",
+		    __func__, dir);
+		goto out1;
+	}
+
+	unit = (num == -1) ? 0 : num;
+
+	if (*pnum >= max || unit >= max) {
+		device_printf(d->dev, "%s(): unit=%d or pnum=%d >= than "
+		    "max=%d\n", __func__, unit, *pnum, max);
+		goto out1;
+	}
+
+	rpnum = 0;
+
+	CHN_FOREACH(c, d, channels.pcm) {
+		if (c->type != type)
+			continue;
+		if (unit == c->unit && num != -1) {
+			device_printf(d->dev,
+			    "%s(): channel num=%d already allocated\n",
+			    __func__, unit);
+			goto out1;
+		}
+		unit++;
+		if (unit >= max) {
+			device_printf(d->dev,
+			    "%s(): chan=%d >= max=%d\n", __func__, unit, max);
+			goto out1;
+		}
+		rpnum++;
+	}
+
+	if (*pnum != rpnum) {
+		device_printf(d->dev,
+		    "%s(): pnum screwed: dirs=%s pnum=%d rpnum=%d\n",
+		    __func__, dirs, *pnum, rpnum);
+		goto out1;
+	}
+
+	PCM_UNLOCK(d);
+	c = malloc(sizeof(*c), M_DEVBUF, M_WAITOK | M_ZERO);
+	c->methods = kobj_create(cls, M_DEVBUF, M_WAITOK | M_ZERO);
+	c->type = type;
+	c->unit = unit;
+	c->pid = -1;
+	strlcpy(c->comm, CHN_COMM_UNUSED, sizeof(c->comm));
+	c->parentsnddev = d;
+	c->parentchannel = parent;
+	c->dev = d->dev;
+	c->trigger = PCMTRIG_STOP;
 	chn_lockinit(c, dir);
+	devname = dsp_unit2name(buf, sizeof(buf), c);
+	if (devname == NULL) {
+		ret = EINVAL;
+		device_printf(d->dev, "%s(): failed to create channel name",
+		    __func__);
+		goto out2;
+	}
+	snprintf(c->name, sizeof(c->name), "%s:%s:%s",
+	    device_get_nameunit(c->dev), dirs, devname);
 
 	b = NULL;
 	bs = NULL;
@@ -1177,20 +1275,31 @@ chn_init(struct pcm_channel *c, void *devinfo, int dir, int direction)
 
 	ret = ENOMEM;
 	b = sndbuf_create(c->dev, c->name, "primary", c);
-	if (b == NULL)
-		goto out;
+	if (b == NULL) {
+		device_printf(d->dev, "%s(): failed to create hardware buffer\n",
+		    __func__);
+		goto out2;
+	}
 	bs = sndbuf_create(c->dev, c->name, "secondary", c);
-	if (bs == NULL)
-		goto out;
+	if (bs == NULL) {
+		device_printf(d->dev, "%s(): failed to create software buffer\n",
+		    __func__);
+		goto out2;
+	}
 
 	CHN_LOCK(c);
 
 	ret = EINVAL;
 	fc = feeder_getclass(NULL);
-	if (fc == NULL)
-		goto out;
-	if (chn_addfeeder(c, fc, NULL))
-		goto out;
+	if (fc == NULL) {
+		device_printf(d->dev, "%s(): failed to get feeder class\n",
+		    __func__);
+		goto out2;
+	}
+	if (chn_addfeeder(c, fc, NULL)) {
+		device_printf(d->dev, "%s(): failed to add feeder\n", __func__);
+		goto out2;
+	}
 
 	/*
 	 * XXX - sndbuf_setup() & sndbuf_resize() expect to be called
@@ -1226,12 +1335,17 @@ chn_init(struct pcm_channel *c, void *devinfo, int dir, int direction)
 	CHN_UNLOCK(c); /* XXX - Unlock for CHANNEL_INIT() malloc() call */
 	c->devinfo = CHANNEL_INIT(c->methods, devinfo, b, c, direction);
 	CHN_LOCK(c);
-	if (c->devinfo == NULL)
-		goto out;
+	if (c->devinfo == NULL) {
+		device_printf(d->dev, "%s(): NULL devinfo\n", __func__);
+		goto out2;
+	}
 
 	ret = ENOMEM;
-	if ((sndbuf_getsize(b) == 0) && ((c->flags & CHN_F_VIRTUAL) == 0))
-		goto out;
+	if ((sndbuf_getsize(b) == 0) && ((c->flags & CHN_F_VIRTUAL) == 0)) {
+		device_printf(d->dev, "%s(): hardware buffer's size is 0\n",
+		    __func__);
+		goto out2;
+	}
 
 	ret = 0;
 	c->direction = direction;
@@ -1251,12 +1365,14 @@ chn_init(struct pcm_channel *c, void *devinfo, int dir, int direction)
 		bs->shadbuf = malloc(bs->sl, M_DEVBUF, M_NOWAIT);
 		if (bs->shadbuf == NULL) {
 			ret = ENOMEM;
-			goto out;
+			device_printf(d->dev, "%s(): failed to create shadow "
+			    "buffer\n", __func__);
+			goto out2;
 		}
 	}
-
-out:
-	CHN_UNLOCK(c);
+out2:
+	if (CHN_LOCKOWNED(c))
+		CHN_UNLOCK(c);
 	if (ret) {
 		if (c->devinfo) {
 			if (CHANNEL_FREE(c->methods, c->devinfo))
@@ -1270,10 +1386,19 @@ out:
 		c->flags |= CHN_F_DEAD;
 		chn_lockdestroy(c);
 
-		return ret;
+		PCM_LOCK(d);
+
+		kobj_delete(c->methods, M_DEVBUF);
+		free(c, M_DEVBUF);
+
+		return (NULL);
 	}
 
-	return 0;
+	PCM_LOCK(d);
+
+	return (c);
+out1:
+	return (NULL);
 }
 
 void
diff --git a/sys/dev/sound/pcm/channel.h b/sys/dev/sound/pcm/channel.h
index 698a1186924f..6aca089a7c4d 100644
--- a/sys/dev/sound/pcm/channel.h
+++ b/sys/dev/sound/pcm/channel.h
@@ -259,7 +259,8 @@ int chn_sync(struct pcm_channel *c, int threshold);
 int chn_flush(struct pcm_channel *c);
 int chn_poll(struct pcm_channel *c, int ev, struct thread *td);
 
-int chn_init(struct pcm_channel *c, void *devinfo, int dir, int direction);
+struct pcm_channel *chn_init(struct snddev_info *d, struct pcm_channel *parent,
+    kobj_class_t cls, int dir, int num, void *devinfo);
 void chn_kill(struct pcm_channel *c);
 void chn_shutdown(struct pcm_channel *c);
 int chn_release(struct pcm_channel *c);
diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index 9d5eaf3f5ad7..cc19d119ac36 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -379,116 +379,6 @@ SYSCTL_PROC(_hw_snd, OID_AUTO, maxautovchans,
     sysctl_hw_snd_maxautovchans, "I",
     "maximum virtual channel");
 
-struct pcm_channel *
-pcm_chn_create(struct snddev_info *d, struct pcm_channel *parent, kobj_class_t cls, int dir, int num, void *devinfo)
-{
-	struct pcm_channel *ch;
-	int direction, err, rpnum, *pnum, max;
-	int type, unit;
-	char *dirs, *devname, buf[CHN_NAMELEN];
-
-	PCM_BUSYASSERT(d);
-	PCM_LOCKASSERT(d);
-	KASSERT(num >= -1, ("invalid num=%d", num));
-
-	switch (dir) {
-	case PCMDIR_PLAY:
-		dirs = "play";
-		direction = PCMDIR_PLAY;
-		pnum = &d->playcount;
-		type = SND_DEV_DSPHW_PLAY;
-		max = SND_MAXHWCHAN;
-		break;
-	case PCMDIR_PLAY_VIRTUAL:
-		dirs = "virtual_play";
-		direction = PCMDIR_PLAY;
-		pnum = &d->pvchancount;
-		type = SND_DEV_DSPHW_VPLAY;
-		max = SND_MAXVCHANS;
-		break;
-	case PCMDIR_REC:
-		dirs = "record";
-		direction = PCMDIR_REC;
-		pnum = &d->reccount;
-		type = SND_DEV_DSPHW_REC;
-		max = SND_MAXHWCHAN;
-		break;
-	case PCMDIR_REC_VIRTUAL:
-		dirs = "virtual_record";
-		direction = PCMDIR_REC;
-		pnum = &d->rvchancount;
-		type = SND_DEV_DSPHW_VREC;
-		max = SND_MAXVCHANS;
-		break;
-	default:
-		return (NULL);
-	}
-
-	unit = (num == -1) ? 0 : num;
-
-	if (*pnum >= max || unit >= max)
-		return (NULL);
-
-	rpnum = 0;
-
-	CHN_FOREACH(ch, d, channels.pcm) {
-		if (ch->type != type)
-			continue;
-		if (unit == ch->unit && num != -1) {
-			device_printf(d->dev,
-			    "channel num=%d allocated!\n", unit);
-			return (NULL);
-		}
-		unit++;
-		if (unit >= max) {
-			device_printf(d->dev,
-			    "chan=%d > %d\n", unit, max);
-			return (NULL);
-		}
-		rpnum++;
-	}
-
-	if (*pnum != rpnum) {
-		device_printf(d->dev,
-		    "%s(): WARNING: pnum screwed : dirs=%s pnum=%d rpnum=%d\n",
-		    __func__, dirs, *pnum, rpnum);
-		return (NULL);
-	}
-
-	PCM_UNLOCK(d);
-	ch = malloc(sizeof(*ch), M_DEVBUF, M_WAITOK | M_ZERO);
-	ch->methods = kobj_create(cls, M_DEVBUF, M_WAITOK | M_ZERO);
-	ch->type = type;
-	ch->unit = unit;
-	ch->pid = -1;
-	strlcpy(ch->comm, CHN_COMM_UNUSED, sizeof(ch->comm));
-	ch->parentsnddev = d;
-	ch->parentchannel = parent;
-	ch->dev = d->dev;
-	ch->trigger = PCMTRIG_STOP;
-	devname = dsp_unit2name(buf, sizeof(buf), ch);
-	if (devname == NULL) {
-		device_printf(d->dev, "Failed to query device name");
-		kobj_delete(ch->methods, M_DEVBUF);
-		free(ch, M_DEVBUF);
-		return (NULL);
-	}
-	snprintf(ch->name, sizeof(ch->name), "%s:%s:%s",
-	    device_get_nameunit(ch->dev), dirs, devname);
-
-	err = chn_init(ch, devinfo, dir, direction);
-	PCM_LOCK(d);
-	if (err) {
-		device_printf(d->dev, "chn_init(%s) failed: err = %d\n",
-		    ch->name, err);
-		kobj_delete(ch->methods, M_DEVBUF);
-		free(ch, M_DEVBUF);
-		return (NULL);
-	}
-
-	return (ch);
-}
-
 int
 pcm_chn_add(struct snddev_info *d, struct pcm_channel *ch)
 {
@@ -569,9 +459,9 @@ pcm_addchan(device_t dev, int dir, kobj_class_t cls, void *devinfo)
 	PCM_BUSYASSERT(d);
 
 	PCM_LOCK(d);
-	ch = pcm_chn_create(d, NULL, cls, dir, -1, devinfo);
+	ch = chn_init(d, NULL, cls, dir, -1, devinfo);
 	if (!ch) {
-		device_printf(d->dev, "pcm_chn_create(%s, %d, %p) failed\n",
+		device_printf(d->dev, "chn_init(%s, %d, %p) failed\n",
 		    cls->name, dir, devinfo);
 		PCM_UNLOCK(d);
 		return (ENODEV);
diff --git a/sys/dev/sound/pcm/sound.h b/sys/dev/sound/pcm/sound.h
index 08aa56cc96f7..09e739ea5c20 100644
--- a/sys/dev/sound/pcm/sound.h
+++ b/sys/dev/sound/pcm/sound.h
@@ -297,7 +297,6 @@ int pcm_setvchans(struct snddev_info *d, int direction, int newcnt, int num);
 int pcm_chnalloc(struct snddev_info *d, struct pcm_channel **ch, int direction,
     pid_t pid, char *comm);
 
-struct pcm_channel *pcm_chn_create(struct snddev_info *d, struct pcm_channel *parent, kobj_class_t cls, int dir, int num, void *devinfo);
 int pcm_chn_add(struct snddev_info *d, struct pcm_channel *ch);
 int pcm_chn_remove(struct snddev_info *d, struct pcm_channel *ch);
 
diff --git a/sys/dev/sound/pcm/vchan.c b/sys/dev/sound/pcm/vchan.c
index c3bc36d924bd..ccac08891220 100644
--- a/sys/dev/sound/pcm/vchan.c
+++ b/sys/dev/sound/pcm/vchan.c
@@ -698,7 +698,7 @@ vchan_create(struct pcm_channel *parent, int num)
 	}
 
 	/* create a new playback channel */
-	ch = pcm_chn_create(d, parent, &vchan_class, direction, num, parent);
+	ch = chn_init(d, parent, &vchan_class, direction, num, parent);
 	if (ch == NULL) {
 		PCM_UNLOCK(d);
 		CHN_LOCK(parent);